Browse files

Fix use of namedtuples

  • Loading branch information...
1 parent 513facb commit 5487d3c9568cffa006a4cc5d83c753cf4cdbd6b6 @paltman paltman committed Apr 18, 2012
Showing with 14 additions and 8 deletions.
  1. +14 −8 waitinglist/models.py
View
22 waitinglist/models.py
@@ -21,25 +21,31 @@ class Meta:
verbose_name_plural = _("waiting list entries")
+Member = collections.namedtuple("Member", ["email", "signup_code", "user", "invited"])
@brosner
Pinax Project member
brosner added a line comment Apr 18, 2012

Can you explain why changing the scope of where Member is defined improves things? I definitely think it should be outside the for loop and that the constructor should be used, but I want more detail on the scope change.

@paltman
Pinax Project member
paltman added a line comment Apr 18, 2012

I never claimed that changing the scope improves anything.

@brosner
Pinax Project member
brosner added a line comment Apr 18, 2012

Let me rephrase it: what was the reason the change was made?

@paltman
Pinax Project member
paltman added a line comment Apr 18, 2012

In fixing the namedtuple usage, I realized that there was no reason for this to be created in every iteration or even every call to members, so I put it at the module level. I don't really care where it lives scope-wise just thought module level would be best.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
+
class Cohort(models.Model):
name = models.CharField(_("name"), max_length=35)
created = models.DateTimeField(_("created"), default=timezone.now, editable=False)
def members(self):
members = []
- for scc in self.signupcodecohort_set.select_related("signup_code"):
- member = collections.namedtuple("Member", ["email", "signup_code", "user", "invited"])
- member.email = scc.signup_code.email
- member.signup_code = scc.signup_code
- member.invited = bool(scc.signup_code.sent)
+ for scc in self.signupcodecohort_set.select_related():
@brosner
Pinax Project member
brosner added a line comment Apr 18, 2012

How is removing an argument from the select_related call fixing the use of namedtuples?

@paltman
Pinax Project member
paltman added a line comment Apr 18, 2012

It isn't.

@brosner
Pinax Project member
brosner added a line comment Apr 18, 2012

Sigh. Oh well, can't write every commit.

@paltman
Pinax Project member
paltman added a line comment Apr 18, 2012

It's obvious it was mistakenly added when building up the commit. rebase it if you wish.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
try:
scr = SignupCodeResult.objects.get(signup_code=scc.signup_code_id)
except SignupCodeResult.DoesNotExist:
- member.user = None
+ user = None
else:
- member.user = scr.user
- members.append(member)
+ user = scr.user
+ members.append(
+ Member(
+ scc.signup_code.email,
+ scc.signup_code,
+ user,
+ bool(scc.signup_code.sent)
+ )
+ )
return members
def member_counts(self):

0 comments on commit 5487d3c

Please sign in to comment.