finish challenge 2 recreate challenge 1 using node.js #20
Conversation
There was a problem hiding this comment.
Cause we will learn file management later on in the express.js basic course, I suggest that you locate the code as per the guidance below:
/srcfor all of the JavaScript core code, such as factorial-functional, factorial-oop, fizzbuzz-functional, fizzbuzz-oop, etc. Also, put anyutils,shared, orhelperscode in this folder./binfor all the JavaScript code related to the terminal execution process./pagesfor all the HTML page that has been created in Chapter 1, including some of the JavaScript code needed to run the HTML input like JavaScript DOM etc.
Thank you for the pull request.
bin/factorial/handleArgument.js
Outdated
|
|
||
| for (let argument of cliArguments) { | ||
| if (!validateArgument(argument)) { | ||
| throw new Error(`invalid argument ${argument}`); |
There was a problem hiding this comment.
You could make a better exception throwing by setting a specific Error instance rather than just a basic Error. See details here: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects#error_objects.
bin/shared/utils.js
Outdated
| @@ -0,0 +1,3 @@ | |||
| export const destructureArgument = argument => argument.split("="); | |||
There was a problem hiding this comment.
Move this file into /src folder. I'll explain it later at the end of this review.
bin/factorial/index.js
Outdated
| import { countFactorial } from "../../pages/functional/js/factorial.js"; | ||
|
|
||
| import { Factorial } from "../../pages/oop/js/Factorial.js"; | ||
|
|
||
| import { handleArgument } from "./handleArgument.js"; |
There was a problem hiding this comment.
No need extra line break for each part of the import code here.
bin/factorial/index.js
Outdated
| let result = 0; | ||
|
|
||
| if (!["oop", "functional"].includes(type)) { | ||
| throw new Error(`invalid type ${type}. must be "oop" or "functional"`); |
bin/factorial/index.js
Outdated
| if (!["oop", "functional"].includes(type)) { | ||
| throw new Error(`invalid type ${type}. must be "oop" or "functional"`); | ||
| } | ||
|
|
||
| if (!["loop", "recursive"].includes(method)) { | ||
| throw new Error(`invalid method ${method}. must be "loop" or "recursive"`); | ||
| } | ||
|
|
||
| if (typeof parseInt(n) != "number") { | ||
| throw new Error(`invalid number ${n}. n should be a number"`); | ||
| } |
There was a problem hiding this comment.
Put this validation code in the handleArgument file so this file will be focused on running the action/logic part.
bin/factorial/index.js
Outdated
| const factorial = new Factorial(n); | ||
| result = factorial.count(method); |
There was a problem hiding this comment.
Simplify this by just trimming the code into one line.
pages/functional/js/factorial.js
Outdated
| } | ||
| } | ||
|
|
||
| export { |
| @@ -0,0 +1,17 @@ | |||
| import { countFactorial } from "../js/factorial.js"; | |||
|
|
|||
| document.getElementById("form").addEventListener("submit", function (event) { | |||
There was a problem hiding this comment.
Turn back this file into functional folder. I'll explain it later at the end of this review.
|
Hello @ianriizky , I have already refactor 2nd challenge's code. this is the commit : refactor : move js to src and utility to utils Thanks, |
|
I found mistakes in previous commit, it is in input validation and I have fix it in here : |
| let parsedInput = parseNumber(input); | ||
| if (typeof parsedInput != "number") { |
There was a problem hiding this comment.
Add line break between this code.
| @@ -0,0 +1,19 @@ | |||
| export const validateFactorialMethod = method => { | |||
There was a problem hiding this comment.
I think these function along with the validateApproach.js can be unified into one function. Then the required list of method (loop, recursive, functional, oop, etc.) can be passed as a paremeter of the function.
Hello, I have finished challenge 2 to recreate challenge 1 using nodejs
This is what I already do :
Thanks,
Dian P.