[Owncloud] Code review? OC_Image class

[Robin Appelman]

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