我有一个小脚本,我们用来读取包含员工的CSV文件,并对该数据执行一些基本操作.
我们读入数据(import_gd_dump),并创建一个Employees包含对象列表的Employee对象(也许我应该想到一个更好的命名约定...大声笑).然后,我们调用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观点来看,你能否从上面看到任何内在的错误？逻辑/设计是否合理？
无论如何,具体来说:
Employees对象有一个方法,clean_all_phone_numbers()它调用其中的clean_phone_number()每个Employee对象.这是不好的设计吗？如果是这样,为什么？还有,我打电话的方式lookup_all_supervisors()不好吗？clean_phone_number()and和包装lookup_supervisor()在单个函数中,其中包含一个for循环.clean_phone_number是O(n),我相信,lookup_supervisor是O(n ^ 2) - 可以将它分成两个这样的循环吗？clean_all_phone_numbers(),我循环Employee对象,并使用返回/赋值设置它们的值 - 我应该在内部设置clean_phone_number()它吗？还有,我是排序的几件事情出来砍死,如果不知道他们是不好的做法-比如print_employee()和gd_formatted()都使用__dict__,并构造了Employee用途setattr()转换的字典为实例属性.
我非常重视任何想法.如果您认为问题太广泛,请告诉我,我可以将其重新分配(我只是不想污染具有多个类似问题的董事会,而且这三个问题或多或少相当紧密相关).
干杯,维克多
对我来说看起来不错。好工作。您打算多久运行一次该脚本？如果这是一次性的事情，那么您的大多数问题都毫无意义。
Employees.cleen_all_phone_numbers()我喜欢代表们的方式Employee.clean_phone_number()hrid在中创建员工的时间为每个员工建立索引O(n)，然后在 中查找他们O(1)。
lookup_*你可能只想索引一个字典。clean_phone_number()应该这样做，员工应该对自己的状态负责。