4
\$\begingroup\$

I'm creating tournament management web-app using wagtail and after searching for different solutions on the Internet came to the following:

YEAR_CHOICES = []
for r in range(1999, (datetime.now().year+1)):
    YEAR_CHOICES.append((r,r))

class Staff(models.Model):
    """
    Each team has staff like: Coach, Assistant etc
    """
    name = models.CharField(max_length=128)
    surname = models.CharField(max_length=128)
    position = models.CharField(max_length=128, choices=POSITIONS)

    panels = [
        FieldPanel('name', classname="col6"),
        FieldPanel('surname', classname="col6"),
        FieldPanel('position'),
    ]

    def __str__(self):
        return "{} {}".format(self.name, self.surname)

class Player(models.Model):
    number = models.IntegerField(default=0)
    name = models.CharField(max_length=128)
    surname = models.CharField(max_length=128)
    position = models.CharField(max_length=128, choices=POSITIONS_SP)
    year_of_birth = models.IntegerField(choices=YEAR_CHOICES, default=datetime.now().year)

    panels = [
        FieldPanel('number'),
        FieldPanel('name', classname="col6"),
        FieldPanel('surname', classname="col6"),
        FieldPanel('position'),
        FieldPanel('year_of_birth'),
    ]

    def __str__(self):
        return "{} {}".format(self.name, self.surname)

class PlayerRelatedModel(models.Model):
    """
    Using ParentalKey allows me to add players dynamically.
    """
    player = models.ForeignKey(Player)
    in_team = models.DateField()
    page = ParentalKey('PlayerContract',
        related_name='spieler_in_team')

    panels = [
        FieldPanel('player', classname="col6"),
        FieldPanel('in_team', classname="col6")
    ]

class PlayerContract(ClusterableModel):
    player = models.ForeignKey(Player)
    team = models.ForeignKey('TeamRooster')
    in_team = models.DateField()

    panels = [
        FieldPanel('team'),
        InlinePanel(
            'player_in_team', label="Add player",
            panels=None
        )
    ]

    def __str__(self):
        return "{} in {} seit {}".format(self.player, self.team, self.jahr)

class StaffContract(models.Model):
    player = models.ForeignKey(Staff)
    team = models.ForeignKey('TeamRooster')
    in_team = models.DateField()

    panels = [
        FieldPanel('player'),
        FieldPanel('team'),
        FieldPanel('in_team'),
    ]

    def __str__(self):
        return "{} in {} seit {}".format(self.player, self.team, self.in_team)

class TeamRooster(Page):
    player = models.ManyToManyField(Player, through=PlayerContract, blank=True)
    staff = models.ManyToManyField(Staff, through=StaffContract, blank=True)

    parent_page_types = ['Teams']

    content_panels = [
        FieldPanel('title'),
        FieldPanel('staff'),
        FieldPanel('player')
    ]

    def __str__(self):
        return self.title

class Teams(Page):
    introduction = models.TextField(
        help_text='Text to describe the page',
        blank=True)

    content_panels = Page.content_panels + [
        FieldPanel('introduction', classname="full"),
    ]

    # Can only have TeamRooster children
    subpage_types = ['TeamRooster']

    # Returns a queryset of TeamRooster objects that are live, that are direct
    # descendants of this index page with most recent first
    def get_teams(self):
        return TeamRooster.objects.live().descendant_of(
            self).order_by('-first_published_at')

    # Allows child objects (e.g. TeamRooster objects) to be accessible via the
    # template. We use this on the HomePage to display child items of featured
    # content
    def children(self):
        return self.get_children().specific().live()

    def get_context(self, request):
        context = super(Teams, self).get_context(request)
        context['teams'] = TeamRooster.objects.descendant_of(
            self).live().order_by(
            '-date_published')
        return context

Adding Players to team

enter image description here

I have very little experience in programming I will be happy with any corrections.

And please can someone add tag wagtail to this question?

\$\endgroup\$
  • 1
    \$\begingroup\$ did you write this code yourself, your phrasing suggests that you found this solution as opposed to writing this solution after researching. \$\endgroup\$ – Malachi Apr 17 '18 at 14:49
  • \$\begingroup\$ @Malachi No, basically I took most of the code from other examples. In this code, everything is very simple, but it is not entirely correct. I'll update this code later. \$\endgroup\$ – khashashin Apr 18 '18 at 6:51
4
\$\begingroup\$

Here are few thoughts how the code could be slightly refactored.

The calculation of YEAR_CHOICES is little bit ugly. You could do it more elegant with a list comprehension:

from datetime import datetime

YEARS = [(year, year) for year in range(1999, datetime.now().year+1)]

Notice that I changed also some variable names. Naming is a very important skill.

It is much easier to understand (year, year) for year in range(...) instead of for r in range(...). What does r stand for? The variable names should have some meaning.

You have another constant POSITIONS, which is not revealed in your question. You should be consistent and keep the same name convention throughout your project.

You could suffix all your choice constants with _CHOICES, like:

  • YEAR_CHOICES
  • POSITION_CHOICES

where the suffix would clutter the code and be redundant, or keep the names succinct, like:

  • YEARS
  • POSITIONS

If you have more choice constants, you could eventually put them in their own file. That's a matter of taste and depends on the size of your project. Personally if I have to use choices more frequently in my models, I create an extra file choices.py or constants.py and put all choices there. Then you can import them in the models.py:

from . import choices

and use them like this:

position = models.CharField(max_length=128, choices=choices.POSITIONS)

Here we have another issue. Why is max_length=128? When you use choices, you would like to store a short string in the database and resolve it in the application. An example would be:

(
    ('FW', 'Forward'),
    ('LB', 'Left Back'),
    ('MF', 'Midfielder'),
)

Then you could set max_length=2 and keep your database more performant.

Further you mix the coding style in the ForeignKey fields. Keep this:

team = models.ForeignKey('TeamRooster')

and adjust this:

player = models.ForeignKey(Player)

to:

player = models.ForeignKey('Player')

If you want to upgrade to Django 2, you'll have to pass on_delete to this field. Make your code future-proof and do that now. Even though it is not a mandatory argument, it is highly recommended to specify related_name. Here is an example:

player = models.ForeignKey('Player', on_delete=models.CASCADE, related_name='contracts')

Enclose in quotes the through arguments in the M2M fields, too:

player = models.ManyToManyField(Player, through='PlayerContract', blank=True)

Without knowing your user-story I can't provide further tips about the business logic of your app.

\$\endgroup\$
  • \$\begingroup\$ Thank you, ill use youre corrections in my code, as I sain in the comments this code is not enought correct, Im too busy at the moment maybe later ill update it. \$\endgroup\$ – khashashin May 9 '18 at 8:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.