Created
June 18, 2012 02:49
-
-
Save camlspotter/2946567 to your computer and use it in GitHub Desktop.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
(* | |
(* 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)+副作用! | |
- コメント日本語ってつらいですね… 英語の方が、 | |
- だらだらと書く英語力がないので短くなる | |
- それでいて、他人に英語が通じるか不安なので意味不明なコメントになりにくい | |
ので日本人同士でもチーム内で英語力がある程度あれば、英語が良いと思っています。 | |
*) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
to は OCaml のキーワードですのでレコードフィールド名等に使うことが出来ません。私はキーワードと被る変数を使いたいときは最後に _ をつけることにしています。
よくやるのが start end での end_ ですね。