URL Shortener Code check help

[MarianoFarace]

Hi guys, I was doing this challenge, and here is post’s Handler method. It is working, but somehow, the code structure does not look nice. I feel that it is not the right way to solve but some workarround that I’ve found. All my logic is stored under the dns.lookup and that is what is bothering me, for I could not find a way to take the error object out of the inner callback and use it in an “if” statement outside the dns.lookup method.

I tought about storing it in a variable outside of the callback function, but it seems more like a workaround than a canonical solution and I did not make dns.lookup a separate middleware because I don’t feel like it is the aproppiate solution, but maybe it is.
I understand that usually there are many ways of solving a problem, but again, there must be a more suited one in this case.
Would you mind taking a look at it, and letting me know what would have been a more conventional or appropriate way of making handler?

create_url_record_on_DB = function(req, res, next) {
    dns.lookup(req.body.urlToBeShortened, function(err, addresses) {
        if (err) {
            return res.json({ error: "invalid url" });
        } else {
            const URLSPair = new URLShortenerModel({

                originalURL: req.body.urlToBeShortened,
            });
            URLSPair.save(function(err) {
                if (err) return console.error(err);
                res.json({
                    original_url: req.body.urlToBeShortened,
                    short_url: "TO BE IMPLEMENTED",
                });
            });
        }
    });
};

And the route

app.post("/api/shorturl/new", create_url_record_on_DB);
Challenge: URL Shortener Microservice

Link to the challenge:

freeCodeCamp.org

[Sky020]

Hello there,

@MarianoFarace

dns.lookup(req.body.urlToBeShortened, function(err, addresses) {

Not quite to do with your question, but this will not pass the freeCodeCamp tests, because the first argument to dns.lookup must be only a domain; this is passing in the full URL.

---

There is nothing wrong with having the response and DB logic within the dns callback. In fact, I am not sure why one would do differently.

Currently, all you might need to improve on is finding an appropriate way to finalise the action. That is, it works fine to attach the necessary object to the response, but there is nothing telling your app the action has been completed.

The easiest way to do this is to return from the app.post callback. At least, that is the only way I am comfortable closing the request.

Hope this helps

[MarianoFarace]

Thanks @Sky020 . I’ll change that about the domain.

@Sky020

The easiest way to do this is to return from the app.post callback. At least, that is the only way I am comfortable closing the request.

Regarding this, I don’t quite understand what you are suggesting. If it is an easy thing, would you mind making the changes so I get to see it? Or do you mean adding a return before this:

res.json({
                    original_url: req.body.urlToBeShortened,
                    short_url: "TO BE IMPLEMENTED",
                });

[Sky020]

@MarianoFarace

would you mind making the changes so I get to see it?

It needs to happen in your app.post (or whatever other verb) callback.

EDIT: After reading up more, It is actually not important in Express. What I was thinking of is preventing the client from hanging, if a res method is not used/completed, or next is never called.