search

mcmtroffaes / inipp

Simple C++ ini parser.
MIT License
282 stars 53 forks source link

New formatting class to more easily allow supporting exotic formats. #11

Closed mcmtroffaes closed 4 years ago

mcmtroffaes commented 4 years ago

See #10

mcmtroffaes commented 4 years ago

@CyberDNIWE I'm pretty happy with this implementation. Would this work for you? See the MixedFormat class in the test suite, along with the corresponding test, for how you could use it.

It breaks compatibility from anyone who was using the Ini class member variables directly. But I doubt anyone would do that. I will bump the mayor release version just in case.

CyberDNIWE commented 4 years ago

@mcmtroffaes I cant test this out right now, but from what I gather, it looks like you've separated polymorphic formatting class and dependency injected it into "main" Ini class and it looks amazing!

Personally, I'm not a fan of templated parameters in base class' virtual functions, because you need to be extra careful for them not to come back to bite you. But hey, if it works, it works.

Since this thing took off, I might as well share what I cobbled together: Naiive approach here: Inheritance

And a bit more hackish one here: TypeErasure

I think both of them are way less elegant than what you did though, and surely way harder to understand (which is a con in my book)

mcmtroffaes commented 4 years ago

Thanks for having a look. Yours are also interesting (and educational!) solutions. Unless a serious concern still comes up, I'll merge the solution from this pull request in the coming days.