1
\$\begingroup\$

I'm building a simple Django app for reviews of different objects (restaurants, car services, car wash etc).

I started with the app, but soon I faced first problem. Every object has features (but every type of object has different features).

For example:

Restaurants have garden, playground, seats, type of kitchen, etc. car washes have external cleaning, internal cleaning etc. So I started to build a typical DB implementation with ManyToMany tables, but then I found Django Model Inheritance, so I implement it in my APP. After that I faced other problem, about getting all objects from some category (only restaurants, only car services etc..). So I changed model logic from Django Model Inheritance to PolymorphicModel, the core of the object is almost ready, but I have some doubts, I will post the code, then few words about my doubts:

models.py:

from django.db import models
from users.models import ProfileUser
from django.utils import timezone
from polymorphic.models import PolymorphicModel

# Create your models here.

class City(models.Model):
    name = models.CharField(max_length=20)

    def __str__(self):
        return f"{self.name}"

class Object(PolymorphicModel):
    author = models.ForeignKey(ProfileUser, on_delete=models.CASCADE, db_column='author')
    title = models.CharField(max_length=300)
    city = models.ForeignKey(City, on_delete=models.CASCADE)
    address = models.CharField(max_length=300)
    phone = models.CharField(max_length=20, default='')
    email = models.CharField(max_length=100, default='')
    site = models.CharField(max_length=100, default='')
    facebook = models.CharField(max_length=100, default='')
    instagram = models.CharField(max_length=100, default='')
    content = models.TextField()
    rating = models.IntegerField(default=10)
    created_date = models.DateTimeField(default=timezone.now)
    approved_object = models.BooleanField(default=False)
    admin_seen = models.BooleanField(default=False)

    def __str__(self):
        return f"{self.title}" 


class Restaurant(Object):
    seats = models.IntegerField()
    bulgarian_kitchen = models.BooleanField(default=False)
    italian_kitchen = models.BooleanField(default=False)
    french_kitchen = models.BooleanField(default=False)
    category_en_name = models.CharField(max_length=100, default='restaurants')
    category_bg_name = models.CharField(max_length=100, default='Ресторанти')
    bg_name = models.CharField(max_length=100, default='Ресторант')
    is_garden = models.BooleanField(default=False)
    is_playground = models.BooleanField(default=False)


class SportFitness(Object):
    is_fitness_trainer = models.BooleanField(default=False)
    category_en_name = models.CharField(max_length=100, default='sportfitness')
    category_bg_name = models.CharField(max_length=100, default='Спорт и фитнес')
    bg_name = models.CharField(max_length=100, default='Спорт и фитнес')


class CarService(Object):
    is_parts_clients = models.BooleanField(default=False)
    category_en_name = models.CharField(max_length=100, default='carservice')
    category_bg_name = models.CharField(max_length=100, default='Автосервизи')
    bg_name = models.CharField(max_length=100, default='Автосервиз')


class BeautySalon(Object):
    is_hair_salon = models.BooleanField(default=False)
    is_laser_epilation = models.BooleanField(default=False)
    category_en_name = models.CharField(max_length=100, default='beautysalon')
    category_bg_name = models.CharField(max_length=100, default='Салони за красота')
    bg_name = models.CharField(max_length=100, default='Салон за красота')


class FastFood(Object):
    is_pizza = models.BooleanField(default=False)
    is_duner = models.BooleanField(default=False)
    is_seats = models.BooleanField(default=False)
    category_en_name = models.CharField(max_length=100, default='fastfood')
    category_bg_name = models.CharField(max_length=100, default='Бързо хранене')
    bg_name = models.CharField(max_length=100, default='Бързо хранене')

urls.py:

from django.urls import path
from . import views

urlpatterns = [
    path('user/<int:pk>/', views.UserObjectsView.as_view(), name='user-objects'),
    path('add/<str:category>/', views.add_object, name='add-object'),
    path('<str:category>/<int:pk>/<int:page_num>/', views.show_object, name='show-object'),
    path('all/<str:category>/', views.show_all_objects, name="show-all-objects"),
]

views.py:

from django.shortcuts import render, redirect
from django.views import generic
from objects.models import Object, ProfileUser, Comment, Restaurant, SportFitness, CarService, BeautySalon, FastFood, CarWash, Fun, Other
from .forms import RestaurantForm, SportFitnessForm, BeautySalonForm, CarServiceForm, CarWashForm, FastFoodForm, FunForm, OtherForm, CommentForm, UploadForm
from django.contrib import messages
from django.db.models import Avg
from django.apps import apps
from django.core.paginator import Paginator
from django.http import JsonResponse

class AllObjects(generic.ListView):
    queryset = Object.objects.all()
    template_name = 'show_all_objects.html'

class UserObjectsView(generic.ListView):
    template_name = 'user_objects.html'

    def get_queryset(self):
        user_id = self.kwargs['pk']
        return Object.objects.filter(author = user_id)

def add_object(request, category):
    if not request.user.is_authenticated:
        messages.info(request, 'За да добавите нов Обект, трябва да сте регистриран потребител!')
        return redirect('account_login')

    if category == 'restaurants':
        form = RestaurantForm(request.POST or None)
    elif category == 'sportfitness':
        form = SportFitnessForm(request.POST or None)
    elif category == 'carservice':
        form = CarServiceForm(request.POST or None)
    elif category == 'beautysalon':
        form = BeautySalonForm(request.POST or None)
    elif category == 'fastfood':
        form = FastFoodForm(request.POST or None)
    elif category == 'carwash':
        form = CarWashForm(request.POST or None)
    elif category == 'fun':
        form = FunForm(request.POST or None)
    elif category == 'other':
        form = OtherForm(request.POST or None)

    upload_form = UploadForm(request.POST or None)
    if form.is_valid():
        obj = form.save(commit=False)
        obj.author = ProfileUser.objects.get(user=request.user)
        obj.save()
        messages.success(request, 'Успешно добавихте нов Обект, може да видите вашите обекти във вашия профил!')
        return redirect('home')


    context = {
        'form': form,
        'upload_form': upload_form
    }

    return render(request, "add_object.html", context)

def show_object(request, category, pk, page_num):
    categories = {'restaurants' : 'Restaurant', 'sportfitness' : 'SportFitness', 'carservice' : 'CarService', 'beautysalon' : 'BeautySalon', 'fastfood' : 'FastFood', 'carwash' : 'CarWash', 'fun' : 'Fun', 'other' : 'Other'}
    obj = apps.get_model('objects', categories[category]).objects.get(id=pk)

    if request.method == 'POST':
        data = {'error': False, 'error_message': ''}
        user = request.user
        author = ProfileUser.objects.get(user=user)
        comment = Comment()
        object = Object.objects.get(id=pk)
        comment.object = obj
        comment.author = author
        comment.content = request.POST.get('content')
        comment.rating = request.POST.get('rating')
        rating = Comment.objects.filter(object_id=pk).aggregate(Avg('rating'))['rating__avg']
        object.rating = rating
        data['rating'] = rating
        comment.save()
        object.save()

        return JsonResponse(data)

    form = CommentForm()
    reviews_count = Comment.objects.filter(object_id=pk).count()
    rating = Comment.objects.filter(object_id=pk).aggregate(Avg('rating'))['rating__avg']
    comments_list = Comment.objects.all()
    paginator = Paginator(comments_list, 2)
    comments = paginator.get_page(page_num)

    context = {
        'form': form,
        'object': obj,
        'reviews_count': reviews_count,
        'rating': rating,
        'category': category,
        'comments': comments,
        'page_num': page_num,
    }

    return render(request, "show_object.html", context)

def show_all_objects(request, category):
    categories = {'restaurants' : 'Restaurant', 'sportfitness' : 'SportFitness', 'carservice' : 'CarService', 'beautysalon' : 'BeautySalon', 'fastfood' : 'FastFood', 'carwash' : 'CarWash', 'fun' : 'Fun', 'other' : 'Other'}
    if category == 'restaurants':
        objects = Object.objects.instance_of(Restaurant)
    elif category == 'sportfitness':
        objects = Object.objects.instance_of(SportFitness)
    elif category == 'carservice':
        objects = Object.objects.instance_of(CarService)
    elif category == 'beautysalon':
        objects = Object.objects.instance_of(BeautySalon)
    elif category == 'fastfood':
        objects = Object.objects.instance_of(FastFood)
    elif category == 'carwash':
        objects = Object.objects.instance_of(CarWash)
    elif category == 'fun':
        objects = Object.objects.instance_of(Fun)
    elif category == 'other':
        objects = Object.objects.instance_of(Other)

    context = {
        'object_list': objects,
        'category': category,
    }

    return render(request, 'show_all_objects.html', context)

Template, where List all category and client can choose a category:

<div class="container_categories_box margin-top-5 margin-bottom-30">
  <a href="{% url 'show-all-objects' category='restaurants' %}" class="utf_category_small_box_part"> <i class="im im-icon-Chef"></i>
    <h4>Ресторантии</h4>
    <span>22</span>
  </a>
  <a href="{% url 'show-all-objects' category='sportfitness' %}" class="utf_category_small_box_part"> <i class="im im-icon-Dumbbell"></i>
    <h4>Спортни и фитнес</h4>
    <span>15</span>
  </a>
  <a href="{% url 'show-all-objects' category='carservice' %}" class="utf_category_small_box_part"> <i class="im im-icon-Car-Wheel"></i>
    <h4>Автосервизи</h4>
    <span>05</span>
  </a>
  <a href="{% url 'show-all-objects' category='beautysalon' %}" class="utf_category_small_box_part"> <i class="im im-icon-Gemini-2"></i>
    <h4>Салони за красота</h4>
    <span>12</span>
  </a>
  <a href="{% url 'show-all-objects' category='fastfood' %}" class="utf_category_small_box_part"> <i class="im im-icon-Hamburger"></i>
    <h4>Бързо хранене</h4>
    <span>08</span>
  </a>
</div>

Okay, this is the most important part from the code, My doubts are:

  1. Templates: category this is hardcoded:

    • {% url 'show-all-objects' category='restaurants' %} - restaurants
    • {% url 'show-all-objects' category='sportfitness' %} - sportfitness (etc..)
  2. These if else in views.py:

    if category == 'restaurants': form = RestaurantForm(request.POST or None) elif category == 'sportfitness': form = SportFitnessForm(request.POST or None) elif category == 'carservice': form = CarServiceForm(request.POST or None) elif category == 'beautysalon': form = BeautySalonForm(request.POST or None) elif category == 'fastfood': form = FastFoodForm(request.POST or None) elif category == 'carwash': form = CarWashForm(request.POST or None) elif category == 'fun': form = FunForm(request.POST or None) elif category == 'other': form = OtherForm(request.POST or None)

and this one:

if category == 'restaurants':
            objects = Object.objects.instance_of(Restaurant)
        elif category == 'sportfitness':
            objects = Object.objects.instance_of(SportFitness)
        elif category == 'carservice':
            objects = Object.objects.instance_of(CarService)
        elif category == 'beautysalon':
            objects = Object.objects.instance_of(BeautySalon)
        elif category == 'fastfood':
            objects = Object.objects.instance_of(FastFood)
        elif category == 'carwash':
            objects = Object.objects.instance_of(CarWash)
        elif category == 'fun':
            objects = Object.objects.instance_of(Fun)
        elif category == 'other':
            objects = Object.objects.instance_of(Other)

Generally I'm not quite sure, that this is the "right way" to implement this app. I'm open to any questions.

\$\endgroup\$
2
\$\begingroup\$

Please, for the love of goodness, don't create your own superclass named Object, be more specific. The lower-case object is already a defined type and this is just confusing.

The multi-language name and category approach isn't scalable. I needed a similar solution recently, and what I tried was something like the following:

class Language(models.Model):
    name = models.CharField(max_length=2)  # Create one "en" instance and one "bg" instance

class Text(models.Model):
    text = models.TextField()
    language = models.ForeignKey('Language', on_delete=models.CASCADE)

class Category(models.Model):
    names = models.ManyToManyField('Text')

    def name(self, language='en'):
        try:
            return self.names.get(language=language).text
        except Text.DoesNotExist:
            return self.names.get(language__name='en').text  # or whatever your default language is

You can then keep a language for each user, and when you want to look up the name, just do category.name(request.user.language). You can thus easily add new languages and tweak translations in the future just by altering the database.

Anyway, back to reviewing your actual code. Since all your tags are simple boolean fields, it would have been much cleaner to keep these categories in their own table, create a many-to-many mapping, and simply do existence checks into your database. To make the types of businesses easily editable and addable, you could create a type system in your database itself, creating a kind of "virtual" inheritance system.

class Business(models.Model):
    name = models.ManyToManyField('Text')
    author = ...
    ...
    kind = models.ForeignKey('BusinessType', null=True, on_delete=models.SET_NULL)
    tags = models.ManyToManyField('Tag')

class BusinessType(models.Model):
    name = models.CharField(max_length=100)  # e.g., 'Restaurant' or 'Sport Fitness'
    possible_tags = models.ManyToManyField('Tag')

class Tag(models.Model):
    base_name = models.CharField(max_length=100)  # e.g. 'bulgarian_kitchen'
    description = models.ManyToManyField('Text')

You still have access to all the same information, but it's all configurable through the database rather than through your code. E.g., to get a restaurant and check if it has an Italian kitchen,

restaurant = Business.objects.get(name__language='en', name__text='YumBob', kind__name='Restaurant')
has_italian_kitchen = restaurant.tags.filter(base_name='italian_kitchen').exists()

or to find all beauty salons that offer laser epilation,

epilators = Business.objects.filter(kind__name='Beauty Salon', tags__name='laser_epilation')

You can also dynamically create forms, etc. for each business type by instantiating a base Form and then adding boolean fields to it for each tag in my_business.kind.possible_tags. And, of course, you could create sidecar tables for particular business types to store additional, non-boolean fields. E.g.,

class RestaurantExtraData(models.Model):
    business = models.OneToOneField('Business', primary_key=True, on_delete=models.CASCADE)
    num_seats = models.IntegerField()

This way, you minimize the amount of special code required for different businesses, but retain the ability to add extra information where necessary. All of this together avoids your need for the stacked if/else statements and the large quantities of hard-coded strings in your views and templates, and allows you to configure everything using data rather than code.

\$\endgroup\$
  • \$\begingroup\$ I will rename it :) "Since all your tags are simple boolean fields, it would have been much cleaner to keep these categories in their own table" well seats are not boolean field, probably in near future there will be and others non boolean fields features. \$\endgroup\$ – MorganFreeFarm Nov 21 '19 at 14:51
  • 1
    \$\begingroup\$ @MorganFreeFarm hence my last suggestion about using one-to-one relationship tables to tag information onto specific rows in the main table, rather than fragmenting the data across a bunch of very similar tables. You could, in theory, hide some of this fragmentation, say by creating a dictionary of models so you can use a string key to get the appropriate model, but the more you can centralize your logic and apply the DRY principle, the more busy-work and hassle you can save yourself. \$\endgroup\$ – scnerd Nov 21 '19 at 14:57
  • \$\begingroup\$ I see. Based on your opinion, current solution doesn't look so bad at all :D \$\endgroup\$ – MorganFreeFarm Nov 21 '19 at 15:02
1
\$\begingroup\$

I found a pythonic way to use dictonary, instead of if else:

params_map = {
    'restaurants': Restaurant,
    'sportfitness': SportFitness,
    'carservice': CarService,
    'beautysalon': BeautySalon,
    'fastfood': FastFood,
    'carwash': CarWash,
    'fun': Fun,
    'other': Other,
}

objects = Object.objects.instance_of(params_map.get(category))
\$\endgroup\$

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.