Why is it that SharedPreferences getXXX methods ar...

# android

[dave08]

Why is it that SharedPreferences getXXX methods are all marked as returning null? Is there any way to avoid val foo: String = prefs.getString("foo", "") ?: ""?

[Tolriq]

Because Java does not have the wonderful Kotlin contracts 😉 just put !! instead of ?: "" as it's more efficient. Or create your own extensions with a contract.

[dave08]

More efficient?

[dave08]

So it can never actually be null?

[Tolriq]

If you do not pass null as a the default value it won't be null no

[dave08]

Oh... it's because the default value is nullable... thanks! Good to know... 🙂

[dave08]

Btw, how would you write that with contracts?

[Tolriq]

This is java, everything is nullable 😉

[Tolriq]

About contracts I have not yet looked precisely about how to write mine 😉 but you can check sample by looking as source for isNullOrEmpty for string for example.

[gildor]

I prefer getString("Foo", null) ?: "Default value"

[gildor]

Or you can just write own extension function for that

[Tolriq]

Even it's it over optimizing, Android already and will always do the if null then return default even if default is null, there's no point to add another if == null return "Default" it's more code for no gain. Even if I admit !! is not that nice to have in code, it's the more efficient way as we just tell the compiler we know it's not null (And we do) so no need to do anything special -> Optimal code.

[gildor]

Optimal code? Not sure what you mean

[gildor]

prefs.getString("foo", "")!! also works

[gildor]

this is just a problem of default value, which cannot be correctly checked by compiler, so writing own kotlin wrapper for this will help

[Tolriq]

Yes this is what I've wrote since start 😉 Either use an extension function or use !!. !! is optimal because using ?: will have generated code that do an if result is null then do that. But we know that result is not null so that check is not necessary. And android will already do that check internally in the getString("xxx","") so removing an unneeded if is optimal 😉 So from resulting bytecode point of view getString("Foo","Default")!! is the optimal way.

[gildor]

using ?: will have generated code that do an if result is null then do that

It’s not true.code after ?: will be used, because we passed null as default value

[gildor]

anyway, generated bytecode is not really something important in this case

[Tolriq]

If you use getString("Foo",null) ?: "". But https://android.googlesource.com/platform/frameworks/base.git/+/master/core/java/android/app/SharedPreferencesImpl.java#289 already does that if check even if you pass null. So you have 2 times the if null check for no purpose.

[Tolriq]

As I also said it's over optimization, but some simple optimization that does not hurt code readability have no reason to not be used IMO.

[gildor]

most probably all those “optimizations” completely unnecessary and will be optimized by jit or even proguard

[gildor]

I thought that this discussion what to do with such code, and you sometimes need more idiomatic version of it, and you don’t want to write real wrapper
So one more null check is not something important, but yeah, if you use this a lot, better to write extension function
Also, using !! also generates bytecode for null check. And if you really care about it, better to reimplement getString() as extension using get(), instead of getString() + null assert

[Tolriq]

Did not know about !! that's not that cool. Anyway yes extension is better and can be reused, pretty sure Android KTX have something for that. But about idiomatic having to write getString("Foo",null) ?: "Default" vs getString("Foo","Default")!! My choice is easily the second one as more clear and less typing.

[gildor]

maybe, but it’s still better than getString("Foo", "") ?: "" or something like that

[Tolriq]

Lol yes of course 😉

[gildor]

that’s not that cool

This is assert, obviously you need some code that will check for null and thow an exception.
But I really don’t understand why do you care about it, everything will be optimized with jit, and even without jit null check is nothing comparing with map lookup and syncronization, so it’s just nothing

[Tolriq]

Micro optimization that cost nothing are never to under estimate on large application. I typically use

-assumenosideeffects class kotlin.jvm.internal.Intrinsics {
    static void checkParameterIsNotNull(java.lang.Object, java.lang.String);
}

In proguard as the dex size impact is non negligeable and the checks only brings better stack traces at the expense of a small if check. But on a large app a million null check is still something. And if you have proper tested code in the end there's no crash and so no stack trace to generate.

[gildor]

I wouldn’t never do this, anyway, your choice

[gildor]

I would like to see any real benchmark for this, rather than theoretical premature optimizations

[Tolriq]

Yep and the gains are very real on very large application for no real impact. After talking with some R8 guys, with R8 it will be a little different as they have special optimizations around those intrinsics to generate other optimisations, but currently it is worth it (And confirmed by R8 guys)

[Tolriq]

When an app is 7 years in Play Store and top 2% of crash rate optimizations are not really premature, they are part of release cycle 😉 But that's another story unrelated to this thread.

[gildor]

You exchange clear stack traces and “fail fast” for highly doubtful “optimization”

[Tolriq]

You doubt I've tested and R8 guys confirmed. About exchanging, it's release build, debug builds + tests have that on, as I said this is a non issue if you have properly tested code.

[Tolriq]

Kotlin have compiler option to not generate those checks too 😉

[gildor]

Yes, which is not officially supported and not recommendet to use

[gildor]

You doubt I’ve tested and R8 guys confirmed

This is micro benchmarking, which is fine for compiler developers, but you real application probably do something more than check for nulls

[gildor]

otherwise we return to enums hollywar

[Tolriq]

On functions that are need to be public due to being defined as an API but called a tons of times per second the impact is here, the stack trace issue is irrelevant when we know that there's no possible problem. Small optimization + small optimization + small, you know the song. I'm not trying to convince you. But for my part I have convinced, and I have the necessary things in place to ensure that the gain is better that what could be lost.