[TASK] Add getArrayRepresentation method for testing

[JakeQZ]

Also include a demo of its usage.

In most classes, a stub that throws an exception is 'implemented'.

Part of #1440.

[JakeQZ]

Notes:

• PHPStan does not support recursve types so we have to use mixed at the second level;
• This is primarily a WIP to see if our solution for Add helper debugging/rendering function stub for (unit) tests #1440 is viable.

[oliverklee]

I like this a lot!

A few thoughts:

• I'd prefer it if we kept the demo implementation out of the first PR.
• Let's cover the methods with tests! (I think that checking for the exception class should be sufficient, without also checking the exception message.)
• To keeps things simple for the reader and to allows us to implement the methods one-by-one, I'd prefer it if we only implemented the method in the leaves of the inheritance tree, but not on abstract classes, and if we didn't do parent::getArrayRepresentation calls.
• Maybe we can add a trait ShortClassRepresentation to avoid having to reimplement the reflection class stuff everywhere. Or we write the class name as a literal (which would further simplify things, and which I would suggest).

WDYT?

[coveralls]

coverage: 68.284% (-0.9%) from 69.191%
when pulling 86a006c on test/add-hepler-method
into 0ae1fde on main.

[oliverklee]

And maybe we can add a corresponding PHPStan custom type to the interface.

[JakeQZ]

• I'd prefer it if we kept the demo implementation out of the first PR.

Agree. I only included it as a proof-of-concept to see how it would work in practice.

• Let's cover the methods with tests! (I think that checking for the exception class should be sufficient, without also checking the exception message.)

If I understand correctly, you're suggesting that each class implementing getArrayRepresentation initially has a test method getArrayRepresentationThrows (or suchlike). Then, when we implement the functionality for a specific class, we replace that with a test method that instead confirms it returns the array data expected (for some simple example). Makes sense. Let me know if we're on the same wavelength.

• To keeps things simple for the reader and to allows us to implement the methods one-by-one, I'd prefer it if we only implemented the method in the leaves of the inheritance tree, but not on abstract classes, and if we didn't do parent::getArrayRepresentation calls.

I think I disagree here :)

Many of the abstract classes contain some properties which should be put in the returned array. It should be the responsibility of those classes to do that for their own properties - not have every 'leaf' class repeating the same job.

I concur that this means that when the method is implemented in a 'leaf' class, it will mean that it also implemented (possibly incompletely) for all of its cousins. I think this is workable - the demo shows that it is.

• Maybe we can add a trait ShortClassRepresentation to avoid having to reimplement the reflection class stuff everywhere. Or we write the class name as a literal (which would further simplify things, and which I would suggest).

I also thought of adding a trait for that (but didn't go that far). I'd prefer that to writing the class name as a literal, which seems a bit clunky. Some day we may rename some of the classes; having a string literal to update would be an extra change that may be missed.

And maybe we can add a corresponding PHPStan custom type to the interface.

I'm not seeing how this would help. It seems to be about defining an array type with specific keys. We will have different keys depending on the class of object being represented. Neither does it offer a solution to the recursive type problem.

Look forward to your further thoughts, and 🎄🦌☃️🌟🍷 ... have a good Christmas :)

[oliverklee]

If I understand correctly, you're suggesting that each class implementing getArrayRepresentation initially has a test method getArrayRepresentationThrows (or suchlike). Then, when we implement the functionality for a specific class, we replace that with a test method that instead confirms it returns the array data expected (for some simple example). Makes sense. Let me know if we're on the same wavelength.

Yes, exactly. :-)

As for the naming, maybe we can use getArrayRepresentationThrowsException.

[oliverklee]

I think I disagree here :)

Many of the abstract classes contain some properties which should be put in the returned array. It should be the responsibility of those classes to do that for their own properties - not have every 'leaf' class repeating the same job.

Okay, I can live with having the method in abstract classes as well. :-)

Merry Christmas to you, too! 🎄

[oliverklee]

As this is the POC (as I understand it), should I create the non-POC version from it, or are you planning to do that?

[JakeQZ]

You're welcome to. I'm away for a couple more days without access to GitHub (can't sign in from phone due to 2FA so hope this message gets through).

[JakeQZ]

You're welcome to. I'm away for a couple more days without access to GitHub (can't sign in from phone due to 2FA so hope this message gets through).

Am back now. I think we've agreed the way forward. One of us should create the next PR for actual merging, whether based on this or a fresh one. But we should avoid duplication of effort... ;)

[oliverklee]

One of us should create the next PR for actual merging, whether based on this or a fresh one. But we should avoid duplication of effort... ;)

I'm currently still focusing on my end-of year "thinking about my life" ritual and won't work on this until including January 1st. If you want to work on this during this time, please go ahead. (I'll be able to do the reviews, though.)