Skip to content

Flattened registry mode#65

Closed
Fedik wants to merge 3 commits intojoomla-framework:2.0-devfrom
Fedik:flattened
Closed

Flattened registry mode#65
Fedik wants to merge 3 commits intojoomla-framework:2.0-devfrom
Fedik:flattened

Conversation

@Fedik
Copy link
Copy Markdown
Contributor

@Fedik Fedik commented Jan 21, 2023

Summary of Changes

The changes should allow to use registry in Flattened mode.
This should speed up the Registry, in cases when nested access to Data does not needed.

Testing Instructions

Unit testing passes.

Documentation Changes Required

New parameters for Registry constructor, that enable Flattened mode.

@HLeithner
Copy link
Copy Markdown
Contributor

I'm not sure if this is a good idea, having a class behaving different without being able to check what method is uses.

also I think this should be it's own class, maybe extend Registry from a base class which doesn't implement deep merge. Also it could be interesting to convert between both methods...

But as this pr is I don't think it's a good approach tbh.

@Fedik
Copy link
Copy Markdown
Contributor Author

Fedik commented Jan 21, 2023

having a class behaving different without being able to check what method is uses.

Can add method to check it, if realy need.

this should be it's own class, maybe extend Registry from a base class which doesn't implement deep merge.

Also as an option, I can make as own class.
This basiclay write another Registry class, because almost all methods in Registry doing "deep check".

Any other opinions? :)

@Fedik
Copy link
Copy Markdown
Contributor Author

Fedik commented Jan 21, 2023

Maybe as alternative way, can do check for non empty separator before each call of:

strpos($path, $separator);
explode($separator, $path); 

This will be kind of the same (but not really).

Currently "empty separator" will cause error.

@Fedik
Copy link
Copy Markdown
Contributor Author

Fedik commented Jan 22, 2023

I made PR for empty separator #66

@Fedik Fedik closed this Jan 22, 2023
@Fedik Fedik deleted the flattened branch January 22, 2023 12:19
@Fedik
Copy link
Copy Markdown
Contributor Author

Fedik commented Feb 11, 2023

The pr for a new class #67 for flat registry

@HLeithner
Copy link
Copy Markdown
Contributor

Looks good but I noticed that $data is a stdClass no idea why. Also I would make this class final.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants