expense tracker - part 6

Made more progress with my AddExpenses UserControl - made a ViewModel class called AddExpenseViewModel that implements all of the logic whenever a user clicks the Add button.  I was able to figure out how to bind my add button functionality in the ViewModel, so my code behind for AddExpenses is clean for now.  Also, I just made the list of expenses a separate ViewModel class, but it is contained in my ExpenseViewModel class.

So here is my new class, my ExpensesCollection class:

public static class ExpensesCollection {
  public static ObservableCollection<Expenses> AllExpenses = new ObservableCollection<Expenses>();
  static IsolatedStorageSettings storage = IsolatedStorageSettings.ApplicationSettings;

  public static bool ContainsExpense(string s) {
    foreach (Expenses e in AllExpenses) {
      if (e.Expense == s) { return true; }
    }
    return false;
  }

  public static Expenses Remove(string s) {
    foreach (Expenses e in AllExpenses) {
      if (e.Expense == s) {
        AllExpenses.Remove(e);
        storage.Remove(s);
        storage.Save();
        return e;
      }
    }
    string message = String.Format("{0} is not in your list", s);
    MessageBox.Show(message);
    return null;
  }

  public static void Add(string s, double d) {
    AllExpenses.Add(new Expenses() { Expense = s, Cost = d });
    if (!storage.Contains(s)) {
      storage.Add(s, d);
    } else {
      storage[s] = d;
    }
    storage.Save();
  }
}

I think there's some room for improvement here but I'll let it be for now.

As a result of this new class I made some changes to my ExpenseViewModel -
Replaced my AllExpenses with ExpensesCollection.AllExpenses, named ExpenseList in my file,
and my new GetSavedExpenses:

private void GetSavedExpenses() {
  foreach (string s in storage.Keys) {
    double d;
    if (Double.TryParse(storage[s].ToString(), out d)) {
      var expense = new Expenses() { Expense = s, Cost = d; };
      ExpenseList.Add(expenses);
    }
  }
}

Yes I realize the if statement is probably unnecessary - the expense, cost pair wouldn't be allowed to be added in the first place if it was an invalid double.  But I have to call TryParse anyway so might as well add a tiny check.

The only changes I made to my AddExpenses.xaml file is binding the text property of my textboxes to properties in my ViewModel -
for txtInput:

Text="{Binding CurrentTxt, Mode=TwoWay}"

and for numInput:

Text="{Binding CurrentNum, Mode=TwoWay}"

And binding my button Command to my AddCommand property in the ViewModel:

Command="{Binding AddCommand}"

Here's the code for the ViewModel itself:

public class AddExpenseViewModel : INotifyPropertyChanged {
  IsolatedStorageSettings storage = IsolatedStorageSettings.ApplicationSettings;

  public AddExpenseViewModel() {
    InitializeCommand();
  }

  #region AddCommand

  private ICommand _AddCommand;
  public ICommand AddCommand {
    get { return _AddCommand; }
    set {
      _AddCommand = value;
      OnPropertyChanged("AddCommand");
    }
  }

  private void InitializeCommand() {
    AddCommand = new AddCommand(UpdateExpensePair);
  }

  private void UpdateExpensePair() {
    var ExpenseStorage = ExpenseViewModel.AllExpenses;

    string s = CurrentTxt;
    double d;
    if (double.TryParse(CurrentNum, out d)) {
      if (ExpensesCollection.ContainsExpense(CurrentTxt) {
        Expenses e = ExpensesCollection.Remove(s);
        d += e.Cost;
      }
      ExpensesCollection.Add(s, d);
    } else {
      MessageBox.Show("Integers and decimals only, please");
    }
  }

  #endregion

  #region Text(Expense) Setting

  const string DEFAULT_TXT = "expense";

  private string _CurrentTxt = "";
  public string CurrentTxt {
    get { return _CurrentTxt; }
    set {
      _CurrentTxt = value;
      OnPropertyChanged("CurrentTxt");
    }
  }

  private bool _DefaultTxtSettings = true;
  public bool DefaultTxtSettings {
    get { return _DefaultTxtSettings; }
    set {
      _DefaultTxtSettings = value;
      OnPropertyChanged("DefaultTxtSettings");
    }
  }

  #endregion

  #region Number(Cost) Settings

  const double DEFAULT_NUM = 0;

  private string _CurrentNum = "";
  public string CurrentNum {
    get { return _CurrentNum; }
    set {
      _CurrentNum = value;
      OnPropertyChanged("CurrentNum");
    }
  }

  private bool _DefaultNumSettings = true;
  public bool DefaultNumSettings {
    get { return _DefaultNumSettings; }
    set {
      _DefaultNumSettings = true;
      OnPropertyChanged("DefaultNumSettings");
    }
  }

  #endregion

  #region INotify Implementation

  private void OnPropertyChanged(string p) {
    if (PropertyChanged != null) {
      PropertyChanged(this, new PropertyChangedEventArgs(p));
    }
  }

  public event PropertyChangedEventHandler PropertyChanged;

  #endregion
}

#region AddCommand Class

public class AddCommand : ICommand {
  Action _executeMethod;

  public bool CanExecute(object parameter) {
    return true;
  }

  public event EventHandler CanExecuteChanged;

  public void Execute(object parameter) {
    _executeMethod.Invoke();
  }

  public AddCommand(Action updateExpensePair) {
    _executeMethod = updateExpensePair;
  }
}

#endregion

Whew.  It's still not complete, I have some more work to do with the AddCommand class, like actually putting in logic for CanExecute.  I noticed there are two functions that do similar things - the Add command in my ExpensesCollection class and my UpdateExpensePair() that is called with the Add button is clicked.  In ExpensesCollection, the Add method assumes that the double value is already the correct value - what I mean is that if you add to an expense that already exists, you leave the string alone but update the double value.  That logic is done in UpdateExpensePair(), but I  suppose I could have moved the logic to my Add class.  My question is, where is it better to implement that logic?  Already there are assumptions in my code about the values it is being passed, what if something weird happens and those assumptions end up being wrong?  But I don't want to do unnecessary checking, either.  In school we just cleaned the input in the calling function, and the callee assumes it is being given valid input so that's just what I did here.  Is that convention?  It's probably better to comment the code, warning that this particular function assumes things about its input.  I could probably implement some error handling too, I guess.

Anyway..

Finally, in the MainPage codebehind, I had to instantiate a new AddExpenseViewModel and set AddExpenseViewOnPage.DataContext to that ViewModel.

So I think the basic logic is complete.  Codewise, it's better than my previous version of this app but it doesn't work as seamlessly when the user is interacting with the app itself.  I might have to add codebehind in my AddExpenses UserControl to control the visual appearance of my TextBoxes, but for now I'm ok with that.

I'd also like to add that while I completely revamped the logic - the functionality and appearance still stayed the same and I only had one bug while making these changes, and it was pretty easy to fix.  I was surprised at how seamless these changes were - I guess that's the point of going through all this trouble to implement this pattern!