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

remove inheritance, use Trait #29

Closed
peetersdiet opened this issue Sep 28, 2016 · 4 comments
Closed

remove inheritance, use Trait #29

peetersdiet opened this issue Sep 28, 2016 · 4 comments

Comments

@peetersdiet
Copy link

I am interested in using a version of Baum based on Traits. Baum looks well-featured, but we cannot use it by extending from it, because that would break our architecture.

At first glance, I don't really see a blocking issue to introduce Traits. In essence, all Baum\Node code should be moved into a separate Trait. You can then have Baum\Node use that trait, to provide backwards compatibility. If testing with instanceof is an issue, a Node-interface can be created to test on that interface.

In short, some refactoring is necessary to achieve the following pattern:

namespace Baum;

interface NodeInterface {
    // method signatures from Baum\Node
}

trait NodeTrait {
    // implementation from Baum\Node
}

// only necessary for BC
abstract class Node implements NodeInterface {
    use NodeTrait;
}

I'm not really in a position to assess the amount of work involved, but it seems like a straightforward refactoring. And I do not see any drawbacks, as this will be BC and makes the library more flexible in use.

If you're open to my proposed change, I can do the modifications and create a PR.

@gazsp
Copy link
Owner

gazsp commented Oct 26, 2016

Sounds good. I'm open to this.

There's a whole other bunch of issues I need to look at, and as this is a BC, we'll probably get some more fixes / updates in place and bump the version to 2.0.

If anybody has any objections to this refactoring (and breaking change), please speak now, or forever hold your peace :-)

@neyaoz
Copy link

neyaoz commented Oct 29, 2017

I am going to take this job. But still not sure that which base should I start from?

https://github.com/gazsp/baum vs https://github.com/Sithira/baum

@gazsp Have you checked his doings?

@gazsp gazsp added the 2.0 label Feb 26, 2018
@gazsp gazsp added this to Backlog in Baum 2.0 via automation Feb 26, 2018
@gazsp
Copy link
Owner

gazsp commented Mar 2, 2018

@neyaoz I've not checked other forks, no.

Feel free to go ahead and work on this, but checkout feature/2.0 and do the work there please. Work can be back-ported to the 1.x branches as needed.

@poing
Copy link
Collaborator

poing commented Dec 17, 2020

Under New Management

Focusing support for later Laravel versions.

Cleaning up and closing some of the really old issues. Things that might still be a problem will be transferred to a consolidated issue.

@poing poing closed this as completed Dec 17, 2020
Baum 2.0 automation moved this from Backlog to Done Dec 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Baum 2.0
  
Done
Development

No branches or pull requests

4 participants