Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Request] Introducing namespaces #1575

Open
killerwife opened this issue Nov 6, 2020 · 5 comments
Open

[Feature Request] Introducing namespaces #1575

killerwife opened this issue Nov 6, 2020 · 5 comments

Comments

@killerwife
Copy link
Contributor

killerwife commented Nov 6, 2020

We have been recently working on utilizing this project for our chat solution. We have run into a small issue with extending classes that could potentially make life easier for a lot of people.

Is it possible to introduce a common namespace, lets say LiveHelperChat namespace, which would enable us to query that namespace? This would enable overriding methods through subclassing (polymorphism), and then utilizing the autoloading for classes in an extension. This would mean that when we need to change one line, for example add a single new column to lets say erLhcoreClassModelChat getState, we can subclass it and append it more easily without copypasting whole class.

Or maybe perhaps we missed a way this should be doable even more easily. We want to extract all code as neatly as possible so that adding your future changes is easier. Currently copying whole class to extension would mean we would not get the benefits of your new changes.

Rough prototype:
`
include 'lib/models/lhchat/erlhcoreclassmodelchat.php';

class erLhcoreClassModelChat extends LHC\erLhcoreClassModelChat {

use erLhcoreClassDBTrait;

public function getState()
{
    $arr = parent::getState();
    $arr['operating_system'] = $this->operating_system;
    $arr['browser'] = $this->browser;
    return $arr;
}

}
`

If you wish, we can always draft a pull request.

@remdex
Copy link
Contributor

remdex commented Nov 6, 2020

That's a good idea. I'll see how to do that :) As project started 10y ago, not much of a choices I had back then :)

@remdex
Copy link
Contributor

remdex commented Nov 6, 2020

Ok, I tried but it for sure won't be out of the box. Basically I would need to separate classes everywhere and it means 2x includes and includes just increasing load... Basically for larger installations you can just fork and time after time just merge with official version. Also you did not specified what for you need to modify class? as for most overrides I have hooks to listen to.

@killerwife
Copy link
Contributor Author

From my understanding if you were to use namespace in all the internal files which are in the livehelperchat repo, you would subsequently not need to change anything in regards to your own code. (perhaps some extensions would need to reach compliance)

Example of your task is this: erlhcoreclassmodelchat.php (lib\models\lhchat) - adding an additional column, lets say operating_system to the model, and then filling it and utilizing that in addition to everything else in the base class implementation. We do not want to migrate the whole file to extension because then if you add new functionality, we would not get it in a maintainable way (manual copypasting would have to be done with us afterwards)

@remdex
Copy link
Contributor

remdex commented Nov 6, 2020

#1576

You can take a look here. But the thing is - to override this class I would need to put namespace class in it's own file and erLhcoreClassModelChat in it's own file also. So you can override it with autoload. If I do that for this one class it would mean I would need add same capabilities for all of them...

If you just have a fork and made your changes directly in class. Update would be just merging changes from my branch. In your scenario as it's very small changes I don't think there would be any troubles :)

@remdex
Copy link
Contributor

remdex commented Nov 6, 2020

If you come to discord perhaps I could advise what hook to listen for the thing you want to do :)

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

No branches or pull requests

2 participants