Для начала, мне непонятно, почему все части вашей программы, которые вполне могут выполняться синхронно, работают в потоках. Есть часть программы, которая выиграла бы от многопоточности − сканирование портов, где логично было бы создать по потоку на каждый порт (если портов слишком много, то создать пул потоков, определить стратегию переиспользования потоков, в общем, тут большой простор для творчества), но у вас порты сканируются в одном потоке, последовательно.
Но давайте притворимся, что так и надо, и поговорим о других проблемах вашего кода.
- Не стоит использовать глобальные переменные в программах длиннее одного экрана. Вместо
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.
Работа со строками тоже напрашивается на улучшения: - нет смысла разбивать строки на списки, к символам в строке можно обращаться так же, как к элементам списка − по индексу и с помощью срезов,
- не нужно копировать строки,
- не нужно печатать всю строку с начала, это делает анимацию неровной − конец строки печатается медленнее, чем начало. Достаточно допечатать один символ в строке.
Например: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' (скан произвольного списка портов)
Это однозначно плюс − показывает, что вы умеете работать в команде и пользоваться системами контроля версий.
Вроде бы наиболее серьёзные проблемы я перечислил. Выложу более полный вариант кода в комментариях.
