Рефакторинг кода, как сделать красиво?

Начал писать Бота для Telegram, который будет помогать в айтишных делах. Основная идея в том, чтобы завести каждый компьютер в базу и плюс привязка по пользователю, который в текущее время сидит за компьютером. Хочу запустить на сервере, чтобы бы в бесконечном цикле собирало инфу и оповещало об изменениях.
Написал класс для сбора данных с компьютера по wmi. Вот пример
import wmi import config
class Computer:
def hdd_info(ip): hdd = [] wql = 'SELECT * FROM Win32_DiskDrive' try: c = wmi.WMI(ip, user=config.admin, password=config.password) for computer in c.query(wql): hdd.append(computer.Model) hdd.append(computer.SerialNumber) hdd.append(computer.Size) return hdd except: return hdd
def motherboard_info(ip): motherboard = [] wql = 'SELECT * FROM Win32_BaseBoard' try: c = wmi.WMI(ip, user=config.admin, password=config.password) for computer in c.query(wql): motherboard = [computer.Manufacturer, computer.Product] return motherboard except: return motherboard
Теперь начинаю работу со всем этим. Вот функция Main
ip_subnet = ["192.168.4.", "192.168.5."]
def main(): i = 55 for ip in range(len(ip_subnet)): while i <= 255: ip_adr = ip_subnet[ip] + str(i) response = subprocess.call(["ping", "-n", "1", "-w", "200", ip_adr]) if(response == 0): try: wql = 'SELECT * FROM Win32_computerSystem' c = wmi.WMI(ip_adr, user=config.admin, password=config.password) for computer in c.query(wql): check_in_db_if_ping(computer.DNSHostName, ip_adr, computer.UserName) except: pass else: check_in_db_if_not_ping(ip_adr) i += 1 i = 0
Тут я циклом перебираю все наши подсети и с помощью запроса получаю первичные данные. Если данные получены, передаю их в следующую функцию, которая проверяет есть ли такой компьютер в базе check_in_db_if_ping. Данная функция очень длинная и я приведу ее не всю.
def check_in_db_if_ping(hostname, ip, user=None): cursor.execute('SELECT IP FROM Computers WHERE IP = ?', (ip,)) check_ip = cursor.fetchall() cursor.execute('SELECT HostName FROM Computers WHERE HostName = ?', (hostname,)) check_hostname = cursor.fetchall() conn.commit() if len(check_ip) == 0 and len(check_hostname) == 0: add_to_db(hostname, ip, user)
user = None стоит потому-что компьютер может быть включен, а текущий пользователь отошел и тогда при запросе, придет None, там вроде есть какой-то таймаут, если пользователь не активен какое-то время.
В фунцкии check_in_db_if_ping я проверяю, есть ли такой IP адрес и HostName в базе не существует, то я начинаю заводить его в базу и передаю в функцию add_to_db
И вот самое главное, как выглядит функция добавления компьютера в базу:
def add_to_db(hostname, ip, user = None): motherboard = Computer.motherboard_info(ip) ram = Computer.ram_info(ip) cursor.execute('INSERT INTO Computers (HostName, IP, Status, OnTime, OffTime, MotherBoard, RAM) VALUES (?,?,?,?,?,?,?)', (hostname, ip, "Online", str(time),"-",motherboard[0] + " " + motherboard[1], str(ram[0]))) last_id = cursor.lastrowid conn.commit() if user != None: cursor.execute('INSERT INTO User (ComputerId, Login) VALUES (?,?)', (last_id, user)) conn.commit()
Мне в эта функция не нравится: этот длинный запрос и склейка данных, но как упростить я его я не знаю.
Можно как-нибудь собрать, грубо говоря, все данные о компьютере в лист и просто листом все записать в базу? И вообще есть ли замечания по коду?

Ответ

Рекомендую обратить ваше внимание к двум книгам, которые считаю достаточно важными для выработки навыков написания чистого кода:
1) "Clean code" Роберта Мартина https://www.ozon.ru/context/detail/id/5011068/
2) "Рефакторинг" Фаулера http://www.ozon.ru/context/detail/id/1308678/
В них вы найдете ответы на ваши вопросы о том, как декомпозировать функцию, что можно вынести в отдельный класс, почему метод на 100 строк плохой и как с этим бороться.
Не проигнорируйте рекоммендацию, а действительно скачайте(купите) книгу и у вас появится понимание о том, как рефакторить.