search

shogun-toolbox / shogun

Shōgun
http://shogun-toolbox.org
BSD 3-Clause "New" or "Revised" License
3.03k stars 1.04k forks source link

Drop legacy code #1263

Open karlnapf opened 11 years ago

karlnapf commented 11 years ago

During fixing errors that made clone/equals tests crash, I touched a vast amount of Shogun's classes. Many of them do not contain anything useful.

1.) Code is not in the current Shogun style (Memory, data structures, output). This is fine up to a certain extend. But if basic things do not work, it is very questionable whether this class will ever be used again

2.) A lot of the code compiles but doesnt work. Example: const char *filename="/fml/ag-raetsch/home/toussaint/scp/aawd_compbio_workshop/code_nora/data/profile/profiles"; std::ifstream fin(filename); This is called from the (non default) constructor of CSpectrumRBFKernel. There are other, less extreme examples.

3.) A lot of the code is so hard to read and so badly documented that I have no idea how to use it etc. HMM.cpp is an extreme example of a collection of hacks. KMeans.cpp is another example of old code that is not really useful anymore.

4.) Those things are not at all useful for our users. In fact, I think they scare people off. I believe all of our stuff exposed to the outside should be well documented and the code should be readable.

5.) The question always is. What to do if things are very ugly but somehow work?

6.) We could put markers to REMOVE_SOON markers to code that should definitely be dropped in the near future. People should be noticed about this (warning or whatever) and after a few months, we could remove things.

What about if we collect a list of classes/parts that could be easily dropped and then start putting in the markers? How to do this in practice?

karlnapf commented 11 years ago

What do you think is the best way to mark classes as deprecated/REMOVE_SOON so that users are warned ?

iglesias commented 11 years ago

What about printing a warning message in the constructor?

karlnapf commented 11 years ago

I would like to have something that allows us to automatically create a list of deprecated classes.

iglesias commented 11 years ago

Then there are two parts here, warn users and crate this list. I think that a #ifdef or #ifndef clause around the whole class (like the HAVE_EIGEN3 or DOXYGEN_SHOULD_SKIP_THIS we currently use) could work for this.