クソコードとは何か

**デフォルトの名無しさん** · 2021/01/30(土) 17:33:05.78

このスレはクソコードとは何かを考えるスレです。

・親クラスが子クラスに依存する処理を持つコード
例...社員クラスを継承した正社員クラスと派遣社員クラスがあり、社員クラスが正社員クラスの知識を持つ状況

・staticにするべきではないモデルにまでstaticにする人
例...社員クラスのメソッドを全てstaticにしたり、社員クラスにシングルトンパターンに相応するものを適用する人

等、クソコードを見た時に「あっ、これクソコードだ」って認識する根拠を挙げていきましょう。

**デフォルトの名無しさん** · 2021/01/30(土) 17:33:42.44

もうレスバの予感しかしない。
産まれたての子鹿のように震えてるよ。

**デフォルトの名無しさん** · 2021/01/31(日) 10:20:47.27

>>1
クソはお前の頭だよ

**デフォルトの名無しさん** · 2021/01/31(日) 11:28:04.42

iostream ωωω

**デフォルトの名無しさん** · 2021/01/31(日) 12:06:18.22

>>3
staticおじさん乙

**デフォルトの名無しさん** · 2021/01/31(日) 12:22:48.25

開幕から自分のコードに突き刺さって痛い

**デフォルトの名無しさん** · 2021/01/31(日) 12:46:25.58

．　／⌒ヽ
　 ∩ ＾ω＾）　な　ん　だ
　 |　　 ⊂ﾉ
　｜　　 _⊃
　し ⌒．　　　＿＿＿
　　 _／ ⌒ ⌒ ＼うん、うん、
　　/)) (●) (●) 　ヽそれで？
　｜∩　(_人_)　 |
　 / ノ､＿ヽノ＿ノ￣)
　/ /　　　　　 /ﾌ_/
　L＿／＼　　　＼(

**デフォルトの名無しさん** · 2021/01/31(日) 12:59:59.35

テストコードが無いのにテストしたとか言うプログラマーの提出するコード

**デフォルトの名無しさん** · 2021/01/31(日) 16:11:11.70

同じコードを何度も書く

**デフォルトの名無しさん** · 2021/01/31(日) 16:33:43.28

コードとコードつなぎ合わせて一つのプログラム作ったけどごちゃごちゃでクソコードになってたw

**デフォルトの名無しさん** · 2021/01/31(日) 16:59:37.31

function getPrice(id) {
const price = clothPrice.filter({ id }).first()
return price
}

はい、どこがクソか指摘してみて

**デフォルトの名無しさん** · 2021/01/31(日) 17:27:33.78

どこもクソじゃない

**デフォルトの名無しさん** · 2021/01/31(日) 17:28:15.94

>>11
getPrice関数いらなくね？
...で合ってる？

理由の指摘の仕方はどうしようかな

**デフォルトの名無しさん** · 2021/01/31(日) 17:29:51.90

>>13
理由を言えないならそれはレビューではない

**デフォルトの名無しさん** · 2021/01/31(日) 17:33:42.78

まず、このコードを見てクソだと気がつけない人相手だと、言い方が重要。
だから、説明に困ってる。
なぜ、気が付かないのかとか。
考えてるだけだよ。気がついた人は先に理由を述べてほしいが。

**デフォルトの名無しさん** · 2021/01/31(日) 17:35:04.85

>>15
相手のことはまず忘れろ。
理由を言え。

そうすればあとはその理由をどう言えば伝わるかという
次の話に行けるだろ

**デフォルトの名無しさん** · 2021/01/31(日) 17:40:45.07

getPrice(id) 関数とclothPrice.filter({ id }).first()
ってやってる事は一緒だよね？
責務も殆ど一緒。
些細な違いと言えばfirst以外を呼べなくしたことだが...
たぶん、インスタンスの名称からしてclothPriceをユーザー側に隠蔽する必要も無いのに隠蔽し、挙げ句に役割のかぶったほぼ無意味な関数を無駄に増やしてしまってる。

だから、クソ...だと思った。

**デフォルトの名無しさん** · 2021/01/31(日) 17:42:50.72

あと、一言で言えば...もしかしてドメインモデル貧血症？

**デフォルトの名無しさん** · 2021/01/31(日) 17:46:41.31

> getPrice(id) 関数とclothPrice.filter({ id }).first()
> ってやってる事は一緒だよね？

関数とその中のコードが
やってることが一緒になるのは当たり前です。

関数にする意味は、コードの可読性を上げるためです。

「clothPrice.filter({ id }).first()って何やってるの？」
「Priceをgetしてます！（ getPrice(id)）！」

という会話がコードレビューであるだろうなと少しでも思ったら
それは関数にすることで可読性が上がっているということです。

**デフォルトの名無しさん** · 2021/01/31(日) 17:57:42.08

>>19

> > getPrice(id) 関数とclothPrice.filter({ id }).first()
> > ってやってる事は一緒だよね？
>
> 関数とその中のコードが
> やってることが一緒になるのは当たり前です。

実装対象の仕様が1行程度のコードで住んでいる点をまず気にして。

> 関数にする意味は、コードの可読性を上げるためです。

可読性は上がったのだろうか？
だって、何のインスタンスのオブジェクトが返されるか名前から想像できますか？

**デフォルトの名無しさん** · 2021/01/31(日) 18:00:20.47

> だって、何のインスタンスのオブジェクトが返されるか名前から想像できますか？

逆。「何のインスタンスのオブジェクトが返されるか」を意識してはいけない。
それはかっこいい言葉で言うならデメテル法則に反している
実装の詳細を意識させるな

**デフォルトの名無しさん** · 2021/01/31(日) 18:02:13.86

>>11

clothPrice.filter({ id }).first()
だったら...clothPriceインスタンスのPriceが帰ってくるんだなーって一目で分かるけど

getPrice(id)
だったら...え？何が帰ってくるのこれ？
ってなりませんか？
仮にJavaみたいな型宣言が必要な言語だったとしても、何のインスタンスのPriceが得られるのかわからないですよね？

...って、これが本当の理由かっ！？
色々、問題が想像つくから、回答に迷ってる

**デフォルトの名無しさん** · 2021/01/31(日) 18:03:52.46

> getPrice(id)
> だったら...え？何が帰ってくるのこれ？
> ってなりませんか？

そのオブジェクトのpriceだろ
そのオブジェクトが内部で使用している～とかを意識しないといけないのはデメテルの法則違反

**デフォルトの名無しさん** · 2021/01/31(日) 18:04:38.99

クソコードじゃないって最初から言ってるだろ

**デフォルトの名無しさん** · 2021/01/31(日) 18:05:06.22

>>21
setじゃなくて、getだよね？
それって、つまり...誰かにgetを呼ばせる前提の関数なんですよ。

あなたに問いますけど、getPriceって誰に何の為に呼んでもらう関数だと思いますか？

**デフォルトの名無しさん** · 2021/01/31(日) 18:06:51.44

> あなたに問いますけど、getPriceって誰に何の為に呼んでもらう関数だと思いますか？

このコードではそれは書いてないのだから、そこを勝手に想像して
自分の都合の良い解釈をするのは、それ自体が間違い

**デフォルトの名無しさん** · 2021/01/31(日) 18:07:52.12

出題者さんの回答が知りたいな...。
まぁ、二人しか答えてないけど。

**デフォルトの名無しさん** · 2021/01/31(日) 18:13:16.87

>>26
> このコードではそれは書いてないのだから、そこを勝手に想像して
> 自分の都合の良い解釈をするのは、それ自体が間違い

では、clothPrice.filter({ id }).first()は何をしているメソッドだも思いますか？
こっちは...なんとなくわかりませんか？
私はstiftを触ったことないですが、似たような言語で似たような記述をしたことがあるので推測できます。わかりやすいですね。

**デフォルトの名無しさん** · 2021/01/31(日) 18:14:04.75

> では、clothPrice.filter({ id }).first()は何をしているメソッドだも思いますか？

getPriceをしてるメソッド

**デフォルトの名無しさん** · 2021/01/31(日) 18:14:59.04

って、よく見たらjsか！？まぁ、どっちでもいいや。
何となくでわかる記述って大切じゃないですか？

**デフォルトの名無しさん** · 2021/01/31(日) 18:19:27.38

>>30
理由が言えないのを「何となくこっちが良いと思った」でごまかすなよ
それは理由がないという

**デフォルトの名無しさん** · 2021/01/31(日) 18:21:37.30

理由は必死に説明しているつもりなんだけどなぁ...。
こうなるから慎重に説明を考えていたんだけどなぁ...

**デフォルトの名無しさん** · 2021/01/31(日) 18:22:44.83

何となくでわかるの意味が分かってないことが分かった...。そもそも、filterの意味が分かってない？

**デフォルトの名無しさん** · 2021/01/31(日) 18:25:20.39

何となくこっちが良い
じゃなくて
コードを見たとき、記述から実装内容を察することができることが重要と言うべきだったかな...
言葉選びを間違えるとこうなるから慎重に説明をしないとなんだが...うーん、説明下手orz

**デフォルトの名無しさん** · 2021/01/31(日) 18:27:15.53

実装者の視点しか持ってないんだよな。

クラス設計というのは、どういうインターフェースにすれば
クラスが"使いやすいか" というクラスの利用者からの視点で設計する

getPriceというメソッドではなく、その中のコードの実装の
話をしてる時点で視野が狭いのは明らか
テストコードも書いたことないだろ？
テストコードは必然的に使いやすいクラスであることが要求されるからな

**デフォルトの名無しさん** · 2021/01/31(日) 18:27:53.96

> コードを見たとき、記述から実装内容を察することができることが重要と言うべきだったかな...

実装の詳細は隠蔽しろって習わなかったのか？

**デフォルトの名無しさん** · 2021/01/31(日) 18:31:40.92

別に、実装者だけではなくライブラリユーザーの事を考えてますよ。
実際、ライブラリ開発者ですしお寿司。

じゃなければ、getPriceって誰か何のためにーなんて質問をする発想はないよ。
たしかに、実装の詳細は隠蔽するべきだね。

でも、私が注目しているのは、まさにユーザビリティに相当する部分です。

**デフォルトの名無しさん** · 2021/01/31(日) 18:33:13.55

じゃあ実装の詳細を無視して話をしよう

function getPrice(id) {
・・・隠蔽。1行かもしれないし100行かもしれない・・・
}

**デフォルトの名無しさん** · 2021/01/31(日) 18:35:19.06

1.getPriceが必要な場面を想像できますか？
→想像すること自体が間違いだと回答しましたが、はいorいいえ　でお願いします。
2.それが想像できないのはなぜですか？

**デフォルトの名無しさん** · 2021/01/31(日) 18:37:59.05

1.getPriceが必要な場面を想像できますか？
いいえ。

2.それが想像できないのはなぜですか？
出題に書いてないからです。

逆に出題に「これは商品オブジェクトです」と書いてあれば
getPriceが必要な場面を想像できます。

**デフォルトの名無しさん** · 2021/01/31(日) 18:39:36.56

>>38
もしかして...getPriceがクラスメソッドの実装ではない事に気がついていない？

**デフォルトの名無しさん** · 2021/01/31(日) 18:41:32.55

>>41
わかってるから"実装を取り除いて"、実装"以外"を書いたんですが？
その質問の意図はなんですか？

**デフォルトの名無しさん** · 2021/01/31(日) 18:43:52.76

じゃあ、もしもですよ？出題内容が
sin関数だったらどうですか？
>>40と同じノリで答えてください。

**デフォルトの名無しさん** · 2021/01/31(日) 18:45:22.65

>>43
何オブジェクトのsin関数ですか？

https://ejje.weblio.jp/content/sin
主な意味　(宗教・道徳上の)罪、罪業、過失、違反、ばち当たりなこと、ばかなこと

**デフォルトの名無しさん** · 2021/01/31(日) 18:46:09.44

>>44
いえいえ、ただのsin関数ですよ。数学の。

**デフォルトの名無しさん** · 2021/01/31(日) 18:47:29.69

>>45
同じノリで答えろと言われたから、同じノリで答えただけですが？

出題に、何オブジェクトのsinか書いてないから想像できない。
数学オブジェクトのsinと出題に書いてあれば想像できます。
>>40と全く同じノリで答えましたが？

**デフォルトの名無しさん** · 2021/01/31(日) 18:49:00.47

...もしかして、三角関数知らない？
もうギブアップ。

**デフォルトの名無しさん** · 2021/01/31(日) 18:49:45.66

>>47
sinという言葉には色んな意味があります。
出題に書いてないことを勝手に想像しないように
同じノリで答えてますｗ

**デフォルトの名無しさん** · 2021/01/31(日) 18:50:29.12

酷い意地悪を見た

**デフォルトの名無しさん** · 2021/01/31(日) 18:54:20.45

面白すぎるだろこのやり取りw
最初から相手の話を聞く気がない人に説明をしても無駄無駄

**デフォルトの名無しさん** · 2021/01/31(日) 18:54:30.12

諦めろ>>49！お疲れ様！

**デフォルトの名無しさん** · 2021/01/31(日) 18:58:21.14

stdnum.ca.sin SIN (Canadian Social Insurance Number).

https://arthurdejong.org/python-stdnum/doc/1.15/stdnum.ca.sin#module-stdnum.ca.sin

実際にあるみたいだなｗ

**デフォルトの名無しさん** · 2021/01/31(日) 18:59:23.14

sin関数は一目で具体的な用途が判るが、getPriceはそうじゃないし使い勝手もイマイチだって言いたいんだろ
察してやれ

**デフォルトの名無しさん** · 2021/01/31(日) 19:00:26.13

>>50
ずっと答えてますが？

数学オブジェクトと出題に書いてないのに
想像で数学オブジェクトだと決めつけるのは良くないですよ？
違いますか？

厳密な定義ができるのはプログラマの能力の一つでしょう

**デフォルトの名無しさん** · 2021/01/31(日) 19:01:36.35

>>53
だから、getPriceには出題に書いてないからわからない。
これが商品オブジェクト、 product.getPrice() と
出題に書いてあればわかると答えたんですが？

**デフォルトの名無しさん** · 2021/01/31(日) 19:03:23.18

>>55
> だから、getPriceには出題に書いてないからわからない。
↑これが問題なの。

**デフォルトの名無しさん** · 2021/01/31(日) 19:04:29.63

>>53
よくわかりましたね。正解。

**デフォルトの名無しさん** · 2021/01/31(日) 19:05:57.07

>>56
俺の答えが正解っていいたいのか？ｗ

俺が答えを言ってから、後出しで同じだって言うなよｗｗｗ

**デフォルトの名無しさん** · 2021/01/31(日) 19:09:21.55

function getPrice(id) {
const price = clothPrice.filter({ id }).first()
return price
}

これが一行（二行）だから関数にすべきでないとか
中身を直接書けば何やってるかわかるから関数にすべきでないというならば、

tan（数学のタンジェント）関数はなぜ用意されているのでしょうか？

double tan(double x) {
return sin(x) / cos(x);
}

たった一行です。中身を直接書けば何をやってるかわかります。
同じ理屈であれば、不要なはずです。
そういう結論になるはずです。

tanを関数にする理由は明らかですね。最初に言った理由と同じです
「clothPrice.filter({ id }).first()は何やってるの？」という質問が出るのであれば
それを関数にすることで可読性が上がるのです。中で何をやってるかは
意識させないようにするのが正しい設計です。

**デフォルトの名無しさん** · 2021/01/31(日) 19:22:21.34

>>58
だって貴方、>>12でどこもクソコードじゃないって答えたじゃん

**デフォルトの名無しさん** · 2021/01/31(日) 19:24:32.72

コードはクソじゃないですが？
出題に書いてないって言うのは、出題がクソという意味ですが？

いい加減諦めたら？ｗ

**デフォルトの名無しさん** · 2021/01/31(日) 19:24:56.89

風呂入ってたら議論が白熱してた
俺どうすればいい？

**デフォルトの名無しさん** · 2021/01/31(日) 19:25:29.66

オナニーしてねろ

**デフォルトの名無しさん** · 2021/01/31(日) 19:26:58.02

はい

**デフォルトの名無しさん** · 2021/01/31(日) 19:28:30.31

おかえり
もう、疲れたよパトラッシュ...

ところで、パトラッシュ、答えが知りたいな

**デフォルトの名無しさん** · 2021/01/31(日) 19:30:52.25

てゆーか、答え言っていいの？
一応想定してた回答はあるけど

**デフォルトの名無しさん** · 2021/01/31(日) 19:32:35.81

いいよ！気になる。

**デフォルトの名無しさん** · 2021/01/31(日) 19:34:13.36

すでに指摘した点を蒸し返すなよ？

**デフォルトの名無しさん** · 2021/01/31(日) 19:35:02.92

せめてデメテルの法則ぐらいはわかった上で発言してくれな

**デフォルトの名無しさん** · 2021/01/31(日) 19:35:18.81

一応俺の想定では>>22が正解
ただ俺はデメテルの法則違反とか知らんから色々勉強になった
一概にこれが正解とは言えないんだなーと

**デフォルトの名無しさん** · 2021/01/31(日) 19:36:30.29

>>70
補足すると関数名ね
getPrice -> getClothPrice
にした方がいいんじゃないかなってこと

**デフォルトの名無しさん** · 2021/01/31(日) 19:37:21.40

>>71
それはgetPriceが属するオブジェクトによる
一概にそう言えないし、それは出題に書いてない

**デフォルトの名無しさん** · 2021/01/31(日) 19:37:54.79

>>71
ありがとう。
まぁ、俺も一部、知らんことあったから勉強にはなったよ。

**デフォルトの名無しさん** · 2021/01/31(日) 19:38:59.71

>>72
まぁ思い付きの出題だから、その辺はすまんかった

**デフォルトの名無しさん** · 2021/01/31(日) 19:39:20.06

俺は何の勉強にもなってねーなｗ

**デフォルトの名無しさん** · 2021/01/31(日) 20:18:52.57

class Box {
private int h;
private int w;
public int getHeight(){
return this.h;
}
public int getWidth(){
return this.w;
}

public int setHeight(int value) {
this.h = value;
}
public int setWidth(int value){
this.w = value;
}

}

俺も出題しよっと
俺は上記、箱クラスを作った
カプセル化もしてるし、インスタンス化もできるし、まさにオブジェクト指向（キリッ）

さて、何が駄目でしょうか

前提...命名規則やインデント等の書き方の問題ではない
考えるべきこと...クラス設計的な問題点を見つけよ

**デフォルトの名無しさん** · 2021/01/31(日) 20:21:04.97

初期値書き忘れた
Javaの言語仕様で0が入るって事で

**デフォルトの名無しさん** · 2021/01/31(日) 20:33:10.20

>>76
クラスレベルでhやwといった一文字は使うべきではない
座標としての意味のxやyなら問題ないが

**デフォルトの名無しさん** · 2021/01/31(日) 20:34:17.92

一文字変数が許されるのは、広くても関数レベル
可能ならそれよりも小さいブロックレベルまで

**デフォルトの名無しさん** · 2021/01/31(日) 20:36:14.99

理由は「パッっと見」でクラス全体を見ることは普通は無理だから。
一文字変数は変数名から意図がわからないのでパッっと見でコードも同時に見れることが前提

**デフォルトの名無しさん** · 2021/01/31(日) 20:36:31.43

うーん、この

**デフォルトの名無しさん** · 2021/01/31(日) 20:38:19.85

問題文に「命名規則は関係ない」って書いてあるのになぁ

**デフォルトの名無しさん** · 2021/01/31(日) 20:39:22.25

一文字変数は命名規則ではない

**デフォルトの名無しさん** · 2021/01/31(日) 20:41:27.15

脳がバグってるのか？
どうみても...ビジ...面白いから黙っておこう

**デフォルトの名無しさん** · 2021/01/31(日) 22:12:44.94

>>76
hとwを高々保持するのが目的のようなやつをクラスにする必要あるやろかとは思う
あとプライベートにしてアクセッサを用意するのはいかにもなんだけど
これは言うほど嬉しくも無いと結論づけてもういいんじゃないかな
こーいうデータ保持クラスおまえらも山ほど書いてきたと思うけど
嬉しかったことあるかこれ？
結局これを受け入れる側のクラスライブラリとべったりの未来しか見えないし

**76（移動中）** · 2021/01/31(日) 22:30:49.34

>>85
素晴らしい！正解！！
理由説明も、その通り！

まぁ、解決するとしたら、そもそもこんなクラスを作るなorドメインロジックをしっかり記述しましょうってところか。

オブジェクト指向（キリッ）とか言いつつ、完全にオブジェクト指向の本質を無視したプログラムでした。

**デフォルトの名無しさん** · 2021/01/31(日) 23:07:03.11

でも教科書にはこういう例しか載っていないんだよな・・・

**デフォルトの名無しさん** · 2021/01/31(日) 23:28:33.14

たしかに

Box box = new Box();

box.setHeight(10);
box.getHeight();
box.setWidth(7);

こんなことやってても虚しいなw
せめて

Box box1 = new Box(10, 7);
Box box2 = new Box(10, 5);

if( box1.area() < box2.area() )｛｝

くらいのことはできてほしいな
あと、コンストラクタで変な値（マイナスとか）を入れたら例外を出すとか

教科書はまだ未開拓なところがあるからしゃーない
最近の学生がどんな教科書を読んでるかは知らないけど

**デフォルトの名無しさん** · 2021/01/31(日) 23:28:37.40

実装ありきでそれに対するアクセッサという順にしか考えないし
実装ではなくてインタフェースに対してプログラミングするとかも分かってない
インタフェースを設計する上で見るべきところに思考がフォーカスされてない
C#はプロパティがあって簡潔で嬉しい！これでアクセッサ書き放題ぐへへってなもん

89 · 2021/01/31(日) 23:29:48.84

あ、流れ読んでなかった
>>89はどれに対するレスじゃなくて雑感ね

**デフォルトの名無しさん** · 2021/01/31(日) 23:32:30.66

ふう。やれやれ

アクセッサにするのは将来の拡張性のためだろ
Javaであればプロパティが存在しないから
そうしないとインターフェース互換性を保ったまま
継承することができない。

言われてコードを書いてるだけで、その理由を知らないんだな

**デフォルトの名無しさん** · 2021/01/31(日) 23:37:56.79

自分か継承をしたことがないからって
クソコードというのはコードの話をしてない
自分の経験の話になっている

**デフォルトの名無しさん** · 2021/01/31(日) 23:39:23.18

大丈夫？ID真っ赤だぞ

**デフォルトの名無しさん** · 2021/01/31(日) 23:45:54.87

IDが赤くなるのはレスが多いからなだけだぞ？
それ以外に意味はない

**デフォルトの名無しさん** · 2021/02/02(火) 07:34:00.26

クラス利用者の事を考えずに実装したクラスは全部クソコード説

**デフォルトの名無しさん** · 2021/02/02(火) 14:23:08.30

お前の書いたのは全てクソコード説

**デフォルトの名無しさん** · 2021/02/02(火) 14:38:58.03

てめえのもな

**デフォルトの名無しさん** · 2021/02/02(火) 15:05:56.95

>>96
このスレで語られている内容を理解できない96はクソコードしか書けないニート説

**デフォルトの名無しさん** · 2021/02/02(火) 15:18:13.00

>>11
コードの良し悪しは用途と仕様が不明確ならまともな判断はできないから
どちらもわからない状態じゃコーディングプラクティスの指摘だけになる

- clothPriceはどっからやってくるの？それでいいの？
- idを指定して複数の値が返されるようなデータ構造を使うべきなの？
- filter(id).first()よりfind(id)のほうがいいのでは？
- clothPriceが1000万件になっても問題ない？
- 俺の知ってるJavaScriptでは({ id })とは書けないけど？
- .first()はどこにどういう仕様で定義されてるの？
- idに対応する値がclothPriceに存在しなかった場合にどうなるの？
- テストコードも一緒に出してね

**デフォルトの名無しさん** · 2021/02/02(火) 16:23:17.93

>>99
書いてないことをいちいち質問したらきりがない
タブの数は2でいいの？とかまででてくるだろ

それらの質問は「問題ない」として扱えば
クソコードではないという答えが正しい

**デフォルトの名無しさん** · 2021/02/02(火) 18:40:58.39

お前ら（>>26 >>99）、国語の問題で「告白された○○君はなぜ顔を赤らめたのですか」と問われたら、「そんなのどこにも書いて無い！！もしかしたら唐辛子ガムを噛んでいたら予想以上に辛くて顔を真っ赤にしただけかもしれないだろ！！！」とか真顔で言い出しそうだな。

なんなんだこのスレは