Skip to content
Browse files

chemfilter improvised Signed-off-by: Syed Asad Rahman <s9asad@gmail.com>

Signed-off-by: Rajarshi  Guha <rajarshi.guha@gmail.com>
  • Loading branch information...
1 parent 8f3c70a commit 2a7ee54783ba2238d7a5ac09f99518757579b594 @asad asad committed with rajarshi
Showing with 25 additions and 19 deletions.
  1. +25 −19 src/main/org/openscience/cdk/smsd/filters/ChemicalFilters.java
View
44 src/main/org/openscience/cdk/smsd/filters/ChemicalFilters.java
@@ -640,25 +640,31 @@ private double getBondScore(double score, Map<IBond, IBond> bondMaps) {
private double getBondFormalChargeMatches(IBond RBond, IBond PBond) {
@egonw The Chemistry Development Kit member
egonw added a note

JavaDoc on private methods is very much recommended. It is not entirely clear to me what this method is doing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
double score = 0.0;
- if (RBond.getAtom(0).getID().equals(PBond.getAtom(0).getID())
- && RBond.getAtom(1).getID().equals(PBond.getAtom(1).getID())) {
- if (!RBond.getOrder().equals(PBond.getOrder())
- && RBond.getAtom(0).getFormalCharge() != PBond.getAtom(0).getFormalCharge()) {
- score += Math.abs(RBond.getAtom(0).getFormalCharge() - PBond.getAtom(0).getFormalCharge());
- }
- if (!RBond.getOrder().equals(PBond.getOrder())
- && RBond.getAtom(1).getFormalCharge() != PBond.getAtom(1).getFormalCharge()) {
- score += Math.abs(RBond.getAtom(1).getFormalCharge() - PBond.getAtom(1).getFormalCharge());
- }
- } else if (RBond.getAtom(1).getID().equals(PBond.getAtom(0).getID())
- && RBond.getAtom(0).getID().equals(PBond.getAtom(1).getID())) {
- if (!RBond.getOrder().equals(PBond.getOrder())
- && RBond.getAtom(1).getFormalCharge() != PBond.getAtom(0).getFormalCharge()) {
- score += Math.abs(RBond.getAtom(1).getFormalCharge() - PBond.getAtom(0).getFormalCharge());
- }
- if (!RBond.getOrder().equals(PBond.getOrder())
- && RBond.getAtom(0).getFormalCharge() != PBond.getAtom(1).getFormalCharge()) {
- score += Math.abs(RBond.getAtom(0).getFormalCharge() - PBond.getAtom(1).getFormalCharge());
+ if (RBond != null
+ && PBond != null
+ && RBond.getAtom(0).getID() != null && PBond.getAtom(0).getID() != null
+ && RBond.getAtom(1).getID() != null && PBond.getAtom(1).getID() != null) {
+
+ if (RBond.getAtom(0).getID().equals(PBond.getAtom(0).getID())
+ && RBond.getAtom(1).getID().equals(PBond.getAtom(1).getID())) {
+ if (!RBond.getOrder().equals(PBond.getOrder())
+ && RBond.getAtom(0).getFormalCharge() != PBond.getAtom(0).getFormalCharge()) {
+ score += Math.abs(RBond.getAtom(0).getFormalCharge() - PBond.getAtom(0).getFormalCharge());
+ }
+ if (!RBond.getOrder().equals(PBond.getOrder())
+ && RBond.getAtom(1).getFormalCharge() != PBond.getAtom(1).getFormalCharge()) {
+ score += Math.abs(RBond.getAtom(1).getFormalCharge() - PBond.getAtom(1).getFormalCharge());
+ }
+ } else if (RBond.getAtom(1).getID().equals(PBond.getAtom(0).getID())
+ && RBond.getAtom(0).getID().equals(PBond.getAtom(1).getID())) {
+ if (!RBond.getOrder().equals(PBond.getOrder())
+ && RBond.getAtom(1).getFormalCharge() != PBond.getAtom(0).getFormalCharge()) {
+ score += Math.abs(RBond.getAtom(1).getFormalCharge() - PBond.getAtom(0).getFormalCharge());
+ }
+ if (!RBond.getOrder().equals(PBond.getOrder())
+ && RBond.getAtom(0).getFormalCharge() != PBond.getAtom(1).getFormalCharge()) {
+ score += Math.abs(RBond.getAtom(0).getFormalCharge() - PBond.getAtom(1).getFormalCharge());
+ }
@egonw The Chemistry Development Kit member
egonw added a note

So, what had changed here? Is the 'improvised' a typo of 'improved'? If not, what did you improvise; if it is, what did you improve? Was there a bug? Well, you get the idea now of good commit messages...

@asad
asad added a note

Line 643 to 646 is new.

@asad
asad added a note

it will skip the test if atom IDs are not set.

@egonw The Chemistry Development Kit member
egonw added a note

Very good. I expected something like that, but as said... you have to guide the reader... add comments in the source code like:

// deal with situations where atom IDs are not set

(Or something like that)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
}
}
return score;

2 comments on commit 2a7ee54

@asad

The idea behind this class is to differentiate between charged atoms and non changed atoms. eg: the score will rank c=o > c-o± > c-o.

@egonw
The Chemistry Development Kit member

Good. Such information would be useful to have as JavaDoc or comments in the source code. You know, but readers of your code do not. The whole point of Open Source is the many eyes, but just like with a journal publication, you must guide the reader through the test... JavaDoc, code comments, and good coding style are only for this purpose... not to make it more difficult to get your code accepted :)

Please sign in to comment.
Something went wrong with that request. Please try again.