Rhythmblock - a graphical, customizable music player

Started by nixgoat, May 07, 2021, 09:52 AM

Previous topic - Next topic

nixgoat


Rhythmblock
a graphical, customizable music player for ComputerCraft



Features
  • easily customizable (just edit the file and change whatever colors you want directly from the variables)
  • compatible with all placeable computers and all monitors
  • keyboard support
  • eject discs directly from the player
  • status bar

Keyboard Shortcuts

space - play/stop
e - eject
q - quit

Install

run the following on the desired CC computer

wget https://fem.mint.lgbt/m/Rhythmblock/raw/branch/master/rhythmblock.lua Rhythmblock
Source Code

you can find the source code for this project at https://fem.mint.lgbt/m/Rhythmblock
mint
they/them
https://nixgoat.me

nixgoat

i'm open for people to review my code! i'm starting out in lua and it would be pretty useful if you sent me suggestions on what to improve
mint
they/them
https://nixgoat.me

exerro

Hello and congrats on the first post! It's a nice program - don't have much use for it myself, but it does one thing and it does it well, that's always a good sign :)

Regarding the code, I noticed some good things:
* Good use of functions! For the most part, the code isn't too indented which is nice. The little "units of functionality" and recurring operations have been taken out of larger blocks of code and put into small, focussed functions.
* There are some comments. You psycho, documenting your code... Everybody knows that to be a successful programmer you need 1000 line unreadable monoliths that nobody but you will understand, and that's on a good day.

A not so good thing...
* You're afraid of `local` - functions a variables should almost always be local. You have a few locals in the program but there should be *way* more. This is probably what I'd focus on if I were you. I get the impression that maybe you're coming from another language? This is a pretty Lua-specific thing that you just need to get used to. Linters can generally catch and warn about global usages.

And some trivial issues...
* Your `round` function is overcomplicated. round(x) = floor(x + 0.5) - Think of rounding 1, 1.25, 1.5, 1.75, and 2 using this.
* You compare to `true` sometimes, which is rarely needed. `if x == true then` can usually be simplified to `if x then`
* Your mouse click box testing is shared between two lines and pretty long, this could maybe be a function that's called where mouse click testing is needed?
* I generally try to group variables, functions, and "main code" together. You've got some variables interspersed between functions which can make them a lil' hard to track down. This is mostly personal preference though.
* It's good to roughly describe what a function does, if it's non trivial, and also the general aim/purpose of a file. Nobody does this.

Good luck with your future projects!

Lupus590

#3
QuoteLinters can generally catch and warn about global usages.
There's also the bios.strict_globals setting which will crash the program if it tries to set a global.
You can use the set program or the settings API to set any settings API value.

Quote from: exerro on May 07, 2021, 01:08 PMYour `round` function is overcomplicated. round(x) = floor(x + 0.5) - Think of rounding 1, 1.25, 1.5, 1.75, and 2 using this.
I can't think of a better way of doing this.

nixgoat

thank you @exerro! i took the time to fix the if statements but couldn't fix the global vars. the rounding variable is actually just a kang from here
mint
they/them
https://nixgoat.me