Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve overall code quality #22

Open
emanruse opened this issue Nov 23, 2023 · 2 comments
Open

Improve overall code quality #22

emanruse opened this issue Nov 23, 2023 · 2 comments

Comments

@emanruse
Copy link

This is a very interesting project. Thank you for working on it.

If I may suggest, an improvement in overall code quality would make this good work even better. After all, this is something which one is supposed to run in dom0 and it is good to have the possibility to go through it easily before using it. Personally, it took me more than 2 hours to check /.lib only (and I write bash scripts almost every day).

While there is no unified standard about shell script coding, there are some good practices. To name a few (not a complete list):

  1. Don't use VAR_NAMES_IN_CAPS. Use lowercase_name instead. This avoids the potential problem of conflicting environment variable names (which are uppercase).

  2. Use local variables whenever possible

my_function()
{
	local my_var="${1}"
	...
}

and you won't need to use cryptic prefixes like _VRP_SIZE.

  1. I think [ x"$a" = x"$b" ] is outdated. The preferred way (especially if you use Bash which is installed in dom0) is [[ "${a}" = "${b}" ]].

  2. If you need to comment what the code does (e.g. # Check if file exists), this means the code is not readable. Commands and variable names should speak for themselves. Comments should explain why something is done, not what is done.

  3. Don't write complex nested loops and ifs (like in .lib/check-with-local.sh). Complex code is not only difficult to read but also difficult to maintain.

  4. Limit line length to 80. It's more readable.

  5. The result of any code execution should be predictable even in unpredictable situations and handled properly.

  6. Exit early.

E.g. this:

my_function()
{
	if [ -e "${some_file}" ]; then
		add_line dom0 "some" "thing"
	else
		add_line dom0 "something" "else"
	fi
}

is better written as:

my_function()
{
	if [ -e "${some_file}" ]; then
		add_line dom0 "some" "thing"
		return
	fi
	add_line dom0 "something" "else"
}
  1. Short lines and vertical orientation are more readable.

This:

command arg1 \
	arg2 \
	arg3 \
	| another_command \
	| third command
fourth_command

is more readable than this:

command arg1 arg2 arg3 | another_command | third command'; fourth_command
  1. Check your scripts with ShellCheck

And so on. There are some good books about general code readability.

Please also add a license, so others can contribute.
Keep up the good work!

@froeb
Copy link

froeb commented Feb 1, 2024

Great sugestions emanruse. The crator seems to have abandoned this project or at least seems not to reply.
Do you have a fork available where you implemented your recomenadations at least for some files/directories so that one can use your improvements?

@emanruse
Copy link
Author

emanruse commented Feb 1, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants