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

Solution #4837

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

Solution #4837

wants to merge 3 commits into from

Conversation

k0tlik02
Copy link

No description provided.

Copy link

@Jion353 Jion353 left a comment

Choose a reason for hiding this comment

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

молодець, дуже гарно і зрозуміло написано)))

const getOff3days = 20;
let totalCost = rentCostForOneDay * days;

if (days >= 7) {
Copy link

Choose a reason for hiding this comment

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

краще використати константу замість простого числа, так само і до наступного )

@k0tlik02 k0tlik02 requested a review from Jion353 October 28, 2024 09:02
Copy link

@Jion353 Jion353 left a comment

Choose a reason for hiding this comment

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

тепер все взагалі ідеально))

@k0tlik02 k0tlik02 requested a review from Jion353 October 29, 2024 06:27
Copy link

@BudnikOleksii BudnikOleksii left a comment

Choose a reason for hiding this comment

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

Good work 👍
Just a few minor fixes are needed

Comment on lines 7 to 11
const rentCostForOneDay = 40;
const LONG_TERM_DISCOUNT = 50;
const SHORT_TERM_DISCOUNT = 20;
const LONG_TERM = 7;
const SHORT_TERM = 3;

Choose a reason for hiding this comment

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

All constants should be in UPPER_CASE and move them outside the function

Comment on lines 12 to 18
let totalCost = rentCostForOneDay * days;

if (days >= LONG_TERM) {
totalCost -= LONG_TERM_DISCOUNT;
} else if (days >= SHORT_TERM) {
totalCost -= SHORT_TERM_DISCOUNT;
}

Choose a reason for hiding this comment

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

Suggested change
let totalCost = rentCostForOneDay * days;
if (days >= LONG_TERM) {
totalCost -= LONG_TERM_DISCOUNT;
} else if (days >= SHORT_TERM) {
totalCost -= SHORT_TERM_DISCOUNT;
}
const totalCost = rentCostForOneDay * days;
if (days >= LONG_TERM) {
return totalCost - LONG_TERM_DISCOUNT;
}
if (days >= SHORT_TERM) {
return totalCost - SHORT_TERM_DISCOUNT;
}

Prefer if with return over if else to simplify later conditions.
image

@@ -3,8 +3,24 @@
*
* @return {number}
*/
const RENTCOSTFORONEDAY = 40;

Choose a reason for hiding this comment

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

Suggested change
const RENTCOSTFORONEDAY = 40;
const RENT_COST_FOR_ONE_DAY = 40;

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.

3 participants