On Mon, Feb 4, 2013 at 5:56 PM, Charles Sprayberry <cspray....@gmail.com> wrote:
> There was a previous discussion on the return value of
> SplFileObject::__toString being
> an alias to SplFileObject::current() but that was about the design decision
> why and not
> really suggesting any kind of change or a technical error. In the
> discussion it was
> established that:
>
> (1) Changing this would be a BC break
> (2) It isn't really a bug in the codebase, the docs were just wrong
>
> The current implementation has a bug in that current() can potentially
> return false, if
> false is returned from the __toString() magic method a catchable fatal
> error occurs.
>
> Code ran on 5.4.11 *nix, Apache 2.2
>
> --TEST--
> Ensuring we get appropriate output for SplFileObject::__toString
> --FILE--
> <?php
>
> $file = fopen('file.txt', 'w');
> fwrite($file, 'A');
> fclose($file);
>
> $FileObject = new SplFileObject('file.txt');
> foreach ($FileObject as $line) {
> echo $FileObject;
> }
>
> echo $FileObject;
> ?>
> --EXPECTF--
> A
>
>
> ---------
> Diff
>
> 002+ Catchable fatal error: Method SplFileObject::__toString() must return
> a string value
> in /path/to/splfileobject_tostring.php on line 12
>
> ---------
>
> I would say that SplFileObject:__toString() being an alias to
> SplFileObject::current()
> makes the use cases for the method very limited and that the current
> implementation is
> mistaken in returning current(). I can think of no use case where
> SplFileObject::__toString()
> should return this value. If need to get the current line you're probably
> iterating and in
> that case should be using foreach() and don't really need to manually take
> care of that.
> Even if there is a use case for manually invoking
> `SplFileObject::current()` you probably
> aren't casting a string in that situation and a more expressive method call
> is better.
>
> (1) Leave the implementation the way it is and simply return a blank string
> when
> current() would return false
>
> (2) Change the implementation to return SplFileInfo::__toString()
>
> (3) Change the implementation to return entire file contents
>
> (4) Return some other unthought of string but remains consistent and logical
>
> I am personally partial to (2) or (3) perhaps in 5.5 but (1) could probably
> happen
> sooner? Thoughts?
I don't really use this particular object, but from a distance I think
something should change. Returning false seems like a bug or design
flaw. I can probably be persuaded for any of options 1-3 though I
prefer them in 2, 1, 3 order.
Looking it up in the helly docs
(http://www.php.net/~helly/php/ext/spl/classSplFileObject.html#a79319d744507f828a752981e6f63bf15)
I see that this was always the intended behavior, though I don't know
why.
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
