initialize fields in constructor

[jlmelville]

Fixes #393.

Testing: I ran make test and got no different results between the output in this PR and the current state of develop. That said I do see these warnings in the output (these are also present in develop):

.Adding all elements (10000)
.Running pickle tests for <hnswlib.Index(space='cosine', dim=32)>
.Running pickle tests for <hnswlib.Index(space='ip', dim=16)>
Warning: 1 labels are missing from ann results (k=25, err_thresh=5)
Warning: 1 labels are missing from ann results (k=25, err_thresh=5)
Warning: 1 labels are missing from ann results (k=25, err_thresh=5)
Warning: 1 ann distance values are different from brute-force values (total # of values=2500, dists_thresh=50)
Warning: 1 labels are missing from ann results (k=25, err_thresh=5)
Warning: 1 labels are missing from ann results (k=25, err_thresh=5)
Warning: 1 labels are missing from ann results (k=25, err_thresh=5)
Warning: 1 ann distance values are different from brute-force values (total # of values=2500, dists_thresh=50)

I am not sure if this is an actual problem or not?

Also, I confirmed that the valgrind error disappears via testing through the R bindings I maintain.

[yurymalkov]

Cool! Thanks!

Before merging, I wonder if the constructor initialization is the preferred way (compared to C++ 11 initialization at the variable declaration)?

[jlmelville]

Before merging, I wonder if the constructor initialization is the preferred way (compared to C++ 11 initialization at the variable declaration)?

I can do that. The body of the third constructor HierarchicalNSW(SpaceInterface<dist_t> *, size_t, size_t, size_t, size_t) (not changed in this PR) sets some of those fields to other values (e.g. ef_construction and M_). Would you want those fields also changed to default member initializers (presumably paying the minimal price of setting them twice) or leave those ones alone?

If we are talking constructor improvements then that third constructor would gain the most from changes, as it is the least C++ idiomatic as it sets most of its fields in the constructor body rather than using initializer lists. This PR will leave the constructor styles a bit inconsistent which might be confusing for readers. But I wanted to keep the scope of this PR as small as I could, especially as clang-tidy has a lot to say about converting hnswlib to C++11 form and it's hard to know where to stop once you start down that path...

Anyway let me know if you would like me to go ahead and use the default initialization approach (I see that clang-tidy may have missed some fields that should be default initialized).

[yurymalkov]

Thanks! I am not a C++ expert, so not sure what is the best.
From my experience with other languages (java mostly) it is convenient to have declaration and default initialization at the same point of code (e.g. adding the default values in the block https://github.com/nmslib/hnswlib/blob/master/hnswlib/hnswalg.h#L88-L125 and not touching the existing constructors - they would only override the default values), so it would be also visible if some of the variables are not initialized without cross-checking. I thought that was introduced in C++11 (which is already used in hnswlib), though not sure if that is the right thing to do.

[jlmelville]

@yurymalkov I have made the changes to use direct initialization for the non-class-type fields like ints and size_ts (I have also used braces for the initialization which seems to be preferred by the C++ Core Guidelines). I believe more complex member types like std::vector will be default initialized without having to do anything but I am happy to be corrected on that.

Valgrind (via R) is still happy and make test gives the same output.

[yurymalkov]

Great! Thank you so much!