It's Not Bikeshedding!

#opinion#organization#style::alemi::2022-10-10 02:04

I often find myself reasoning on variable names or code style to an extent which may be classified as "bikeshedding", but I believe that my efforts are always worth the extra time investment in the long term.

Refactoring is annoying and rarely a priority, but code is shared and read constantly. "I'll fix it later" most of the time is just not going to happen soon enough or at all.

Ambiguous code is technical debt

All code will be read again sooner or later, be it for bug fixing, updating, or just learning. Even if you're writing a small script for yourself, something will eventually change and you'll have to fix it again.

While I'm writing something, I understand the issue, the solution and the choices I'm making. It's very important to embed all this knowledge in the piece of code I'm writing.

A decent way to enforce this behavior is to have colleagues constantly read your code: you shouldn't have to explain it! Because now you can explain it, but will the explanation in ~3 months be as good?

Many times I had to come back to awful Flask microservices thrown together in 15 minutes which frustrated me to the point that I rewrote them instead of understanding them again.

txt = html5lib.parse(r.text)  # this is not text
b = []  # what is this?
for frag in txt[1][1:]:  # fragments of what?
	load_img = frag[4][0].get("src")  # image, but loaded from what?
	t = dt_patch(frag[2].text)  # what does "patching" do?
	b.append((frag[3].text.replace("'", ""), t, load_img))  # weird tuple

Ok, this is some html parser, but which one of these is the main div? Was clear when I wrote it, now not as much!

Code should be easily accessible for your sanity and for its health. A codebase quick to set up, easy to build and properly documented will receive much more features and fixes than some forgotten binary which you can't really build anymore. And spending time re-understanding code periodically is not a good investment.

Variable names are comments

This may come as a hot take, but bear with me.

int operation(t_obj object, char * dest, int dest_size) {
	link to_free;
	int actual_size;

	if (object->head == NULL) return 0;

	if (object->head == object->tail) {
		object->tail = NULL;
	}

	to_free = object->head;
	object->head = object->head->next;

	actual_size = to_free->size > dest_size ? dest_size : to_free->size;

	if (dest != NULL) {
		memcpy(dest, to_free->data, actual_size);
	}

	free(to_free->data);
	free(to_free);
	object->count--;

	return actual_size;
}

I removed any comment from this function, took it out of its context, removed its explicit typedef and function name. Yet I believe it's still possible to understand what this function is doing, and maybe even on what data structure.

When you name things a or test you're setting yourself up for a headache later on. Names like buffer and count and i are still pretty generic but may make sense in specific cases.

Properly naming your data makes the understanding of data flow way easier. I believe that it's worth it to spend ~10 minutes more naming everything properly rather than spending hours debugging later on.

If you're having a hard time coming up with appropriate names, maybe due to multiple instances of the same "thing", consider scoping these items under function calls. Each item (function, variable, constant, ...) you use should describe itself with a consistent format (having local_ip and remoteAddr will cause many headaches in the long run). I try to keep my names short for laziness, but I don't shy away from long names when needed.

Comments overload is as bad as no comments

When dealing with external libraries I sometimes end up over commenting my code

Server::builder() // make server instance
	.add_service(SessionService::new(state.clone()).server()) // register session service
	.add_service(WorkspaceService::new(state.clone()).server()) // register workspaces service
	.add_service(BufferService::new(state.clone()).server()) // register buffer service
	.serve(addr) // bind on address
	.await?; // block and run

Each of these comment is not needed, since a better documentation exists for each of these functions.

Usually, if I find myself commenting single operations, I try to instead document the function I called or the variable I changed. Useful comments should explain how code combines operations to achieve its goals.

While comments can overload, there's never too many docstrings!

TLDR:

Form over function? I don't believe so! For any software, being understandable is part of its function, so I think that spending time to make it easily understandable is not bikeshedding.