kaorahi / lizgoban

Leela Zero & KataGo visualizer
GNU General Public License v3.0
169 stars 28 forks source link

Improvement: Consider refactoring the draw_pv function in renderer.js #115

Closed qcgm1978 closed 7 months ago

qcgm1978 commented 7 months ago

The draw_pv function is overly complex and difficult to debug compared to the draw_main function. The current code uses nested ternary operators, which makes it difficult to read and maintain.

Refactor the draw_pv function using switch statements and object mapping to improve readability and maintainability. Refer to the code example for refactoring:

const draw_pv = with_opts((...args) => {
    const strategy = getStrategy();
    const { draw } = strategies[strategy];
    draw(...args)
}, ignore_mouse)

const strategies = {
    subboard: {
        draw: D.draw_goban_with_subboard_stones_suggest
    },
    originalPv: {
        draw: D.draw_goban_with_original_pv,
        check: showing_pv_trail_p
    },
    rawGoban: {
        draw: D.draw_raw_goban,
        check: () => truep(showing_until())
    },
    futureMoves: {
        draw: D.draw_goban_with_future_moves,
        check: showing_branch_p
    },
    another: {
        draw: draw_another,
        check: already_showing_pv_p
    },
    principalVariation: {
        draw: D.draw_goban_with_principal_variation
    }
};

function getStrategy() {
    switch (true) {
        case R.subboard_stones_suggest:
            return 'subboard';

        case showing_pv_trail_p():
            return 'originalPv';

        case truep(showing_until()):
            return 'rawGoban';

        case showing_branch_p():
            return 'futureMoves';

        case already_showing_pv_p():
            return 'another';

        default:
            return 'principalVariation';
    }
}
kaorahi commented 7 months ago

The issue may be relevant for other projects, but I'm not planning to adopt it for this project.

The sequence a ? A : b ? B : c ? C : ... is simply an else-if chain and is quite straightforward. It should be distinguished from evil nests like a ? b ? X : Y : Z.

This is my hobby project, and I enjoy my favorite style here, even though it doesn't align with typical coding standards of JavaScript.

As background, I mainly use huge fonts to avoid eye strain on my small notebook PC, which severely limits the number of visible lines. So, I generally prefer long lines with dense logic over the popular style of many simple lines.

qcgm1978 commented 7 months ago

This is indeed a trade-off between pros and cons, between ensuring code readability or conciseness. Whichever approach taken may not be able to reduce the inherent complexity of things. Perhaps in this refactoring process, it is more important to understand the internal logic of things.