14 ms·

Don't be stupid: Grasp SOLID

39 points by nikic 15y ago

z0r 15y ago
I have no idea how the 'optimization' example could be considered as such. The advice to not engage in premature optimization is often repeated, but it's not really illustrated here. No comment on the rest of the article.
deleted 15y ago
[deleted]
fleitz 15y ago
"But honestly: Most of the code being written around the globe is an unmaintainable, unreusable mess." Yeah it is, and that's completely fine because most software is written to solve a particular problem and doesn't need to be reused. SOLID and GRASP have great applications but honestly for most software YAGNI is a much better principle to follow. If you're building libraries, or frameworks SOLID and GRASP are essential, if you're building applications your software is likely mostly glue. I don't worry too much about reusing glue. This is what you get when you folow SOLID and GRASP all the time: http://publib.boulder.ibm.com/infocenter/cicsts/v2r3/index.jsp?topic=/com.ibm.cics.ts23.doc/dfhpj/dfhpj7b.htm http://publib.boulder.ibm.com/infocenter/cicsts/v2r3/index.j... All you really need to know about GRASP is that it advocates the factory anti-pattern. If you're using the factory pattern you've likely chosen a language that couldn't figure out the separation of concerns between allocation and initialization and has STUPID baked into the core of it.
- AndrewDucker 15y ago
  Factories are a perfectly good solution to the problem of "I have numerous possible ways of carrying out task X, and I want to use one of them that will not be specified until the application is running." So if I want to write some input to a user-defined place then giving the user the list of options and then passing the option to a WriterFactory to give me the correct Writer makes sense. You have to put the choice of which Writer to use in different scenarios somewhere, so why not a class by itself?
  - Bjartr 15y ago
    Why not an if statement?
    - AndrewDucker 15y ago
      Ok, but where does the if statement go, particularly if it's reused in a few places, from different classes? Putting it into a util class by itself seems like a reasonable thing to do in that case.
  - fleitz 15y ago
    Factories are a good way to solve the problem of not being able to pass a function that creates an object. They're generally an OO kludge around passing an anonymous function, because for some reason passing a function needs to be insanely difficult. And so another class is created to manage the thing that the constructor was supposed to do in the first place (manage the creation of objects) but if you called it the constructor pattern they'd have to admit the language was broken. OO languages that support passing functions generally don't have very many instances of the factory pattern, and languages that have abstracted allocation of memory and initialization of memory have even fewer instances of it. That's why I consider the factory pattern to be an anti-pattern; you've chosen a language that's ill suited to the task you are solving and instead of fixing the real problem (can't pass functions that create objects in a reasonable amount of effort) you're inventing kludges. I've only seen a handful of cases where accepting a parameter of type () -> T or x -> T doesn't solve the factory pattern issue.
    - AndrewDucker 15y ago
      I don't understand, I'm afraid. How does function passing negate the need to choose between multiple implementors of an interface?
      - fleitz 15y ago
        The interface of a factory pattern is generally a single method, so most factory patterns are an interface with a single function. Normally a factory class looks like this: class MyFooFactory : FooFactory { Foo createFoo() { return new MyKindOfFoo(); } } createFoo has a type of () -> Foo, that is to say a function that takes no parameters an returns an object of class Foo. Sometimes the pattern is String -> Foo, but the more general case is () -> T When you need to use a factory you'll create it and then pass it to the object like: FooFactory f = new MyFooFactory(); FooUser fu = new FooUser(f); fu.doSomething(); And FooUser will have some method in it that does something like public doSomething() { Foo foo = fooFactory.createFoo(); // does something with the foo. } If FooUser took a parameter of () -> Foo instead of FooFactory your code would now look like this: FooUser fu = new FooUser(() => new MyKindOfFoo()); fu.doSomething();
- andrewvc 15y ago
  Completely agreed. There
- andrewvc 15y ago
  Completely agreed. When I work on code I have a sliding scale of likelihood of re-usability in mind for a given task that determines how I write it. A piece of code that defines complex behavior for a weird list type with strange properties on a single page? It's going to be short, tightly coupled, and not very extensible. If it needs to be extended later, we'll throw it away. That's OK. If I'm writing a framework, hell yeah I'll think carefully through things like composition, and adhere more closely to some design patterns. Really though, I like designing APIs from extractions, and determining coupling based on what I learn from those extractions, rather than adhering to generalized principles so closely.
- nikic 15y ago
  I'm not really sure why you think that this only applies to frameworks and libraries. In my eyes this applies to any unit tested code. It is really hard or near impossible to test tightly coupled code. Or do you disagree about testing code in general? Also I don't really understand why you think that factories are an antipattern. As I see it factories play an important role when using dependency injection.
  - onemoreact 15y ago
    High quality tightly coupled code tends to need less testing because it's tiny in comparison. I have seen plenty of cases where throw away code was less than 10 lines be replaced with 10-100x as much "reusable" code that without adding any end user related flexibility or improving performance in any way. PS: I once wrote a 4 line hack in under 30 minutes for another team. They re-factored it into 12 classes over the course of a month and zero functionality was added. So, sure it was ugly and hard to write test cases for but they still put it into production to fix a major bug.
- olavk 15y ago
  Reuse is one thing, but no code should be unmaintainable. If code is written in an unmaintainable style it is also hard and error-prone to develop in the first place. But I agree about YAGNI, and SOLID principles are not always appropriate.
- rickmb 15y ago
  "most software is written to solve a particular problem and doesn't need to be reused" Sure. But most software needs to be maintained, and usually over a way longer period than the original authors imagined (we're sometimes even talking decades here). And not only that: that "particular problem" your code solved? Well guess what: the problem changed. Because that's what happens in the real world, nothing is ever static. Shit happens, things change, that's the only constant. Most software development in the real world is not writing fresh new code in greenfield projects. It's trying to keep the old crap running, old crap written by coders who thought they only needed to solve that one particular problem once. So no, it's not fine that most code is an unmaintainable mess. It's the primary reason why most software work outside the small world of internet startups mainly consists of digging through steaming piles of crap instead of the joy of writing code. YAGNI does not stand for "I'm not gonna need it, so fuck everybody else".
breckinloggins 15y ago
Doing things like avoiding singletons and concrete class references (tight coupling) at all costs IS premature optimization! It's just premature architecture optimization, rather than performance optimization. Sure, if you know ahead of time that some construct is going to be trouble, don't use it. BUT, if you're writing "Mom's Recipe Book", I don't see the point in avoiding a simple singleton DB class just because you fancy that someday you won't be able to handle the spectacular load that Mom's Recipe Book is sure to generate. The "avoid singletons and tight coupling always" argument invariably leads to J2EE-esque code and, before you know it, you find yourself writing a DBFactoryConnectionInterfaceProxyFactory or something. I'm not saying this advice is always bad, just that taken religiously it can lead to code that is over-engineered for the task at hand. Lastly, let's say Mom's Recipe Book DOES become the next Twitter. What's wrong with removing the singleton then? If you are replacing one bit of boilerplate code with another, grep and replace across the project works just fine, doesn't it?
- AndrewDucker 15y ago
  It's about testing. How are you going to write tests for your code when calling the SaveMyLovelyChocolateBrownieRecipe method invokes a database call? Having the database connection passed in to the RecipeSaver class means that you can pass in a fake one instead, and test it.
  - breckinloggins 15y ago
    How about a compromise? IDB is what gets passed to stuff. Singleton::DB implements IDB, and the PHP page code passes DB::GetInstance() to the functions that take IDB.
    - AndrewDucker 15y ago
      Yup, that works. Or the functions check to see if they've been passed something, and if not then they use the singleton. So long as you can choose when to call it with an actual database link, and when to call it with a fake one your code is all perfectly testable.

39 points by nikic 15y ago

z0r 15y ago

I have no idea how the 'optimization' example could be considered as such. The advice to not engage in premature optimization is often repeated, but it's not really illustrated here. No comment on the rest of the article.

deleted 15y ago

[deleted]

fleitz 15y ago

"But honestly: Most of the code being written around the globe is an unmaintainable, unreusable mess." Yeah it is, and that's completely fine because most software is written to solve a particular problem and doesn't need to be reused. SOLID and GRASP have great applications but honestly for most software YAGNI is a much better principle to follow. If you're building libraries, or frameworks SOLID and GRASP are essential, if you're building applications your software is likely mostly glue. I don't worry too much about reusing glue. This is what you get when you folow SOLID and GRASP all the time: http://publib.boulder.ibm.com/infocenter/cicsts/v2r3/index.jsp?topic=/com.ibm.cics.ts23.doc/dfhpj/dfhpj7b.htm http://publib.boulder.ibm.com/infocenter/cicsts/v2r3/index.j... All you really need to know about GRASP is that it advocates the factory anti-pattern. If you're using the factory pattern you've likely chosen a language that couldn't figure out the separation of concerns between allocation and initialization and has STUPID baked into the core of it.

AndrewDucker 15y ago
Factories are a perfectly good solution to the problem of "I have numerous possible ways of carrying out task X, and I want to use one of them that will not be specified until the application is running." So if I want to write some input to a user-defined place then giving the user the list of options and then passing the option to a WriterFactory to give me the correct Writer makes sense. You have to put the choice of which Writer to use in different scenarios somewhere, so why not a class by itself?
- Bjartr 15y ago
  Why not an if statement?
  - AndrewDucker 15y ago
    Ok, but where does the if statement go, particularly if it's reused in a few places, from different classes? Putting it into a util class by itself seems like a reasonable thing to do in that case.
- fleitz 15y ago
  Factories are a good way to solve the problem of not being able to pass a function that creates an object. They're generally an OO kludge around passing an anonymous function, because for some reason passing a function needs to be insanely difficult. And so another class is created to manage the thing that the constructor was supposed to do in the first place (manage the creation of objects) but if you called it the constructor pattern they'd have to admit the language was broken. OO languages that support passing functions generally don't have very many instances of the factory pattern, and languages that have abstracted allocation of memory and initialization of memory have even fewer instances of it. That's why I consider the factory pattern to be an anti-pattern; you've chosen a language that's ill suited to the task you are solving and instead of fixing the real problem (can't pass functions that create objects in a reasonable amount of effort) you're inventing kludges. I've only seen a handful of cases where accepting a parameter of type () -> T or x -> T doesn't solve the factory pattern issue.
  - AndrewDucker 15y ago
    I don't understand, I'm afraid. How does function passing negate the need to choose between multiple implementors of an interface?
    - fleitz 15y ago
      The interface of a factory pattern is generally a single method, so most factory patterns are an interface with a single function. Normally a factory class looks like this: class MyFooFactory : FooFactory { Foo createFoo() { return new MyKindOfFoo(); } } createFoo has a type of () -> Foo, that is to say a function that takes no parameters an returns an object of class Foo. Sometimes the pattern is String -> Foo, but the more general case is () -> T When you need to use a factory you'll create it and then pass it to the object like: FooFactory f = new MyFooFactory(); FooUser fu = new FooUser(f); fu.doSomething(); And FooUser will have some method in it that does something like public doSomething() { Foo foo = fooFactory.createFoo(); // does something with the foo. } If FooUser took a parameter of () -> Foo instead of FooFactory your code would now look like this: FooUser fu = new FooUser(() => new MyKindOfFoo()); fu.doSomething();
andrewvc 15y ago
Completely agreed. There
andrewvc 15y ago
Completely agreed. When I work on code I have a sliding scale of likelihood of re-usability in mind for a given task that determines how I write it. A piece of code that defines complex behavior for a weird list type with strange properties on a single page? It's going to be short, tightly coupled, and not very extensible. If it needs to be extended later, we'll throw it away. That's OK. If I'm writing a framework, hell yeah I'll think carefully through things like composition, and adhere more closely to some design patterns. Really though, I like designing APIs from extractions, and determining coupling based on what I learn from those extractions, rather than adhering to generalized principles so closely.
nikic 15y ago
I'm not really sure why you think that this only applies to frameworks and libraries. In my eyes this applies to any unit tested code. It is really hard or near impossible to test tightly coupled code. Or do you disagree about testing code in general? Also I don't really understand why you think that factories are an antipattern. As I see it factories play an important role when using dependency injection.
- onemoreact 15y ago
  High quality tightly coupled code tends to need less testing because it's tiny in comparison. I have seen plenty of cases where throw away code was less than 10 lines be replaced with 10-100x as much "reusable" code that without adding any end user related flexibility or improving performance in any way. PS: I once wrote a 4 line hack in under 30 minutes for another team. They re-factored it into 12 classes over the course of a month and zero functionality was added. So, sure it was ugly and hard to write test cases for but they still put it into production to fix a major bug.
olavk 15y ago
Reuse is one thing, but no code should be unmaintainable. If code is written in an unmaintainable style it is also hard and error-prone to develop in the first place. But I agree about YAGNI, and SOLID principles are not always appropriate.
rickmb 15y ago
"most software is written to solve a particular problem and doesn't need to be reused" Sure. But most software needs to be maintained, and usually over a way longer period than the original authors imagined (we're sometimes even talking decades here). And not only that: that "particular problem" your code solved? Well guess what: the problem changed. Because that's what happens in the real world, nothing is ever static. Shit happens, things change, that's the only constant. Most software development in the real world is not writing fresh new code in greenfield projects. It's trying to keep the old crap running, old crap written by coders who thought they only needed to solve that one particular problem once. So no, it's not fine that most code is an unmaintainable mess. It's the primary reason why most software work outside the small world of internet startups mainly consists of digging through steaming piles of crap instead of the joy of writing code. YAGNI does not stand for "I'm not gonna need it, so fuck everybody else".

breckinloggins 15y ago

Doing things like avoiding singletons and concrete class references (tight coupling) at all costs IS premature optimization! It's just premature architecture optimization, rather than performance optimization. Sure, if you know ahead of time that some construct is going to be trouble, don't use it. BUT, if you're writing "Mom's Recipe Book", I don't see the point in avoiding a simple singleton DB class just because you fancy that someday you won't be able to handle the spectacular load that Mom's Recipe Book is sure to generate. The "avoid singletons and tight coupling always" argument invariably leads to J2EE-esque code and, before you know it, you find yourself writing a DBFactoryConnectionInterfaceProxyFactory or something. I'm not saying this advice is always bad, just that taken religiously it can lead to code that is over-engineered for the task at hand. Lastly, let's say Mom's Recipe Book DOES become the next Twitter. What's wrong with removing the singleton then? If you are replacing one bit of boilerplate code with another, grep and replace across the project works just fine, doesn't it?

AndrewDucker 15y ago
It's about testing. How are you going to write tests for your code when calling the SaveMyLovelyChocolateBrownieRecipe method invokes a database call? Having the database connection passed in to the RecipeSaver class means that you can pass in a fake one instead, and test it.
- breckinloggins 15y ago
  How about a compromise? IDB is what gets passed to stuff. Singleton::DB implements IDB, and the PHP page code passes DB::GetInstance() to the functions that take IDB.
  - AndrewDucker 15y ago
    Yup, that works. Or the functions check to see if they've been passed something, and if not then they use the singleton. So long as you can choose when to call it with an actual database link, and when to call it with a fake one your code is all perfectly testable.