Learn how easy it is to sync an existing GitHub or Google Code repo to a SourceForge project! See Demo

Close

#194 Implementation of CLOCKWISE and ANTICLOCKWISE stere

Needs_Review
closed
nobody
master (162)
5
2012-10-28
2010-05-03
Egon Willighagen
No

Using the new framework proposed in patch:

2995814 More general stereochemistry framework

It introduces a new Java package org.openscience.cdk.stereo with the class ILigancyFourChirality.

This is the kind of stereochemistry found in SMILES, which is my next target.

Aimed at master.

Discussion

  • A very minor thing, but "Clockwise" is a whole word so I would prefer "ANTI_CLOCKWISE" and "CLOCKWISE" rather than "ANTI_CLOCK_WISE" and "CLOCK_WISE". I didn't know the word 'ligancy' but now I do :) Oh, and the convention for Java enums are capitalised lowercase, so 'Stereo' rather than 'STEREO'.

    I think that the doc comment for getLigands is wrong : "Returns an array IS ligand atoms..." - should it be "array OF ligand atoms"? This is naturally repeated in the interface and implementation doc comments.

    The only actually code-related point that I can see is the implementation of the getLigands method. Is there a reason for it returning a copy of its atoms, rather than just a reference?

     
  • Ack on CLOCKWISE, the Stereo enum, and the getLigands() JavaDoc. Will upload a new patch version shortly.

    Then array copy is to prevent people from changing the data model unintentionally.

     
  • Updated the patch, applying the suggestions by Gilleain.

     
  • Rajarshi Guha
    Rajarshi Guha
    2010-05-16

    Patch looks good. Applied and pushed to master