Kazuki Ohta
mover****@hct*****
2005年 8月 22日 (月) 13:21:59 JST
太田です。
僕は自分自身でもこういった細かいコーディング習慣等にまだまだ欠けている事を
認識しているので、バシバシ指摘して下さい。というか逆にこういった事を指摘され
る事で僕も勉強になるので、今回YamakenさんがSigSchemeのコードを事細かに
チェックして、わざわざツッコミを入れて頂いた事に逆に感謝します。ありがとう御座
います。
以下、一つ一つ返信します。井上さんの [Anthy-dev 2252] もふまえつつ返信し
ているつもりです。
> * コーディングスタイル
>
> - 以下のようなコードが多く見られますが、
>
> 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;
> }
これは逆にcase分の後に長いロジックを書く場合においては、"{"でくくった方が読み
やすいと思っています。もちろん単に1関数の呼び出しの場合だけだとこちらの方が
良いとは思いますが。どう思われますか？
caseのインデントが深くなっているのは修正します。
> - 井上さんのおっしゃるように原則として79桁に収めてほしいです。端末もエ
> ディタも80桁表示が標準なので。私自身もemacsは80桁表示設定になってい
> るのでなるべく折り返しが無い方が嬉しいです。
了解です。なるべく意識します。
# 僕も80桁表示設定になっていますが...
> - インデント中に時々タブが混じっていますが、スペースで揃えていいんです
> よね?
はい。これはEmacsの設定を見直します。
> * コード各論
>
> - EQ(), NEQ()マクロは容易に他のライブラリと衝突するのでプリフィクスの
> 付いたSCM_EQ()等だけを残して消すべきです(利用をやめるだけでは不十分)。
この辺りはsigschemeinternal.hに追い出すという事で。
> - 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_という実例があります。
なるほど。確かにそうですね。SScm_に統一したい所ですが、コード中にSScm_が
たくさん出てくるのもなぁと思うのでScm_に統一しようと思います。
# 井上さんの提案のCAR等にはprefixは付けないという方針も賛成です
> - 以下のように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"で置き換えて変換関数である事を明示・強調し、視認性も
> 上げるという習慣が割と広く根付いています。もし趣味に合えば採用してみ
> てはどうでしょう。
そうですね、こちらの方がよりUNIX的というか支持を得ていますね。
> - 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はそのまま)。これも適当なタイミングで。
了解です。これは一応uimが安定して動いたら変更します。
> -------------------------------
> ヤマケン yamak****@bp*****
> _______________________________________________
> Anthy-dev mailing list
> Anthy****@lists*****
> http://lists.sourceforge.jp/mailman/listinfo/anthy-dev
--
-------------------------------------------------
Kazuki Ohta : mover****@hct*****
-------------------------------------------------