Блог

Главная - Вопросы по программированию - нормально ли использовать while для этого?

нормально ли использовать while для этого?

#c#

#c#

Вопрос:

         while(player.CloseMenu(menuType))
        {
        }

player.CloseMenu(menuType) закроет одно меню выбранного типа или вернет false, если такого типа не осталось.

нормально ли использовать пустой цикл, подобный этому, чтобы закрыть все меню данного типа?

Комментарии:

1. Это кажется немного нетрадиционным.

2. да, это именно моя точка зрения. что еще я мог бы сделать вместо этого, не имея возможности прикоснуться к телу функции?

3. Какой фреймворк вы используете?

4. Это кажется странным, и возврат false кажется очень неуместным из-за имени метода CloseMenu . С первого взгляда, как другие разработчики (включая вас через 3 недели!) могут знать, что этот метод возвращает логическое значение?

5. Есть ли другой способ получить список всех открытых меню, а затем выполнить цикл по этому списку, закрыв их? Это казалось бы менее странным способом сделать это.

Ответ №1:

Будьте осторожны, когда придумываете умные способы выполнения чего-либо в коде. Это может сэкономить несколько нажатий клавиш в краткосрочной перспективе, но когда-нибудь кто-то другой может посмотреть на этот код и удивиться:

  • Что это вообще делает?
  • Хорошо, я вижу, что он делает, но почему это было сделано таким образом?
  • Есть ли веская причина для этого, и я должен избегать этого, чтобы не сломать что-то еще?

Имейте в виду, что кто-то другой вполне может оказаться вами через несколько месяцев после того, как вы забудете детали этого кода.

Сохранение нескольких строк кода на самом деле не имеет большого значения. Все, что нужно написать только один раз, требует конечного объема работы. Все, что вносит путаницу в поддержку в дальнейшем, приводит к неизвестному и менее конечному объему работы.

Ответ №2:

Я бы выбрал больше самодокументации на случай, если другим людям понадобится ее прочитать.

Проблема в том, что вы должны сделать вывод из того факта, что вызов выводится как bool, чтобы понять.

Может быть, если бы вы назвали это player.IsMoreAfterClose().

или

 while(true)
{
   bool b = player.CloseMenu(menuType);
   if(!b) break;
}

или

 bool b = true;
while(b)
{
  b = player.CloseMenu(menuType);
}

Комментарии:

1. while (true){ /* blah */ break; } — это дурной тон — что могло бы быть лучше, так это while (player. HasMenusOpen(menuType)){ проигрыватель. CloseMenu(menuType); } — здесь условие прерывания находится там, где оно должно быть

2. имхо, я не согласен с его плохой формой в цикле (тело которого имеет относительно низкое количество строк). комбинация ‘while (true)’ и ‘break’ формирует (для меня) четкую картину ‘вот что происходит’ и ‘вот что заставляет это остановиться’.

Ответ №3:

Я бы расширил все эти ответы и создал метод с именем CloseAllMenus(MenuType menuType) . Тогда вы можете поместить туда любую уродливую реализацию, и будет очевидно, что она делает, когда вы ее вызовете. Ваш текущий код не объясняет, что именно происходит, если вы уже не знаете, что может быть открыто несколько меню определенного типа, и что этот вызов закроет только одно из них.

Комментарии:

1. ну, на самом деле это в методе расширения для player, void CloseAllMenues(this Player player, Type menuType)

2. @Nico Это немного меняет дело. Если это единственное, что есть в функции, я бы сказал, что вы, вероятно, могли бы просто добавить к ней комментарий типа // CloseMenu returns false when it could not find a menu of that type to close , и это было бы прекрасно

Ответ №4:

Очевидно, что это сработает, но это немного затрудняет чтение / сопровождение кода. Возможно, вам было бы лучше, если бы условием цикла была проверка любых открытых меню, а тело цикла закрывало одно.

Ответ №5:

Это нормально делать так, как это означает do something until certain condition is met . Но лучше включить это в CloseMenu функцию, чтобы вам не приходилось повторять эту инструкцию много раз, и фактически это вариант закрытия меню, когда вы хотите закрыть все меню. Вы можете добавить логический аргумент к функции, чтобы указать, хотите ли вы закрыть все меню.

 bool CloseMenu(type, closeAll){
   if(closeAll)
      while(exists(type))
      { close...
   else
      if(exists(type)
      { close...
}

Комментарии:

1. Логические аргументы не интуитивно понятны. Трудно понять, что делает метод, например, разница между CloseMenu (‘type’, false) и CloseMenu(‘type’, true) не очевидна. Я бы предложил добавить два разных общедоступных метода с осмысленными именами, такими как CloseAllMenus.

2. Это очевидно, если вы читаете API (достаточно даже подписи функции, поскольку (правильно выбранное) имя аргумента может указывать на его назначение), что и должен делать разработчик.

3. Гораздо лучше, если код будет прост для понимания без чтения API. Почему разработчик должен тратить время на чтение API? 🙂

Ответ №6:

Возможно, «правильная» вещь, которую нужно сделать, это инвертировать из While / Close без тела в For / Open с телом, выполняющим операцию закрытия.

 foreach(var menu in player.OpenMenus)
  menu.Close();

или:

 player.OpenMenus.ForEach(c => c.Close());