How to Review Code as a Junior Developer
medium.com
medium.com
On a related note I can’t wrap my head around how I had to grift over a decade to get roles which would pay as much ( and include this level of responsibility and respect) in the UK. Even now I would be aghast with surprise if a company asked me to spend time writing a blog during company time, pleasantly surprised if I was asked to code review. And I mean surprised. Even with my level of experience. ( normally there would be a silo coder who has been there a decade who is the only one allowed to review code).
I’m not saying it’s unfair I’m just pointing out how different a programmers life can be if they don’t slot it into a FAANG company and ends up working at small ones in Europe. Any thoughts are welcome.
Loved the article and the vibe I think it’s great to include junior devs, just pointing out the disparity in wealth and working conditions within 1st world countries tech world
As a final note I was not expecting any empathy it’s taken me aback slightly. I’m ok now guys I sorted my life out with another revenue stream thanks for replies.
I'm not even in western europe, and I had the same feeling, but looks like with advent of remote it's rapidly changing. Not in FAANGs yet, but I think they'll come around to it, too.
Basically, the same story that happened to US blue-collar workers when Kissinger opened China to international trade and put a billion of starving peasants on the path to modern economic prosperity: everybody but those who enjoyed geographic privilege before is bound to win.
The middle area filled with $200k/yr consultants will probably hurt a bit, but I'm not sure they haven't been competing with that group already.
>got your last 3 jobs from recruiters and LinkedIn, you're equally unlikely for different reasons.
This is in the midwest US but we're now recruiting nationally since everyone is remote.
Guess it's something of a function of where you live, but e.g. in London there will be plenty of companies with a better experience.
For pay, there is indeed a large disparity with US salaries. I have seen graduate developer roles going for as low as £19k. And that is the case in many other industries as well, like healthcare. I think this is just about supply and demand, and how much companies are willing to pay.
Is there simply more opportunity in the US or is this a matter of how inexperienced people are viewed? Perhaps there is more holdover from traditional professions where experience was paramount.
and that's good tbh.
we need more people that do hard tech that'll write blogs than jrs
If you value your team mates and strive to have a maintainable code base, you better let someone else review your code. And for me it doesn't end with looking for typos. I actually want someone else to critically read my code.
Sometimes there is a comment missing, or variables and function names could be improved upon, default values should be different, and documentation could be improved. So many things can be improved when you show what you got to your team mates and they comment with questions and suggestions. I actually love the code suggestion feature in Gitlab.
Some Git hosting tools are set up the way that you need approvement by N other people (N >0) and I'm not sure if this is a good thing. For some very simple changes this might be too much bureaucracy, for larger changes (maybe in multiple repositories or some larger architecture change) you might want to involve more than N developers. So I think it should be a cultural thing that is driven by the team.
The first one just didn't do reviews because it was a small startup that kept doing small POC for pitches where code quality wasn't a concern, and that one offered the juniors to set up a youtube channel showing the devices we worked with (it was VR related).
As for pay, the article didn't mention salary I think (?)
It's never safe to have a single point of failure for code review. If all code goes through one guy then they can get lazy and not thoroughly check the code (I've done it myself when I've had a backlog). Sometimes a developer will just submit buggy code because they're lazy! Two lazy developers causes an issue in production. Code reviews need to be spread around the team fairly evenly.
Some developers may have more experience in certain areas, like a core library or service. In those cases it does make sense to have a final pass by a more specific developer. But initial code review passes can be done by anyone.
Pay and responsibilities in tech in the UK really depends on what part of the tech industry you're in. Start ups don't pay that well. Small businesses don't really pay well. Most medium to large businesses can pay fairly well depending on industry.
There's a reason why there are no European FAANGs. And the culture around devs vs non-technical roles at tech companies is a big part of the problem (but most Europeans won't acknowledge it!).
Especially since other people reading the code will probably run into the same issues later on. A code review is the best time for such concerns to come up - the code is still fresh in the author's mind, and it can easily be made clearer or commented better.
One is easier to measure than the other.
But distinguishing ICs based on some model of seniority isn’t “usually there to justify the wage band in companies who also hire fresh grads as you can't pay someone fresh out of school the same as someone with 10 years of experience”, it’s done because the person with more competency is more valuable to the organisation.
So you can certainly have both newgrads and people with years of experience being paid the same. Although not for the reasons that you initially mentioned.
I don't get it either. I'd like to get some recognition for 10+ years of experience, but for now I'm just "a developer". At the same time I see job openings for senior developers asking for only three years of experience. ???
I think the trick is to find a company to hire you and convince them to bump you to "senior developer", even on paper. All that matters is the title, because once you get it, you can put it on your résumé and apply directly for that role, and other companies won't be asking much questions. Repeat for principal developer, but keep in mind the risk of not being able to find an actual coding job.
> I don't know how it is in the US, but over here it's really hard to find good developers, let alone ones that are willing and able to stick with one company for a longer period of time, so you really need to get people on a 'lower' level up to your level.
US is still vacuuming up software talent from all other the world. Anecdote: of my real-life software developer friends and acquaintances, a good half of those I consider to be good developers are either working for FAANG or running a startup and either receiving or applying for YC funding.
As for sticking with one company, this won't be solved while typical raises are non-existent. If the choice is ~2% per year of raise vs. 50-400% salary increase by jumping ship, good developers will keep jumping ship until they're either comfortable or hit some salary ceiling.
Another point I tentatively bring up is, career growth opportunities tend not to be aligned with retaining good developers. Take a look at Dropbox's Career Framework, currently on the frontpage[0], or any other typical SWE leveling system. Senior+ is frequently no longer software development, but management in disguise. In other words, for software developers who want to get better at the technical aspects, career ends at the senior level. At this point, it's only sane to jump to a company that will at least pay more.
This is something I frankly don't understand: a senior software developer is roughly the level where you can call a programmer competent. It's also a level where companies want them to stop programming and start managing juniors. This feels like a huge waste in terms of productivity, letting someone gain proficiency in a skill, and then prevent them from using it. But I guess the market likes it that way, so ¯\_(ツ)_/¯.
--
- Approve one another's PRs (which are either 1 or 2 line things, or 2000 line behemoths. Never in between) without a review, or approve ignoring comments left by others
- Just comment out tests that won't pass because of a change they made, rather than fix it
- Because of the first point, sloppy mistakes and debug stuff (print statements) gets left in the codebase
- Follow poor code health practices, like disabling deprecation warnings instead of updating deprecated code
Whereas most of the mid & junior level devs in the team spend time writing tests, suggest new test cases for one another in PRs, discuss clarity of syntax, try to keep their changes minimal and understandable.
I'm only really sticking around at this point until the pandemic situation has calmed down a bit more, and because I don't want to have too many short experiences on my CV.
They're probably good developers but they lost motivation somewhere along the way.
A senior dev is experienced, pragmatic, effective (at both tech and communication), self-directing and low-maintenance (from a management perspective), and always leaves the campsite cleaner than they found it. They're someone a manager knows they can point at a general problem area, and feel confident that "good things will happen".
By comparison, a junior dev needs much more hand-holding, and "pre-chewing of their food". They are less experienced, so have less of a sense of what the core of a problem is, what goes into identifying it, what a good solution looks like, how to bring it about, what techniques can go into that, and which people need to be involved at what point. Their tasks need to be broken down and specified more, they might need help along the way that they don't even know they should ask for, and their solutions need review.
Being able to identify and classify these traits is helpful - both to help junior devs succeed despite their larger need for assistance, but to give all devs something to aspire to - everyone can always get better.
I do agree though that regardless of your level or the level of the person you’re talking to, you should be respectful, humble, and generous.
But the junior/senior labels definitely exist on a spectrum. I have definitely moved from a more junior place on it towards a more senior place on it over time, even if exactly where I stand on it is for my colleagues to judge.
Something I've found surprisingly useful is to turn my code into a tutorial. I've been working on an educational product recently and I've found writing explanations of my code extremely useful for improving the code; I've made dozens and dozens of changes even after I thought the code was "done" and it was time to begin writing the text.
I'm recording videos based on the text and that's making me realize things about the code that I didn't realize just by writing about it. It's as though, for me, writing and speaking activate different ways of thinking.
It's really been a major contributor to my loss of interest and passion for this industry over the last few years I think.
For example, and oldie but goodie is: http://www.catb.org/~esr/faqs/smart-questions.html
That might be overkill for internal review situations, but it is worth reading and bearing in mind.
Context is all. Sometimes it is the problem of the person you asked. Sometimes it is just the wrong time. Sometimes they are under pressure or just feeling a bit grumpy and snap at you. Sometimes there really is a "stoopid question"! Sometimes you need to try asking the rubber duck first (https://en.wikipedia.org/wiki/Rubber_duck_debugging)!
Keep your chin up and don't take it personally!
There is usually a baseline level of knowledge and skill expected even from a fresh coding bootcamp grad. If they’re not familiar with certain fundamental concepts, answering some of their questions can be frustrating for more senior colleagues.
Business domain knowledge or company-specific tooling is a different thing, but even there, some people tend to avoid note-taking (or looking up documentation), which can result in them repeatedly asking the exact same questions.
Of course, even in those cases, some degree of tact and patience on part of the seniors should be expected, but as harsh as that sounds, there is such a thing as a stupid question.
Many schools seems to take pride in not teaching you how it is really done.
Banning IDEs is a classic.
A friend of mine got some remarks after I'd helped him: I'd introduced him to the BlockedQueue in Java and he wasn't supposed to know about that.
> Business domain knowledge or company-specific tooling is a different thing, but even there, some people tend to avoid note-taking (or looking up documentation), which can result in them repeatedly asking the exact same questions.
My current client actually praises me for just being honest and saying that the existing documentation isn't good enough.
YMMW but many times the training you get for the business specific part you get is awful or non existent and the documentation consist of whatever the previous person (who invented the tool/procedure/whatever) needed to remember.
I never heard of that.
Last year, I've been tutoring a teenager who wanted to take CS on his maturity exam[0]. The exam has a theoretical (algorithms, knowledge) and practical (programming, data science, databases) parts; for the latter, you're expected to write some code in one of several approved languages, using one of several sets of approved tooling (which includes FOSS tooling).
My student wanted to use C++, and thus had a choice between Visual C++, DevC++ and CodeBlocks. It's quite obvious that one of these three is entirely not like the others. Between advanced IntelliSense, errors being flagged as you type, limited on-the-fly static analysis, and semantic suggestions[1], you don't have to know as much about C++ to succeed - the IDE will let you click your way into a working program.
All of these are very useful features when you already mostly know what you're doing. But if you're being tested for your knowledge? That defeats the assumptions of the exam, and of the educational framework behind it. Good education needs to walk a fine line of making sure the tooling augments, not replaces your understanding - so I can understand if some universities choose to skip IDEs initially, letting the students understand the problems their tools are solving for them.
--
[0] - aka. "matura", https://en.wikipedia.org/wiki/Matura#In_Poland. CS is one of the optional topics you can take, and since our universities don't do admission exams, your score on STEM topics on matura is important if you want to continue formal education in technical subjects.
[1] - Green, squiggly lines, programmer equivalent to grammar checks from Word :).
I think the onboarding should make sure there's a list of required reading - books, articles, etc - that the current employees know about, things that influenced how they made the application, what best practices they adopted, etc.
As the person asking the question, always check first if you can google it - the question has to be specific to the application and its authors, not e.g. programming or framework specific questions.
That said, the people answering the questions should make sure to "teach a man to fish", by both answering the question and linking to the relevant resources.
That said, linking to resources and the like is nice, but I also think a lot of people can't be bothered to read - I may be one of those. I will quickly scan something to solve my current problem, but hardly ever sit down to study the materials. One reason is just how quickly technology changes, so it's not worth investing in one technology unless you're actively working with it Right Now. I stopped keeping up with things like Angular, Java and Swift / iOS because I just don't work with those anymore.
That's the "work-should-be-like-stackoverflow" attitude that many folks have. It's toxic.
What may sound like "a stupid question" might be a signal that the asker just needs some direction to get oriented. Or in the case with a bootcamp grad they might be missing whole areas of knowledge that others take for granted. You may argue that they shouldn't be there, but then the blame has to go to whoever did the hiring. It's not like bootcamp grads have been certified by a board to be programmers.
Part of being a senior/expert is being able to communicate with, onboard and assess juniors and new members. That always means MORE than just trying to exactly answer their specific questions. It means probing behind the question and trying to understand where they are coming from and what they need rather than simply what they're asking.
If you can't do that, and you just expect folks to "hit the ground running", you're not really a senior, you're a cog that needs other cogs to mesh with.
People are always busy but as a senior engineer I see teaching/helping junior engineers as a big part of the Job and so long as they are willing to put in some effort and think about it I am happy to give up some time to help.
Assuming you’re not a help vampire (I’ve worked with junior engineers like that, and it’s frustrating because they eat up time unnecessarily), then to the extent that you can, I would recommend finding a new team that doesn’t do that. It’s toxic to your growth and career development. Junior engineers might be junior, but “junior” is not the same as “incompetent”, and teams that don’t appreciate the difference are probably not good places to spend your early career.
I like the rule of 3. Review the code but make notes to yourself first about things you want to ask about. Then pick the top 3 most important ones and ask those on the code review. For the others, try to figure them out on your own or ask someone privately if you can't figure it out.
I would even say that juniors tend to do better at these tips than seniors, especially asking questions. I don't see junior devs having too much of a problem asking questions. They are expected to and reminded if they don't. I see senior devs having problem asking questions. And then nobody points it out to them.
Calibrating the feedback? Seniors tend to run on an autopilot and routine and some just don't notice they need to keep doing these things.
> “Just ask questions.”
Some of the best junior devs I work with do this really well, and the more time I spend explaining in detail the more fine details are discovered.
More senior levels should fill in the blanks, from proposing alternatives, to making recommendations or enforcing certain aspects.
This is the kind of thing that has always bothered me about code reviews. I'm confident that my teammates can write working code or any (obvious) bugs can be caught through code review, but is it possible there is a better solution by modifying code somewhere else?
For example, there's a bug that is causing several issues, and there is a report for one of those issues. A patch is submitted to fix the issue without fixing the root cause and you're reviewing it. You verify the code looks good and that the issue is fixed - not knowing that the root cause itself is not fixed due to unfamiliarity with the system and it not being anywhere in the patch - and approve it.
How much time should be spent looking at whats not in the patch for alternate solutions? How would I know if something could be updated "downstream"? Personally, I used to do it but not so much anymore.
My perspective on code reviews derives from the fact that really, a pull request/changelist is a conversation of complete ownership of code by the author to joint ownership by author and team/organization.
If you're any developer, junior or otherwise, participating in a code review simply means helping that ownership conversion happen however it needs to, whether that means finding and fixing mistakes in the code itself, bringing your own understanding of the code up to speed, or helping to clarify and unify the strategies and logic represented in the code with what will work for the team, which includes you.
If I ever go to an environment like that again, I wonder if there's a way to make code reviews (both reviewers and reviewees) anonymous, to prevent bias like that.
Doing things like looking at code coverage for tests, is a good way to look at the implementation, but it doesn’t inspect the design.
The article describes a great way to teach newer folks, while forcing older folks to review their architectural decisions.
A good way to improve, once we have achieved a certain level, is to teach, and that works by forcing us to review what we do, from a new perspective.
I really hate learning APIs if there is no example of how it can be used. Microsoft has done a stellar job on their documentation but it still isn't enough at times. The "just do it" or "it just works" mentality with code is the #1 thing I hate when using other people's code. Half the time I have to read stuff directly out of their own codebases and actually run it to even figure it out. All because they were too lazy to write a comment somewhere to say "an example of this variable is: ...". In theory, I should be able to use your code right away, not pick at it for hours and go "ohhhh now I see."
I've almost always just read the code and "ran it in my head".
It's the same as proofreading. Your brain just papers over repeated words or typos, or off-by-one errors.
Obviously there's a middle ground and it's unreasonable to ask someone to run every single line of code, but it's good to be aware of that.
Q: Hi <insert dev name here>, say, why are you asking in my review if the class I introduced is really necessary?
Answer from the Paranoia Distroya.
PD: (Without taking their eyes from the screen or their hands off the keyboard). What is exactly the reason for the class? You: Refactor common functionality. PD: Well, long pause while typing really fast, that common functionality may change. You: May? PD: (long pause) I'll take a another look.... You leave after an awkward silence, next day your check-in is reverted and a new check in from PD has the module updated and the comment in the code review marked as resolved.
Answer from Mr. Rogers.
MR: See, base classes are like like a regular class but they are also special. They provide an added layer of encapsulation. You: So.... MR: I tell you what, lets go to your desk and I'll help you work on the base class and update the code with something that is much better, then we will update your review and having it all tied up before lunch. You and Mr. Rogers sit with you and tells what to change while he proselytize on the amazing powers of some editor. You try not to sight. He approves the review with a smiley emoji.
Answer from My Friend From Across the World (over SMS).
MFFAW: What review? Can you give me review no? You: DFDDS-4322343 MFFAW: Code change, check co: 5234543 You: I see commit, what of it? MFFAW: Library removed, NA. You: What library? MFFAW: brb You don't hear back from MFFAW, during scrum you ask again what to do about the code review, MFFAW gets the story assigned and the review is closed. The code is checked in. A week later MFFAW updates the code and the base class you added is gone.
Take away: Ask questions; be ready to roll with the punches.
It was a bit intimidating at first, being the new guy, and at first I would read through things and mostly say "+1, I can't find any obvious fault with this", and gradually, I became confident enough to start asking questions, and started to catch logic errors and uncover edge-cases. But boy do I ask a lot of questions that have some trivial answer that I just didn't know.. Learning not to care if others think you're ignorant, or maybe, accepting that to some degree, everyone are.. Was key for me to get more out of reviewing.
And the answer to those questions is a good signal to classify "seniors" from "experts". Seniors explain, experts know.
I wish a lot more so-called senior developers would start their critique with "Why did you do X?" rather than "You should've done Y".
Sure, if there's an obvious error it's good to just go ahead & point it out, but you're often landed with the negative criticism first, simply because your colleague didn't understand the problem space enough. Your code might still need work since they didn't — e.g. better naming/test cases/comments — but it sets a completely different tone.