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

Unify naming of operations in nfa.hh #172

Open
Adda0 opened this issue Feb 6, 2023 · 5 comments
Open

Unify naming of operations in nfa.hh #172

Adda0 opened this issue Feb 6, 2023 · 5 comments
Labels
For:library The issue is related to library (c++ implementation) Module:nfa The issue is related to Nondeterministic Finite Automata Priority:normal Work on this sooner rather than later. Type:suggestion A suggestion for feature/change that is not necessary at this moment

Comments

@Adda0
Copy link
Collaborator

Adda0 commented Feb 6, 2023

There are a few functions, such as Nfa::determinize() which operate on automata passed as a parameter and return a new automaton. There are also method, usually called Nfa::Nfa::get_..._automaton() or Nfa::Nfa::get_..._words, etc., e.g., Nfa::Nfa::get_one_letter_automaton(), which operate on the underlying automaton object and return a new automaton. Finally, there are methods which operate on the underlying automaton object and perform the operation in place (for example, Nfa::Nfa::trim()).

We should agree on whether we want to try to unify these variants, or when each variant can be used.

@Adda0 Adda0 added For:library The issue is related to library (c++ implementation) Module:nfa The issue is related to Nondeterministic Finite Automata Type:suggestion A suggestion for feature/change that is not necessary at this moment Priority:normal Work on this sooner rather than later. labels Feb 6, 2023
@Adda0
Copy link
Collaborator Author

Adda0 commented Jun 20, 2023

There is a mismatch of names such as concatenate() (verb) and intersection() (noun). We should unify them.

Further suggestions are:

  • Rename intersection_eps() to intersection_with_eps().
  • Rename concatenate to concatenate_with_eps().
  • Rename uni to something that at least slightly resembles union operation. Even unite() would be better.

@tfiedor
Copy link
Member

tfiedor commented Jun 20, 2023

I'm also strongly voting for renaming uni function. Like what...the...fuck????

@Adda0
Copy link
Collaborator Author

Adda0 commented Jun 20, 2023

Definitely, I will add it to the list of names to change.

@Adda0
Copy link
Collaborator Author

Adda0 commented Jul 24, 2023

We agreed that methods should compute operations in-place, if possible, functions should return a new instance, making the computation of the operation out-of-place, and both should use verbs for their names. Furthermore, the method and its corresponding function should be named the same.

@Adda0
Copy link
Collaborator Author

Adda0 commented Jul 9, 2024

Should we add lang_union() (or something similar) as a single function to encapsulate calling operations union_nondet() and union_product() from #412, as discussed in #412 (comment)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For:library The issue is related to library (c++ implementation) Module:nfa The issue is related to Nondeterministic Finite Automata Priority:normal Work on this sooner rather than later. Type:suggestion A suggestion for feature/change that is not necessary at this moment
Projects
None yet
Development

No branches or pull requests

2 participants