Code review conducted by an engineer who codes in a different language. Is it constructive?

Question

I'm a Technical Support Engineer who recently discovered how fun programming can be. My boss noticed this interest and suggested that I learn Ruby as a first language since the company can benefit from it, it has a very elegant syntax, and we have no need for more Java/C/C++ programmers.

Well I've written my first fairly large Ruby script that is essentially an automated front-end client to our web application product. The script works .. But since it's my first complete application I'm sure there are plenty of bugs (which I'm squashing slowly) , and I'm sure I didn't write it in the best way it could be written.

Now my boss wants to do a code review - hes a Java programmer and doesnt know any Ruby at all. He admits that he always hated code reviews , but its a necessary evil.

My question is:

Can a programmer who writes in one language effectively perform a helpful/constructive code review of another language they have no knowledge of?

The last review we had was horrible, I had to walk out of the building - because his criticisms didn't really guide me in any direction due to the fact that he couldnt suggest specific/better ways to do things in the proper "Ruby way".

Answer 1

I don't think so, for exactly the reasons you hint at in the end of your question. Someone who doesn't program Ruby isn't going to be able to give you advice on coding in the proper Ruby style. You'll end up writing Java in Ruby.

He might be able to give you good general advice on software engineering, but I don't think that's really what a code review is for.

Answer 2

I do think it can be effective, but I believe in order to achieve the most effectiveness out of it, you have to do a code review from more of a language-agnostic approach. Your boss can't be thinking about the "java way" while you think about the "ruby way" when doing the review. You both have to think in more of pseudo-code terms. Your last statement about you having to walk out the building suggests that maybe your boss should learn ruby especially since he suggested that you learn Ruby. Good luck!

Answer 3

Can a programmer who writes in one language effectively perform a helpful/constructive code review of another language they have no knowledge of?

I'd guess so, but guess that it depends on the programmer.

He can asks questions like:

• What functionality have you implemented?
• Which parts of this software implement what parts of the functionality?
• How have you tested this?

Part of a reason for a code review might be to help the reviewer, for example if you're unavailable and he needs to maintain/be responsible for the code; so sometimes the direction of the knowledge transfer is from you to him, and not just from him to you.

However, if "he always hated code reviews", that's a symptom that he is/you are not doing it right: and maybe he/you should find out more about how to make a code review more successful.

Answer 4

Well, there are several "code smells" an experienced programmer can notice directly even without any prior knowledge in the language. Things related to software engineering in general. But when it comes to Ruby I suspect that he can give a useful syntax review if he never did functional programming before or at least did some code in modern languages such as Ruby like: Groovy, Scala or Python.

Answer 5

They can be, but yours isn't going to be because the Java guy doesn't sound like a great programmer. There are some things which are common across many languages (e.g., declare variables in the smallest scope possible, functions should do 1 thing, loose binding and tight coupling, proper abstraction, etc.)

Answer 6

I'll discuss this as code review being synonymous with code inspection: developer and reviewer(s) sitting in room going over code. Ignore the bits that are not relevant to your situation.

Try not to think of a code review as a criticism of your code. If it becomes that, they're doing it wrong.

The strategy should be to get them to walk through the code, explaining what they see it doing. If misunderstood, you can stop or explain. Any issues or problems spotted can be briefly discussed, and either noted as a bug, code comment (not tidy, braces suggested etc), or action - something to investigate.

Note that none of these are criticisms. You all work for the same company. Your code becomes their code and vice versa (if they write code). The goal is to a) produce solid, maintainable, correct code and b) learn from doing so. You'll be surprised how much you learn reviewing others' code, and from having them review your code. Yes the idea of reading out code is PAINFUL, but in practice it's a great educational experience, and it gives you ideas of what to look out for in future.

NOTE: If the problems they identify are not helpful - "this is stupid", "this could be done soo much better", versus more useful - "if you refactor this out and unroll that look you can change it from Order O(n^2) to Order O(n)" - then be sure to address this - perhaps make an agreement at the start to not attack the writer or the code, but to help improve. This may even come as a surprise to them - they may not have realised they were being confrontational in their criticism.

To the question of cross-language review - sure. The code syntax from one language to another may differ, but so many of the tricks learned, algorithms, logic progressions, common bugs and more are learned independently of the language, and can be applied as experience to future reviews or coding.

Answer 7

If the review stays relatively high level then it can be very constructive. As long as he is asking architectural questions such as why you have used a certain pattern or algorithm then this should be very helpful. New programmers typically need guidance in how to structure their applications at a higher level and I believe this can be provided by any good software developer.

If he is purely concerned about coding style (casing, indention etc.) then this will be less helpful. though he may be able to point out where you are inconsistent.

Answer 8

A coder who knows another language might be able to find bugs, but Java and Ruby are so different stylistically that I am not certain he will give useful style points. Also if the other programmer only knows Java he may not even be able to read Ruby well enough to find bugs (blocks are very foreign to a Java programmer).

Answer 9

A code review from a non-ruby-ist might be useful. As you describe yourself as a beginner there are likely some general, language agnostic software design principles that will be useful to you ("code smells"). A few things off the top of my head -

• are you duplicating code?
• are you writing code that will be hard for others to understand? (e.g. bad naming, multiple operations mixed together)
• are you doing too much with global state?
• are your functions too long? are they doing 1 thing, or 10 things?
• are you mixing object and instance state (the sort of thing you notice at higher loads, as concurrency errors start becoming more noticeable)?

These are all more "general practice" sorts of things, but are probably more important than the language specific syntax stuff in the long run.

There are, of course, things that won't translate so well, e.g. because of mixins there'll be lots of things you'll do differently in terms of inheritance. A java programmer will probably also be uncomfortable with the differences in typing.

Answer 10

I'd say yes as I see a couple of different options for how it could work:

1. The boss reads the code on his own and does some playing around to see how things generally work and then comes back to walk you through various parts that could be changed to improve parts of the application.

2. The boss leads a review in terms of saying, "I want to focus on this piece," and where he doesn't understand something you explain it so that he gets to understand what you did while also thinking of various ideas of the "gotchas" that are possibly universal,e.g. garbage input, logging and exception handling.

In both cases, there is something to be said for identifying good parts, bad parts, and what can be done to fix the bad parts. Giving constructive criticism as well as what should be done next and over what timeframe are key points to this. Saying, "This is crap, redo all of it now," isn't exactly useful.

Answer 11

Perhaps you can separate out a design review from the code review.

An experienced developer should be able to effectively participate and add value to a design review of any application.

The difference would be that you wouldn't need to discuss or review the actual code, or syntax of this or that feature, but look at it from a higher level of what algorithms are used, how you have laid out the classes and areas of responsibility, etc.

Answer 12

Can a programmer who writes in one language effectively perform a helpful/constructive code review of another language they have no knowledge of?

You may be making a bit of a hasty generalization here about code reviewers, namely that if a programmer writes proficiently in one language that he or she will perform helpful/constructive code review sin that language. That is often not the case.

To provide a helpful/constructive code review in any language, you have to be a good teacher, in addition to having a good working knowledge of programming (in some language). A very high percentage of code review comments (the most helpful ones, anyway) are not specific to the syntax of a given language, but rather deal with high level concepts. So an effective reviewer will usually have a good working knowledge of "code smells" and common refactoring techniques.

So the short answer to your question is "Yes". But they have to be a good teacher as well.

The last review we had was horrible, I had to walk out of the building - because his criticisms didn't really guide me in any direction...

Review comments should not be taken as criticism (and they shouldn't be dished out that way either). The review is a chance for both the author and reviewer to learn something. The best code reviews have more than one reviewer where a group discussion can lead everyone to the best answer. The code itself is almost secondary to the review. The review is a success not when the code is fixed, but when the participants learn a better way to code that they can apply to future efforts.

Answer 13

"but its a necessary evil" is already a tipoff that you and your boss and the organization have some issues.

I doubt you are going to benefit from code reviews - not because code reviews are evil or because people not proficient with a language are reviewing code, but because your culture just won't let it happen.

You all need to learn some good methods of conducting reviews. Throw out all your notions of what they are and either get some help with them or read some good material.

Start with very low impact, informal reviews.

good luck

Answer 14

Eh, do you have a choice? I thought he was your boss?

(in other worlds, indulge him - you will eventually anyway :) )

On second thought I think you should consider the fact that your Ruby programs should be readable and maintainable even for a non-Ruby programmer. I think it is fair, to let your boss see that you can write such code. Code should be "owned" by everybody on the programming team.

Answer 15

One of the things I have found useful in a code review no matter what the skill level of the reviewer is the opportunity to explain what I was doing in words. Sometimes just putting what you did into an explanation can make you (not the reviewer) see where something was incomplete or could be approached a different way.

The most important thing about a code review is attitude - your attitude. If you go into it expecting it to be worthless, then it will be. If you go into it expecting to be insulted, then you will be. If you go into it with an open mind and a willingness to use it as a learning experience or a chance to find bugs or problems before they go to production and become PROBLEMS, then that is what you will find.

Nobody's code is perfect, there are multiple ways to do everything. Sometimes other people have an approach that you didn't think of but which might be a better approach than the one you took. Even if you decided not to change to that approach this time due to time constraints, it will now be in your potential tool kit as an approach to consider.

Even if you have to explain why Ruby does things a different way than Java does, I suspect you will learn both more about Ruby (to defend your coding choices) and something about Java (based on the things that seem strange to the Java programmer.) I call that a win.

And yes an experienced coder can usually find things to point out in a code review even if it isn't his usual language.

Answer 16

Really depends on how it is framed. Lots of bad ways to frame it, such that you end up getting abused.

It can be useful, but not as efficient as having someone who knows the language. Just as having the code reviewed by a junior ruby programmer might have some benefit, having a more experienced person will help more.

The other aspect of this is to frame it so the Java people can learn some Ruby. Reviews can go both ways. You'll also get good experience presenting your work.

Like others say, it doesn't sound like you have much choice, and it really can be useful.

• present your work confidently
• have clear goals for the meeting-- create specific questions for the reviewers to answer
• have someone especially tasks with keeping things on topic
• listen, acknowledge what people say, but try to avoid getting assigned any action items-- sounds like the meeting may be too messy to get the right next steps.
• take everything with a grain of salt

Answer 17

All the time it happens to me:

• I have to explain all the basics to the reviewer who obviously doesn't care about learning at least the bare minimum
• The reviewer only grasps few parts of the logic unless it's a very simple file
• I have to discuss about trivialities
• I do get some useful comments once in a while like you could make it a constant here

Ruby is easier than Java so it might not hurt too much, but the Java guy will be tempted to apply concepts that are unusual in ruby.

In my opinion, it is almost worthless to have your code reviewed by:

• A guide who doesn't actually write code
• A guy that doesn't program on a regular basis in the language you're using
• A manager or any non technical guy who believes he knows technology, just because he reads article.
• A woman, who can miss that one :-). Yes, I know what some are going to say but stil... :-) There aren't many women doing programming for obvious reasons :-)