chore: CSVパーサーのリファクタリング #700
Open
+210
−88
Conversation
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
Deploying kcmsx with
|
| Latest commit: |
3ae6dac
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d0fe50d3.kcmsx.pages.dev |
| Branch Preview URL: | https://chore-csv-parser-to-object.kcmsx.pages.dev |
kiharu3112
requested review from
tufusa,
laminne and
speak-mentaiko
and removed request for
tufusa and
laminne
…onnet/kcmsx into chore/csv_parser_to_object
tufusa
requested changes
Nov 16, 2024
There was a problem hiding this comment.
通知の表示は明らかに副作用なので、この関数から分離しているとうれしいです。
| }; | ||
|
|
||
| export const checkData = (data: CSVRow[]) => { | ||
| const newErrors = data.map(() => [false, false, false, false, false, false]); |
There was a problem hiding this comment.
二重配列でなく、CSVDataのようにオブジェクトにして、値にbooleanではなくエラーコードを仕舞うようにするのがよいでしょう。
Comment on lines
+64
to
+80
| const teamNames = data.map((row) => row.teamName); | ||
| const duplicateTeamNames = teamNames.filter( | ||
| (name, index) => teamNames.indexOf(name) !== index | ||
| ); | ||
|
|
||
| // 重複するteamNameがあれば、該当する全ての行にエラーフラグを設定 | ||
| if (duplicateTeamNames.length > 0) { | ||
| duplicateTeamNames.forEach((dupName) => { | ||
| teamNames.forEach((name, index) => { | ||
| if (name === dupName) { | ||
| newErrors[index][0] = true; | ||
| isError = true; | ||
| notifyError("duplicateTeamName"); | ||
| } | ||
| }); | ||
| }); | ||
| } |
There was a problem hiding this comment.
計算量がO(n^2)になっているので、SetやHashを使ってください。
| }); | ||
| }); | ||
| } | ||
| return { newErrors, isError }; |
There was a problem hiding this comment.
Suggested change
| return { newErrors, isError }; | |
| return { errors, isError }; |
|
|
||
| export const checkData = (data: CSVRow[]) => { | ||
| const newErrors = data.map(() => [false, false, false, false, false, false]); | ||
| let isError = false; |
There was a problem hiding this comment.
letはやめましょう。
この場合、isErrorはerrorsから導出できるはずです。
| @@ -169,7 +123,7 @@ export const RegisterBulk = () => { | |||
|
|
|||
| return ( | |||
| <> | |||
| <h1>一括エントリー</h1> | |||
| <Text>一括エントリー</Text> | |||
There was a problem hiding this comment.
Suggested change
| <Text>一括エントリー</Text> | |
| <Title>一括エントリー</Title> |
Comment on lines
+31
to
+32
| robotType: RobotType; | ||
| departmentType: DepartmentType; |
There was a problem hiding this comment.
パースした直後にこの型を使っているので、RobotTypeやDepartmentTypeとは確定しないはずです。stringがよいでしょう。
| }); | ||
| }; | ||
|
|
||
| export const checkData = (data: CSVRow[]) => { |
There was a problem hiding this comment.
React的にはこれはカスタムフックであったほうが嬉しいです。
const {isError, errors} = useCheckData(data);
のような使い方ができるようにしてほしいです。
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
close #637
意図と違ったらごめん