Fog Creek Software
Discussion Board

Dealing with crap code written by your peers

How do people deal with the situation where they see crap code written by one of their peers (in  this case someone who should know better)?

I find it easy to go speak to someone who is a novice and explain their error and show them how to do it properly etc.
But when it's one of my peers and it smacks of laziness or ignorance I find I don't know what to say to them.

Darren Greaves
Friday, May 9, 2003

Start brushing up on talkin' schmack.

Nat Ersoz
Friday, May 9, 2003

I guess it depends on how your peers deal with criticism.

I'm low man on our totem pole, and have dealt with the same thing.  Sometimes I'll say "Did you mean to do this that way?", or "I think I have a better implementation."

I have the benefit of my peers not being jerks, so when I do that, they see it as me benefitting the project and proving my worth to the company.

Friday, May 9, 2003

This is one of the good reasons to have a good code review process. A good code review will improve the code (if needed), educate others on the team about how/why the code was written the way it was, and increase the general knowledge of the team.

Of course, a bad code review degenerates into personality conflicts and can destroy any team chemistry. It's very important to have someone who knows how to de a good review teach/moderate the first few reviews. 

We did this at my prior employeer and it really increased the overall quality of the code by both making it more understandable and by catching potential bugs before putting code into the QA cycle.

Friday, May 9, 2003

Wrap their code into a shell and trace problems. When the thing fails, you can show where and why. So they have to fix.
Defensive programming...
Arguing will never solve the thing. Wrapping it will avoid the "it's not my fault, it's yours" lame excuse that they could use against you. It just cannot be used since you can demo the problem in a controlled environment.

Friday, May 9, 2003

I agree with RocketJeff, without a review process it sounds like "you know better than he does."  Also, one thing I noticed is that good developers get better with age.  You start questioning something I coded five years ago, maybe I do know better now. 

All things considered, it would seem this is the time to argue for a code review.  However, if so everyone needs to agree they are a learning experience not a personal one. 

I may even start another thread on this topic alone...

Mike Gamerland
Friday, May 9, 2003

35 % of the people in our industry owe it to the 65% who write crap code, without them there might not be so many jobs:-)

Prakash S
Friday, May 9, 2003

Before anyone asks: I pulled those %'s out of thin air:-)

Prakash S
Friday, May 9, 2003

Everybody makes mistakes, but nobody likes having their mistakes pointed out to them.

On the other hand, people like answering questions.

"Does this code handle the case where...?"

"Did you consider...?"

Joel Spolsky
Friday, May 9, 2003

The term "crap code" is somewhat imprecise.

What was the problem with it?  There are usually many ways to solve a problem.  If it just doesn't work, you should be able to provide some examples.

Friday, May 9, 2003

I don't want to explain the code in detail as the author of it may read it here :-)

It was something that has been agreed by the entire team as a no-no and is something generally considered bad practice amongst programmers worldwide.

I did speak to him - I asked him if there was a reason he did it that way as usually it's not the correct way to do it.
But the whole event made me uncomfortable and I am reading some good suggestions on how to deal with it should it happen again.

Thanks all.

Darren Greaves
Friday, May 9, 2003

It may be that all code written by other people is bad. People think differently, and it is impossible to “see” at a glance all the dependencies that force a particular developer to do what he/she has done. A software product is an evolutionary beast that can be sometimes tamed by a process. It has its own life. With a good process, we are all the mindless code mechanics of a high quality product.

william williams
Friday, May 9, 2003

"It was something that has been agreed by the entire team as a no-no and is something generally considered bad practice amongst programmers worldwide."

So it's either goto or a global variable.

There can be fair reasons to use either of these, but they should not be used indiscriminately and the programmer should be able to instantly describe how he wrestled with the problem and carefully chose the goto or global as the best solution for specific reasons.

Did he have a reason then?

X. J. Scott
Friday, May 9, 2003

I'm a fan of nice, elegant code but it kinda sounds like you're nitpicking here.  Maybe that isn't the case, but I can't say for sure without more specific information on what your particular issue is.

If the code in question works (particularly if it has _been_ working for months/years) and the design isn't actively hampering other developers from using it, just reign in your idealism-based disgust, move on and concentrate on other things.

Anyway, I'm sure everyone here has written things they aren't proud of due to inexperience (at the time), time constraints, API constraints, or whatever other factors.

George McBay
Friday, May 9, 2003

As a student at University, I often have to deal with 'crap code' written by my peers. If your co-worker is a good programmer and a friend, I'd suggest talking with him briefly. Also, sendhim a patch or two to his code, perhaps it will guilt him into putting more pride into his work.

Just a thought.

Andrew Murray
Friday, May 9, 2003

> something that has been agreed by the entire team as a no-no and is something generally considered bad practice amongst programmers worldwide.

Who are these "programmers worldwide?" I suspect what you're really trying to do is impose some probably trivial, possibly wrong-headed stricture on someone who rejects the need for it.

There's a lot of this political correctness in software development these days, in larger organisations generally. That is, it correllates with less capable developers, not better.

Is he or she not formatting his braces in the Wirth style? Not prefixing his or variables with o, i or whatever to denote type?

What you should do is get on with improving your own work, and learn tolerance.

Friday, May 9, 2003

Quality is not consistent... if it were it would be called average.  Average programmers believe they are quality programmers because of conformance.

Conformance to standards, just like conformance to specs doesn't mean anythng but conformance.  To break either requires a good reason, not just a "cause I can".

I think it was Hitler that said something along the lines of "the great majority of the people will believe the big lie over the small lie".

If it's crap, it will definitely stink... but if conformance is your only guideline, then you won't be able to tell quality if it smashes your nose.

Joe AA
Saturday, May 10, 2003

All of us on the team (including myself) have written some crappy code too from time to time.
However I think as a team we struggle to deal with it when it happens.

We are a small team, who have worked together for 3+ years.
We are all friends - and sometimes it makes it difficult to go up to someone and point out something that is an obvious error.

I know nobody has done it to me very often, yet I have seen errors in my own code when I've looked back it 6 months later and I know others have been looking at it.

The problem in this example was related to poor error handling, so yes, it worked now and may carry on working but when something went wrong (which is how I found it) it was difficult to find out where/what happened etc.

This sort of thing happens very rarely as we are a high quality experience team - which is why we struggle to deal with it I guess.
As I said at the start I have no problem if it is a junior member of staff - I just find it awkard when it's a peer.

Darren Greaves
Saturday, May 10, 2003

Well, if he's writting crap code then he's not my peer. He's my flunky. On occaision good old fashioned learning may elevate a flunky to peer status.

But not often.

Clutch Cargo
Saturday, May 10, 2003

People who're complaining at Darren and telling him that he's being too picky are missing the point.  He asked about how to deal with the situation of telling someone that you don't agree with his or her code or architecture when you don't do that very often.  He didn't ask whether or not you thought what he wanted to tell the person was justified.

Monday, May 12, 2003

Had some recent luck with this...after the other programmer made changes in MY code while I was on vacation, decided to go ahead and do a bug-fix in his.  What's good for the goose, and all.  Then, after we'd discussed that and he didn't seem too upset, I went on to fix all the exception handling logic in that module.

After that, we had a quite interesting (and atypically positive) discussion about how long ago that code was written and how crappy it probably was, where he was being somewhat defensive and I was agreeing about specific changes I'd made in the code, but carefully and politely not blaming the author.

Somehow it's okay for the person who wrote something to say, "Man what crap!!  What was I thinking?" but not okay for someone else to say it.  If someone is really your peer, they have their own strengths which make them worth having around and learning from, and it could be worth remembering to be polite and not step on their ego.  They might respond much better, if they don't feel under attack, and eventually the code will improve.

Sometimes you may not consider a particular person's talents worth the effort of being polite, but that's a whole different question.  Polite but focused on the code seemed to work for me.

Monday, May 12, 2003

*  Recent Topics

*  Fog Creek Home