How to address the concerns of the software architect but still maintain collective code ownership? [duplicate]

Question

The quality of the code in our software is indeed lacking. When you change a line of code in one component it usually breaks code in other components.

Our software architect blames this on collective code ownership. He believes that if every developer was responsible for his own piece of code then he would tidy it up all the time because he would feel ownership over the component.

In the current situation, developers change code in multiple components and they break other components either because they don't understand how the component works (because they work with so many components) or because they don't care about how their changes will affect the other clients of the component (as long as they are not affected).

How can I address the concerns of the software architect but still maintain collective code ownership?

Answer 1

When you change a line of code in one component it usually breaks code in other components. ...if every developer was responsible for his own piece of code then he would tidy it up all the time because he would feel ownership over the component.

So hang on, if the problem is that updating your component breaks someone else's component (due to a dodgy architecture that doesn't have any decent decoupling), how is it that making a component be owned by a developer suddenly stops someone else's component magically decouple so it doesn't break on updating the first!?!

The only answer is to properly organise the components so that are no longer as intertwined as they currently are. That, and documenting the interfaces and data contracts between them.

I do agree with component ownership as it can help with quality of a lot more than just code, but a substitute is code reviews - someone after all needs to know a lot about each component in order to enforce the API and correct behaviour of it. In this case, I think code reviews are just a way of implementing ownership anyway.

Answer 2

Just three things: code reviews, code reviews, code reviews - they will help you improve the code quality as well as the feeling of responsibility for "the shared code".

Answer 3

When you change a line of code in one component it usually breaks code in other components.

Ideal world scenario: You should have your APIs formalized and the API invariants tested in unit tests. If this is covered, when a developer makes a change that touches the invariants, tests will fail. When a developer makes a change that breaks unit tests, they should consider either updating the unit test, or updating the documentation of the API (and removing the unit test).

Our software architect blames this on collective code ownership.

No. Just no. As I understand it, it is more of a problem with everyone modifying code ad-hoc, across multiple components at the same time. This is not an issue with code ownership, but with the methodology of your team not formalizing/respecting API contracts and invariants.

He believes that if every developer was responsible for his own piece of code then he would tidy it up all the time because he would feel ownership over the component.

That's not realistic. If I wrote a piece of code and some other developer breaks it, I will not drop what I am doing to fix bugs introduced by said developer (as I have my own tasks and deadlines).

In current situation developers change code in multiple components and they break other components either because they don't understand how the component works (because they work with so many components) or because they don't care about how their changes will affect the other clients of the component (as long as they are not affected).

Developers changing multiple components (and breaking random stuff) to make their current code work, means lack of clear/formal definitions for your APIs (i.e. at this point there is no clear image of what the component should do), and lack of proper documentation for your APIs (i.e. developers entering functions to see what the implementation does, instead of reading the documentation for it).

What do you believe we should do to address the concerns of the software architect but still maintain collective code ownership?

• Formalize your interfaces and document them. Ensure that the documentation of an API is always accurate (if your function states that a null value yields a MyException thrown, then you better have a unit test that calls the API with a null and catches the exception).

• Give up cowboy coding and "five-minutes" fixes to any defect. There is no "quick fix" to anything. A defect fix should cover all of these:

  • API redesign (this is done on documentation, then API declaration)
  • unit test completion (add unit test for new behavior, remove and edit unit tests for modified behavior, only remote unit tests for behavior that is no longer actual)
  • API implementation changes
  • all tests passing

• ensure clean code and clean diffs (no commented dead code, no code that is weirdly arranged, etc).

Most importantly, DO NOT ACCEPT public commits with failing tests or that have not gone through two rounds of code review (if a change causes failing tests, it is not a fix, it is one or more introduced defects). Two rounds of code review mean:

• reviewer reviews changes and provides feedback to developer
• developer performs changes according to review feedback
• reviewer reviews changes again and gives approval/rejection
• developer commits changes (after approval)

Ideally, each fixed defect will have a new unit test added to the system that checks exactly the behavior described in the defect.

When done like this, a one-liner "five minutes fix" becomes a "30 minutes or more" fix. Ultimately, this is not wasted time, it is time you will spend on implementing new features, instead of time spend in the debugger.

Answer 4

Ownership, collective or individual, isn't going to change the fact that changing things breaks code in other components.

Start adding unit tests for everything that gets touched. If possible, add tests for some (or all) of the callers of whatever you touch. This will (eventually) make it really hard for people to break other code without knowing it, which is probably what's going on right now (I'm assuming your team isn't deliberately committing broken code).

They'll also probably point out that there's a lot of weird inter-twining of your code, which is probably at least a part of the problem.

Answer 5

This is more of a supplement/commentary to Doc Brown's answer.

IMHO - the software architect (SA) is looking for a way to pinpoint blame on someone else. I'm not hearing whether or not everyone is following the plan. Some companies may rely on a team lead or the team as a whole to be responsible for following the plan, so the architect is only involved when there is a question about the plan itself. Others may rely on the SA to decide if the plan is being followed.

Some plans are better than others. Imagine a head chef who has a very poor recipe for chicken and relies on one cook who is particularly good at it (basically, he has his own recipe). Some nights another cook has to step in and gets blamed for the poor quality, because he attempted to follow a bad recipe (no seasoning). Often there aren't enough good coders to bail-out a poor design or follow a plan if left on their own.

As Doc Brown indicated in his answer, a code review is the best way to make sure this is happening along with the following of other standards. There are those that think people will take responsibility for code if they have to fix their own bugs. The bottom line is, someone has to make sure code is shippable and if there is no review, you're just taking your chances.

Answer 6

How can I address the concerns of the software architect but still maintain collective code ownership?

I'm probably bikeshedding, but that's not your problem. The company owns the code, not a developer or an architect. Everyone works to better the whole. You have problems in your engineering process, and it's resulting in a blame game later.

You need to determine the cause of the failure (failed self test and broken components), and put policies and procedures in place to address the gaps. Once the policies and procedures are in place, there's no one to blame because "things just work".

---

In the current situation, developers change code in multiple components and they break other components either because they don't understand how the component works (because they work with so many components) or because they don't care about how their changes will affect the other clients of the component (as long as they are not affected).

Perhaps there's a third: the developers don't know they've nudged or broken code in other components.

That begs the question: what is in your engineering process to address the gaps? Are there policies and procedures in place to catch the problems early?

Process One

I can say for certain that liberal use of ASSERTs (as opposed to POSIX's useless assert) will ensure a developer knows about nearly every unexpected condition at development time. I call it "self-debugging" code, and I spend nearly no time under a debugger because the code tells me where the problems are.

When I say "liberal use of ASSERTs", I mean everything. You assert all parameters are expected when entering a function. You assert all program state used in a function. And you assert all return values from system calls. If there's an if to validate something, then there has to be a matching ASSERT. If there's an ASSERT to validate something, then there has to be a matching if. The code will not be able to fart or sneeze without a developer knowing about it.

Another way to look at it: why should you debug my code, and why should I debug your code? Let the code debug itself and tell you where the problems are.

Another way to look at it: why should you spend time debugging? Your time is better spent on adding features. Let the code debug itself and tell you where the problems are.

If you have not studied how to ASSERT, then it would a treat to read John Robbins. He is a master bug slayer. The book I learned from was Debugging Applications. His blog is at http://www.wintellect.com/blogs/jrobbins.

Process Two

There must be complete self tests for components. I don't waste a lot time on the positive self tests. I can hire a kid from India or Pakistan for US$10 a day to copy/paste code that works as expected under benign conditions. Rent-a-Coder is full of them.

I focus on the negative self test. I want to know how the program fails and ensure it recovers or fails gracefully. And when I say testing, I mean testing everything - even protected and private interfaces in a C++ class.

I don't buy into the arguments that "only public interfaces need testing". Hogwash until someone produces a program without bugs. I've got code in the field that goes years between bug reports because my engineering process requires 100% coverage with negative test cases.

Process Three

Continuous integration into a staging area. Every commit kicks off a build and self tests. Reject the check-in if it breaks a build.

The Real Gem

The real gem in the three processes above is you don't need expert coders. You can hire interns and junior developers to instrument code with ASSERTs and write test cases. I would even argue you need fewer expert programmers because the code is doing the heavy lift by debugging itself. That should make management happy.

And your expert developers will get pissed off when a junior starts breaking their code in obvious ways. I can't tell you how many times a guys with 10 or 15 years of experience complained to me about all the damn asserts that are firing. It's amazing what you find when you start looking for problems in code you thought was solid.

Answer 7

There is validity to his analysis, generally a programmer will 'load' a problem into their head and work on it as a whole.

Holding programs in one's head - pg essay

Don't have multiple people editing the same piece of code. You never understand other people's code as well as your own.

Some real life circumstances will dictate that programmers work with each other's code, but it should not be the standard operating procedure.

Incidentally, this is a big reason why TDD and code quality are popular. They catch bugs when they are the cheapest to fix. It costs a lot more to fix problems later than as the code is written and the information starts to fade away.

---

Part of the software architect's role should be to help migrate the architecture in this direction.

The software architect should be actively assisting in this process. Since I assume they have a deeper knowledge of the architecture, they should be able to help determine the specifics of this plan, what can be broken out into modules? how are dependencies managed? Who should start to take ownership of which part? If the software architect is simply saying 'take ownership' and not helping to hash out all the details, they are a part of this problem as well.

One approach is to find the parts with the least amount of dependencies, and refactoring them. Don't refactor just for the sake of it, but more often than not each module will have a feature request or two that would make it worthwhile to refactor. Don't make it a rewrite!

If testing and code reviews are added to this process, the programmers will actually be better equipped to work with the modules other people create, because they will be writing more similar code that should be more easily tested and more debuggable, but code reviews and code ownership should be part of a plan to improve the software, not the entirety of the plan.

Answer 8

I personally think your architect is correct. I've never bought into this 'code ownership is bad' notion. We're craftsmen. Coders need to be responsible for what they create. And if they create a mess, they need to be the ones who clean it up. Otherwise you end up with programmers making bugs, others fixing them, and the original coder never learning from their mistake. Or worse, you end up with certain poor coders always being the ones making mistakes, and others always being stuck fixing them, carrying their weight.

Yes, code reviews will help that. And its always good to have people familiar with other modules for the proverbial run over by a bus situation. But nothing helps a coder code responsibly and defensively more than learning the hard way that if they screw something up, they'll be the ones stuck fixing it.

That said, with a dainty code base that breaks easily such as the one you describe, I doubt the lack of code ownership is the only problem in play here. That indicates to me a lack of clear thought moving forward, and probably a lot of short term thinking on the part of everyone involved.

Answer 9

If changes in components influence other components too often, it means the bad architecture. So, every such error has the cause in the architect himself.

After he acknowledges it, you could agree with the info on personal authorship, for easier finding WHO KNOWS the component better, not for any blaming. Together with code reviews made by owners of the neighbouring components, it could work.

This way no bad atmosphere is created.

Answer 10

In the current situation, developers change code in multiple components and they break other components either because they don't understand how the component works (because they work with so many components) or because they don't care about how their changes will affect the other clients of the component (as long as they are not affected).

This sounds like a management problem really - that the developers have not been given the time to learn the skills they need to own the code. Other answers offer practical solutions to this, yet I do not think they solve the root problem.

I'd recommend looking into the 5 whys and applying it to your group to find out why the skills are not being learned - this would be an example of it.

• Why don't developers know how the components work?
  • Maybe because they need training in them?
• Why haven't they gotten training in them?
  • Maybe because they don't have time to do training?
• Why don't they have time to do the training?
  • Maybe because they have too much work to do?
• Why do they have too much work to do?
  • Maybe because their manager prioritized work over training

At this point, you need to convince your manager that the benefit of investing in this training now will allow true code ownership and hopefully improved quality.

Answer 11

It's a shared responsability.

Each programmer should be responsable for their own code, and, at the same time, the architect should check every programmer's code.

In order to implement that "shared responsability", your company should also have some policies about code. They should be "close" enough to avoid bugs, and, "open" enough to allow programmers to apply their skills, without feeling too much restricted.

An example of a policy could be to install version control software, and every developer should store their last changes to the code every day before leaving.

Another example of a policy could be how to write an "if-else" sentence, using a "C"-like language. In a company I worked in, we always use brackets, even if we only use a single instruction.

So, this:

if ()
    DoSomething();
else
    DoSomethingElse();

Became this:

if ()
{
    DoSomething();
}
else
{
    DoSomethingElse();
}

When a programmer is hired, he should be informed about the coding policies. And, sign a contract. But, the policies should be explained as something that helps do the job, with examples or training.

You may allow a developer to skip the rules sometimes, too much workload, or an unexpected scenario. Sometimes the broken rule becomes the one rule.

But, if a developer breaks them, frequently, with a non-significant reason, you should fire him, because he may spoil the other coworkers.

If your company doesn't have formal policies, or those policies require to be updated, check them together with the programmers, not just the architect or project manager.