views:

1187

answers:

23

Long methods are evil on several grounds:

  • They're hard to understand
  • They're hard to change
  • They're hard to reuse
  • They're hard to test
  • They have low cohesion
  • They may have high coupling
  • They tend to be overly complex

How to convince your fellow developer to write short methods? (weapons are forbidden =)

question from agiledeveloper

+3  A: 

Force him to read Code Complete by Steve McConnell. Say that every good developer has to read this.

smok1
There are only two developers, me including, at my company who actually read it. And forcing them to read it is rather complicated :-/ Ok, we have a lot of hardware and embedded development, where Code Complete doesn't really matter, but it's still just 30% or so...
OregonGhost
If you're doing embedded delvelopment, then short methods are even more important. Try reading "Safer C" instead.
ijw
Short methods may be important for embedded, but Code Complete is likely not. Thanks for mentioning "Safer C", though I'm one of the few non-embedded guys at the company :)
OregonGhost
You may try to convince your boss, that Code Complete is worth reading, and every developer should do this. If you have an enlightened boss, this will work. I have managed to do something like this with “Pragmatic programmer” (few people even bought this book for themselves). I tried to do this with “Code complete”, but most of developers refused to read something that is not written in their mother tongue. Since no one in Poland published Code Complete 2nd Edition (and the only available polish translation was first edition, it was very poor translation by the way) I failed.
smok1
The mother tongue wouldn't be a problem, since a German translation of the 2nd edition is available (and relatively well translated). My bosses are generally easy to convince about making things better, though one of them is hardware and one is embedded. We (i.e. few developers) try to plan a real "library", but you know how things go from there (where's book XYZ?) in a small company :)
OregonGhost
@OregonGhost: You may use excel file. Write the title of the book and who is currently reading it. Put this file into source control. The only problem is that every book should have a default “owner” to whom must be returned.
smok1
I am a software developer, not an Excel user :P No, really, if you have an Excel file where the current reader of every book has to be entered, someone has to enter that information, and that's the actual problem with that ;) A bar code scanner would be much better, because it's just that much easier / faster to check-in/check-out, yet still people will not do it correctly. Sigh.
OregonGhost
Just put it on a page in the wiki. Or keep an index card in each book; you want to borrow it for a while, just write your name on the index card and stick it in a card file. Done with it? Find the index card, cross off your name, and stick it back inside the cover of the book.
Adam Jaskiewicz
This is why every book should have a default user, who is responsible for it :)
smok1
+40  A: 

Ask him to write unit tests for the methods.

Here Be Wolves
Great advice too. That will make them think twice ;)
OregonGhost
There is no ask. Tell, or tell not.
Kris
okay, correction: "<b>Tell</b> him to write unit tests for the methods." :)
Here Be Wolves
Be civil. If you come off as a dick he's not going to listen to you and the only thing you will accomplish is pissing him off. Unless you are his boss, in which case go nuts
rotard
A: 

You could start refactoring every single method they wrote into multiple methods, even when they're currently working on them. Assign extra time to your schedule for "refactoring other's methods to make the code maintanable". Do it like you think it should be done, and - here comes the educational part - when they complain, tell them you wouldn't have to refactor the methods if they would have made it right the first time. This way, your boss learns that you have to correct other's lazyness, and your co-workers learn that they should make it different.

That's at least some theory.

OregonGhost
No offense, but that sounds like a quick way to get labeled a jerk by your team and "not a team player" by your boss. Just think how you would react if your boss constantly corrected every bit of work you did? I understand your frustration though...but there are better ways to achieve your desired outcome.
mezoid
You are right, playing in a team is typically better. But in a good team, the others should listen when you tell them to write shorter methods because of [20 reasons]. In a bad team, there's nothing to lose if you do it like I described. Well, other than just going to another company. I for myself don't have any of these problems (apart from some rather large C++ include and code files (not methods) in one project), but I know how frustating it gets when there is no real team.
OregonGhost
Ah, but the OP didn't say "write short methods because they're good", he said "What you're doing is bad and wrong and evil! Stop doing that!" Most people, when approached in that manner, will just say "yeah, whatever" and go on as they were instead of doing what you tell them to.
Dave Sherohman
That may be true, but rather for people who don't care, not for good team players. Technically, everything that is contra long methods is pro short methods - a good developer should recognize the essence of the message anyway, no matter whether it is formulated positive or negative. But then again, that's the difference between a team that has good team players and a team that doesn't, and you're back to the problem that bad team players don't listen to well-formulated complains anyway, which was my point. Of course, this does not mean that you should not try to be kind to them ;)
OregonGhost
+2  A: 

Get him drunk? :-)

The serious point to this answer is the question, "why do I consistently write short functions, and hate myself when I don't?"

The reason is that I have difficulty understanding complex code, be that long functions, things that maintain and manipulate a lot of state, or that sort of thing. I noticed many years ago that there are a fair number of people out there that are significantly better at dealing with this sort of complexity than I am. Ironically enough, it's probably because of that that I tend to be a better programmer than many of them: my own limitations force me to confront and clean up that sort of code.

I'm sorry I can't really provide a real answer here, but perhaps this can provide some insight to help lead us to an answer.

Curt Sampson
perhaps average developer is too lazy?
dfa
+20  A: 

You made a list of drawbacks. Try to make a list of what you'll gain by using short methods. Concrete examples. Then try to convince him again.

Nick D
+2  A: 

Force them to read the book "Clean Code", there are many others but this one is new, good and an easy read.

Shawn
Clean code is great. Despite being the "let's keep our code clean"-guy in many situations it really refocussed me on doing object orientation right. And an important part of this is the single responsibility principle, which among other things causes you to write short methods.
jens
+7  A: 

In my experience the best way to convince a peer in these cases is by example. Just find opportunities to show them your code and discuss with them the benefits of short functions vs. long functions. Eventually they'll realize what's better spontaneously, without the need to make them feel "bad" programmers.

UncleZeiv
you mean, teach them to recognize bad smells in code, by setting examples :)
Here Be Wolves
And what if, after spending time with them, you end making them realize that long functions are better? :D
happy_emi
+2  A: 

Asking them to write Unit tests for the complex code is a good avenue to take. This person needs to see for himself what that debt that complexity brings when performing maintenance or analysis.

The question I always ask my team is: "It's 11 pm and you have to read this code - can you? Do you understand under pressure? Can you, over the phone, no remote login, lead them to the section where they can fix an error?" If the answer is no, the follow up is "Can you isolate some of the complexity?"

If you get an argument in return, it's a lost cause. Throw something then.

David Robbins
+1  A: 

When everything else fails, you can start using brutal force :) Nah, just joking!

pregzt
no weapons please! =)
dfa
+10  A: 

I read this quote from somewhere:

Write your code as if the person who has to maintain it is a violent psycho, who knows where you live.

JonoW
Most likely you will be right. There are not that much people that can avoid maintaining its own code :)
voyager
+1  A: 

I'd normally show them older projects which have well written methods. I would then step through these methods while explaining the reasons behind why we developed them that way.

Hopefully when looking at the bigger picture, they would understand the reasons behind this.

ps. Also, this exercise could be used in conjuction as a mini knowledge transfer on older projects.

kevchadders
+15  A: 

That depends on your definitions of "short" and "long".

When I hear someone say "write short methods", I immediately react badly because I've encountered too much spaghetti written by people who think the ideal method is two lines long: One line to do the tiniest possible unit of work followed by one line to call another method. (You say long methods are evil because "they're hard to understand"? Try walking into a project where every trivial action generates a call stack 50 methods deep and trying to figure out which of those 50 layers is the one you need to change...)

On the other hand, if, by "short", you mean "self-contained and limited to a single conceptual function", then I'm all for it. But remember that this can't be measured simply by lines of code.

And, as tydok pointed out, you catch more flies with honey than vinegar. Try telling them why your way is good instead of why their way is bad. If you can do this without making any overt comparisons or references to them or their practices (unless they specifically ask how your ideas would relate to something they're doing), it'll work even better.

Dave Sherohman
I agree. Massive call stacks should certainly enter the discussion.
TURBOxSPOOL
Isn't "short" what fits into a single screen? But then, I'm sometimes using Consolas at an insanely small size with the monitor rotated by 90 degrees... :)Anyway, the unit test thing best defines what's short. If you can't easily write a unit test for a method, it's too long ;)
OregonGhost
Short vs. long is an "I know it when I see it" thing. When I see a method that's 400 lines long and has a McCabe index of 40+, though, that falls into "long".
Adam Jaskiewicz
+2  A: 

I would give them 100 lines of code all under 1 method and then another 100 lines of code divided up between several methods and ask them to write down an explanation of what each does.

Time how long it takes to write both paragraphs and then show them the result.

...Make sure to pick code that will take twice or three times as long to understand if it were all under one method - Main() -

Nothing is better than learning by example.

Nael El Shawwa
"Make sure to pick code that will take twice or three times as long to understand if it were all under one method" ...also known as "stacking the deck". They're more likely to be (and stay) convinced if you use a fair set of examples rather than cherry-picking those that make your way look the best (and the other way look the worst) that it possibly can. Otherwise it's just a simple "this test was biased" and they can legitimately ignore its results.
Dave Sherohman
I agree, what I meant was a non real life example like "hello world" will not go a long way to convince them. Using something like code that will list group posts by author in a forum for example where the benefits of this would be more visible and more valuable.
Nael El Shawwa
+6  A: 

Code Reviews!

I suggest you try and get some code reviews going. This way you could have a little workshop on best practices and whatever formatting your company adhers to. This adds the context that short methods is a way to make code more readable and easier to understand and also compliant with the SRP.

willcodejavaforfood
from some webcomic: the only dependable metric of code quality is the number of wtfs per minute during code review.
Here Be Wolves
A: 

Long methods usually mean that the object model is flawed, i.e. one class has too many responsibilities. Chances are that you don't want just more functions, each one shorter, in the same class, but those responsibilies properly assigned to different classes.

ammoQ
+3  A: 

If you've tried to explain good design and people just aren't getting it, or are just refusing to get it, then stop trying. It's not worth the effort. All you'll get is a bad rep for yourself. Some people are just hopeless.

Basically what it comes down to is that some programmers just aren't cut out for development. They can understand code that's already written, but they can't create it on their own.

These folks should be steered toward a support role, but they shouldn't be allowed to work on anything new. Support is a good place to see lots of different code, so maybe after a few years they'll come to see the benefits of good design.

I do like the idea of Code Reviews that someone else suggested. These sloppy programmers should not only have their own code reviewed, they should sit in on reviews of good code as well. That will give them a chance to see what good code is. Possibly they've just never seen good code.

Clayton
+4  A: 

Finding the right blend between function length and simplicity can be complex. Try to apply a metric such as Cyclomatic Complexity to demonstrate the difficulty in maintaining the code in its present form. Nothing beats a non-personal measurement that is based on testing factors such as branch and decision counts.

Ryan VanIderstine
+2  A: 

short or long are terms that can be interpreted differently. For one short is a 2 line method while some else will think that method with no more than 100 lines of code are pretty short. I think it would be better to state that a single method should not do more than one thing at the same time, meaning it should only have one responsibility. Maybe you could let your fellow developers read something about how to practice the SOLID principles.

Mez
+1  A: 
  • Show him how much easier it is to test short methods. Prove that writing short methods will make it easier and faster for him to write the tests for his methods (he is testing these methods, right?)

  • Bring it up when you are reviewing his code. "This method is rather long, complicated, and seems to be doing four distinct things. Extract method here, here, and here."

Adam Jaskiewicz
+5  A: 

To expand upon rvanider's answer, performing the cyclomatic complexity analysis on the code did wonders to get attention to the large method issue; getting people to change was still in the works when I left (too much momentum towards big methods).

The tipping point was when we started linking the cyclomatic complexity to the bug database. A CC of over 20 that wasn't a factory was guaranteed to have several entries in the bug database and oftentimes those bugs had a "bloodline" (fix to Bug A caused Bug B; fix to Bug B caused Bug C; etc). We actually had three CC's over 100 (max of 275) and those methods accounted for 40% of the cases in our bug database -- "you know, maybe that 5000 line function isn't such a good idea..."

It was more evident in the project I led when I started there. The goal was to keep CC as low as possible (97% were under 10) and the end result was a product that I basically stopped supporting because the 20 bugs I had weren't worth fixing.

Bug-free software isn't going to happen because of short methods (and this may be an argument you'll have to address) but the bug fixes are very quick and are often free of side-effects when you are working with short, concise methods.

Though writing unit tests would probably cure them of long methods, your company probably doesn't use unit tests. Rhetoric only goes so far and rarely works on developers who are stuck in their ways; show them numbers about how those methods are creating more work and buggy software.

Austin Salonen
+1  A: 

Not sure where this great quote comes from, but:

"Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it"

David Plumpton
A: 

You might want to tell them that he might have a really good memory, but you don't. Some people are able to handle much longer methods than others. If you both have to be able to maintain the code, it can only be done if the methods are smaller.

Only do this if he doesn't have a superiority complex

[edit] why is this collecting negative scores?

Stephan Eggermont
A: 

No use teaching a pig to sing. It wastes your time and annoys the pig.

Just outshine someone.

When it comes time to fix a bug in the 5000 line routine, then you'll have a ten-line routine and a 4990-line routine. Do this slowly, and nobody notices a sudden change except that things start working better and slowly the big ball of mud evaporates.

Ian