Make getArgs() non-nullable

[hannesstruss]

Currently Controller.getArgs() is marked @Nullable and either returns the argument bundle or null if no args were set. The consequence of this is IDE warnings and cumbersome null checks whenever using getArgs().

I'd argue that getArgs() should return an empty bundle when no args were set, making it unnecessary to do a null check (or worse, ignore the warning). This would make it impossible to distinguish between empty and absent args, but I don't think it would be of much use anyway.

Happy to take a stab at this!

[PaulWoitaschek]

The same with getActivity(). I know that it returns null when im at the wrong part of the lifecycle. I really would prefer it to be without a null/nonNull annotation, since the change a few hundred lint warnings popped up I have to suppress manually or just start ignoring lint warnings.

[hannesstruss]

I think getActivity() is a slightly different case, since there is no equivalent default value like the empty Bundle. If something can in fact be null, in my opinion @Nullable is legit.

[EricKuck]

Seems like a reasonable change. Done in the latest snapshot. Thanks!

[dimsuz]

Yeah, having getActivity() suddenly become @Nullable is what keeps me from updating an app to 2.0.5. It is a rather large codebase written in Kotlin and Kotlin respects nullability annotations when inferring types coming from java-land.

And suddenly all my calls to getActivity() now return a different type (Activity? instead of Activity), so this release is actually a breaking change for Kotlin... :) Not so in less restrictive Java, where you may be annoyed by warnings, but no compilation errors at least.

I'm not saying this is a wrong change, just wanted to mention this as something to be aware of...

[EricKuck]

@dimsuz Almost all of our apps are Kotlin as well, so I feel your pain. This should have been more than a patch release, as it is kind of a breaking change. My temporary approach was to add this as a class extension:

fun Controller.unsafeActivity() = activity!!

I then did a replace all on usages of this.activity or getActivity(). This isn't more unsafe than what was already happening (implicit vs. explicit non-null). I definitely don't recommend keeping this around long-term, but it's a suggestion for a quick path to updating until you get a chance to do it properly.

[dimsuz]

@EricKuck wow, I don't know how that happened, but I didn't realize that I can add extension function to a Controller. This idea of unsafe version of activity was first thing that came to my mind, but I didn't connect the dots and instead thought of some base class which I would have to introduce and I didn't want that.

Thank you. I also not sure at which times an activity might be null (I hope that an upcoming wiki or whatever will make it more clear - as a collective effort of doc improvement). Can you give a hint please?

I never had an issue with null activity so far. Maybe it's because I use mosby-conductor, so almost all logic happens after view (controller) is attached to it and a presenter starts managing it...