slangsoft
@slangsoft

Are you sure you want to delete the question?

Leaving a resolved question undeleted may help others!

皆さんが考える「糞コード」とは?ご意見募集中です!

改訂履歴

  • 2020/08/04
    質問内で使用していた「定義」という言葉を削除しました。
    個人的に知りたいことは「こんなコードは嫌われる可能性がある」ということなので、「糞コードを定義づけする」という意味に誤解されそうな文脈を修正しました。

質問の意図

@kotauchisunsun 氏が投稿された「糞コードは直すな。」という記事がトレンド入りしていて、私もコメントさせていただきました。その中に @_akiyama_ 氏の「皆の頭に描かれてる糞コードの具体例が一致してなさそう」とうコメントがあり、はっとさせられました。

確かに、関わっているプロジェクトや使用しているプログラミング言語によっても「糞コード」と見なされてしまうコードは大きく異なるのでは?と思い、言語ごとに「どんなコードが糞コードと呼ばれて嫌われるのか」をまとめてみることにしました。

これから新人としてエンジニアの世界に飛び込んでいくであろう次世代エンジニアたちにとっても、決して無駄な情報にはならないと思ったので、Qitta の新機能 質問フィード を使って皆さんのご意見を募ってみようと思った次第です。

皆さんの経験から得られた知見をお寄せください!上手くブラッシュアップして纏められるように頑張りますので、どうぞ宜しくお願い致します!

この質問を読んでくれた方にお願いしたいこと

あなたが考える「糞コード」とは?について以下の3点をお答えください。そもそもコンパイルすら通らないとか、動作しないというものは除外でお願いします。

[質問.3] は秘密保持契約でコードを曝せない方も多いと思いますので、任意回答としています。

  • 質問.1 :(必須)あなたの考える糞コードとは

    • example) 著しくメンテナンス性が低く、読みにくい、他モジュールと結合度合いが高い
    • example) 今後の仕様変更に柔軟に対応できない実装方法でコーディングされている(仕様変更のたびに全見直しが必要になるなど)
    • example) 製品の最終系をイメージできず、突き付けられた仕様書だけが正と信じて実装されたコード
  • 質問.2 :(必須)上記糞コードが実装されていたプログラミング言語

    • example) C言語
  • 質問.3 :(任意)具体的な糞コードの実装例

    • example) 秘密保持契約の関係で具体例は投稿できません。

ここからは纏めエリア

言語が多くなってきたら、言語別に分けることも考えます。

No. 言語 糞コードとみなされる可能性が高いもの 情報提供者
(敬称略)
01
  • C言語
  • 可読性が低い
  • モジュール結合が密
  • 仕様変更に柔軟に対応できない
@slangsoft
02
  • Ruby
  • JavaScript
    (Node.js)
  • 可読性が低い
  • 実装が一般化できておらず、実行環境に密に依存している
  • 呼び出し先のコードが呼び出し元の条件を意識した実装になっている
  • メソッドやクラスの名前が役割と異なるものになっている
@getty104
03
  • JavaScript
  • 中級者レベルの人が、なんか思いつきで「いい手法!」と判断して実装しているようなのですが、それがシンプルとはかけ離れた複雑さをもっていて何を解決したいのかさっぱりわからない
  • 他言語の delegate などの概念を JS に持ち込んで、単純に関数を渡せばすむところに複数関数を登録できるようなイベントハンドラの実装をしている
  • 処理があっちこっちに飛んでいて、デバッグ実行で内部値を確認しながらでないと処理を追えない
@standard-software
04
  • Java
  • オブジェクト指向言語で static ばっかり使っている
  • 変数名などが略されすぎて、何を意味するのか分からないコード
@javaboy
05
  • Go
  • Rust
  • 変数への再代入
  • null や undefined のチェックをしていない
  • 非スレッドセーフな操作、global/static 変数の濫用
  • 使用されていない変数の宣言・初期化
  • 使用されていないライブラリの宣言(require、import etc.)
  • 生ポインタの濫用
@Naughie
06
  • HTML
  • CSS
  • JavaScript
  • ホームページビルダーと手で書き足したコードの合わせ技
@dojyorin
07 ご意見募集中!

参考情報

3

質問.1

  • 可読性が低い
  • 実装が一般化できておらず、実行環境に密に依存している(ハードコーディングや、特定の実行環境でしか使えない関数やモジュールを直接参照しているなど)
  • 呼び出し先のコードが呼び出し元の条件を意識した実装になっている(引数がこういう条件の時は例外的に特定の値を返す関数、など)
  • メソッドやクラスの名前が役割を異なるものになっている(クラスがめちゃくちゃいろんなメソッドを持っていたり、get_hogehogeなのに副作用がある、など)

質問.2

個人的に質問.1で挙げた定義はどの言語にも当てはまりそうだと思っていますが、一旦僕がずっと触っている言語を書いておきます!

  • Ruby
  • JavaScript(Node.js)

質問.3

一旦スキップします!

1Like

直接の返答ではありませんが。

参考図書: Cプログラミング診断室
27年も前の書籍ですが、内容が全文公開されています。

最近、1000行以上の関数ゴロゴロ、1万行以上のファイルゴロゴロ、循環的複雑度100以上の関数ゴロゴロ、コピペ上等、自動テストゼロなC++のプロジェクトにかなりの変更をする必要があったのですが、それに比べれば『Cプログラミング診断室』の糞コードはかわいいとすら感じます。

1Like

@Zuishin さん
情報提供ありがとうございます!
このサイト、数年前に私も見た記憶があります。エンジニアあるある的なものから笑えるものまで、いろいろ揃っていて面白いですよね:smile:
参考情報として追加させて頂きました!

@getty104 さん
ご回答ありがとうございます!
挙げて頂いた様なコードは私も何度となく目にしたことがあります。中でも呼び出し先のコードが呼び出し元の条件を意識した実装になっているが非常に厄介だと感じますね:joy:

@error_401 さん
有益な情報をありがとうございます!
@error_401 さんが対応されたプロジェクトに比べれば、私が担当しているプロジェクトの糞コードなんて可愛いものです。世の中には想像以上に大変な思いをしているメンテナーがいることを知って、改めて自分も頑張らなければ!と思うことができました:smiley:
Cプログラミング診断室のリンクは参考情報として追加させて頂きました!

0Like

質問1
定義は、ひどいコード。ですね。
細かい定義を決める必要もないです。見てられないほどひどいコード。

質問2
JavaScript
最近ずっとこれ一本。

質問3
コードとしては上げにくいので日本語で書いておきます。

単純な文法ミスとか、そういう書き方じゃないとか、
初心者が知らないだけなんだろうな、っていうコードならあんまり悩まされません。
動作を変えずにコードを変えるという厳密な意味でのリファクタリングが容易なので
文句言うほどでもなく即時で書き換えてしまいます。

ウンコード・マニア、で紹介されているコードはわかりやすい例だからほとんどこれ。
なので、これはほぼ問題にならない。

元記事にあるこれ。

大体、1日、2日で直すことができないから糞コードなのです。
まさに、こういうものに苦しめさせられます。

ド初心者レベルの人の書いたものではなく
中級者レベルの人が、なんか思いつきで「いい手法!」と判断して実装しているようなのですが
それがシンプルとはかけ離れた複雑さをもっていて
何を解決したいのかさっぱりわからず。
しかし、フレームワーク的に組み込まれているので、回避できずに
その仕組みのっとってプログラム組まなきゃいけない、という状況で開発していると
結構苦い思いをします。

実例ですが
他言語のdelegateなどの概念をJSに持ち込んでいて
動的である必要は全くないのに、将来の拡張?を予期してなのか
単なる関数を渡せばすむところに、複数関数を登録できるようにイベントハンドラの実装をしていたりとか、

Reactのchild指定で、自動的にプロパティを下位オブジェクトに渡してたりとか

こういうことされているとプログラムを読んでも処理があっちこっち飛び、
全然見通せなくて、動かしながら内部値を確認しながらでないとコードが追えないんですわ。

他には、軽微な痛いコードですが
Reactのstateの内部のオブジェクトの値に対して代入してて
実質、setStateを使わずにオブジェクト直接書き換えていたりとか。
(オブジェクトが参照だと知らんのかと...)

DRY原則を履き違えていて、共通化した関数内部で処理分岐したりとか。
そういうのですかね。伝わるかな...

シンプルなやり方はもっとあるのに、
知らないのか、頭が混乱しているのか、脳に虫がわいてるのかで、
コードを複雑にしてしまっている人の過去作品をみていると、ぶち壊したくなります。

自分より下手くそだなあと思われる人の
思いつきのアイデアに踊らされていて、
なるべく改善したいのですが、
バカの意図を完全理解しなければ修正できない、のがつらい。
しかもそれに何日もかかったりして、異様に足を引っ張られる感がつらい。
最初からまともなコードにしておいてくれたら開発はらくらくなのに

「下手の考え休むに似たり」って諺通りというか、
それ以上に足をひっぱるなと、という感じです。

ものごとを、もっとシンプルにして解決して欲しいと思いますが
なかなかそういうレベルに到達するのは簡単じゃないのでしょうね。

ド初心者だった「こうじゃないよ。こうしてね。こっちが簡単だから」と教えたら
すぐに理解して修正してくれるのですが
意図のよくわからない複雑なものを採用する人って、
言ってもゆずらないことが多くて苦労します。

let は悪で const のみ原理主義とかね...
sum処理のためにlet使いたくないからreduce使います。そして読みにくいだけ。とか。
あれはネタなんでしょうけれども。

2Like

糞コードの定義

  • オブジェクト指向言語でstaticばっかりつかっているコード
  • 変数名などが略されすぎて、何を意味するのか分からないコード

上記糞コードが実装されていたプログラミング言語

  • Java

具体的な糞コードの実装例

import javafx.application.Application;
import javafx.stage.window.Stage;

public class Main extends Application {
    public static Stage stage;
    @Override 
    public void start(Stage stage) {
        Main.stgae = stage;
    }
}
1Like

@standard-software さん
ご回答ありがとうございます!
上手く汲み取れているか自信がないのですが、質問.3の回答として記載いただいた内容を、かいつまんで本文に追加してみました。「いやいや、そーゆーことなじゃいんだよ」みたいな部分があれば、ご指摘いただけると助かります!
ちなみに「意図のよくわからない複雑なものを採用する人って言ってもゆずらないことが多くて・・・」←これ、すごく解ります。
いちいちイラつくのも嫌なので「あぁ、この人はこの方法しか知らないんだなw」と考えて精神の均衡を保つように努めてます:grin:

@javaboy さん
ご回答ありがとうございます!
やはり「staticおじさん」は出ちゃいますよね:smile:「変数名などが略されすぎて~」もすっごく解ります!せめて英語圏で一般的に使用されている略称なら良いのですが、勝手に何かの頭文字だけを取って並べただけの変数名とか意味不明ですよね!

2Like

糞コードというのは文脈に依存するのだと思います。

たとえば、どんなに糞コードを嫌悪している人でも、「10分でコーディングして、数回実行して、はい終わり」な場面では後先考えない ad hoc なコードを書いてしまうでしょう。しかしそれを「糞コード」だと拒絶ことはありません。どうせ今後手を付けることなんて無いからです。綺麗なコードを書くよりもコーディングの速さの方が重要です。

一方、OSS で他人と共有したり、仕事でプログラムを書いたり、という場面になれば、それらは途端に糞コードと化します。なぜなら、そのような場面では長期的なメンテナンス性が求められるからです。

要は、将来的(それは数分後のデバッグ後かもしれない)に必要な修正を加えるのが難しい、という基準でしょう。あるいは、何が必要な修正か判断できない、という基準もあると思います。

私の実体験としては、二年前にこのような Bash スクリプトを書きました。もし仕事場でこのようなプログラムを見せられたら、「ふざけるな」と言いたくなります。リダイレクトの使用法が奇妙だからです。しかしこれは趣味で書いたものであり、このまま二年間使い続けています。

他には、この Ruby ワンライナーは例外を適切に処理していないため、本来ならばバグの温床です。でも使用者(つまり私)は、何がバグを引き起こしうるか理解しているので問題ありません。それよりも「ワンライナーで書ける」ことの方が優先的です。

話は変わりますが、Go や Rust といったモダンな言語で(コンパイラによって)非明示的な利用が禁止されているコーディングは、糞コードの十分条件たりえますね。(あくまで非明示的な implicit、あるいは非自覚的な unconscious 利用です。)たとえば

  • 変数への再代入等
  • 非スレッドセーフな操作、global/static 変数の濫用
  • 使用されていない変数の宣言・初期化
  • 使用されていないライブラリの宣言(requireimport etc.)
  • 生ポインタの濫用
  • nullundefined のチェックをしない
  • etc.

このリストを眺めると、C や Javascript はやはり糞コードが多い傾向にあるのかな、と思いますね。趣味で書くようなプログラムではほぼ全部やってしまいます。

4Like

@Naughie さん
ご回答ありがとうございます!

どんなに糞コードを嫌悪している人でも、「10分でコーディングして、数回実行して、はい終わり」な場面では後先考えない ad hoc なコードを書いてしまうでしょう。しかしそれを「糞コード」だと拒絶ことはありません。

これは仰る通りだと思います。
私も「一度きりの作業だけど時間と手間がかかる」みたいな仕事があるときは、その作業を自動化する使い捨てコマンドラインツールを作ったりします。この類のコードは他人がメンテするものではないので、エラーハンドリングだの可読性だのは一切気にしないです。

そう考えると、嫌われるコードが生成される背景には、そのコードのライフサイクルも大きく影響していると言えるのかもしれませんね:grinning:

0Like

Webフロントエンド周りで私が遭遇した話ですが...

私はいわゆる"社内ソロエンジニア"で、私が来る前まではホームページビルダーを使ってたそうです。

あとは社内システムが全体的にIEというかActiveXに依存しており、もうどうにも手に負えない状態でした。

そして、そのホームページビルダーはとても古いバージョンでサポート切れはおろかホームページビルダー本体そのものすらいつの間にか無くなってて、それ以降はニュースフィードなどWordや手でコピペ追記してたそうです。

HTMLやJSのインデントはごちゃごちゃで、スタイルはタグの属性にインラインで書いてあるわ、ビルダーやWordのよく分からない数十行にも及ぶメタ情報やコメントが点在してたり、JSもActiveXにべったりだったりES3相当で今じゃ非推奨の機能も満載でした。

全体で数千行ありましたが、実際に表示される部分のコードは数百行だったと思います。

質問1
ホームページビルダーと手で書き足したコードの合わせ技

質問2
HTML/JS/CSS

質問3

document.write("<span>...")
new ActiveXObject("...")

これで"セキュリティ()"と高らかに宣言してるのは滑稽ですね。
下手すればShellまで扱えますし。

<table><tr><tr><tr>...
<div align...>

など...

流石に、この保守をドンと全部1人で任されたときはVue.jsとES11(draft)/Babelでコンテンツの内容はそのまま0から全部書き直しました。

一応、数年後これが"クソコード"にならないため、簡単に仕様はまとめましたが。

この意見交換の元となった記事では「安易に直すな」と仰られていましたが、下手したら20年くらいも前の遺産は流石にどうにかしないとならないんじゃないかと...
当時の書き方を巻き戻って会得するコストもありますし...

2Like

@dojyorin さん
ご回答ありがとうございます!

HTMLやJSのインデントはごちゃごちゃで、スタイルはタグの属性にインラインで書いてあるわ、ビルダーやWordのよく分からない数十行にも及ぶメタ情報やコメントが点在してたり、

これ、私も経験あります!
ホームページビルダーで作られたホームページのメンテには苦労させられました:sweat_smile:

0Like

Your answer might help someone💌