16 ms·

My favorite example of code you should almost always[0] duplicate are view classes (in MVC terminology). Instead, I see people subclassing views to change thei

My favorite example of code you should almost always[0] duplicate are view classes (in MVC terminology). Instead, I see people subclassing views to change their behavior and dealing with absolutely mind-numbing behavioral bugs in their subclass. Furthermore, the subclass's implementation is totally dependent on the _exact_ implementation of the superclass. Change it, and you get a whole crop of new, hard to debug, bugs. Solution? When you have a view class that almost, but not quite, does what you want, copy the entire class, rename it, and make it do what you want. You'll be done before your next break, and nothing that happens anywhere else will cause it to break in the future. [0] The one exception are situations where all you are doing is configuring an existing view class in code and/or adding behavior in places that were _specifically_ anticipated by the class in question. For example, setting up the options on an existing table view class, and responding to row selections using a method meant to be overridden by subclasses. But if your table view is actually different than the class you've got now—for example, it does something strange in responses to selections—you're much better off finding an open source replacement for the table view (if it's vendor-provided, like UITableView in iOS), copying it, renaming, and modifying it to do exactly what you need to it do.

gnaritas 10y ago
> Instead, I see people subclassing views to change their behavior and dealing with absolutely mind-numbing behavioral bugs in their subclass. Because they're breaking a rule of good design, implementations should depend on abstractions, not other implementations. Never subclass a concrete class, only abstract classes. > Solution? When you have a view class that almost, but not quite, does what you want, copy the entire class, rename it, and make it do what you want. Better solution, move the duplicate code into a common abstract superclass. Allow the views to differ where they're concrete and keep the common code abstract in the superclass. Then you can't ever break one implementation by changing another implementation, your core complaint.
- TeMPOraL 10y ago
  > Better solution, move the duplicate code into a common abstract superclass. Allow the views to differ where they're concrete and keep the common code abstract in the superclass. Then you can't ever break one implementation by changing another implementation, your core complaint. And then you suddenly find yourself having a "god object", a class that does huge amount of things and most of them are not related to anything else in the class - they just exist to facilitate code deduplication lower in the inheritance tree. And then, one page really needs a different version of a behaviour that's common to many of them, and you're back to either duplicating or polluting the "god object" even more. Inheritance is a bad tool for this problem. It's better to separate different pieces of functionality and then compose them in views. If your view needs a component behaving in a slightly different way, then you can just create a new component (or even subclass the old one, if you must).
  - gnaritas 10y ago
    > And then you suddenly find yourself having a "god object", a class that does huge amount of things and most of them are not related to anything else in the class - they just exist to facilitate code deduplication lower in the inheritance tree. No, you don't. Point of fact if most of the subclasses are duplicating some code, it does relate to what the superclass is and that code belongs there. This is not a god object. > And then, one page really needs a different version of a behaviour that's common to many of them, and you're back to either duplicating or polluting the "god object" even more. That's what overriding is for. > Inheritance is a bad tool for this problem. It's better to separate different pieces of functionality and then compose them in views. If your view needs a component behaving in a slightly different way, then you can just create a new component (or even subclass the old one, if you must). If composition is possible, it's always a better choice than inheritance of course; when it isn't, a shallow inheritance hierarchy is fine. Deep hierarchies are never ok. > (or even subclass the old one, if you must). No, you don't subclass concrete implementations to change them, that leads back to the whole problem you're complaining about. Implementations should never descend from other implementations.
- thangalin 10y ago
  Exactly this. At Abebooks[1], they had a Servlet called "AbeServlet" that was duplicated for every new page that needed behaviour. By the time I arrived there were close to 200 different servlets all copied from the same "template". Eventually a new requirement came: we need to support i18n because of a merger with European counter-parts. French, British English, Spanish, and German. Hundreds of servlets had to change. Took about two weeks of coding by several developers. When I arrived, I created "AbeMetaServlet" (bad name, but good idea). All my servlets extended from it, as it contained common behaviour. To make my pages i18nized, I changed something like a dozen lines of code and was done in less than an hour--about 16 servlets, if I recall. I didn't know about the upcoming merger. I simply despise duplicating code. There was another example at that shop where they had processes that started up, ran to completion, then paused until time for the next run. All the batch processes had been copied from a template with no thought to the future. I was tasked with updating a batch job for associating pictures of book covers with inventory that vendors uploaded. Rather than copy/paste, I rolled my own and fixed a bug. (There wasn't any code that could be re-used, architecturally.) The bug added several hours of run time to the picture maintenance process. The first task each batch job performed was to fetch a list of 10,000 vendor names over an abysmally slow network drive. Undoubtedly this bug was copy/pasted into the other processes. I re-wrote the code to request the vendor names from the database--and only open up directories for valid vendors. The code, which originally ran in something like 12 hours, was reduced to 8 minutes. All the batch jobs likely suffered from this error. Including the one that took almost 24 hours to run. Anyone know what happens when a batch job needs more than 24 hours to finish? Anyway. Had a proper design been developed at the start, one simple change would have made all the batch processes faster. [1]: http://www.abebooks.com/ http://www.abebooks.com/
theodorton 10y ago
Been bitten by this in React using ES6 where we would sub-class components and override render to provide another layout (or whatever; change behavior). The right way to do it in React is to define a new Component and make the Component you'd like to depend on wrap around the returned JSX in render instead. Probably not a revolutionary paradigm, but it has made me better at reasoning about and structuring React components.
carterehsmith 10y ago
That might work if you have a very small app (example: HN, Reddit, Twitter web client etc) Imagine, on the other hand, large-ish web app (example: Salesforce, Netsuite). 300 tables in the database. Perhaps 50 of those can be manipulated directly from the menu. Customers, Purchase orders, Quotes, Reports, plus all the supporting stuff like Countries, States, Currencies and so on and so forth. Each of those need at least CRUD, plus printing, paging, searching -- are you really going to copy-paste that 50 times? And if there is a bug in any of those, you now need to fix it in 50 places. Who has time for that Or, say, one day you want to add "Export" functionality to all of those. If you have a nice inheritance hierarchy (or composition or whatever), you add it in one place. You are done. If you were copy-pasting, then you do it in 50 places? So no, that copy-pasting business does not work beyond smallish apps..
- erichocean 10y ago
  > Each of those need at least CRUD, plus printing, paging, searching -- are you really going to copy-paste that 50 times? In MVC terms, that's either (a) the M, or (b) the C. Not the V. I suggested not trying to fight the (hidden) behavior in a superclass when working on the V layer. I stand by that. I've also advised some extremely large projects (over 5000 unique screens). My advice scales really well. You can do whatever the hell you want on small (toy) projects.
  - carterehsmith 10y ago
    >> In MVC terms, that's either (a) the M, or (b) the C. Not the V. Wait, what? How is the user supposed to invoke CRUD ops or printing or searching without a V? I don't think we are talking about same things at all. In fact all of these operation span M, V and C. It's not anything new either it has been around at least since Smalltak (in current terminology).

gnaritas 10y ago

> Instead, I see people subclassing views to change their behavior and dealing with absolutely mind-numbing behavioral bugs in their subclass. Because they're breaking a rule of good design, implementations should depend on abstractions, not other implementations. Never subclass a concrete class, only abstract classes. > Solution? When you have a view class that almost, but not quite, does what you want, copy the entire class, rename it, and make it do what you want. Better solution, move the duplicate code into a common abstract superclass. Allow the views to differ where they're concrete and keep the common code abstract in the superclass. Then you can't ever break one implementation by changing another implementation, your core complaint.

TeMPOraL 10y ago
> Better solution, move the duplicate code into a common abstract superclass. Allow the views to differ where they're concrete and keep the common code abstract in the superclass. Then you can't ever break one implementation by changing another implementation, your core complaint. And then you suddenly find yourself having a "god object", a class that does huge amount of things and most of them are not related to anything else in the class - they just exist to facilitate code deduplication lower in the inheritance tree. And then, one page really needs a different version of a behaviour that's common to many of them, and you're back to either duplicating or polluting the "god object" even more. Inheritance is a bad tool for this problem. It's better to separate different pieces of functionality and then compose them in views. If your view needs a component behaving in a slightly different way, then you can just create a new component (or even subclass the old one, if you must).
- gnaritas 10y ago
  > And then you suddenly find yourself having a "god object", a class that does huge amount of things and most of them are not related to anything else in the class - they just exist to facilitate code deduplication lower in the inheritance tree. No, you don't. Point of fact if most of the subclasses are duplicating some code, it does relate to what the superclass is and that code belongs there. This is not a god object. > And then, one page really needs a different version of a behaviour that's common to many of them, and you're back to either duplicating or polluting the "god object" even more. That's what overriding is for. > Inheritance is a bad tool for this problem. It's better to separate different pieces of functionality and then compose them in views. If your view needs a component behaving in a slightly different way, then you can just create a new component (or even subclass the old one, if you must). If composition is possible, it's always a better choice than inheritance of course; when it isn't, a shallow inheritance hierarchy is fine. Deep hierarchies are never ok. > (or even subclass the old one, if you must). No, you don't subclass concrete implementations to change them, that leads back to the whole problem you're complaining about. Implementations should never descend from other implementations.
thangalin 10y ago
Exactly this. At Abebooks[1], they had a Servlet called "AbeServlet" that was duplicated for every new page that needed behaviour. By the time I arrived there were close to 200 different servlets all copied from the same "template". Eventually a new requirement came: we need to support i18n because of a merger with European counter-parts. French, British English, Spanish, and German. Hundreds of servlets had to change. Took about two weeks of coding by several developers. When I arrived, I created "AbeMetaServlet" (bad name, but good idea). All my servlets extended from it, as it contained common behaviour. To make my pages i18nized, I changed something like a dozen lines of code and was done in less than an hour--about 16 servlets, if I recall. I didn't know about the upcoming merger. I simply despise duplicating code. There was another example at that shop where they had processes that started up, ran to completion, then paused until time for the next run. All the batch processes had been copied from a template with no thought to the future. I was tasked with updating a batch job for associating pictures of book covers with inventory that vendors uploaded. Rather than copy/paste, I rolled my own and fixed a bug. (There wasn't any code that could be re-used, architecturally.) The bug added several hours of run time to the picture maintenance process. The first task each batch job performed was to fetch a list of 10,000 vendor names over an abysmally slow network drive. Undoubtedly this bug was copy/pasted into the other processes. I re-wrote the code to request the vendor names from the database--and only open up directories for valid vendors. The code, which originally ran in something like 12 hours, was reduced to 8 minutes. All the batch jobs likely suffered from this error. Including the one that took almost 24 hours to run. Anyone know what happens when a batch job needs more than 24 hours to finish? Anyway. Had a proper design been developed at the start, one simple change would have made all the batch processes faster. [1]: http://www.abebooks.com/ http://www.abebooks.com/

theodorton 10y ago

Been bitten by this in React using ES6 where we would sub-class components and override render to provide another layout (or whatever; change behavior). The right way to do it in React is to define a new Component and make the Component you'd like to depend on wrap around the returned JSX in render instead. Probably not a revolutionary paradigm, but it has made me better at reasoning about and structuring React components.

carterehsmith 10y ago

That might work if you have a very small app (example: HN, Reddit, Twitter web client etc) Imagine, on the other hand, large-ish web app (example: Salesforce, Netsuite). 300 tables in the database. Perhaps 50 of those can be manipulated directly from the menu. Customers, Purchase orders, Quotes, Reports, plus all the supporting stuff like Countries, States, Currencies and so on and so forth. Each of those need at least CRUD, plus printing, paging, searching -- are you really going to copy-paste that 50 times? And if there is a bug in any of those, you now need to fix it in 50 places. Who has time for that Or, say, one day you want to add "Export" functionality to all of those. If you have a nice inheritance hierarchy (or composition or whatever), you add it in one place. You are done. If you were copy-pasting, then you do it in 50 places? So no, that copy-pasting business does not work beyond smallish apps..

erichocean 10y ago
> Each of those need at least CRUD, plus printing, paging, searching -- are you really going to copy-paste that 50 times? In MVC terms, that's either (a) the M, or (b) the C. Not the V. I suggested not trying to fight the (hidden) behavior in a superclass when working on the V layer. I stand by that. I've also advised some extremely large projects (over 5000 unique screens). My advice scales really well. You can do whatever the hell you want on small (toy) projects.
- carterehsmith 10y ago
  >> In MVC terms, that's either (a) the M, or (b) the C. Not the V. Wait, what? How is the user supposed to invoke CRUD ops or printing or searching without a V? I don't think we are talking about same things at all. In fact all of these operation span M, V and C. It's not anything new either it has been around at least since Smalltak (in current terminology).