Skip to content

Conversation

HoussamLh
Copy link

Learners, PR Template

Self checklist

  • I have committed my files one by one, on purpose, and for a reason
  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • I have tested my changes
  • My changes follow the style guide
  • My changes meet the requirements of this task

Changelist

Title:
Complete JS and Unit Testing Challenges

Description:
This PR merges all completed katas and JS exercises, including:

Factorial, Car Sales, Average, Largest Number, Remove Vowels, Roman Numerals, Password Verifier, String Calculator

Cowsay CLI, Weather App, Dog Photo Gallery

  • All Jest tests pass for katas
  • Browser projects work with expected functionality
  • Modular, tested, and ready for review

Question

  1. Are there ways to make my test-writing and implementation more efficient?
  2. Are there any best practices I should follow when structuring my functions and test files?
  3. How can I improve the readability and maintainability of my code for these katas?

- Accepts command-line arguments
- Displays a speech bubble around the input
- Shows a simple ASCII cow below the bubble
- Defaults to "Moo!" if no input is provided
- Uses readline to prompt the user for input
- Displays a speech bubble around the input
- Shows a simple ASCII cow below the bubble
- Defaults to "Moo!" if no input is provided
…ather app.

- Add app.js with placeholders for OpenWeather and Unsplash API keys.
- Fetch weather data from OpenWeather and display temperature and description.
- Fetch images from Unsplash based on weather description and display main image.
- Add clickable thumbnails that update main photo and photographer credit.
- Implement search form to update weather and images for user-entered city.
- Add try/catch to handle API errors and display error message in UI.
- Insert real OpenWeather and Unsplash API keys into app.js.
- Added heading, "Add Dog" button, "Clear Gallery" button
- Added empty <ul> for dog images
- Linked external app.js script
- Basic CSS for layout, buttons, and gallery styling
- Added async function to fetch from https://dog.ceo/api/breeds/image/random
- Created <li> and <img> for each new dog image
- Appended dog images to gallery dynamically
- Added error handling with alert and console error
- Added event listener to clear-gallery button
- Clears all <li> dog images from the gallery
- Keeps app simple and easy to reset
- Add some css style
- return 0 when input is an empty string
- handle single number input
- support summing two numbers
- allow summing an unknown amount of numbers
- ignore numbers greater than 1000
- throw error when negative numbers are passed
- Reject password if it is shorter than 8 characters
- Reject password if it is null
- Reject password if it has no uppercase letter
- Reject password if it has no number
- Return 'Password accepted' when all rules are satisfied
- implement simple mapping for numbers 1 to 10
- add first test for 1 → I
- add tests for 5 → V and 10 → X
- implement basic old Roman numeral mapping for numbers 1–10
- add first test for 1 → I
- add tests for 4 → IIII, 5 → V, 8 → VIII, and 10 → X
- implement sales function to calculate total price per make
- add test for sample carsSold array
- implement factorial function using iterative approach
- implement average function to calculate average of numbers only
- add test for mixed array of numbers and strings
- implement getLargestNumber function to find largest number in array
- add test to check largest number and ensure original array is unchanged
- implement removeVowels function to remove vowels from a single word
- implement removeVowelsFromWords to remove vowels from all words in an array
- add test to check array of words against expected vowel-free output
@HoussamLh HoussamLh added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Aug 20, 2025
@sambabib sambabib added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Aug 23, 2025

// 3. Make a cow that takes a string
// 3. Make a simple cow
function makeCow() {

Choose a reason for hiding this comment

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

curious. how did you get the image of the cow?

// 2. Make supplies for our speech bubble

// 3. Make a cow that takes a string
function makeBubble(text) {

Choose a reason for hiding this comment

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

the implementation of the bubble is clever.

// ====================

// Your working API keys
const WEATHER_API_KEY = "13940029dbb0ac27fa8e76f770798f71";

Choose a reason for hiding this comment

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

it is generally not good practice to commit secrets/keys to source control. I'm not sure if this was the recommended approach but it is something to note moving forward. you should look up environment variables. In a situation where you have environment variables set, I would need to create my own keys to test your application, so leaving your keys here makes testing/iterating quicker. but this is something to always keep in mind as you go.

const UNSPLASH_ACCESS_KEY = "1EkJLQ64exkT3BxWkvtunY6nGsyoUpmmVOIycUfobC4";

// DOM elements
const photoEl = document.getElementById("photo");

Choose a reason for hiding this comment

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

while I can tell what this variable name does by just looking at it, you should look to have more descriptive variable names. so, photoElement or photoContainer reads better than just photoEl. searchForm and searchInput are very clear and their use case can be inferred at first glance, so keep the clarity consistent.

async function getWeather(city = "London") {
const url = `https://api.openweathermap.org/data/2.5/weather?q=${city}&appid=${WEATHER_API_KEY}&units=metric`;
const res = await fetch(url);
if (!res.ok) throw new Error("Invalid API key or city");

Choose a reason for hiding this comment

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

good response check here. however, the error description may have nothing to do with why the fetch request has failed. it could be a server timeout, it could be a 404 not found, it could be anything. but your returned description assumes that it is either the api key or city. if you want to match the error with the description, you would need to handle different response status codes.

therefore, if you want to use a generic response here, you would be doing something like:

if (!res.ok) {
  throw new Error(`request failed with status ${res.status} (${res.statusText})`);
}

}

// Load default city
updateCity("London");

Choose a reason for hiding this comment

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

your code says async function getWeather(city = "London"), if you are passing "London" as a parameter to the getWeather function then having city = "London" in the function declaration is redundant. it's not the end of the world btw. your implementation is very good, you only need to do getWeather(city) and then pass "London" as a parameter so it loads as your default city.

try {
const response = await fetch("https://dog.ceo/api/breeds/image/random");
if (!response.ok) {
throw new Error(`HTTP error! Status: ${response.status}`);

Choose a reason for hiding this comment

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

see (from my comments in the weather app), you have handled the !response.ok excellently here.


test("should return sum of two numbers", () => {
expect(add("3,6")).toBe(9);
});

Choose a reason for hiding this comment

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

very good add function and well-written tests. no notes.


function average(numbers) {}
if (numsOnly.length === 0) return 0;

Choose a reason for hiding this comment

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

very good.

});

return result;
return words.map(word => removeVowels(word));

Choose a reason for hiding this comment

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

cleaner. very good

@sambabib sambabib added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Aug 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Complete Volunteer to add when work is complete and all review comments have been addressed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants