What I pointed out as a reviewer for half a year

For the past six months, I've been working on a project in Scrum as a team. Regarding the program, I took the stance of merging if I was seriously asked to review it and it was OK.

I carefully selected what I pointed out to the members in the review and summarized it into six. Since it was developed with ruby, I think that there is a part closer to ruby, but basically I think that it can be said in any language.

1. Use nouns for class names and verbs for method names

# NG
class RequestUser
  def email
    ...
  end
end

In object-oriented programming, class names are basically nouns and method names are verbs.

Classes are objects and things, so be sure to give them nouns. Methods do something, so be sure to add a verb.

# OK
class UserRequests
  def send_email
    ...
  end
end

2. Clarify whether the variable name is singular or plural

# NG
user = User.all

I'm getting all the data in the user table with User.all. Multiple users should be returned, but since the variable name is user, is it a single user? I will misunderstand. Be sure to name the variable that contains multiple data such as an array as the plural form, and the variable name for a single unit as the singular form.

# OK
users = User.all

3. I want to avoid the reverse in the judgment process

# NG
if !notSelected

Well ... the opposite of selected ...? ?? When I look at the code, two processes are added and the readability is reduced. I think it is better to avoid using variables that have not or un as much as possible.

# OK
if selected

4. Add #TODO to those that will be modified later

# NG
#Set dummy data and modify later
user_name = "cure_milky"

Dummy data may be set for the time being because the specifications have not been decided. It's good to leave a comment, but sometimes even the specs are forgotten and left forever. At the very least, I think it's better to put TODO in the comment. If there is TODO in the code with IDE etc., it will be listed in many cases, so it is easy to notice. (Still, I may forget it.)

# OK
# TODO:Set dummy data and modify later
user_name = "cure_milky"

5. Place the positive button on the right and the negative button on the left

NG FireShot Capture 014 - Edit fiddle - JSFiddle - Code Playground - jsfiddle.net.png

All the buttons are on the right side, and they all look the same when viewed on the pad, and the visibility is poor. It is easier to understand if the next item (positive item) is placed on the right to emphasize it, and the item that returns (negative item) is moved to the left side, making it less likely that a mistake will occur.

OK FireShot Capture 011 - Edit fiddle - JSFiddle - Code Playground - jsfiddle.net.png

6. Concise dialogs and make button expressions easy to understand

NG FireShot Capture 017 - Edit fiddle - JSFiddle - Code Playground - jsfiddle.net.png

It's a common dialog, but the text is a bit verbose and it takes time to understand the content. In such a dialog, it seems that you will press "Yes" without thinking about anything. In the dialog, it is much easier to understand if the text is concise and clear about what to do, and the expression of the button also describes what to do.

OK FireShot Capture 020 - Edit fiddle - JSFiddle - Code Playground - jsfiddle.net.png

Recommended Posts

What I pointed out as a reviewer for half a year
Summary of what I was pointed out after receiving a code review as a beginner
Write what you thought after using mybatis for half a year
[Note] What I learned in half a year from inexperienced (Java)
[Note] What I learned in half a year from inexperienced (Java) (1)
Awareness of object-oriented programming for half a year
What was good for a fledgling engineer who started programming for a year and a half, and what I thought I should have done
I made a SPA with Rails + Nuxt.js for half a year of self-study, so please take a look.
A story that I finally understood Java for statement as a non-engineer
[For programming beginners] What is a method?
I made a plugin for IntelliJ IDEA