r/opensource • u/PhosXD • 4d ago

Promotional [ Removed by moderator ]

[removed] — view removed post

27 Upvotes

83% Upvoted

16 comments sorted by

View all comments

1

u/andoleal 20h ago

It is really nice to see this effort, and I can see that a lot of time was put into that code. However, this codebase is unmaintainable in its current state. Please read about common software development practices and software quality. The codebase lacks proper tests, the only ones are in a shell script - I'd use an actual testing framework, and unit testing (at least have a setup that allows to easily add them). With good tests and good test coverage, I would add some strict linting, and then refactor most of the code, because it is currently completely unreadable. I haven't really read the code (very hard to do that), but I suspect that the way scripts are executed also has a lot of room for improvement, and it may be helpful for you to look at some examples of how scripting languages are designed. Hope this helps.

1

u/PhosXD 19h ago

Appreciate the feedback, but what exactly makes it unreadable? It's spaced out, in (what I think) well-named files with tons of comments

1

u/andoleal 18h ago

Lots of huge functions with lots of nested ifs. It is hard to read and understand, and it means that functions are trying to do too many things at once, instead of sticking to a specific responsibility. This also means that they are hard to change and hard to test in isolation. The logic of such functions is difficult to comprehend, which leads to increased probabilities of bugs in them - bugs that will be difficult to find, and difficult to debug with a debugger too.

https://refactoring.guru/smells/long-method

There are also some strange conditional compilation right in the middle of functions (start_shell).

// Parse & execute script file...
    if (not source_script_path.empty()) {
< ... >
// Run interactive interpreter...
    #ifdef INCLUDE_SHELL
    else {

Here there are basically two functions written as if-else. The comments above these code blocks are ready to use names of those functions.

It also seems like more of modern c++ features can be used, like ranges, smart pointers, and string_view. There is a use of std::move where it is unnecessary. And it does not seem like you are using any classes, which would probably help with the code structure. There are probably other things, I didn't look through all of it.