2
\$\begingroup\$

Please let me know if the MVVM pattern I followed in this is correct or not.

Here is the code for ViewModel

public class LoginViewModel
{
    public string Email { get; set; } = "";
    public string Password { get; set; } = "";



    public ProgressBar ProgressBar { get; set; }
    public Label StatusLabel { get; set; }

    public Command LoginCommand
    {
        get
        {
            return new Command(async()=> {
                ProgressBar.IsVisible = true;
                ProgressBar.Progress = 0;
                await ProgressBar.ProgressTo(0.2, 250, Easing.BounceOut);


                if(Email.Length>0 && Password.Length > 0)
                {
                    ApiServices apiServices = new ApiServices();
                    var isSuccess = await apiServices.LoginAsync(Email, Password);
                    if (isSuccess)
                    {
                        await ProgressBar.ProgressTo(1, 250, Easing.Linear);
                        Application.Current.MainPage = new FirstMasterDetailPage();
                    }
                    else
                    {
                        await ProgressBar.ProgressTo(1, 250, Easing.Linear);
                        StatusLabel.IsVisible = true;
                        StatusLabel.Text = "Failed to login.";
                    }
                }
                else
                {
                    await ProgressBar.ProgressTo(1, 250, Easing.Linear);
                    StatusLabel.IsVisible = true;
                    StatusLabel.Text = "Username and Password is required to login.";
                }
                ProgressBar.IsVisible = false;
            });
        }
    }

    public Command RegisterCommand
    {
        get
        {
            return new Command(()=> {
                Application.Current.MainPage = new RegisterPage();
            });
        }
    }
}  

this is code behind page

public LoginPage ()
    {
        InitializeComponent ();

        var current = BindingContext as LoginViewModel;
        current.ProgressBar = progressBar;
        current.StatusLabel = lblStatus;
    }
\$\endgroup\$
5
\$\begingroup\$

No, it's not correct.

  1. View-model shouldn't contain anything related to view. It means LoginViewModel shouldn't contain references on progress bar and label. ProgressBar and Label properties inside a view-model violate MVVM.
  2. Any changes in UI elements should be done via bindings specified in XAML. Those bindings have to be linked to properties of a view-model. Changing these properties, UI get updated.
  3. To make bindings work you should notify them about changes in view-model. For that purpose you need to implement INotifyPropertyChanged interface on the view-model and raise PropertyChanged event every time a property changed.

Let's see example of how you can remove ProgressBar from the LoginViewModel.

First of all, view -model should implement INotifyPropertyChanged interface which contains PropertyChanged event:

public class LoginViewModel : INotifyPropertyChanged
{
    public event PropertyChangedEventHandler PropertyChanged;
}

Then remove ProgressBar property and add LoginInProgress:

private bool _loginInProgress;

public bool LoginInProgress
{
    get => _loginInProgress;
    set
    {
        if (value == _loginInProgress)
            return;

        _loginInProgress = value;
        PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(LoginInProgress)));
    }
}

Now in the LoginCommand body instead of

ProgressBar.IsVisible = true;

write

LoginInProgress = true;

And instead of

ProgressBar.IsVisible = false;

write

LoginInProgress = false;

Then in XAML bind Visibility of the progressBar to this property:

<ProgressBar x:Name="progressBar"
             Visibility="{Binding Path=LoginInProgress, Mode=OneWay, Converter="{StaticResource BooleanToVisibilityConverter}"}"
             .../>

Somewhere in resources you should define BooleanToVisibilityConverter:

<BooleanToVisibilityConverter x:Key="BooleanToVisibilityConverter"/>

And that's all. You need to eliminate all the view related stuff from the view model in such way. It is how MVVM have to be implemented.

\$\endgroup\$
  • \$\begingroup\$ can you suggest me what's the correct way of writing ViewModel for this class @Maxim \$\endgroup\$ – Siraj M Sep 25 '17 at 5:54
  • \$\begingroup\$ @Siraj I agree with my learned friend Maxim also - why not set the command property with a method called from the LoginViewModel constructor rather than using an anonymous function - because you'll be instantiating every time the command property is called? secondly why not also post your XAML as well - because data binding to properties exposed by your ViewModel class is essential to the MVVM design pattern. \$\endgroup\$ – BKSpurgeon Sep 25 '17 at 9:09
  • \$\begingroup\$ @Siraj I've added an example of how you can implement MVVM correctly. \$\endgroup\$ – Maxim Sep 28 '17 at 2:30

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.