エラーハンドリングを実装 #46
エラーハンドリングを実装 #46
Conversation
|
急に思いついて作業しただけなので、思いついたときにレビューお願いしますw |
There was a problem hiding this comment.
良い感じだと思います、例外が起こりうるときに throw するか返り値を Either 風にするかみたいな問題ですね。
型安全になるのはいいんですが、アルゴリズム的に色々やってるぶん予期しないところで throw される可能性も拭えないので、公開する関数を絶対に throw しない仕様にしたければ try は残したほうがいい気もしなくもないですし、そこまで気にしなければ別にいいかなと思います。
以前はハンドリングしておらず自動で落ちていた
エラーハンドリングしていなかったのは test.ts だけで、外部に公開している index.ts の方ではハンドリングできてた気はします。
|
あれ、レビューが消えてる |
| } else { | ||
| return { status: 'Error', message: 'Undefined Error.' }; | ||
| } | ||
| const pat = new Parser(src, flags).parse(); |
There was a problem hiding this comment.
parse は構文エラーを throw する可能性があるので try で囲ったほうがいいと思います。
| return { status: 'Error', message: 'Undefined Error.' }; | ||
| } | ||
| const pat = new Parser(src, flags).parse(); | ||
| const enfa = buildEpsilonNFA(pat); |
There was a problem hiding this comment.
buildEpsilonNFA も文字集合を構築するときに throw する可能性があった気がします……。
| export type BuildChildResult = { type: 'BuildChildResult' } & Pick< | ||
| EpsilonNFA, | ||
| 'initialState' | 'acceptingState' | ||
| >; |
There was a problem hiding this comment.
たぶん buildChild 限定の内部的な型だと思うので enfa.ts にあった方がいい気がします。
| throw new Error('Illigal use LineBegin'); | ||
| for (let i = 0; i < node.children.length; i++) { | ||
| if (i > 0 && node.children[i].type === 'LineBegin') { | ||
| return { type: 'Error', error: new Error('Illigal use LineBegin') }; |
There was a problem hiding this comment.
このコミットのものではないですが、以下のほうが良い気がしたのでついでに修正してもらいたいです……。
| return { type: 'Error', error: new Error('Illigal use LineBegin') }; | |
| return { type: 'Error', error: new Error('Illegal use of LineBegin') }; |
| i < node.children.length - 1 && | ||
| node.children[i].type === 'LineEnd' | ||
| ) { | ||
| return { type: 'Error', error: new Error('Illigal use LineEnd') }; |
There was a problem hiding this comment.
| return { type: 'Error', error: new Error('Illigal use LineEnd') }; | |
| return { type: 'Error', error: new Error('Illegal use of LineEnd') }; |
|
レビューありがとうございます！(EitherとかResultみたいな雰囲気を醸し出したいなと思いました) |
解決する Issue
close #45
改善内容