4 ms·

I have a current example where this bit my team. A fairly common pattern that I've seen over and over in multiple domains is this: Given a group of of "th

I have a current example where this bit my team. A fairly common pattern that I've seen over and over in multiple domains is this: Given a group of of "things" with a start and stop date, list all the things that are "active" during a given date range. Some one abstracted it because we have several "things" that use this logic. Then it had a bug because some of the things are inclusive and some are exclusive. Then it had a bug because some of the things use dates and some timestamps. Then it had a bug because some of the things are timezone aware and some are not. So we started down the path of a rather simple query construction becoming a complex thing with flags for inclusive/exclusive for start and end, timezone settings ...

alphanumeric0 3y ago
Sounds like each thing should know how to search for active instances of itself, given a date range, which is a common OO abstraction.
- chiefbucket 3y ago
  The point isn’t the interface though it’s the implementation. And if many of those things are implementing the same search functionality slightly differently, you’re back to the same spot, except now your bugs are spread across multiple sites, often with duplication. The underlying issue is just that correctness is hard I think.
- marcosdumay 3y ago
  One should at minimum name the things that behave differently by different names, what is a common practice in data modeling. I expect all those bugs to return again and again as different people maintain that code. At least with code deduplication they would have a clear alarm telling them their knowledge is wrong and they must pay attention. But with each query doing everything people will just assume they know it all.
IshKebab 3y ago
That sounds like your code just isn't properly typed. For example in Rust the first bug would be caught by `Range` vs `RangeInclusive`. The second bug would trivially be caught because dates and timestamps are different types. The third is trickier, but (depending on exactly what you mean) that can be caught with static types too. Pointing your finger in the wrong place IMO. If anything this refactoring highlighted worrying inconsistencies in your code that probably would have cropped up as bugs elsewhere.
williamdclt 3y ago
> So we started down the path of a rather simple query construction becoming a complex thing with flags for inclusive/exclusive for start and end, timezone settings ... Forcing the caller to _think_ about inclusivity and timezone awareness is not a bad thing, rather the opposite. These are important decisions to be taken: the abstraction is not trivial because what it abstracts actually does have inherent complexity. If the abstraction forces you to take the necessary decisions (inclusive? timezone?) without having to think of how to implement them, it doesn't sound like a bad abstraction. Too often these decisions are not thought about, and the expected behaviour is "whatever is implemented".
willio58 3y ago
Great example. One way to avoid these problems is having lots of tests written for the various uses of the abstracted thing so you know they’re all covered. But also, if all of these things function in different nuanced ways, is it really any benefit to have them all jammed into the same abstraction in the first place? I’ve found this comes down to personal taste. I prefer a little duplication if it means not having to “own” an abstraction that I’ll need to heavily document and hope people read the documentation for in order to not break. But some would rather own the one point of failure.
wvenable 3y ago
Who knows whether some things are inclusive or not? Who knows what use dates and timestamps? It seems like this should be abstracted somewhere and this knowledge codified in one single place. It sounds like your abstraction, in this case, isn't very abstract at all. That is common for bad abstractions -- they add a layer but they don't actually encapsulate any knowledge. To use this abstraction, you shouldn't be passing any flags for inclusive/exclusive, etc -- it should know that for you.

alphanumeric0 3y ago

Sounds like each thing should know how to search for active instances of itself, given a date range, which is a common OO abstraction.

chiefbucket 3y ago
The point isn’t the interface though it’s the implementation. And if many of those things are implementing the same search functionality slightly differently, you’re back to the same spot, except now your bugs are spread across multiple sites, often with duplication. The underlying issue is just that correctness is hard I think.
marcosdumay 3y ago
One should at minimum name the things that behave differently by different names, what is a common practice in data modeling. I expect all those bugs to return again and again as different people maintain that code. At least with code deduplication they would have a clear alarm telling them their knowledge is wrong and they must pay attention. But with each query doing everything people will just assume they know it all.

IshKebab 3y ago

That sounds like your code just isn't properly typed. For example in Rust the first bug would be caught by `Range` vs `RangeInclusive`. The second bug would trivially be caught because dates and timestamps are different types. The third is trickier, but (depending on exactly what you mean) that can be caught with static types too. Pointing your finger in the wrong place IMO. If anything this refactoring highlighted worrying inconsistencies in your code that probably would have cropped up as bugs elsewhere.

williamdclt 3y ago

> So we started down the path of a rather simple query construction becoming a complex thing with flags for inclusive/exclusive for start and end, timezone settings ... Forcing the caller to _think_ about inclusivity and timezone awareness is not a bad thing, rather the opposite. These are important decisions to be taken: the abstraction is not trivial because what it abstracts actually does have inherent complexity. If the abstraction forces you to take the necessary decisions (inclusive? timezone?) without having to think of how to implement them, it doesn't sound like a bad abstraction. Too often these decisions are not thought about, and the expected behaviour is "whatever is implemented".

willio58 3y ago

Great example. One way to avoid these problems is having lots of tests written for the various uses of the abstracted thing so you know they’re all covered. But also, if all of these things function in different nuanced ways, is it really any benefit to have them all jammed into the same abstraction in the first place? I’ve found this comes down to personal taste. I prefer a little duplication if it means not having to “own” an abstraction that I’ll need to heavily document and hope people read the documentation for in order to not break. But some would rather own the one point of failure.

wvenable 3y ago

Who knows whether some things are inclusive or not? Who knows what use dates and timestamps? It seems like this should be abstracted somewhere and this knowledge codified in one single place. It sounds like your abstraction, in this case, isn't very abstract at all. That is common for bad abstractions -- they add a layer but they don't actually encapsulate any knowledge. To use this abstraction, you shouldn't be passing any flags for inclusive/exclusive, etc -- it should know that for you.