Refactoring a Command Line Shell | Code Roast Part 2

[@ArjanCodes]

👷 Join the FREE Code Diagnosis Workshop to help you review code more effectively using my 3-Factor Diagnosis Framework: www.arjancodes.com/diagnosis

[@rrwoodyt]

Whoo hoo-- part 2!

[@daephx]

Pedantic change, less of a refactor and more of a UX issue: Remove the exit animation. There is no cleanup steps being executed; it's just there to be flashy and holds the user hostage for 3 seconds every time they exit the shell.

[@M3MYS3LF1979]

Was thinking the same thing. ctrl-c / ctrl-d ftw

[@cetilly]

This was a great refactoring session; Loved It!

[@Soltaiyou]

Love your work!

[@nikohegeheiskanen]

Instead of having two separate dicts for encoding and decoding funcs why not have a single dict with algo as key and tuple with enc and dec funcs?

[@minamagdy4126]

That would make the API implementation for encode-decode a bit more complex, although I agree that the change is worth the effort, at least conceptually.

[@SkielCast]

One change that I would do is migrate from scripts to a module. Currently, that shell app only works if the interpreter is executed from the src folder. To be able to execute the app from anywhere in the system, maybe migrating to modules and using relative imports is a good idea

[@ArjanCodes]

Great suggestion!

[@xnick_uy]

The clean version of the code is "oddly satisfying". Some features to consider adding to this system is a command history, some input/output capability (to avoid the need to copy and paste the various results and to keep track of the algorithms applied) and also some way to perform batch processing. Isn't there a one-to-one correspondence between the encoding and decoding algorithms? I suppose there is... If so, I think it would be a good idea to have this fact reflected in the data structures, by combining the ENCODING_ALGORITHMS and DECODING_ALGORITHMS dictionaries into a single one.

[@dongiovanni1993]

I wouldn't be so sure about the algorithms' one-to-one correspondence. If we really consider future extensibility, the code could easily be broken with such an optimistic assumption, which in fact is just speculation.

[@PascalLemaître]

Why not using the cmd module in the standard library ?

[@marcotroster8247]

I like it when code is kept to the point such that people can fork and do their thing with it. Nice job! 😄 I'd like to see you code a performant game engine in Mojo. The tradeoff between "clean" design and fast execution could be very interesting 😁😉

[@virolana]

Thank you Arjan for all your videos, they have helped me becoming a better programmer! What is the rationale for an empty return vs no return at all?

[@M3MYS3LF1979]

This is a good question. Functions return none by default. Zen of python maybe? (explicit is better than implicit) en.wikipedia.org/wiki/Zen_of_Python

[@talwald1680]

Great video! Regarding the algorithmic function duplications... If you want to share code between the different commands - create a execute_algorithm_command function, that gets the algorithms dictionary, number of arguments, and the docs and then just partial apply the hash, encode and decode commands. Alternatively you can do the same thing with a AlgorithmsExecutor class. This is also relevant for the api for the algorithms duplicate code. One thing is that I don't like the printing mixed in with the application logic - what do you think about either making the data required to decide what to print returned from an inner layer function, or inject a presenter class and pass it events?

[@jeremy9959]

One simplification to consider is to drop the keywords "hash", "encode" and "decode" since those are implicit in the algorithms. Then the command would be (for example) md5 . Then you wouldn't need separate logic to handle the three types of commands, they all have the same shape .

[@grzegorzryznar5101]

I am still waiting for a series about making a game! :)

[@Scorpisces1911]

Could I ask what pattern or approach is using in this project? How could I define the structure before starting this kind of project?

[@willtheorangeguy]

Noticed part 1 wasn’t linked in the description: kzbin.info/www/bejne/inXKoKiejKZkaaM

[@cinostroza]

Man you lost a lot of weight!

[@ArjanCodes]

Yep... it feels a lot better too.

[@samarbid13]

Disabling the squiggly lines in your code can be beneficial for visual clarity when creating KZbin videos, as it can be quite bothersome to have them scattered all over.!!!

[@andrewmenshicov2696]

i don't like that you define COMMANDS as a constant, but then use add_command to modify it 🤔. very strange. Also parse_command_string doesn't look like it'll work well when a user tries to encode a string of two words for example. probably you missed it out cuz u didn't want to bother your code reviewers to review your code review 😂. overall, still useful, good luck w/ further vids!

[@maleldil1]

> i don't like that you define COMMANDS as a constant, but then use add_command to modify it I only disagree with the name. Since it's obviously not a constant, it shouldn't be ALL_CAPS. However, having a module global and exposing functions to act on it isn't necessarily a bad idea. The standard library's random module does something similar: `random.seed` changes a global Random instance, and the same happens every time you call any function to generate a new random number.

[@andrewmenshicov2696]

@@maleldil1 yup that name is confusing. but still, not very fond of the module globals. personal preference ig 🤷‍♂️