3
\$\begingroup\$

I am using Django multi-table inheritance: Video and Image are models derived from Media. There are two views: video_list and image_list, which are just proxies to media_list. media_list returns images or videos (based on input parameter model) for a certain object, which can be of type Event, Member, or Crag. The view alters its rendering behaviour based on input parameter mode, which can be of value "edit" or "view".

The problem is that I need to ask whether the input parameter model contains Video or Image in media_list so that I can do the right thing. Similar condition is also in helper method media_edit_list that is called from the view.

I don't particularly like it but the only alternative I can think of is to have separate (but almost the same) logic for video_list and image_list and then probably also separate helper methods for videos and images: video_edit_list, image_edit_list, video_view_list, image_view_list. So four functions instead of just two. That I like even less because the video functions would be very similar to the respective image functions. What do you recommend?

Here is extract of relevant parts. I'll also paste the code here:

URLs

url(r'^media/images/(?P<rel_model_tag>(event|member|crag))/(?P<rel_object_id>\d+)/(?P<action>(view|edit))/$', views.image_list, name='image-list')
url(r'^media/videos/(?P<rel_model_tag>(event|member|crag))/(?P<rel_object_id>\d+)/(?P<action>(view|edit))/$', views.video_list, name='video-list')

Views

def image_list(request, rel_model_tag, rel_object_id, mode):
    return media_list(request, Image, rel_model_tag, rel_object_id, mode)


def video_list(request, rel_model_tag, rel_object_id, mode):
    return media_list(request, Video, rel_model_tag, rel_object_id, mode)


def media_list(request, model, rel_model_tag, rel_object_id, mode):
    rel_model = tag_to_model(rel_model_tag)
    rel_object = get_object_or_404(rel_model, pk=rel_object_id)

    if model == Image:
        star_media = rel_object.star_image
    else:
        star_media = rel_object.star_video

    media_list = model.get_object_media(rel_object).filter(~Q(id=star_media.id)).order_by('date_added')

    context = {
        'media_list': media_list,
        'star_media': star_media,
    }

    if mode == 'edit':
        return media_edit_list(request, model, rel_model_tag, rel_object_id, context)

    return media_view_list(request, model, rel_model_tag, rel_object_id, context)


def media_view_list(request, model, rel_model_tag, rel_object_id, context):
    if request.is_ajax():
        context['base_template'] = 'boxes/base-lite.html'
    return render(request, 'media/list-items.html', context)


def media_edit_list(request, model, rel_model_tag, rel_object_id, context):
    if model == Image:
        get_media_edit_record = get_image_edit_record
    else:
        get_media_edit_record = get_video_edit_record

    media_list = [get_media_edit_record(media, rel_model_tag, rel_object_id) for media in context['media_list']]

    if context['star_media']:
        star_media = get_media_edit_record(context['star_media'], rel_model_tag, rel_object_id)
    else:
        star_media = None

    json = simplejson.dumps({
        'star_media': star_media,
        'media_list': media_list,
    })
    return HttpResponse(json, content_type=json_response_mimetype(request))


def get_image_edit_record(image, rel_model_tag, rel_object_id):
    record = {
        'url': image.image.url,
        'name': image.title or image.filename,
        'type': mimetypes.guess_type(image.image.path)[0] or 'image/png',
        'thumbnailUrl': image.thumbnail_2.url,
        'size': image.image.size,
        'id': image.id,
        'media_id': image.media_ptr.id,
        'starUrl':reverse('image-star', kwargs={'image_id': image.id, 'rel_model_tag': rel_model_tag, 'rel_object_id': rel_object_id}),
    }
    return record


def get_video_edit_record(video, rel_model_tag, rel_object_id):
    record = {
        'url': video.embed_url,
        'name': video.title or video.url,
        'type': None,
        'thumbnailUrl': video.thumbnail_2.url,
        'size': None,
        'id': video.id,
        'media_id': video.media_ptr.id,
        'starUrl': reverse('video-star', kwargs={'video_id': video.id, 'rel_model_tag': rel_model_tag, 'rel_object_id': rel_object_id}),
    }
    return record

Models

class Media(models.Model, WebModel):
    title = models.CharField('title', max_length=128, default='', db_index=True, blank=True)
    event = models.ForeignKey(Event, null=True, default=None, blank=True)
    crag = models.ForeignKey(Crag, null=True, default=None, blank=True)
    member = models.ForeignKey(Member, null=True, default=None, blank=True)
    tagged_in_members = models.ManyToManyField(Member, blank=True, related_name='tagged_in_media_set')
    added_by = models.ForeignKey(Member, related_name='added_images')
    date_added = models.DateTimeField('date added', auto_now_add=True, null=True, default=None, editable=False)

    @classmethod
    def get_object_media(cls, rel_object):
        filter_params = {}

        if rel_object.__class__ == Event:
            filter_params['event'] = rel_object.id
        elif rel_object.__class__ == Member:
            filter_params['member'] = rel_object.id
        elif rel_object.__class__ == Crag:
            filter_params['crag'] = rel_object.id

        return cls.objects.filter(**filter_params).all()



class Image(Media):
    image = ProcessedImageField(upload_to='uploads',
                                processors=[ResizeToFit(width=1024, height=1024, upscale=False)],
                                format='JPEG',
                                options={'quality': 75})
    thumbnail_1 = ImageSpecField(source='image',
                                processors=[SmartResize(width=178, height=134)],
                                format='JPEG',
                                options={'quality': 75})
    thumbnail_2 = ImageSpecField(source='image',
                                #processors=[SmartResize(width=256, height=192)],
                                processors=[ResizeToFit(height=164)],
                                format='JPEG',
                                options={'quality': 75})


class Video(Media):
    url = models.URLField('url', max_length=256, default='')
    embed_url = models.URLField('embed url', max_length=256, default='', blank=True)
    author = models.CharField('author', max_length=64, default='', blank=True)
    thumbnail = ProcessedImageField(upload_to='uploads',
                                processors=[ResizeToFit(width=1024, height=1024, upscale=False)],
                                format='JPEG',
                                options={'quality': 75}, null=True, default=None, blank=True)
    thumbnail_1 = ImageSpecField(source='thumbnail',
                                processors=[SmartResize(width=178, height=134)],
                                format='JPEG',
                                options={'quality': 75})
    thumbnail_2 = ImageSpecField(source='thumbnail',
                                #processors=[SmartResize(width=256, height=192)],
                                processors=[ResizeToFit(height=164)],
                                format='JPEG',
                                options={'quality': 75})

  class Crag(models.Model, WebModel):
      name = models.CharField('name', max_length=64, default='', db_index=True)
      normalized_name = models.CharField('normalized name', max_length=64, default='', editable=False)
      type = models.IntegerField('crag type', null=True, default=None, choices=crag_types)
      description = models.TextField('description', default='', blank=True)
      country = models.ForeignKey('country', null=True, default=None) #TODO: make this not null when db enables it
      latitude = models.FloatField('latitude', null=True, default=None)
      longitude = models.FloatField('longitude', null=True, default=None)
      location_index = FixedCharField('location index', length=24, default='', editable=False, db_index=True) # handled by db, used for marker clustering
      added_by = models.ForeignKey('member', null=True, default=None)
      #route_count = models.IntegerField('route count', null=True, default=None, editable=False)
      date_created = models.DateTimeField('date created', auto_now_add=True, null=True, default=None, editable=False)
      last_modified = models.DateTimeField('last modified', auto_now=True, null=True, default=None, editable=False)
      star_image = models.ForeignKey('Image', null=True, default=None, related_name='star_crags', on_delete=models.SET_NULL)
      star_video = models.ForeignKey('Video', null=True, default=None, related_name='star_crags', on_delete=models.SET_NULL)
\$\endgroup\$
4
\$\begingroup\$

Since posting this question I have rewritten the code completely. However, to solve this particular issue, one could rewrite:

if model == Image:
    star_media = rel_object.star_image
else:
    star_media = rel_object.star_video

to

star_media = model.get_star_media(rel_object)

get_star_media would be class method of Video and Image models (defined as stub in Media) that returns star media of the respective type. Shifting the problem to model layer and using polymorfism is the key. Otherwise, the approach of using one view is alright.

\$\endgroup\$
3
\$\begingroup\$

You could avoid the "is Image or Video" checks in media_list, by passing in some extra arguments, for example in image_list:

def image_list(request, rel_model_tag, rel_object_id, mode):
    get_star_media = lambda rel_object: rel_object.star_image
    return media_list(request, Image, get_star_media, get_image_edit_record, rel_model_tag, rel_object_id, mode)

I added two extra arguments there, the get_star_media function to extract rel_object.star_image and get_image_edit_record. Similarly for video_list:

def video_list(request, rel_model_tag, rel_object_id, mode):
    get_star_media = lambda rel_object: rel_object.star_video
    return media_list(request, Video, get_star_media, get_video_edit_record, rel_model_tag, rel_object_id, mode)

This way you could rewrite the media_list and media_edit_list methods without the Image/Video checks like this:

def media_list(request, model, get_star_media, get_media_edit_record, rel_model_tag, rel_object_id, mode):
    rel_model = tag_to_model(rel_model_tag)
    rel_object = get_object_or_404(rel_model, pk=rel_object_id)
    star_media = get_star_media(rel_object)

    media_list = model.get_object_media(rel_object).filter(~Q(id=star_media.id)).order_by('date_added')

    context = {
        'media_list': media_list,
        'star_media': star_media,
    }

    if mode == 'edit':
        return media_edit_list(request, get_media_edit_record, rel_model_tag, rel_object_id, context)

    return media_view_list(request, model, rel_model_tag, rel_object_id, context)

def media_edit_list(request, get_media_edit_record, rel_model_tag, rel_object_id, context):
    media_list = [get_media_edit_record(media, rel_model_tag, rel_object_id) for media in context['media_list']]

    if context['star_media']:
        star_media = get_media_edit_record(context['star_media'], rel_model_tag, rel_object_id)
    else:
        star_media = None

    json = simplejson.dumps({
        'star_media': star_media,
        'media_list': media_list,
    })
    return HttpResponse(json, content_type=json_response_mimetype(request))

You're passing many arguments to media_view_list that are not used:

def media_view_list(request, model, rel_model_tag, rel_object_id, context):
    if request.is_ajax():
        context['base_template'] = 'boxes/base-lite.html'
    return render(request, 'media/list-items.html', context)

You could simplify by dropping model, rel_model_tag and rel_object_id from the argument list.

\$\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.