Skip to content

Volodymyr-w1-UsingAPIs #51

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .test-summary/TEST_SUMMARY.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
## Test Summary

**Mentors**: For more information on how to review homework assignments, please refer to the [Review Guide](https://github.com/HackYourFuture/mentors/blob/main/assignment-support/review-guide.md).

### 3-UsingAPIs - Week1

| Exercise | Passed | Failed | ESLint |

Choose a reason for hiding this comment

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

@Volodymyr-coder Great job completing week 1 assignment. Overall, this is an impressive solution, and I can see that you put a lot of effort into it. Keep up the great work, and don't hesitate to ask if you have any questions or need further clarification on any of the points above. I will Approve it now.

|-----------------------|--------|--------|--------|
| ex1-johnWho | 9 | - | ✕ |
| ex2-checkDoubleDigits | 11 | - | ✕ |
| ex3-rollDie | 7 | - | ✕ |
| ex4-pokerDiceAll | 7 | - | ✕ |
| ex5-pokerDiceChain | 5 | - | ✕ |
32 changes: 19 additions & 13 deletions 3-UsingAPIs/Week1/assignment/ex1-johnWho.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,28 @@ Rewrite this function, but replace the callback syntax with the Promise syntax:
- If the Promise `rejects`, pass an error as the argument to reject with: "You
didn't pass in a first name!"
------------------------------------------------------------------------------*/
// TODO see above
export const getAnonName = (firstName, callback) => {
setTimeout(() => {
if (!firstName) {
callback(new Error("You didn't pass in a first name!"));
return;
}

const fullName = `${firstName} Doe`;

callback(fullName);
}, 1000);
export const getAnonName = (firstName) => {

Choose a reason for hiding this comment

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

Great job with your solution to this first exercise (using Promise syntax instead of callbacks). The logic inside the setTimeout function is clear and follows the expected flow.

return new Promise((resolve, reject) => {
setTimeout(() => {
if (firstName) {

Choose a reason for hiding this comment

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

This looks great but a couple of things we can improve are:

  1. prevent redundant return statement:
    The return statement after resolve(fullName) is unnecessary because the resolve call ends the execution of the promise immediately.

  2. "Fail first" or "Fail Fast" approach:
    It is sometimes good to identify and report errors as early as possible in our code or development cycle. The benefit is that, it helps prevent bugs from propagating and becoming more difficult and costly to fix later on.

Original solution:

...
setTimeout(() => {
      if (firstName) {
        const fullName = `${firstName} Doe`;
        resolve(fullName);
        return;
      } else {
        reject(new Error("You didn't pass in a first name!"));
      }
    }, 1000);
...

Alternative: Here, we handle the case where the promise reject first. In addition, since, we use return statement in the if () {} statement, we don't have to add else {} block to our code.

...
setTimeout(() => {
  if (!firstName) {
    reject(new Error("You didn't pass in a first name!"));
    return;
  }
  resolve(`${firstName} Doe`);
}, 1000);
...

const fullName = `${firstName} Doe`;
resolve(fullName);
return;
} else {
reject(new Error("You didn't pass in a first name!"));
}
}, 1000);
});
};

function main() {
getAnonName('John', console.log);
getAnonName('John')
.then((fullName) => {
console.log(fullName);
})
.catch((error) => {
console.error(error.message);
});
}

// ! Do not change or remove the code below
Expand Down
10 changes: 8 additions & 2 deletions 3-UsingAPIs/Week1/assignment/ex2-checkDoubleDigits.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,14 @@ Complete the function called `checkDoubleDigits` such that:
"Expected a double digit number but got `number`", where `number` is the
number that was passed as an argument.
------------------------------------------------------------------------------*/
export function checkDoubleDigits(/* TODO add parameter(s) here */) {
// TODO complete this function
export function checkDoubleDigits(number) {

Choose a reason for hiding this comment

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

Well done, this is well written. Kudos.

return new Promise((resolve, reject) => {
if (number >= 10 && number <= 99) {
resolve('This is a double digit number!');
} else {
reject(new Error(`Expected a double digit number but got ${number}`));
}
});
}

function main() {
Expand Down
75 changes: 36 additions & 39 deletions 3-UsingAPIs/Week1/assignment/ex3-rollDie.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,53 +10,50 @@ Full description at: https://github.com/HackYourFuture/Assignments/tree/main/3-U
explanation? Add your answer as a comment to be bottom of the file.
------------------------------------------------------------------------------*/

// TODO Remove callback and return a promise
export function rollDie(callback) {
// Compute a random number of rolls (3-10) that the die MUST complete
const randomRollsToDo = Math.floor(Math.random() * 8) + 3;
console.log(`Die scheduled for ${randomRollsToDo} rolls...`);

const rollOnce = (roll) => {
// Compute a random die value for the current roll
const value = Math.floor(Math.random() * 6) + 1;
console.log(`Die value is now: ${value}`);

// Use callback to notify that the die rolled off the table after 6 rolls
if (roll > 6) {
// TODO replace "error" callback
callback(new Error('Oops... Die rolled off the table.'));
}

// Use callback to communicate the final die value once finished rolling
if (roll === randomRollsToDo) {
// TODO replace "success" callback
callback(null, value);
}

// Schedule the next roll todo until no more rolls to do
if (roll < randomRollsToDo) {
setTimeout(() => rollOnce(roll + 1), 500);
}
};

// Start the initial roll
rollOnce(1);
import { error } from '@actions/core';

export function rollDie() {
return new Promise((resolve, reject) => {
const randomRollsToDo = Math.floor(Math.random() * 8) + 3;
console.log(`Die scheduled for ${randomRollsToDo} rolls...`);
// Compute a random number of rolls (3-10) that the die MUST complete
const rollOnce = (roll) => {
// Compute a random die value for the current roll
const value = Math.floor(Math.random() * 6) + 1;
console.log(`Die value is now: ${value}`);
// Use callback to notify that the die rolled off the table after 6 rolls
if (roll > 6) {
reject(new Error('Oops... Die rolled off the table.'));
}
// Use callback to communicate the final die value once finished rolling
if (roll === randomRollsToDo) {
resolve(value);
}
// Schedule the next roll todo until no more rolls to do
if (roll < randomRollsToDo) {
setTimeout(() => rollOnce(roll + 1), 500);
}
};

// Start the initial roll
rollOnce(1);
});
}

function main() {
// TODO Refactor to use promise
rollDie((error, value) => {
if (error !== null) {
console.log(error.message);
} else {
rollDie()
.then((value) => {
console.log(`Success! Die settled on ${value}.`);
}
});
})
.catch((err) => {
console.log(err.message);
});
}

// ! Do not change or remove the code below
if (process.env.NODE_ENV !== 'test') {
main();
}

// TODO Replace this comment by your explanation that was asked for in the assignment description.
// Promises make error handling easier because errors can be "caught" using .catch() rather.
// The function now works correctly and everything is allowed and rejected correctly and errors and successes are handled correctly.
7 changes: 4 additions & 3 deletions 3-UsingAPIs/Week1/assignment/ex4-pokerDiceAll.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ exercise file.
import { rollDie } from '../../helpers/pokerDiceRoller.js';

export function rollDice() {
// TODO Refactor this function
const dice = [1, 2, 3, 4, 5];
return rollDie(1);

return Promise.all(dice.map((die) => rollDie(die)));
}

function main() {
Expand All @@ -43,4 +43,5 @@ if (process.env.NODE_ENV !== 'test') {
main();
}

// TODO Replace this comment by your explanation that was asked for in the assignment description.
// Promise.all() accepts an array of promises, waits for them to be fulfilled, and returns a promise. In our situation we wait when all promise will be done.
// In the event that at least one of the promises is rejected, the returned promise will enter the rejected state, and its value will be an error.
13 changes: 12 additions & 1 deletion 3-UsingAPIs/Week1/assignment/ex5-pokerDiceChain.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,23 @@ import { rollDie } from '../../helpers/pokerDiceRoller.js';
export function rollDice() {
const results = [];

// TODO: expand the chain to include five dice
return rollDie(1)
.then((value) => {
results.push(value);
return rollDie(2);
})
.then((value) => {

Choose a reason for hiding this comment

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

Great job, this works as expected.

But if you noticed, we are repeating these two codes from line 22 to 39:

 results.push(value);
 return rollDie(4);

We can prevent this with a Software engineering principle called "DRY Principle". "Don't Repeat Yourself" (DRY) is a software development principle that helps us to avoid duplicating code. If you aren't aware of it, you can Google Search.

results.push(value);
return rollDie(3);
})
.then((value) => {
results.push(value);
return rollDie(4);
})
.then((value) => {
results.push(value);
return rollDie(5);
})
.then((value) => {
results.push(value);
return results;
Expand Down
21 changes: 21 additions & 0 deletions 3-UsingAPIs/Week1/test-reports/ex1-johnWho.report.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
*** Unit Test Error Report ***

PASS .dist/3-UsingAPIs/Week1/unit-tests/ex1-johnWho.test.js
api-wk1-ex1-johnWho
✅ should exist and be executable (3 ms)
✅ should have all TODO comments removed (1 ms)
✅ `getAnonName` should not contain unneeded console.log calls (1 ms)
✅ should call `new Promise()` (1 ms)
✅ should take a single argument
✅ `resolve()` should be called with a one argument (1 ms)
✅ `reject()` should be called with a one argument (1 ms)
✅ should resolve when called with a string argument (2 ms)
✅ should reject with an Error object when called without an argument (1 ms)

Test Suites: 1 passed, 1 total
Tests: 9 passed, 9 total
Snapshots: 0 total
Time: 1.139 s
Ran all test suites matching /C:\\Users\\vlady\\Desktop\\HYF\\Assignments-cohort51\\.dist\\3-UsingAPIs\\Week1\\unit-tests\\ex1-johnWho.test.js/i.
No linting errors detected.
No spelling errors detected.
23 changes: 23 additions & 0 deletions 3-UsingAPIs/Week1/test-reports/ex2-checkDoubleDigits.report.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
*** Unit Test Error Report ***

PASS .dist/3-UsingAPIs/Week1/unit-tests/ex2-checkDoubleDigits.test.js
api-wk1-ex2-checkDoubleDigits
✅ should exist and be executable (3 ms)
✅ should have all TODO comments removed
✅ `checkDoubleDigits` should not contain unneeded console.log calls (1 ms)
✅ should call new Promise()
✅ `resolve()` should be called with a one argument
✅ `reject()` should be called with a one argument
✅ should be a function that takes a single argument
✅ (9) should return a rejected promise with an Error object (1 ms)
✅ (10) should return a promise that resolves to "This is a double digit number!" (1 ms)
✅ (99) should return a promise that resolves to "This is a double digit number!" (1 ms)
✅ (100) should return a rejected promise with an Error object (1 ms)

Test Suites: 1 passed, 1 total
Tests: 11 passed, 11 total
Snapshots: 0 total
Time: 1.689 s
Ran all test suites matching /C:\\Users\\vlady\\Desktop\\HYF\\Assignments-cohort51\\.dist\\3-UsingAPIs\\Week1\\unit-tests\\ex2-checkDoubleDigits.test.js/i.
No linting errors detected.
No spelling errors detected.
19 changes: 19 additions & 0 deletions 3-UsingAPIs/Week1/test-reports/ex3-rollDie.report.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
*** Unit Test Error Report ***

PASS .dist/3-UsingAPIs/Week1/unit-tests/ex3-rollDie.test.js
api-wk1-ex3-rollDie
✅ should exist and be executable (2 ms)
✅ should have all TODO comments removed
✅ should call `new Promise()` (1 ms)
✅ `resolve()` should be called with a one argument
✅ `reject()` should be called with a one argument
✅ should resolve when the die settles successfully (1 ms)
✅ should reject with an Error when the die rolls off the table (4 ms)

Test Suites: 1 passed, 1 total
Tests: 7 passed, 7 total
Snapshots: 0 total
Time: 1.198 s
Ran all test suites matching /C:\\Users\\vlady\\Desktop\\HYF\\Assignments-cohort51\\.dist\\3-UsingAPIs\\Week1\\unit-tests\\ex3-rollDie.test.js/i.
No linting errors detected.
No spelling errors detected.
19 changes: 19 additions & 0 deletions 3-UsingAPIs/Week1/test-reports/ex4-pokerDiceAll.report.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
*** Unit Test Error Report ***

PASS .dist/3-UsingAPIs/Week1/unit-tests/ex4-pokerDiceAll.test.js
api-wk1-ex4-pokerDiceAll
✅ should exist and be executable (2 ms)
✅ should have all TODO comments removed (1 ms)
✅ `rollDice` should not contain unneeded console.log calls (1 ms)
✅ should use `dice.map()`
✅ should use `Promise.all()` (1 ms)
✅ should resolve when all dice settle successfully (11 ms)
✅ should reject with an Error when a die rolls off the table (44 ms)

Test Suites: 1 passed, 1 total
Tests: 7 passed, 7 total
Snapshots: 0 total
Time: 0.963 s, estimated 2 s
Ran all test suites matching /C:\\Users\\vlady\\Desktop\\HYF\\Assignments-cohort51\\.dist\\3-UsingAPIs\\Week1\\unit-tests\\ex4-pokerDiceAll.test.js/i.
No linting errors detected.
No spelling errors detected.
17 changes: 17 additions & 0 deletions 3-UsingAPIs/Week1/test-reports/ex5-pokerDiceChain.report.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
*** Unit Test Error Report ***

PASS .dist/3-UsingAPIs/Week1/unit-tests/ex5-pokerDiceChain.test.js
api-wk1-ex5-pokerDiceChain
✅ should exist and be executable (2 ms)
✅ should have all TODO comments removed (1 ms)
✅ `rollDice` should not contain unneeded console.log calls (1 ms)
✅ should resolve when all dice settle successfully (18 ms)
✅ should reject with an Error when a die rolls off the table (91 ms)

Test Suites: 1 passed, 1 total
Tests: 5 passed, 5 total
Snapshots: 0 total
Time: 1.097 s
Ran all test suites matching /C:\\Users\\vlady\\Desktop\\HYF\\Assignments-cohort51\\.dist\\3-UsingAPIs\\Week1\\unit-tests\\ex5-pokerDiceChain.test.js/i.
No linting errors detected.
No spelling errors detected.