Yes Java doesn't the bytecode/JVM does :-). Here is the setup to show it
would break without recompilation downstream:
[john@sentinel:test]% cat Upstream_void.java
> public class Upstream {
> static void Hello() {
> System.out.println("Hello (void)!");
> }
> }
>
> [john@sentinel:test]% cat Upstream_String.java
> public class Upstream {
> static String Hello() {
> System.out.println("Hello (String)!");
> return "Why oh why!?!";
> }
> }
>
> [john@sentinel:test]% cat Main.java
> public class Main {
> public static void main(String[] args) {
> Upstream.Hello();
> }
> }
We compile/run this like this:
[john@sentinel:test]% cp Upstream_void.java Upstream.java && javac
> Upstream.java Main.java && java -cp . Main
> Hello (void)!
Now if I try it with Upstream_String instead and *DON'T* recompile
Main.java you should the following error:
[john@sentinel:test]% cp Upstream_String.java Upstream.java && javac
> Upstream.java && java -cp . Main
> Exception in thread "main" java.lang.NoSuchMethodError: 'void
> Upstream.Hello()'
> at Main.main(Main.java:3)
If you recompile Main.java it's OK again.
[john@sentinel:test]% cp Upstream_String.java Upstream.java && javac
> Upstream.java Main.java && java -cp . Main
> Hello (String)!
>
J
On Mon, 5 Sept 2022 at 15:12, Uli Fechner <[email protected]> wrote:
> 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 <[email protected]>
> 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 <[email protected]>
>> 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 <[email protected]> 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 <[email protected]>
>>>> 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 <[email protected]> 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
>>>>>> [email protected]
>>>>>> https://lists.sourceforge.net/lists/listinfo/cdk-user
>>>>>>
>>>>> _______________________________________________
>>>>> Cdk-user mailing list
>>>>> [email protected]
>>>>> https://lists.sourceforge.net/lists/listinfo/cdk-user
>>>>>
>>>>
_______________________________________________
Cdk-user mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/cdk-user
