@Ahmadhassan0
Submitted
You can look at my code and If you have any type of feedback on how can I improve my code quality I'll be very happy to look at your feedback😊
@JJacobPR
@Ahmadhassan0
Submitted
You can look at my code and If you have any type of feedback on how can I improve my code quality I'll be very happy to look at your feedback😊
@JJacobPR
Posted
Hey!
This is a very short app so it is difficult to spot any major problems with your code but what I would definitely suggest as it is a standard, is to remove any logs and stuff for development before let's call it pushing to production :)
Marked as helpful
@cancomertpay
Submitted
Greetings everyone! Please share your feedback and suggestions for more optimized solutions. Thank you!
@JJacobPR
Posted
Hey,
Nice solution you have here but I found a problem. The leap year is not considered and you can check 29/02 for each year.
My suggestion is that you can create Date object based on year (it will know if it is leap year) and validate properly!
Good Luck!
Marked as helpful
@Maksio07
Submitted
Hi everyone, there is my solution for Advice Generator App. Thanks for this challenge I learnt how to use API in my projects. I'll be happy to hear your advices and comments for my solution.
@JJacobPR
Posted
Hey Maksym,
Good job implementing this task! I have two things you might consider improving:
Good Luck!
Marked as helpful
@Fejiro001
Submitted
Hi this is Fejiro, and this is my solution to the Advice Generator
Any suggestions on how I can improve are welcomed
@JJacobPR
Posted
Hey,
I love your dice animation. What I would improve is the random data generation. As this API tends to get a little bit sloppy, you can use the documentation to find route for a specific advice and generate the ids yourself in frontend to ensure that user would get random advice. I know that normally it would be backend's fault but here we are just practicing, aren't we 😀
@opolis8
Submitted
My code is not perfect, I had a hard time how to compute the age if you had your birthday this year. but on the brighter side, i learned so much on this project as I practiced how to call a function inside a function and use it many times.
Any comments and suggestion for me to improve my coding is much appreciated
@JJacobPR
Posted
Hello,
As I've check that you eventually didn't implement the "this year birthday case", let me give you a hint.
If I were you, I would add additional condition that checks if input year is current year and if it is, additional checks for month and day, since then the provided day cannot be bigger than current day and the same with month.
If it was not the problem and you had had this idea but simply couldn't implement it, just ask in the reply and I'll send you a snippet. You can also just check in my code of course :)
@Julie-Gz
Submitted
Hi Frontend mentor community. This was a wonderful challenge. I'm not sure if the app is 100% accurate, so I would be grateful if someone tested it and gave me feedback. Any other feedback is also appreciated.
Thank you :)
@JJacobPR
Posted
Hey,
I did a little bit of testing as you suggested and things that (in my view) should be enhanced:
Hope, this feedback is what you wanted :) Should you have more questions, just ask :)
@aboobaqr
Submitted
If you have the time to check please let me know where and how I can improve, I wrote over 230 lines of JavaScript. 😩 I really think it's too much for this project.
@JJacobPR
Posted
Hey,
as you are probably beginner, don't worry that your code is that long, you will improve as you practice. But here are 2 major tips for your programming habits not only js but overall.
if (monthInput.value == 2 && dayInput.value > 28)
When you can, try to use other methods. Here recommended are inbuilt Date functions that you should explore. This is my example of validating day by inbuilt functions:
const dayValidator = () => {
//Checking if day is okay in current year case
if (year === currentYear && month === new Date().getMonth() + 1 && day > new Date().getDate()) return false;
//Checking if day is okay based on particular month length
if (day >= 1 && day <= new Date(year, month, 0).getDate()) return true;
else return false;
};
Marked as helpful
"This is the solution to the 'age calculator' challenge, it's practically finished, although I have the following issue.
"Depending on the months, an error is generated if the day is not valid, but when entering the day first and then specifying the month, the validation is not performed, which results in February 31st being considered a valid day."
@JJacobPR
Posted
Hola amigo,
I think that I found the problem with your code :) Let's take a look at this snippet
switch(e.target.id) {
case 'day':
validateField('day', 'label-day','error-text-day' , e.target, maxDays, 2);
break;
case 'month':
validateField('month', 'label-month','error-text-month', e.target, 12, 2);
break;
case 'year':
validateField('year', 'label-year','error-text-year', e.target, 2024, 4);
break;
}
Here you are checking field based on event.target.id. But only one field each time, only on day, month or year. So when you put day 31, it checks on your default case:
} else {
maxDays = 31;
}
But when you put in the month, since you use switch case, it doesn't recheck the day, only the month. The same goes for year. The easiest solution would be to check all fields each time. It is a small app so it won't be a problem.
Another thing, I would consider changing, is the way you check day amount per month. Remember the February has 28 or 29 days. I know it is a bit more of a challenge but that's what these are for.
Hope that I helped and good luck!
Marked as helpful
@ParvizAzeroglu
Submitted
Finding out how many days February has proved to be a bit challenging for me, but I never really needed to know anyway, XD! I just use 31 days as a reference for all the months
@JJacobPR
Posted
Hey, maybe this snippet will help you. It checks if day exists by creating Date object based on year and month input and extracting day count of needed month.
if (props.day >= 1 && props.day <= new Date(props.year, props.month, 0).getDate()) return true;
else return false;
Marked as helpful