Python - 将 CSV 转换为对象 - 代码设计

Question

我有一个小脚本，我们用它来读取包含员工的 CSV 文件，并对这些数据执行一些基本操作。

我们读入数据 (import_gd_dump)，并创建一个 Employees 对象，其中包含一个 Employee 对象列表(也许我应该想一个更好的命名约定...... 。哈哈)。然后我们在 Employees 上调用 clean_all_phone_numbers()，它在每个 Employee 上调用 clean_phone_number()，以及 lookup_all_supervisors()，关于员工。

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()，它在每个 Employee 上调用 clean_phone_number()里面的物体。这是糟糕的设计吗？如果是这样，为什么？另外，我调用 lookup_all_supervisors() 的方式不好吗？
2. 最初，我将 clean_phone_number() 和 lookup_supervisor() 方法包装在一个函数中，其中有一个 for 循环。 clean_phone_number 是 O(n)，我相信 lookup_supervisor 是 O(n^2) - 可以像这样将它分成两个循环吗？
3. 在 clean_all_phone_numbers() 中，我在 Employee 对象上循环，并使用返回/赋值设置它们的值 - 我应该在 clean_phone_number 中设置它吗() 本身？

还有一些我被黑掉的东西，不确定它们是否是不好的做法 - 例如print_employee() 和 gd_formatted() 都使用了 __dict__，Employee 的构造函数使用了 setattr( ) 将字典转换为实例属性。

我会重视任何想法。如果您认为问题太宽泛，请告诉我，我可以分成几个部分重新发布(我只是不想用多个类似的问题污染董事会，这三个问题或多或少是相当紧密相关的)。

干杯，维克多

Answer 1

我觉得不错。做得好。你打算多久运行一次这个脚本？如果这是一次性的事情，那么您的大部分问题都没有实际意义。

1. 我喜欢Employees.cleen_all_phone_numbers()委托(delegate)给Employee.clean_phone_number()的方式
2. 你真的应该在这里使用索引(字典)。在 O(n) 中创建每个员工时，您可以按 hrid 索引他们，然后在 O(1) 中查找他们。
   • 但只有在您必须再次运行脚本时才这样做...
   • 只是养成使用字典的习惯。它们是无痛的，并且使代码更易于阅读。每当您编写方法 lookup_* 时，您可能只想索引字典。
3. 不确定。我喜欢明确设置状态，但这实际上是糟糕的设计 - clean_phone_number() 应该这样做，员工应该对自己的状态负责。