くそコードが無くなる日を目指して

実例はC++/Javaがメインです|良いコードは健康を増進します

Category: ひとりごと

ソースコードを見てて最近気になったこと。
public void someCalculation(long number1, long number2) {
...
    // 最大公約数を計算する
    long a = number1;
    long b = number2;
    while (b != 0) {
        a = b;
        b = a % b;
    }
    long gcd = a;
...
}
みたいなコメント。
コメント以下の部分の説明をしていて、丁寧!ってことなんだと思いますが、あまりいけてないと思います。
いけてないポイントたち
1)コメントの対象がどこまでなのかはっきりしない。
2)コードが正しいかどうか検証しにくい
3)一行コメント書くくらいなら、名前をつければいい

なので、 こういうなんか以下は〜です系のコメントは、
public void someCalculation(long number1, long number2) {
...
    long gcd = computeGreatestCommonDivisor(number1, number2);
...
}

public long computeGreatestCommonDivisor(long a, long b) {
    while (b != 0) {
        a = b;
        b = a % b;
    }
    return a;
}
みたいにしたほうが良いと思う 。
こうすれば、
1) どこまでが計算なのかがはっきりする
2) メソッド化したので、ユニットテストできる
3) 結局コメントと同じ効果が得られる
 
じゃあどういうコメントが良いのか。
いろいろな意見はあると思いますが、何しているのかではなく、なぜこう書かれているのかを説明するコメント。
さっきの例ではこんな感じ。
public void someCalculation(long number1, long number2) {
...
    long gcd = computeGreatestCommonDivisor(number1, number2);
...
}

public long computeGreatestCommonDivisor(long a, long b) {
    // ユークリッド互除法を使って計算を高速化
    while (b != 0) {
        a = b;
        b = a % b;
    }
    return a;
}

 

三項演算子って嫌われる風潮がありませんか?
なんか特に理由もなく三項演算子は醜い、読みづらい→悪!的な・・・

個人的には使いどころ次第だと思うんですよね。
三項演算子NG!と言ってる人はたぶんこういうのを気にしてるんですよね
int number = (high > low) ? highNumber : (middle > low) ? middleNumber : lowNumber;
三項演算子がひたすらつながっているようなもの

確かに三項演算子はつなげたら読みにくいので、良くないと思います。
でも繋げないときはむしろいいことのほうが多いので、個人的には(繋げないという条件付きで)三項演算子は使うのを推奨します。
例えば、初期化するときに
int number = 10;
if (condition) {
    number = 20;
}
int number;
if (condition) {
    number = 20;
} else {
    number = 10;
}
より
int number = (condition) ? 20 : 10;
のほうが良いと思いませんか?
だって、最初のは一度10を入れてるけど、条件によっては20がその後で代入されてて、それって初期化なの?初期化って最初に入れる値が一意に定まってるべきじゃない?って感じですし、2つ目のは初期化するのに5行も使うの????って感じじゃないですか。
まだ、上のは一つしかないから耐えられるかもしれないですけど、2つ以上初期化したかったらどうするんですか?
int number = 10;
if (condition) {
    number = 20;
}
int number2 = 20;
if (condition2) {
    number2 = 30;
}
int number3 = 30;
if (condition3) {
    number3 = 40;
}
...
みたいに書くんですか???初期化に10行も20行も費やして良いんですか???number5を見る頃にはnumberがいくつに初期化されてるかなんて忘れてます…
そう考えると、三項演算子で
int number = (condition) ? 20 : 10;
int number2 = (condition2) ? 30 : 20;
int number3 = (condition3) ? 40 : 30;
の方が絶対いいですよね。初期化の行数も変数の数分だし。

あと最近みたんですが、こういうのも良くないと思うので書き直すべきです。
if (condition) {
    functionA(argument1)
} else {
    functionA(argument2)
}
...
を、
functionA((condition) ? argument1 : argument2)
...
のように。
これはただ短くなるからという話ではありません。
functionAは共通して適用したい操作なのにもかかわらず、条件分岐されて、二箇所に書かれているというところがポイントです。
つまり、conditionで分かれているのは引数だけなのに、functionAを適用するという操作まで、条件に入ってしまっているところが問題です。
共通だからくくりだすとはよく言われることなんですが、そもそも、何が条件によって分かれているの?っていうのを考えたらfunctionAは一箇所にしか現れないはずなんですよね。
まぁ、これは三項演算子とは少し違う話ですが。

とりあえず、このブログは三項演算子は結構いけてると思っています
 

オープンソースの世界では、GPLを始めとして、ApacheライセンスやMITライセンスなどなど、Copyleftなライセンスがよく使われています。

このブログもくそコードが無くなる日を目指して、皆さんと知恵や経験を共有するため、記事をクリエイティブ・コモンズ・ライセンスで公開しています。

ライセンスの詳しい中身はブログ内のバナーから見られますが、
簡単には、記事(ソースコード含む)の出処が明記されていれば転載等、商用・非商用問わず利用は自由というライセンスです。

記事はどんどん使って構わないので、是非、使ってください。

オープンソースの世界のライセンスの話はまた別のときに書こうと思います。

コードをずっと書いていると、目が疲れてくるので薦められて最近こんなの買いました。

これのいいところは電子レンジで30秒チン(600W)するだけで
あの、蒸気で…とは違って何回も使えるところ。
 
みなさんもよかったら是非。

コードの生産性が上がります。多分。。。

こんばんは。
今日は短めに。こんなのはやめましょうという話。
 
    try {
        exceptionThrower();
    } catch (Exception e) {
        // Catch everything
    }

ExceptionがexceptionThrower(注:これはこの例のために作ったメソッドです)から投げられて、なんかtry〜catchを書かないとコンパイルが通らないという理由で上のように書くのはやめましょう。 めんどくさいからとりあえず、Exceptionでキャッチしとけ!はやめましょう。 最初想定したExceptionだけキャッチするようにしましょう。 全部キャッチしてしまうと、想定外のバグが埋もれたり、変なところに影響がでて迷宮入りしやすくなります。

↑このページのトップヘ