5 ms·

Wasn't it the case that the requirements were the same at the time the code was written? TextBlock.resizeTopLeft(...) and Rectangle.resizeTopLeft(...) did the s

by leoc 7y ago

Wasn't it the case that the requirements were the same at the time the code was written? TextBlock.resizeTopLeft(...) and Rectangle.resizeTopLeft(...) did the same thing for the same reason, not by coincidence. The issue was the possiblity (and eventually the reality) of future divergence.

BeetleB 7y ago
I disagree. If they had separate classes for TextBlock and Rectangle, then likely there are separate requirements involved. It may have been that at the time they needed separate classes the requirements for resizeTopLeft was identical, but they are still two separate requirements. I suggest everyone take a minicourse or read a book on systems engineering - at least the part where they discuss requirements. The fact that two distinct entities have an identical requirement for something does not mean there is one requirement. It means there are two requirements which (for now) happen to be identical. The key phrase is happen to be. You do not apply DRY to separate requirements even if they are the same/similar. Look at it from a testing perspective[1], which is similar to how a systems engineer will look at it. If you would write a separate test for it, it is a distinct requirement. (Actually, the ordering should be the opposite: If it is a separate requirement, it should have a separate test). [1] And not from a TDD perspective!
- _____s 7y ago
  > I suggest everyone take a minicourse or read a book on systems engineering - at least the part where they discuss requirements. The fact that two distinct entities have an identical requirement for something does not mean there is one requirement. It means there are two requirements which (for now) happen to be identical. I'd love to do this -- do you have any recommendations for a book like this?
  - BeetleB 7y ago
    Not really. I took it as part of an inhouse training at work.
thefifthsetpin 7y ago
The issue wasn't divergence. The issue was that there was no requirement that TextBlocks should behave like rectangles. It's very possible that DRY could apply even where divergence is expected. Suppose we had a requirement that "A user should not be able to resize a TextBlock if they are not allowed to read the text." At the outset, the read & resize permission logic might be identical & DRY is easy to apply. Nobody would be surprised if eventually resizing permissions became more restrictive. Even at that point, DRY applies. If you repeated the shared logic, and later you add new restrictions to the read permissions, it'd be easy to introduce a bug where you forget to restrict resizing permissions accordingly.
- dragonwriter 7y ago
  > The issue wasn't divergence. The issue was that there was no requirement that TextBlocks should behave like rectangles. But they had the same current requirements. So, ideally, the implementation of the common behavior should have been shared but shared in a way which didn't tightly couple the two consumers with each other, so that it would be simple for either to stop using the shared code.
- mattkrause 7y ago
  > The issue was that there was no requirement that TextBlocks should behave like rectangles. Sure, though that’s not quite what the new code did. Instead, it added a new abstraction (createBox), and then used that to incidentally assign the same behavior to TextBlocks and Rectangles. Your change could easily be accomplished by changing the last line to check the permissions and call createBox or something else, depending. I’d guess that createBox would also be useful for many other shapes, but if you wanted to do something totally different, like a star with handles at each point, you could just....not call createBox, and write a totally independent “let star()” function that does its own thing; it doesn’t seem like a base class constructor is forcing you to call createBox or anything. So....this code doesn’t actually seem that bad to me. What am I missing?

by leoc 7y ago

BeetleB 7y ago

I disagree. If they had separate classes for TextBlock and Rectangle, then likely there are separate requirements involved. It may have been that at the time they needed separate classes the requirements for resizeTopLeft was identical, but they are still two separate requirements. I suggest everyone take a minicourse or read a book on systems engineering - at least the part where they discuss requirements. The fact that two distinct entities have an identical requirement for something does not mean there is one requirement. It means there are two requirements which (for now) happen to be identical. The key phrase is happen to be. You do not apply DRY to separate requirements even if they are the same/similar. Look at it from a testing perspective[1], which is similar to how a systems engineer will look at it. If you would write a separate test for it, it is a distinct requirement. (Actually, the ordering should be the opposite: If it is a separate requirement, it should have a separate test). [1] And not from a TDD perspective!

_____s 7y ago
> I suggest everyone take a minicourse or read a book on systems engineering - at least the part where they discuss requirements. The fact that two distinct entities have an identical requirement for something does not mean there is one requirement. It means there are two requirements which (for now) happen to be identical. I'd love to do this -- do you have any recommendations for a book like this?
- BeetleB 7y ago
  Not really. I took it as part of an inhouse training at work.

thefifthsetpin 7y ago

The issue wasn't divergence. The issue was that there was no requirement that TextBlocks should behave like rectangles. It's very possible that DRY could apply even where divergence is expected. Suppose we had a requirement that "A user should not be able to resize a TextBlock if they are not allowed to read the text." At the outset, the read & resize permission logic might be identical & DRY is easy to apply. Nobody would be surprised if eventually resizing permissions became more restrictive. Even at that point, DRY applies. If you repeated the shared logic, and later you add new restrictions to the read permissions, it'd be easy to introduce a bug where you forget to restrict resizing permissions accordingly.

dragonwriter 7y ago
> The issue wasn't divergence. The issue was that there was no requirement that TextBlocks should behave like rectangles. But they had the same current requirements. So, ideally, the implementation of the common behavior should have been shared but shared in a way which didn't tightly couple the two consumers with each other, so that it would be simple for either to stop using the shared code.
mattkrause 7y ago
> The issue was that there was no requirement that TextBlocks should behave like rectangles. Sure, though that’s not quite what the new code did. Instead, it added a new abstraction (createBox), and then used that to incidentally assign the same behavior to TextBlocks and Rectangles. Your change could easily be accomplished by changing the last line to check the permissions and call createBox or something else, depending. I’d guess that createBox would also be useful for many other shapes, but if you wanted to do something totally different, like a star with handles at each point, you could just....not call createBox, and write a totally independent “let star()” function that does its own thing; it doesn’t seem like a base class constructor is forcing you to call createBox or anything. So....this code doesn’t actually seem that bad to me. What am I missing?