https://kotlinlang.org logo
#android
Title
# android
d

dave08

11/22/2018, 12:49 PM
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", "") ?: ""
?
t

Tolriq

11/22/2018, 2:18 PM
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.
d

dave08

11/22/2018, 2:19 PM
More efficient?
So it can never actually be null?
t

Tolriq

11/22/2018, 2:21 PM
If you do not pass null as a the default value it won't be null no
d

dave08

11/22/2018, 2:22 PM
Oh... it's because the default value is nullable... thanks! Good to know... 🙂
Btw, how would you write that with contracts?
t

Tolriq

11/22/2018, 2:23 PM
This is java, everything is nullable 😉
🙈 1
👍 1
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.
👌🏼 1
g

gildor

11/23/2018, 3:51 AM
I prefer getString("Foo", null) ?: "Default value"
Or you can just write own extension function for that
👍🏼 1
t

Tolriq

11/23/2018, 8:46 AM
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.
g

gildor

11/23/2018, 9:33 AM
Optimal code? Not sure what you mean
prefs.getString("foo", "")!!
also works
this is just a problem of default value, which cannot be correctly checked by compiler, so writing own kotlin wrapper for this will help
t

Tolriq

11/23/2018, 9:54 AM
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.
g

gildor

11/23/2018, 9:56 AM
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
anyway, generated bytecode is not really something important in this case
t

Tolriq

11/23/2018, 9:57 AM
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.
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.
g

gildor

11/23/2018, 10:01 AM
most probably all those “optimizations” completely unnecessary and will be optimized by jit or even proguard
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
t

Tolriq

11/23/2018, 10:13 AM
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.
g

gildor

11/23/2018, 10:14 AM
maybe, but it’s still better than
getString("Foo", "") ?: ""
or something like that
t

Tolriq

11/23/2018, 10:15 AM
Lol yes of course 😉
g

gildor

11/23/2018, 10:16 AM
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
t

Tolriq

11/23/2018, 10:19 AM
Micro optimization that cost nothing are never to under estimate on large application. I typically use
Copy code
-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.
g

gildor

11/23/2018, 10:20 AM
I wouldn’t never do this, anyway, your choice
I would like to see any real benchmark for this, rather than theoretical premature optimizations
t

Tolriq

11/23/2018, 10:22 AM
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)
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.
g

gildor

11/23/2018, 10:26 AM
You exchange clear stack traces and “fail fast” for highly doubtful “optimization”
t

Tolriq

11/23/2018, 10:27 AM
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.
Kotlin have compiler option to not generate those checks too 😉
g

gildor

11/23/2018, 10:28 AM
Yes, which is not officially supported and not recommendet to use
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
otherwise we return to enums hollywar
t

Tolriq

11/23/2018, 10:33 AM
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.
3 Views