python - Redundant code in Django models.py. How do I improve it? -
i trying create task list each task having datetime attribute. tasks needs in order t_created
being first , t_paid
being last. order shown in step_datetime
. description each tasks in steps
.
i have 2 methods all_steps
, next_step
shows task list information. 2 methods need display name of user_created
, variable won't defined until methods called. that's why doing string replace
method.
i feel repeating code lot, , want follow dry principle of django. there way improve code?
here full code:
class order( models.model ) : def __unicode__( self ) : return unicode( self.id ) def comments_count( self ) : return ordercomment.objects.filter( order = self.id ).count() def all_steps( self ) : user = self.user_created.first_name steps = [] step_datetime = [ self.t_created, self.t_action, self.t_followup_one, self.t_vendor_appt_one, self.t_vendor_appt_two, self.t_work_done, self.t_followup_two, self.t_paid, ] ( i, step ) in enumerate( self.steps ) : steps.append( ( step_datetime[ ], step.replace( '<user_created>', user ), ) ) return steps def next_step( self ) : user = self.user_created.first_name step = 0 if self.t_action none : step = 0 elif self.t_followup_one none : step = 1 elif self.t_vendor_appt_one none : step = 2 elif self.t_vendor_appt_two none : step = 3 elif self.t_work_done none : step = 4 elif self.t_followup_two none : step = 5 elif self.paid none : step = 6 return str( step ) + ": " + self.steps[ step ].replace( '<user_created>', user ) steps = [ "review, either approve or reject order.", "follow <user_created>", "contact vendor quote , arrange appointment <user_created>.", "review quote, (get owner approval), arrange second appointment repairs.", "confirm finished repairs , pay vendor.", "follow again <user_created>", "confirm payment , close order.", ] action_choices = ( ( 'p', 'pending' ), ( 'a', 'approved' ), ( 'r', 'rejected' ), ( 'c', 'closed' ), ) user_created = models.foreignkey( user, related_name = 'user_created', verbose_name = 'created by' ) user_action = models.foreignkey( user, related_name = 'user_status' , verbose_name = 'action by' , null = true, blank = true ) t_created = models.datetimefield( auto_now_add = true, verbose_name = 'created' ) t_action = models.datetimefield( null = true, blank = true, verbose_name = 'action' ) t_followup_one = models.datetimefield( null = true, blank = true, verbose_name = 'first follow-up' ) t_vendor_appt_one = models.datetimefield( null = true, blank = true, verbose_name = 'first appointment' ) t_vendor_appt_two = models.datetimefield( null = true, blank = true, verbose_name = 'second appointment' ) t_work_done = models.datetimefield( null = true, blank = true, verbose_name = 'work done' ) t_followup_two = models.datetimefield( null = true, blank = true, verbose_name = 'second follow-up' ) t_paid = models.datetimefield( null = true, blank = true, verbose_name = 'paid' ) action = models.charfield( max_length = 1, choices = action_choices, default = 'p' ) quote = models.decimalfield( max_digits = 8, decimal_places = 2, null = true, blank = true ) payment = models.decimalfield( max_digits = 8, decimal_places = 2, null = true, blank = true ) items = models.manytomanyfield( item, null = true, blank = true ) t_modified = models.datetimefield( auto_now = true, verbose_name = 'modified' )
after accepting @dougal's answer. changed of variables around , came this:
def all_steps( self ) : user = self.user_created.first_name return [ ( getattr( self, attr ), task.format( user = user ) ) ( attr, task ) in self.tasks ] def next_step( self ) : user = self.user_created.first_name task_num = next( ( ( i, ( attr, task ) ) in enumerate( self.tasks ) if getattr( self, attr ) none ), none ) if task_num == none : return "done!" else: return "{number}: {task}".format( number = str( task_num + 1 ), task = self.tasks[ task_num ][ 1 ].format( user = user ) ) tasks = ( ( "t_action" , "review, either approve or reject order." ), ( "t_followup_one" , "follow {user}." ), ( "t_vendor_appt_one", "contact vendor quote , arrange appointment {user}." ), ( "t_vendor_appt_two", "review quote, (get owner approval), arrange second appointment repairs." ), ( "t_work_done" , "confirm finished repairs , pay vendor." ), ( "t_followup_two" , "follow again {user}." ), ( "t_paid" , "confirm payment , close order." ), )
adding on @marcin's answer:
you make tuple of property names (say _step_names
@ module level; make @ class level, steps
, or combine 2 tuple of pairs of attributes , names; might little cleaner). also, steps
should tuple, since shouldn't modifiable @ runtime.
then can reduce code to:
def all_steps(self): user = self.user_created.first_name return [(getattr(self, attr), step.replace('<user_created>', user)) attr, step in zip(_step_names, self.steps)] def next_step(self): user = self.user_created.first_name step = next((i i, attr in enumerate(_step_names) if getattr(self, attr) none), none) # assumes python 2.6+ if step == none: return "done!" else: return str(step) + ": " + self.steps[step].replace('<user_created>', user)
if need python 2.4/2.5 compatability, next
line can replaced by
try: step = (i i, attr in enumerate(_step_names) if getattr(self, attr) none).next() except stopiteration: return "done!" return str(step) + ": " + self.steps[step].replace('<user_created>', user)
Comments
Post a Comment