by Malte Skarupke
I’ve had some frustrations with code ownership recently. I did not have problems where someone broke something in code that they were not responsible for. I’ve found that people tend to be pretty careful about modifying code that they don’t often work with. Instead my problems have always been that someone doesn’t want other people to touch their code. And then you can’t fix even simple things because you have never looked at their code before and then if a small problem is holding you back, you have to wait at least a day for it to get fixed by whoever “owns” that code.
The simple way to solve that would be to say “everyone can modify any code” and I do like that idea a lot. But let’s see what other people say about code ownership. I’m going to use this post to collect my thoughts on code ownership.
Two important points up front: You have to trust other developers, and you have to be able to let people do it their way. Trust first: You have to work with people who you can trust to make good changes. Then letting people do it their way: Let’s say someone changes your code and the change is just terrible. Well maybe it is just a different style. Just because the other person didn’t do it the way you would have done it, and just because you can instantly see five ways of doing it better, you should not get angry at the person. The only valid reason for complaints is that the change breaks something (and with “break” I do include “the code produces warnings” and I do include”the framerate went from 60 to 40 just because of your change”) or if it makes it much harder to work with the code. Any stylistic differences are not reasons for complaints about changes in “your” code. You should resist your instincts to instantly go in and change the code. Sure, you are allowed to do that, but try to resist. If after a month you still consider it a bad piece of code, change it. Most likely you will never look at that code again and you will promptly forget about it.
Notes from http://c2.com/cgi/wiki?CodeOwnership:
I like Dave Smith’s post at the top: Divide code ownership into
- What the code is supposed to do (requirements)
- How is the code supposed to do it (design, coding standards)
- Who makes changes (coordination)
- When do changes get made (coordination, project planning, configuration management)
I think you should have one person responsible for declaring what the code is supposed to do, and you should have one person responsible for saying how the code should do it. (they can be the same person)
I don’t think that you need to decide on a single person or a group of people who are allowed to make changes. Everyone can make most changes, including deleting other people’s code. As long as they don’t change what the code does or how it works. So what changes can you then make?
- You can fix a bug that you have encountered.
- You can add small features that don’t change what the code does. (for example you may expose more functionality of a class to scripting)
- You can make changes to the interface that don’t change how the code works. For example let’s say a class has a function called “setNextAnimation()”, then you may split that up into two functions called “getNextAnimation()” and “setAnimation()”.
- You can clean up a class and remove methods that are no longer needed
What are changes that you aren’t allowed to do? Well let’s say the previously mentioned animation class has to register itself somewhere so that the AI system can talk to it more easily. I could see that design breaking in many ways and maybe you want to change it or maybe it is no longer needed. Well you are not allowed to change it without talking to both the person who says what the code should do, and how it should do it, as this affects both of those points. Or let’s say you want to replace a slerp in the animation code with an iSlerp. You can’t do that without talking to the person who says how the code does what it does, because choosing a different algorithm is not something that anyone can do.
As for the “when do changes get made” part: I don’t think that this should apply to any changes that don’t modify what the code is supposed to do or how it does it. Meaning the changes I mentioned above that everyone can do: Do those whenever you feel like you need to do them. They should mostly be refactoring, bug fixing and small tweaks here and there that you may need. As for what the code does and how it does it: I could see that you may want a third person who is responsible for saying when those changes get made. As in: the person who is responsible for how the code does it’s job decides that she wants to switch to a more efficient algorithm: She has to talk to the “when” person to decide whether now is actually the time to do it or not.
Notes from http://jamesshore.com/Agile-Book/collective_code_ownership.html:
This is an excellent article. I just want to send this to as many people as I can.
I still like Dave Smith’s points from above, where he says that you should have ownership for what the code is supposed to do and how the code does it. I think many people know this implicitly, but it is worth pointing out.
A couple other points: This person obviously lives in an ideal world where there are unit tests for everything and where you can do paired programming whenever you want to without wasting a ton of time. I have had great experiences with paired programming, but it’s going to cost if you want to use it as much as this person seems to suggest, and sometimes it’s plain boring. Also in real world games development there aren’t going to be unit tests for everything. So yeah, show some restraint in what you modify.
I also really like the point he makes about collective code ownership compared to no code ownership:
Don’t use collective code ownership as an excuse for no code ownership. Managers have a saying: “shared responsibility is no responsibility at all.” Don’t let that happen to your code. Collective code ownership doesn’t mean someone else is responsible for the code; it means you are responsible for the code—all of it. (Fortunately, the rest of the team is there to help you.)