7

Basically, If I use MVVM and expose public ICommands, should my delegates be public or private?

michael
  • 14,844
  • 28
  • 89
  • 177

3 Answers3

11

I would make them private - they aren't part of your class's public interface, that's what the public ICommand properties are for.

Dan J
  • 16,319
  • 7
  • 50
  • 82
  • My class doesn't have an interface though, it's just a class. – michael Jul 28 '11 at 19:03
  • 3
    @michael The collection of `public` methods and properties on a class is considered that class's "interface", insofar as that's how consumers interface with that class. – dlev Jul 28 '11 at 19:04
  • 4
    I am not referring to the .NET construct of an [Interface](http://msdn.microsoft.com/en-us/library/ms173156.aspx), I am referring to [the set of operations that your class makes available to external consumers.](http://en.wikipedia.org/wiki/Public_interface) If we've cleared that up, I'd appreciate it if you'd remove your downvote (assuming that's where it came from). :) – Dan J Jul 28 '11 at 19:05
  • 1
    @dj: I completely agree. Exposing the ICommand properties is the way to go (as `get; private set;`) and then using those to call the `CanExecute` and `Execute` members properly is the right thing to do. – myermian Jul 28 '11 at 19:07
4

Personally, I'd go with private methods and I'll tell you why. You're exposing an ICommand, which to me says that the consuming view should call a CanExecute prior to calling an Execute. If they don't, they're going against the API and shooting themselves in the foot and at that point it is out of your hands. Just like if someone used reflection to set an important private variable to null and broke your class design because of this... shooting themselves in the foot. So why make the members private? Because there is no need to expose members that should not be called directly.


Basically, when you unit test the members you don't do so individually, you do so in the way the API intends for the members to be executed. So, you're not really testing the members, but moreover you're testing the command, which again means they should be tested in a pair in the specific order of:

if (CanExecute)
{
    Execute;
}
myermian
  • 31,823
  • 24
  • 123
  • 215
  • @chibacity: Sometimes I tend to rant. I've asked something similar before when it comes to unit testing and someone told me that if people go against the API then that's on them. You can only go so far to protecting someone from destroying a proper library or app. – myermian Jul 28 '11 at 19:09
2

I have MVVM for something simple control of increase, decrease buttons and Slider show value.

If you have test ICommand and INotifyPropertyChanged, you can make kind of UnitTest:

[TestMethod]
public void TestViewModel3()
{
    int min = -10;
    int max = 10000;
    int initVal = 50;
    bool initState = false;

    ToglledSliderModel model = new ToglledSliderModel(initState, initVal, min, max);
    ToglledSliderViewModel viewModel = new ToglledSliderViewModel();
    viewModel.Model = model;

    int status = 567;
    viewModel.PropertyChanged += delegate
    {
        status = 234;
    };

    for (int i = 1; i < 100; i++)
    {
        status = 567;
        ICommand ic = viewModel.IncreaseValue;
        ic.Execute(this);
        Thread.Sleep(2);
        Assert.AreEqual(status, 234);
        Assert.AreEqual(model.SliderValue, initVal + i);
    }
}

you can see, i test INotifyPropertyChanged behaviour and ICommand executing

zzfima
  • 1,528
  • 1
  • 14
  • 21