#1306 QueryAtomContainer adds null atoms & bonds

cdk-1.4.x
closed
nobody
None
1
2014-09-21
2013-06-18
Duece99
No

Hi,

I noticed this in QueryAtomContainer.java

    /**
     *  Adds an atom to this container.
     *
     *@param  atom  The atom to be added to this container
     */
    public void addAtom(IAtom atom)
    {
        if (contains(atom))
        {
            return;
        }

        if (atomCount + 1 >= atoms.length)
        {
            growAtomArray();
        }
        atom.addListener(this);
        atoms[atomCount] = atom;
        atomCount++;
        notifyChanged();
    }

    /**
     *  Adds a Bond to this AtomContainer.
     *
     *@param  bond  The bond to added to this container
     */
    public void addBond(IBond bond) {
        if (bondCount >= bonds.length) growBondArray();
        bonds[bondCount] = bond;
        ++bondCount;
        notifyChanged();
    }

And that it allows null atoms & bonds to be added. Is this intentional? It causes exceptions to be thrown in the get methods if a null is found before the "end" of the loops.

Related

Bugs: #1306

Discussion

  • John May
    John May
    2013-06-18

    Yes, don't add null bonds then :-). It's the same with the other implementations not just query. Where are you getting null bonds introduced?

    J

    On 18 Jun 2013, at 16:16, Duece99 duece99@users.sf.net wrote:

    [bugs:#1306] QueryAtomContainer adds null atoms & bonds

    Status: open
    Created: Tue Jun 18, 2013 03:16 PM UTC by Duece99
    Last Updated: Tue Jun 18, 2013 03:16 PM UTC
    Owner: nobody

    Hi,

    I noticed this in QueryAtomContainer.java

    /**
     *  Adds an atom to this container.
     *
     *@param  atom  The atom to be added to this container
     */
    public void addAtom(IAtom atom)
    {
        if (contains(atom))
        {
            return;
        }
    
        if (atomCount + 1 >= atoms.length)
        {
            growAtomArray();
        }
        atom.addListener(this);
        atoms[atomCount] = atom;
        atomCount++;
        notifyChanged();
    }
    
    /**
     *  Adds a Bond to this AtomContainer.
     *
     *@param  bond  The bond to added to this container
     */
    public void addBond(IBond bond) {
        if (bondCount >= bonds.length) growBondArray();
        bonds[bondCount] = bond;
        ++bondCount;
        notifyChanged();
    }
    

    And that it allows null atoms & bonds to be added. Is this intentional? It causes exceptions to be thrown in the get methods if a null is found before the "end" of the loops.

    Sent from sourceforge.net because you indicated interest in https://sourceforge.net/p/cdk/bugs/1306/

    To unsubscribe from further messages, please visit https://sourceforge.net/auth/subscriptions/


    This SF.net email is sponsored by Windows:

    Build for Windows Store.

    http://p.sf.net/sfu/windows-dev2dev_______
    Cdk-bugs mailing list
    Cdk-bugs@lists.sourceforge.net
    https://lists.sourceforge.net/lists/listinfo/cdk-bugs

     

    Related

    Bugs: #1306

  • Duece99
    Duece99
    2013-06-18

    I fixed the bug in my code that was introducing the null bonds anyway. I just think it odd that no warnings or exceptions are thrown saying that a null bond has been introduced or whatever.

     
  • John May
    John May
    2013-06-18

    Yes it should, but it doesn't. Not sure if that will change anytime soon.

    J

    On 18 Jun 2013, at 16:34, Duece99 duece99@users.sf.net wrote:

    I fixed the bug in my code that was introducing the null bonds anyway. I just think it odd that no warnings or exceptions are thrown saying that a null bond has been introduced or whatever.

    [bugs:#1306] QueryAtomContainer adds null atoms & bonds

    Status: open
    Created: Tue Jun 18, 2013 03:16 PM UTC by Duece99
    Last Updated: Tue Jun 18, 2013 03:16 PM UTC
    Owner: nobody

    Hi,

    I noticed this in QueryAtomContainer.java

    /**
     *  Adds an atom to this container.
     *
     *@param  atom  The atom to be added to this container
     */
    public void addAtom(IAtom atom)
    {
        if (contains(atom))
        {
            return;
        }
    
        if (atomCount + 1 >= atoms.length)
        {
            growAtomArray();
        }
        atom.addListener(this);
        atoms[atomCount] = atom;
        atomCount++;
        notifyChanged();
    }
    
    /**
     *  Adds a Bond to this AtomContainer.
     *
     *@param  bond  The bond to added to this container
     */
    public void addBond(IBond bond) {
        if (bondCount >= bonds.length) growBondArray();
        bonds[bondCount] = bond;
        ++bondCount;
        notifyChanged();
    }
    

    And that it allows null atoms & bonds to be added. Is this intentional? It causes exceptions to be thrown in the get methods if a null is found before the "end" of the loops.

    Sent from sourceforge.net because you indicated interest in https://sourceforge.net/p/cdk/bugs/1306/

    To unsubscribe from further messages, please visit https://sourceforge.net/auth/subscriptions/

     

    Related

    Bugs: #1306

  • At some point it was decided to have code not check all possible things users could get wrong in the input... (it was very many)... Thus, CDK users are expected to check themselves the provide sensible input... a null bond is indeed not... Part of this discussion is that additional checking also requires additional computation, slowing the CDK down...

    Maybe it is time to reconsider this?

     
  • John May
    John May
    2014-07-23

    • Description has changed:

    Diff:

    --- old
    +++ new
    @@ -1,4 +1,3 @@
    -
     Hi,
    
     I noticed this in QueryAtomContainer.java
    
    • status: open --> closed
     
  • John May
    John May
    2014-07-23

    Closed - possible solution is to use lang asserts which can be turned on and off.