- 
                Notifications
    You must be signed in to change notification settings 
- Fork 16
Develop #11
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?
Develop #11
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|  | @@ -5,7 +5,20 @@ | |
| */ | ||
| function generateFibonacciUsingLoop(sequence) { | ||
| // write your code here | ||
| return []; | ||
| let fibonacci = [0, 1]; | ||
|  | ||
| if (sequence == 1) { | ||
| fibonacci.pop(); | ||
| return fibonacci; | ||
| 
      Comment on lines
    
      +11
     to 
      +12
    
   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please set a line break between each part of your code. For example: fibonacci.pop();
return fibonacci;So the other team will easily read your code. | ||
| } else if (sequence == 2) { | ||
| return fibonacci; | ||
| } | ||
| 
      Comment on lines
    
      +8
     to 
      +15
    
   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you have  let fibonacci = [0, 1];
if (sequence == 2) {
  return fibonacci;
}
if (sequence == 1) {
  fibonacci.pop();
  return fibonacci;
} | ||
|  | ||
| for (let index = 2; index < sequence; index++) { | ||
| fibonacci.push(fibonacci[index - 1] + fibonacci[index - 2]); | ||
| } | ||
|  | ||
| return fibonacci; | ||
| } | ||
|  | ||
| /** | ||
|  | @@ -15,7 +28,19 @@ function generateFibonacciUsingLoop(sequence) { | |
| */ | ||
| function generateFibonacciUsingRecursive(sequence) { | ||
| // write your code here | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my review at #11 (comment). | ||
| return []; | ||
| let array = []; | ||
|  | ||
| if (sequence == 1) { | ||
| array.push(0); | ||
| } else if (sequence == 2) { | ||
| array.push(0, 1); | ||
| } else { | ||
| array = generateFibonacciUsingRecursive(sequence - 1); | ||
|  | ||
| let len = array.length; | ||
| array.push(array[len - 1] + array[len - 2]); | ||
| } | ||
| return array; | ||
| 
      Comment on lines
    
      +42
     to 
      +43
    
   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my review at #11 (comment). | ||
| } | ||
|  | ||
| document.getElementById("form").addEventListener("submit", function (event) { | ||
|  | ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -1,11 +1,14 @@ | ||
| import { parseString } from "../../js/helper.js"; | ||
| /** | ||
| * Determine whether the given value is a palindrome or not using reverse way. | ||
| * | ||
| * @param {string} value | ||
| */ | ||
| function isPalindromeUsingReverse(value) { | ||
| // write your code here | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my review at #11 (comment). | ||
| return true; | ||
| value = value.replace(/[^a-zA-Z]/g, "").toLowerCase(); | ||
| let reverseValuestr = value.split("").reverse().join(""); | ||
| return value == reverseValuestr; | ||
| 
      Comment on lines
    
      +9
     to 
      +11
    
   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my review at #11 (comment). | ||
| } | ||
|  | ||
| /** | ||
|  | @@ -15,6 +18,13 @@ function isPalindromeUsingReverse(value) { | |
| */ | ||
| function isPalindromeUsingLoop(value) { | ||
| // write your code here | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my review at #11 (comment). | ||
| value = value.replace(/[^a-zA-Z]/g, "").toLowerCase(); | ||
| let valueLen = value.length; | ||
|  | ||
| for (let index = 1; index < valueLen / 2; index++) { | ||
| if (value[index] == value[valueLen - index - 1]) { | ||
| } | ||
| 
      Comment on lines
    
      +25
     to 
      +26
    
   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the purpose of this code? | ||
| } | ||
| return true; | ||
| 
      Comment on lines
    
      +21
     to 
      28
    
   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my review at #11 (comment). | ||
| } | ||
|  | ||
|  | @@ -26,7 +36,15 @@ function isPalindromeUsingLoop(value) { | |
| */ | ||
| function isPalindromeUsingRecursive(value, index = 0) { | ||
| // write your code here | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my review at #11 (comment). | ||
| return true; | ||
|  | ||
| value = value.replace(/[^a-zA-Z]/g, "").toLowerCase(); | ||
| if (index >= Math.floor(value.length / 2)) { | ||
| return true; | ||
| } | ||
| if (value[index] !== value[value.length - 1 - index]) { | ||
| return false; | ||
| } | ||
| return isPalindromeUsingRecursive(value, index + 1); | ||
| 
      Comment on lines
    
      +40
     to 
      +47
    
   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my review at #11 (comment). | ||
| } | ||
|  | ||
| document.getElementById("form").addEventListener("submit", function (event) { | ||
|  | ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| import { Member } from "./Member.js"; | ||
|  | ||
| function generateMember12() { | ||
| const member = new Member( | ||
| "Habib Al Fitrah", | ||
| 12, | ||
| "Makassar, Sulawesi Selatan", | ||
| "07", | ||
| "https://github.com/habibalftrh", | ||
| ); | ||
|  | ||
| member.generateTrElement(); | ||
| } | ||
|  | ||
| export { generateMember12 }; | 
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|  | @@ -12,6 +12,25 @@ export class Factorial { | |
| } | ||
|  | ||
| // write your code here | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my review at #11 (comment). | ||
| calculateFactorialUsingLoop() { | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "Factorial" part on this method name is unnecessary, as this class itself already named "Factorial". | ||
| if (this.n === 0 || this.n === 1) { | ||
| return 1; | ||
| } | ||
|  | ||
| let factorial = 1; | ||
| for (let index = this.n; index > 0; index--) { | ||
| factorial *= index; | ||
| } | ||
| return factorial; | ||
| 
      Comment on lines
    
      +20
     to 
      +24
    
   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my review at #11 (comment). | ||
| } | ||
|  | ||
| calculateFactorialUsingRecursive(n = this.n) { | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my review at #11 (comment). | ||
| if (n < 2) { | ||
| return 1; | ||
| } | ||
|  | ||
| return n * this.calculateFactorialUsingRecursive(n - 1); | ||
| } | ||
| } | ||
|  | ||
| document.getElementById("form").addEventListener("submit", function (event) { | ||
|  | @@ -21,7 +40,17 @@ document.getElementById("form").addEventListener("submit", function (event) { | |
| const n = event.target["n"].value; | ||
| const method = event.target["method"].value; | ||
|  | ||
| const result = new Factorial(n); | ||
| const resultObj = new Factorial(n); | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Give a proper name for this variable.  | ||
|  | ||
| let result = 0; | ||
|  | ||
| if (method == "loop") { | ||
| result = resultObj.calculateFactorialUsingLoop(); | ||
| } else if (method == "recursive") { | ||
| result = resultObj.calculateFactorialUsingRecursive(); | ||
| } else { | ||
| throw new Error("Method must be loop or recursive."); | ||
| } | ||
| 
      Comment on lines
    
      +43
     to 
      +53
    
   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my review at #11 (comment). | ||
|  | ||
| document.getElementById("result").textContent = result; | ||
| } catch (error) { | ||
|  | ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|  | @@ -12,6 +12,35 @@ export class Fibonacci { | |
| } | ||
|  | ||
| // write your code here | ||
| generateFibonacciUsingLoop() { | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my review at #11 (comment). | ||
| let fibonacci = [0, 1]; | ||
| if (this.sequence == 1) { | ||
| return fibonacci.pop(); | ||
| } else if (this.sequence == 2) { | ||
| return fibonacci; | ||
| } | ||
|  | ||
| for (let index = 2; index < this.sequence; index++) { | ||
| fibonacci.push(fibonacci[index - 1] + fibonacci[index - 2]); | ||
| } | ||
| return fibonacci; | ||
| 
      Comment on lines
    
      +16
     to 
      +26
    
   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my review at #11 (comment). | ||
| } | ||
|  | ||
| generateFibonacciUsingRecursive(sequence = this.sequence) { | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my review at #11 (comment). | ||
| let array = []; | ||
|  | ||
| if (sequence === 1) { | ||
| array.push(0); | ||
| } else if (sequence === 2) { | ||
| array.push(0, 1); | ||
| } else { | ||
| array = this.generateFibonacciUsingRecursive(sequence - 1); | ||
|  | ||
| let len = array.length; | ||
| array.push(array[len - 1] + array[len - 2]); | ||
| } | ||
| return array; | ||
| 
      Comment on lines
    
      +41
     to 
      +42
    
   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my review at #11 (comment). | ||
| } | ||
| } | ||
|  | ||
| document.getElementById("form").addEventListener("submit", function (event) { | ||
|  | @@ -21,8 +50,15 @@ document.getElementById("form").addEventListener("submit", function (event) { | |
| const sequence = event.target["sequence"].value; | ||
| const method = event.target["method"].value; | ||
|  | ||
| const result = new Fibonacci(sequence); | ||
|  | ||
| let result = []; | ||
| const resultObj = new Fibonacci(sequence); | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my review at #11 (comment). | ||
| if (method === "loop") { | ||
| result = resultObj.generateFibonacciUsingLoop(sequence); | ||
| } else if (method === "recursive") { | ||
| result = resultObj.generateFibonacciUsingRecursive(sequence); | ||
| } else { | ||
| throw new Error("Method must be loop or recursive."); | ||
| } | ||
| 
      Comment on lines
    
      +54
     to 
      +61
    
   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should keep our main code clear. So I think this "method" checking can be organized into one method on the Fibonacci class. | ||
| document.getElementById("result").textContent = JSON.stringify(result); | ||
| 
      Comment on lines
    
      +53
     to 
      62
    
   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my review at #11 (comment). | ||
| } catch (error) { | ||
| alert(error.message); | ||
|  | ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|  | @@ -11,19 +11,18 @@ export class FizzBuzz { | |
| this.sequence = parseNumber(sequence); | ||
| } | ||
|  | ||
| generate() { | ||
| generateFizzBuzz(sequence = this.sequence) { | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my review at #11 (comment). | ||
| let result = []; | ||
|  | ||
| for (let index = 1; index <= this.sequence; index++) { | ||
| if (index % 4 === 0 || index % 7 === 0) { | ||
| for (let index = 1; index <= sequence; index++) { | ||
| if (index % 4 == 0 || index % 7 == 0) { | ||
| result.push("fizz buzz"); | ||
| } else if (index % 2 === 1) { | ||
| } else if (index % 2 == 1) { | ||
| result.push("fizz"); | ||
| } else if (index % 2 === 0) { | ||
| } else if (index % 2 == 0) { | ||
| result.push("buzz"); | ||
| } | ||
| } | ||
|  | ||
| return result; | ||
| } | ||
| 
      Comment on lines
    
      +14
     to 
      27
    
   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 
 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This class is already done, so there is no changes needed here. | ||
| } | ||
|  | ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| import { parseString } from "../../js/helper.js"; | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my review at #11 (comment). | ||
| export class Palindrome { | ||
| /** @type {string} */ | ||
| value; | ||
|  | @@ -10,6 +11,35 @@ export class Palindrome { | |
| } | ||
|  | ||
| // write your code here | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my review at #11 (comment). | ||
| isPalindromeUsingReverse(value = this.value) { | ||
| value = value.replace(/[^a-zA-Z]/g, "").toLowerCase(); | ||
| let reverseValuestr = value.split("").reverse().join(""); | ||
| return value == reverseValuestr; | ||
| 
      Comment on lines
    
      +15
     to 
      +17
    
   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my review at #11 (comment). | ||
| } | ||
|  | ||
| isPalindromeUsingLoop(value = this.value) { | ||
| value = value.replace(/[^a-zA-Z]/g, "").toLowerCase(); | ||
| let valueLen = value.length; | ||
|  | ||
| for (let index = 0; index < Math.floor(valueLen / 2); index++) { | ||
| if (value[index] !== value[valueLen - index - 1]) { | ||
| return false; | ||
| } | ||
| } | ||
| return true; | ||
| 
      Comment on lines
    
      +21
     to 
      +29
    
   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my review at #11 (comment). | ||
| } | ||
|  | ||
| isPalindromeUsingRecursive(value = this.value, index = 0) { | ||
| value = value.replace(/[^a-zA-Z]/g, "").toLowerCase(); | ||
| if (index >= Math.floor(value.length / 2)) { | ||
| return true; | ||
| } | ||
| if (value[index] !== value[value.length - 1 - index]) { | ||
| return false; | ||
| } | ||
| 
      Comment on lines
    
      +33
     to 
      +39
    
   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my review at #11 (comment). | ||
|  | ||
| return this.isPalindromeUsingRecursive(value, index + 1); | ||
| } | ||
| } | ||
|  | ||
| document.getElementById("form").addEventListener("submit", function (event) { | ||
|  | @@ -19,7 +49,14 @@ document.getElementById("form").addEventListener("submit", function (event) { | |
| const word = event.target["word"].value; | ||
| const method = event.target["method"].value; | ||
|  | ||
| const result = new Palindrome(word); | ||
| let result; | ||
| if (method === "reverse") { | ||
| result = new Palindrome(word).isPalindromeUsingReverse(word); | ||
| } else if (method === "loop") { | ||
| result = new Palindrome(word).isPalindromeUsingLoop(word); | ||
| } else if (method === "recursive") { | ||
| result = new Palindrome(word).isPalindromeUsingRecursive(word); | ||
| } | ||
| 
      Comment on lines
    
      +52
     to 
      +59
    
   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my review at #11 (comment). | ||
|  | ||
| document.getElementById("result").textContent = result; | ||
| } catch (error) { | ||
|  | ||
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.
Remove this unnecessary comment.