07/10/08 17:45:19
俺が今日作った関数500行ぐらいある。
疲れた。そして明日からももっと疲れるだろう。
851:仕様書無しさん
07/10/08 17:45:51
もはや釣りかネタとしか思えん
852:仕様書無しさん
07/10/08 18:10:51
>>801
本気でわからんので回答ください。
800 ：仕様書無しさん ：2007/10/08(月) 09:40:32
>>796
見りゃわかるから、そんなコメントいらね！ってこと？
もしくは、 0 < cnt のほうが見やすいだろ！ってことか？
801 ：仕様書無しさん ：2007/10/08(月) 11:29:35
>>800
そんなこともわからんのも
まずいと思う。
803 ：仕様書無しさん ：2007/10/08(月) 12:16:56
>>801
や、すまん。俺も悩んだ。
853:仕様書無しさん
07/10/08 18:17:47
見りゃわかるからだろ
どうでもいいとこにコメント書きまくってる奴に限って
本当に必要な所にはコメントなかったりするんだよな
854:仕様書無しさん
07/10/08 18:22:39
たいてい関数は短くするけど
ロジック的なものじゃなくて力任せにいろいろ構成を書いていく部分のコードだと長くなる 300とか
855:仕様書無しさん
07/10/08 18:34:56
ライブラリとか低位レイヤの関数は短いよね？
業務レイヤになってくると分割が意味を成さないような処理もあるから
ちょっとだけ長くなるよね？ ね？(一連動作を明示する意味もある！？)
短いとか長いとか両極端に走っちゃだめだよね？ ね？
856:仕様書無しさん
07/10/08 18:50:47
>>855
本来、業務ロジックなんか一番短くなってしかるべき
857:仕様書無しさん
07/10/08 19:14:48
長い関数の中の一行を修正しなくちゃならんという時、
その一行で使われている変数が前後でどう使われているか
普通チェックするもんじゃない？
関数が短ければ追っかけなきゃならんロジックが減って
あとで楽できると思うんだが。
俺だったらhoge()っていう300行の関数作らず
100行ぐらいのhoge_a(),hoge_b(),hoge_c()を作って
それを順番に呼び出すだけのhoge()を作るなぁ。
858:仕様書無しさん
07/10/08 19:43:39
>>855
ログ吐く
処理1を呼び出す
結果を見てエラーがあればコードに応じて処理を書く
ログ吐く
ログ吐く
処理2を呼び出す
・
・
・
みたいな感じで長くなることはあるな
859:仕様書無しさん
07/10/08 20:16:01
>>853
おｋ。納得した。
860:wolf ◆8VH3XAqjlU
07/10/08 20:34:59
>>852
// cnt が 0 より大きかったら処理する
if (cnt > 0) {
コメントがここに書いてある６番目の心掛けなければイケナイ事
URLﾘﾝｸ(www.ambysoft.com)
6. Document why something is being done, not just what
何をしているかをコメントにするのではなく、何故そうするのかを書きなさい
に抵触はしていますね
861:仕様書無しさん
07/10/08 20:38:32
「他人に分からせる気はこれっぽっちもない」けど「懇切丁寧ぶったコードには見える」んだよ ククク
862:仕様書無しさん
07/10/08 20:45:29
// cnt が 0 より大きかったら処理する
if (cnt > 0) {
ここの条件分岐が、実際の業務と直接リンクする場合は上のようなコメントを書くと思う。
そういうのはあり？たとえば「じいちゃんがココで死にそうだったら110番する」みたいな。
863:仕様書無しさん
07/10/08 20:48:57
でも0 ゼロだぞ
「○○がまだ残っている場合」とか
そういう書き方にならね
864:仕様書無しさん
07/10/08 20:52:58
>>862
それはありだと思う。
「（金額） cnt が 0 より大きかったら処理する」のように、cnt がもつ情報が類推できるような場合だよね。
ただし、それは他のコードやコメントと読み合わせてはじめて理解できるものじゃない？
コード例だけだと、類推のための手がかりがないから、論争になってるわけで。
865:仕様書無しさん
07/10/08 20:55:34
変数名がcntだから、count=「個数」であることは確か(なはず)なので、
金額は例としてはまずいな。
しかし、個数である場合に、
「あれば～」とか「残っていれば～」
などでなく本当に
「0より大なら～」
というコメントにすべき例を思いつかない。
866:仕様書無しさん
07/10/08 20:57:45
カウンタ絡みのテキトー変数名でcntがついても不思議じゃない
867:仕様書無しさん
07/10/08 20:58:05
つまり、cntが何を表すか、処理ってどんな処理か書けっって言いたいんだろう。
>>862 でいう「じいちゃん」とか「110番」って情報が抜けてる。
868:仕様書無しさん
07/10/08 21:01:41
// まだ在庫があれば、賞味期限を確認する
if (cnt > 0) {
/*賞味期限確認*/
}
869:仕様書無しさん
07/10/08 21:20:24
// cntが0以上だったら処理する
// 19XX.X.X. レビューで修正
//if (cnt > 0 || cnt == 0) {
if (!(cnt < 0)) {
870:仕様書無しさん
07/10/08 21:21:05
おまいら、なんか丁寧な議論をしてて好感が持てるぞ。
要は内部的な単なる処理に自明なコメント付けるなってことだよね。
i=i+1; //iに1を加算する みたいな。
871:仕様書無しさん
07/10/08 21:22:54
howじゃなくてwhatを残すんだろ
コメントの基本
872:仕様書無しさん
07/10/08 21:28:30
>>868
コメントを書きたくなったら変数名や関数名を考え直すんだ。
こんなかんじで。
if (zaiko > 0) {
shoumikigen_kakunin();
}
873:仕様書無しさん
07/10/08 21:30:33
>>869
わろす
874:仕様書無しさん
07/10/08 21:39:53
>>869
レビュワーのレベルも知れるな。
875:仕様書無しさん
07/10/08 21:41:45
修正前の方がまだマシのような気が・・・
876:仕様書無しさん
07/10/08 21:43:17
指摘した人間の想定の遙か上をいくアホであったのかも
877:仕様書無しさん
07/10/08 21:44:09
>>869
レビュワーの主義が知りたい。
878:仕様書無しさん
07/10/08 21:45:30
>>869
自分は今までIFは肯定文と否定文両方必要だと思ってたけど
今いるところで、無駄な文は作るなと直された。
SQLだと否定文を書くのはNGだけど
今の規約ではJavaの方は否定文なら否定文だけ書かされる（当たり前だが）
if (!methodA()) {
// 処理
}
879:仕様書無しさん
07/10/08 22:11:53
ちょい上のほうで関数分割ネタがあるんだが、
今漏れが見てるソースにおもろいのがあった。
VB6ですまんが
Sub calc()
処理１
・
・
処理１０
Call calc2
End Sub
Sub calc2()
処理１１
・
・
処理２０
Call calc3
End Sub
Sub calc3()
略
これは関数分割といっていいのか？ｗ
880:仕様書無しさん
07/10/08 22:43:55
>>879
分割なのかと言われると確かに分割してるなｗ
881:仕様書無しさん
07/10/08 22:49:44
>>879
そんな感じで分割(?)されてて、しかも連番の振り方が処理内容ではなく改修の順番だったというクソコードのメンテさせられたことがある。
882:仕様書無しさん
07/10/08 23:27:16
>879
calc2がcalc以外からも呼び出されるなら、まあ許せる範囲にはいるかもしれん
883:仕様書無しさん
07/10/09 00:26:20
ぱっと見てどれほどの量の処理なのかわかんないから、
どんどん続きの処理が出てきてウヴォアァってなるなー
メンテでそんなコード見たくないねぇ
884:仕様書無しさん
07/10/09 00:32:25
>>877
条件式は!と<さえあれば表現できるので、
わざわざ==や!=や>のような余計なものを使って
初心者を混乱させるべきでない
とか？
885:仕様書無しさん
07/10/09 00:34:16
4種類の単品マスターをセット組する処理で
単品マスターがカナとコードで検索が可能で
検索結果の並び順をカナ、コード、更新日時順で指定可能
24個のコピペ関数がKensaku1とかSearch2とかって名前で存在し、
cmdKensaku1で呼んでるのがKensaku4だった
ぐわーっとコメントアウトして書き直してしまった。
886:仕様書無しさん
07/10/09 00:37:26
>>885
俺、perlでそういうのに出会ったことがある。
正直あれは思い出したくない。
887:仕様書無しさん
07/10/09 00:51:31
あー。あるある。
888:仕様書無しさん
07/10/09 00:58:45
//if (cnt > 0 || cnt == 0) {
って結構見た目綺麗だよなぁ、と思ったりw
>=ってなんかキチャナイ気がするし
889:仕様書無しさん
07/10/09 03:30:10
左辺がもっと長かったら２回書くのは鬱陶しいよ
評価するために何らかの処理が走る場合は２回走っちゃうし（一時変数使えばいいけどやっぱ手数が増える）
890:879
07/10/09 05:50:29
>>880,882
一応きりのいいところできられてるっぽいが、
変数のスコープが狭くできているとかいうこともない。
calc2以下は前の関数から呼ばれることを前提に作られている。
関数分割のメリットはどこにあるのやらｗ
>>881
うっ・・・こっちのほうがまだましだったか
当時の事情はよくわかんねえんだが、たぶん、
関数の長さが長すぎるといけない、とかいうんで無理矢理ちぎった
というのがオチだろうと思います。
891:仕様書無しさん
07/10/09 08:48:12
case型構文があるときはどうすんの？ ちょんぎれないよね。
892:仕様書無しさん
07/10/09 11:22:48
ちょんぎるんじゃない、構造を見直すんだ
893:仕様書無しさん
07/10/09 12:14:08
case文ひとつで何百行にもなるのなら、その時点でおかしい。
ちょんぎるとかいうレベルじゃない。
894:仕様書無しさん
07/10/09 12:53:47
だって100とおりのcaseなら各中身が平均４行だって500行ぐらいすぐなるじゃない。
コマンドが何十種類とか。caseの数が多いこと＝設計ミスとは決めつけられないでしょ。
895:仕様書無しさん
07/10/09 13:01:03
決め付けはできないけど、大抵の言語でダメだよ
896:仕様書無しさん
07/10/09 13:03:01
>>893
つWindowsアプリのソースコード
897:仕様書無しさん
07/10/09 13:42:24
>>894
それはcase文ひとつで数行である、ってことでは？
898:仕様書無しさん
07/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:仕様書無しさん
07/10/09 18:35:36
>>898
それって非常にありがちだけど、個人的にはそういう書き方は自己満足にしかなってないと思うなあ。
一度反省的に自分のそういうコードを見直してみて欲しいよ。
一箇所からしか呼ばれないコードを関数に括りだすことが本当に可読性に資するのか。
むしろ、コードを読む際にそこに飛ぶ手間を増やしているだけじゃないのか。
「caseの中につらつら書いた」コードが本当に読みにくいのか。
むしろ教条主義的にそう思い込んでるだけじゃないのか。
900:仕様書無しさん
07/10/09 19:00:35
抽象ってどういう意味か考えてみて欲しいね
「読まければ意味がわからない関数」は括り出す意味は無い
可読性をあげるためには、「今まさに関心のある」レイヤ以外に存在する
関数について、「読む必要性自体」を消し去る必要があるのだが
901:仕様書無しさん
07/10/09 19:10:34
「コードを読む際にそこに飛ぶ」のにそんなに手間がかかるのかｗ
902:仕様書無しさん
07/10/09 19:13:13
>>894
俺ならcaseマッチングのテーブルを挟んでループさせるな。
マッチングオブジェクトは当然処理関数への参照を持たせる。
マッチングしたら対象オブジェクトの処理を呼び出す。
C/C++なら普通に書ける関数テーブル。
処理部分はスマートになるし、定義部分ではコメント多用して
逆に分かり易く表現出来る。
VBはしらねーよw
903:仕様書無しさん
07/10/09 19:17:03
OOPLにおいては、
どうやって分岐するか、と分岐した先で何をやるか
は、分離しとかないと基本的にはOCP違反になる
904:仕様書無しさん
07/10/09 19:34:57
関係の無いメッセージに対する処理を脳内でスキップする方が
関数1個参照に行くよりよほど時間かかるよな
何のサポートもしてくれないエディタ使ってる場合はどうか知らんが
長大な関数は変数のスコープなんかで
気にしないといけないことも爆発的に増えるし
905:仕様書無しさん
07/10/09 20:53:01
>>902
状態遷移表の実装なんかで俺もソレ良くやるけど、
関数ポインタ(Cなんで)使うと、静的解析で処理が追い辛くなるのが難点かねぇ。
906:仕様書無しさん
07/10/09 21:03:31
こういうときデリゲートとかラムダ式があれば…
boost禁止だしのぅ
907:仕様書無しさん
07/10/09 21:11:55
>>904
分かってないね。
「関係の無いメッセージに対する処理をスキップ」する必要性は
各メッセージの処理を関数に括り出そうがなくならない。
ただエディタ上で「読み飛ばす」行数が減るだけの効果しかないんだよ。
それに、ウィンドウプロシージャのような定型的で分かりきった処理は
行数が増えてもコードの見通しは悪くならない。
実際試せば実感できると思うけど、
こういう場合はむしろ関数呼び出しによる抽象化の方がコードの不透明性を高めるから
読んでイライラする蓋然性が上昇するよ。
908:仕様書無しさん
07/10/09 21:23:47
>>907
わかってないのはお前だ。だまされたままで良いから周囲にあわせとけ。
909:仕様書無しさん
07/10/09 21:26:18
んー・・・凄い個人的な主観的な話になるけど、
caseとcaseの間がエディタの半ページ以内なら気にならない。
1つのcaseでエディタ1ページを超えると途端に読み辛くなる。
開発環境によってエディタの行数変わるんだけど
(WindowsだとVCかサクラ、Linuxだとemacsとか)、
俺は大体こんな感じ。
910:仕様書無しさん
07/10/09 21:33:48
>>907
ガラクタを無秩序に押入れに押し込むことを抽象化と呼ぶのなら
お前のいうとおりだろうな
関数名に連番とか振ってないだろうなｗ？
911:仕様書無しさん
07/10/09 21:48:45
>>910
＞ガラクタを無秩序に押入れに押し込むことを抽象化と呼ぶのなら
そんなトンデモな前提に立った議論ではない。
君とか904は、まあダメなプログラマにありがちだけど
暗黙のうちに「プログラムを書いている時点の視点」に立っている。
確かにプログラムを書いている時点に限定すれば、ほとんどの場合は
関数に括り出した方が読みやすいし、見た目にもスッキリする。
「caseの中につらつら書いた」コードなんて醜いし見難いように思われる。
ところが同じコードを一年後に読むと評価が逆転するんだなこれが。
912:仕様書無しさん
07/10/09 21:53:11
なにそれ、全然理由になってない
913:仕様書無しさん
07/10/09 21:54:05
>>910
>関数名に連番とか振ってないだろうなｗ？
関数名、変数名がすべて [A-Z]{2,3}[0-9]{5} なコードの保守ならやらされたことがある。
それはともかく、case 文の中身が10行超えたら、普通 state か command か chain of responsibility あたりを使わね？
914:仕様書無しさん
07/10/09 21:54:51
case 1:
{
} break;
case 2
{
って最初に作っておいたらスコープ付くしbreak強制になるから便利
915:仕様書無しさん
07/10/09 21:55:11
>>911
オブジェクト指向にOCPってあるんだけど、きいたことある？
いろいろ勉強して、実践して、その上でいってるのか心配になっちゃうんだけど
916:仕様書無しさん
07/10/09 21:58:03
デリゲートを条件に使われる定数をインデックスにして連想配列にぶちこむのは一種のストラテジーパターンらしい
917:仕様書無しさん
07/10/09 21:59:40
>>915
そいつのレスはムダに長いが、主張したいことは最後の1-2行だけ。
そこ見たらこれ以上そいつにかまう必要が無いことがわかる。
918:仕様書無しさん
07/10/09 22:11:46
>>899
> >>898
> 一箇所からしか呼ばれないコードを関数に括りだすことが本当に可読性に資するのか。
Testability
919:仕様書無しさん
07/10/09 22:22:07
>918
君の一言を待っていた
920:仕様書無しさん
07/10/09 23:55:45
( ﾟдﾟ )
921:仕様書無しさん
07/10/10 00:47:25
この類の話は
センスがない人にはいくら説明しても理解できないんだよね。
たぶん>>899,907,911は文系か元コボラ。
922:仕様書無しさん
07/10/10 01:30:46
>>921
今追いついたけど同意
923:仕様書無しさん
07/10/10 01:38:13
厳しいこといえば、センスとか言ってるやつもヤバイ
基本中の基本だ
924:仕様書無しさん
07/10/10 01:57:12
こういうやつらが消火の悪いスパゲティ作るんだろうな
925:仕様書無しさん
07/10/10 03:54:45
燃え盛ったときに消しにくいんだな
926:仕様書無しさん
07/10/10 04:32:11
負け組PGの揚げ足取り乙
927:仕様書無しさん
07/10/10 08:17:56
>>899
逆でしょw
武道みたいにプログラミングでもスタイルや「型」は大事だし、
それらはほとんどの場合正しいが、いつでも正しいわけじゃない。
意味を持った処理は関数に括り出して抽象化することによって読みやすくなる、
という「型」はほとんどの場合正しいが、いつでも正しいわけじゃないんだが、
自分の頭で考えられない教条主義者（＝「センス」を欠いた人間）にはそれが
判断できないし、そういう発想が頭をよぎっても抑圧してしまうんだね。
王様は裸だ、と言えないのと同じ心理的メカニズムが作用するからねw
928:仕様書無しさん
07/10/10 08:38:05
何が「逆」なのか、説明してくれ
929:仕様書無しさん
07/10/10 10:11:44
おまえら、当然ファウラーの「リファクタリング」を読んだ上で発言しているだろうな。
930:仕様書無しさん
07/10/10 11:43:05
>>929
教科書主義者乙
931:仕様書無しさん
07/10/10 12:09:26
> 教科書主義
932:仕様書無しさん
07/10/10 12:15:19
いくら教条主義はNGだからつって、そもそも教科書読んでねえやつは論外なわけだ
933:仕様書無しさん
07/10/10 15:16:54
同意
934:仕様書無しさん
07/10/10 21:36:24
>>927
あいかわらずムダな長文だな。自分にレスして楽しいか？
935:仕様書無しさん
07/10/10 23:40:23
俺には無意味な事を二度書いて、
最後に無関係なたとえ話をしている様に見える。
誰か >>927 をリファクタリングしてくれ。
936:仕様書無しさん
07/10/10 23:49:49
case毎の処理を関数に切り出すと読み辛くなるってのは、
caseの切り分け方がおかしい証拠に他ならんと思うのだが。
あと
>927
>意味を持った処理は関数に括り出して抽象化することによって読みやすくなる、
>という「型」はほとんどの場合正しいが、いつでも正しいわけじゃないんだが
の「正しくないケース」ってどんな時さ。
抽象化によって読み辛くなるのは、抽象化の仕方がおかしい時だけでしょ。
937:仕様書無しさん
07/10/10 23:50:25
>>935
#修正しておきました。
938:仕様書無しさん
07/10/11 00:07:05
>>937
全消しかいｗ
939:仕様書無しさん
07/10/11 00:39:54
>>936
>>898-を参照
940:仕様書無しさん
07/10/11 00:45:20
WTLのメッセージマップとか激しく便利だお
941:仕様書無しさん
07/10/11 19:34:57
>>936
正しくないケースを考えてみた。
1. >>927みたいな奴しかいない現場
2. 作成した関数を全て紙で管理している現場
3. 将来メンテする奴への嫌がらせ
4. リソースがカツカツ
5. そもそもサブロジックを切り出せない言語だ
6. コンパイラの限界に挑戦中
こんなもんか？
942:936
07/10/12 02:21:01
>941
納得してしまった。w
943:仕様書無しさん
07/10/12 09:24:34
>>941
>5. そもそもサブロジックを切り出せない言語だ
そうかN88-BASICだったんだ
944:仕様書無しさん
07/10/12 11:56:55
N88-BASICじゃ仕方ないなｗ
N88-BASICでも、変数名と戻り値代わりに使う変数名を工夫すれば
GOSUB との組み合わせで何とかなるかもしれんが
本質じゃない不毛なコードが大量生産されそうだ
945:仕様書無しさん
07/10/12 13:33:21
いろんな火災現場を転々としてるダメPGが言うのもなんだけど
どちらの書き方でも、どんなに拘りの秘伝のソースでもいいが
可読性の無いソースを書くのはやめて欲しい。。
>>944
BASICの頃でも、ON (式) GOSUB *Label...でサブルーチン化はしてたよ。
946:仕様書無しさん
07/10/12 13:59:28
>945
>可読性の無いソースを書くのはやめて欲しい。。
無理。
自分の常識は他人の非常識である場合が多い。
# だからflgで３値を返すなとあれほど･･･
947:仕様書無しさん
07/10/13 15:07:00
いやそもそも可読性が無ければ保守不能だし。「可読性が低い」だろ。
じゃあその可読性の水準を向上させるためにはどうすれば良いか。
という話になるけど。これは99%以上が独学だろ。
スレの上のほうに出てきた本を読んでおくのは当然。
OSSという容易に読めるコードもあるじゃないか。