Python - Преобразование CSV в объекты - Code Code

Question

У меня есть небольшой скрипт, который мы используем для чтения в CSV-файле, содержащем сотрудников, и выполняем некоторые основные манипуляции с этими данными.

Мы читаем данные (import_gd_dump) и создаем объект Employees, содержащий список объектов Employee (может быть, я должен подумать о лучшем соглашении об именах ... lol). Затем мы звоним clean_all_phone_numbers() по номеру Employees, который называет clean_phone_number() по каждому Employee, а также lookup_all_supervisors(), на Employees.

import csv 
import re 
import sys 

#class CSVLoader: 
# """Virtual class to assist with loading in CSV files.""" 
# def import_gd_dump(self, input_file='Gp Directory 20100331 original.csv'): 
#  gd_extract = csv.DictReader(open(input_file), dialect='excel') 
#  employees = [] 
#  for row in gd_extract: 
#   curr_employee = Employee(row) 
#   employees.append(curr_employee) 
#  return employees 
# #self.employees = {row['dbdirid']:row for row in gd_extract} 

# Previously, this was inside a (virtual) class called "CSVLoader". 
# However, according to here (http://tomayko.com/writings/the-static-method-thing) - the idiomatic way of doing this in Python is not with a class-function but with a module-level function 
def import_gd_dump(input_file='Gp Directory 20100331 original.csv'): 
    """Return a list ('employee') of dict objects, taken from a Group Directory CSV file.""" 
    gd_extract = csv.DictReader(open(input_file), dialect='excel') 
    employees = [] 
    for row in gd_extract: 
     employees.append(row) 
    return employees 

def write_gd_formatted(employees_dict, output_file="gd_formatted.csv"): 
    """Read in an Employees() object, and write out each Employee() inside this to a CSV file""" 
    gd_output_fieldnames = ('hrid', 'mail', 'givenName', 'sn', 'dbcostcenter', 'dbdirid', 'hrreportsto', 'PHFull', 'PHFull_message', 'SupervisorEmail', 'SupervisorFirstName', 'SupervisorSurname') 
    try: 
     gd_formatted = csv.DictWriter(open(output_file, 'w', newline=''), fieldnames=gd_output_fieldnames, extrasaction='ignore', dialect='excel') 
    except IOError: 
     print('Unable to open file, IO error (Is it locked?)') 
     sys.exit(1) 

    headers = {n:n for n in gd_output_fieldnames} 
    gd_formatted.writerow(headers) 
    for employee in employees_dict.employee_list: 
     # We're using the employee object's inbuilt __dict__ attribute - hmm, is this good practice? 
     gd_formatted.writerow(employee.__dict__) 

class Employee: 
    """An Employee in the system, with employee attributes (name, email, cost-centre etc.)""" 
    def __init__(self, employee_attributes): 
     """We use the Employee constructor to convert a dictionary into instance attributes.""" 
     for k, v in employee_attributes.items(): 
      setattr(self, k, v) 

    def clean_phone_number(self): 
     """Perform some rudimentary checks and corrections, to make sure numbers are in the right format. 
     Numbers should be in the form 0XYYYYYYYY, where X is the area code, and Y is the local number.""" 
     if self.telephoneNumber is None or self.telephoneNumber == '': 
      return '', 'Missing phone number.' 
     else: 
      standard_format = re.compile(r'^\+(?P<intl_prefix>\d{2})\((?P<area_code>\d)\)(?P<local_first_half>\d{4})-(?P<local_second_half>\d{4})') 
      extra_zero = re.compile(r'^\+(?P<intl_prefix>\d{2})\(0(?P<area_code>\d)\)(?P<local_first_half>\d{4})-(?P<local_second_half>\d{4})') 
      missing_hyphen = re.compile(r'^\+(?P<intl_prefix>\d{2})\(0(?P<area_code>\d)\)(?P<local_first_half>\d{4})(?P<local_second_half>\d{4})') 
      if standard_format.search(self.telephoneNumber): 
       result = standard_format.search(self.telephoneNumber) 
       return '0' + result.group('area_code') + result.group('local_first_half') + result.group('local_second_half'), '' 
      elif extra_zero.search(self.telephoneNumber): 
       result = extra_zero.search(self.telephoneNumber) 
       return '0' + result.group('area_code') + result.group('local_first_half') + result.group('local_second_half'), 'Extra zero in area code - ask user to remediate. ' 
      elif missing_hyphen.search(self.telephoneNumber): 
       result = missing_hyphen.search(self.telephoneNumber) 
       return '0' + result.group('area_code') + result.group('local_first_half') + result.group('local_second_half'), 'Missing hyphen in local component - ask user to remediate. ' 
      else: 
       return '', "Number didn't match recognised format. Original text is: " + self.telephoneNumber 

class Employees: 
    def __init__(self, import_list): 
     self.employee_list = []  
     for employee in import_list: 
      self.employee_list.append(Employee(employee)) 

    def clean_all_phone_numbers(self): 
     for employee in self.employee_list: 
      #Should we just set this directly in Employee.clean_phone_number() instead? 
      employee.PHFull, employee.PHFull_message = employee.clean_phone_number() 

    # Hmm, the search is O(n^2) - there's probably a better way of doing this search? 
    def lookup_all_supervisors(self): 
     for employee in self.employee_list: 
      if employee.hrreportsto is not None and employee.hrreportsto != '': 
       for supervisor in self.employee_list: 
        if supervisor.hrid == employee.hrreportsto: 
         (employee.SupervisorEmail, employee.SupervisorFirstName, employee.SupervisorSurname) = supervisor.mail, supervisor.givenName, supervisor.sn 
         break 
       else: 
        (employee.SupervisorEmail, employee.SupervisorFirstName, employee.SupervisorSurname) = ('Supervisor not found.', 'Supervisor not found.', 'Supervisor not found.') 
      else: 
       (employee.SupervisorEmail, employee.SupervisorFirstName, employee.SupervisorSurname) = ('Supervisor not set.', 'Supervisor not set.', 'Supervisor not set.') 

    #Is thre a more pythonic way of doing this? 
    def print_employees(self): 
     for employee in self.employee_list: 
      print(employee.__dict__) 

if __name__ == '__main__': 
    db_employees = Employees(import_gd_dump()) 
    db_employees.clean_all_phone_numbers() 
    db_employees.lookup_all_supervisors() 
    #db_employees.print_employees() 
    write_gd_formatted(db_employees) 

Во-первых, мой преамбула вопрос, вы можете увидеть что-нибудь по существу неправильно с вышеизложенным, либо из конструкции класса или Python точка-обзора? Является ли логика/дизайн звуком?

Во всяком случае, к специфике:

1. Объект Employees имеет метод, clean_all_phone_numbers(), который вызывает clean_phone_number() на каждом Employee объекта внутри него. Это плохой дизайн? Если да, то почему? Также, как я называю lookup_all_supervisors() плохой?
2. Первоначально, я обернул метод clean_phone_number() и lookup_supervisor() в одной функции, с одним внутри него внутри. clean_phone_number - это O (n), я считаю, lookup_supervisor - это O (n^2) - нормально ли он разбивает его на две петли вроде этого?
3. В clean_all_phone_numbers() я зацикливаюсь на объектах Employee и задает их значения с использованием return/assign - должен ли я устанавливать это внутри clean_phone_number()?

Есть также несколько вещей, которые меня сортируют из-за взлома, не уверен, что они плохие практики - например, print_employee() и gd_formatted() оба используют __dict__, а конструктор для Employee использует setattr() для преобразования словаря в атрибуты экземпляра.

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

Приветствия, Виктор

Answer 1

Выглядит хорошо для меня. Отличная работа. Как часто вы собираетесь запускать этот скрипт? Большинство ваших вопросов являются спорными, если это одноразовая вещь.

1. Мне нравится, как Employees.cleen_all_phone_numbers() делегатов Employee.clean_phone_number()
2. Вы действительно должны быть с помощью индекса (словарь) здесь. Вы можете проиндексировать каждого сотрудника по номеру hrid при создании их в O(n), а затем посмотреть их в O(1).
   • Но только делать это, если вы когда-нибудь, чтобы запустить сценарий снова ...
   • Просто попасть в привычку использовать словари. Они безболезненны и облегчают чтение кода. Всякий раз, когда вы пишете метод lookup_*, вы, вероятно, просто хотите индексировать словарь.
3. не уверен. Мне нравится явно устанавливать состояние, но на самом деле это плохой дизайн - clean_phone_number() должен это сделать, сотрудники должны нести ответственность за свое собственное состояние.

Answer 2

вы должны закрыть файлы после их прочтения Я предлагаю переместить все скомпилированные Ре карапуз он верхнего уровня (в противном случае вы собираете их каждый вызов) если self.telephoneNumber не None или self.telephoneNumber == «»: cen быть легко rewrittent как если бы не self.telephoneNumber