7

I've moved this from Stack Overflow and this was the suggested place to ask. This has been marked as a possible duplicate of a question about the merits of composition over inheritance. I'm not asking that question, I'm looking for strategies to manage development given the code we have.

The context of the question is a set of applications written in C++ with Qt. We work in a pseudo agile manner, i.e. we have 2 week sprints but don't have frequent releases.

They are all variations of a theme but each modifies the one before it.

This is not an ideal scenario but what I have to work with.

So application A uses Alib. application B uses Blib and Alib; most of the classes in Blib are descendents of those in Alib. Similarly application C uses Alib Blib and Clib, which subclasses from Blib primarily but also occasionally Alib.

We use CVS and it is a proper pain in the neck to branch our builds as the MSVC projects never merge correctly.

What currently happens is a developer working on Alib can cause unforeseen issues in both Blib and Clib unless we adopt some strategy for review or rules on development.

We are under a lot of time pressure and don't currently have the time to reorganise the code, so I am looking for things we can do within the current framework.

If we were using git it would be easier to branch the code for each dev but again we're not currently there just yet (it is in the pipeline).

I'm thinking along the lines of requiring all new developments which alter (as opposed to add) functionality in ALib to subclass that behaviour.

Other options I've read about include "fail often" i.e. being brash about committing/changing the stuff in Alib and make sure that Blib and Clib are review often enough to keep up with the changes.

Similar to this is frequent code reviewing (one developer (me) has good knowledge of all three libs so should be able to spot potential downstream effects early), which begs the question pre or post commit.

Edit: Thanks for the broad range of solutions, I wish I could mark a couple of these as answers, but have gone with John W'us answer. He's articulated my dilemma quite well in the third point - trying to reduce repeating ourselves (i.e. don't violate DRY) while having some isolation for each lib. Ultimately a code refactor is in order, but more (some!) unit tests should highlight broken code. One issue we've had is that the developer in Alib would see their changes affecting Blib and Clib and go and change those in ways the developers of Blib and Clib didn't exactly appreciate. So a rule about keeping code closed (extend but don't modify) is also very pertinent.

Community
  • 1
mike
  • 181
  • 4
  • 3
    Make the person working on Alib responsible for fixing downstream issues: Either require them to run all the tests at regular times or checkpoints, or provide them feedback from your continuous integration build. – BobDalgleish Oct 04 '16 at 14:08
  • we're not quite there with continuous integration unfortunately. What I've just had is a week spent trying to get Clib back a) compiling and b) working as it did after a big commit on Alib. Our tests are pretty substandard too tbh - it's heavy on UI and the behaviour is very flexible, so there's a lot to try and capture. We do have an automated test system that is not exactly exhaustive though, we could run that more often. – mike Oct 04 '16 at 14:13
  • Possible duplicate of [Code Smell: Inheritance Abuse](http://programmers.stackexchange.com/questions/12439/code-smell-inheritance-abuse) – gnat Oct 04 '16 at 14:13
  • ["just don't use inheritance..."](http://programmers.stackexchange.com/questions/328202/inheritance-and-polymorphism-decrease-readability/328208#comment697389_328202) – gnat Oct 04 '16 at 14:16
  • 1
    @gnat: He's not using inheritance yet. – Robert Harvey Oct 04 '16 at 14:16
  • 1
    @RobertHarvey per my reading, he does: "most of the classes in Blib are _descendents_ of those in Alib. Similarly application C uses Alib and Blib, and _subclasses_ from Blib primarily but occasionally Clib" (and from the rest of the question it looks like this happens in quite an abusive manner) – gnat Oct 04 '16 at 14:18
  • 1
    version your libraries – Ewan Oct 04 '16 at 14:19
  • 1
    @RobertHarvey I am using inheritance but it is in the way it is intended as far as I can tell. subclasses are used in the same way as their parents but will do things slightly differently. We could make more of an effort to add new functionality with composition, I suppose. – mike Oct 04 '16 at 14:20
  • "I'm thinking along the lines of requiring all new developments which alter (as opposed to add) functionality in ALib to subclass that behaviour." - so AClassV100 extends AClassV99 extends AClassV98 extends ... extends AClassV2 extends AClass? – user253751 Oct 04 '16 at 23:21
  • @immibis, more like if AClass in ALib (which BClass in BLib inherits which CClass in CLib inherits) needs modifying we create an AAClass to use in Alib without borking BClass or CClass. If there's new code that BClass and CClass could make use of, put it in AClass. – mike Oct 05 '16 at 08:58
  • @mike but then BBClass inherits AAClass and then when you want to change AAClass you need to write AAAClass. And so on. – user253751 Oct 05 '16 at 09:13
  • @immibis, nope. BClass inherites AClass, there is no BBClass. AAClass exists to provide ALib specific behaviour without affecting BLib. This is just moving towards having a common CoreLib that ALib, BLib and CLib can all use. At the minute ALib is trying to be CoreLib and ALib. – mike Oct 05 '16 at 09:26
  • @mike Then what does BLib do when it wants to extend the behaviour of AAClass? – user253751 Oct 05 '16 at 09:46
  • Let us [continue this discussion in chat](http://chat.stackexchange.com/rooms/46339/discussion-between-mike-and-immibis). – mike Oct 05 '16 at 10:07

3 Answers3

3

Three options for you

Follow the Open/Closed Principle

From a code and process management perspective, getting a teamwide agreement to adhere to the Open/Closed Principle should help resolve the problem.

If you are able to get everyone to follow this principle pretty strictly then it may be all that you need to do.

Continuous integration builds with automated unit tests

Develop an automated continuous build process in which ALib, BLib, and CLib are built whenever any of them change.

Develop a suite of automated unit tests that execute as part of the build. If any of the tests fail, the build should fail.

Publicly shame anyone who breaks the build..

Use interfaces instead

If you cannot help yourself and ALib needs to be able to be modified all the time, then perhaps you should move away from implementation inheritance and stick strictly with interface inheritance, as follows:

  1. Develop a new lib, call it ILib. This library should contain zero implementation code and should only contain interface definitions.
  2. Remove the inheritance relationship between ALib, BLib, and CLib
  3. Add an interface relationship between ALib->ILib, BLib->ILib, and CLib->ILib
  4. Freeze ILib

That way, any implementation changes in ALib will have zero impact on BLib and CLib.

The drawback is that you may end up duplicating a lot of code, i.e. violate DRY.

Community
  • 1
John Wu
  • 26,032
  • 10
  • 63
  • 84
2

You're unlikely to find a silver bullet I'm afraid. You have all the symptoms of the fragile base class problem.

In the short term, I would focus as much effort as possible on getting a CI build up and running and getting test coverage as wide as possible. That way you at least mitigate the problem if not eliminate it.

One structural idea you could investigate is to try and have application A not use Alib directly but subclasses in A'lib. It may not be OOP pure but, if the functionality you need for app A isn't in fact needed for B and C, you can put it in A'lib and mitigate some of the fragility.

Good luck. I've been in your shoes and it wasn't the most pleasant experience.

Alex
  • 3,882
  • 1
  • 15
  • 16
  • thanks, that (separate A*lib) is one direction i'm hoping to move in if we start making changes to Alib as new subclasses. That and increasing the use of composition. I have to admit it's all my own fault as I created all the libs! its been a case of "we need this new application and it's only a bit different from the previous one" where each has grown more and more different, with no time to sit down and refactor.. – mike Oct 04 '16 at 15:51
1

a developer working on Alib can cause unforeseen issues in both Blib and Clib

That sounds you have no or too few automated tests for your Alib, and too few for your other libs as well. If a developer changes something in Alib which he expects to be upward compatible, the tests should make sure the existing functionality does not get broken. And if the change is intentional incompatible, only the expected set of tests should break.

So that is what you should work on - automate a big portion of your manual reviewing process by using tests.

Doc Brown
  • 199,015
  • 33
  • 367
  • 565
  • You are totally correct and this is something I intend to change. We currently have *zero* unit tests. – mike Oct 05 '16 at 08:59