patternpythondjangoMinor
Is one view that handles two sibling models a good idea?
Viewed 0 times
ideahandlesviewsiblingonetwothatgoodmodels
Problem
I am using Django multi-table inheritance:
The problem is that I need to ask whether the input parameter
I don't particularly like it but the only alternative I can think of is to have separate (but almost the same) logic for
Here is extract of relevant parts. I'll also paste the code here:
URLs
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:
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(event|member|crag))/(?P\d+)/(?P(view|edit))/
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:
, views.image_list, name='image-list')
url(r'^media/videos/(?P(event|member|crag))/(?P\d+)/(?P(view|edit))/
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:
, 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:
Solution
Since posting this question I have rewritten the code completely. However, to solve this particular issue, one could rewrite:
to
if model == Image:
star_media = rel_object.star_image
else:
star_media = rel_object.star_videoto
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.Code Snippets
if model == Image:
star_media = rel_object.star_image
else:
star_media = rel_object.star_videostar_media = model.get_star_media(rel_object)Context
StackExchange Code Review Q#35103, answer score: 4
Revisions (0)
No revisions yet.