[Cdk-devel] [ cdk-Patches-2996119 ] Implementation of CLOCKWISE and ANTICLOCKWISE stere

[SourceForge.net <no...@so...>]

Patches item #2996119, was opened at 2010-05-03 19:49
Message generated for change (Comment added) made by gilleain
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=320024&aid=2996119&group_id=20024

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: master
Group: Needs Review
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Egon Willighagen (egonw)
Assigned to: Nobody/Anonymous (nobody)
Summary: Implementation of CLOCKWISE and ANTICLOCKWISE stere

Initial Comment:
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.

----------------------------------------------------------------------

Comment By: gilleain maclean torrance (gilleain)
Date: 2010-05-04 12:32

Message:
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?

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=320024&aid=2996119&group_id=20024