クソコードとは何か

**デフォルトの名無しさん** · 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と同じノリで答えてください。