shnewto / bnf

Parse BNF grammar definitions
MIT License
256 stars 22 forks source link

change public vectors to iterators #13

Closed CrockAgile closed 6 years ago

CrockAgile commented 6 years ago

The types Expression, Production, and Grammar all publicly expose internal vectors. I assume the intention was to allow users to traverse the nodes in the grammar. However, exposing the internal vector locks the project to supporting the vector type interface forever onward. Imagine in the future that the data structures were changed to trees, arrays, or anything besides vector. This would cause a breaking change in the public interface for users.

Instead, if these types only exposed iterators, they would still support tree traversal, but changing the internal implementation would not be a breaking change.

shnewto commented 6 years ago

Ah that's something I hadn't really foreseen, it sounds like it's probably a high value add though. I'll have to start something like a 0.2.0-devel branch where revisions that add a lot but represent a break (like this and the return Result issue) can be staged for a minor version bump down the line.

CrockAgile commented 6 years ago

@snewt If you are feeling extra fancy, maybe we could start a 0.2.0 project board and milestone to track which issues are intended for that version?

shnewto commented 6 years ago

haha @CrockAgile I am feeling extra fancy! You up for taking the lead on that?

CrockAgile commented 6 years ago

@snewt consider it done: project and milestone

I took the liberty to assign the new additions, but if there is anything included you are not sure about, please feel free to pull it out

CrockAgile commented 6 years ago

@snewt my initial approach has brought up a few questions:

shnewto commented 6 years ago

@CrockAgile regarding the first point, I'd like it if a user could programmatically add/remove from their grammars, even build one up without an input string.

for the second one, I'm not sure that at this point the scope of the problem we're solving requires handling that kind of validation.