Please help. In which direction should I move?

B

belikovss2021-03-28 23:52:14

Python

belikovss, 2021-03-28 23:52:14

Hi everyone. I am 16 years old, interested and fond of programming. I really liked the Python language.
I wrote a program that checks ports, I'm putting it up for evaluation.
I understand very well that it is terrible, but still I want to hear the opinion of more experienced people in this matter.
Before using it, you need to install modules:
termcolor
threading

import time
import sys
from termcolor import colored
import threading
import socket

# создание красивого меню (вверх)
def animate_menu_up():
  # объвяляю глобальную переменную в функции
  global flag
  # особождение строки
  print("\n")
  # переменная symbol будет содержать символ применяемый в анимации
  symbol = '~'
  # переменная string будет содержать результат анимации
  string = ""
  # анимация в цикле
  while len(string) <= 50:
    string += symbol
    sys.stdout.write(f"\r{string}")
    time.sleep(0.01)
  # освобождение места для следующей анимации
  print("", flush=True)
  flag = 1

# создание анимации центрального меню
def animate_menu_center():
  # объвяляю глобальную переменную в функции
  global flag
  # настройки для анимации
  string_first = list("[1] scan all ports")
  string_second = list("[2] scan enter port")
  string_three = list("SCANNER V1.0")
  # результат
  string_result_one = ""
  string_result_two = ""
  string_result_three = ""

  # анимация названия
  while len(string_result_three) != len(string_three):
    for char in string_three:
      string_result_three += char
      sys.stdout.write(f"\r\t\t{string_result_three}")
      time.sleep(0.04)
  # выделение места под другую строку
  print("\n")
  # сама анимация
  while len(string_result_one) != len(string_first):
    for char in string_first:
      string_result_one += char
      sys.stdout.write(f"\r\t{string_result_one}")
      time.sleep(0.04)
  # выделение места под другую строку
  print("")
  # анимация второй строки
  while len(string_result_two) != len(string_second):
    for char in string_second:
      string_result_two += char
      sys.stdout.write(f"\r\t{string_result_two}")
      time.sleep(0.04)
  # принудительно ощичаем поток
  sys.stdout.flush()
  print("", flush=True)
  flag = 2

# создание анимации меню (низ)
def animate_menu_down():
  # объвяляю глобальную переменную в функции
  global flag
  # тоже самое, что и в animate_menu_up
  symbol = '~'
  # тоже самое, что и в animate_menu_up
  string = ""
  # тоже самое, что и в animate_menu_up
  while len(string) <= 50:
    string += symbol
    sys.stdout.write(f"\r{string}")
    time.sleep(0.01)
  # принудительно очищаем поток
  sys.stdout.flush()
  print("")
  flag = 3

# функция, которая будет проверять выбранный режим
# и что-то делать
def select_mode():
  # переменная, которая будет содержать выбранный режим
  answer = input("\n[scan] Select mode: ")
  # переменная, которая будет содержать хост, для проверки портов
  hostname = input("[scan] Enter hostname: ")
  
  if answer == "1":
    # если режим проверки всех популярных портов
    # создается список с известными портами
    ports = [22, 80, 7777, 2516]

    print("")

    while True:
      for port in ports:
        # создаю объект сокета, который в будущем будет помогать проверять порты
        sock = socket.socket()
        try:
          sock.settimeout(0.5)
          sock.connect((hostname, port))
        except socket.error as socketerror:
          print(colored(f"[!] ", "red") + str(port) + " --CLOSE")
        else:
          print(colored(f"[#] ", "green") + str(port) + " --OPEN")
      break


# переменная, которая содержит выбранный режим
answer = ""
# переменная - флаг, нужна чтобы контролировать выполнение потоков
flag = 0
# создание объекта Thread
th_one = threading.Thread(target=animate_menu_up)
th_two = threading.Thread(target=animate_menu_down)
th_three = threading.Thread(target=animate_menu_center)

# запуск потоков
th_one.start()
while True:
  try:
    if flag == 1:
      th_three.start()
    elif flag == 2:
      th_two.start()
    elif flag == 3:
      select_mode()
  except RuntimeError:
    continue

  
#th_two.start()

link to the file (for everyone) - https://disk.yandex.ru/d/Mq8d9johP45W0g

Reply

Answer the question

In order to leave comments, you need to log in

3 answer(s)

J

Jock Tanner, 2021-03-29
@belikovss

For starters, I don't understand why all the parts of your program that could well run synchronously are running in threads. There is a part of the program that would benefit from multithreading - port scanning, where it would be logical to create a thread for each port (if there are too many ports, then create a pool of threads, determine a strategy for reusing threads, in general, there is a lot of room for creativity), but your ports are scanned in one thread, sequentially.
But let's pretend that's the way it should be and talk about other problems in your code.

You should not use global variables in programs longer than one screen. Instead global, it's better to declare a global object and hide flagit in it:

class PortScanner:
    
    def __init__(self):
        self.flag = 0
        
    def animate_menu_up(self):
        print("\n")
        ...
        self.flag = 1

It is also better to transfer the program initialization code to a __init__()global object. And move the main loop into a separate method, for example, run(). Then at the bottom level we will be left with something like:
```
import ...

class PortScanner:
    ...

if __name__ == '__main__':
    main_obj = PortScanner()
    main_obj.run()
```
this idiomatic code will allow you to import the PortScanner class into another script, and also provide a plus in the interview / review,

learn to use docstrings instead of comments:

def animate_menu_up():
    """ Создание красивого меню (вверх). """

extra cycle while True:in select_mode(),
too much magic. As the program grows, it becomes more and more difficult to keep and compare all sorts of abstract values in your head. Here are the literals that, in my opinion, should be defined as constants - either in the "header" of the script, or as class attributes:
```
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
```
such things are very painful to shoot in the leg and are almost guaranteed to fail interviews:
```
except RuntimeError:
    continue
```
if you really want to continue running the program after such an error (which under normal circumstances is pointless and dangerous), then at least take care of the correct indication:
```
import traceback
...
except RuntimeError:
    traceback.print_exc(file=sys.stdout)
    continue
```

Since we're stuck here, let's make the dispatcher more idiomatic:

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

Such a naive approach brings a lot of husks to the screen, but it doesn't matter. The important thing is that such code is easier to read and modify than the if...elif...else.

Working with strings is also in need of improvement:

it makes no sense to split strings into lists, characters in a string can be accessed in the same way as elements of a list - by index and using slices,
no need to copy lines,
no need to print the whole line from the beginning, this makes the animation uneven - the end of the line is printed slower than the beginning. It is enough to print one character per line.

For example:

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

(I removed the colors for simplicity.)

usability suffers greatly due to the lack of a normal function to exit the application,
do not leave commented code in the file that you submit for review, this is a minus. Instead, write a comment marked “TODO”, like so:
```
# TODO: реализовать режим '2' (скан произвольного списка портов)
```
This is definitely a plus - it shows that you know how to work in a team and use version control systems.

It seems that I have listed the most serious problems. I'll post a more complete version of the code in the comments.

M

mkone112, 2021-03-29
@mkone112

I wrote a program that checks ports, I'm putting it up for evaluation.
I understand very well that it is terrible, but still I want to hear the opinion of more experienced people in this matter.

Yes, she's really terrible. Contact.

S

Sergey Gornostaev, 2021-03-29
@sergey-gornostaev

The most obvious thing is to get rid of global. This statement is always a bad code marker.