jsherfey / dnsim

Dynamic Neural Simulator - a modular modeling tool for large ODE systems.
3 stars 3 forks source link

Pruning archaic/deprecated code: Unused functions and Graveyard-commented-code #63

Open asoplata opened 8 years ago

asoplata commented 8 years ago

This is an issue for two little things, only concerning the "real" DNSIM code in dnsim/matlab/functions. The following functions do not seem to be used/called by anything else (some are interdependent, not that it matters), and I've never seen them used in any capacity like during instructional "how-to" scripts. We may have to update these functions when we make the post-SFN changes to the data structure, model structure, or the main control flow of the program IF they are necessary, but if they are not or are deprecated, we should delete them:

In addition, after looking through the files in this subdirectory for functions that have a significant amount of commented-out code (especially at the end), I've found several files seem to have a large amount of this "graveyard code" as I call it. Given that versions of these files containing the commented-out code will still be present in the Git history, and we can clearly mark such a commit where they're removed, I think it's best to go ahead and remove most of the commented-out code in these files:

SO, my job for other people is to answer: Which files of the first group do we NOT want to delete (and therefore, when we make big architecture changes, have to port to the new formats/stuff), and is there any good reason, on a file-by-file basis, why the commented out code in the second group shouldn't be deleted?

jsherfey commented 8 years ago

I think all functions in group one can be deleted.

I'm in favor of removing all uninformative "graveyard code".

Jason

On Wed, Oct 7, 2015 at 3:22 PM, asoplata notifications@github.com wrote:

This is an issue for two little things, only concerning the "real" DNSIM code in dnsim/matlab/functions. The following functions do not seem to be used/called by anything else (some are interdependent, not that it matters), and I've never seen them used in any capacity like during instructional "how-to" scripts. We may have to update these functions when we make the post-SFN changes to the data structure, model structure, or the main control flow of the program IF they are necessary, but if they are not or are deprecated, we should delete them:

  • comparemodels.m
  • extractfeatures.m
  • getfeatures.m
  • prepare_spec.m (actually just a script, not a function)
  • studydriver.m
  • StudyDriverUI.m

In addition, after looking through the files in this subdirectory for functions that have a significant amount of commented-out code (especially at the end), I've found several files seem to have a large amount of this "graveyard code" as I call it. Given that versions of these files containing the commented-out code will still be present in the Git history, and we can clearly mark such a commit where they're removed, I think it's best to go ahead and remove most of the commented-out code in these files:

  • biosimdriver.m
  • dnsim2xpp.m
  • MechanismBrowser.m
  • modeler.m
  • plotpow.m
  • rk.m
  • runsim.m
  • write_dnsim_script.m

SO, my job for other people is to answer: Which files of the first group do we NOT want to delete (and therefore, when we make big architecture changes, have to port to the new formats/stuff), and is there any good reason, on a file-by-file basis, why the commented out code in the second group shouldn't be deleted?

— Reply to this email directly or view it on GitHub https://github.com/jsherfey/dnsim/issues/63.

asoplata commented 8 years ago

Continuation of graveyard code: There are several "if 0" statements in the same codebase that are effectively the same as the graveyard comments. They are

For all the same reasons as above (namely, that these are possibly archaic comments, and will still be present in the codebase via previous versioning), I think we should delete these too.

asoplata commented 8 years ago

Update: I think I was wrong about extra code in MechanismBrowser.m, as the rest of the file seems to resemble real code after Line 74. On Lines 73 and 74, what appear to be commented-out, previous versions of header and footer are at the end of their definitions; HOWEVER, because there isn't a space between the % and { in some of this inline comments, I think the creation of a %{ is what causes the rest of the file to be commented out (since that's the symbol for a block comment in MATLAB). I think the %{ was an accident, and the rest of the code in that file is actually real code that was commented out by accident. Therefore I don't think MechanismBrowser.m has a bunch of graveyard code.

asoplata commented 8 years ago

I forgot to add that only studydriver.m and StudyDriverUI.m are dependent on simdriver.m, and since they are removed, simdriver.m is no longer used by any remaining function.

@jsherfey Jason, can you confirm that simdriver.m is deprecated? Or is it a real alternative to biosimdriver.m that still has its uses?