• Написал программу, которая определяет тип числа - составное или простое. Так ли всё ужасно написано?

    @UberPool
    Python/JS coder
    По коду вроде не всё так плохо,единственное зря столько комментариев,они нужны в моментах где прям сложная конструкция.А вообще по оформлению и чистоте кода почитайте pep8.
    Ответ написан
    1 комментарий
  • Прошу помощи. В каком направлении мне двигаться?

    Tanner
    @Tanner
    Огромный человекоподобный боевой робот
    Для начала, мне непонятно, почему все части вашей программы, которые вполне могут выполняться синхронно, работают в потоках. Есть часть программы, которая выиграла бы от многопоточности − сканирование портов, где логично было бы создать по потоку на каждый порт (если портов слишком много, то создать пул потоков, определить стратегию переиспользования потоков, в общем, тут большой простор для творчества), но у вас порты сканируются в одном потоке, последовательно.

    Но давайте притворимся, что так и надо, и поговорим о других проблемах вашего кода.
    • Не стоит использовать глобальные переменные в программах длиннее одного экрана. Вместо global лучше объявить глобальный объект и спрятать flag в него:
      class PortScanner:
          
          def __init__(self):
              self.flag = 0
              
          def animate_menu_up(self):
              print("\n")
              ...
              self.flag = 1


    • Код инициализации программы тоже лучше перенести в __init__() глобального объекта. И основной цикл вынести в отдельный метод, например, run(). Тогда на нижнем уровне у нас останется что-то вроде:
      import ...
      
      class PortScanner:
          ...
      
      if __name__ == '__main__':
          main_obj = PortScanner()
          main_obj.run()

      этот идиоматичный код позволит импортировать класс PortScanner в другой скрипт, а также обеспечит плюсик на собеседовании/ревью,
    • приучайтесь использовать докстринги вместо комментариев:
      def animate_menu_up():
          """ Создание красивого меню (вверх). """

    • лишний цикл while True: в select_mode(),
    • слишком много магии. По мере роста программы становится всё тяжелее держать и сопоставлять в голове всякие абстрактные значения. Вот литералы, которые, по моему мнению, стоило бы определить как константы − или в «шапке» скрипта, или как атрибуты класса:
      MF_INITIAL = 0
      MF_MENU_CENTER = 1
      MF_MENU_DOWN = 2
      MF_SELECT = 3
      
      SCREEN_WIDTH = 50
      ALL_PORTS = [22, 80, 7777, 2516]
      SOCK_TIMEOUT = 0.5
      ANIM_SYMBOL = '~'
      ANIM_DELAY = 0.02

    • такие вещи очень больно стреляют в ногу и почти гарантированно проваливают собеседования:
      except RuntimeError:
          continue
      если вы действительно хотите продолжить выполнение программы после такой ошибки (что в обычных обстоятельствах бессмысленно и опасно), то позаботьтесь хотя бы о правильной индикации:
      import traceback
      ...
      except RuntimeError:
          traceback.print_exc(file=sys.stdout)
          continue

    • раз уж мы задержались здесь, давайте сделаем диспетчер более идиоматичным:
      while True:
          try:
              {
                  MF_MENU_CENTER: th_three.start,
                  MF_MENU_DOWN: th_two.start,
                  MF_SELECT: select_mode,
              }[flag]()
          except RuntimeError:
              traceback.print_exc(file=sys.stdout)
              continue
      Такой наивный подход выводит много шелухи на экран, но это не важно. Важно то, что такой код проще читать и дорабатывать, чем цепочку if...elif...else.

    • Работа со строками тоже напрашивается на улучшения:
      1. нет смысла разбивать строки на списки, к символам в строке можно обращаться так же, как к элементам списка − по индексу и с помощью срезов,
      2. не нужно копировать строки,
      3. не нужно печатать всю строку с начала, это делает анимацию неровной − конец строки печатается медленнее, чем начало. Достаточно допечатать один символ в строке.
      Например:
      def animate_menu_center():
          """ Создание анимации центрального меню. """
          # настройки для анимации
          output_strings = [
              '[1] scan all ports',
              '[2] scan enter port',
              '[3] exit',
              'SCANNER V1.0',
          ]
      
          # анимация названия
          for output_string in output_strings:
              print('\r\t\t', end='')
              for ch in output_string:
                  print(ch, end='')
                  time.sleep(ANIM_DELAY)
              # last string?
              if output_string != output_strings[-1]:
                  # new line
                  print()
      
          print('', flush=True)
          flag = MF_MENU_DOWN
      (Я убрал цвета для простоты.)
    • юзабилити сильно страдает из-за отсутствия нормальной функции выхода из приложения,
    • не оставляйте закомментированный код в файле, который отдаёте на ревью, это минус. Вместо этого напишите комментарий с пометкой “TODO”, например:
      # TODO: реализовать режим '2' (скан произвольного списка портов)
      Это однозначно плюс − показывает, что вы умеете работать в команде и пользоваться системами контроля версий.

    Вроде бы наиболее серьёзные проблемы я перечислил. Выложу более полный вариант кода в комментариях.
    Ответ написан
    1 комментарий
  • Прошу помощи. В каком направлении мне двигаться?

    @mkone112
    Начинающий питонист.
    Написал программу, которая осуществляет проверку портов, выставляю ее на оценку.
    Очень прекрасно понимаю, что она ужасная, но всё же хочу услышать мнение более опытных людей, в этом деле.

    Да, она действительно ужасная. Обращайся.
    Ответ написан
    Комментировать
  • Прошу помощи. В каком направлении мне двигаться?

    sergey-gornostaev
    @sergey-gornostaev Куратор тега Python
    Седой и строгий
    Самое очевидное - избавьтесь от global. Этот оператор - всегда маркер плохого кода.
    Ответ написан
    Комментировать