Can anyone have a quick look at my architecture + use of `std::mem::replace`?

code review
1

So I've been trying to learn rust by creating a TUI application to keep track of all my coding projects using ratatui. https://github.com/fgassmann/projektor (No vibecoding btw.)
However there are some things I'm not sure if I'm doing the right way, so I would appreciate some feedback before I continue.

One thing is I'm currently using this enum to keep track of the ui state (e.g. you start in Projectview, press n and then it changes to CreatingProject).
I was fairly happy with this until I introduced the Error state which basically just wraps the state the error occured in and displays it as a kind of
popup until the user acknowledges it.
However since then i've been having to use std::mem::replace a bunch of times to convert back from Error to the previous state.
(also I'm not sure how well this will work once I build the other tabs?)


#[derive(Debug)]
pub enum AppMode {
    ProjectView,
    EditingProject(editor::ProjectEditor),
    CreatingProject(editor::ProjectEditor),
    Error(String, Box<AppMode>),
}

let mode = std::mem::replace(&mut self.mode, AppMode::ProjectView);
match mode {
    AppMode::EditingProject(p) => {
        // do something
    }
}

I also feel like I have a wierd mix between the Elm architecture and Component architecture?
But that might just be because I'm using components like ratatui-textarea?
So any feedback on architecture/ structure of the project is also appreciated.

Also I know I still have some bad error handling,
unused import and missing features (currently the config doesn't get red from a file but is hardcoded for testing).
So please don't mind those.

2

That's your first hint. Doing things the better way should help, or at the very least not interfere, with all the changes/features you have in mind. That's not quite your experience here, is it?

(Most of) Errors are, by the their nature, transitory. Making them an inherent part of your application state feels like a clunky, awkward, and a somewhat confusing choice. Even assuming you do wish to preserve your last as a part of your state, in your code you already have:

#[derive(Debug)]
pub struct App {
    pub running: bool,
    pub events: EventHandler,

    pub mode: AppMode,
    pub config: config::Config,
}

#[derive(Debug)]
pub enum AppMode {
    ProjectView,
    EditingProject(editor::ProjectEditor),
    CreatingProject(editor::ProjectEditor),
    Error(String, Box<AppMode>),
}

Why not move the Error up into the App, as a Box<dyn Error>? Moving your whole app state / mode onto the heap (into the Box<AppMode>) on any unexpected event, only to deallocate it as soon as you're done handling it, doesn't make much sense. Later on you discard it entirely, right on the next key press. Why not return a Result from here, instead of updating the whole state?

3

another hint is, does it make sense to stack another error on top an error state (and so on)?

// this is a perfectly representable state with the current design
// but does this kind of recursive type make sense for the application?
AppMode::Error(
    "error foo".to_owned(),
    Box::new(AppMode::Error(
        "error bar".to_owned(),
        Box::new(AppMode::Error(
            "error baz".to_owned(),
            todo!("and so on and so on...)
        )
    )
)

EDIT:

I should clarify, what I meant to say is, the error state (or more generally, a modal pop-up state), should be handled at different abstraction level than (regular) state transitions of the application.

4

That makes sense to me I guess. The reason I did it this way is because
it fits nicely into the existing match expressions in handle_key_event
etc.

Reading your comment I'm also not sure I entirely understand the underlying memory layout,
which might be why I was confused?

In my mind App contained a pointer to AppMode (looking at it now this doesn't make sense as it's owned?)
But in that case I was thinking popping of the error should be as simple as
changing the pointer to in App to point to the Appmode contained in the error state?
Kinda like

App -> Error -> Projectview

to

App -> Projectview

If that makes sense? I guess this is also why it felt so wrong to use the std::mem::replace.

5

I was kinda just trusting myself to not do that xD. That being said in my mind it could make sense to have nested popups like this in some scenarios.

e.g. the current Editor has a file picker which is part of it's internal state but I was at some point thinking about representing it like this:

AppMode::Filepicker(
    Box::new(AppMode::EditingProject(...))
)

In the end I'm glad I didn't as this would also mean this would be a valid state, which doesn't make sense:

AppMode::Filepicker(
    Box::new(AppMode::Error(...))
)
6

Honestly I often find this kinda nice in state machines. If you're running transition logic, you replace it to Error, then naturally if anything goes wrong -- panics, ? short circuits, etc -- it's automatically in Error state without you needing to manually do that. Then only at the end once the transition is all built up properly does the state get updated to the new one.

And replaceing the state variant means you get by-move access to the thing in the variant too, which is often handy.

7

one of the benefit of strong typing is to make invalid state unrepresentable. it does not only remove the chance for this kind of human errors, it also helps the compiler and code analysis tools to better understand the code.

IMO that's orthogonal to the normal state transitions.

you should implement the stacking mechanism on top of (as opposed to as part of) state transitions/scene switching, which can be more powerful, extensible, and flexible.

for instance, you can have both modal and non-modal dialogs, notification pop-ups, and maybe layers or overlays, etc, which are not limited to a single layer. basically, it becomes a window manager in the terminal, or terminal multiplexer, however you like to call it.

8

Not only that, but it reduces the chance that you'll design yourself into a corner later, where you end up having to refactor everything. I've also personally found it often makes my code more ergonomic and "flow-ey".

All this to say, you should almost always model your program's state and logic as accurately as possible in the type system.