0

Looking around for reasons why to use a static factory method like NewInstance instead of a constructor which takes arguments and ran into this answer Best practice for instantiating a new Android Fragment (which I don't really find compelling, but that's another story).

When I'm looking at the Xamarin Developer's guide, they have code which uses the factory, but then sets the local property directly without using a bundle or anything.

public class DatePickerFragment : DialogFragment, 
                              DatePickerDialog.IOnDateSetListener
{
    // TAG can be any string of your choice.
    public static readonly string TAG = "X:" + typeof (DatePickerFragment).Name.ToUpper();

    // Initialize this value to prevent NullReferenceExceptions.
    Action<DateTime> _dateSelectedHandler = delegate { };

    public static DatePickerFragment NewInstance(Action<DateTime> onDateSelected)
    {
        DatePickerFragment frag = new DatePickerFragment();

        // WHY IS THIS ANY DIFFERENT FROM SETTING ANY OTHER VALUE DIRECTLY?
        frag._dateSelectedHandler = onDateSelected;

        // If this next line is bad, why is the above not also bad?
        // frag.myIntValue = 5;
        return frag;
    }

    public override Dialog OnCreateDialog(Bundle savedInstanceState)
    {
        DateTime currently = DateTime.Now;
        DatePickerDialog dialog = new DatePickerDialog(Activity, 
                                                   this, 
                                                   currently.Year, 
                                                   currently.Month,
                                                   currently.Day);
        return dialog;
    }

    public void OnDateSet(DatePicker view, int year, int monthOfYear, int dayOfMonth)
    {
        // Note: monthOfYear is a value between 0 and 11, not 1 and 12!
        DateTime selectedDate = new DateTime(year, monthOfYear + 1, dayOfMonth);
        Log.Debug(TAG, selectedDate.ToLongDateString());

        // HOW COME WE CAN USE THIS DIRECTLY WITHOUT A BUNDLE?
        _dateSelectedHandler(selectedDate);
    }
}

From my understanding (which is limited), setting variables directly has the chance of the fragment being destroyed and recreated and the values being lost.

The recommendation I've seen is to use the Bundle and call setArguments / getArguments to ensure the values are not lost.

Maybe this is because it is an Action? Are there other types which are safe to directly set on a fragment like this? Is the sample code actually doing something which is unsafe? Is there a better / more appropriate / consistent way? Is that why they initialize the field it to prevent NullReferenceException? Even if it is, doesn't that mean that this fragment will no longer work correctly if destroyed and recreated?

Community
  • 1
  • 1
MPavlak
  • 2,133
  • 1
  • 23
  • 38
  • *The recommendation I've seen is to use the Bundle and call setArguments / getArguments to ensure the values are not lost* ... true ... and it is a cause that you can't pass "object as is" because Bundle is Parcellable is "Serialization/object by value"(of course with great simplification) ... That's why we use interface and "object that should exist after recreation"(read: getActivity() cast to some well know interface wich parent Activity implements) – Selvin Aug 11 '16 at 15:48
  • I'm not sure how that's relevant to what I'm saying. Is the above code not susceptible to the issues related to Fragments being destroyed and recreated? If not, why? If it is, then how do you handle making sure the reference to the action is not lost? – MPavlak Aug 11 '16 at 15:52
  • If your fragment would be automagically recreated(fx after Activity recreation) and you did not set `_dateSelectedHandler` then it would do the default action (do nothing) (so, yes, there will be no NPE but also there will be no other action) – Selvin Aug 11 '16 at 15:54
  • How to properly set the reference to the Action so that it will not be lost? With value types or serializable types, you just put them into the Bundle. What to do here? – MPavlak Aug 11 '16 at 15:55
  • ... huh ... it depends ... where you set the listener(Action) ... if in parent Activity ... then do not use Action but as i wrote implement interface `IDoSomthingWithDate` in your Activity ... then in Fragment check if parent activity implements the interface then cast and call the method .. so even if a) fragment was recrated then you will give the reference to parent ...b) there is a chance that parent Activity was recreated asle - so if you would keep old references to the action it may point to Activity which is not valid - but with getActivity you would get the right(new) activity – Selvin Aug 11 '16 at 15:58
  • I see you edited your comments. While I agree that the interface approach will work, is there no way to store the Action safely? – MPavlak Aug 11 '16 at 16:02
  • if you still wana use Action then you should set the delegate every time parent Activity hit onCreate (of course it would be a new callback) – Selvin Aug 11 '16 at 16:02
  • https://developer.android.com/training/basics/fragments/communicating.html – Selvin Aug 11 '16 at 16:04
  • That links describes communicating between fragments. I'm not doing that. I just create something in my activity. – MPavlak Aug 11 '16 at 16:07

0 Answers0