Skip to content

Conversation

@aliceklim
Copy link

No description provided.

Copy link
Contributor

@rlrio rlrio left a comment

Choose a reason for hiding this comment

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

создавай, пожалуйста, под каждое задание отдельную ветку и отдельный ПР, а то я немного запуталась, проверять ли мне задание про прогноз погоды или про дома из игры престолов. Так же как совет, осмысленно ставить пустые строки, они тоже важны и могут повлиять на читаемость кода. В целом, очень хорошая работа! Молодец! аппрув)

public String getWeatherReport(String city){
if (weatherReport.containsKey(city)){
return weatherReport.get(city).toString();
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

можно убрать else {, вытащить то что сейчас в этом блоке else из него, так как если поинтер зайдет в блок if и if будет true, то блок if отработает и мы выйдем из метода

if (weatherReport.containsKey(city)){
weatherReport.remove(city);
return String.format(Message.CITY_DELETED, city);
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

то же самое что и выше, не обязательно писать throw в блоке. И я бы написала throw в блоке if сделав инверсию, а остальной код без блока, вот так:
if (!weatherReport.containsKey(city)){ throw new ForecastException(String.format(Message.NO_SUCH_CITY, city)); } weatherReport.remove(city); return String.format(Message.CITY_DELETED, city);

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

Comment on lines 12 to 13
String motto;
String sigil;
Copy link
Contributor

Choose a reason for hiding this comment

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

private

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