Improvise JSONObject getter methods to pass/accept default values

[sagrawal31]

Current Problem

Consider you have a JSONObject as given below-

JSONObject requestData = request.JSON // or new JSONObject(data)

To read a property, let's say rating from the requestData, the Grails developer has to write a lengthy code-

Integer usersRating = 5  // 5 being the default value
if (requestData.has("rating") {
    usersRating = requestData.getInt("rating")
}

or

Integer usersRating = 5  // 5 being the default value

try {
    usersRating = requestData.getInt("rating")
} catch (JSONException ignore) {
    usersRating = 5
}

or

Integer usersRating = (requestData.getInt("rating") ?? 5) as Integer

These approaches-

1. Increases the number of lines.
2. Reduces code readability
3. Increases chances of code failure if someone has not handled the missing key scenario.
4. Unnecessary typecasting.

Solution

This PR adds overloaded methods (with 2nd argument as the default value) for 8 getter methods of JSONObject which does not throw JSONException instead it returns the default values. So one can simply write-

Integer usersRating = requestData.getInt("rating", 5)

Motivation

This technique is used by SharedPreference in Android.

[aulea]

There exists methods like optInt(“rating”, 5), why those cannot be used instead

[sagrawal31]

🤦🏻

Shit, 9 years, I'm working with Grails and how could I missed these methods.

Thanks, @aulea for pointing me to that.

[aulea]

It shows that by someone chosen method names with prefix 'opt' isn't intuitive and you are probably not only one who has missed it.

[sbglasius]

I actually think that @sagrawal31 should reopen this issue, and perhaps improve it by marking the original opt methods @Deprecated

PS. I did not know about the opt methods either.

[sagrawal31]

Thanks, @sbglasius I was sceptical about saying that we should deprecate the others. Reopened this.

[puneetbehl]

We discussed this in the Grails Engineering meeting and it was decided not to deprecate optInt(“rating”, 5) and it should the way forward. But, I believe the Grails documentation should be updated to include optInt.

@sagrawal31 please create a separate issue for updating documentation.

[sagrawal31]

Thanks @puneetbehl for considering it in the discussion and providing the feedback.

[sagrawal31]

@puneetbehl Should I create a separate issue for documentation in grails-core only? I don't see any documentation about JSONObject in https://docs.grails.org/4.0.12/guide/single.html that's why this question.

[puneetbehl]

Grails documentation is here