To the best of my knowledge the return type is not part of the method signature as such in Java.
You might think of covariant return types which come into play when overriding a method in a child class, and the child’s return type can be a subtype of the parent’s return type. This was introduced in Java 5. If the parent's method return type is void, however, you won't be able to change the return type of the child's method. On Tue, Sep 6, 2022 at 12:04 AM John Mayfield <john.wilkinson...@gmail.com> wrote: > I did contemplate making the addAtom() return the new "ref" the container > has... however I think this is technically an API breakage (void => IAtom > return type). This would not have been in prior Java versions but at some > point they added the return type to the signature. It's kind of a minor > breakage, basically providing the downstream gets recompiled byte code it's > OK. > > On Mon, 5 Sept 2022 at 14:59, John Mayfield <john.wilkinson...@gmail.com> > wrote: > >> Yup! Sorry just drafting stuff out :-) Made the same mistake in the PR >> which used predicates: >> https://github.com/cdk/cdk/pull/889/commits/a160def65f79218a410c8fa4fe6ece5e2ed40dde >> >> >> On Mon, 5 Sept 2022 at 14:47, Daniel Katzel <dkat...@gmail.com> wrote: >> >>> Surely that line output.getAtom( atom.getAtomicNumber() -1 ) >>> >>> Was meant to use output.getAtomCount() -1 >>> >>> To get the last atom added? >>> >>> On Mon, Sep 5, 2022, 6:44 AM John Mayfield <john.wilkinson...@gmail.com> >>> wrote: >>> >>>> Yes and it's correct, your code is adding bonds to atoms which don't >>>> exist yet! You need to add the atoms of the bond before the bond - 0..k not >>>> k .. n. >>>> >>>> As an aside the Mappings API has this method >>>> already (toSubstructureStream()) if you're coming via CDK substructure >>>> Pattern (although it doesn't do the radials/lone pairs... possible but I >>>> never found a use to have them explicitly like that). >>>> >>>> In general this code is an inefficient way to build the container... I >>>> think it's O(N^3) - but perhaps only O(N^2) with AtomContainer2 :-). Much >>>> better to add all the atoms, record these in a set/map, then loop all the >>>> bonds. Optionally for best performance you should do this "resync" >>>> operation where you map the source => target AtomRef. Maybe we should add >>>> this to the ACmanipulator, I thought there was something similar >>>> already though. >>>> >>>> John >>>> >>>> public static IAtomContainer extractSubstructure(IAtomContainer source, >>>> List<IAtom> atoms) { >>>> IAtomContainer output = source.getBuilder().newAtomContainer(); >>>> Map<IAtom,IAtom> remap = new HashMap<>(); >>>> for (IAtom atom : atoms) { >>>> output.addAtom(atom); >>>> >>>> source.getConnectedLonePairsList(atom).forEach(output::addLonePair); >>>> >>>> source.getConnectedSingleElectronsList(atom).forEach(output::addSingleElectron); >>>> // resync: get the AtomRef in the context of the new container. >>>> This >>>> // presumes atoms gets added at last position which is currently >>>> // always the case >>>> remap.put(atom, output.getAtom(atom.getAtomicNumber() - 1)); >>>> } >>>> for (IBond bond : source.bonds()) { >>>> IAtom beg = remap.get(bond.getBegin()); >>>> IAtom end = remap.get(bond.getEnd()); >>>> if (beg != null && end != null) { >>>> output.addBond(bond); >>>> >>>> // or the more efficient but you get a "NEW" bond so may need >>>> to some fudging with >>>> // setting aromatcity/ring membership etc, however if you're >>>> selecting a substructure >>>> // you MUST recalculate these anyways. >>>> // output.addBond(beg.getIndex(), end.getIndex(), >>>> bond.getOrder(), bond.getStereo()); >>>> } >>>> } >>>> return output; >>>> } >>>> >>>> >>>> On Mon, 5 Sept 2022 at 11:10, Uli Fechner <u...@pending.ai> wrote: >>>> >>>>> >>>>> Hi, >>>>> >>>>> I get a NoSuchAtomException when executing the following code (that I >>>>> inherited): >>>>> >>>>> public static IAtomContainer extractSubstructure(IAtomContainer source, >>>>> List<IAtom> atoms) { >>>>> IAtomContainer output = >>>>> SilentChemObjectBuilder.getInstance().newAtomContainer(); >>>>> int k = 0; >>>>> for (IAtom atom : atoms) { >>>>> output.addAtom(atom); >>>>> source.getConnectedLonePairsList(atom).forEach(lp -> >>>>> output.addLonePair(lp)); >>>>> source.getConnectedSingleElectronsList(atom).forEach(se -> >>>>> output.addSingleElectron(se)); >>>>> k++; >>>>> for (int i = k; i < atoms.size(); i++) { >>>>> IBond bond = source.getBond(atom, atoms.get(i)); >>>>> if (bond != null) { >>>>> output.addBond(bond); >>>>> } >>>>> } >>>>> } >>>>> return output; >>>>> } >>>>> >>>>> The stack trace starts at the line with >>>>> output.addBond(bond); >>>>> >>>>> and continues with the following lines: >>>>> >>>>> org.openscience.cdk.exception.NoSuchAtomException: Atom is not a member >>>>> of this AtomContainer >>>>> at >>>>> app//org.openscience.cdk.silent.AtomContainer2.getAtomRef(AtomContainer2.java:185) >>>>> at >>>>> app//org.openscience.cdk.silent.AtomContainer2.newBondRef(AtomContainer2.java:221) >>>>> at >>>>> app//org.openscience.cdk.silent.AtomContainer2.addBond(AtomContainer2.java:908) >>>>> >>>>> The exception *isn't* thrown if I replace the instantiation of the >>>>> IAtomContainer output with >>>>> IAtomContainer output = new AtomContainer(); >>>>> I understand that the exception has something to do with the way >>>>> AtomContainer2 is implemented, but I don't know how to switch to the >>>>> AtomContainer2 implementation and still retain the intent of the code - >>>>> which is to only add atoms, their connected LPs, their connected single >>>>> electrons and their connected bonds, but not connected atoms - to the >>>>> returned subgraph. Best >>>>> Uli >>>>> >>>>> >>>>> _______________________________________________ >>>>> Cdk-user mailing list >>>>> Cdk-user@lists.sourceforge.net >>>>> https://lists.sourceforge.net/lists/listinfo/cdk-user >>>>> >>>> _______________________________________________ >>>> Cdk-user mailing list >>>> Cdk-user@lists.sourceforge.net >>>> https://lists.sourceforge.net/lists/listinfo/cdk-user >>>> >>>
_______________________________________________ Cdk-user mailing list Cdk-user@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/cdk-user
