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?
Software Engineering Stack Exchange is a question and answer site for professionals, academics, and students working within the systems development life cycle. It only takes a minute to sign up.
Sign up to join this communityI'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?
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.
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.
As a regular contributor over at Code Review Stack Exchange, I encounter a lot of questions suffering from Language-agnostic issues, for example:
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.
Here's the bottom line, in my opinion:
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.
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.
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.
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.
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:
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.
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.
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.
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.
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.