I have an application that uses x11 lib for drawing some user interface. Now I am trying to refactor it with MVP (model-view-presenter) pattern, but stacks on some details about view-presenter part. Ex: I have an left/right arrow key pressed event, by this event I need to cycle by elements on the view (like tab in browser). Now this event just flows to Presenter from View, then Presenter selects next element to activate and pushes it to ui back. But(!) to select next element I need to have an iterator on BEGIN and END (c++, in other langs could just be a reference/index at the end of the list of viewed objects) it causes to have a special method for getting it in the IView interface, that doesnt look clear. Wouldn't be better to perform a key press in the view, selects next element there, and then notify presenter about current active element?
Ive read many articles about that, and checked repos with this pattern. But still cant understand:(
pseudo-code:
class IView {
public:
virtual Element* firstElement();
virtual Element* lastElement();
};
class Presenter {
public:
void onRightKeyPressed() {
if(!currentElement) {
currentElement = view->firstElement();
} else {
currentElement->resetActive();
++currentElement;
if(currentElement == view->lastElement()) {
currentElement = view->firstElement():
}
}
currentElement->setActive();
}
private:
IView* view;
Element* currentElement;
};
vs something like
class IView {
public:
// Notify the presenter about the currently active element
virtual void notifyElementActivated(Element* element) = 0;
// Handle arrow key press event and select the next element
virtual void handleRightKeyPressed() = 0;
};
class Presenter {
public:
void onRightKeyPressed() {
view->handleRightKeyPressed();
}
// Receive the notification from the view about the currently active element
void notifyElementActivated(Element* element) {
// React to the active element change
}
private:
IView* view;
};
class View : public IView {
public:
void handleRightKeyPressed() override {
if (!currentElement) {
currentElement = firstElement();
} else {
currentElement->resetActive();
++currentElement;
if (currentElement == lastElement()) {
currentElement = firstElement();
}
}
currentElement->setActive();
// Notify the presenter about the currently active element
notifyElementActivated(currentElement);
}
void notifyElementActivated(Element* element) override {
presenter->notifyElementActivated(element);
}
private:
Presenter* presenter;
Element* currentElement;
};
I expect a more clear solution for this case!
There are different "right" solutions to your problem. Which one to pick depends on what the Element objects are, how much Presenter logic is controlled by the active element, if any, and how that logic looks like.
In case "Element" is nothing but a dumb tab page, and the presenter contains almost no logic depending on the active element, you can make the View completely responsible for holding the current element and the forward/backward logic. For this, your second solution looks like a good start. However, the virtual method
IView::notifyElementActivatedseems to be obsolete, it can probably stay a private non-virtual method of the view, no need to make it part of the public view interface).Also, your code does not give any clue if
Presenter::notifyElementActivatedis really needed, or if a methodIView::getCurrentElementwould be sufficient.In your first solution, the management of the current element is part of the Presenter, which is fine as long as the View does not have its own management of an "active page/element", which could drift away from the presenter. You wrote about your concerns:
Well, why not design your IView interface like this?
I would also suggest to rename
onRightKeyPressed()in the Presenter togotoNextElement(). Your view should have an event handleronRightKeyPressed()which callspresenter->gotoNextElement()- which keys exactly cause the forward or backward navigation is something the Presenter should not be concerned with.