Hatena::Groupbugrammer

蟲!虫!蟲!

Esehara Profile Site (by Heroku) / Github / bookable.jp (My Service)
過去の記事一覧はこちら

なにかあったら「えせはら あっと Gmail」まで送って頂ければ幸いです。
株式会社マリーチでは、Pythonやdjango、また自然言語処理を使ったお仕事を探しています

 | 

2012-07-31

[]ウンコードに掲載されているコードは本当にウンコなのか 04:08

 ウンコード・マニアというサイトがリリースされて、ちょこちょことウンコなコードと思わしきものが投稿されていて、その話を知り合いと話をしたりしたら、「いやー、このコードって意外と意味があったりするんですよ」みたいな話になったりした。その辺の一部始終を今回はメモしておきます。

[VBA] それ、If使う必要ないから!!!-ウンコード・マニア

If a = b Then
    result = True
Else
    result = False
End If

「これは議論が別れるところだなあ。実際にサンプルで言われていた『result = a = b』のほうが気持ち悪いという人もいるし、こっちのほうが明確だよね、という話が出ていますね。自分としてはJavaScriptをやっているので、『result = a = b』と書かれると、『a = b』が比較演算子であることを忘れてしまう。あとはTrueとかFalseというのが無いのもありますよね」

「確かに、この場合の、比較で使われているa = bは、値としてTrueとFalseを生成して、それを判定しているから、もし比較の結果を代入するためにIfを使うくらいだったら、そのまま使った方がいいですよね。例えば、某サイトでは下のような式を見たりしました。

$hoge = $a === $b ? true : false;

これは、実質は下のように書き換えられる。

$hoge = ($a === $b);

「これはやっぱりVBAの特徴である、代入と比較が同じシングルイコールであるというのが問題だと思うんだよね。代入がシングルイコールで、比較がダブルイコールだったら、意図が伝わりやすいとは思うんだけど、今回の場合はそうではないわけで。ただ個人的にはカッコでくくってあるほうが好きだなあ。そのあたりはリーダブルさとの兼ね合いでありますね」

[PHP] 決して否定しない人。-ウンコード・マニア

if (hoge == 10) {
} else {
    echo "hogehoge";
}

「これは良くやるんですよ。さすがにこの例みたいなことにはならないけど、例えば、次のように二つの比較がある場合。

if ($hoge == 10 && $fuga == 3) {
} else {
    echo "hogehoge";
}

 これ、僕自身は次のコードより読みやすくて、さらに言えば面倒くさくないなあと思うわけです。

if ($hoge != 10 || $fuga != 3) {
    echo "hogehoge";
}

 あるいは下ね。

if (!($hoge == 10 && $fuga == 3)){
    echo "hogehoge";
}

「なるほど、これもウンコードというかは、プログラマーがそわそわしがちという意味がありそうな気がするなあ。自分がこのコードを遭遇したときのことを考えると、やっぱり『なんで処理が書かれてないの……』ってどうしても思っちゃう。もしありうるとするなら、コメントで『ここは実装されない』みたいな、意図を書いておく必要があるけど、コメントを書かなきゃいけないコードはどうしてもスマートとは言えないんじゃないか、っていう気がするね。Beautifulではない。

 あともう一つとしては、ブロックがあるよりは、ないほうが、その処理がないことが伝わりやすいってのはある気がする。ブロックがあると、ここの処理が無いのはおかしいんじゃないの、って思ったりする」

[JavaScript] 型宣言がないってステキやん-ウンコード・マニア

if( hoge != "" && hoge != null && hoge != "null" && hoge != false && hoge != "false" && hoge != NaN && hoge != "NaN" && hoge != 0 && hoge != "0" ){
alert('見えない恐怖に打ち勝った');
}

「これは、恐らくは型宣言が無いっていうより、nullなのかfalseなのかそれともNaNが入っているのかわからないから全部列挙するというパターンですね」

「残念!このコードにはundefindがありませんでした!」

「しかし、これはJavaScriptをかじった身としては、なんていうか別の意味で問題を感じるコードかなあ。たぶん、このコードを書いた人は他の言語を書いていて、いきなりJavaScriptにまわされたのかなーとか思ったりするんですけど、実はfalse値は、ダブルイコールだと勝手に0になったり、""にキャストされたりしますからね。ただ、nullやNaNにはキャストされないから、いいのかなー、どうなんだろうな。まあこういう形式で列挙するなら '!==' にしておいたほうが無難でしょうね」

「そのほうが、元の意図を伝えやすいところではありそうだなあ」

[Java] static おじさんもびっくり!?-ウンコード・マニア

public class MyUtil {
 
  public static boolean isValid(MyData data) {
    return data.isValid();
  }
 
}

「実は、これ意外とやるんですよ。恐らく、この人が疑問を持っているのは、MyUtilに渡されたインスタンスに、既にメソッドがあって、それを呼び出すなら、それを使えばいいじゃないの、って話だと思うんですけれども、たまーに必要なデータ構造が変化することがあって、相手のソースがそれを変更しないとか、あるいは対応に遅れる場合は、一度同じメソッドでWrapperしてあげることで、その変更に対応する、ということはあります」

「なるほど、Fixが完了したら、受け取ったインスタンスメソッドをそのまま渡してあげると」

「はい。例えば、次のような感じですかね。

public class MyUtil {
    public static boolean WrapperHoge(MyData data) {
        int result = data.hoge();
        result = this.prefixSomethingMethod(result);
        return result;
    }
}

 で、修正がおきたときに、prefixSomethingMethodの処理を消すと。あとから見ると何がおきたのかさっぱりわからなくなりますね」

「場合によっては、最初のコードが

public class MyUtil {
    public static boolean WrapperHoge(MyData data) {
        return true;
    }
}

 みたいな感じだったのかな。でもだとするとeclipse使え!とかそういう話になっちゃいそうだな」

Java書いている人がeclipseを使っていないのは想像できないですけどね。もちろん、Netbeansとか、他のを使う場合もあるとは思いますが」

[Java] 無駄にString-ウンコード・マニア

class Rect {
	private int width;
	private int height;
	/**
	 * コンストラクタ
	 * @param str 例 "100x200"
	 */
	 public Rect (String str) {
		String[] args = str.split("x");
		width = Integer.parseInt(args[0]);
		height = Integer.parseInt(args[1]);
	}
	…
}
class Point {
	private int x;
	private int y;
	/**
	 * コンストラクタ
	 * @param str 例 "(100,200)"
	 */
	public Point (String str) {
		String substr = str.substring(1, str.length()-1);
		String[] args = substr.split(",");
		x = Integer.parseInt(args[0]);
		y = Integer.parseInt(args[1]);
	}
	…
}
 
Rect rectA = new Rect("320x480");
Rect rectB = new Rect("640x800");
 
Point pointA = new Point("(160,240)");
Point pointB = new Point("(320,400)");

「個人的には好きなアプローチではあるんですけどね。Pointの部分はウンコですけど、Rectの部分は『これダメなの』っていう人もいるんじゃないかな。ただ、キャストとかSplitの部分で、微妙にパフォーマンスが落ちるかもしれないから、という実務的な部分はありそうですけどね。自分としては、Stringで渡すことによって読みやすくなっている側面があるから嫌いじゃない」

「難しいですね。自分の立場としては、ユーザー入力の問題で無いならば、こういうアプローチにするのはやめたほうがいいな、という立場ですね。というのは、その関数が欲しいデータというのは数値であって、その文字列自体じゃないわけなので。できるだけ、その関数が欲しいと思っているような形式で渡すのが望ましいかなとは思います」

[C] strlen が好き-ウンコード・マニア

for (int i = 0; i < strlen(str); i++) {
   some_func(str[i]);
}

「これは油断しているとうっかりやっちゃうタイプですね。同様の問題はJavaScriptにもあるので、"var len = hoge.length"とか代入しています」

「要するに、これは次のブロックを処理するかのときに、変数と比較するんですが、変数と比較するたびにわざわざ関数が走るわけで、それはとても効率が悪いから、なんらかの形で結果を入れておくほうがベターだって話だとは思うんですね。たぶん、C言語のstrlenって、文字列があるかどうかをひとつずつ数えているわけで。あと、これってもしかしたらstrの長さが変わったりしたら、無限ループだったりするわけですよね」

「そういう意味では、バグの可能性が潜んでいる気がします」

まとめ

 ……というような話をしていたのですけれども、クソと思うのは人間であるとするならば、そのコードを「クソ」と呼ぶことは、「クソ」とコードを呼ぶその人の力量が同時に判断されるという側面もあるのだなという意味で、日々勉強が必要だなという気がします。つい最近でも、某コードサイトでろくでもない指摘をしてしまったことがあるので。そういう意味では、クソを覗き込むものは、またクソに覗き返されるということなのかもしれません。

 |