-
Notifications
You must be signed in to change notification settings - Fork 16
Finish Challenge JS Basic and JS OOP #21
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
base: develop
Are you sure you want to change the base?
Conversation
no message
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your hard work. Unfortunately, there are still some reviews you need to work on with some other reviews I don't write here. But I leave this for now so you can focus on fixing the code while implementing it in the other code you've made. Thank you.
pages/functional/js/factorial.js
Outdated
* Count factorial number from the given "n" value using loop way. | ||
* | ||
* @param {number} n | ||
* Factorial Using Loop |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see a lot of whitespace at the end of each code line. If you're using VSCode, I suggest you install this extension as it will help you to automate the coding format process.
pages/functional/js/factorial.js
Outdated
|
||
return n * countFactorialUsingRecursive(n - 1); | ||
} | ||
console.log (factorialUsingLoop (6)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove the console.log() and any other console
function as it is unnecessary and therefore will expose your code execution process which also opens up the vulnerability inside the program itself.
pages/functional/js/factorial.js
Outdated
* | ||
* @param {number} n | ||
* Factorial Using Loop | ||
* @param {*} n |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although this n
parameter accepts any type of data, you must define the exact type of this parameter in terms to suggest the user to use the correct data type when using the function/method.
pages/functional/js/factorial.js
Outdated
* @param {number} n | ||
* Factorial Using Loop | ||
* @param {*} n | ||
* @returns |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use @returns
JSDoc only if you have to. By default, the IDE will automatically give the right @returns
data type based on the return value.
|
||
for (let index = n; index > 0; index--) { | ||
result *= index; | ||
for (let i = 1; i < n; i += 1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use a better variable naming instead of i
as a non-expert will be confused by the meaning of this variable.
pages/functional/js/fizz-buzz.js
Outdated
try { | ||
const sequence = event.target["sequence"].value; | ||
/** | ||
* Using concept "Const of" and "Function as a parameter" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The usage of for of
is correct, but the function as a parameter
is wrong. If you want to pass the numberGenerator
function as a parameter for the other function (ex: generateFizzBuzz2), don't add the paranthesis at the end of numberGenerator
. See this example below:
/**
* @param {function} callback
*/
function generateFizzBuzz2(callback) {
callback(8);
}
/**
* @param {number} value
*/
const numberGenerator = function (value) {
return value;
}
generateFizzBuzz2(numberGenerator);
pages/functional/js/palindrome.js
Outdated
console.log (array); | ||
console.log (reverseArray); | ||
console.log (JSON.stringify (array)); | ||
console.log (JSON.stringify (reverseArray)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #21 (comment).
pages/functional/js/palindrome.js
Outdated
|
||
for (let index = 0; index < Math.floor(value.length / 2); index++) { | ||
const lastCharacterIndex = value.length - (index + 1); | ||
console.log (isPalindrome ("racecar")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #21 (comment).
pages/functional/js/palindrome.js
Outdated
|
||
return true; | ||
} | ||
console.log (newValue); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #21 (comment).
pages/functional/js/palindrome.js
Outdated
|
||
const firstCharacter = value[index]; | ||
const lastCharacter = value[lastCharacterIndex]; | ||
console.log (isPalindrome2 ("racecar")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #21 (comment).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've created a video to explain my review of this project. Please kindly check it here: https://drive.google.com/file/d/1E8e0qDcXOOCTxxg6hsieYkZjZ1aqBCC3/view?usp=sharing. Thank you.
- [ianriizky/rwid-git](https://github.com/ianriizky/rwid-git) | ||
- [sdesakt/rwid-challenge](https://github.com/sdesakt/rwid-challenge) | ||
- [dianprsty/rwid-git](https://github.com/dianprsty/rwid-git) | ||
- [JayaMustika/rwid-gi](https://github.com/JayaMustika/rwid-git)t |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong github URL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aman sudah diperbaiki Mas
Hello Mas Ian,
I've just finished the assignment focusing on JavaScript's Functional and Object-Oriented Programming paradigms. This PR encompasses all the tasks and sub-tasks related to the assignment.
Key Highlights:
I would highly appreciate it if you could review my code. I'm open to feedback and any suggestions you might have to enhance or correct my implementation.
Looking forward to your constructive feedback.
Best,
Jaya Mustika