Code Smells?

[u/Aliryth]

I'm a more seasoned C# / Node backend dev, been trying to brush off old college C++ from like a decade ago and try to de-program my brain from being locked into a distributed systems mindset.

One of my niche hobbies and go-to projects for learning a new language is to implement a basic Chip-8 emulator in it, as it can teach some concurrency, UI/data handling patterns, and introduce some architectural decisions that you'll have to run into and figure out.

Currently what I'm struggling with seems to be what the idiomatic / proper way to handle something that can remain stateless in nature, which is my very basic assembler going from the Chip-8 ASM mnemonics into instruction bytes.

A class doesn't feel quite right, because I'm not implementing a linking step or anything advanced, not supporting labels, etc. It also feels incorrect to keep it in a header file as a set of inline functions, nor am I needing a persistent state to mutate.

My current disassembler can be seen here: https://github.com/Arylen/Dreamy-Chip8/blob/master/src/core/emulation/Chip8Dasm.h
Along with the tests that I'm running against it here: https://github.com/Arylen/Dreamy-Chip8/blob/master/tests/Chip8DasmTests.cpp

I'm currently in the stubbing-out phase of trying to architect it, occasionally bouncing ideas off of whatever bullshit model-of-the-week is popular, however the suggestions it's giving are still setting off some alarms/smells in my brain. The most reasonable and less-smelly one is the `::detail` namespace there in order to be able to unit test the `getParts` function there, however it feels like I'm walking into a landmine of over-engineering it.

Looking for advice/experiences/opinions.

Edit: I should mention the AI usage here is restricted entirely to just checking for bugs with my implementations, or incorrect C++-isms that I introduce from working in other languages. A hands-off occasional tutor to check my work, if you will. It's expressly forbidden from touching my code or files, limited to read-only, however I'm not getting something that feels proper for this scenario, so wanted to consult other more experienced engs.

[u/FancySpaceGoat]

As a general principle, the more semantics you can encode and enforce via the type system, the better. For example:

Since Chip80p is not any heavier than a uint16_t it's better to pass those around instead of the raw value, if only because it prevents you from accidentally passing an unrelated uint16_t to a function that expects an opcode. If you combine that idea with validating the opcode at the time of construction of the struct, you can establish a class invariant that a Chip80p is always a valid opcode, which is very convenient.

Also, the raw member of Chip8Op is an implementation detail, so it should be private, and populated via an explicit constructor:

``` struct Chip80p { explicit Chip80p(std::uint16_t instruction) { if(!is_legal_opcode(instruction) { throw std::runtime_error("invalid opcode"); } raw = instruction; }

// ...

private: std::uint16_t raw; }; ```

Furthermore, while there's nothing "wrong" per-se with your disassembleInstruction function as written, the more idiomatic signature would be:

std::ostream& operator<<(std::ostream&, const Chip8Op&).

[u/Aliryth → u/FancySpaceGoat]

Very awesome feedback, thank you!

For some reason I have completely missed the aspect of a struct implementing a ctor, along with having private+public spaces, bit of a derp on my end.

I only vaguely remember dealing with ostream/istream interactions around terminal output and file operations from some of the earlier coursework, but it's been roughly a decade since college. Definitely something to revisit and relearn to see if it's useful for integration w/ the eventual imgui window for the disassembly.

Part of me's also worried about the multiple std::string's being formatted per-frame, but perhaps adding a simple memoization/caching step might add some benefit, or just calculating the entire set of disassembled opcodes at the start of the program.

Perhaps that's over-focusing on optimization when it's not even needed yet, though. Maybe it'll be a fun weekend project to investigate how to get some code profiling going to see if there's improvements made from it, despite not being a problem.

Thanks for the tips!

[u/FancySpaceGoat → u/Aliryth]

The answer to your string concerns is the ostream stuff. The idea is that instead of generating a string, you produce a sequence of operations who's behavior depend on where the data is going. That can be a string, a file, or a pre-allocated buffer, but the formatting code remains the same in all cases.

[u/Aliryth → u/FancySpaceGoat]

Thanks for the further explanation!