Conversation
📝 HackYourFuture auto gradeAssignment Score: 0 / 100 ✅Status: ✅ Passed Test Details |
mo92othman
left a comment
mo92othman
left a comment
There was a problem hiding this comment.
Hi @hannahwn , well done! 👍
Good job refactoring the code in Task 4. You show a good understanding of the DRY principle. Good luck in the next modules!
| if (!searchString) { | ||
| return false; | ||
| } | ||
| let cleanedSearch = searchString.trim().toLowerCase(); | ||
|
|
||
| if (cleanedSearch === "") { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
if (!searchString) {
return false;
}
and
if (cleanedSearch === "") {
return false;
}
Good job, just a small note: both of these are basically doing the same thing! The second one after the trim would be enough!
| let isFound = bookTitle.includes(cleanedSearch); | ||
|
|
||
|
|
||
| return isFound; |
There was a problem hiding this comment.
Nice, this works! But as another option: the .includes() method already returns a boolean (true or false), so there is no need to store it in a variable before returning it.
You can simplify this:
let isFound = bookTitle.includes(cleanedSearch);
return isFound;
To
return bookTitle.includes(cleanedSearch);
| return false; | ||
| } | ||
|
|
||
| let bookTitle = "The fundamentals of JavaScript".toLowerCase(); |
There was a problem hiding this comment.
This variable and the previous one are not reassigned, so const would be more appropriate than let.
| if(format === "MDY"){ | ||
| month = first; | ||
| day = second; | ||
| } | ||
|
|
||
| if (format === "DMY"){ | ||
| day = first; | ||
| month = second; | ||
| } |
There was a problem hiding this comment.
If the format is not MDY or DMY, day and month will be undefined. Consider handling this case.
Example where it won’t work parseDateString("MYD 10-21-1983"); -> the result will be:
{ day: undefined, month: undefined, year: 1983 }
You could add an else block to handle that.
| let parts = dateString.split(" "); | ||
|
|
||
| let format = parts[0]; | ||
| let datePart = parts[1]; | ||
|
|
||
|
|
||
| let numbers = datePart.split("-"); | ||
|
|
||
|
|
||
|
|
||
| let first = Number(numbers[0]); | ||
| let second = Number(numbers[1]); | ||
| let third = Number(numbers[2]); | ||
|
|
||
| let day; | ||
| let month; | ||
| let year = third; |
There was a problem hiding this comment.
You can use const instead of let for variables that are not reassigned.
| export function convertHoursToMinutes(hours) { | ||
| return hours * 60 | ||
| } | ||
| export function convertMinutesToHours(minutes) { | ||
| return minutes / 60 | ||
| } | ||
| export function convertDaysToHours(days){ | ||
| return days * 24 | ||
| } | ||
| export function convertHoursToDays(hours){ | ||
| return hours / 24 | ||
| } | ||
| export function convertMinutesToSeconds(minutes){ | ||
| return minutes * 60 | ||
| } | ||
| export function convertSecondsToMinutes(seconds){ | ||
| return seconds / 60 | ||
| } No newline at end of file |
| console.log("Status: Warm"); | ||
| } else { | ||
| console.log("Status: Hot"); | ||
| function CelsiusToFahrenheit(c){ |
| console.log("Status: Warm"); | ||
| } else { | ||
| console.log("Status: Hot"); | ||
| function CelciusToKelvin(c){ |
| let fahrenheit = CelsiusToFahrenheit(celsius); | ||
| let kelvin = CelciusToKelvin(celsius); | ||
|
|
||
|
|
||
| let feeling = AddDescription(celsius); |
There was a problem hiding this comment.
You can use const instead of let for variables that are not reassigned.
| let name1 = "Amsterdam"; | ||
| let temp1 = 22; | ||
| let wind1 = 15; | ||
| showWeather(name1, temp1, wind1); | ||
|
|
||
|
|
||
| let name2 = "Berlin"; | ||
| let temp2 = 15; | ||
| let wind2 = 20; | ||
| showWeather(name2, temp2, wind2); | ||
|
|
||
|
|
||
| let name3 = "Copenhagen"; | ||
| let temp3 = -5; | ||
| let wind3 = 25; |
There was a problem hiding this comment.
Nice! we could also further reduce repetition, you could group the city data into an array of objects and iterate over it. This would make adding new cities even easier.
const cities = [
{ name: 'Amsterdam', tempCelsius: 22, windSpeed: 15 },
{ name: 'Berlin', tempCelsius: 15, windSpeed: 20 },
{ name: 'Copenhagen', tempCelsius: -5, windSpeed: 25 },
{ name: 'Dublin', tempCelsius: 8, windSpeed: 10 },
{ name: 'Edinburgh', tempCelsius: 12, windSpeed: 18 },
];Then call the functions on these in different ways. One of them is through a loop, which is a future topic :)
mo92othman
added
Reviewed
2 participants
Functions and Refactoring code