techygrrrl / timerrr

A CLI-based timer app with a TUI written in Go
https://blog.techygrrrl.stream/cross-platform-tui-cli-go-lang
MIT License
7 stars 2 forks source link

Enhance tea.Msg handling using a polymorphic approach #12

Open AdjectiveAllison opened 1 year ago

AdjectiveAllison commented 1 year ago

I've noticed that the Update function in the tableModel struct uses a switch statement to check for message types and handle them accordingly. Since tea.Msg is an interface, I was pondering a more Go-native, polymorphic approach that provides better separation of responsibilities and makes the code more organized and maintainable.

I have no idea the upstream bubbletea reasons for creating an interface nor do I know the codebase very well yet, it's just something I noticed. In the spirit of over engineering - Here is a recommendation that AI helped me create with my thoughts - just putting it here for reference.

The idea is to create a MsgHandler interface that all message handlers should implement:

type MsgHandler interface {
    Handle(msg tea.Msg) (tea.Model, tea.Cmd)
}

Then, separate structs can be created for each message type to handle, and the Handle method can be implemented for each of them:

type WindowSizeHandler struct {
    model *tableModel
}

func (h WindowSizeHandler) Handle(msg tea.Msg) (tea.Model, tea.Cmd) {
    // Handle WindowSizeMsg here
}

type KeyHandler struct {
    model *tableModel
}

func (h KeyHandler) Handle(msg tea.Msg) (tea.Model, tea.Cmd) {
    // Handle KeyMsg here
}

// Add more handlers for other message types if needed

Finally, the Update function can use a map to map message types to their respective handlers:

func (m tableModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
    handlers := map[reflect.Type]MsgHandler{
        reflect.TypeOf(tea.WindowSizeMsg{}): WindowSizeHandler{model: &m},
        reflect.TypeOf(tea.KeyMsg{}):         KeyHandler{model: &m},
        // Add more handlers for other message types if needed
    }

    handler, ok := handlers[reflect.TypeOf(msg)]
    if ok {
        return handler.Handle(msg)
    }

    // If no handler is found for the given message type, update the table as before
    m.table, cmd := m.table.Update(msg)
    return m, cmd
}

This approach would make it easier to add new message handlers without modifying the Update function, just by adding new structs that implement the MsgHandler interface and updating the handlers map.

techygrrrl commented 1 year ago

Thanks for this idea!

In the spirit of over engineering

😄 This made me chuckle.

The current code follows the bubbletea examples. Here's a specific example.

I was wondering how this kind of approach might impact performance. In Java/Kotlin/JVM languages, reflection is an expensive operation. I'm wondering if that is the case in Go as well.

I am a bit hesitant to take this approach as it adds an additional abstraction layer, which adds additional conceptual overhead. Although I do like the idea of having something named keyHandler and windowSizeHandler then I can know where to go when I have bugs to fix (like this one).

I'll think about this for a bit before making a decision. I like the idea of having named functions in terms of code organization, but I'm not sure what could go wrong with a reflection-based approach. It also adds conceptual overhead and more interfaces and patterns to think about.

Another thing to consider is if this type of extra complexity is warranted for an app of this size. I don't imagine this app growing much beyond what it does as a simpler timer app so I don't know if it's necessary to plan for such a future.

I'll give it some thought, I appreciate the feedback.