Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

スコア算出のリファクタリング #64

Merged
merged 1 commit into from
Nov 15, 2024

Conversation

Kubosaka
Copy link
Collaborator

@Kubosaka Kubosaka commented Nov 15, 2024

User description

概要

  • スコア算出する際のロジックを修正した

PR Type

enhancement


Description

  • scoreRegister function has been refactored to include an assignmentId parameter, enhancing the score calculation logic.
  • The assignmentDate function now requires an assignmentId parameter to fetch the assignment date.
  • Improved error handling by adding a missing semicolon.
  • Adjusted the score calculation logic to utilize the new parameter, ensuring more accurate results.

Changes walkthrough 📝

Relevant files
Enhancement
route.ts
Update function call to include assignmentId parameter     

app/src/app/api/minio/route.ts

  • scoreRegister function now takes an additional assignmentId parameter.

  • +1/-1     
    scoreRegister.ts
    Refactor score calculation with assignmentId parameter     

    app/src/functions/scoreRegister.ts

  • assignmentDate function now requires assignmentId parameter.
  • scoreRegister function updated to accept assignmentId parameter.
  • Improved error handling with added semicolon.
  • Adjusted score calculation logic to use new parameter.
  • +10/-4   

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    @Kubosaka Kubosaka self-assigned this Nov 15, 2024
    const assignment = await prisma.assignment.findFirst({
    where: { id: assignmentId },
    Copy link
    Collaborator Author

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    ここで取ってくる課題が違ったので修正

    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    パラメータの追加
    scoreRegister 関数に assignmentId パラメータが追加されました。この変更が他の関数やメソッドに影響を与えていないか確認する必要があります。

    関数の変更
    assignmentDate 関数が引数 assignmentId を必要とするように変更されました。この変更により、関数の呼び出し元でのエラーが発生していないか確認が必要です。

    ロジックの変更
    スコア計算ロジックが変更されています。特に、正規化された時間の計算方法が変更されているため、期待されるスコアの出力が正しいかテストを行う必要があります。

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Best practice
    HTTPレスポンスのステータスを確認してからJSONを解析する。


    assignmentDate関数の戻り値を直接JSONとして解析する前に、HTTPステータスコードを確認してください。これにより、エラーレスポンスが適切に処理され、ランタイムエラーを防ぐことができます。

    app/src/functions/scoreRegister.ts [36-37]

     const response = await assignmentDate(assignmentId);
    -const assignmentDateValue = await response.json();
    +if (response.ok) {
    +  const assignmentDateValue = await response.json();
    +} else {
    +  // エラーハンドリング
    +}
    Suggestion importance[1-10]: 8

    Why: This suggestion correctly identifies a potential runtime error when parsing JSON without checking the HTTP response status. Implementing this check enhances error handling and robustness.

    8
    scoreオブジェクトの生成前にプロパティの検証を行う。


    scoreRegister関数内でprisma.score.createを呼び出す前に、生成されるscoreオブジェクトのプロパティが適切な型と値を持っているかを検証してください。

    app/src/functions/scoreRegister.ts [50-51]

    +if (typeof point !== 'number' || isNaN(point)) {
    +  throw new Error('Invalid point value');
    +}
     const score: Score = await prisma.score.create({
       data: {
         ...
       }
     });
    Suggestion importance[1-10]: 7

    Why: Validating the properties of the score object before database insertion is a best practice that ensures data integrity and prevents potential errors from invalid data types or values.

    7
    Possible bug
    scoreDataオブジェクト自体のnullチェックを追加する。


    scoreRegister関数内で、scoreDataのプロパティがnullでないかのチェックを行っていますが、scoreData自体がnullまたはundefinedでないことも確認してください。

    app/src/functions/scoreRegister.ts [22-28]

    -if (
    +if (!scoreData || 
       scoreData.similarity == null ||
       scoreData.answerTime == null ||
       ...
     ) {
       return;
     }
    Suggestion importance[1-10]: 7

    Why: Adding a null check for the scoreData object itself is a valid precaution to prevent runtime errors due to dereferencing null or undefined, enhancing the function's reliability.

    7
    normalizedTimeの計算式を修正して、値が1を超えないようにする。

    scoreRegister関数で計算されるnormalizedTimeが1を超える可能性があります。これを防ぐために、計算式を見直してください。

    app/src/functions/scoreRegister.ts [43-45]

    +const timeDifference = (answerIntervalTime / 1000 - minTime);
     const normalizedTime = Math.max(
       0,
    -  Math.min(1, (answerIntervalTime / 1000 - minTime) / (maxTime - minTime)),
    +  Math.min(1, timeDifference / (maxTime - minTime)),
     );
    Suggestion importance[1-10]: 6

    Why: The suggestion correctly identifies a potential issue with the calculation of normalizedTime that could exceed 1. The proposed fix ensures the value remains within expected bounds, improving the accuracy of score calculation.

    6

    Copy link
    Collaborator

    @hikahana hikahana left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    LGTM

    @hikahana hikahana merged commit b315f72 into main Nov 15, 2024
    2 checks passed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants