Skip to content
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

Francisco Hernandez - Challengues #28

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

franc1sc0sv
Copy link

Solution for the nerdery challengues

Copy link

@fabio248 fabio248 left a comment

Choose a reason for hiding this comment

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

Good job so far, I left some comments for improvements, let me know if you have any questions or doubts.

@@ -31,17 +31,50 @@ node solution.js name1 name2 name3
5. another challenge is: after you solve the challenge using callback style, in another file promisify the callback and solve it again
** give a look to node.js util.promisify, avoid to alter the validate-user.file **
*/
import validateUser from "./validate-user.js";

Choose a reason for hiding this comment

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

We are using CommonJS module, so we should to use require.

Suggested change
import validateUser from "./validate-user.js";
const validateUser = require("./validate-user");

Comment on lines 62 to 67
counter++;
}

if (user) {
succesUsers.push(user);
counter++;

Choose a reason for hiding this comment

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

We can move the counter++ outside of the if statements, and have only one.

Comment on lines 27 to 28
import lastnames from "./lastnames.js";
import firstnames from "./firstnames.js";

Choose a reason for hiding this comment

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

Same comment for imports

Comment on lines 31 to 32
const isValid = Math.random() > 0.5;
if (isValid) {

Choose a reason for hiding this comment

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

To improve readability we should add a blank space before the if statement

Comment on lines 2 to 16
INSTRUCTIONS

1. using async/await API consume products and prices methods
2. don't use .then(), .catch() or .finally() here
3. both, products and prices methods expect a positive integer id
4. use Promise.all() and Promise.allSettled() to consume both methods in parallel
5. to generate the id do the following: invoke Date.now(), and take the last two digits, this will be your id
6. log the results with console.log(), the format is up to you, but it must include id, product and price

Example:
{
id:100,
product:'paper',
price:1
}

Choose a reason for hiding this comment

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

We don't want to delete the instrucctions, so put them back and move your solution under them like other exercises.

Comment on lines 1 to 2
import products from "./products.js";
import prices from "./prices.js";

Choose a reason for hiding this comment

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

Same comment for imports

Comment on lines +48 to +56
function solution() {
const numberOfIds = 5;
const IDs = Array.from({ length: numberOfIds }, generateRandomId);

Promise.all(IDs.map((id) => fetchNames(id))).then((names) => {
names.forEach((name) => {
console.log(`${name}\n`);
});
});

Choose a reason for hiding this comment

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

A good solution here! 🔥

Comment on lines 27 to 48
const allFulfilled = resultsAllSettled.every(
(result) => result.status === "fulfilled"
);

function solution() {
// YOUR SOLUTION GOES HERE
if (allFulfilled) {
const productName = resultsAllSettled[0].value;
const productPrice = resultsAllSettled[1].value;

// You generate your id value here
console.log(`(Promise.AllSettled):`, {
id: ID,
product: productName,
price: productPrice,
});
}

// You use Promise.all() here
if (!allFulfilled) {
console.log(
`Error (Promise.allSettled): ${
resultsAllSettled[0].reason || "Unknown error"
}`
);
}

Choose a reason for hiding this comment

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

We can take the first error approach, where we first have the error validation and then our code without errors. That improves the readability of the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants