DBAIWangGroup / nns_benchmark

Benchmark of Nearest Neighbor Search on High Dimensional Data
199 stars 52 forks source link

Parent class constructing derived class? #5

Open 0Xellos opened 6 years ago

0Xellos commented 6 years ago

(This is related to DPG only; I haven't read the other algorithms' code in detail, so I can't comment on them, but this issue may appear there as well.)

Initialising KGraph by calling KGraph::create() is a valid, but very strange approach, and can lead to really confusing code, e.g. when attempting to initialise a class derived from the DPG ANN-solver (whether from KGraphImpl or KGraph).

KGraphImpl extends KGraph. create() is a member function of KGraph, but it creates its child (which shouldn't exist in the context of KGraph, it only does since it's put in the same file), implicitly casts it to KGraph and returns that. Why not replace create() by the constructor of KGraph, since it already acts as a constructor? Next, KGraphImpl acts as the parent class despite being the child class; this relationship should be switched. If KGraphImpl is the child in order to make KGraph unable to imitate parts of it - I can't see any other reason - that's basically the same as protected constructor (of KGraphImpl as the parent of KGraph).