[3.0] Cast objs to string when used as keys

[sbulen]

Fixes #8306

Cannot use the object itself as an array key. Using the __toString() method instead wherever used as a key.

Tests OK.

[live627]

The explicit calls to that magic method makes the code look funny. Since that routine appears to work only with strings, could you instead change new Ip(/* ... */) to either new Ip(/* ... */)->ip or (string) new Ip(/* ... */)?

[sbulen]

Found a few spots I missed.

The explicit calls to that magic method makes the code look funny. Since that routine appears to work only with strings, could you instead change new Ip(/* ... */) to either new Ip(/* ... */)->ip or (string) new Ip(/* ... */)?

I looked into this, & considered using the existing ->expand() method, but then some of the comparisons/dupe checks would fail.

I also considered adding an ->ip() method, but that would be inconsistent with the other classes we have, which explicitly define __toString() for string conversion such as this.

[Sesquipedalian]

Just cast to string. Not only is that the simplest and most concise approach, but it also communicates the most information to developers reading the code later. When we see an object being cast to a string, we know we are getting a string and that the object must implement the Stringable interface, and we know it without needing to look up property types or method return types.

[sbulen]

Done & retested.

It's a nit, but, something bugs me about requiring a type cast, it seems inconsistent, e.g., IP.php has toHex() and toBinary(), but instead of providing a similar toString(), we require a cast instead. Requiring a cast on our own object feels like we missed something to me.

Then again, there aren't magic methods for __hex() & __binary()...

And as long as we're consistent going forward, we're good.

[Sesquipedalian]

Well, it's not that we require
a cast. Calling __toString() works. It's just a style preference that the majority of us share.

Personally, I wish PHP would add some more magic methods for casting to scalar values (e.g. __toInt(), __toFloat(), etc.) so that developers could use this approach more broadly for the reasons stated above.

[jdarwood007]

To be fair, in my opinion, our force casting should be fixed in the future. For 3.0, using force casting is acceptable as rewriting lots to get the app into a app supported by singletons, so we can move to a more modern approach later that uses factories, compartmentalizes logic into smaller replaceable objects or other methods will be easier.

I would see the IP class as having a ToString(), ToInet, ToBinary, etc method to support use cases in our code.

[live627]

I prefer explicit casting because it communicates intent in a familiar way common to C-style languages such as PHP, C, C++, and C#..

…

On Mon, Nov 11, 2024 at 4:14 PM Jeremy D ***@***.***> wrote: To be fair, in my opinion, our force casting should be fixed in the future. For 3.0, using force casting is acceptable as rewriting lots to get the app into a app supported by singletons, so we can move to a more modern approach later that uses factories, compartmentalizes logic into smaller replaceable objects or other methods will be easier. I would see the IP class as having a ToString(), ToInet, ToBinary, etc method to support use cases in our code. — Reply to this email directly, view it on GitHub <#8321 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADJNN7DMXZGBQI5MWYLGAT2AE25HAVCNFSM6AAAAABPTLD4SCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINRZGI3DIOJTGE> . You are receiving this because you commented.Message ID: ***@***.***>