Don't be stupid: Grasp SOLID | Hacker News Reader

Don't be stupid: Grasp SOLID

nikic.github.com

39 pointsnikic25 comments

"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.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.

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?

Why not an if statement?

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.

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.

I don't understand, I'm afraid. How does function passing negate the need to choose between multiple implementors of an interface?

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();

Yes, in the case where a Factory class contains one method that takes no parameters and just contains a "new" call, you can get rid of the class and pass in the method.

I was more thinking of the kind of thing I mentioned in my earlier comment, where you have an IWriter interface, a bunch of different classes that implement it, and a Factory class that passes back the correct one for different situations. I completely agree that a whole class that does nothing but call "new" is complete overkill, and almost certainly just there to fit into a pattern because someone thinks that Patterns Are Awesome.

The kind of thing I'm thinking of is this:

https://github.com/andrewducker/FeedThisToThat/blob/master/F...

Completely agreed. There

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.

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.

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.

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.

"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".

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?

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.

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.

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.

In my world, we solve the "what if it needs to write to the DB" problem by... having a DB.

Seriously, creating a test database as part of the test run, and populating it from fixtures and letting the code interact with it the way it will in production is actually very, very simple.

This is, essentially, the YAGNI principle; sure, in theory I can come up with a use case where that code needs to talk to something that isn't a database, but A) it's not going to happen in real life and B) re-architecting everything in the name of that possibility is premature architecture astronautics that will cost much more time and effort than just saying "we aren't gonna need that" and moving on.

It's not just DB. What about 3rd-party Web Services (Facebook, Amazon, etc).

Also, can you run the whole thing in your continuous integration build? Setting up Database to run on a CI server sometime is problematic (depending on how you setup your CI, if you're using Amazon EC2 instances...). Rails solved this problem to some extend by using SQLite but one needs to install + configure SQLite for various machines.

Speed is another thing. The more you write tests that hits the DB, your automation-test will take a performance hit and you'll have to do something eventually.

What if you have caching by utilizing some sort of caching server? that needs to be installed and configured as well.

What if the only way to test the code is to deploy it to the server and write raw HTTP GET/POST because those are the only entry point?

In 2011, where there are better tools and known practice, I don't think we should labelled some of these practices as YAGNI and Premature Optimization.

First of all, it's not YAGNI if you have to refactor or write another framework, tools, or some sort of patches in order to have an acceptable build time.

Second, it's not Premature Optimization if the tools, the libraries, and the environment provide you with ways to implement them in no time.

I think the excuse of Premature Optimization is used a lot in a context where it doesn't mean what it supposed to mean.

I think the excuse of Premature Optimization is used a lot in a context where it doesn't mean what it supposed to mean.

Which may be why I didn't use the phrase.

In all seriousness, though, I can sit down and come up with dozens -- heck, hundreds -- of "well, what if" cases. And you know what? I don't care about 'em.

I went through the phase of abstractly decoupling everything to the Nth degree because of "what if" cases. And I paid for it; I was overlooking simple and often more elegant solutions to actual problems in favor of complex solutions to hypothetical problems.

Does using a real database instead of decoupled MockableWriteableDataStorageInterface mean my tests take a few extra seconds to run? Yup.

Does the time I'd save running tests by writing and passing in a MockableWriteableDataStorageInterfaceImplementation outweigh the increased complexity and thus mental load of the codebase? Unlikely.

And, importantly: does testing with something that isn't a real database potentially cover up bugs that only happen when I use a real database? Yup, and that happens often enough that the whole mockable thing just goes out the window right there.

Anyway. I think most developers go through this phase, too, and I think it's similar to the way people spend disproportionate amounts of time worrying about dramatic but unlikely things (shark attacks, terrorists, etc.) rather than dealing with the mundane but extremely likely things right in front of them.

Those "what ifs" had hit me in a few projects already. This is why spring-test module from Spring Framework is very helpful if you're doing Java projects.

I don't have to deploy "war" to Tomcat or Jetty. I can mock the Request and Response object.

We do have integration-tests that test specifically component that interact with databases. But there are another set of unit-tests that test the business/process workflow and assume the database will return something relevant. We do this because we want to write many tests with focus specifically to business/process workflow related code, not the database layer.

This way we can write more tests without paying the taxes of: performance and setup (when you have a huge database models, it's not fun to prepare the models for each-single-unit-test that has to touch the database even though the purpose of that unit-test is not to test the database).

So far this model has worked well for us (i.e.: no more excuses that build takes too long or writing unit-tests take too long due to the setup both infrastructure and preparing huge model).

Do you have the DB on the dev machines, or just as part of the build server?

We found that running unit tests that hit databases was far too slow to have people running them all the time. Ones which mocked out database calls were over 100 times faster, which meant we can run the whole test suite in under three minutes, where the old test suite we used to have which made DB calls took over an hour and a half, which was infeasible to run on check-in, let alone on dev machines.

You guys ran into this while developing a 'Mom's Recipe Book' type app?

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.

Reply on news.ycombinator.com