Skip to content
This repository has been archived by the owner on Mar 9, 2021. It is now read-only.

Incorrect treatment of java.lang.Boolean #43

Open
ainslec opened this issue Mar 28, 2014 · 4 comments
Open

Incorrect treatment of java.lang.Boolean #43

ainslec opened this issue Mar 28, 2014 · 4 comments

Comments

@ainslec
Copy link

ainslec commented Mar 28, 2014

If I have code

boolean myMethod() {
   Boolean myBool = null;
   myBool = something();
   return myBool;
}

It is converted to:

bool myMethod() {
   bool myBool = null; // INVALID AS BOOL IS PRIMITIVE IN C#
   myBool = something();
   return myBool;
}

Problem is that it converts Boolean and boolean as primitives, when the java code might depend upon a boolean being able to hold a null value.

A trivial but incorrect fix :

bool myMethod() {
   bool? myBool = null; // NOW VALID BECAUSE NULL IS NOW AN ACCEPTED VALUE
   myBool = something();
   return myBool; // NOW THIS IS AN INVALID BECAUSE C# does not unbox
}

Obviously, this logic is complex so perhaps the answer is to require (and check) that the java code does not use the unbox feature and simply translate boolean to bool?, and within the java code and translate the booleanValue() method on Boolean to the Value property on the bool? instance.

Possible complete solution (depends upon autounboxing detection)::

boolean myMethod() {
   Boolean myBool = null;
   myBool = something(); // Assuming something() is trusted not to return null
   return myBool.booleanValue(); // Manual UNBOXING
}
bool myMethod() {
   bool? myBool = null; // NOW VALID BECAUSE NULL IS NOW AN ACCEPTED VALUE
   myBool = something();
   return myBool.Value; // NOW VALID BECAUSE THIS IS A MANUAL UNBOXING TRANSLATED FROM JAVA
}
@hazzik
Copy link
Contributor

hazzik commented Mar 29, 2014

You can map types as you want. It is only matter of type mappings.

@ainslec
Copy link
Author

ainslec commented Mar 29, 2014

I think as I explain, that simply mapping java.lang.Boolean to bool? (even if it were possible), it would still cause a problem with the generated code because in the autounboxing positions, the bool? instance would still require accessing the Value property. As such, I think it is something that is dealt with in the code itself - although of course, I'm not an expert.

But, hypothetically, if I wanted to deal with the above scenario via mappings, where would such mappings be defined (if in sharpen-all-options then there is no explicit mapping for java.lang.Boolean so I assume that this mapping is hardcoded somewere in sharpen itself)?

As I'm doing some work with Sharpen at the moment, it would be good to have an informal chat away from the issues forum too. Feel free to message me.

@hazzik
Copy link
Contributor

hazzik commented Mar 30, 2014

if in sharpen-all-options then there is no explicit mapping for java.lang.Boolean so I assume that this mapping is hardcoded somewere in sharpen itself

Yes and yes. The mapping from sharpen-all-options will override built-in mappings.

So I think you can add two mappings:

-typeMapping java.lang.Boolean bool?
-propertyMapping java.lang.Boolean.booleanValue Value

@Kavignon
Copy link

Evaluating your boolean expression with myBool.booleanValue() can be mapped in C# simply by making sure that the final code looks something like this (I'm not sure thought, I'm thinking out loud)

bool myMethod() {
bool? myBool = null; // NOW VALID BECAUSE NULL IS NOW AN ACCEPTED VALUE
myBool = something();
return (myBool.HasValue) ? myBool.Value :false //This will have a better result than simply accessing the value property of the nullable.
}

The only problem with this work around is that you have to not only evaluate the condition once of the nullable when invoking something() but you have to check a second time the boolean before making sure you can actually return some kind of value and be sure no code will break during the operation.
@Takapa @hazzik

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants