19

A little background: I am one of two programmers for our department of 10 people (the rest are artists and management). The two of us do all of the coding required to make things flow well, and develop any projects that come up. I've been programming for about 4 years now, where this is his first "real" job (as he puts it). We generally are working on different projects at any point in time.

A couple months ago I developed a (by no means perfect) set of classes that were to be used for a later project. A large portion of that project was delegated to him (for billing reasons) to design and program a GUI interface. Since he was new, I helped a bit with the designing, and said to ask for help if he needed it with the rest. He finished up the interface a few weeks ago, which he demo'd to show that it worked, although a little slow.

The next part of that project has started which I'm working on. I opened up the interface to start with the next steps, and immediately ran into issues (a little slow was a little understatement, errors on common actions, etc.). I looked in to the code for a few issues and am finding O(n^n) on calls that should be O(n), type assumptions with no error checking (it's in Python), references to the GUI added to the original code, and so on.

Now, I definitely would like to teach him what was wrong and how to fix it, but he's already moved on to his next project, and this was a few weeks ago. I'm afraid me saying "Go back and do it right!" (with help of course) is too harsh, and we still have other projects to get done in the meantime. Should I just fix the code myself for now and try to catch things in the future?

Improve this question
6
  • 4
    In the future, is there a possibility to agree on coding guidelines, that would prevent mistakes like you described?
    – Benni
    Oct 10, 2011 at 21:32
  • 5
    It is a good thing that you are not immediately running to the management and telling on him. Some companies are blame-oriented. As you check the fixes in, find a way to group them together and then have this guy look at them later. On the other hand, even a fresh graduate should not be coding anything that is O(n^n) unless there is just no other way. If they do, then they probably got a C in algorithms or did not take it or had a crappy teacher. Leveraging some sort of tool to help find common problems would be nice. Perhaps as the next task this guy can write some performance tests?
    – Job
    Oct 10, 2011 at 21:35
  • An O(n^n) without documentation as to why is simply wrong, period. If you truly have to do it the comments had better explain why.
    – Loren Pechtel
    Oct 10, 2011 at 23:49
  • Was about to write that "hey, O(n*n) is not that bad, many applications requires it..." but then I realized that it wasn't a multiplication sign, but a killer ^!
    – Max
    Oct 11, 2011 at 1:32
  • O(n^n) can be by a magnitude faster than O(n) if O(n) has a huge constant, and n is small. codinghorror.com/blog/2007/09/… Then again, n^n is extreme :D
    – Coder
    Oct 28, 2011 at 21:37

4 Answers 4

Reset to default
33

Sounds like instituting some sort of code review policy might be beneficial on multiple levels. Some immediate benefits:

  • You can directly influence the quality of his code before code is committed thus keeping the code base quality high
  • Keeps you from making similar mistakes that another set of eyes may catch
  • In the absence of coding guidelines, reviews naturally lead to consistency in coding style
  • Knowledge sharing. If there's only two of you and one gets hit by a bus...

Now when you go ahead and start cleaning up his code, use that as a teaching exercise when you seek a review of this code. You will be getting your stuff reviewed, and he may learn how to do it better next time.

Improve this answer
5
  • 3
    +1 Code review is a great way to go about this. I would suggest phrasing it more along the lines of "Would you mind taking a look at the changes I made to make sure I didn't miss something" rather than "Here are the ways I improved your code".
    – Steve Jackson
    Oct 10, 2011 at 22:36
  • 1
    +1 I'd say code review is a much better fit than any "golden rule coding guidelines".. Not many things is never okay.
    – Max
    Oct 11, 2011 at 1:34
  • I really like this idea, thanks. Now I'll just have to research a bit on good ways to do code reviews!
    – TorelTwiddler
    Oct 11, 2011 at 19:36
  • 1
    There's actually a good and entertaining paper with some basics available at mumak.net/stuff/your-code-sucks.html. It's mostly about behaviorial techniques for conducting reviews in a constructive manner, which is hugely important for successful reviews.
    – nithins
    Oct 12, 2011 at 3:17
  • @TorelTwiddler, just remember that code reviews are for learning, not blaming. Point out things that he did well, so he knows they are good at the same time as suggesting ways to improve.
    – CaffGeek
    Oct 28, 2011 at 16:56
5

Never ever never fix their code, otherwise they won't learn anything other than if they makes mistakes you'll catch them and fix them. The task isn't done until it's done. I got really lucky when I started professional and my direct supervisor rechecked everything I committed, and if there was a better solution or I'd made a silly mistake would tell me, which meant my skills got better, which meant I improved quicker and developed a harder skin.

Letting it slide will breed bad habits, correcting now will make them cope better with criticism, and to triple check before claiming it's done.

Improve this answer
2

Can we infer that the project "works" and was done in a reasonable amount of time (albeit with some egregious but fixable design issues)? If so, it is in a lot better shape than many projects I've seen over the years.

I think that more communication would help your team-- and this could be done with regular code review.

It is good that you're sensitive to being "too harsh" and I think you'll keep that in mind that code review doesn't have to be a demoralizing hot-seat experience where the junior folks get grilled and scrutinized. It can also be a way for senior developers to demonstrate good practices and for everybody to gain trust in each other by being polite and friendly even in the presence of "mistakes".

People learn well when they see what really good stuff looks like. This is better than systematically pointing out every little flaw. The O(n^n), however, should be gently and constructively pointed out.

Improve this answer
0

Share your knowledge.

I would offer him help on his new project in exchange to some teaching from a senior to a junior.

Why not pair programming on both projects?

Improve this answer

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.