Improve API to production quality about hdt-cpp

[mielvds]

Again thanks for taking this forward! As I'm not sufficiently qualified in C++, I can't fully assess this. @MarioAriasGa @joachimvh any thoughts?

Dependent applications at the moment are at least the GUI application hdt-it (also in this repo) and the HDT-Node library. I believe we should design a versioning approach for the code releases, and the indexes they generate (#7), first.

[RubenVerborgh]

I have encountered problems 1, 2, and 3 myself and I agree that a fix would indeed be good. The exception thing has bitten us several times. And the const thing troubled me in at least one location, where I was not allowed to read something from a const object. I think I indeed also saw a couple of instances of 4 as well.

So thanks in advance for looking at these :-)

[artob]

Thanks @RubenVerborgh and @mielvds for your feedback.

I've begun tackling these issues, starting with const-correctness since that is somewhat lower hanging than the other actionables, in that while it may affect the library ABI, it does preserve source compatibility—meaning that existing applications only need recompile, at worst.

This work is happening on the const-correctness branch of our fork. I would expect to submit that branch as a pull request sometime next week once further along.

[RubenVerborgh]

Thanks, @bendiken, much appreciated!

[mielvds]

I created version 1.1.1 (https://github.com/rdfhdt/hdt-cpp/releases/tag/1.1.1) at the point where the code moved from google code to github (as named by @MarioAriasGa in d343fa8). The code on the website is 1.0.0. This gives us a point of reference for future releases and incompatibilities.

[RubenVerborgh]

Thanks a lot, Miel! Will indeed be handy to reference. We can/should probably stay within 1.x as long as the outputted HDT files are compatible.

[MarioAriasGa]

Thanks bendiken,

I agree with your improvements. I will keep an eye in case I can help.

I will make sure that the HDT-it! app also compiles with your changes.

Thanks again!

Mario.

[artob]

@mielvds That's good thinking, and means we might perhaps consider these changes to constitute a 1.2.x API.

@MarioAriasGa Thank you for weighing in—I will hence proceed according to the outlined plan, and submit pull requests for each of these action items.

[mielvds]

I would propose 1.1.2, since no breaking changes in the HDT file or index are introduced, right? See my proposal on versioning in #7

[artob]

I've merged the initial const-correctness work in 42a5b07. This improves type safety as well as reduces memory allocations and spurious string copying. The semantic changes to the API are minor and unlikely to affect any users (such as HDT-it!).

[webdata]

Hi, @bendiken I couldn't run the code with the new API because of the new exception in ControlInformation. I had to perform several changes to catch this exception throughout the code, already considered in the last commits 42caf47, 0253fe2

[artob]

@webdata Right, that type of thing could be a consequence of const-correctness changes.

If the exception in case of nonexistent keys proves too onerous for client code (it certainly complicated the client code in 42caf47 and 0253fe2), we could make the method return a const reference to a static empty string in the nonexistent key case, which would preserve full source compatibility with existing code. It'd be easy enough to do.

What do you think? You did already make the client code changes, but Git also makes them easy to revert.

[webdata]

Yes, if you don't need to distinguish between an existing empty property and a non-existing property, that could be possible.

[LaurensRietveld]

I took a first stab at modifying exception handling. See this PR: #35

[mielvds]

Let's try to have these fixes by mid 2017

[RubenVerborgh]

The majority of these issues seem to be addressed, so I will close this. Feel free to create new issues for subpoints where necessary.