jsweet icon indicating copy to clipboard operation
jsweet copied to clipboard

Version 2.0 adds constructor assignments to null when it shouldn't

Open DavidRigglemanININ opened this issue 7 years ago • 4 comments

Given the following Java code

public class Test {
    public String foo;
    public String bar;

    public Test(String foo) {
        this.foo = foo;
    }
}

V1 works as expected, as seen below

namespace main.test {
    export class Test {
        public foo : string;

        public bar : string;

        public constructor(foo : string) {
            this.foo = foo;
        }
    }
    Test["__class"] = "main.test.Test";
}

V2, however, does some odd things. First, it assigns foo to null in the constructor even though it is immediately set right afterwards. Second, it assigns bar to null. This could cause issues in existing V1 code or where undefined is desired as null has a different meaning than undefined. It also just increases the size of the generated code unnecessarily in many cases.

namespace main.test {
    export class Test {
        public foo : string;

        public bar : string;

        public constructor(foo : string) {
            this.foo = null;
            this.bar = null;
            this.foo = foo;
        }
    }
    Test["__class"] = "main.test.Test";
}

DavidRigglemanININ avatar Jun 20 '17 13:06 DavidRigglemanININ

That's because in Java, fields and variables are initialized with default values. Objects are initialized to null, numbers are initialized to 0, booleans are initialized to false, ... Since it is the expected behavior in Java, we have decided to implemented it. After all, JSweet being based on Java, people expect that it behaves like it. I am opened to suggestion to make it better... Maybe would we need an annotation to keep the field un-initialized?

Annotating Foo with @DisableJavaInitialization would sound like a good idea? (knowing that with JSweet 2 your can centralize annotations in the jsweetconfig.json file)

renaudpawlak avatar Jun 20 '17 14:06 renaudpawlak

That's what I figured was the case. Because I could manually assign the values to defaults in java and then the constructor assignments went away. Certainly, in the case for the "foo" variable, there's no reason to ever set that to null (i.e. there is only one constructor and the variable is set guaranteeing it will always be set), although I guess that's an optimization. So for our specific use case, we have java objects that represent api objects going across the wire. So on the TS side, we will just be casting the http response to the TS type. Therefore all the null assignments just make our client JS code larger than it needs to be. Also, in many cases, we may have a response that omits certain fields that are optional (because it seems like that is a common paradigm rather than passing the field with a null value). I'd be fine with just a transpiler option to disable it (or some global flag), possibly specified via config in the maven plugin. While a annotation might work, we'd probably have to use it hundreds of times, which is less than ideal.

DavidRigglemanININ avatar Jun 20 '17 14:06 DavidRigglemanININ

Oh, you say that because you don't know yet the jsweetconfig.json syntax. You can easily annotate several elements of your program by using an expression. For example, to annotate all the classes within the x.y.z package, you can just write:

{
	"@DisableJavaInitialization": { "include": [ "x.y.z.**" ] }
}

Of course, it is less intuitive than a flag option, but it is more generic.

renaudpawlak avatar Jun 20 '17 14:06 renaudpawlak

Nice, that would work for me.

DavidRigglemanININ avatar Jun 20 '17 14:06 DavidRigglemanININ