Comment by shadowmint | Hacker News Reader

Comment by shadowmint | Hacker News Reader

Parent Full thread

I think a lot of what makes SOLID valuable is that its articulating patterns that make sense to people because they’re obviously good ideas.

S -> don’t write spagetti code.

O -> If you want your code to be extendable, explicitly choose the parts that can be extended so you can predictably deal with new functionality.

L -> If you extend code, don’t screw it up so the code doesn’t work the same way. Extensions shouldn’t break stuff.

I -> Make your interfaces tiny.

D -> Dependency inversion to make things testable; because testing is good right?

Well, DI is always a bit contraversial, but at least its easy to say why its good; its just a bit of pain to setup.

The interface segregation has always been the odd one out for me.

One method interfaces? I know the golang best practice folk love that stuff too, but I just find it irritating when I have to work on code that uses it extensively.

Anyone actually explain tangibly why its a good idea?

If you’re using it to defend against workmates who might abuse an interface with too many functions on it... well, thats kind of lame imo.

Sure, mega-interfaces are bad... I guess... but so are objects with massive sets of methods on them. Why the special focus on interfaces?

I don't think the push is for 1 method interfaces.

In C#, a good example is that many many APIs use the IList<T> interface to pass lists around. But that interface contains methods like .Add .Remove .RemoveAt .Insert... So while in most cases, you'd be just fine passing a IReadOnlyList<T>, using the IList<T> interface means that as a user of the API, you have to implement these methods which may have no sense for what you're doing.

And so more often than not, you see things like:

void Add(T element) { throw new Exception("Shouldn't go in there"); }

And actually I've just checked, the ReadOnlyList object itself implements IList (cause IList is so used), and so has these exceptions all over the place: https://goo.gl/G6L9Ko

Wow, the ReadOnlyList really does implement all methods of the IList interface. That's just ridiculous, %50 of the methods on the class throw an exception!

Why would the class implement these methods if they aren't even public?

Also, aren't statically typed languages like c# meant to reduce run-time errors by catching them during compilation? Wouldn't throwing exceptions from unimplemented methods from interfaces frequently break this idea?

Ideally yes you would catch this problem during compile time. However apparently the situations where you need to provide an IList for something that really shouldn't be changed is common enough that they decided to provide this functionality in the ReadOnlyCollection.

Now if you read carefully you actually do need to jump through some hoops in order to get an exception.

First of all the methods aren't public, you need to explicitly convert the ReadOnlyCollectionto to an IList or ICollection to access them.

Secondly the dependency inversion principle requires that any ReadOnlyCollection is stored as an IReadOnlyList or some other appropriate interface, there should be no way to convert it to an IList accidentally.

And finally the ReadOnlyCollections seems to be designed for the very specific scenario where you can't solve things using interfaces and need to design a class that supports IList, but throws an exception when it is modified. In all other cases you should use something else.

> Sure, mega-interfaces are bad... I guess... but so are objects with massive sets of methods on them. Why the special focus on interfaces?

Its not a special focus; the SRP is a bigger deal and focuses on objects.

OTOH, violations of the ISP force violations of the SRP, because an interface with unnecessary methods means that objects which must implement the interface will be forced to have unnecessary methods.

Screw up SRP in a leaf class and you've created a problem for that class; screw it up in a branch class or an interface and you've screwed up a wider swath of the code base. As the LSP encourages composition over inheritance, if you are doing the rest of SOLID, interfaces are a bigger opportunity to screw up but chunks of the code base at once than classes.

Concrete example:

Many languages's standard libraries have stuck methods for mutation into the base interface for collections of objects. This makes it awkward (at best) to try and use immutable collections. Either you create your own base interface, which makes you incompatible with the rest of the standard library, or you inherit the standard interface and then do something like throw an exception when someone attempts to mutate your collection, and just hope that never happens in production.

Exactly the example I've written in my other comment... I guess many people have been bitten by this!

It seems like a pretty ubiquitous problem.

I feel like it's actually just about the least painful in C#, since pretty much every collection interface derives from IEnumerable<T>. It doesn't define have methods for random access or getting the size of the collection, but it is at least immutable, so you can get away with using it quite a bit, maybe even most of the time.

In Java, on the other hand. . . woof.

>Dependency inversion to make things testable

Dependency inversion makes things unit testable.

>testing is good right?

All other things being equal, wouldn't a form of testing that doesn't require rearchitecting your code be better than one that doesn't?

Rearchitecting sure, but when designing a code base from scratch, designing for DI is not a bad approach.

It can be. I think it's a trade off that needs to be applied on a case by case basis. Too much dependency inversion leads to writing way more code than necessary, all of which needs maintenance. Too little leads to brittle, tightly coupled code.

I think you have to weigh up the relative merits of having less code vs. having a lower cost of code change. Furthermore, following YAGNI dictates that you shouldn't really do DI until you actually do need it.

I think doing it to facilitate unit testing is a universally bad approach, and a code smell that indicates that what you really wanted was an integration test.

Another reason for small (not tiny) interfaces is to help with unit testing, when different smaller interfaces could be mocked in different ways.

Reply on news.ycombinator.com