First, the good feedback.
Good use of exception blocks, code style, built-ins, standard libs etc..
Although I can't comment on the implementation on the end product, you have
clearly made an effort to use as many of the features offered by Django (and
python) as possible in this project. The fact you are using a decent code
repo (imo), is also a good sign. Though you still have a long way to go
(it's a never ending road lol), this is exceptionally good work for
a beginner.
And here's the criticisms / a few things that stand out..
Firstly:
path = path.replace("..", "")
path = path.replace("..", "")
(repeated 20 odd times)
Try replacing the above with os.path.basename() and os.path.abspath(), I
think a combination of both is what you are looking for (unless I've
mistaken what you were doing)
Secondly,
resp = '<div style="display:inline-block;
position:relative; border:1px solid black;">'
resp += '<img id="image_center-'+str(COUNTER)+'"
src="'+reverse('image.views.image',args=(value.image_path,'width=150&height=150&mode=scale'+extra_parms))+'"
onclick=""/>'
resp += '<img id="image_center_crosshair-'+str(COUNTER)+'"
src="'+reverse('image.views.crosshair')+'" style="position:absolute;
left:0; top:0;" />'
It is *extremely* bad practise to use string concatenation (even when
casting in-line). You should instead be using a sprintf style:
Example:
"""<div class="%s"""" % (
class_name
)
or
"<div class='%s'" % ( class_name )
etc
Also, instead of using string appending (+=),you should instead append them
to a list, then do a string join on the list ("\n".join(resp))
Thirdly, I'm seeing a lack of RequestContext(), you should always include it
(even if you don't use it), makes life easier in the future.
Forthly, you are using a global within your code (global COUNTER). Using
globals within a multi-threaded application is extremely bad, and can/will
introduce undesirable race conditions. And believe me, this won't be the
first or last time a race condition screws you over :)
I see you are also using os.system(cmd), I would recommend looking into
using popen instead.
It might also be worth revisiting the code in a few days/weeks/months,
looking over it, and making a list of things you dislike about it. As ones
experience with any language progresses, their style changes slightly and
they find better ways of doing things. The key to a great programmer, is not
about making mistakes, but about recognising that you made one :) *imo
anyway!*
Hope this helps
Cal
On Sun, Jun 12, 2011 at 2:26 PM, francescortiz <francescor...@gmail.com>wrote:
> Hi,
>
> I developed a django app and I would like to know what you think about
> it. I am relatively new to python and django so I expect more
> criticism than enything else.
>
> Description:
> Django application that provides resizing and thumbnailing for images
> and videos with the ability to set the center of attention, heads
> won't get cut anymore.
>
> Link:
> https://github.com/francescortiz/image
>
> I am thinking of calling it "django-headcut"
>
> Thank you,
>
> Francesc
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django users" group.
> To post to this group, send email to django-users@googlegroups.com.
> To unsubscribe from this group, send email to
> django-users+unsubscr...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/django-users?hl=en.
>
>
--
You received this message because you are subscribed to the Google Groups
"Django users" group.
To post to this group, send email to django-users@googlegroups.com.
To unsubscribe from this group, send email to
django-users+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-users?hl=en.
