#536 accept the matching IChemObject interfaces in accepts()

Needs_Revision
closed
John May
cdk-1.4.x (181)
5
2012-10-08
2012-08-05
No

The patch adds lines like:

if (IChemFile.class.equals(classObject)) return true;

matching the interfaces the reader/writer accepts against the passed object, so that one can now properly do:

someReader.accepts(IMolecule.class)

This address bug #3553780. The matching unit test is attached to that bug report:

https://sourceforge.net/tracker/index.php?func=detail&aid=3553780&group_id=20024&atid=120024#

Discussion

  • John May

    John May - 2012-08-05

    The attached patch seems to be just the unit test.

     
  • Egon Willighagen

    Oops... :(

    Now I attached the right one.

     
  • John May

    John May - 2012-09-05

    Looks good but I'm wondering if there is a more flexible way of doing this. It might not be the case for all readers but for one I looked at RXNV2000 there is an accepts(Object) and accepts(Class) which have mostly duplicate code. It should be possible to unify these into a single method for accepts(Object) passing 'object.getClass()' to the other accepts. This could even be done in the super-class and minimise the code in the actual reader.

    I had a quick play using a static Set to store what objects were accepted. Heres what it looks like https://github.com/johnmay/cdk/commit/c7e8d1ebcfc4758a5cd4c77f03c79ac81160986b

    The method is a lot cleaner and also completely null safe (I provided another commit for null check on accepts(Object)). It may also be alot faster as the branching is minimised.

    An even better alternative that might be even better would be to have a non-static set in the super class then pass 'accepted' classes to the super-class in the constructor. I didn't do this here as the type the reader/writer accepts does not change between instance I wanted to minimise footprint but then I realised most of the time we will create a reader instance and reuse it (setReader()).

    Even better.... (you can tell I'm thinking as a write this) we could have a dispatch table in the super-class that would provide the type of classes that are accepted and also dispatch the correct 'read' method for each class type. It would be nice to minimise the boiler plate code between the readers so that the actually code in the concrete classes is just the reader specific code.

    Anyways let me know what you think.

    Thanks,
    J

     
  • John May

    John May - 2012-10-05

    Okay I re-reviewed and have signed the patch.

    Will add a feature request for abstracting io functionality in due course.

     
  • Egon Willighagen

    John, where did that signed-off patch go?

     
  • John May

    John May - 2012-10-07

    Should be in the attached files:
    0001-[SIGNED]-Fixed-readers-and-writers-accept.patch

     
  • Egon Willighagen

    Got it now. I did not see the patch earlier today. Maybe to do with the upgrade?

     

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

Sign up for the SourceForge newsletter:





No, thanks