8 Replies - 2303 Views - Last Post: 17 March 2014 - 12:00 PM Rate Topic: -----

#1 andrewsw  Icon User is offline

  • blow up my boots
  • member icon

Reputation: 6544
  • View blog
  • Posts: 26,526
  • Joined: 12-December 12

MVP very simple example

Posted 16 March 2014 - 04:37 PM

I want to create a very simple example of the MVP (Model View Presenter) pattern using WinForms. My application doesn't use formal data-binding, formal notifying of property changes, or any events in the presentation layer, etc. These are things I will move onto later.

My questions are:

  • At what point does my example currently fail to demonstrate this model? I feel sure that it does, so be honest please ;)
  • What changes, or additions, should I make to move this example forward? I don't need code! Just some suggestions please.


Attached Image

I apologise in advance for the length of the code. For more experienced members here, though, I'm hoping it won't be too tricky for them to scan through it ;)

The model is simple:
namespace SeparationMVP {
    class Task {
        public string Name { get; set; }
        public string Priority { get; set; }
        public DateTime StartDate { get; set; }
        public DateTime DueDate { get; set; }
        public bool Completed { get; set; }
        public DateTime CompletionDate { get; set; }
    }
}


The interface: (can this be called the messaging layer?)

Spoiler

With this I am using StatusChange and isDirty as, effectively, messaging between the model (via the presenter) and the view. StatusChange is just a string which the view can read and, if it so chooses, display in a label.. or ignore.

isDirty can be changed by both the view and the presenter. I originally used a (DirtyTask) event of the form (view), but all the presenter did for this event was to set its own isDirty property, so it seemed redundant to complicate it with an event.

The form (view) code:

Spoiler

What interests me with this code is the use of the following if isDirty is true:
        private DialogResult AbandonEdit() {
            return MessageBox.Show("Abandon current editing?", "Abandon", 
                MessageBoxButtons.YesNo, MessageBoxIcon.Question);
        }


I understand that there are differences of opinion about where validation should occur, although I believe that model itself should also have validation/constraints: integrity. However, I wanted to produce a MessageBox, and I couldn't see how to do this sensibly from the presenter? That is, to produce a MessageBox that was still in the context of the Form. I realise that it could supply view as the owner of the MessageBox but wasn't quite happy with this.

Finally, the presenter:

Spoiler

I appreciate that this:
        private int currentIndex = 0;
        private bool isNew = true;


is a primitive way to maintain state between the model and view but, for a very basic MVP example, I hope that it is reasonable?

Note: I am using DateTimePickers and rather than trying to deal with non-values (error handling and messaging) I'm just defaulting to a specific date of "1999-01-01".

Just some suggestions and general advice will be very much appreciated ;)

This post has been edited by andrewsw: 16 March 2014 - 04:56 PM


Is This A Good Question/Topic? 0
  • +

Replies To: MVP very simple example

#2 Skydiver  Icon User is online

  • Code herder
  • member icon

Reputation: 6164
  • View blog
  • Posts: 21,252
  • Joined: 05-May 12

Re: MVP very simple example

Posted 16 March 2014 - 07:23 PM

The following is mostly my opinion. It would be interesting to hear other people's points of view.

I've been taught that the correct (but in my opinion inefficient) way for validation in MVP with Passive View was that the Presenter would attempt to update the Model with the data from the View. If the Model throws an exception due to invalid data, then it was Presenter's job to bring the View and Model back into a consistent state -- typically through the implementation of the Command design pattern.

Most people I know have implemented MVP where there is duplicate code between the the Model, View, and Presenter as to what constitutes valid data. Although less efficient (and more error prone) because of the duplication of code, it also cuts down on the chattiness of the objects because each components wouldn't try to pass on data that it knows in correct.

Let's take for example the CompletionDate and the StartDate. The CompletionDate shouldn't be before the StartDate. If we followed pure MVP, the View shouldn't even care whether these fields were strings, numbers, or dates, it would just pass in whatever the user enter back out of it.

Obviously, though we want some nicer presentation, so now we limit these fields to be dates. So now we have one level of duplication for validation already between the model and the view. (Some people claim that type information in not duplication of validation information, but my counter is that if all you had was Console.ReadLine() for input, don't you need extra code to try to parse the string into a DateTime, and considering the input invalid if it the parsing fails?)

Next we want to enforce the business rule that the CompletionDate is not less than the StartDate. We should have that validation in the Model, but then as a good UI designers, we should also prevent the user from entering such data. Now we have another level of duplication for validation.
Was This Post Helpful? 1
  • +
  • -

#3 Skydiver  Icon User is online

  • Code herder
  • member icon

Reputation: 6164
  • View blog
  • Posts: 21,252
  • Joined: 05-May 12

Re: MVP very simple example

Posted 17 March 2014 - 05:53 AM

As a quick aside, as I recall there was a major Microsoft security vulnerability that was partly a result of the original MVP design. There was a COM object (the model) which assumed that the program and it's GUI (the presenter and the view) would do all the validation prior to passing on data to the model. All was good because it was just one integrated product until a couple of years later, the COM object started getting redistributed separately. People doing the code reviews didn't catch the problem because they had designed the original system. In the back of their minds no input fuzzing would ever happen because the GUI would only let good data into the COM object, and so they didn't think to consider what would happen if bad data made it into the model.
Was This Post Helpful? 0
  • +
  • -

#4 andrewsw  Icon User is offline

  • blow up my boots
  • member icon

Reputation: 6544
  • View blog
  • Posts: 26,526
  • Joined: 12-December 12

Re: MVP very simple example

Posted 17 March 2014 - 05:58 AM

Thank you very much @Skydiver.

You anticipated correctly that I would want to compare the start and due dates ;)

Starting with the simplest validation though, I will check that there is some text entered for the task-name. It just seems so obvious, and straight-forward, that the form will do this on clicking a button: if it's empty produce a message and focus on the textbox. Trying to push this via the presenter just seems.. unnecessary.

Let's say that I want the task-name to be unique. Sensibly the model - in reality, a database of some kind - would have a unique constraint. (This is not directly relevant to the pattern though.) How might this be handled? I'm coming to the conclusion that the right answer is a distribution of labour. Patterns are great to try and follow.. but they are not the one true way.
Was This Post Helpful? 1
  • +
  • -

#5 Skydiver  Icon User is online

  • Code herder
  • member icon

Reputation: 6164
  • View blog
  • Posts: 21,252
  • Joined: 05-May 12

Re: MVP very simple example

Posted 17 March 2014 - 06:14 AM

View Postandrewsw, on 17 March 2014 - 08:58 AM, said:

Patterns are great to try and follow.. but they are not the one true way.


Bingo!

I remember agonizing over my first WPF project and that I was being forced to break the MVVM pattern because I really, really, really needed to add in a Controller otherwise the VM would just be insanely tightly coupled. It was reassuring to see that other people working in Silverlight had realized as well that MVVM doesn't cut it all the time so the more appropriate pattern was MVVM+C. In short, the patterns will only get you so far, but at least you didn't have reinvent the wheel.
Was This Post Helpful? 0
  • +
  • -

#6 andrewsw  Icon User is offline

  • blow up my boots
  • member icon

Reputation: 6544
  • View blog
  • Posts: 26,526
  • Joined: 12-December 12

Re: MVP very simple example

Posted 17 March 2014 - 06:25 AM

Back to my original question though ;) is there anything dramatic that stands out as being wrong about my example? That is, is it an acceptable example of the MVP pattern?
Was This Post Helpful? 0
  • +
  • -

#7 Skydiver  Icon User is online

  • Code herder
  • member icon

Reputation: 6164
  • View blog
  • Posts: 21,252
  • Joined: 05-May 12

Re: MVP very simple example

Posted 17 March 2014 - 10:54 AM

Looks great to me!
Was This Post Helpful? 1
  • +
  • -

#8 Curtis Rutland  Icon User is offline

  • (╯□)╯︵ (~ .o.)~
  • member icon


Reputation: 5103
  • View blog
  • Posts: 9,283
  • Joined: 08-June 10

Re: MVP very simple example

Posted 17 March 2014 - 11:34 AM

Just adding my agreement about design patterns. I've known developers to spend hours, even days, trying to make their ideas fit arbitrary patterns that they decided to use early on. And it bugs me, because patterns exist to make things easier on devs, not harder. If your business model doesn't fit your chosen pattern, adapt or choose a new pattern. Use them for what they are, don't trap yourself in a cage with them.
Was This Post Helpful? 1
  • +
  • -

#9 andrewsw  Icon User is offline

  • blow up my boots
  • member icon

Reputation: 6544
  • View blog
  • Posts: 26,526
  • Joined: 12-December 12

Re: MVP very simple example

Posted 17 March 2014 - 12:00 PM

View PostSkydiver, on 17 March 2014 - 05:54 PM, said:

Looks great to me!

That's good enough for me :)

Although, now that you've raised the spectre of StartDate vs. DueDate it's on my mind..

I'll concentrate on a more direct question though: MessageBoxes. The view can raise an event, the presenter can then produce a MessageBox, but still in the context of the view, MessageBox.Show(view, "..."). The response can be fed back to the view by a property or an event of the presenter.

I suppose this is what we have been discussing recently. This event-based approach is more in-line with the principles behind MVP. But, in the real world.., it is the view that should produce the MessageBox and process the response to it IMO. (Now I'm falling into the trap that has been discussed: other-thinking it and attempting to follow the pattern too tightly ;))

I suggest that simple validation and the use of MessageBoxes belong in the view. Any further thoughts are welcomed :)
Was This Post Helpful? 0
  • +
  • -

Page 1 of 1