Jumpy Code with Descriptive Function Names

Question

I'm currently building out some functionality on a page. When writing JS I've been making a set of smaller functions like FindCarouselLocations(elem), GetImagesFromLibrary(url), and BuildCarousels(images, locs) which all use a subset of functionalities found in my global JS file.

When designing a page that's fairly heavily JS-reliant, does it make sense to have a single function that tells a story through other function calls? With the example functions above I'd make:

// previous function definitions...

function AddImageCarousels (){    
    var imageLibraryUrl = 'http://mysite.com';
    var parentElem = document.querySelectorAll('.image-carousel');

    var images = GetImagesFromLibrary(imageLibraryUrl);
    var locs = FindCarouselLocations(parentElem);
    BuildCarousels(images, locs);
}
document.addEventListener('DOMContentLoaded', AddImageCarousels, false);

Which I think tells a story on what happens when the page loads. I could alternatively remove some abstraction but would then have to start adding more comments or refactor another way. I, personally, think this creates a bit of self-documentation but I'm fairly new to development practices...

If I had a lot of things going on in the page, I'd even take one final step further and make a function that could look like this:

// previous function definitions...
function AddImageCarousels(){ /* */ }

function BuildPage () {
    AddImageCarousels();
    AddChatbot();
    AddMaintenanceNotification();
}
document.addEventListener('DOMContentLoaded', BuildPage, false);

Where each main functionality was defined by a single function, but those functions are built similar to the AddImageCarousels function.

Is this good practice or a code smell that implies a need for better design? And if it does, then what would be a better alternative?

I prefer to keep production files into as few separate files as possible to reduce the number of HTTP calls, but each functionality is usually in its own development file and transpiled later into the same .js.

Answer 1

does it make sense to have a single function that tells a story through other function calls?

Yes.

But using other function calls is not what's so nice about this code. What's nice is that it tells one short story, at one level of abstraction, and it uses good names. Using other function calls is simply how you pulled that off.

Solving a complex problem with short functions demands many functions. Many functions demand many names. Therefor the limiting factor here is your ability to come up with good names. If you can't give me good names then give me long functions. Spare me bad names on short functions.

Another nice thing is that it's clear which functions work by side effects and which work by returning a value. That's a feature of following Command Query Segregation.

The pattern I see in AddImageCarousels() is: construct dependencies and pass them to something that does what we promised we were going to do. This separation ensures parameterized versions of functions exist but doesn't dump dependency construction all over your entry point.

The limitation of this design is that add functions can't share. Unless that becomes critical I would simply tolerate the duplication in favor of the clean separation.

Another issue is that BuildCarousels() being parameterized is only useful if it's called in more than one way. That other way needs a name that differentiates from AddImageCarousels(). Again, what limits you're power here is ability to come up with good names.

I have to say, this is some of the most beautiful code I've seen in awhile. Hope it stands up to use.

Answer 2

function BuildPage () {
   AddImageCarousels();
   AddChatbot();
   AddMaintenanceNotification();
}
document.addEventListener('DOMContentLoaded', BuildPage, false);

This is great code. The function names are descriptive and it is easy to see how the page is built. If you wanted more details about how the chatbot was added, you could look at that method, but if you just want to see how the page is built, or what is on the page at a high-level, you just need to look at this one method.

Note that a very similar bit of code would be terrible:

function BuildPage () {
   AddIcs();
   Chat();
   AddEntity();
}
document.addEventListener('DOMContentLoaded', BuildPage, false);

Because the function names are not at all descriptive. In order to figure out what is on the page, you have to look at each of the 3 functions, meaning you have to hold a lot in your brain about what this bit of code is doing.

One caveat to the code you wrote - you have to ensure that each of the methods called in BuildPage does exactly what it says it's going to do. You don't want your AddChatBot() method to be messing with the image carousels or getting stuff set up for the notifications, or anything else - just do what it needs to do to add the chat bot.