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

Make cleaner separation between parser and main. #665

Closed
jonathf opened this issue Jan 29, 2022 · 6 comments
Closed

Make cleaner separation between parser and main. #665

jonathf opened this issue Jan 29, 2022 · 6 comments

Comments

@jonathf
Copy link
Collaborator

jonathf commented Jan 29, 2022

Currently the main interface creates CLArgs to parser arguments, which (depending on input) get passed down into the vroom machinery.
For the most part this is done cleanly in such a way that you don't need CLArgs to use any machinery, with input parser being the only exception.

I propose we make the interface cleaner by keeping CLArgs contain to the main file and just passing it's attribute along explicitly.

@jcoupey
Copy link
Collaborator

jcoupey commented Jan 31, 2022

Yes, makes perfect sense.

@jonathf
Copy link
Collaborator Author

jonathf commented Jan 31, 2022

Before merging though (or after for that matter), I am wondering if I need to make an adjustment here.

The problem I am realizing I will be faced with is that as soon as sub-class Input, using the parser will be unavailable.
So to be able to subclass Input, I either need to copy content over, or make my own parser.

A better solution, I think, is to initialize Input in main and only pass input and input_str to the parser function. That way, any subclass of Input can also use the parser to fill in content from json.

Sounds good?

@jcoupey
Copy link
Collaborator

jcoupey commented Jan 31, 2022

What would be the need behind inheriting from Input?

@jonathf
Copy link
Collaborator Author

jonathf commented Jan 31, 2022

I subclass the C++ classes in Python to add lots of extra Python features which is a lot more difficult to do from pure C++.

@jcoupey
Copy link
Collaborator

jcoupey commented Jan 31, 2022

I have no problem to use another approach and maybe you can sketch that in a separate PR, but I fear that you won't be able to create an Input object outside of the parsing code since the ctor requires to know the size of the amounts arrays in the first place.

@jcoupey
Copy link
Collaborator

jcoupey commented Feb 1, 2022

This has been merged with #666, thanks @jonathf.

@jcoupey jcoupey closed this as completed Feb 1, 2022
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