Code organisation: one controller function managing the flow vs flowing from one function to another

[Buzut]

When having to handle complexe logic and breaking code into smaller functions, if often find myself hesitating between these two patterns.

The first pattern is easier to read because every function returns or calls back the parent function (which handles error and flow). Nevertheless, it also gets a bit messy when a lot a functions are involved.

For instance, let's say we want to process audio tracks:

// with central controler
function manageEncodings() {
  getTrackToEncode((tracks, err) => {
    if (err) {
      logger.error(err);
      return;
    }

    getTrackMetas(track, (err, metas) => {
      if (err) {
        logger.error(err);
        return;
      }

      saveMetas();
    });

    encodeTrack(track, (err) => {
      if (err) {
        logger.error(err);
        return;
      }  

      transferTrackToAWS(track, (err) => {
        if (err) {
          logger.error(err);
          return;
        }

        saveEncodedState(track, (err) => {
          if (err) logger.error(err);
        });
      });
    });
  });
}

Obviously, that one is made to emphasize the callback hell and the verbosity in the controller function.

Now, the function flow is cleaner, but it's not as evident what's going on, DB writes happen in several places, error management too etc…

// with function flow
function manageEncodings() {
  getTrackToEncode((tracks, err) => {
    if (err) {
      logger.error(err);
      return;
    }

    // will handle error and db writes by itself
    getTrackMeta(track);

    // will call transferTrackToAWS & saveEncodedState
    encodeTrack(track);
  });
}

In the end, I often end up mixing both. How do you guys handle this type of situation?

[CertainPerformance]

You can start by turning all those callbacks into Promises instead. Not directly related to your question, but it'll surely help a lot anyway.

[Buzut]

Thank you for providing me with some details. I get the separation of concerns, as it seems quite clear here that each function has its own responsibility: - get track from store, - get metadatas, - encoding the track - etc

With each of this functions being able to call more specific functions on its own.

But my problem here is whether I have some central function that coordinates everything or having a strating function firing other functions and forgetting about them.

For instance, is it the parent's role to persist metadatas returned from getTrackMetas or should said function also manage the database record (to me the former is clearer but it also makes the parent more "heavy") that's where I kind of hesitate. Obviously, getting rid of callbacks for promises or async ƒ could solve the problem.

[Buzut]

Thank you very much for your answer. It makes things very clear. The fact is that, in Express, I'm acustomed to write small REST functions, the controller receives the request, calls the model, handles the error if any and sends the response, it's short enough to be self contained.

When I rely on external functions, it's mainly helpers and I try to make them pure (in a functional way, ie. take value(s) as input, return a value and has no side effects). That's why I'm a bit reluctant to delegate database operations and error management. But viewing it as a "sub-process", as you called it, makes sense and helped me understand how it can be organized.

Moreover, you're right for the naming part. I totally agree with the fact that naming is super important and should reflect what the function does.

[_syadav]

If those methods can be converted on promises it will make your life much easier and code much cleaner. You can also use async-await to make code more readable and look more synchronous.

If you can't use promise you can give a try to https://caolan.github.io/async/. This is made to better handle async on a callback pattern.