Feature #10058: [PATCH] Fix some coding styles

• Status: Closed
• Assignee: marcandre (Marc-Andre Lafortune)
• Target version: -

[gogotanaka (Kazuki Tanaka)]

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 && and and. So, I Unify with &&

• There are both send :xxx and send(:xxx) expressions. So, I Unify with send(:xxx)

• I reorder methods. I think Matrix#real should be upon Matrix#imaginary(From what I see complex.c)

[ngoto (Naohisa Goto)]

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" も 用意されており、ライブラリではこちらを使うべきです。」と書いてあるからです。

[ngoto (Naohisa Goto)]

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

[marcandre (Marc-Andre Lafortune)]

• Assignee set to marcandre (Marc-Andre Lafortune)

[marcandre (Marc-Andre Lafortune)]

Even though it can be a pain, please split unrelated patches in different requests.

1. Unify && vs and:

Thanks, this is good.

2. 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.

3. There are both send :xxx and send(:xxx) expressions

Sure, I don't mind.

[marcandre (Marc-Andre Lafortune)]

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.

[ngoto (Naohisa Goto)]

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.

[gogotanaka (Kazuki Tanaka)]

@ 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)

2. 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の優先順位についてもありがとうございました.

[marcandre (Marc-Andre Lafortune)]

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.

[gogotanaka (Kazuki Tanaka)]

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.

[marcandre (Marc-Andre Lafortune)]

• Status changed from Open to Closed