IssueTracker

Component used: Navigation
Version used: 2.1.0-rc01
Devices/Android versions reproduced on:

Proposal: Safe Arg generated Builder Classes should have default values inside.

This is actually not a bug for navigation component but a kind of side effect which can make this feature more versatile.
In my scenario, we were trying to use generated safe arg Builder classes to pass arguments to the fragments in our project, as the normal bundle arguments are not type safe and kinda messy to work with.
 
The project I'm working on is relative large, in our team we decided to migrate gradually to navigation component.
As a first step we wanted to put the fragment in a nav graph just to generate safe args so that we can use those classes to safely pass arguments to our existing fragment with fragment transaction and eventually use the graph in future point. So we are trying to use Safe arg as mentioned below:

ChoosePackageFragmentArgs args =
             new ChoosePackageFragmentArgs.Builder("testName", "testPackage")
                 .setIsExtend(true)
                 .build();
ChoosePackageFragment fragment = new ChoosePackageFragment();
fragment.setArguments(args.toBundle());

getSupportFragmentManager().beginTransaction()
   .replace(R.id.container, fragment)
   .addToBackStack(null)
   .commit();
 
As you can see from the above example the "isExtend" property is optional and has a default value false which is set in the graph like this,
 
<argument
   android:name="isExtend"
   app:argType="boolean"
   android:defaultValue="false" />
 
 
However if we use the generated safe arg builder class (ChoosePackageFragmentArgs) to pass the arguments to ChoosePackageFragment, without setting "isExtend" the ChoosePackageFragmentArgs.fromBundle(requireArguments()).getIsExtend() throws a NullPointerException because the builder doesn't set the default value when we call the args.toBundle() on the builder instance.
 
Moreover, when we try to call the getter method it can't find a value from the map, so it tries to cast a null value to (boolean), thus throwing the NullPointerException. Remember we're not using a NavController, just trying to take advantage of SafeArg feature.

This is the code from ChoosePackageFragmentArgs.Builder:

public class ChoosePackageFragmentArgs implements NavArgs {
 ....
     @SuppressWarnings("unchecked")
     public boolean getIsExtend() {
       return (boolean) arguments.get("isExtend");
     }
 ...
}

We were wondering why the default value is not present in the object. After digging for a while we've found that NavController.navigate() pulls the default values from NavAction.getDefaultArguments(); as we're not using NavComponent we're actually not getting the default value from Builder. Here's the code that pulls default args in NavComponent Library.

Code from NavController.java:

final NavAction navAction = currentNode.getAction(resId);
Bundle combinedArgs = null;
if (navAction != null) {
   if (navOptions == null) {
       navOptions = navAction.getNavOptions();
   }
   destId = navAction.getDestinationId();
   Bundle navActionArgs = navAction.getDefaultArguments();
   if (navActionArgs != null) {
       combinedArgs = new Bundle();
       combinedArgs.putAll(navActionArgs);
   }
}
 
Suggestion:
If the safe arg plugin added the default values to the Builder while generating those SafeArgs Builder classes, the conventional fragment approach could get huge benefit out of it, and as these classes are accessible apis, I believe it makes more sense to have predictable behaviour for Builder classes as that seems like a standard predictable behaviour.