Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fields with null value are given the non nullable type Any #23

Closed
saied89 opened this issue Apr 8, 2018 · 17 comments
Closed

fields with null value are given the non nullable type Any #23

saied89 opened this issue Apr 8, 2018 · 17 comments

Comments

@saied89
Copy link

saied89 commented Apr 8, 2018

Jsons that contain fields with null value could not be parsed with the generated model due to the fields being assigned the non nullable type Any.
I think Any? is more appropriate.

example:
json:
{
"_id": null,
"name": "test",
"type": "test"
}
is parsed to
data class test(
@JSON(name = "_id") val id: Any, //null
@JSON(name = "name") val name: String, //test
@JSON(name = "type") val type: String //test
)

@wuseal
Copy link
Owner

wuseal commented Apr 8, 2018

yeah,Good job! Thanks for your advise, It should be that as you discribed ,It will be fixed next version soon!

@saied89
Copy link
Author

saied89 commented Apr 8, 2018

Happy to be of help to such a useful tool

@saied89 saied89 closed this as completed Apr 8, 2018
@wuseal wuseal reopened this Apr 13, 2018
@wuseal
Copy link
Owner

wuseal commented Apr 14, 2018

@saied89 Hi,After thinking about this issue again,I found that if JSON always not sure whether it be null or not ,We'd better to make all the field to to nullable, and the now plugin has support this now by selecting the Property type be nullable(?) option, We can select it and then Moshi could parse the json string.
em..Or ,you also could change the type Any to Any? Manually ,Mostly we want to Known the real type of the JSON field instead of Any type .

For your condition ,We may have a better solution:

  • First, Kotlin is designed try to avoid null, And then could we make some works to avoid null assign?
  • Run the next code, we could filter the json null fields, may be there a better code to do that.
    fun filterNullJsonField(jsonString:String):String {
        val moshi=Moshi.Builder().add(KotlinJsonAdapterFactory()).build()
        if (jsonString.startsWith("{")) {

            val adapter = moshi.adapter(Map::class.java)
            val tempMap = adapter.fromJson(json)
            return adapter.toJson(tempMap)
        } else {
            val adapter = moshi.adapter(List::class.java)
            val tempList = adapter.fromJson(json)
            return adapter.toJson(tempList)
        }

    }
  • JSON string is converted

from

{
"_id": null,
"name": "test",
"type": "test"
}

to

{"name":"test","type":"test"}
  • Use moshi to parse the result JSON string without null filed ,then the result data could like this:
User(id=java.lang.Object@3b0090a4, name=test, type=test)

Hope useful for you ,If any problem welcome to reply me directly.

@saied89
Copy link
Author

saied89 commented Apr 14, 2018

I don't like default values when parsing JSON. I'd rather it fail and throw an exception than keep working with wrong value. I also don't know what "lang.Object@3b0090a4" means
Making all fields nullable is also not a good option.
I just think that json fields that have value null should be assigned nullable type Any?. One could later in development (when it comes to actually using the field) give a more specific(still nullable) type but at the start you have for example 50 fields and it doesn't parse.
Making them nullable manually is indeed how I handle it right now. But that can be tiresome on big models.

@wuseal
Copy link
Owner

wuseal commented Apr 14, 2018

@saied89 Glad to receive your response, Yeah, you could choose no default value.
In JSON data,the only condition that JSON field with null value is the String type or Object type or Array type, and as you say ,You want the null value field to be Any? type declaration ,In general,every field with those type may be null, is that means we should make all that field to be Any? type?
For example:
the data returned from service may like this:

{
"_id": null,
"name": "test",
"type": "test"
}

And also some times may like this:

{
"_id": null,
"name": null,
"type": "test"
}

Or this:

{
"_id": null,
"name": null,
"type": null
}

when in the third condition ,Did you want to make the generate model to be like as this ?:

data class User(
        @Json(name = "_id") val id: Any?,
        @Json(name = "name") val name: Any?,
        @Json(name = "type") val type: Any?
)

@saied89
Copy link
Author

saied89 commented Apr 15, 2018

No. but imagine you're given a postman for a project and part of the json that is returned there is the third case. But you don't need User class right now. You just need it to parse so that you can use other parts. You will fill out the right types further in development when non null values are returned(case 1 and 2 in your example) and you actually need the User class. It is a common scenario for me at least.

@wuseal
Copy link
Owner

wuseal commented Apr 15, 2018

@saied89 Yes ,we can fill out the right types further in development when non null values are returned. but tell me ,what the maybe model declaration looks like when the return data is the third case, If probably could you make some cases with code,Then I can catch your mind more accurate

@saied89
Copy link
Author

saied89 commented Apr 17, 2018

I think we're over complicating the matter a bit :)
I think that the model generated for a json should be able to parse that json. Don't you agree?
It doesn't happen when a field with value null is present and thats a minor bug imo.

@wuseal
Copy link
Owner

wuseal commented Apr 18, 2018

@saied89 Hi,Friends,you are right ,what we want is that the model could be used to parse the json.
For example,the json like this:

{
"_id": null,
"name": null,
"type": null
}

we only could declare a nullable type that could used to parse the json ,And we could declared it like this :

data class User(
        @Json(name = "_id") val id: Any?,
        @Json(name = "name") val name: Any?,
        @Json(name = "type") val type: Any?
)

then we can parse the json .
And if the server may return the json like this ,we only have to declare the class with all propertes the nullable type. Am I right?
So .make all properties to be nullable would be a prefer way if you don't filter the null fileds in json .

@saied89
Copy link
Author

saied89 commented Apr 18, 2018

Does it ever make sense to declare such a json non-nullable? Would a user ever want that?
what if they're mixed meaning only some fields are nullable and some fields are required?
I think making fields with null value nullable regardless of all settings is the correct behaviour

@wuseal
Copy link
Owner

wuseal commented Apr 19, 2018

@saied89 Ok ,I think I got it ,the json you parsed with contains null value meaning the field would be nullable some times, and others with real value meaning that they are non-nullable every time , Is that right? If so , An option Auto Make Type ( auto determine nullable or non-nullable type from current json field value ) would fix your request. If you are that meaning ,Then the option will be added next version.

@saied89
Copy link
Author

saied89 commented Apr 19, 2018

yeah that would be great! thanks

@wuseal
Copy link
Owner

wuseal commented Apr 20, 2018

@saied89 Hi ,I just released version 2.0-beta, Which has complete the advice you raised ,If you have time ,you could have a test if it works for you. 😄 Just download the jar in the release url and install local ,I think you know how to download and install local the plugin .thanks for your awesome advice again !

@saied89
Copy link
Author

saied89 commented Apr 20, 2018

I humbly suggest that you move the option to a check box(its not really a third type) which is only enabled if type non-nullable is selected. It might be called "Assign nullable type to fields with value null"

@wuseal
Copy link
Owner

wuseal commented Apr 21, 2018

@saied89 First, If now the version reach your needs?
Then as a third type is there any problem ? In my opinion ,It is a third type describe as 'Dynamic type determined from JSON value', Not Non-Nullable or Nullable neither. Some times it will be Nullable ,and some times it would be Non-Nullable.If any reason it should not be a third type ,please tell me , TKS.

@saied89
Copy link
Author

saied89 commented Apr 21, 2018

Yeah it's working great 😄 thanks!
You are right about type thing too

@wuseal
Copy link
Owner

wuseal commented May 8, 2018

Close this issue if any other problem, welcome to raise a new one.

@wuseal wuseal closed this as completed May 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants