-
Notifications
You must be signed in to change notification settings - Fork 206
feat: Resolve tasks from basic js module #163
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: master
Are you sure you want to change the base?
Conversation
devmentor-pl
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.
Żaneto,
Zadania wyszły bardzo dobrze! 👍
Zostawiłem Ci komentarze jak jeszcze można usprawnić Twój kod ;)
01/assets/js/app.js
Outdated
| console.log('Wynik dekrementacji jest większy od 20'); | ||
| } else { | ||
| console.log('Wynik dekrementacji jest mniejszy od 20'); | ||
| } |
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.
PS. Może warto było zapisywać dane w tablicy i potem przy pomocy pętli iterować po każdym elemencie i wykorzystać if do warunku - takie rozwiązanie może być optymalniejsze :)
02/app.js
Outdated
| counter++; | ||
| result = result * a; | ||
|
|
||
| displayString = `${displayString} * ${a}`; |
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.
Jak zrobić, aby pominąć znak na samym początku? Może wystarczy if sprawdzający czy to "pierwsza" iteracja?
03/app.js
Outdated
| const firstComb = (a > b && b > c) || (a < b && b > c && a > c) ? a + b : null; | ||
| const secondComb = a > b && c > b ? a + c : null; | ||
| const thirdComb = b > a && c > a ? b + c : null; | ||
|
|
||
| if (firstComb === null && secondComb === null) { | ||
| return thirdComb; | ||
| } else if (firstComb === null && thirdComb === null) { | ||
| return secondComb; | ||
| } else if (thirdComb === null && secondComb === null) { | ||
| return firstComb; | ||
| } |
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.
Może warto dodać liczby do tablicy i je posortować? Myślę, że będzie prościej :)
03/app.js
Outdated
| const isNull = `Podany arguments ${num} nie jest liczbą`; | ||
| const isTrue = `Podany arguments ${num} jest parzysty`; | ||
| const isFalse = `Podany arguments ${num} jest nieparzysty`; |
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.
Jeśli idziemy w tą stronę to można jeszcze ładniej to zrobić tj. wykorzystać strukturę danych np.
const messages = {
'null': '...',
'true': '...',
}
console.log(messages[bool])
Tutaj znajdziesz szczegóły: https://devmentor.pl/b/mniej-instrukcji-warunkowych
| }; | ||
|
|
||
| const biggestTen = getTenBiggest(newArr); | ||
| console.log('Średnia arytmetyczna 10 największych liczb:', biggestTen); |
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.
👍
05/app.js
Outdated
| const initVal = 0; | ||
| const sumWithInit = grades.reduce((acc, curr) => acc + curr, initVal); | ||
| const avgSum = sumWithInit / grades.length; | ||
|
|
||
| return avgSum; |
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.
Zwróć uwagę, że tutaj mamy powielony kod z linii 39-43 - może warto napisać funkcję, którą wykorzystasz w obu tych miejscach.
devmentor-pl
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.
Żaneto,
Zdecydowanie teraz wygląda lepiej! 👍
Zostawiłem jeszcze kilka komentarzy do przejrzenia :)
01/assets/js/app.js
Outdated
| } | ||
|
|
||
| if (item < 20) { |
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.
Jeśli chcemy wykluczyć "równe" to lepiej zapisać else if zamiast if - bo wtedy wykona się jedno lub drugie. A teraz jedno i drugie porównanie. Z punktu widzenia optymalizacji obecne rozwiązanie jest gorsze.
Natomiast jeśli przypisać też równie to lepiej użyć jedynie else
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.
Chciałam wypisać w konsoli zarówno listę wyników większych od 20 i listę mniejszych od 20, tak jak zakłada polecenie :)
Użyj instrukcji warunkowej, aby sprawdzić, który z wyników jest większy od 20, a który mniejszy.
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.
I oczywiście przy użyciu else if otrzymuje to co chciałam :)
| displayString = `${a}`; | ||
| } else { | ||
| displayString = `${displayString} * ${a}`; | ||
| } |
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.
👍
| const initVal = 0; | ||
| const sumWithInitVal = twoBiggest.reduce((acc, curr) => acc + curr, initVal); | ||
|
|
||
| return sumWithInitVal; |
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.
👍
| }; | ||
|
|
||
| const randomBool = bool; | ||
| const randomBool = messages[bool]; |
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.
Teraz już nie potrzebujemy switch-a, rozwiązanie jest już dostępne w randomBool :)
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.
O, wow :o
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.
:)
| return grades; | ||
| } else { | ||
| return allGrades; | ||
| } |
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.
👍
devmentor-pl
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.
👍
| } | ||
|
|
||
| if (item < 20) { | ||
| } else if (item < 20) { |
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.
👍
| }; | ||
|
|
||
| const randomBool = bool; | ||
| const randomBool = messages[bool]; |
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.
:)
No description provided.