Update services/s3.class.php #48

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants

I've changed the get_object_filesize() method.  Originally, the method gets the object.  If the object doesn't exists, it returns 0.
This is problematic in applications I've used the method in.  Getting a filesize of 0 implies that a file was written with 0 bytes (Usually indicating a failure of some kind when creating the original object, or a file that is in progress of being uploaded or created).
Instead, I propose returning -1 for an object that doesn't exist to distinguish a missing file from a 0 byte (corrupted) file.
I also added a check for filesize = -1 to the "friendly_format" if statement to bypass the size_readable method when a file doesn't exists.

Update services/s3.class.php
I've changed the get_object_filesize() method.  Originally, the method gets the object.  If the object doesn't exists, it returns 0.
This is problematic in applications I've used the method in.  Getting a filesize of 0 implies that a file was written with 0 bytes (Usually indicating a failure of some kind when creating the original object, or a file that is in progress of being uploaded or created).
Instead, I propose returning -1 for an object that doesn't exist to distinguish a missing file from a 0 byte (corrupted) file.
I also added a check for filesize = -1 to the "friendly_format" if statement to bypass the size_readable method when a file doesn't exists.

tyd commented Nov 28, 2012

I think returning -1 is kind of messy. A more common way to deal with this is to return boolean false when an object doesn't exist and return 0 for objects with a filesize of 0 bytes.

if( $filesize === false ) // doesn't exist

Yes, that is a good point. I had considered this, but in my mind it seemed like the method's responses should all be of the same type (INT). I suppose the solution that satisfies both lines of reasoning would be to return a whole CFResponse, where isOk() would denote the file existing, and a separate XML node contains the file size. This of course would add overhead to the method.

Contributor

jeremeamia commented Mar 11, 2013

I added this in version 1.6.0 of the SDK. The method returns false if there is a failure. This will allow for === checks and is not really a breaking change since 0 == false. Since we do not accept pull requests directly to this project, I have added you to the contributors document. Thanks!

@jeremeamia jeremeamia closed this Mar 11, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment