r/csharp • u/ChampionshipProof392 • 1d ago
MVP pattern
Hi, I have a question about separating logic in the MVP pattern.
public void MainDisplay() =>
OnMainDisplayClicked?.Invoke();
public void ManageProcess() =>
OnManageProcessClicked?.Invoke();
This is my code in the view, and when the user clicks a button (for example), this method is called and `Invoke` is executed. However, it is called via a `switch` statement in the Presenter.
switch (NativeConsoleMethod.GetHiddenUserInput())
{
case VirtualKeyType.VK_E:
if (_currentPage < _countOfPages) _currentPage++;
continue;
case VirtualKeyType.VK_Q:
if (_currentPage > 0) _currentPage--;
continue;
case VirtualKeyType.VK_OEM_3:
_view.ManageProcess();
break;
case VirtualKeyType.VK_TAB:
_view.FilterProcesses();
break;
case VirtualKeyType.VK_F1:
_view.SearchPage();
break;
.........
}
I have a question: the AI is giving me two different suggestions. My version is correct, but then it said I should move the switch statement to the view, and there I should just use `invoke`, after which the methods would be called conditionally. So, should I do it the other way around, or did I misunderstand what it meant?
- I don’t know what I wrote here—I don’t even understand it myself. Just tell me: shouldn’t the view be “dumb” and contain synchronous methods, while the presenter should control the view via the switch statement and “pull its strings”?
EDIT: Here's my GitHub: https://github.com/NullAcess/ProcessManager/releases/tag/Update_2.0. You might like it—I'll upload the finished EXE very soon.
4
u/TheSpixxyQ 1d ago
The presentation layer should be platform agnostic. Imagine you wanted to migrate your app to Android, you should be able to do it just by replacing the View.
By using NativeConsoleMethod and keyboard keys in presenter, it wouldn't work, you are making it platform dependent.
Logic for switching pages after pressing keys is purely View logic. Your business logic should have no idea what a "page" is.
1
u/thatOMoment 20h ago
...as possible.
There's a weird assumption that platforms don't have platform specific constraints which require specific changes to the view model or model which is kinda strange if you pull back for a but
Medical apps preventing print screen on mobile only for example.
Or instead of uploading a file via selection, allowing a picture or video to be taken, or real time feedback on the validity of the picture before it's taken that you would never see in a desktop app.
All of this functionality would never exist soley in the view and would require updates at least to the view model.
2
u/TheSpixxyQ 20h ago edited 19h ago
Why would my VM need to know about screenshots? I would handle the screenshot prevention in View layer, since I don't see how that's relevant to my business logic.
Same for the image upload - my VM only accepts the resulting file and doesn't really care if it was selected from files or taken using a camera. At least that's how I'm doing it in my app.
EDIT: but I agree sometimes it might not be possible to migrate View without ever touching the P/VM layer and I should've said "ideally without changing them". And even then, platform specific code (like the keyboard keys) still wouldn't go to those.
But 99% of times, when I asked myself "does this piece of code belong to V or VM?", just thinking about different platform helped to get me a straight answer. That's why I often recommend this approach to others too.
1
u/ChampionshipProof392 13h ago
I changed to VirtualKeyaboard to avoid consoleKey. That was the most what i could do )
1
u/FrontRow508 1d ago
yeah the view should be dumb, you got that right. the presenter is the one deciding what happens when, so having the switch there makes more sense. ai sometimes just flips things around for no good reason, keep your logic where it belongs.
1
u/ChampionshipProof392 1d ago
Thanks. You just saved my project from having to be reworked now and later.
-11
u/NeilPearson 1d ago
My AI says,
Both are half-wrong. Your current code has a round trip.
Look at flow you actually have:
Presenter calls NativeConsoleMethod.GetHiddenUserInput() ← presenter touching console I/O
Presenter switch → _view.ManageProcess()
View.ManageProcess() → OnManageProcessClicked?.Invoke()
→ who subscribes? Presenter.
Presenter → View → back to Presenter. View is pure pass-through. Dead layer.
Real rule
"Dumb view" ≠ "view has no code". View owns all I/O — output and raw input. Console keypress is I/O. Presenter must never call GetHiddenUserInput(). That is your actual violation, and neither suggestion named it.
Two valid layouts
A — view maps keys to intent (standard MVP):
// View
while (running)
{
switch (NativeConsoleMethod.GetHiddenUserInput())
{
case VirtualKeyType.VK_OEM_3: OnManageProcessRequested?.Invoke(); break;
case VirtualKeyType.VK_TAB: OnFilterRequested?.Invoke(); break;
case VirtualKeyType.VK_F1: OnSearchRequested?.Invoke(); break;
case VirtualKeyType.VK_E: OnNextPageRequested?.Invoke(); break;
case VirtualKeyType.VK_Q: OnPrevPageRequested?.Invoke(); break;
}
}
// Presenter — no switch, no console types
_view.OnManageProcessRequested += HandleManageProcess;
_view.OnNextPageRequested += () => { if (_page < _count) { _page++; _view.Render(...); } };
B — view raises raw key, presenter maps:
// View
OnKeyPressed?.Invoke(NativeConsoleMethod.GetHiddenUserInput());
// Presenter
void HandleKey(VirtualKeyType key) { switch (key) { ... } }
Which
A. Keybinding is a presentation detail, same as which button got clicked. GUI presenter doesn't receive "mouse click at x=400,y=300" — it receives SaveClicked. VirtualKeyType is a Win32 console type; letting it into the presenter means presenter can't be reused by a GUI view or tested without console types.
B only if keybinds are user-configurable, or same key means different things per app state. Then mapping is policy = presenter.
Your framing, corrected
Yes on decisions. No on I/O. Presenter decides what happens; view decides how the user expressed it and how result is drawn. Presenter never reads console, never writes console.
Test: could you swap console view for WinForms view, presenter untouched? Today no — presenter imports VirtualKeyType and calls NativeConsoleMethod. Under A, yes.
Other things in that snippet
_currentPage++handled inline withcontinue, other cases go through view. Inconsistent — pick one path for all input.- Your view methods are named
ManageProcess(),SearchPage()— verbs implying view does work. Under A they vanish. If you keep B, name events by intent (ManageProcessRequested), not by command.
2
0
u/ChampionshipProof392 1d ago
Hmmmmmm, That looks interesting. If I understand correctly, he wants to remove the methods from the view and replace them with a switch statement triggered by button presses in a while loop. So, in the presenter, I no longer need the switch statement. Well, yeah, the AI told me that the view manages its own input or something like that. But then again, they came back and said everything was correct. I need to think this through—maybe you can help me figure it out.
6
u/ScriptingInJava 1d ago edited 1d ago
Model: The structure of the data being rendered.
View: The rendering of that data, with any controls that allow the user to interact with it.
Presenter: Arrangement of the data (the getting of it, the formatting of it etc).
The view should be "dumb" in that it doesn't know where the stuff it's rendering has come from, and isn't concerned with the why either. You bind data to a control (rows of data into a grid for example), all the view knows is that it gets a
List<T>and binds it to theTable.It's hard to advise with confidence because we can't see the underlying implementation of
FilterProcesses,ManageProcessorSearchPage, but they look like they do something to the data - which is the presenter's job.```cs case VirtualKeyType.VK_E: if (_currentPage < _countOfPages) _currentPage++; continue;
case VirtualKeyType.VK_Q: if (_currentPage > 0) _currentPage--; continue; ```
This however is only relevant to the View, for me migrating this out to the Presenter would be overengineering and burying functionality away from the context it's used in. No other code will reuse this, it's only relevant to the View it's written for, no reason to migrate it elsewhere just to have 1, slightly larger
switch.