Review Strongly Prefer Composition over Implementation Inheritance

[mbland]

Requesting editorial feedback on the Strongly Prefer Composition over Implementation Inheritance section.

[arowla]

My concern with this section is that it is a little heavy with terms seldom used/encountered by programmers using the dynamic scripting languages that we specialize in at 18F. It seems like for Python or Ruby, the advice could be summed up by saying "go broad and shallow with your object hierarchy. Think modular. Don't overuse inheritance, especially where it obfuscates features, making them hard to know and test."

The stuff about interfaces makes me think back to my C# days, much of which I have blocked from my memory. Which is to say that as a day to day Python programmer, I can barely remember how an interface works.

[mbland]

@arowla Took another pass, incorporated some of your language, but could maybe still use some pruning. Please take another look?

[arowla]

This is an improvement. However, I'm still not convinced this is geared well enough towards the usual web-facing programming languages. I know you want to talk about Go, @mbland, but paragraphs 3 and 4 don't even mention testing. ;)

I rarely, if ever, program using composition in the sense that it's used here: http://en.wikipedia.org/wiki/Composition_over_inheritance (notice all the examples are C++ and C#) I would lean more toward talking about limiting levels of inheritance, such as in the discussion here: http://c2.com/cgi/wiki?MaxThreeLayersOfInheritance and perhaps mention the usefulness of mixins.

All that said, I'm not sure I have strong enough opinions or have thought enough about how dynamic programming architecture relates to testability in these abstract terms to write about it well. I do like the way these architecture recommendations are put in this example in Learn Python the Hard Way:

1. Avoid multiple inheritance at all costs, as it's too complex to be reliable. If you're stuck with it, then be prepared to know the class hierarchy and spend time finding where everything is coming from.
2. Use composition to package code into modules that are used in many different unrelated places and situations.
3. Use inheritance only when there are clearly related reusable pieces of code that fit under a single common concept or if you have to because of something you're using.

Need moar input from the rest of the Testing Grouplet! @micahsaul @DavidEBest @cpapazian @shawnbot @monfresh @adelevie (and anyone else I forgot)

[adelevie]

It provides some nice background over confusing testing terminology (mocks, stubs, fakes). I'm wondering if it's possible to successfully test an application in blissful ignorance of these.

[arowla]

Hi, @adelevie! Specifically we're referring to just the text under this heading, https://github.com/18F/automated-testing-playbook/blob/gh-pages/pages/principles-practices-idioms.md#strongly-prefer-composition-over-implementation-inheritance and the stubs mocks and fakes section will be reviewed in a different issue. Sorry if that wasn't clear.

But to answer your question, I do tend toward thinking that blissful ignorance of mocks, stubs and fakes is acceptable a large portion of the time. :)

[adelevie]

Err, sorry! I agree with you, @arowla, that as Ruby (and sometimes JS and Python) person, I don't use these terms (composition) often or at all.

Re the germane issue of this thread, maybe some code samples in Python that explain the underlying values would be helpful?

[shawnbot]

I'm honestly not sure if I'm qualified to weigh in on this because I'm a self-taught programmer, but here goes:

I've learned that composition is usually preferable to inheritance the hard way. And, in my recent experience, I've found more composition-oriented code to be easier to test. But (for instance) we're also building lots of Django apps at 18F, and inheritance is just how things are done in that framework (and, to some extent, Python more generally). So I'm not sure what to make of a testing document's suggestion that I should prefer one pattern over another when we don't really have much of a choice when we're using one of our most popular frameworks.

I guess I'd like to see more in this section specifically about why composition can make code easier to test, even with some examples. It could be that my eyes just glaze over when I read phrases like "implementation inheritance tightly couples base and derived classes", though. And maybe even suggest some strategies for writing better tests in an environment that favors (if not imposes) inheritance? :trollface:

[monfresh]

I'm self-taught as well, and I don't use or think in those terms either. I follow patterns and best practices established by the community, such as those mentioned in this CodeClimate blog, this Code School course, Sandi Metz's 5 rules, and the Ruby Style Guide.

In Rails, you can write tests for each component of the MVC structure, as well as routes tests, and integration tests. Which one you choose depends on what you're trying to test. For example, if you want to test that a user is redirected to another page and/or that they see a particular error message when they try to access a page they shouldn't have access to, a Controller test would be better than an integration test because it's faster.

In some cases, there are gems that provide a DSL to make it easier for you to test the functionality. For example, Pundit provides authorization policies, and it comes with its own DSL for you to test who has access to what. shoulda-matchers lets you test Model functionality in a very readable and concise way.

Like Shawn said, frameworks already provide a particular structure, and in my Rails experience, writing better tests wasn't a result of changing the code but rather reading about testing best practices and learning when to use the different types of tests.

In my case, when testing a complex portion of the code, I would make sure to have complete test coverage, which would then allow me to refactor the code in confidence. I don't always write tests first, but when I code, I usually solve the problem first in the easiest and simplest way that comes to mind, which is sometimes ugly. Then I make sure I have tests for every scenario so that I can refactor while having peace of mind. So, for me, the code improves as a result of having tests. It's not the code that makes the tests better. It's the other way around.

[mbland]

Guys, I appreciate the feedback. @monfresh Your bits about Rails testing would go well in the Ruby section of the Cookbook, I think.

That said, while I will make an effort to incorporate your ideas about testing in dynamically-typed languages and frameworks, and continue to refine the language to make it easier to understand, believe me when I say there's still plenty of projects (even in government) saddled with C++ and Java that do need to hear some of what I say here, using the terminology I'm using. It will ring painfully familiar.

Also, @arowla, to your point that paragraphs 3 and 4 don't mention testing: Fair point, but testability and design are tightly coupled concepts. ;-) I do want to make that point somehow, that in order to do effective testing, one must design the code and the application to support it; and where a framework gives you no option but to inherit from something in order to hook into it, that doesn't mean the rest of your code has to follow the same pattern. (Bits of the Hub actually illustrate this; I'll include some links.)

I'll try to get around to taking another pass this afternoon.

[nathantypanski]

@mbland I agree testability and design are tightly coupled, but you may wish to emphasize why you're mentioning design here, since apparently it does confuse people. I guess you could add something like:

Interface inheritance, on the other hand, can make it easier to swap out objects for testing, and can make it easier to grow and change your code.

I realize you don't want to say something like that, since duck typing doesn't need interface inheritance, and that's one of its major selling points. What I'd do is state that explicitly:

"Duck-typed" languages (e.g., Python) let you apply this same principle, but you don't have to inherit, define, or implement interfaces to use it - an object's fitness for a certain use case is just based on what methods it has bound.

In order to ease the language curve a little bit, defining some of the terminology might be helpful. I realize these paragraphs are short and sweet as it stands, but I remember asking you to define even terms like "implementation inheritance", which is self-explanatory, simply because I hadn't heard those two words used next to each other before.

I really love using footnotes in my writing for cases where the terminology is dense, but defining it in-place might get in the way. Your mileage may vary.

Bikeshed alert! The nitpicky part of me noticed that dynamically typed languages aren't actually the same thing as duck-typed. That is, I could have a dynamically typed language, where types are inferred at runtime, that doesn't do duck-typing because it requires explicit interface declarations (i.e., the typing is still strong, but it is dynamic since it isn't performed statically). Likewise my language could be statically duck-typed, as long as the checking of duck types ("Is a method bound for that call? Great!") is performed at compile-time. If that even matters at all is your call.

[nathantypanski]

Linking to the Wikipedia article Composition over inheritance might also not be a bad idea.

[nathantypanski]

Also, (and apologies for spamming the hell out of this GitHub issue) I'd like to mention that, anecdotally, I showed these paragraphs to a Java dev at NASA Langley last week and sat down to explain it to him.

About halfway through me talking, I noticed him staring off to the side with a concerned look on his face. So I stopped and asked what was wrong - was the terminology too dense, or did he follow it?

Oh, I was just thinking about this project I'm on with these people. They don't do this at all!

And subsequently he changed to ranting about what was wrong with his project, and how he could improve things by applying these principles.

There definitely are areas, inside government, where this stuff needs to be heard.

[mbland]

@nathantypanski Thanks for the suggestions and encouragement!

All, I've taken another major pass over the text. Closer? (I even dropped mention of my beloved Go.)

[arowla]

Finally got back to reviewing this. I am +1 to the current version. Reads much more clearly to my eye.

[mbland]

Thanks, @arowla. @shawnbot @monfresh @nathantypanski Want to take another pass before I close this issue?

[nathantypanski]

I read it again and thought it was good/better than before.

Showed this to @keikori and have some additional feedback from her. Her impression was that you need to make it seem more dangerous. That is, emphasizing the dangerousness of implementation inheritance is necessary to catch stragglers who see it as perfectly normal (and, indeed, the point of object-oriented programming) to reuse classes via implementation inheritance. While the description of the benefits rang true to her - the importance was stressed - but if your goal is to influence people who would otherwise prefer implementation inheritance, you will not achieve that goal with the text as it exists.

I feel like somebody reading it wouldn't change the way they code just because of his description of what needs to happen.