DjangoBB LoFi version

Полная версия: Покритикуйте пожалуйста мой код

Начало » Python для новичков » Покритикуйте пожалуйста мой код

1 2

Bjiq

Ноя. 9, 2017 07:02:48

 class Main:
    def __init__(self, ip_addr, mask, ident_number):
        self.ip_addr = ip_addr
        self.mask = mask
        self.ident_number = ident_number
    input_file = 'data/log_test.txt'
    read_file = open(input_file, mode='r', encoding='latin_1')
    output_file = 'data/info_in_log.txt'
    write_file = open(output_file, mode='w', encoding='latin_1')
    def search_info_inline(self):
        for num, line in enumerate(obj1.read_file, 0):
            if self.ip_addr in line:
                if self.mask in line:
                    if self.ident_number in line:
                        print(" Line № " + str(num) + " " + line)
                        obj1.write_file.write(" " + line)
obj1 = Main('183.39.242.81', 'CHN', '1')
obj1.search_info_inline()

 [code python][code python][/code][/code]

Я только начал писать код на python, поэтому принимается любая критика))
Что я хочу сделать:
Есть лог-файл, пользователь вводит ip-адрес, маску названия лога и числовой идентификатор, по которому нужно найти информацию в файле.
Необходимо написать функцию, которая по входным данным найдёт:
1. Строку в логе с идентификатором.
2. Выведет +100 и -100 строк от этого идентификатора.(реализация в процессе)

Примерно так выглядит файл:
183.39.233.3 # 2017-09-11, 183.39.233.3, CHN, 1
183.39.233.101 # 2017-09-09, 183.39.233.101, CHN, 1
183.39.233.232 # 2017-09-05, 183.39.233.232, CHN, 1
183.39.235.31 # 2017-09-10, 183.39.235.31, CHN, 1

и еще, не совсем понял как работает глобальные переменные в python, pycharm ругается на глобальную переменную внутри функции, если указать без объекта класса(например, obj1.read_file)

FishHook

Ноя. 9, 2017 07:54:17

Bjiq
Покритикуйте пожалуйста мой код

Плохо, у вас несколько очень принципиальных ошибок

Во-первых, если вы открываете файл, то вы дожны его и закрывать. Интуитивно понятно, что чем меньше времени открыт файл, тем лучше. Вы открываете файл на уровне класса, то есть в тот момент когда код вообще импортируется, еще до того, как начала работать какая-то логика класса. Не говоря о том, что так делать вообще-то очень плохо (потому что ошибка открытия файла уронит вообще всю программу еще до фактического начала её работы), не говоря о том, что в местах возможных исключений их надо перехватывать и обрабатывать, это плохо потому что вы открываете файл заранее не зная нужен ли он вам будет вообще. У вас есть класс, вы можете использовать объекты этого класса в разных частях программы и вы не знаете заранее, будете ли вы создавать какие-то объекты в момент импорта класса. Но файлы вы открываете уже в момент импорта!

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

     def search_info_inline(self):
        for num, line in enumerate(obj1.read_file, 0):

вы понимаете, что obj1 и self - это две ссылки на один и тот же объект?

В-третьих. Какова семантика класса Main? Что конкретно он инкапсулирует? Вы просто взяли код без классов, перенесли все объявления и вызовы в класс и…? Чего вы добились то? Это какой-то ООП ради ООП. Это бессмысленно. Классы должны быть самодостаточными узкоспециализированными сущностями отображающими некие логические единицы или объекты предметной области.

Короче, очень плохо.

Rodegast

Ноя. 9, 2017 10:34:59

 if self.ip_addr in line:
      if self.mask in line:
            if self.ident_number in line:
                  print(" Line № " + str(num) + " " + line)

 if self.ip_addr in line and self.mask in line and self.ident_number in line:
        print("Line № %i %s"%(num, line))

scidam

Ноя. 9, 2017 14:22:04

Вероятно, что вместо self.indent_number in line должно быть, что-то self.indent_number == line.split(',').strip().
С self.mask – тоже самое, хотя такая маска в строках единственная и in, наверно, подходит.

Bjiq

Ноя. 9, 2017 16:21:55

FishHook, спасибо большое) разобрался с self, немного подправил код:

 class Main:
    def __init__(self, ip_addr, mask, ident_number):
        self.ip_addr = ip_addr
        self.mask = mask
        self.ident_number = ident_number
    def open_file_to_read(self):
        input_file = 'data/log_test.txt'
        read_file = open(input_file, mode='r', encoding='UTF-8')
        return read_file
    def open_file_to_write(self):
        output_file = 'data/info_in_log.txt'
        write_file = open(output_file, mode='w', encoding='UTF-8')
        return write_file
    def close_file(self, file):
        file.close()
    def search_info_inline(self):
        for num, line in enumerate(self.open_file_to_read(), 0):
            if self.ip_addr in line and self.mask in line and self.ident_number in line:
                print("Line № %i %s" % (num, line))
                print("Number of line = " + str(num))
                self.open_file_to_write().write(line)
                self.close_file(self.open_file_to_write())
                self.close_file(self.open_file_to_read())

Всем большое спасибо

FishHook

Ноя. 9, 2017 16:49:16

Плохо поправил
'data/log_test.txt'
'data/info_in_log.txt'

это же константы! Почему константы у вас каждый раз заново определяются при вызове функции?

Bjiq

Ноя. 9, 2017 17:49:07

Сам не додумался

Bjiq

Ноя. 9, 2017 20:36:12

Можете подсказать, как узнать номер последней строки файла?

Bjiq

Ноя. 9, 2017 20:46:07

пока так сделал, но я думаю можно улучшить код, если учесть, что иногда файл может весит больше 5 Мб

     def search_last_line(self):
        last_line = None
        for num, line in enumerate(self.open_file_to_read(), 0):
            last_line = num
        print('last_line = ' + str(last_line))
        return last_line

noob_saibot

Ноя. 10, 2017 09:31:46

 def search_last_line(self):
        last_line = self.open_file_to_read().readlines()[-1]:
        print('last_line = ' + str(last_line))
        return last_line