#204 More interfaces instead of implementations

Needs_Review
closed
nobody
master (162)
5
2012-10-28
2010-05-21
No

Three patches, aimed at master.

Discussion

  • Rajarshi Guha

    Rajarshi Guha - 2010-05-22

    patch 2 seems to replace usage of Molecule with Molecule, rather than IMolecule

     
  • Egon Willighagen

    The second patch doesn't actually replace the use of Molecule; the reordering of the import is a side-effect of Eclipse trying to be smart. Should I change that? It is in the tests, which are free to pick any implementation.

    The explain this use of IAtom instead of Atom, is that after the refactoring I am working on, PseudoAtom extends NNPseudoAtom, and not Atom.

     
  • Rajarshi Guha

    Rajarshi Guha - 2010-05-22

    Aah, OK. That makes sense. But given that we tell people to look at the unit tests to see how to use classes, shouldn't the unit test be an exemplar of 'good' CDK style?

     
  • Egon Willighagen

    I fully agree. That said, many unit tests need coding cleaning, much like the library itself. I had not really given it though yet, but it should not be too difficult to write PMD tests to test of local fields are Molecule or IMolecule. Or, Atom versus IAtom, etc.

    Anyway, the code change does not change any line with Molecule/IMolecule, except for the bloody reordering by Eclipse...

    If it is OK with you, I focus on getting the refactoring finished... but will give the PMD test a quick look...

     
  • Rajarshi Guha

    Rajarshi Guha - 2010-05-22

    OK, makes sense. Applied and pushed

     

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks