Skip to content

Instantly share code, notes, and snippets.

@camlspotter
Created June 18, 2012 02:49
Show Gist options
  • Star 9 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save camlspotter/2946567 to your computer and use it in GitHub Desktop.
Save camlspotter/2946567 to your computer and use it in GitHub Desktop.
(*
(* CR jfuruse: なんたら *) というのは前職でのコードレビューの書き方で、私の癖になっている。すべて、「私ならば…こうするかな?」が省略されています。
私ならやっつけモードでこう書く、という例です。人様のコードを元にしているので、ほんとにこう書くのかよ?という突っ込みはありかと思います。
元コードも実際のものを簡略化されたものだそうですので、私の提案コードのように書きたいけれども実は書けないんだ!ということもあるでしょう。
OCaml のプログラミングスタイルは決まったものはなくいろいろと流儀があります。その一つと思ってください。
*)
type request = { code : string; from : string; to_ : string }
(* CR jfuruse: 私はやっつけ仕事でも string が3つ並ぶ型は嫌いだ。なのでレコードにする。
(通貨ペア, 問い合わせfrom, to) だそうだ
業界人として code ではなく ccypair (Currency Pair) としたいwところ
to のような OCaml での keyword を変数やフィールド名に使いたい時はいくらダサくとも機械的に後に _ をつけることにしている。
*)
type quote = { price : int; }
(* CR jfuruse: 私はダブルセミコロンは syntax error が出た時のエラー場所探索の時しか使わない *)
(* CR jfuruse: このところ Emacs の align-regexp がお気に入り *)
type response = {
request : request; (* CR jfuruse: 型名とラベル名は同じにできるなら同じにしたい *)
quote : quote;
seq : int; (** id/position of this response in the whole *)
total : int; (** the total number of responses expected *)
}
(* 再送信要求のレスポンスは送信idと紐付けられていない(!)ため、
一度に多くのリクエストを投げた後は、レスポンスを整列させる必要がある。 *)
(* メッセージを整列させつつ格納するバッファ。
(通貨ペア, 問い合わせfrom, to) をキーとして
(受信した数, メッセージの配列) のリストで管理する。
リストになっているのは、重複するキーを持つレスポンスに対応するためである。
*)
(* CR jfuruse: コメントにするくらいなら型にするけど。名前が思いつかないので elem にする *)
type elem = {
mutable filled : int;
quotes : quote option array; (** things already received *)
}
(* CR jfuruse: destructive な map ならば Hashtbl を使うべき *)
let buf : (request, elem list) Hashtbl.t = Hashtbl.create 1023 (* magic number *)
(* 株価のリストをデータベースに保存する. ダミーで画面への出力関数にしてある *)
let save code quotes = (* CR jfuruse: さすがにここに来る前に option は外すか *)
print_string (code ^ " ");
List.iter (fun q -> Printf.printf "%d;" q.price) quotes;
print_newline ()
(*
メッセージrをバッファに格納する。 レスポンス全体が得られたら、saveで保存する。
*)
(* CR jfuruse: (r:response) よりは resp と書いたほうが *)
let receive_quote resp =
if resp.total = 1 then save resp.request.code [resp.quote] (* 総レコードが1件のみであればすぐに保存 *)
else begin (* CR jfuruse: この begin いらないけど、書きたくなるし書くべき *)
(* このリクエストに対するレスポンスのリスト(更新前) *)
let all = try Hashtbl.find buf resp.request with Not_found -> [] in
(* レスポンスのリストを走査し適切な位置に格納 / レスポンス全体が満たされたら保存 *)
let rec add_entry rev_skipped = function (* CR jfuruse: accumulator で reversed list 運んでる時は rev_ をつけることにしている *)
| [] ->
(* バッファに新しいレスポンスを追加 *)
let quotes = Array.make resp.total None in
quotes.(resp.seq) <- Some resp.quote;
{ filled= 1; quotes } :: all
| e::es -> (* CR jfuruse: elem にしたので e::es *)
match e.quotes.(resp.seq) with
(* CR jfuruse: 短い方を先に書く(書ければ) *)
| Some _ -> add_entry (e :: rev_skipped) es (* このレスポンスでは格納済み. 次のレスポンスを見る *)
| None ->
e.quotes.(resp.seq) <- Some resp.quote; (* レスポンスの該当する連番を満たす *)
e.filled <- e.filled + 1;
if e.filled = resp.total then begin
(* レスポンス全体が満たされた. 保存後、この配列をバッファから除く *)
save resp.request.code (List.map (function None -> assert false | Some v -> v) (Array.to_list e.quotes)) (* 保存 *);
List.rev_append rev_skipped es (* = all - e *)
end else
(* また足りない. *)
(* CR jfuruse: ま*だ*足りない. *)
all (* CR jfuruse: mutable rocks! *)
in
match add_entry [] all with
| [] -> Hashtbl.remove buf resp.request
| es -> Hashtbl.replace buf resp.request es (* Not Hashtbl.add. Replace. *)
end
let test =
let a = { code= "A"; from= "1"; to_= "4" }
and b = { code= "B"; from= "1"; to_= "2" } in
let resps = [
{request= a; quote= {price=1100}; total= 4; seq= 0};
{request= a; quote= {price=1098}; total= 4; seq= 1};
{request= a; quote= {price=1100}; total= 4; seq= 0};
{request= b; quote= {price=910}; total= 2; seq= 0};
{request= a; quote= {price=1098}; total= 4; seq= 1};
{request= a; quote= {price=1081}; total= 4; seq= 2};
{request= a; quote= {price=1081}; total= 4; seq= 2};
{request= a; quote= {price=1120}; total= 4; seq= 3};
{request= b; quote= {price=940}; total= 2; seq= 1};
{request= a; quote= {price=1120}; total= 4; seq= 3};
] in
List.iter receive_quote resps
(* 感想
- 変数一文字にしたり cons にスペース無かったり、引数に型書きたいあたり、 Haskell の人っぽいコードですねー
- Map は遅いぞハスケロウ! log(n)+副作用なぞ悪いとこ取り! 毒を食らわばキャミバ様なら log(1)+副作用!
- コメント日本語ってつらいですね… 英語の方が、
- だらだらと書く英語力がないので短くなる
- それでいて、他人に英語が通じるか不安なので意味不明なコメントになりにくい
ので日本人同士でもチーム内で英語力がある程度あれば、英語が良いと思っています。
*)
@camlspotter
Copy link
Author

今気づいたけど resp って resp 単体で使われてないから初めから { request; quote; ... } とパターンマッチした方がよいですね

@tmaeda
Copy link

tmaeda commented Jun 19, 2012

大変興味深く読ませて頂きました。

一点だけわからなかったので教えて頂けないでしょうか?
to を to_ にするのはどういう意味でしょうか?
なぜ、他のフィールド名にはアンダーバーはつかなくて、toだけ特別扱いなのでしょうか?

@camlspotter
Copy link
Author

to は OCaml のキーワードですのでレコードフィールド名等に使うことが出来ません。私はキーワードと被る変数を使いたいときは最後に _ をつけることにしています。
よくやるのが start end での end_ ですね。

@tmaeda
Copy link

tmaeda commented Jun 19, 2012

なるほど。ありがとうございました。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment