[Owncloud] Code review? OC_Image class

Bartek Przybylski bart.p.pl at gmail.com
Sun Jan 1 18:05:10 UTC 2012


'}else{' is not correct if i understand guideline correct

http://owncloud.org/contribute/development-setup/
closing brackets in a sepearate line

2012/1/1 Robin Appelman <icewind1991 at gmail.com>:
> Hi,
>
> constructors and destructors shouldn't return anything.
>
> '}else{' is currently used in oc and as far as I know the "correct"
> way to do it according to our guidelines.
>
> I would rename imageResource() to resource(), so you get
> $image->resource() instead of $image->imageResource() which seems
> redundant to me.
>
> Also, it seems better to me to use imagecreatefrompng/jpg/etc instead
> of imagecreatefromstring(file_get_contents()) which needs to load the
> image in memory twice.
>
> - Robin Appelman
>
>
>
> On Sun, Jan 1, 2012 at 18:27, Bartek Przybylski <bart.p.pl at gmail.com> wrote:
>> 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
>> _______________________________________________
>> Owncloud mailing list
>> Owncloud at kde.org
>> https://mail.kde.org/mailman/listinfo/owncloud



More information about the Owncloud mailing list