DEV Community: EgzonaVllasaliu

Code review and clean coding principles at ORNIO

EgzonaVllasaliu — Wed, 12 May 2021 10:24:39 +0000

How to make a good review of the code? Where to start? What you should remember? Why is clean coding important?
I will show you my practices which will help not only with the product itself, but also make wonders for the development team.

Code review

Code review helps give a fresh set of eyes to identify bugs and simple coding errors before your product gets to the next step, making the process for getting the software to the customer more efficient. Simply reviewing someone's code and identifying errors is great.

Here at Ornio we use this guideline to do an organised code review.

Ask open-ended questions instead of making strong or opinionated statements. Offer alternatives and possible workarounds that might work better for the situation without insisting those solutions are the best or only way to proceed. These reviews assume the reviewer might be missing something and ask for clarification instead of correction.
Once a reviewer completes their review, they can either mark it approved, block the review with change requests, or not set a specific status, leaving it in a “not yet approved” state.
Flexible on the practice: sometimes, certain comments are addressed by the author with a separate, follow-up code change. For changes that are more urgent than others, reviewers should try to make themselves available for quicker reviews.
Leave as many comments and questions as needed. If the revision does not address all of them, they will note those as well. When the conversation gets into a long back-and-forth, reviewers should try to switch to talking to the author in-person instead of burning more time using the code review tool.
Proactively reach out to the person making the change after they do a first pass on the code and have lots of comments and questions. This will save a lot of time, misunderstandings, and hard feelings this way. The fact that there are many comments on the code indicates that there is likely some misunderstanding on either side. These kinds of misunderstandings are easier identified and resolved by talking things through.
Make it clear when changes are unimportant nitpicks. They usually mark comments like these distinctively, adding the “nit:” prefix to them. Too many of these can become frustrating and take the attention away from the more important parts of the review, so reviewers aim to not go overboard with these.
Realize that too many nitpicks are a sign of lack of tooling or a lack of standards. Reviewers who come across these frequently will look at solving this problem outside the code review process. For example, many of the common nitpick comments can be solved via automated linting. Those that cannot, can usually be resolved by the team agreeing to certain standards and following them—perhaps even automating them, eventually.
Ensure that all engineers take part in the code review process—even those that might be working on solo projects.
Have hard rules around no code making it to production without a code review—just as business logic changes don’t make it to production without automated tests.
It is the author’s responsibility to submit PRs that are easy to review in order not to waste reviewers’ time and motivation:
- Scope and size. Changes should have a narrow, well-defined, self-contained scope that they cover exhaustively.
  - For example, a change may implement a new feature or fix a bug. Shorter changes are preferred over longer ones. If a PR makes substantive changes to more than ~5 files, or took longer than 1–2 days to write, or would take more than 20 minutes to review, consider splitting it into multiple self-contained PRs.
  - For example, a developer can submit one change that defines the API for a new feature in terms of interfaces and documentation, and a second change that adds implementations for those interfaces.
- Review fewer than 400 lines of code at a time.
- Do not review for more than 60 minutes at a time
- Only submit complete, self-reviewed (by diff), and self-tested PRs (Authors should annotate source code before the review). In order to save reviewers’ time, test the submitted changes (i.e., run the test suite) and make sure they pass all builds as well as all tests and code quality checks, both locally and on the CI servers, before assigning reviewers.
- It is customary for the committer to propose one or two reviewers who are familiar with the code base. Often, one of the reviewers is the project lead or a senior engineer.
- Project owners should consider subscribing to their projects in order to get notified of new PRs.
- As a reviewer, it is your responsibility to enforce coding standards and keep the quality bar up.
- Reviewing code is more of an art than a science. The only way to learn it, is to do it; an experienced reviewer should consider putting other less experienced reviewers on their changes and have them do a review first.

Maintainability

Read the tests. If there are no tests and there should be, ask the author to write some. Truly untestable features are rare, while untested implementations of features are unfortunately common. Check the tests themselves: are they covering interesting cases? Are they readable? Does the PR lower overall test coverage? Think of ways this code could break. Style standards for tests are often different from core code, but still important.
Does this code need integration tests? Sometimes, code can’t be adequately tested with unit tests alone, especially if the code interacts with outside systems or configuration.

Code Reviews for New Joiners

The codebase is new, the style of programming is different than before, and people review your code very differently. So should code reviews be gentler for new starters, to get them used to the new environment, or should they keep the bar just as high, as it is for everyone else?
- Use the same quality bar and approach for everyone, regardless of their job title, level or when they joined the company. Following the above, code reviews have a kind tone, request changes where needed, and will reach out to talk to reviewers when they have many comments.

Clean coding

Decide on the indentation and keep it that way, follow standard conventions.
- The code should be divided into logical units and it’s important to keep the style consistent throughout the whole document. In most of the programming languages, spaces and indentations do not affect the function. If you are using an IDE, it is recommended that you create a formatted config so everyone uses the same one.
- Make comments
  - Comments will help you and others understand why you did what you did.
    - Always try to explain yourself in code.
    - Don't be redundant.
    - Don't add obvious noise.
    - Don't use closing brace comments.
    - Don't comment out code. Remove it instead.
    - Use as explanation of intent.
    - Use as clarification of code.
    - Use as warning of consequences.
- Consistent name scheme
  - Be consistent. If you do something a certain way, do all similar things in the same way. Naming needs to stay consistent, otherwise, it will be very difficult to find things inside the document.
- Don’t repeat code
  - Repeating code will make your document very long and it will break the reading flow. If you have pieces of code that will be used more than once, it is preferable to make a separate file.
- Avoid writing long code lines
  - Writing long lines of code makes it very difficult to read, moving back and forth horizontally, losing any sense of what it’s actually written. Style and indentation will help with this.
- Break down a big task into smaller chunks
  - A whole new feature will never be just a few lines long. Even with comments, a 500-lines function will still be a pain to browse, understand and edit. Your best choice is to break down the big task into a smaller chunk of code.
- Organize your program into smaller files
  - Having a file with thousands of lines of code doesn’t help anyone, but broken down into shorter files organized based on function or feature will help you get right to the point when something needs fixing. Having the whole code organized in files and folders is very good for maintainability, especially if different teams and people are working on it.
    - Small.
    - Do one thing.
    - Use descriptive names.
    - Prefer fewer arguments.
    - Have no side effects.
    - Don't use flag arguments. Split method into several independent methods that can be called from the client without the flag.
- Keep it simple, write clever code that is also readable
  - Write clever code, but the main focus should be on readability and maintainability. If the code is kept short it is easier to go through, but if it’s too smart, it will take too much time to understand and edit it. You know that your code was too clever if you can’t read it after 3 months. So be clever but not too much!
- Code refactoring
  - Every developer knows this – you write some code because you want to finish the feature and in the end, it works. When you then look at it after some time you think – did I really write that? I could have shortened it so much! So you start rewriting that piece of code better, probably shorter, without changing any functionality. Ps. It is the developer’s responsibility to create tasks for refactoring the code.
- Delete unnecessary code
  - You wrote new code, great. Is the new code working? If yes, then just delete the old one! There is no point in keeping the old code, and comment it out. It will just look messy and unnecessarily long.
- Must avoid doing:
  - Rigidity. The software is difficult to change. A small change causes a cascade of subsequent changes.
  - Fragility. The software breaks in many places due to a single change.
  - Immobility. You cannot reuse parts of the code in other projects because of involved risks and high effort.
  - Needless Complexity.
  - Needless Repetition.
  - Opacity. The code is hard to understand.
  - Try to avoid temporary variables.
    - Using too many temporary variables can cause memory leaks, because of that try to use temporary variables only when:
      - statements are getting too long and too complex
    - Don’t use temporary variables when:
      - Return a value from a function:

Don’t do:

function name(){
  const message = "Hello";
  return message;
}

Do:

function name(){
  return "Hello";
}

My journey as intern at Ornio

EgzonaVllasaliu — Tue, 08 Sep 2020 06:16:12 +0000

My name is Egzona, I just wanted to share with you my jurney as an intern at Ornio as Full-stack developer from June - August of 2020.

I’m a 3rd-year Computer Engineer student from the University of Prishtina, Database and Artificial Intelligence branch.

This was my second internship, first one as Web developer that I completed, so I had a bit of experience with one other company, and I really had a good idea about what I did and did not like about being an intern!

I want to mention and thank everyone at ICK & partners for making this internship happen. After winning a scholarship and finishing successfully Coding Dojo bootcamp they gave me this opportunity, for which I will always be thankful.

First Two Weeks (First phase)

When I learned that I was going to be the only intern, I was excited to have the chance to shape the program and give feedback on what could be done better, and what to do more of.

That being said, the onboarding process was by far the smoothest and most efficient of any internship I’ve done. I spent the first day meeting other employees. In these meetings, they each went on a deep dive of their section to give me a real understanding of how everything works and how it all works together. This gave me a ton of insight into the organisation and its products.

First two weeks I had to watch some tutorials to get to know better the technology that the company was using. This was my first phase as intern.

What I Actually Worked On (Phase two)

After finishing the first phase, I had to work on an intern project and complete it by the end of the term. Beforehand I mentioned that I was interested in Full-stack development.

The team, in dialog with me decided that it will be a good challenge for me to create an app that helps with task estimation while doing Sprint planning.

Ornio uses Scrum and in Scrum methodology story points are used to estimate each task. Points follows the Fibonacci numbers when task are being estimated and therefore we named the project for just that, "FIBONACCI" :)

The stack

On Front-End we used a JavaScript framework called React.js with TypeScript, while for the Back-End we used Hasura (an extremely lightweight, high performance product that gives instant realtime GraphQL APIs on a Postgres database) additionally to that we needed some more advance functionality and for that we used Node.js with Express.js and TypeScript as well. JWT was used for authentication.

Live reloading of votes was accomplished using GraphQL subscription, which uses WebSockets under the hood.

This was a really cool project to work on because it allowed me to work with lots of different tools and technologies in additional to the above mentioned such as Sass, Docker, Kubernetes and more.

The functionality of the app

To fully use our application the users must be authenticated. Authenticated users can:

Create and join sessions
See their own sessions (sprint plannings), edit and delete them.
Manage their personal informations.
Can invite other teammates to a session (sprint planning) by sending them the session id/link.
Within a session (sprint planning), creator (the moderator) can manage (create/edit/delete) stories and initiate voting so other teammates / players can estimate them. When all the players have voted on a story the votes are displayed and one of them selected as estimation for a story (task).

Unauthenticated users can:

Join a session
Invite a teammate
Vote on ongoing sessions (sprint plannings).

The entire process of working on this project was interesting and much different from my previous projects. I was responsible for the project from start to finish. Being a part of the entire lifecycle was an eye-opening experience to how the team/company operates, and it allowed me to learn who to go to and when to ask for help to get things done quickly and efficiently.

The Experience

When I got added to the Ornio Events calendar, I was surprised. There were multiple, interesting events every week which, as an intern, I definitely took advantage of.

We had an event called “Competence Development”, where we used to learn new things everyday and then we had the chance to share our perspective of learning things with each other.

Every day we had “Daily touch base event” where we used to inform each other what we have done during that day. Another cool event was “Social event” where we used to create quizzes using Kahoot!, and then “compete” with each other who will get more answers right.

We also used Slack as communication tool with teammates, where we could chat and share resources that we found interesting, we also had a channel called team daily standup where we had to share with the team what we did yesterday and what we will be doing during that day, which I think was the best part because we had to start the day by planning it and then work toward things we said we will accomplish.

I was included in everything the team did, so when people ask about my internship experience at Ornio, I tell them that I pretty much forgot that I was an intern.

The End and Future

I learned a lot at Ornio about efficient web development processes, good code design and standards, all of which will be very helpful in my future career.

I really enjoyed being part of the intern program at Ornio, and I’m looking forward to starting a full-time job as web developer soon!

UPDATE:
This article was write before I finished the internship, at the end of the internship Ornio offered me a position as Junior Full-stack developer and I accepted :)