How do I convince my team to use smaller classes/methods?

Question

Disclaimer: I'm a newcomer (this is my third day of work), and most of my teammates are more experienced than me.

When I look at our code, I see some code smells and bad engineering practices, like the following:

• Somewhat inconsistent naming guidelines
• Properties not marked as readonly when possible
• Large classes - I noticed a utility class that consisted of hundreds of extension methods (for many types). It was more than 2500 lines long!
• Large methods - I'm trying to refactor a method which is 150 lines long.

The latter two seem to be a real problem. I want to convince my teammates to use smaller classes and methods. But should I do that? If yes, then how?

My team got a mentor from the main team (we're a satellite team). Should I go to him first?

---

UPDATE: Since some responses asked about the project, please know that it's a working project. And IMHO, huge classes/methods of that size are always bad.

Anyways, I never want to piss my team off. That's why I asked - Should I do that, and if yes, then how I do that gently?

UPDATE: I decided to do something based on accepted answer: because I'm newcomer, so I see everything in "fresh eyes" I will take note on all code smells that I found (position, why it bad, how can we do it better,...), but at the moment, I just try hard to gather respects from my team: write "better code", know people, know why did we do that... When the time is right, I will try to ask my team about some new code-policies (naming guidelines, smaller classes, smaller methods,...), and if possible, refactor some old code. It should work, IMHO.

Thank you.

Answer 1

You have the benefit of viewing the code with fresh eyes. Take notes to document what you discover of bad practices. Then, as you're getting settled with the team, pull out your notes at an opportune moment, like when it's time for refactoring.

Answer 2

Code Complete, by Steve McConnell, has tons of good statistics on the very topic you're talking about. I don't recall all the numbers, but he talks about how numbers of bugs increase with longer methods/classes, how much longer it takes to debug, etc.

You could buy a copy of the book, and show your peer some of the studies... stats (although they lie all the time) tend to convince people.

Answer 3

Disclaimer: I'm a new comer (this is my third day of work), and most of my team are more experienced than me.

You may want to slow down a bit, listen, and learn from your team before you go suggesting too many changes. There may or may not be good reasons the code is structured as it is but either way taking the time to listen and learn first can only help.

After that any suggestions you may make will most certainly be viewed more positively and be met with less resistance.

You're chances of successfully introducing change is greatly improved if you earn the respect, or at least don't lose the respect, of your coworkers first.

How does it go? "Measure twice cut once.." Something like that.

Answer 4

Unless you were hired with the specific intention of overhauling how your team writes code, you might wish to moderate your enthusiasm for drastic overhauls. Most working code works for a reason :) no matter how rubbish it is, and sometimes drastic overhauls makes those annoying corner cases even uglier.

I think the easiest lever into writing smaller code would be asking developers to focus on unit testing. Nothing forces concise code like being asked to test it; it's amazing how developers suddenly have an aversion to global data structures, passing too many objects too many layers deep, when they know they've got to write tests for it all.

I'm not a huge fan of TDD but I do love the fact that it forces developers to consider how they'd write tests. And that often is why the code is better, not some magic about actually having the tests. (Though that sure comes in handy when you make changes later.)

Best of luck.

Answer 5

You should not convince your team. As a newcomer, you won't be taken seriously - hence wasting the time.

Instead, go ahead and write compact and clean code yourself. Then hopefully, after a while and some code reviews, some teammates might start mimicking your style.

If not, you'll still be more productive and your hard work will eventually get you to a higher position where you can start enforcing some of these rules.

And yes by all means show stuff from Code Complete to everyone.

Answer 6

Here's a couple tricks:

• Learn the team's current state and history - it sounds like they have a mentor, how much influence does the mentor have? Also, how new is the mentor and was there a long time with no mentor? When does the problem code originate? Criticizing the current team's baby can be a lot different than criticizing some old code that no one actually remembers writing.

• One thing at a time - don't drop the bomb on all your thoughts at a team meeting. Start with some tentative questions that come from your specific perspective. For example - "Hey, as the new guy, I noticed that some of the utility classes are really big, is there a reason for that?"

• Suggest baby steps - it's almost never possible to do an immediate total overhaul, so figure out some starting steps to suggest in case everyone agrees that this is a good plan.

• Suggest future prevention mechanisms - for example, the team could agree to a goal that it will never add to the top few largest classes, but will refactor when there's a need to grow them further.

• Listen to concerns about risk. If this is really legacy code, there may be enough unknowns and dependencies that refactoring is extremely risky. That may not be a reason to avoid refactoring, but it may mean you need some better test strategies or some other way to reduce risk before you tackle the real rework.

• Be aware of body language and go slow. You're bringing up a problem in a code base that you haven't had a lot of experience with. You have a new guy window right now, where you can ask some naive questions and get helpful answers, and you can use those questions to probe the team to consider their own design choices. But it goes both ways - as the new guy, you also don't have a ton of "cred" yet, so go slow and be aware of closed faces or postures. If people start shutting down, suggest a way to delay any decisions and look for ways to win them over.

I can say as a manager and team member, I've been glad for New Guy Insights. I didn't accept every single piece of constructive commentary that a new team member gave me, but I was generally willing to listen if the criticism was voiced as honest concern and curiosity and not delivered as a lecture. The mark of respect to the new guy goes when he can deliver the insight and then step back and handle whatever comes - it's easy to feel good when your decisions are heard and taken up, it's harder when the team tells you "no". You may still be right, the trick is to figure out what to do next... usually waiting a bit and looking for more information is a good next step in those cases.

Answer 7

How do I convince my team to use smaller classes/methods?

Don't.

Buy yourself a Resharper license and lead by example. [Lean heavily on the 'Extract Method' refactoring.]

Over time, others should come to appreciate your more readable code and be persuaded to do likewise.*

---

• Yes - There's a chance that they won't be persuaded; but it's still your best bet.

IMO - It's not worth your syllables to try to persuade your teammates to become better programmers, read 'Code Complete', follow @Uncle Bob. 's SOLID principles, and become better programmers if they're not already persuaded.

Remember: You can't use logic to argue someone out of a position they didn't use logic to get into in the first place.

Answer 8

This seems to be more of an management question than technical question. All you said is valid, what your team really need is an good architect who can make sure everyone adapts to a single design pattern and enforce it, the team need to be constantly and regularly refactoring the code.

However, there is another "you ain't gonna need it" principal, if what ever existed works for a fairly long time, no matter how ugly it is, changing it is always not a good idea. Instead if your team need to rebuild the whole thing or rebuild part of it, accumulate a document of bad practices and problems before carrying out the coding.

Answer 9

Some teams don't do any kind of quality control for code because they don't know the right tools for it. There are a lot of tools that can help a team code better.

Visual Studio has "Code Analysis" that may help with naming conventions.

Also code metrics could be used, like cyclomatic complexity. This helps point out classes and methods that are too complex.

Keeping records is also a good idea. If team members just verbally express what needs to be done, then people are bound to forget. Humans have very frail memories! =)

I would not make a lot of noise about this... A programmer's development team is like his own family... If you point out mistakes, people can get angry with you. This kind of culture change not only requires a lot of coding, but it also requires a delicate touch with human beings.

Answer 10

As a manager I just want to add, that I want my team to write good code the first time. Code reviews, TDD and all that. But once it's in production and it works, you'd have to make a strong case to have us go back at it.

I do follow Uncle Bob's advice to always leave code better than you found it. So if we have bugs to fix or small enhancements, I'd hope we were doing some of the cleanup then.

But as it stands, the business really watches it's money. I'd have to make a case to them that refactoring benefits them enough for them to give my team the time and resources. Just not liking the way the code looks isn't enough.

So, if it works, as much as you might hate it, you might have to leave it alone.

Now new code, that's different. That should be good code.

Answer 11

Methods that are 150 lines... I've seen methods with 10.000 lines of code.

Two of your problems can be solved with external tools:

• Somewhat inconsistent naming guidelines
• Properties are not marked as readonly when possible

In C# Resharper can check both issues. Names which do not follow your guidelines are marked as errors. Properties not marked as readonly show as errors too. FxCop might also be a help.

These tools can also help to split up huge methods into multiple smaller ones thanks to its refactoring.

Answer 12

I don't know that large classes are always all that bad if they are structured well with well named methods. I use Eclipse as my IDE so it has something called the "outline" view, which is something all IDE's have just with a different name, most likely, that provides the name and link to each method within the class, you can sort it alphabetically, etc. When using this its easy to navigate a large class, its more that having really long methods would be bad, I think because its harder to navigate intelligently within that method unless you happen to be really familiar with it. I'm not advocating long classes but I think they are manageable in some cases and don't necessarily need to be broken down into multiple classes.

Answer 13

Bring the subject up with some of your team members and get their opinion on the size of methods. You may be surprised to find they agree with you. What you're seeing could be the result of bad previous practices, former developers no longer with the company, or that part was a rush job and now they've hired someone with time to refactor it ;)

Answer 14

You're still the new guy. Build up some reputation by taking on challenging assignments and completing them quickly and with no bugs. If you try to start changing things before you earn the respect of your peers, you may have a much harder time getting buy in (and possibly alienate your co-workers).

If you can find ways to introduce the better coding habits in your own work that effectively demonstrate how they reduce development time and result in more robust solutions, you might even have them coming to you to see how you achieved this.

Answer 15

Further to all the other great answers, maybe you can kill two birds with one stone and do some code cleanup as a project aimed at getting some understanding of the code-base. You can sell it to your team/manager as a learning opportunity for you, and you'll get feedback from your peers when they look over your changes which will guide you in your best approach for tackling the issue of poor design.