Async function inside async function, callback hell

1

Hi,

Can anyone give some advice on how to structure/refactor my code please? To be specific, I’m attempting to refactor the routes.js file createAndInsertUrl (line 62) and where it’s used in line 90, but got wrapped up in spaghetti code and callbacks. It’s currently returning undefined for the uniqueIdCount, which I believe is due to the createAndInsertUrl running before the uniqueIdCount is returned. I’m a bit lost with where to go next.

I had the whole thing working from one file, but wanted to refactor it to best practices… problem is, I’m not well practiced!

Thanks,
Tim

2

Hi Tim,

Nice job on the project! A couple of tips for you to refactor your code. These are just from my experience so take them for what it’s worth.

  1. Consider moving your schema definition into it’s own file and using module.exports and require to import the model into your server script. http://mongoosejs.com/docs/guide.html

  2. Use environment variables to hide your connection URI http://stackoverflow.com/questions/22312671/node-js-setting-environment-variables

  3. Consider generating your short link using a random combination of letters and numbers so you can minimize the size of the url. Here’s an systems design problem that goes into how to make that happen https://www.interviewcake.com/question/javascript/url-shortener. An added benefit to this approach is that you wouldn’t need those nested calls on your server within findNextId(), improving performance since you eliminate server calls. It also leads to shorter URLs - A combo of upper/ lowercase letters and numbers gives you 62 options for each character, so even with 5 characters you would get 62^5 combinations ~ 900 million possibilities.

  4. For error handling, you can create an error type in javascript let error = new Error(“This my my error text”); error.status(400); and then return that error as your response or let the error be handled by error handling middleware with next(). Be careful of throwing an error without handling since that will generate an uncaught exception and crash your server. http://stackoverflow.com/questions/7310521/node-js-best-practice-exception-handling

  5. For your /new route, I would suggest capturing the url using the url parameter /new/:url let url = req.params.url or using a post request so you can get the request body.

  6. To further clean up your code: a) You can export your helper functions into a separate file and then import them (see code sample below). b) Save function results to variables before passing them into other functions. c) When you have to pass a ton of parameters into a function consider grouping them into an object beforehand i.e. const opts = { cat: “meow”, dog: “bark”, whale: “waaoo” }; makeAllNoses(opts); In ES6 you can use destructuring to make this even better funfunfunction: Destructuring: What, Why and How - Part 1 of ES6 JavaScript Features

// url.js

module.exports = {
    createURL: function() {
       // code
    },
    isValidURL: function() {
       // code
    }
}

// routes.js

const url = require("./url");

const stub = url.createURL();
1 Like
3

Wow, thanks for such a thorough response Joe! I really appreciate it. Lots of refactoring to do. I’ll look at each of these points. You’ve opened my eyes to several new concepts there… probably a week’s worth of reading to do now :slight_smile:

Cheers,
Tim.