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.
# 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
# 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
# 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
# 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"
NG
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
NG
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
Recommended Posts