Skip to content

Homework 7 is done#4

Open
ushki92 wants to merge 1 commit into
masterfrom
lesson1.7
Open

Homework 7 is done#4
ushki92 wants to merge 1 commit into
masterfrom
lesson1.7

Conversation

@ushki92

@ushki92 ushki92 commented Sep 14, 2021

Copy link
Copy Markdown
Owner

No description provided.

@DisterDE DisterDE left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Хорошо, но много стилистических косяков.
Советую их доработать.

}
}

private static void calculateYearAndPrint(int a) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Имеет смысл переименовать в printIsLeapYear или что-то вроде того.
Метод calculateAndPrint подразумевает, что в нем вызываются два метода, а именно calculate и print. %)

return calculateDeliveryDays;
}

private static void checkDeviceVersionRecomendationsAndPrint(int a, int b) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

a и b очень плохие имена для параметров.
Что из этого версия, а что год?
Нужно давать более очевидные имена.

Comment on lines +87 to +93
if (a < 20) {
calculateDeliveryDays += 1;
} else if (a < 60) {
calculateDeliveryDays += 2;
} else {
calculateDeliveryDays += 3;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Не хотел проверять саму логику методов, но тут придется.
Я рассказывал на первом или втором QA вебинаре причины, которые не позволяют писать так.
Стоит посмотреть вебинар и переработать.


}

private static int calculateDeliveryDays(int a) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Снова плохое имя параметра.
Это не a, а, возможно, deliveryDistance или что-то подобное.

}
}

private static void stringCheckForDoubles(String arr) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Имя метода принято называть с глагола.
findDuplicatesInString, если уж очень хочется упомянуть String.

Comment on lines +60 to +68
for (int i = 0; y > i; ) {
if (a[i] != a[z]) {
x = a[i];
a[i] = a[z];
a[z] = x;
}
z--;
i++;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

i следует итерировать в 3ем блоке цикла for, а вот от z вообще стоит отказаться, так как это значение всегда вычисляется с помощью длины и i.

return sumMonth;
}

public static void calculateAverageDaySpentAndPrint(int a, int b) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Такие методы, опять же, подразумевают, что внутри вызывается два других метода.
Снова плохие имена.

Comment on lines +41 to +42
float averageDaySpent;
averageDaySpent = a / b;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Это должно быть одной строкой.

public static void calculateAverageDaySpentAndPrint(int a, int b) {
float averageDaySpent;
averageDaySpent = a / b;
System.out.println("Средняя трата за один день " + averageDaySpent);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Или вообще должно быть вставлено здесь вместо переменной в виде (a / b)

import java.time.LocalDate;
import java.util.Arrays;

public class lesson7 {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Классы принято называть только с большой буквы.
Примером выше выделяются классы Arrays и LocalDate.

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.

2 participants