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

pattern.find_matches(tree) or treematcher.find_matches(tree, pattern)? #43

Open
jhcepas opened this issue Aug 9, 2017 · 3 comments
Open
Assignees

Comments

@jhcepas
Copy link
Member

jhcepas commented Aug 9, 2017

We need to decide what strategy is best (or maybe keep both), and organized the code in treematcher consequently. pros? cons?

p = Tree('(A+,B);') 
t = Tree('((A,B), C);')
treematcher.find_matches(t, p)
p = Tree('(A+,B);') 
t = Tree('((A,B), C);')
p.find_matches(tree)
@jhcepas
Copy link
Member Author

jhcepas commented Aug 9, 2017

I don't see any clear benefit in option 1, do you?

@jhcepas jhcepas assigned dpliakos and unassigned dpliakos Aug 9, 2017
@dpliakos
Copy link
Collaborator

dpliakos commented Aug 9, 2017

I think the first is unnecessary too.
The only case the first syntax I find it to be worthy is the case that a user is not familiar with OOP and sees function calls "as commands" (like "print ' text text' " ), of course one import (from treematcher import find_matches) will be needed. In that case the call maybe is a bit easier to understand.

The other is shorter and more clear, if you can think in OOP way.

Keeping both may make the tutorials unclear and the intention to help not so experienced users may end up making things more complex (at least when I meet something new I just want to know how to do make it work and stick with it at first), so I suggest to keep only the second.

@jhcepas
Copy link
Member Author

jhcepas commented Aug 10, 2017

we can maybe keep the algorithm as separate functions (like now) and add methods to the TreePattern class as needed. With the current TreePattern.find_matches method should be enough for now.

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

No branches or pull requests

4 participants