pradosoft / prado

Prado - Component Framework for PHP
Other
187 stars 70 forks source link

TMultiList and TMultiMap for accessing multi dimensional arrays. #958

Open belisoful opened 1 year ago

belisoful commented 1 year ago

I'm reviewing Yii2 BaseArrayHelper and comparing it to what we have. Their BaseArrayHelper is basically a replacement for TMap. It adds a few things that we don't do, like various ways of ordering, indexing, and organizing arrays, merging, sorting, determining the type of array (associative or indexed), and some html functions. However, there are some things we can learn and do better.

// maybe these introspective methods as well. these use a common modality. Even if custom made by hand, the code would look like Yii2 (that's what I mean by common modality). public function getIsAssociative (bool $isAllString = true):bool; public function getIsIndexed(bool $consecutive = false):bool;



- TList::removeAllItem to remove all of an item, to match TMap::removeItem. an array of indices removed is returned.  
   or maybe `remove` should remove all of an item rather than just the first instance,  that would be more consistent and parallel yii2.  
   @ctrlaltca Do you have any insight into what to do here?  should we update the `remove` function or add a new `removeAllItem` function?
    If we update the existing, it would return false if not found, the index if found, and an array if multiple are found.  I like updating the existing function to remove all an item rather than having a second function.  It's more consistent with expectation.

- TMap and TList could be updated such that their itemAt, add/insertAt, remove[At], indexOf accepts "." for traversal of the array and returning, setting, and unsetting of a sub-array or item-object property.    keyOf and indexOf could, upon an optional parameter, become recursive and search sub-arrays, returning the path.  This is something I am looking at as a function of TEXIF to traverse its tree and traversal of JPEG metadata (IPTC, EXIF, XMP) accessibly via "EXIF.GPSInfo.GPSLatitude".  This would be easier if it was already built into the TList/TMap class.

 The TList and TMap index/key could act like this "0.10.mapkeyitem" to get, and set an item in the structure.   and "0.10.mapkeyitem.propertyA" to get and set internal properties.
   @ctrlaltca Any thoughts on implications of this functionality in general?  Do you like the idea?  good?  bad?  important?  low value?  does it add any value?  what about "competing" with yii[2/3]?   

` $map->add('0.10.mapkey.propertya', $value)` may seem odd instead of `$map[0][10]['mapkey']->setPropertyA($value)`, but this is the intended use case:
`$list['0.10.mapkey.propertya'] = $value;`

I suggest that when adding, a special end index "NULL" be used for inserting at the end of the map/list.
In this way, $map['key'][] = $value becomes $map['key.NULL'] = $value 

While this could be implemented as a subclass of TMap and TList, like TMapTree and TListTree, my thinking is that adding this to the core would be beneficial in the long run.  Having these functions on a basic array is awesome and why I was looking at it to begin with.

there needs to be regression and unit tests for this as well, of course.

This is a low priority enhancement.
ctrlaltca commented 1 year ago

TMap currently is a key based unordered map, backed by a php array. As such, keys are unique but values can be duplicated. Trying to sum up the proposed improvements:

belisoful commented 1 year ago

Thank you for your insight, esp the last point.

some clarification and subtlety:

belisoful commented 1 year ago

Also, the way PNG does multidimensionality is with a ":" character instead, which would be both more compatible (likely) and more confusing. I think making the character a choice of the subclass would be ideal. This way a TPNGFileMetaData type class could override the '.' and use the ':' character instead.

Application::Paramete might be able to use the new TMap multidimensionality.

belisoful commented 1 year ago

It is looking like the multidimensional access should be its own TMultiMap class and TMultiList class.

belisoful commented 1 year ago

@ctrlaltca Is TList::remove only removing one Item, rather than all of the item, the proper result of the function?

obviously if one item is found, only return it's index and remove (backward compatibility), but multiple item? maybe remove should operate on all placements of an item in the list and return the [key => item, key2 => item] if multiple are found?

belisoful commented 1 year ago

I have an interesting idea.... Instead of multidimensional functionality be built into TList and TMap with a switch (default off? eh?), or new classes TMultiMap/TMultiList, What if it were a behavior to attach to TMap and TList?

Prado\Collections\TMultiDimensionalBehavior

It should work on all TList and TMap, so some regression would be needed for the more advanced Collection classes.

This would continue the integration of behaviors into Collections.

I'm still doing research into which modality would be "best." If you have input, please, your guidance has been of the greatest contribution each time.

<./ important >

Just FYI, The reason for multidimensional array capability is for accessing image metadata. I'd like to get this functional:

$author = $imageMeta['IPTC:ByLine'];
$imageMeta['IPTC:1#90'] = 'UTF-8';
$latitude = $imageWebPMeta['EXIF:GPSInfo:GPSLatitude'];
$imagePNGMeta['XMP:Keyword:NULL'] = 'new appended keyword';
$numKeywords = $imagePNGMeta['XMP:Keyword:count()'] // special keyword for counting arrays?
$imageJPEGMeta['EXIF:IPTCNAA'] = $imageMeta['IPTC'];  //. <- IPTC can be stored in EXIF, making things that much more complex.

$imageWebPMeta['EXIF']['GPSInfo']['GPSLatitude'] === $imageWebPMeta['EXIF:GPSInfo:GPSLatitude'];

While a chain of PHP array access is ok, having string access to arrays and properties opens up uniformity and configurability to accessing multi-dimensional array data in a more user friendly and configuration friendly way.

PHPs support is extremely poor for creating, read, updating, deleting, and saving metadata and metadata within images. for example, PHP's getImageSize returns the EXIF data from a JPEG but if XMP is saved in the same tag, it is not retrievable. EXIF overwrites the XMP as only one metadata app tag is "enabled" for getImageSize metadata. Once this data is retrieved (or not, in the case of EXIF overloading XMP in a double APP1 circumstance), there is no function to write new metadata to a JPEG.

It is not clear what formats getImageSize supports. clearly JPEG. but webp meta? and/or PNG meta? it could be tested, but it should already be clear. jpeg only? iptcparse is crap and there is no method to go back from its outputted array back to binary for saving. The "open source" comment is good enough but needs to be better, proofed and tested.

So JPEG needs a better read/write metadata functions. but then so does PNG and WebP... and if IPTC is getting better support, in comes EXIF and XMP. really good support for image metadata, I envision, is part of the new asset manager.

JPEG, PNG, and WebP should have a Multi-dimensional array access of their respective metadata for configuration. I envision image filters selecting on meta data properties coming down the line.

belisoful commented 1 year ago

I'm leaning towards a TMultiMap and TMultiList again after imagineering other ideas.

belisoful commented 1 year ago

In working on Image file metadata reading and updating and the meta data (iptc, exif, xmp), I found a bug in PHPStan.

I think that's some kind of GitHub badge of something. maybe an honorary badge of "too much work"? The issue report: https://github.com/phpstan/phpstan/issues/9341

The bug https://phpstan.org/r/7304abb9-505e-40e3-b8cc-1d340f4e802b