3

So, everyone knows that passing a Context reference (which is not Application) to ViewModel is a bad thing. In my case, there are some items to be ordered alphabetically using an android string resource representation (so, to read it I need an Activity Context).

What is the recommended way to do it? Passing a List of items from ViewModel to Activity, to read those strings and back to ViewModel does look a bit not so MVVM-ish, and injecting ViewModel with a string resource reader would leak the Context..

Any thoughts on that?

Ryan M
  • 18,333
  • 31
  • 67
  • 74
DoruAdryan
  • 1,314
  • 3
  • 20
  • 35
  • Use [`AndroidViewModel`](https://developer.android.com/reference/android/arch/lifecycle/AndroidViewModel) which holds reference to `Application` context that's not susceptible to leaking. Can you post some code snippet why application context would not be good enough? – Pawel Sep 02 '19 at 17:30
  • You don't need an `Activity` to read a String from Resources – PPartisan Sep 02 '19 at 17:39
  • Using Application Context, as @dglozano also stated in his Answer, is a bad practice, as it does not react to Local configuration change, displaying obsolete Strings, so I kind of need the Activity Context to load the correct strings. – DoruAdryan Sep 04 '19 at 06:22

2 Answers2

3

One option would be to extend from AndroidViewModel instead, which has a reference to the Application Context. You can then use that Context to load the string resources and deliver them back to your Activity.

public class MyViewModel extends AndroidViewModel {
    private final LiveData<String> stringResource = new MutableLiveData<>();

    public MyViewModel(Application context) {
        super(context);
        statusLabel.setValue(context.getString(R.string.labelString));
    }

    public LiveData<String> getStringResource() {
        return stringResource;
    }
}

However, as it is pointed out in this Android Developers Medium Post by Jose Alcerreca, this is not the recommended practice because if, for example, the Locale changes and the Activity gets rebuilt, the ViewModel will not react to this configuration change and will keep delivering the obsolete strings (from the previous Locale).

Therefore, the suggested approach is to only return the resources ids from the ViewModel and get the strings on the Activity.

public class MyViewModel extends ViewModel {
    public final LiveData<Integer> stringResource = new MutableLiveData<>();

    public MyViewModel(Application context) {
        super(context);
        stringResource.setValue(R.string.labelString);
    }

    public LiveData<Integer> getStringResource() {
        return stringResource;
    }
}

UPDATE

Since you must get the string resources from your Activity but apply the sorting logic in your ViewModel, I don't think you can't avoid passing the List<String> back to your ViewModel to be sorted:

public class MyViewModel extends ViewModel {
    public final MutableLiveData<Integer> stringArrayId = new MutableLiveData<>();

    public MyViewModel(Application context) {
        super(context);
        stringArrayId.setValue(R.array.string_array_id);
    }

    public LiveData<Integer> getStringArrayId() {
        return stringArrayId;
    }
}


public class MyActivity extends AppCompatActivity {

    public void onCreate(Bundle savedInstanceState) {
        MyViewModel viewModel = ViewModelProviders.of(this).get(MyViewModel.class);

        viewModel.getStringArrayId().observe(this, strArrayId -> {
            String[] resolvedStrings = getResources().getStringArray(strArrayId);
            List<String> sortedStrings = viewModel.sortList(Arrays.asList(resolvedStrings));
            updateUi(sortedStrings);
        });
    }
}

If you think that's not MVVM'ish enough, maybe you can keep resolved List<String> in your ViewModel and have an extra LiveData with the sorted list, that will be updated every time the LiveData holding the original string list changes.

public class MyViewModel extends ViewModel {
    public final MutableLiveData<Integer> stringArrayId = new MutableLiveData<>();
    public final MutableLiveData<List<String>> stringsList = new MutableLiveData<>();
    public final LiveData<List<String>> sortedStringList;


    public MyViewModel(Application context) {
        super(context);
        stringArrayId.setValue(R.array.string_array_id);

        sortedStringList = Transformations.map(stringsList, l -> {
            Collections.sort(l);
            return l; 
        });
    }

    public LiveData<Integer> getStringArrayId() {
        return stringArrayId;
    }

    public LiveData<List<String>> sortedStringList() {
        return sortedStringList;
    }

    public void setStringsList(List<String> resolvedStrings) {
        stringsList.setValue(resolvedStrings);
    }
}


public class MyActivity extends AppCompatActivity {

    public void onCreate(Bundle savedInstanceState) {
        MyViewModel viewModel = ViewModelProviders.of(this).get(MyViewModel.class);

        viewModel.getStringArrayId().observe(this, strArrayId -> {
            String[] resolvedStrings = getResources().getStringArray(strArrayId);
            viewModel.setStringsList(Arrays.asList(resolvedStrings));
        });

        viewModel.sortedStringList().observe(this, sortedStrings -> updateUi(sortedStrings));
    }
}

It feels over-engineered to me, and you still have to send the List<String> back to your ViewModel. However, having it this way might help if the sorting order depends on a Filter that can change during runtime. Then, you can add a MediatorLiveData to react either when the Filter changes or the list of Strings changes, then your view only have to inform those changes to the ViewModel and will observe the sorted list.

dglozano
  • 6,369
  • 2
  • 19
  • 38
  • 1
    I am aware of AndroidViewModel, but as you also stated, is a bad practice using the Application Context for loading Strings. – DoruAdryan Sep 04 '19 at 06:23
  • @DoruAdryan yeap, it's a bad practice to use Context to load strings in the ViewModel. That's why you should get only the string id from the VM and load your strings in your Activity. Then, you can do the sorting in the Activity itself. Is your question answered or is there anything else ? – dglozano Sep 04 '19 at 10:04
  • As I said in the question itself, `Passing a List of items from ViewModel to Activity, to read those strings and back to ViewModel does look a bit not so MVVM-ish`, as I have to move 'business logic' to the View. I was looking for a better solution, if it is one.. – DoruAdryan Sep 05 '19 at 06:54
  • Yeah, that is what I have done in the end, I still don't like that I have logic in Fragment. View's only purpose should only be to care about observing the processed data (from ViewModel) to display it, leaving ViewModel to care about the rest. I am accepting your answer. – DoruAdryan Sep 05 '19 at 11:45
1

Ideally Data Binding should be used with which this problem can easily be solved by resolving the string inside the xml file. But implementing data binding in an existing project can be too much.

For a case like this I created the following class. It covers all cases of strings with or without arguments and it does NOT require for the viewModel to extend AndroidViewModel and this way also covers the event of Locale change.

class ViewModelString private constructor(private val string: String?,
                                          @StringRes private val stringResId: Int = 0,
                                          private val args: ArrayList<Any>?){

    //simple string constructor
    constructor(string: String): this(string, 0, null)

    //convenience constructor for most common cases with one string or int var arg
    constructor(@StringRes stringResId: Int, stringVar: String): this(null, stringResId, arrayListOf(stringVar))
    constructor(@StringRes stringResId: Int, intVar: Int): this(null, stringResId, arrayListOf(intVar))

    //constructor for multiple var args
    constructor(@StringRes stringResId: Int, args: ArrayList<Any>): this(null, stringResId, args)

    fun resolve(context: Context): String {
        return when {
            string != null -> string
            args != null -> return context.getString(stringResId, *args.toArray())
            else -> context.getString(stringResId)
        }
    }
}

USAGE

for example we have this resource string with two arguments

<string name="resource_with_args">value 1: %d and value 2: %s </string>

In ViewModel class:

myViewModelString.value = ViewModelString(R.string.resource_with_args, arrayListOf(val1, val2))

In Fragment class (or anywhere with available context)

textView.text = viewModel.myViewModelString.value?.resolve(context)

Keep in mind that the * on *args.toArray() is not a typing mistake so do not remove it. It is syntax that denotes the array as Object...objects which is used by Android internaly instead of Objects[] objects which would cause a crash.

Anonymous
  • 4,470
  • 3
  • 36
  • 67
  • Problem here was not to display the string resolved by a Context into the View, the problem was to manipulate the list (order alphabetically by a String which would be resolved by an Activity Context), and then continue to do something with them in ViewModel. – DoruAdryan Feb 01 '21 at 08:44