subjective issue, idk what title to give

[necauqua]

Hey, I just started reading the too many brainfuck compilers article and it is very interesting, however, I immediately noticed a code smell I would instantly refactor - it is subjective and I may be wrong by making a whole issue about it, but here it is:

So you have a code that looks like:

pub enum Inst {
    IncPtr(usize),
    DecPtr(usize),
    IncByte(usize),
    DecByte(usize),
    WriteByte(usize),
    ReadByte(usize),
    LoopStart(usize, usize),
    LoopEnd(usize, usize),
}

Then you spend extra 3 lines of the article documenting explaining what are these usizes for, but this can be easily made into a self-documented and much more readable code (and at places where you use it too). For example (with compact formatting here in the issue):

pub enum Inst {
    IncPtr { by: usize },
    DecPtr { by: usize },
    IncByte { by: usize },
    DecByte { by: usize },
    WriteByte { times: usize },
    ReadByte { times: usize },
    LoopStart { times: usize, end_idx: usize },
    LoopEnd { times: usize, start_idx: usize },
}

The run-length encoding is way too repetitive and by definition is in each enum variant, so it can be abstracted away in a struct:

pub struct Inst {
    pub kind: InstKind,
    pub times: usize,
}

pub enum InstKind {
    IncPtr,
    DecPtr,
    IncByte,
    DecByte,
    WriteByte,
    ReadByte,
    LoopStart { end_idx: usize },
    LoopEnd { start_idx: usize },
}

This improves readability and usability by a lot with no downsides (not like there is or will be 7 layers of structs/enums), and I honestly don't really know why tuple structs even exist at all. As I've said, this is subjective, feel free to ignore this, it just immediately tingled me when I was reading your article (didn't even read past this yet, but it seems very-very interesting).

[nilsmartel]

the additional usize was confusion to me as well. Also this way one can introduce InstructionKind first. As soon as one gets to Inst all is obvious

[pretzelhammer]

I appreciate the feedback. I want the article and the source code repo to stay aligned so I'd have to make this change there first before updating the article, but I'll find some time this week to do it.

[pretzelhammer]

refactored code in companion code repo here: https://github.com/pretzelhammer/brainfuck_compilers/pull/2

updated article in this commit: https://github.com/pretzelhammer/rust-blog/commit/26e05f938e40a958565b0c0859bf0eec5b368076

i consider this issue fixed so I'm gonna close it, thanks again for the feedback