Search code examples
c#xamarinmvvmbindingrefactoring

Decompose huge legacy ViewModels/Views and implement immutability


In our app there are some pretty horrible ViewModels and Fragments that now we need to refactor. They are huge and should be decomposed of course. But proper way of decomposition isn't apparent to me( Order of classes and data loading leads to binding difficulties

p.s. binding framework is leagacy and custom(

BEFORE: huge and cluttered classes

public class HugeLegacyViewModel : ViewModelBase
{
    public TextViewModel FooText {get; set;}
    public ListViewModel FooList {get; set;} = new ListViewModel();

    public TextViewModel BarText {get; set;}
    public ListViewModel BarList {get; set;} = new ListViewModel();

    public HugeLegacyViewModel(IDependancy1 dependancy1)
    {
        ...
    }

    public void LoadData() // is called after object creation
    {
        var data = ...
        FooText = data.Foo.TextDto;
        FooList = data.Foo.ListDto.ToListViewModel();
        BarText = data.Bar.TextDto;
        BarList = data.Bar.ListDto.ToListViewModel();
    }
}


public class HugeLegacyFragment : FragmentBase<HugeLegacyViewModel>
{
    public void OnViewCreate(View view)
    {
        var fooText = view.FindLayoutById(Resources.Layout.fooTextLayout);
        var barText = view.FindLayoutById(Resources.Layout.barTextLayout);
        var fooList = view.FindLayoutById(Resources.Layout.fooListLayout);
        var barList = view.FindLayoutById(Resources.Layout.barListLayout);

        ShittyLegacyBinder.CreateBindings(
            () => fooText.Text == ViewModel.FooText.Text,
            () => fooText.Font == ViewModel.FooText.Font,
            () => barText.Text == ViewModel.BarText.Text,
            () => barText.Font == ViewModel.BarText.Font,

            () => fooList.List == ViewModel.FooList.List,
            () => fooList.Title == ViewModel.FooList.Title,
            () => barList.List == ViewModel.BarList.List,
            () => barList.Title == ViewModel.BarList.Title
        );
    }
}

AFTER: set of tiny classes

public class FooViewModel : ViewModelBase
{
    public string Text {get;}
    public ReadOnlyCollection List {get;}
    ...
}

public class CoolNewViewModel : ViewModelBase
{
    public FooViewModel FooVm {get; private set;} = new FooViewModel();
    public BarViewModel BarVm {get; private set;} = new BarViewModel();

    public CoolNewViewModel(IDependancy1 dependancy1)
    {
        ...
    }

    public void LoadData()
    {
        var data = ...
        FooVm = data.Foo.ToVm();
        BarVm = data.Bar.ToVm();
    }
    ...
}

public class FooView : View
{
    private FooViewModel _vm;
    private TextView _textVew;
    private ListView _listView;

    public FooView(FooViewModel vm, TextView textView, ListView listView)
    {
        ...
        ShittyLegacyBinder.CreateBindings(
            () => _textVew == _vm.Text,
            () => _listView == _vm.List
        );
    }
}

public class CoolNewFragment : FragmentBase<HugeLegacyViewModel>
{
    private FooView? _foo;
    private BarView? _bar;

    public void OnViewCreate(View view) 
    // issue is here: OnViewCreate() is called eralier than LoadData() in vm,
    // so Views are now binded to instances that will be replaced in LoadData()
    //
    // bindings lead to stub empty classes
    {
        _foo = new FooView(
            vm: ViewModel.FooVm,
            textView: view.FindLayoutById(Resources.Layout.fooTextLayout),
            listView: view.FindLayoutById(Resources.Layout.fooListLayout)
        );
        _bar = new BarView(
            vm: ViewModel.BarVm,
            textView: view.FindLayoutById(Resources.Layout.barTextLayout),
            listView: view.FindLayoutById(Resources.Layout.barListLayout)
        );
    }
    ...
}

I would like to implement immutability for properties and fields of new classes, but it's quite challenging

Issue is described in comment above: although properties should be immutable, actually in vm they can't. Data is loaded after instance creation, so FooVm and BarVm will be overwritten. Due to fact that Fragment's code executes before vm's, bindings will be lost after overwriting(

There are some not-as-good-as-they-may-be solutions provided bellow:

SOLUTION 1: forget immutability

public class FooViewModel1 : ViewModelBase
{
    public MutableString Text {get;}
    public Collection List {get;}

    public void ReplaceData(Foo data)
    {
        Text.String = data.Text;
        List.ReplaceAllWith(data.List);
    }
}

public class MediocreNewViewModel1 : ViewModelBase
{
    public FooViewModel FooVm {get; private set;} = new FooViewModel();
    public BarViewModel BarVm {get; private set;} = new BarViewModel();

    public MediocreNewViewModel1(IDependancy1 dependancy1)
    {
        ...
    }

    public void LoadData()
    {
        var data = ...
        FooVm.ReplaceData(data.Foo);
        BarVm.ReplaceData(data.Bar);
    }
    ...
}

SOLUTION 2: aliases in main vm

public class FooViewModel2 : ViewModelBase
{
    public string Text {get;}
    public ReadOnlyCollection List {get;}
    ...
}

public class MediocreNewViewModel2 : ViewModelBase
{
    private FooViewModel _fooVm = new FooViewModel();
    private BarViewModel _barVm = new BarViewModel();

    public string FooText => _fooVm.Text;
    public string FooList => _fooVm.List;
    public string BarText => _barVm.Text;
    public string FooList => _barVm.List;

    public MediocreNewViewModel2(IDependancy1 dependancy1)
    {
        ...
    }

    public void LoadData()
    {
        var data = ...
        _fooVm = data.Foo.ToVm();
        _barVm = data.Bar.ToVm();
    }
    ...
}

public class FooView2 : View
{
    private TextView _textVew;
    private ListView _listView;

    public FooView2(string text, ReadOnlyCollection list TextView textView, ListView listView)
    {
        ...
        ShittyLegacyBinder.CreateBindings(
            () => _textVew == text,
            () => _listView == list
        );
    }
}

public class MediocreNewFragment2 : FragmentBase<HugeLegacyViewModel>
{
    private FooView2? _foo;
    private BarView2? _bar;

    public void OnViewCreate(View view) 
    {
        _foo = new FooView2(
            text: ViewModel.FooText,
            list: ViewModel.FooList,
            textView: view.FindLayoutById(Resources.Layout.fooTextLayout),
            listView: view.FindLayoutById(Resources.Layout.fooListLayout)
        );
        _bar = new BarView2(
            text: ViewModel.BarText,
            list: ViewModel.BarList,
            textView: view.FindLayoutById(Resources.Layout.barTextLayout),
            listView: view.FindLayoutById(Resources.Layout.barListLayout)
        );
    }
    ...
}

SOLUTION 3: wrapper class mapping View to actual Vm

public class FooViewModel3 : ViewModelBase
{
    public string Text {get;}
    public ReadOnlyCollection List {get;}
    ...
}

public class FooViewModelWrapper
{
    private FooViewModel3 _vm;

    public string Text => _vm.Text;
    public ReadOnlyCollection List => _vm.List;

    public FooViewModelWrapper(FooViewModel3 vm)
    {
        ...
    }
}

public class MediocreNewViewModel3 : ViewModelBase
{
    private FooViewModel _fooVm = new FooViewModel();
    private BarViewModel _barVm = new BarViewModel();

    public FooViewModelWrapper FooVm {get; private set;} = new FooViewModelWrapper(_fooVm);
    public BarViewModelWrapper BarVm {get; private set;} = new BarViewModelWrapper(_barVm);

    public CoolNewViewModel(IDependancy1 dependancy1)
    {
        ...
    }

    public void LoadData()
    {
        var data = ...
        _fooVm = data.Foo.ToVm();
        _barVm = data.Bar.ToVm();
    }
    ...
}

public class FooView3 : View
{
    private FooViewModelWrapper _vm;
    private TextView _textVew;
    private ListView _listView;

    public FooView3(FooViewModelWrapper vm, TextView textView, ListView listView)
    {
        ...
        ShittyLegacyBinder.CreateBindings(
            () => _textVew == _vm.Text,
            () => _listView == _vm.List
        );
    }
}

public class MediocreNewFragment3 : FragmentBase<HugeLegacyViewModel>
{
    private FooView3? _foo;
    private BarView3? _bar;

    public void OnViewCreate(View view)
    {
        _foo = new FooView3(
            vm: ViewModel.FooVm,
            textView: view.FindLayoutById(Resources.Layout.fooTextLayout),
            listView: view.FindLayoutById(Resources.Layout.fooListLayout)
        );
        _bar = new BarView3(
            vm: ViewModel.BarVm,
            textView: view.FindLayoutById(Resources.Layout.barTextLayout),
            listView: view.FindLayoutById(Resources.Layout.barListLayout)
        );
    }
    ...
}

So, question:

is it possible to refactor this without compromises and boilerplate code?


Solution

  • I faced this problem several times later, and came to this solution:

    Every ViewModel has a corresponding View and a method Bind(TViewModel vm). One View <=> one ViewModel.

    View deals with binding content of it's VM, but nothing else. If parent has a nullable content, View waits till content will receive value, and than binds it. If View has other child Views, it only calls their Bind() methods, and lets them deal with binding.

    This helps with loading of data: every ViewModel is always valid and 'bindable'. If some of it's content is not loaded, it's just null, and once loaded - is ready for usage in View.

    Example:

    public class MainViewModel : ViewModel
    {
        public record ContentViewModels(ChildViewModel1 Child1, ChildViewModel2 Child2);
        
        public ContentViewModels? Content { get; private set; }
    
        public async Task LoadContent()
        {
            Content = await _loader.LoadContent();
        }
    }
    
    public class MainView : BindableView<MainViewModel>
    {
        private ChildView1 _childView1 = new();
        
        private ChildView2 _childView2 = new();
        
        public void Bind(MainViewModel vm)
        {
            new EBinding
            {
                //executes on every Content change
                () => BindVmContent(vm.Content), 
            };
        }
        
        private void BindVmContent(MainViewModel.ContentViewModels? content)
        {
            if (content is null)
                return;
            
            _childView1.Bind(content.Child1);
            _childView2.Bind(content.Child2);
        }
    }