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

Wrapper classes StatePost, and for initial and final states sets? #325

Closed
Adda0 opened this issue Sep 7, 2023 · 7 comments
Closed

Wrapper classes StatePost, and for initial and final states sets? #325

Adda0 opened this issue Sep 7, 2023 · 7 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 Sep 7, 2023

Question originally raised in #277 (comment).

But initial and final can still have size(), right? since they are just sets.

They will have to be unless you want to create a wrapper class SparseStateSet. It would have a member SparseSet, method num_of_states() which would only call return this->sparse_set.size(); and add the boilerplate code for all other functions unmodified, as what we intend to do for StatePost.

However, if we do this for StatePost, we could create the wrapper for sparse set, too. It is the same wrapper as StatePost is for OrdVector<SymbolPost>. Opinions?

@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 Sep 7, 2023
@Adda0 Adda0 changed the title Wrapper classes for StatePost, initial and final states sets? Wrapper classes StatePost, and for initial and final states sets? Sep 8, 2023
@Adda0
Copy link
Collaborator Author

Adda0 commented Sep 11, 2023

@kilohsakul @jurajsic @tfiedor @vhavlena Any opinions?

@kilohsakul
Copy link
Collaborator

kilohsakul commented Sep 12, 2023

I would probably not implement these wrappers. It is a lot of work and code and will only complicate the interface.

@jurajsic
Copy link
Member

Would the wrapper be useful in some way? Because I don't really see it

@vhavlena
Copy link
Collaborator

I would keep the things as simple as possible. I see it as a complication, which is not so beneficial to me now.

@Adda0
Copy link
Collaborator Author

Adda0 commented Sep 13, 2023

Unless we plan on somehow extending the wrappers in the future (with methods named specifically for states, etc.), the wrapper is useless in my opinion. The only advantage in the current stage would be getting a number of states would be consistent with Delta and Nfa.

@tfiedor
Copy link
Member

tfiedor commented Sep 13, 2023

Sadly, I don't really care about this.

@Adda0
Copy link
Collaborator Author

Adda0 commented Sep 13, 2023

No one sees a reason for the wrappers. We will keep the types as they are now. I will close this issue as resolved, then.

@Adda0 Adda0 closed this as completed Sep 13, 2023
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

5 participants