-
Notifications
You must be signed in to change notification settings - Fork 16
Beny #4
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
base: main
Are you sure you want to change the base?
Beny #4
Conversation
Tamir198
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey so 2 main things that i see repeating:
No console logs
You dont have to booleanVariable === true inside a condition, if(booleanVariable) is eonugh
store.ts
Outdated
| ) /* : add return types */ { | ||
| return []; | ||
| function getAvailableProducts(store: Store): Product[] { | ||
| return store.products.filter((product) => product.inStock === true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could do something like this :
return store.products.filter((product)=>product.inStock);Because the inStock in a boolean
store.ts
Outdated
| function getAvailableProducts(store: Store): Product[] { | ||
| return store.products.filter((product) => product.inStock === true); | ||
| } | ||
| console.log("getAvailableProducts:", getAvailableProducts(store)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we submit prs in the industry we remove the console.logs
store.ts
Outdated
| (p) => p.price >= minPrice && p.price <= maxPrice | ||
| ); | ||
| } | ||
| console.log("getProductsInPriceRange", getProductsInPriceRange(store, 10, 100)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Save 10 and 100 inside variables, otherwise those are just magic numbers and we dont really have a context for them
No description provided.