111

I'm an experienced developer, but have not done many code reviews. I'm being asked to review code written in Python but I do not know Python.

Does it make any sense at all to review code in a language I don't know?

16
  • 62
    Tangentially, consider going to CodeReview.SE and glancing at the python tag. Looking only at the question, consider what advice you would give to the code and then look to see if that is represented in the answers.
    – user40980
    Jan 13, 2016 at 15:19
  • 10
  • 14
    Sounds like an excellent way to learn more about Python. Question anything that looks funny - some of it will be you not understanding the language, and some will be legitimate code review items. Jan 13, 2016 at 15:33
  • 9
    @RobbieDee Absolutely! Explaining your code to someone is often worthwhile even if it's just a teddy bear. Jan 13, 2016 at 15:41
  • 34
    You say you are being asked to do so. The person asking you thinks that you performing this task will add value to your organization. If you want to know what the nature of that value is ask that person, not strangers on the internet! We don't know what is going on inside that person's head. Perhaps the code is of such low quality that even novices can find problems. Perhaps the code is of such high quality that you will learn good habits from it. Who can say? Someone thinks it is of value; ask that person what the value is. Jan 13, 2016 at 16:46

11 Answers 11

120

Any sense? Yes. Even if you know nothing about the semantics of a programming language, you can still read characters and notice inconsistent formatting, missing comments, badly chosen identifiers, obvious duplication etc.

Much sense, or enough sense to repay the cost of your time? I'm not sure. This depends on your position, the importance of code reviews in the workflow of your team, and several other factors that we can't quantify well enough.

10
  • 51
    Doing some code reviews is a great way to get to know a module, framework or even language, so it might be worth your time anyway (and maybe you're being hinted at it...). I wouldn't sign off on such a review though, not without getting someone who is familiar with both the language and the code to look at it.
    – Ordous
    Jan 13, 2016 at 15:19
  • 8
    Also if you have good domain knowledge and the code is reasonably well named/commented you can still spot incorrect/misunderstood/missing functionality at a high level (you may not spot mistakes due to syntax issues though)
    – Dan
    Jan 13, 2016 at 17:04
  • 4
    As an outsider, you might even be more likely to spot fencepost errors, or errors in applying the correct amount of equal signs, and that kind of things.
    – KlaymenDK
    Jan 13, 2016 at 20:06
  • 2
    @KlaymenDK Assignments in boolean contexts are (at least typically) syntax errors in Python. Fencepost errors are less likely since well written Python pretty much never has you working directly with array/list indexes. (Even when you do, it's usually with enumerate.) I think your comment is a great example of why trying to review a language you're not familiar with should at most be educational for yourself.
    – jpmc26
    Jan 13, 2016 at 21:54
  • 1
    @Mawg - I'd tend to say that's what automated testing is for. Even with expert knowledge of the language, it's difficult to say whether or not code will actually meet the design spec by just looking at it / without executing it and observing the result (unless your design spec is so detailed that it's essentially code in itself). The aspects the answer enumerates cover many (though not all) valid reasons for doing code reviews. A code review does not generally include execution of the code being reviewed.
    – aroth
    Jan 14, 2016 at 13:19
56

As a regular contributor over at Code Review Stack Exchange, I encounter a lot of questions suffering from Language-agnostic issues, for example:

  • Formatting, indentation
  • Scope
  • Loops
  • Type operations

and the list goes on. However, while I don't need to know the language, I can still review those issues / points.

A few of our top users have top answers in languages they either don't actively use, or don't know. Even two of my top ten are in languages I neither know nor can compile / run on my machine.

I'd even say it'd be the same as reviewing someone's pseudo code. As long as you can observe and comment on things relevant to things you understand, you'll be fine, and it'll be relevant.

5
  • 2
    I even wrote an answer to a PHP program. It isn't the greatest answer, but I still maintain that the original loop declaration is ugly.
    – user113093
    Jan 13, 2016 at 17:17
  • 4
    I learned C# through a combination of reviewing it, writing it, and having what I wrote reviewed.
    – RubberDuck
    Jan 14, 2016 at 10:13
  • 5
    If you don't know Python, you are unlikely to appreciate the importance of indentation in the language...
    – Floris
    Jan 16, 2016 at 17:39
  • 2
    Regarding 2/10 of your top answers being in languages you don't know. Who's to say the voters know anything about them either? Jan 17, 2016 at 13:43
  • While I don't know the languages, I still fact-check my statements, and if I messed up... Trust me, I'd hear about it
    – Quill
    Jan 17, 2016 at 13:45
44

General advice

Here's the bottom line, in my opinion:

  • If you don't know the language well enough to know the features and common idioms, you are probably not going to contribute much to the review.
  • If you want to learn the language's features and idioms, you could participate in the review. Your focus should be observing the idioms and asking questions about patterns and organization that don't make sense to you. This may help identify problem areas, but only in the sense that you might force the developer to defend what they've done. Ask your questions in a way that leaves room for your gap in knowledge. Note that this may end up being a net cost in the review itself, but that cost is an investment in your knowledge.
  • Your ability to contribute until you have familiarized yourself with the language's features, idioms, and standards will be limited. I would not expect this to change until you have actually written a significant amount of code in the language.

Python specific considerations and examples

For the specific situation of not knowing Python, I would be especially wary of this. Python has a lot of idioms and standard practices that end up making good Python look very different from what you might expect in other languages. (Indeed, I think the things Python emphasizes have made my code look better in other languages, and not the other way around.) Beyond PEP8 has a good example of how you might completely miss the mindset Python encourages.

Let's look at a simple example. Take this code:

f = open('/home/me/something.txt')
try:
    content = f.read()
finally:
    f.close()

See the problem with this code? If you haven't worked with Python, you probably don't. The problem is that there is a much preferred style in Python that does exactly the same thing:

with open('/home/me/something.txt') as f:
    content = f.read()

This is a context manager. Do you know what they're good for? Do you know when it would be appropriate to use one? Do you know when it would be appropriate to create your own? No? Then you're probably not ready to review Python.

Let's look at another example.

def add_fifty(other_list):
   result = list()
   for i in other_list:
       result.append(i + 50)
   return result

x = range(10)
y = add_fifty(x)

See the problem? The problem is this method is completely unnecessary. You should probably just use a comprehension in place, when the operation is this simple:

x = range(10)
y = [i + 50 for i in x]

If you didn't see that, you're not familiar with Python's features and idioms.

5
  • 1
    In your last example, if I saw the final code I'd probably have some idea what it might do (without knowing Python) and I might start wondering if I can do something similar in my favourite language, which I never considered before.
    – gnasher729
    Jan 15, 2016 at 15:58
  • 1
    @gnasher729 "Indeed, I think the things Python emphasizes have made my code look better in other languages, and not the other way around." =) The only thing I know of in other languages that would be even remotely similar is .NET's LINQ and Java 8's Streams. Ruby might have something, and I'm sure functional languages come close with something. My point with the example is you might not even know to do that if you're not familiar with Python, though, so you wouldn't even know to challenge the "bad" version.
    – jpmc26
    Jan 15, 2016 at 17:10
  • @x = 1 .. 10; @y = map { $_+50 } @x;
    – pipe
    Jan 15, 2016 at 20:00
  • 2
    Code review is more about logic errors than implementing some (to outside programmers) obscure syntax in a particular language. It's about finding logic bugs, like implementing an algorithm completely incorrectly (like using division when it should be addition). So the code won't be precisely up to specs. That's not important as long as the resultant output is correct. At the least, if you're worried about particular optimizations like this, a team code review would be even better.
    – phyrfox
    Jan 15, 2016 at 22:33
  • 3
    @phyrfox That is simply not true. Logic errors may be found, but code review is also (or primarily) about best practices, security, performance, reliability, readability/maintainability, etc. Before something is reviewed, it really ought to have been tested. StackExchange's definition of code review is pretty spot on, imo. In Python, the examples I've mentioned are not "small optimizations." A Python developer who doesn't use these patterns is very inexperienced, behind the times, or incompetent. These are basic elements of the language.
    – jpmc26
    Jan 15, 2016 at 22:51
22

They may have asked you to review Python code precisely because you don't know Python. There's a management theory that it's useful to have a "fool" on a team. I'm not calling you a bad name :) The idea is that a team may suffer from group think and develop tunnel vision. One way to break out of this is to include someone on the team who the other team members would consider a "fool", that is, someone who doesn't know the subject matter. You'll ask questions to inform yourself, and the questions will come from a point-of-view that the other team members likely never considered.

You don't know Python, so what may seem ordinary to the Python coders may seem strange to you. You might suggest an improvement that the team never considered.

2
  • 1
    This was to be my answer. I'd just add that, on any given day with any given Python routine and any given group of Python gurus...that code is GARBAGE.
    – dwoz
    Jan 16, 2016 at 4:08
  • And often, the mere act of explaining something to "a fool" will elicit the "Doh!" moment from an "expert" when they suddenly realise that the code isn't doing what it's meant to, or misses some key edge-case. It's also a decent way of starting the spreading of knowledge.
    – TripeHound
    Jan 18, 2016 at 9:23
21

Code review is not about searching for variables with invalid spelling and wrong formatting. If you use code review to find such things, then stop wasting your time and use a tool.

Code review is about improving design and detecting common mistakes by a novice programmers.

Since I program in C++, and I don't know Python well enough, I wouldn't dare to review Python code. However I could help with a Java code review.

You didn't say in which language you program, but I do not see what you could contribute in a code review, if you do not know the language it is programmed in.

9
  • 24
    Really, you have a tool that can distinguish badly named variables from good ones? I'd be reeeeealy interested in seeing one. Jan 13, 2016 at 16:24
  • 1
    @DavorŽdralo Off the bat I know Java has Checkstyle. More formal static analysis tools are common for many languages, and having them enforce a coding standard is typically the least of their duties.
    – Shaz
    Jan 13, 2016 at 16:32
  • 9
    I have a feeling there's a bit of difference between the definitions of Code Review here, kinda reminds me of this question on CR Meta: What IS a Code Review?. I would not exclude something specific and say "Code Review is NOT about XYZ". It also sounds like you're saying that experienced programmers does not need their code reviewed, something that I very much disagree with. Jan 13, 2016 at 18:08
  • 3
    @SimonForsberg That is not what I am saying. Even senior programmers can learn something in a good code review. But if the only comments are "you mispelled a variable here"-kind of comments, then they are wasting their time. Jan 14, 2016 at 8:31
  • 2
    @BЈовић Nope, it's not the worst you can find, but it is pretty close to "the smallest thing found that is still worthwhile".
    – Vatine
    Jan 14, 2016 at 13:34
10

Code reviews (in addition to actually looking for flaws) are a good introduction from one team member to others for the code being added or changed. If you are an experienced developer, you should be able to read through enough to mostly understand what is going on.

Look at a code review from a team leader's point of view: there is someone there who understands what the application should be doing (business logic), there is someone there who understands the code is doing (implementation logic), and possibly several other people there who need to have an idea of how all of that fits together.

7

You should definitely not be the only reviewer, but there are lots of good reasons for you to be one of the reviewers. Not knowing the language is not much of a hindrance for a lot of questions that need answering in a code review. As an example, I'm one of the top 20 answerers in the C# tag on this site, and I've not so much as compiled hello world in C#.

Some expertise you can share without knowing the language:

  • Domain knowledge.
  • General object-oriented design.
  • General programming practices: naming, clarity, and so forth.

It's also a good way to come up to speed on a new product. I just joined a new team, where I know the languages used quite well, but don't know the domain. Participating in code reviews has helped me learn the domain side better, even though I haven't been able to contribute much along those lines yet.

In your case, it will be a good way to learn the idioms of a new language, as you see the comments other reviewers leave. These are the kinds of things that are very difficult to learn any other way, because your interpreter doesn't care if your code is pythonic or not.

5

This could be a win-win situation. I would go as far as to say that you could be an especially valuable reviewer because you are a Python virgin who has not been tainted by the Curse of Knowledge.

Think of it this way: if code is clear enough that even a Python virgin can understand it, then it must be good code. The parts that you have trouble understanding might be candidates for rework or better commenting.

Obviously, it would also be beneficial for you, because you would be picking up a new language as you go. (Hopefully, the code you are given is a good example to learn from.) This arrangement should work particularly well for Python, a language that has a reputation of being "executable pseudocode". If you are an experienced developer, then you shouldn't have much difficulty understanding the gist of a Python program.

The caveat would be that you wouldn't be expected to spot bugs arising from language-specific gotchas. But bug-finding is not the only purpose of code reviews. If nothing else, you would be participating in knowledge transfer simply by being aware of what kind of stuff goes on in your colleague's code.

2

I was once asked to audit a project that was being undertaken by a subcontractor and appeared to have serious performance problems. I fairly quickly established that the critical factor was a single Perl module. I had never come across Perl before and we had no-one in the organisation who knew it, so I set about trying to understand it myself. I never got as far as understanding the detail, but it was very clear that the algorithm it was using was quadratic in data size and this was the cause of all the trouble. So yes, reading code in a language you don't fully understand can definitely be productive. The bonus is that you learn new tricks while you're about it.

0

A few observations:

1) If you are an experienced developer, you will pick up Python (or at least as much as you need to know), just by working with it. It will be a case of "learning by doing." It will be hard at first, but will get easier as you pick up the language. Think of this as an opportunity to learn another language (people often learn "foreign" languages through "immersion.")

2) There are a number of valuable people on SE sites that are "non-technical," but are skilled with grammar, communications and logic. Such people bring a "fresh eye" to subjects, and make a number of "no brainer" fixes that others miss, because they are too "tied up" in the material. You are being consulted presumably for your non "technical" (i.e. non Python) skills such as logic and overall programming savvy.

And if you haven't done a lot of code review, almost any code review experience will help you as a developer. This looks like a good match between your skills and needs and the team's.

2
  • 1
    I question whether reviewing code constitutes "learning by doing" to a meaningful degree. Perhaps my reviewing experience is different from yours, but there's very rarely meaningful or substantial writing of code, and almost never execution. Jan 13, 2016 at 16:42
  • @EsotericScreenName - I think it depends on how much feed-back you get from someone who wrote or knows the code. It's also more likely to be in a domain area you're working in with another language making it more relavant than the typical code example. Most coding examples just show how to do small snippets of a tack and are rarely shown as part of a large project.
    – JeffO
    Jan 13, 2016 at 16:51
0

That depends on what the goal of the review is; i.e. what you mean by effective.

You will still likely be able to detect some problems. If you're all they got to review with and they're just hoping that you taking a look over it helps some and possibly catches something, then sure. Many concepts of structure are similar between languages. One especially is being able to review the commenting. It should be commented well enough that a programmer not of that particular language should still be able to get a good feel of what's happening. If not...then you can tell them where their commenting is lacking. If it is that well commented...then you should be able to review quite a bit of their structure just through the annotations of what's going on rather than actually reading the code of what's going on.

But you will not likely detect many other problems. So if they intend for your review to be an exhaustive determination of whether or not this is a well made / workable program they will be disappointed.

Whether or not that outcome is worth the time of you doing it depends largely on the project.

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