[Owncloud] Code review? OC_Image class

Bartek Przybylski bart.p.pl at gmail.com
Sun Jan 1 17:27:59 UTC 2012


howdy!
Thanks for your work for owncloud :) i hope oc_image will fast fit
into current ownCloud source
here are my suggestions:
__constructor, __destructor: should it return any value? if yes, then
you also should return true on success
loadFromFile:119 -> there is no need for checking is given parameter
string, if not then is_file will evaluate to false
line 246: } else { -> } \n else {
maybe add width and height functions ?
mayba add function put(resource, x, y, w, h) which will copy local
resource to given resource on coordins x and y and crop it to h and w
dimentions ?

bartek



2012/1/1 Thomas Olsen <thomas at tanghus.net>:
> I've added the class to the 'oc_image' branch:
>
> https://gitorious.org/~tanghus/owncloud/tanghus-owncloud/blobs/oc_image/lib/image.php
>
> Anyone willing to do a little code review and catch my errors?
>
> On Saturday 31 December 2011 22:26 Thomas Olsen wrote:
>> I've just recently added thumbnails to the contact list in the Contacts app.
>> In the process I made yet another script for resizing/cropping images which
>> means that there are several scripts doing basically the same.
>>
>> After advice from Bart I have started coding an OC_Image class for
>> manipulating images.
>>
>> For now it has only the functionality I need for creating the thumbnails
>> which boils down to:
>>
>> $image = new OC_Image(path or image resource);
>> $image->centerCrop(); // crops image to a square.
>> $image->resize(25); // resizes to max width/height while keeping aspect
>> ratio $image->show();
>>
>> What other methods would be useful? Should it be able to load an image from
>> a string or maybe from OC_Filesystem?
>>
>> Bring on your (useful) ideas :-)
> --
> Med venlig hilsen / Best Regards
>
> Thomas Tanghus
> _______________________________________________
> Owncloud mailing list
> Owncloud at kde.org
> https://mail.kde.org/mailman/listinfo/owncloud



More information about the Owncloud mailing list