Feature #10058
closed[PATCH] Fix some coding styles
Description
When I read codes, I notice some issues about coding style.
If you don't mind that, you can ignore my patches. It's just coding style problem.
コードを読んでいる中でコーディングスタイルに関して気になる事があったのでPATCHを投げますが、
こちら(redmine, rubyコミュニティ)のコーディングスタイルに対する温度感が掴めてないので
気にする事でなければ、無視してください.
あげると切りがないので今回は特に気になる以下の三点に絞りました.
-
There are both
&&
andand
. So, I Unify with&&
-
There are both
send :xxx
andsend(:xxx)
expressions. So, I Unify withsend(:xxx)
-
I reorder methods. I think Matrix#real should be upon Matrix#imaginary(From what I see complex.c)
Files
Updated by ngoto (Naohisa Goto) about 10 years ago
There are both send :xxx and send(:xxx) expressions. So, I Unify with send(:xxx)
I think "send" should be replaced with "send", because the Japanese version of reference manual for Object#send says that libraries should use "send".
http://docs.ruby-lang.org/ja/2.1.0/class/Object.html#I___SEND__
"send" は "send" にするのがよいと思います。なぜなら、Object#send の日本語のリファレンスマニュアルには「"send" が再定義された場合に備えて別名 "send" も 用意されており、ライブラリではこちらを使うべきです。」と書いてあるからです。
Updated by ngoto (Naohisa Goto) about 10 years ago
There are both && and and. So, I Unify with &&
Be careful that "&&" and "and" have different operator precedence, and they can not always be simply replaceable.
&& と and は演算の優先順位が異なるので、単純に置換できない場合もあるのは要注意です。
http://qiita.com/fukayatsu/items/920ad8e4d099c3fec890
By the way, FYI, Rails coding guideline says "Prefer &&/|| over and/or".
関係ありませんが参考までに、Railsのガイドラインには&&/||のほうをand/orより優先して使うように書いてありますね。
http://guides.rubyonrails.org/contributing_to_ruby_on_rails.html#follow-the-coding-conventions
Updated by marcandre (Marc-Andre Lafortune) about 10 years ago
- Assignee set to marcandre (Marc-Andre Lafortune)
Updated by marcandre (Marc-Andre Lafortune) about 10 years ago
Even though it can be a pain, please split unrelated patches in different requests.
- Unify && vs and:
Thanks, this is good.
- I reorder methods. I think Matrix#real should be upon Matrix#imaginary
Methods are sorted alphabetically within sections. I believe this was the original convention when I became committer. I see no compelling reason to change this.
- There are both send :xxx and send(:xxx) expressions
Sure, I don't mind.
Updated by marcandre (Marc-Andre Lafortune) about 10 years ago
Naohisa Goto: Good point about __send__
, I imagine it is to avoid classes that redefined send
? In any case, I revised the uses of send
in the Matrix lib and they are all to call our own private methods, so I think it's fine like this.
Updated by ngoto (Naohisa Goto) about 10 years ago
Naohisa Goto: Good point about send, I imagine it is to avoid classes that redefined send? In any case, I revised the uses of send in the Matrix lib and they are all to call our own private methods, so I think it's fine like this.
Yes, using send is fine when calling Matrix classes' methods which are under your maintenance. When calling other classes which may not have the original behavior of send method, using "send" is better.
Updated by gogotanaka (Kazuki Tanaka) about 10 years ago
@ Mr. Marc-Andre Lafortune
Even though it can be a pain, please split unrelated patches in different requests.
Sorry, I'm not familiar with how to use redmine. so you mean I need to make new 2 features?
(About unify_product_operator.patch, about reorder_Matrix#real_and_Matrix_imaginary.patch)
- I reorder methods. I think Matrix#real should be upon Matrix#imaginary
Methods are sorted alphabetically within sections. I believe this was the original convention when I became committer. I see no compelling reason to change this.
OK, I got your point. I will try that from now.
@ Naohisa Gotoさん
承知致しました. なるほど、今回のようにとりわけ理由がなければ __send__
を使うべきであると言う事はよくわかりました.
一つ賢くなれました. ご鞭撻ありがとうございました!
&& と andの優先順位についてもありがとうございました.
Updated by marcandre (Marc-Andre Lafortune) about 10 years ago
gogo tanaka wrote:
Sorry, I'm not familiar with how to use redmine. so you mean I need to make new 2 features?
Yes, ideally. If they can be discussed separately, they should be separate issues.
No need for this issue, it's already been committed.
Updated by gogotanaka (Kazuki Tanaka) about 10 years ago
Marc-Andre Lafortune wrote:
Yes, ideally. If they can be discussed separately, they should be separate issues.
No need for this issue, it's already been committed.
I see, I try this next time.
Thank you for your committing my patches.
Updated by marcandre (Marc-Andre Lafortune) over 9 years ago
- Status changed from Open to Closed