Technical debt - Appointments controller

[IGORnvk]

Introduction

Hey code wranglers! Ever found yourself knee-deep in spaghetti code, desperately wishing for a magic wand to untangle the mess? Today we're diving into the wild and wonderful world of code refactoring. Join me on this journey as we explore the art of cleaning up our digital playgrounds, turning chaos into elegance, one line at a time. Let's refactor like there's no tomorrow!

Code, Code Smells and Refactoring

So, while looking through this amazing repo about a veterinarian clinic I came across this interesting function in appointmentsController.js:

export async function setAppointmentsForAYearAndMonthAndADay(req, res) {
  let body = req.body;
  let url = req.url;
  var url_parts = url.replace(/\/\s*$/, '').split('/');
  url_parts.shift();
  if (req.body.starttime && req.body.id && req.body.name) {
    res
      .status(200)
      .send(
        `Hi ${req.body.name}! I made an appointment for: ${url_parts[3]}-${url_parts[2]} at ${req.body.starttime}!`
      );
  } else {
    res
      .status(200)
      .send('Hi love. I cannot make any appoints because something is missing');
  }
}

What can you see at the first glance? If you are a JavaScript beginner, you would probably think that this function is nicely written and has no mistakes, which is in fact true. The code is correct and if you run it, it wouldn't throw any errors and perform as expected by the functionality.

Wait, what functionality? What does this piece of software actually do?

That is the question that might pop out in your head, so I will briefly explain the whole concept. The setAppointmentsForAYearAndMonthAndADay(req, res) is executed when a POST request is sent with the data about the client in body and the date of appointment in the url e.g.: /appointments/2023/12/14. The point of our function is to output the data with a nice message if the request contains all the neccessary information e.g.: Hi John! I made an appointment for: 14-12 at 10:00! or in case something is missing, output the following: Hi love. I cannot make any appoints because something is missing.
Not that hard, right? However, if you are more experienced SvelteKit developer, you would notice, or 'smell', that something is not right in this code, that it can be optimized somehow. To be precise, it has some Dispensables code smell, and we can apply some Organizing Data refactoring to it. Soo, shall we begin?

Let's have a look at this line:

let body = req.body;

We can see that variable body contains the body of our request, which contains data about the appointment. Although, If we continue reading through the code, we can notice that this variable is never changed or even used. The author uses req.body instead of the variable that already contains the values of req.body. What could be the fix to that issue? Yes, just use the variable and change its declaration from let to const, since it is not altered in any way:

const body = req.body;

And:

if (body.starttime && body.id && body.name) {
    res
      .status(200)
      .send(
        `Hi ${body.name}! I made an appointment for: ${url_parts[3]}-${url_parts[2]} at ${body.starttime}!`
      );
  } else {
    res
      .status(200)
      .send('Hi love. I cannot make any appoints because something is missing');
  }

It was interesting, isn't it? So let's continue exploring:

let url = req.url;
var url_parts = url.replace(/\/\s*$/, '').split('/');
url_parts.shift();

These three lines of code are responsible for retrieving information from the url, so if we sent a POST request with url appointments/2023/12/14, the url_parts array would contain 2023, 12 and 14 that we could later access, like here:

`Hi ${req.body.name}! I made an appointment for: ${url_parts[3]}-${url_parts[2]} at ${req.body.starttime}!`

As we can see, everything works as expected, but it is quite hard to read and understand the code with this url.replace(/\/\s*$/, '').split('/') part especially. And what If I told you that there is a more efficient way of retrieving the parameters from url? Here is the thing: we can get the exact same data just by writing req.params. It would contain everything we need in a form of an object. So, instead of getting the url from request and splitting it into array, we could do:

const detailsAboutAppointment = req.params;

And then access the properties like that:

`Hi ${body.name}! I made an appointment for: ${detailsAboutAppointment.day}-${detailsAboutAppointment.month} at ${body.starttime}!`

I also changed name of the variable so it would be more understandable of what it contains.

Conclusion and Test

Congratulations, we optimized an important piece of code for the veterinarian application! Let's check out how the function looks now:

export async function setAppointmentsForAYearAndMonthAndADay(req, res) {
  const body = req.body;
  const detailsAboutAppointment = req.params;
  if (body.starttime && body.id && body.name) {
    res
      .status(200)
      .send(
        `Hi ${body.name}! I made an appointment for: ${detailsAboutAppointment.day}-${detailsAboutAppointment.month} at ${body.starttime}!`
      );
  } else {
    res
      .status(200)
      .send('Hi love. I cannot make any appoints because something is missing');
  }
}

Much more readable, right? The only thing that is left for us is to actually test if the code still works! We can do it in Postman:

And there you have it, fellow code warriors! We've navigated the twists and turns of refactoring, transforming convoluted code into a masterpiece of clarity. Remember, the key to maintaining a healthy codebase is continuous improvement. So, go forth, refactor fearlessly, and may your future coding adventures be as smooth as a well-refactored function! Happy coding!

[rimmertzelle]

Thanks for your feedback, I really appreciate it! Can you create a pull request for me?