- 1 名前：仕様書無しさん [2007/08/14(火) 23:48:45 ]
- この会社辞めようと思ったソースコード。
プログラマとして幻滅するソースコード。
プログラマを悩ませるソースコード。
をつらつらと綴っていって頂戴。
ちなみにここは質問スレじゃないので
技術的な質問がしたいならム板 pc11.2ch.net/tech/ に逝って。
前スレ
この会社辞めようと思ったソースコード＃17
pc11.2ch.net/test/read.cgi/prog/1183700531/
- 880 名前：仕様書無しさん mailto:sage [2007/10/08(月) 22:43:55 ]
- >>879
分割なのかと言われると確かに分割してるなｗ
- 881 名前：仕様書無しさん mailto:sage [2007/10/08(月) 22:49:44 ]
- >>879
そんな感じで分割(?)されてて、しかも連番の振り方が処理内容ではなく改修の順番だったというクソコードのメンテさせられたことがある。
- 882 名前：仕様書無しさん mailto:sage [2007/10/08(月) 23:27:16 ]
- >879
calc2がcalc以外からも呼び出されるなら、まあ許せる範囲にはいるかもしれん
- 883 名前：仕様書無しさん mailto:sage [2007/10/09(火) 00:26:20 ]
- ぱっと見てどれほどの量の処理なのかわかんないから、
どんどん続きの処理が出てきてウヴォアァってなるなー
メンテでそんなコード見たくないねぇ
- 884 名前：仕様書無しさん mailto:sage [2007/10/09(火) 00:32:25 ]
- >>877
条件式は!と<さえあれば表現できるので、
わざわざ==や!=や>のような余計なものを使って
初心者を混乱させるべきでない
とか？
- 885 名前：仕様書無しさん mailto:sage [2007/10/09(火) 00:34:16 ]
- 4種類の単品マスターをセット組する処理で
単品マスターがカナとコードで検索が可能で
検索結果の並び順をカナ、コード、更新日時順で指定可能
24個のコピペ関数がKensaku1とかSearch2とかって名前で存在し、
cmdKensaku1で呼んでるのがKensaku4だった
ぐわーっとコメントアウトして書き直してしまった。
- 886 名前：仕様書無しさん mailto:sage [2007/10/09(火) 00:37:26 ]
- >>885
俺、perlでそういうのに出会ったことがある。
正直あれは思い出したくない。
- 887 名前：仕様書無しさん mailto:sage [2007/10/09(火) 00:51:31 ]
- あー。あるある。
- 888 名前：仕様書無しさん mailto:sage [2007/10/09(火) 00:58:45 ]
- //if (cnt > 0 || cnt == 0) {
って結構見た目綺麗だよなぁ、と思ったりw
>=ってなんかキチャナイ気がするし
- 889 名前：仕様書無しさん mailto:sage [2007/10/09(火) 03:30:10 ]
- 左辺がもっと長かったら２回書くのは鬱陶しいよ
評価するために何らかの処理が走る場合は２回走っちゃうし（一時変数使えばいいけどやっぱ手数が増える）
- 890 名前：879 mailto:sage [2007/10/09(火) 05:50:29 ]
- >>880,882
一応きりのいいところできられてるっぽいが、
変数のスコープが狭くできているとかいうこともない。
calc2以下は前の関数から呼ばれることを前提に作られている。
関数分割のメリットはどこにあるのやらｗ
>>881
うっ・・・こっちのほうがまだましだったか
当時の事情はよくわかんねえんだが、たぶん、
関数の長さが長すぎるといけない、とかいうんで無理矢理ちぎった
というのがオチだろうと思います。
- 891 名前：仕様書無しさん mailto:sage [2007/10/09(火) 08:48:12 ]
- case型構文があるときはどうすんの？ ちょんぎれないよね。
- 892 名前：仕様書無しさん mailto:sage [2007/10/09(火) 11:22:48 ]
- ちょんぎるんじゃない、構造を見直すんだ
- 893 名前：仕様書無しさん mailto:sage [2007/10/09(火) 12:14:08 ]
- case文ひとつで何百行にもなるのなら、その時点でおかしい。
ちょんぎるとかいうレベルじゃない。
- 894 名前：仕様書無しさん mailto:sage [2007/10/09(火) 12:53:47 ]
- だって100とおりのcaseなら各中身が平均４行だって500行ぐらいすぐなるじゃない。
コマンドが何十種類とか。caseの数が多いこと＝設計ミスとは決めつけられないでしょ。
- 895 名前：仕様書無しさん mailto:sage [2007/10/09(火) 13:01:03 ]
- 決め付けはできないけど、大抵の言語でダメだよ
- 896 名前：仕様書無しさん mailto:sage [2007/10/09(火) 13:03:01 ]
- >>893
つWindowsアプリのソースコード
- 897 名前：仕様書無しさん mailto:sage [2007/10/09(火) 13:42:24 ]
- >>894
それはcase文ひとつで数行である、ってことでは？
- 898 名前：仕様書無しさん mailto:sage [2007/10/09(火) 14:28:56 ]
- >>896
WndProc か?
LRESULT CALLBACK WndProc(HWND hWnd, UINT msg, WPARAM wp, LPARAM lp) {
switch(msg) {
case WM_PAINT: return OnPaint(hWnd, wp, lp);
case WM_SIZE: return OnResize(hWnd, wp, lp);
default: return DefWndProc(hWnd, msg, wp, lp);
}
}
ってするな、俺は。
実際には WPARAM や LPARAM をそのまま渡したりはせずに
MSDNの記述に従って分割してから渡すけど。
仮に処理するウィンドウメッセージが多くて、
この case が 1000 個連なったとしても OK だろ、この場合は。
よくあるプログラミング講座みたいに case の中につらつら書くのはダメだ
- 899 名前：仕様書無しさん mailto:sage [2007/10/09(火) 18:35:36 ]
- >>898
それって非常にありがちだけど、個人的にはそういう書き方は自己満足にしかなってないと思うなあ。
一度反省的に自分のそういうコードを見直してみて欲しいよ。
一箇所からしか呼ばれないコードを関数に括りだすことが本当に可読性に資するのか。
むしろ、コードを読む際にそこに飛ぶ手間を増やしているだけじゃないのか。
「caseの中につらつら書いた」コードが本当に読みにくいのか。
むしろ教条主義的にそう思い込んでるだけじゃないのか。
- 900 名前：仕様書無しさん mailto:sage [2007/10/09(火) 19:00:35 ]
- 抽象ってどういう意味か考えてみて欲しいね
「読まければ意味がわからない関数」は括り出す意味は無い
可読性をあげるためには、「今まさに関心のある」レイヤ以外に存在する
関数について、「読む必要性自体」を消し去る必要があるのだが
- 901 名前：仕様書無しさん mailto:sage [2007/10/09(火) 19:10:34 ]
- 「コードを読む際にそこに飛ぶ」のにそんなに手間がかかるのかｗ
- 902 名前：仕様書無しさん mailto:sage [2007/10/09(火) 19:13:13 ]
- >>894
俺ならcaseマッチングのテーブルを挟んでループさせるな。
マッチングオブジェクトは当然処理関数への参照を持たせる。
マッチングしたら対象オブジェクトの処理を呼び出す。
C/C++なら普通に書ける関数テーブル。
処理部分はスマートになるし、定義部分ではコメント多用して
逆に分かり易く表現出来る。
VBはしらねーよw
- 903 名前：仕様書無しさん mailto:sage [2007/10/09(火) 19:17:03 ]
- OOPLにおいては、
どうやって分岐するか、と分岐した先で何をやるか
は、分離しとかないと基本的にはOCP違反になる
- 904 名前：仕様書無しさん mailto:sage [2007/10/09(火) 19:34:57 ]
- 関係の無いメッセージに対する処理を脳内でスキップする方が
関数1個参照に行くよりよほど時間かかるよな
何のサポートもしてくれないエディタ使ってる場合はどうか知らんが
長大な関数は変数のスコープなんかで
気にしないといけないことも爆発的に増えるし
- 905 名前：仕様書無しさん mailto:sage [2007/10/09(火) 20:53:01 ]
- >>902
状態遷移表の実装なんかで俺もソレ良くやるけど、
関数ポインタ(Cなんで)使うと、静的解析で処理が追い辛くなるのが難点かねぇ。
- 906 名前：仕様書無しさん mailto:sage [2007/10/09(火) 21:03:31 ]
- こういうときデリゲートとかラムダ式があれば…
boost禁止だしのぅ
- 907 名前：仕様書無しさん mailto:sage [2007/10/09(火) 21:11:55 ]
- >>904
分かってないね。
「関係の無いメッセージに対する処理をスキップ」する必要性は
各メッセージの処理を関数に括り出そうがなくならない。
ただエディタ上で「読み飛ばす」行数が減るだけの効果しかないんだよ。
それに、ウィンドウプロシージャのような定型的で分かりきった処理は
行数が増えてもコードの見通しは悪くならない。
実際試せば実感できると思うけど、
こういう場合はむしろ関数呼び出しによる抽象化の方がコードの不透明性を高めるから
読んでイライラする蓋然性が上昇するよ。
- 908 名前：仕様書無しさん mailto:sage [2007/10/09(火) 21:23:47 ]
- >>907
わかってないのはお前だ。だまされたままで良いから周囲にあわせとけ。
- 909 名前：仕様書無しさん mailto:sage [2007/10/09(火) 21:26:18 ]
- んー・・・凄い個人的な主観的な話になるけど、
caseとcaseの間がエディタの半ページ以内なら気にならない。
1つのcaseでエディタ1ページを超えると途端に読み辛くなる。
開発環境によってエディタの行数変わるんだけど
(WindowsだとVCかサクラ、Linuxだとemacsとか)、
俺は大体こんな感じ。
- 910 名前：仕様書無しさん mailto:sage [2007/10/09(火) 21:33:48 ]
- >>907
ガラクタを無秩序に押入れに押し込むことを抽象化と呼ぶのなら
お前のいうとおりだろうな
関数名に連番とか振ってないだろうなｗ？
- 911 名前：仕様書無しさん mailto:sage [2007/10/09(火) 21:48:45 ]
- >>910
＞ガラクタを無秩序に押入れに押し込むことを抽象化と呼ぶのなら
そんなトンデモな前提に立った議論ではない。
君とか904は、まあダメなプログラマにありがちだけど
暗黙のうちに「プログラムを書いている時点の視点」に立っている。
確かにプログラムを書いている時点に限定すれば、ほとんどの場合は
関数に括り出した方が読みやすいし、見た目にもスッキリする。
「caseの中につらつら書いた」コードなんて醜いし見難いように思われる。
ところが同じコードを一年後に読むと評価が逆転するんだなこれが。
- 912 名前：仕様書無しさん mailto:sage [2007/10/09(火) 21:53:11 ]
- なにそれ、全然理由になってない
- 913 名前：仕様書無しさん mailto:sage [2007/10/09(火) 21:54:05 ]
- >>910
>関数名に連番とか振ってないだろうなｗ？
関数名、変数名がすべて [A-Z]{2,3}[0-9]{5} なコードの保守ならやらされたことがある。
それはともかく、case 文の中身が10行超えたら、普通 state か command か chain of responsibility あたりを使わね？
- 914 名前：仕様書無しさん mailto:sage [2007/10/09(火) 21:54:51 ]
- case 1:
{
} break;
case 2
{
って最初に作っておいたらスコープ付くしbreak強制になるから便利
- 915 名前：仕様書無しさん mailto:sage [2007/10/09(火) 21:55:11 ]
- >>911
オブジェクト指向にOCPってあるんだけど、きいたことある？
いろいろ勉強して、実践して、その上でいってるのか心配になっちゃうんだけど
- 916 名前：仕様書無しさん mailto:sage [2007/10/09(火) 21:58:03 ]
- デリゲートを条件に使われる定数をインデックスにして連想配列にぶちこむのは一種のストラテジーパターンらしい
- 917 名前：仕様書無しさん mailto:sage [2007/10/09(火) 21:59:40 ]
- >>915
そいつのレスはムダに長いが、主張したいことは最後の1-2行だけ。
そこ見たらこれ以上そいつにかまう必要が無いことがわかる。
- 918 名前：仕様書無しさん mailto:sage [2007/10/09(火) 22:11:46 ]
- >>899
> >>898
> 一箇所からしか呼ばれないコードを関数に括りだすことが本当に可読性に資するのか。
Testability
- 919 名前：仕様書無しさん mailto:sage [2007/10/09(火) 22:22:07 ]
- >918
君の一言を待っていた
- 920 名前：仕様書無しさん mailto:sage [2007/10/09(火) 23:55:45 ]
- ( ﾟдﾟ )
- 921 名前：仕様書無しさん mailto:sage [2007/10/10(水) 00:47:25 ]
- この類の話は
センスがない人にはいくら説明しても理解できないんだよね。
たぶん>>899,907,911は文系か元コボラ。
- 922 名前：仕様書無しさん mailto:sage [2007/10/10(水) 01:30:46 ]
- >>921
今追いついたけど同意
- 923 名前：仕様書無しさん mailto:sage [2007/10/10(水) 01:38:13 ]
- 厳しいこといえば、センスとか言ってるやつもヤバイ
基本中の基本だ
- 924 名前：仕様書無しさん mailto:sage [2007/10/10(水) 01:57:12 ]
- こういうやつらが消火の悪いスパゲティ作るんだろうな
- 925 名前：仕様書無しさん mailto:sage [2007/10/10(水) 03:54:45 ]
- 燃え盛ったときに消しにくいんだな
- 926 名前：仕様書無しさん mailto:sage [2007/10/10(水) 04:32:11 ]
- 負け組PGの揚げ足取り乙
- 927 名前：仕様書無しさん mailto:sage [2007/10/10(水) 08:17:56 ]
- >>899
逆でしょw
武道みたいにプログラミングでもスタイルや「型」は大事だし、
それらはほとんどの場合正しいが、いつでも正しいわけじゃない。
意味を持った処理は関数に括り出して抽象化することによって読みやすくなる、
という「型」はほとんどの場合正しいが、いつでも正しいわけじゃないんだが、
自分の頭で考えられない教条主義者（＝「センス」を欠いた人間）にはそれが
判断できないし、そういう発想が頭をよぎっても抑圧してしまうんだね。
王様は裸だ、と言えないのと同じ心理的メカニズムが作用するからねw
- 928 名前：仕様書無しさん mailto:sage [2007/10/10(水) 08:38:05 ]
- 何が「逆」なのか、説明してくれ
- 929 名前：仕様書無しさん mailto:sage [2007/10/10(水) 10:11:44 ]
- おまえら、当然ファウラーの「リファクタリング」を読んだ上で発言しているだろうな。
- 930 名前：仕様書無しさん mailto:sage [2007/10/10(水) 11:43:05 ]
- >>929
教科書主義者乙
- 931 名前：仕様書無しさん mailto:sage [2007/10/10(水) 12:09:26 ]
- > 教科書主義
- 932 名前：仕様書無しさん mailto:sage [2007/10/10(水) 12:15:19 ]
- いくら教条主義はNGだからつって、そもそも教科書読んでねえやつは論外なわけだ
- 933 名前：仕様書無しさん mailto:sage [2007/10/10(水) 15:16:54 ]
- 同意
- 934 名前：仕様書無しさん mailto:sage [2007/10/10(水) 21:36:24 ]
- >>927
あいかわらずムダな長文だな。自分にレスして楽しいか？
- 935 名前：仕様書無しさん mailto:sage [2007/10/10(水) 23:40:23 ]
- 俺には無意味な事を二度書いて、
最後に無関係なたとえ話をしている様に見える。
誰か >>927 をリファクタリングしてくれ。
- 936 名前：仕様書無しさん mailto:sage [2007/10/10(水) 23:49:49 ]
- case毎の処理を関数に切り出すと読み辛くなるってのは、
caseの切り分け方がおかしい証拠に他ならんと思うのだが。
あと
>927
>意味を持った処理は関数に括り出して抽象化することによって読みやすくなる、
>という「型」はほとんどの場合正しいが、いつでも正しいわけじゃないんだが
の「正しくないケース」ってどんな時さ。
抽象化によって読み辛くなるのは、抽象化の仕方がおかしい時だけでしょ。
- 937 名前：仕様書無しさん mailto:sage [2007/10/10(水) 23:50:25 ]
- >>935
#修正しておきました。
- 938 名前：仕様書無しさん mailto:sage [2007/10/11(木) 00:07:05 ]
- >>937
全消しかいｗ
- 939 名前：仕様書無しさん mailto:sage [2007/10/11(木) 00:39:54 ]
- >>936
>>898-を参照
- 940 名前：仕様書無しさん mailto:sage [2007/10/11(木) 00:45:20 ]
- WTLのメッセージマップとか激しく便利だお
- 941 名前：仕様書無しさん mailto:sage [2007/10/11(木) 19:34:57 ]
- >>936
正しくないケースを考えてみた。
1. >>927みたいな奴しかいない現場
2. 作成した関数を全て紙で管理している現場
3. 将来メンテする奴への嫌がらせ
4. リソースがカツカツ
5. そもそもサブロジックを切り出せない言語だ
6. コンパイラの限界に挑戦中
こんなもんか？
- 942 名前：936 mailto:sage [2007/10/12(金) 02:21:01 ]
- >941
納得してしまった。w
- 943 名前：仕様書無しさん mailto:sage [2007/10/12(金) 09:24:34 ]
- >>941
>5. そもそもサブロジックを切り出せない言語だ
そうかN88-BASICだったんだ
- 944 名前：仕様書無しさん mailto:sage [2007/10/12(金) 11:56:55 ]
- N88-BASICじゃ仕方ないなｗ
N88-BASICでも、変数名と戻り値代わりに使う変数名を工夫すれば
GOSUB との組み合わせで何とかなるかもしれんが
本質じゃない不毛なコードが大量生産されそうだ
- 945 名前：仕様書無しさん mailto:sage [2007/10/12(金) 13:33:21 ]
- いろんな火災現場を転々としてるダメPGが言うのもなんだけど
どちらの書き方でも、どんなに拘りの秘伝のソースでもいいが
可読性の無いソースを書くのはやめて欲しい。。
>>944
BASICの頃でも、ON (式) GOSUB *Label...でサブルーチン化はしてたよ。
- 946 名前：仕様書無しさん mailto:sage [2007/10/12(金) 13:59:28 ]
- >945
>可読性の無いソースを書くのはやめて欲しい。。
無理。
自分の常識は他人の非常識である場合が多い。
# だからflgで３値を返すなとあれほど･･･
- 947 名前：仕様書無しさん mailto:sage [2007/10/13(土) 15:07:00 ]
- いやそもそも可読性が無ければ保守不能だし。「可読性が低い」だろ。
じゃあその可読性の水準を向上させるためにはどうすれば良いか。
という話になるけど。これは99%以上が独学だろ。
スレの上のほうに出てきた本を読んでおくのは当然。
OSSという容易に読めるコードもあるじゃないか。
|
 |
