YamaKen
yamak****@bp*****
2005年 8月 21日 (日) 22:49:13 JST
ヤマケンです。 SigSchemeの実用化が近くなってきたようなので、コーディング面の要望をま とめてみました。今すぐとは言わないので、適当なタイミングで徐々に実現し てもらえると嬉しいです。 色々と小うるさい事がてんこ盛りに書いてありますが、理路整然としたコード を実現するために必要な作業だと考えています。 安定した論理・形式スタイルに基づいたコードの記述と整理整頓は単なる自己 満足のためではなく、安全な記述パターンを習慣化する事でバグの入れ込みを 減らし、さらにその安全なスタイルから外れた怪しいコードを敏感に検出する ためのものでもあります。スタイルから外れているという事はいい加減に書か れたという事であり、いい加減に書かれたコードにはバグが混入している可能 性が高いです。 聞いた話ですが、良い工場では整理整頓、整備及び掃除を徹底するそうです。 そうする事によって、いつもの正常な状態との微妙な違い、例えばかすかな異 音等に敏感に気付いてトラブルを防ぐ事ができるそうです。また、別の話で偽 札鑑定を仕事にしている人は毎日本物のお札ばかりを触ったり眺めたりしてい るそうです。そうすると偽札の微妙な違和感にすぐ気付くそうです。 #どこまで本当かは知りませんが uimの開発もいつかその境地に達する事ができたらいいなと思います。もちろ ん偉そうな事書いてる私自身にまだまだ精進が足りませんが、少しづつでも良 い作法を確立して習慣化していきたいと思っています。他のcommitterの皆さ んも同じ目標を意識してもらえると嬉しいです。 * コーディングスタイル - 以下のようなコードが多く見られますが、 if (cond) return SCM_TRUE; else return SCM_FALSE; if (cond) return SCM_TRUE; return SCM_FALSE; 特にポリシーがなければ次のように条件式で統一しませんか? 私はこの方が 見やすくていいと思います。論理も視覚的に一瞬で把握できますし。 return (cond) ? SCM_TRUE : SCM_FALSE; - switch文のインデントが以下のようにかなり深くなっていますが、 switch (val) { case c1: { int i; foo(); } break; case c2: { bar(); } break; case c3: baz(); break; default: return; } 次のような形式に変えませんか? 変数宣言が無い場合は{}も外して。無条件 に{}で括りたいという動機はよくわかりますが、ソースが読みづらくなるデ メリットの方が大きいと感じられるので。 switch (val) { case c1: { int i; foo(); } break; case c2: bar(); break; case c3: baz(); break; default: return; } - 井上さんのおっしゃるように原則として79桁に収めてほしいです。端末もエ ディタも80桁表示が標準なので。私自身もemacsは80桁表示設定になってい るのでなるべく折り返しが無い方が嬉しいです。 - インデント中に時々タブが混じっていますが、スペースで揃えていいんです よね? * コード各論 - EQ(), NEQ()マクロは容易に他のライブラリと衝突するのでプリフィクスの 付いたSCM_EQ()等だけを残して消すべきです(利用をやめるだけでは不十分)。 - SCM_TRUEP()が#tとの同値チェックとして定義されてますが、#tは真の代表 値に過ぎないので、真かどうかをチェックするためにこれが使われるとバグ を生みます。これを防ぐためにSCM_TRUEP()は消して代わりにSCM_NFALSEP() を定義して利用すべきです。どうしても#tとの同値チェックが必要な時は SCM_EQ(x, SCM_TRUE)が利用できます。 - SCM_UNSPECIFIEDはR5RSで ==> unspecifiedとなっているのに対応している と思われますが、これは何が返るか仕様として規定されてない (unspecified)というだけの話なので、実際に返す値としてはSCM_UNDEFや SCM_FALSEを使うべきです。特にpredicateでSCM_UNSPECIFIEDを返すと真と 解釈されてしまうのでSCM_TRUE, SCM_FALSEのいずれかを返すかエラーにす べきです。 - FALSEP()等を使わず、EQ(ScmOp_numberp(obj), SCM_FALSE)のように直接 SCM_FALSEと比較しているコードが多くありますが、特にポリシーがなけれ ばFALSEP()やNFALSEP()で単純化しましょう。 * 命名規則 - プリフィクスの使い分け(SigScm_, Scm_, ScmExp_, ScmOp_, sig)が不明確 なので整理する必要あり * Scm_で統一していいのでは(井上さん) Schemeのフォームとして呼ばれるものとlibsscmのユーザ向けの関数の2つぐ らいには分けてもいいかも。いい名前が思い付きませんが。 ただし、Scm_とSigScm_の使い分けで何らかの違いを表現するのはまずいや り方です。名前自身に記号の違い以上の情報が含まれておらず、混乱を招く 上に単なる一貫性の欠如と見なされる可能性が高いからです。これにはrb_ とruby_という実例があります。 - 以下のようにFuncNameスタイルとfunc_nameスタイルが混在してますが、ど ちらかに統一して欲しいです。Schemeフォームとlibsscm関数の区別も目的 なのかもしれませんが、あまり一般的でないやり方なので。 SigScm_InitStorage() SigScm_FinalizeStorage() SigScm_gc_protect() SigScm_gc_protect_stack() - 現在SigScheme用の名前空間の確保にはScm_プリフィクスが使われています が、uim以外の組み込み用途をどれだけ広く狙うかによってはSScm_等に変え た方がいいかもしれません。特にUNIX外の世界ではどんな名前が使われてい るか分からないので。"scm embedded"でググると結構色々出てきます。ただ キリのない話なので、コード中での語感に馴染めなかったり長すぎると感じ るならScm_で十分だと思います。 * 名前各論 - SCM_NIL等、"nil"という名前が空リストを指すものとして各所で使われてい ますが、これは伝統的lispの空リストでもあり偽でもあるthe nilとの混同 を招くので"null"に変えて欲しいです。他の実装がnilという名前を使って いるかどうかに関係なく。R5RSもnilとの訣別は強調しています。 - SCM_SETINT(obj), SCM_SETCONS(obj)等はScmObjInternalのtypeだけを設定 するマクロですが、名前から実際の動作が想像しにくく、また数値やconsセ ルの内容を設定するためのマクロだと誤解される恐れがあるので、型情報の 設定のみを行うマクロである事をもっとアピールすべきだと思います。 SCM_ENTYPE_CONS(obj)形式はどうでしょう? - ScmObjのsetterマクロがSCM_SETINT_VALUE(), SCM_SETSYMBOL_NAME()のよう に"SET"の後に"_"を付けず、また"SET"が型名の前に来るスタイルになって いますが、SCM_INT_SET_VALUE(), SCM_SYMBOL_SET_NAME()のような形式にし てはどうでしょう。読みやすいし、getterマクロであるSCM_INT_VALUE(), SCM_SYMBOL_NAME()との対称性も確保できるので憶えやすいと思います。 - SCM_GETTYPE()という名前は他のgetterマクロのSCM_INT_VALUE()等と一貫性 がないのでSCM_TYPE()とした方がよいと思います - SCM_CHAR_CH(obj)のように、その型のオブジェクトが保持する唯一の値を表 現する場合は、無理に"CH"のような名前を付けるよりもSCM_INT_VALUE(obj) と同様にSCM_CHAR_VALUE(obj)とした方が概念的に理解しやすく、また憶え やすくなるんじゃないでしょうか。SCM_C_POINTER_DATA()と SCM_C_FUNCPOINTER_FUNC()も同様にSCM_C_POINTER_VALUE()と SCM_C_FUNCPOINTER_VALUE()とした方が自然で憶えやすいと感じます。 - sigscheme.h経由で公開されるマクロや型名にはプリフィクスを付けるべき です。 typedef void (*C_FUNC) (void); struct trace_frame { #define DEBUG_GC 0 #define USE_EUCJP 1 #define USE_SRFI1 0 #define CHECK_1_ARG(arg) \ : - グローバルシンボルは全てプリフィクスを付けるべきです。uim-scmの下請 けとして使うだけなら問題ありませんが、libsscmを直接リンクする用途で は名前空間を汚染します。 /*======================================= Variable Declarations =======================================*/ /* datas.c */ extern ScmObj *stack_start_pointer; /* error.c*/ extern ScmObj current_error_port; /* eval.c */ extern struct trace_frame *trace_root; /* io.c */ extern ScmObj current_input_port; extern ScmObj current_output_port; - ScmOp_symbol_to_string()のような変換関数は、ScmOp_symbol2string()の ように"to"を"2"で置き換えて変換関数である事を明示・強調し、視認性も 上げるという習慣が割と広く根付いています。もし趣味に合えば採用してみ てはどうでしょう。 - ScmOp_SRFI_1_xcons()等の関数名はプリフィクスが細分化されて読みにくい ので、ScmOp_SRFI1_xcons()スタイルはどうでしょう? USE_SRFI1マクロとも 整合が取れるし。 - datas.cというファイル名はミススペルだと思われるのでdata.cに直すか別 の名前に。storage.cなんてどうでしょう。今やってしまうとsvn diff等が 不便になると思うので落ち着いたところでお願いします。ついでに ScmExp_case()のdatumsもdataに修正しましょう。 - uim-scm.cがuim-scm APIに対するSigScheme実装のコードになっていますが、 将来プロトタイピング目的等でGauche等のリッチな実装を使う時のために、 SigScheme依存部とそうでないところを分けておきたいと思います。と言っ てもやる事はmv uim-scm{,-sigscheme}.cしてビルドまわりを追従させるだ けです(uim-scm.hはそのまま)。これも適当なタイミングで。 ------------------------------- ヤマケン yamak****@bp*****