kyu08 / fzf-make

A command line tool that executes make target using fuzzy finder with preview window.
MIT License
77 stars 10 forks source link

refactor(app): Making impossible states impossible #265

Closed kyu08 closed 3 months ago

kyu08 commented 4 months ago

Motivation

What's changed

Made impossible states impossible.

At first, I show main change point.

Before

pub struct Model<'a> {
    pub app_state: AppState,
    pub current_pane: CurrentPane,
    pub makefile: Makefile,
    pub search_text_area: TextArea_<'a>,
    pub targets_list_state: ListState,
    pub histories: Option<Histories>, // When home dir could not be found, this is None.
    pub histories_list_state: ListState,
}

pub enum AppState {
    SelectingTarget,
    ExecuteTarget(Option<String>),
    ShouldQuit,
}

After

pub struct Model<'a> {
    pub app_state: AppState<'a>,
}

pub enum AppState<'a> {
    SelectTarget(SelectTargetState<'a>),
    ExecuteTarget(Option<String>),
    ShouldQuit,
}

pub struct SelectTargetState<'a> {
    pub current_pane: CurrentPane,
    pub makefile: Makefile,
    pub search_text_area: TextArea_<'a>,
    pub targets_list_state: ListState,
    pub histories: Option<Histories>,
    pub histories_list_state: ListState,
}

By this change, we can no longer express impossible states like below.

Model {
    app_state: AppState::ShouldQuit,
    current_pane: CurrentPane::Main,
    makefile: Makefile::new_for_test(),
    targets_list_state: ListState::with_selected(ListState::default(), Some(0)),
    search_text_area: TextArea_(TextArea::default()),
    histories: init_histories(vec!["history0".to_string(), "history1".to_string()]),
    histories_list_state: ListState::with_selected(ListState::default(), Some(0)),
}

When app_state is AppState::ShouldQuit, the app need not have any data.

The above code become like this:

Model {
    app_state: AppState::ShouldQuit,
}

Good effect of this change

This makes some good effect:

1. Improve readability

It made readability improved because the data structure describes the application state more precisely.

This may good especially for the person who are new to this project too.

2. Reduce the chance of potential bugs because invalid data accesses can't happen.

As the title says.

3. Remove unnecessary data for testing.

This is not huge advantage for now, but the effort for testing will not increase even though the data structure grows.

// Before
Case {
    title: "Quit",
    model: init_model(),
    message: Some(Message::Quit),
    expect_model: Model {
        app_state: AppState::ShouldQuit,
        ..init_model() // this is unnecessary data
    },
},

// After
Case {
    title: "Quit",
    model: Model {
        app_state: AppState::SelectTarget(SelectTargetState {
            ..SelectTargetState::new_for_test()
        }),
    },
    message: Some(Message::Quit),
    expect_model: Model {
        app_state: AppState::ShouldQuit,
    },
},

Minor changes