Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have used INotifyPropertyChanged for both Model and ViewModel. Is it correct? First of all I was trying to use INotifyPropertyChanged only with Model, but for that I had to use observable collections in UI class.

Please comment on my design.

MainPage.xaml

<phone:PhoneApplicationPage
    x:Class="LocLog.MainPage"
    xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
    xmlns:phone="clr-namespace:Microsoft.Phone.Controls;assembly=Microsoft.Phone"
    xmlns:shell="clr-namespace:Microsoft.Phone.Shell;assembly=Microsoft.Phone"
    xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
    xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
    mc:Ignorable="d"
    FontFamily="{StaticResource PhoneFontFamilyNormal}"
    FontSize="{StaticResource PhoneFontSizeNormal}"
    Foreground="{StaticResource PhoneForegroundBrush}"
    SupportedOrientations="Portrait" Orientation="Portrait"
    shell:SystemTray.IsVisible="True">

    <!--LayoutRoot is the root grid where all page content is placed-->
    <Grid x:Name="LayoutRoot" Background="Transparent">
        <Grid.RowDefinitions>
            <RowDefinition Height="Auto"/>
            <RowDefinition Height="*"/>
            <RowDefinition Height="100"/>

        </Grid.RowDefinitions>


        <!--TitlePanel contains the name of the application and page title-->
        <StackPanel x:Name="TitlePanel" Grid.Row="0" Margin="12,17,0,28">
            <TextBlock Text="MY APPLICATION" Style="{StaticResource PhoneTextNormalStyle}" Margin="12,0"/>
            <TextBlock Text="page name" Margin="9,-7,0,0" Style="{StaticResource PhoneTextTitle1Style}"/>
        </StackPanel>

        <!--ContentPanel - place additional content here-->
        <Grid x:Name="ContentPanel" Grid.Row="1" Margin="12,0,12,0">
            <ScrollViewer>
                <TextBlock Text="{Binding Path=logText}"></TextBlock>

            </ScrollViewer>
        </Grid>

        <Button Name="btn" Height="70" Width="130" Grid.Row="2" Click="btn_Click">Refresh</Button>

    </Grid>

</phone:PhoneApplicationPage>

Model

using System.ComponentModel;
using System.Windows;

namespace LocLog
{
    class Model : INotifyPropertyChanged
    {
        private string dispalyText="";
        public string DispalyText
        {
            get
            {
                return dispalyText;
            }

            set
            {
                dispalyText = value;

                if (PropertyChanged != null)
                    PropertyChanged(this, new PropertyChangedEventArgs("DispalyText"));
            }
        }

        public Model()
        {
        }



        public event PropertyChangedEventHandler PropertyChanged;
    }
}

ViewModel

using Microsoft.Phone.Reactive;
using StorageMech;
using System.ComponentModel;
using System.Windows;

namespace LocLog
{
    public class ViewModel:INotifyPropertyChanged
    {
        private   Model model;
        Store storage;
        private string LogText;
        public string logText 
        { 
            get 
            {
                return LogText;
            }

            set
            {
                LogText = value;
                if(PropertyChanged != null)
                    PropertyChanged(this,new PropertyChangedEventArgs("logText"));
            }
        }

        //Get log files from storage to dispaly on UI
        public void RefreshLog()
        {

            model.DispalyText = storage.ReadFromFile();

        }

        public ViewModel()
        {
            storage = new Store();
            model = new Model();
            model.PropertyChanged += model_PropertyChanged;
        }

        void model_PropertyChanged(object sender, System.ComponentModel.PropertyChangedEventArgs e)
        {
            this.logText = model.DispalyText;
        }

        public event PropertyChangedEventHandler PropertyChanged;
    }
}

Storage Class

using System;
using System.IO;
using System.IO.IsolatedStorage;
using System.Net;
using System.Threading;
using System.Windows;

namespace StorageMech
{
    public class Store
    {
        Mutex m = new Mutex(false, "LOCLOGIC");

        string strFileName = "locLogFile";
        IsolatedStorageFile file;
        IsolatedStorageFileStream fileStream;

        public Store()
        {
            file = IsolatedStorageFile.GetUserStoreForApplication();

        }

        public void SaveToLog(string str)
        {
            try
            {
                m.WaitOne();

                if (file != null)
                {
                    //open file in append mode
                    fileStream = new IsolatedStorageFileStream(strFileName, System.IO.FileMode.Append, file);
                }

                using (var streamWriter = new StreamWriter(fileStream))
                {
                    streamWriter.WriteLine(str);
                }
            }
            catch(Exception ex)
            {
                MessageBox.Show(ex.StackTrace);
            }
            finally
            {
                if (fileStream != null)

                fileStream.Close();
                m.ReleaseMutex();
            }
        }

        public string ReadFromFile()
        {
            try
            {
                m.WaitOne();
                string line="";

                if (file != null)


         {

                fileStream = new IsolatedStorageFileStream(strFileName, System.IO.FileMode.Open, file);
            }

            using (var streamReader = new StreamReader(fileStream))
            {
                string temp;
                //read line by line and finally return
                while ((temp = streamReader.ReadLine()) != null)
                {
                    line = line + temp+"\n";
                }
            }

            return line;
        }
        catch (Exception ex)
        {
            MessageBox.Show(ex.Message);
            return "";
        }
        finally
        {
            if(fileStream != null)
                fileStream.Close();
            m.ReleaseMutex();
        }

    }

}

}

share|improve this question

1 Answer 1

up vote 2 down vote accepted

Only your viewmodel should implement InotifyPropertyChanged. That's kind of the whole point of mvvm. Normally, your viewmodel properties wrap your model properties. Also, it is your model, that should contain all the business logic. Viewmodel should know nothing about Storage or any other details of implementation. Viewmodel should only manage interactions with UI. Being said, you should have something like:

    class Model
    {
        private Storage storage = new Storage();
        public string DispalyText { get; set; }

        public void RefreshLog()
        {
            DispalyText = storage.ReadFromFile();
        }
    }

    public class ViewModel : INotifyPropertyChanged
    {
        private   Model model;

        //you should use capital first letter for properties
        public string LogText 
        { 
            get 
            {
                return model.DispalyText;
            }
            set
            {
                model.DispalyText = value;
                if(PropertyChanged != null)
                    PropertyChanged(this,new PropertyChangedEventArgs("LogText"));
            }
        }

        public void RefreshLog()
        {
            model.RefreshLog();
            //this call should be refactored to separate method
            if(PropertyChanged != null)
                    PropertyChanged(this,new PropertyChangedEventArgs("LogText")); 
        }

        public ViewModel()
        {
            model = new Model();
        }

        public event PropertyChangedEventHandler PropertyChanged;
    }
}
share|improve this answer
    
Great answer, just one more thing, what exactly should I write in MainPage.xaml.cs to get VM's changes back? –  Pranit Kothari Feb 18 at 10:47
    
@pranitkothari, nothing. As long as you use data binding to a property in xaml, the changes will be automatically propagated to UI whenever you raise a PropertyChanged event for that property. –  Nikita Brizhak Feb 18 at 10:53
    
+1. Thanks Nikita, you solved my problem. –  Pranit Kothari Feb 18 at 10:53

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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