Developer#3916
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Decision: REQUEST_CHANGES — There are critical blockers that break required functionality and fail core specs.
- In
src/components/Sum/Sum.jsx(top of file): the component signature is incorrect. You wroteexport const Sum = (a = 0, b = 0) => ..., which treats the first argument as the props object, soais an object anda + bwon’t compute correctly. Per requirement, accept props and default missing values: use destructuring with defaults, e.g.({ a = 0, b = 0 }) => .... This ensures correct numeric addition and proper defaulting to 0 when a prop is omitted. - In the same file, the paragraph content should be a single interpolated string [CHECKLIST ITEM #2]. Instead of multiple nodes like
Sum of {a} and {b} is {a + b}, use:<p>{Sum of ${a} and ${b} is ${a + b}}</p>to match the exact expected output. - In
src/App.jsx: the App currently renders four identical<Sum />components and includes placeholder comments. Replace them with the five required instances exactly:<Sum a={2} b={3} />,<Sum a={-5} b={5} />,<Sum a={10} />,<Sum b={5} />, and<Sum />. Remove placeholder comments and keep tags on one line for short attribute lists [CHECKLIST ITEM #1].
Minor: After changes, run tests to verify the exact string output and defaulting behavior. You’re close—fixing the props handling and rendering the specified instances will meet the requirements. Keep going!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| <Sum /> | ||
| <Sum /> | ||
| <Sum /> | ||
| <Sum /> |
There was a problem hiding this comment.
This violates the requirement from the task description: "The App should contain the next Sum components:
a = 2andb = 3;a = -5andb = 5;- just
a = 10; - just
b = 5; - no params at all."
Currently these lines (four identical <Sum />) do not pass any props and do not include the five required combinations. Replace the existing <Sum /> entries with the five required usages, passing props only where specified so the default behavior (omitted prop -> 0) can be tested. Example usages you should include (one per line):
<Sum a={2} b={3} /><Sum a={-5} b={5} /><Sum a={10} /><Sum b={5} /><Sum />
| {/* Replace paragraphs with Sum componets */} | ||
| {/* And remove commented lines :) */} | ||
| </> | ||
| </div> |
There was a problem hiding this comment.
Please remove placeholder comments and keep the rendered Sum tags directly in the JSX. Also follow checklist item #1 when writing JSX tags with short attribute lists: "1. [CODE STYLE] - If you have < 3 attribues on a tag and the values are short
write the tag in one line (to make it easier to write and read)." Use one-line tags like <Sum a={2} b={3} /> rather than multi-line attribute formatting.
| @@ -1 +1,5 @@ | |||
| // export const Sum = () => (); | |||
| export const Sum = (a = 0, b = 0) => ( | |||
There was a problem hiding this comment.
This breaks the core requirement to accept a and b as props. The component is declared as export const Sum = (a = 0, b = 0) => ..., which treats the first React argument as the props object rather than the numeric a. From the task description: "Create a Sum component accepting a and b props ... If the a or b props are not passed, set them to 0."
Consider using destructuring with defaults: ({ a = 0, b = 0 }) => ... so a and b are the values passed via props.
| <p> | ||
| Sum of {a} and {b} is {a + b} |
There was a problem hiding this comment.
This violates checklist item #2: "2. [CODE STYLE] - Use string interpolation inside tag content" (see checklist for examples).
The current JSX uses multiple text nodes and expressions (Sum of {a} and {b} is {a + b}) instead of a single interpolated string. Use a template literal inside the paragraph to produce a single string node, e.g.:
<p>{`Sum of ${a} and ${b} is ${a + b}`}</p>| export const Sum = (a = 0, b = 0) => ( | ||
| <p> | ||
| Sum of {a} and {b} is {a + b} | ||
| </p> | ||
| ); |
There was a problem hiding this comment.
Because the function signature is incorrect, a will be the props object and a + b will not compute the expected numeric sum (and may cause "Objects are not valid as a React child" errors). Fix the signature to ({ a = 0, b = 0 }) and then use interpolation so the paragraph renders the exact required string: Sum of {a} and {b} is {a + b}. The task requires that missing props default to 0 and that the paragraph render the exact sentence (see description).
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Approval: The implementation meets the core requirements. The Sum component is a named export, correctly destructures a and b with numeric defaults of 0, and renders a single <p> with the exact template literal format: Sum of ${a} and ${b} is ${a + b}. The App component renders the five required instances: with both props, negative/positive, only a, only b, and neither.
Minor non-blocking reminders:
- Ensure the PR description includes a working demo link (
https://<your_account>.github.io/react_sum/). - Run
npm testand confirm formatting rules (Prettier/VSCode) are applied.
Nice work keeping the components concise and adhering to the checklist style rules.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
DEMO LINK and add it to the PR description.