How do you control code quality on your team?

Question

Here is my situation. After almost 2 years I was able to convince my boss to use TDD, version control, coding standards for the entire team and a MVC framework for our web projects.

I was finally there. It should be heaven but it's ending up like hell.

I prepared some material about MVC and code samples showing common unit tests. Showed to the team, explained how things should be done and got their support. It seemed all fine, after all, team members were very experienced.

I also made sure that the members had time to do things. At first, writing tests can take a very long time. So I tried to make them comfortable with "losing" this time without any manager asking them to do things quickly and dirty.

After 15 days I was doing the first review of the code. And the result was not good. I found business logic on controllers and on views and the tests folder were almost empty

How do you deal with these kind of things? Do you recommend any tools that can help me on code review or this is something that I have to do manually?

I'm using PHP with Zend Framework.

Answer 1

You implemented a totally new process and you expected it to be perfect on the first try?

I would be shocked if everything was even marginal on the first pass.

Calm down and go over what you see with the other developers. They may have questions and you may have not explained things very well to begin with.

Also remember that concepts like "Business Logic" can be very hard to understand and categorize. Understanding it takes time and practice.

So take time and practice.

Answer 2

I was in a somewhat similar situation to yours, and my solution was mentoring via pair programming sessions.

Take time to sit down with each developer for a couple of hours, to do some development together. Lead them through implementing some small feature the TDD way using MVC properly. Discuss any questions which come up. Ideally, repeat a couple of times (like once a week), until you ensure that they internalize the new approaches. At first you might be required to take control altogether, but gradually you can give the lead back to them.

It requires patience. Changing the way people think is one of the most difficult things in life.

Answer 3

The solution is both simple and hard: Set and Enforce High Standards - Do not accept code that has minor defects.

Define your development process and enforce it - It your process says that all tests will be created prior to actual code development, do not allow code development until all of the tests are complete and accepted.

That you had a bad first code reveiw should be expected. It will take time for your developers to adapt to the new process. There will be some passive resistance (Things like not writing tests) you will need to overcome. It will be painfull and probably hurt morale in the short term. But once they start producing good code and are in the process they will either grow to like it or move on. Your team is probably full of professionals that appreciate the results and in the end understand the means.

Answer 4

Going 15 days without giving your developers feedback on how well they're learning the concept is a long time. A better way to approach this is probably to sit down with each developer often during the early part of introducing these concepts, and go over his code with him, so that he doesn't spend 15 days going off in the wrong direction before you can get him back on track.

Answer 5

If they are truly experienced, without being very familiar with MVC, it's going to be pretty tough to change they way they code/think.

The next natural step in this process is to review the code and its problems with your peers. They need feedback, and you should be expecting the start to be a little rough.

Answer 6

I had a similar issue when I started at my old job back in '06. I found that peer code reviews were tough in that there were large variance in the developers' experience and specialties.

What we did start to have was semi-formal group reviews, we would get together on a light Friday afternoon (at least once a month) to go over a specific problem someone had, how they solved it and what feedback we had. This eventually morphed into doing stuff like coding challenges and other fun stuff.

It was not very rigid (it was a small advertising shop, not high tech), but it kept people interested and it brought out a bit of competition which definitely got people researching best practices.

Also, how experienced are the devs if they are not used to MVC? It is hard to fight the good fight, but keep it up.

I like this question, BTW.

Answer 7

I've never come across any tool which can determine if you have business logic outside of the business layer so we always do it as part of the code review process. The good thing is, as developers become more familiar with the correct development methods (which will only happen if code reviews are strict) the issues will be less and less. I'd also suggest that you should do your code reviews more frequently (at first especially) as after 15 days if the code is fundamentally incorrect there will be a lot of work to correct it and the develop will be disheartened whereas if the code review was done after a couple of days (even if the functionality was incomplete) any problems can be easily rectified.

What source control system are you using? There are tools available for TFS to prevent code being checked in unless there is a certain %age of code coverage. I'm sure similar things exist for other products so it may be worth looking in to them.

Answer 8

The issue with logic in the controller and views that you're describing is Anaemic Domain http://martinfowler.com/bliki/AnemicDomainModel.html.

One of the most common reasons for unit testing falling by the wayside is them becoming dirty hybrid tests see http://blog.stevensanderson.com/2009/08/24/writing-great-unit-tests-best-and-worst-practises/. These become more trouble than they're worth maintaining. Turning these back into unit tests proper will help with them become more productive.

As you say, they're all experienced, so they will have been through learning processes before and changing is hard, so it seems like a matter of identifying:

• the strengths of the work so far
• the issues that they have faced
• a common solution

together and work towards the solution.

Answer 9

TDD, coding standards and MVC are all hard things to do if people are not used to them. Put them all together and you have a recipe for a very steep learning curve. This was never going to be a smooth ride.

You might have been better off introducing these concepts one by one. The easiest is coding standards, so that's where I would have started. The way to make sure they are being followed is with peer reviews. That way people get used to getting critized (and helped at the same time) by their colleagues.

The move on to version control, and after that TDD. TDD is a concept that needs to be thoroughly understood and embraced before it can work. And the introduction of TDD into an existing project is an uphill battle, since there is so much work in writing all the tests for existing code. This alone can take several months just to get to a decent level of acceptance and coverage.

And finally MVC. Now that can be a complete paradigm change, especially for experienced people who have never used it before. Most people who don't use MVC write their applications with control coming from the user, whereas in MVC the user action just triggers something and then the changes propagate through the application by way of events. I have seen good, indeed otherwise very good coders who have never fully grasped the concept and its consequences. A lot of teaching will be required.

There aren't really any tools I can recommend for this. I presume you are using either simpletest or PHPUnit for the unit testing (or something similar). The code reviews are an inherently human thing, and you should not underestimate the team dynamics that come from the review sessions. Once again, this needs mentoring, mentoring and more mentoring. And lots of time.

Answer 10

I think the discipline that you're looking for is called Agile Coaching, from what I heard at a Meetup I attended a few months ago. The head coach (at an agile bank!) recommended two books, Coaching Agile Teams and Agile Coaching. She also recommended the blog of one of the latter books' authors.

I think you'd also benefit from some of Johanna Rothman's work, see her site for blog and book links.

Apart from chasing up those resources, I think it is even more important to get some training from an experienced coach.

Answer 11

Search for "Continuous Integration" over the Web. I use Hudson/Jenkins to run a set of quality tools every time source code is commited. Findbugs, Checkstyle, Sonar are great tools for quality control. These tools are very common in Java projects but i guess you can find similar ones for PHP.

Another suggestion is to define goals for your team like have 10% of test coverage, no more than 10 warnings detected by Findbugs and so on.

It´s hard to keep your team motivated doing Code Reviews over time. It´s a bit boring! A checklist to guide your reviewers and a tool like Codestriker should help. Better if you use a tool integrated in your IDE. Eclipse has some and can be used with PHP. Pair programming is a great alternative for tradicional Code Review too.

And be calm! Quality is a continuous process which you are going to be always identifying problems and solving them. Quality is important, don´t give up! :)

Answer 12

Firstly, are you sure your idea of heaven is the same as the teams? If they've not bought into it, maybe the problem is you not them.

Assuming they have bought into it then you do have to give them plenty of time to get used to the new ways of working. I would hold a post-project meeting (or friday afternoons) to get everyone together to talk through how they like the new ways, use the codebase as a discussion point, then you can raise a section of code and say that you would expect the business logic seen in a view to have been put in a model instead, etc etc. You get to ask them why it was done like that and the answers shoud be informative.

The lack of tests in a TDD environment are another matter though, but again - ask why they didn't write them and listen to the answers.

Answer 13

There are two looks you should consider to hybrid:

• The tangible look:

Is the code under Version control?

If it's not, that's the first step, then you have a Code Delta. So you can take a prcise vision of the implementations.

• The methodology look:

Review meetings as in Scrum Agile Methodology can be useful as well, this makes your own team to ask themselves what have they done wrong, what have they done ok and what could be improved. I would recommend to do the other Scrum meeteings as well.

Applying TDD would be another choice, generating the test before you even start to code could be hard to understand at first, but one of the best possibilities once your team has revolutioned their mindset to this.

You should use both!

Answer 14

Staff your team with developers who care about code quality and create a culture that values this quality. I know this is not the answer you were looking for, but IMO it's the best solution in the long term.