Url Shortener Code Review

Code Feedback
1

Hey guys, i just finished the first somewhat substantial backend project and would love to get some tips on how I did/how to improve my code going forward. If anyone has some free node/express videos/articles that they could recommend as well that would be awesome, especially for better understanding middleware.

repo: https://github.com/twmilli/url_shortener
app: https://secure-crag-25604.herokuapp.com/

Thanks in advance!

2

I’m getting an application error when I try to use the app. :frowning:

3

It works for me - what were you trying to do when it failed?

4

I had just attempted to create a new short link. It seems to be working now though.

5

You’ve structured yours the way I did mine the first time - I actually redid mine again from scratch after seeking feedback on Gitter…

Here was the exchange regarding my old one on gitter - I scrubbed their username so they don’t get pinged by me posting this :slight_smile: :

username scrubbed May 22 21:29
@JacksonBates : It indeed is horrible You gotta modularize it.
The very first problem I see here is, every time your server receives a request, you server is going to open a new connection to the db port. Connecting to db is one of the most time consuming activities you can do.
What you need to do is cache the connection and reuse it afterward, instead of opening it over and over again.

Jackson Bates @JacksonBates May 22 21:38
so would I do that by opening the db connection outside of the various 'app.get’s?
I tried modularizing before, but I have trouble passing request and response objects around like that…I didn’t really feel like I learned modules from the Learnyounode lessons - are there other resources for learning modules you’d recommend?

username scrubbed May 22 21:48
@JacksonBates

  1. Separate your db logic from routing logic.
    I’ll share some review comments in some time. I am at work right now.
    You can take a look at FCC code base on github to see how they’re achieving modularity.

Jackson Bates @JacksonBates May 22 21:51
Thanks, I appreciate the pointers. I think I have enough to go on for now - but if there are glaring issues you haven’t mentioned yet and you have time to point them out later that would be great. You’ve been really helpful so far - cheers

2 Likes
6

@JacksonBates thank you so much, this is exactly what I was looking for! I went back and checked out how you structured yours and did a major refactor and I feel so much better about my code now. Even though it doesn’t make too much difference on small apps, I know this will be huge once I get started on the full stack projects!

7

As a followup, are any of you guys using local virtual environments for backend development over c9? I’d like to switch over but I tried once and found the process confusing and couldn’t quite get everything to work. If anyone has any tutorial recommendations/general advice, I would love to hear them!

8

I totally agree with the rest, that your code is nicely structured.

I haven’t used MongoDB a lot, but does this code only create a document if there isn’t already one for the specific url?

db.collection("urls")
        .update(
            {key: obj.key}, obj, {upsert: true});

Setting up a local environment can be quite easy (but it can also be quite difficult). What have you done already? Are you using Windows?

9

‘What you need to do is cache the connection and reuse it afterward, instead of opening it over and over again’

I don’t get what they mean by this. When I did mine, I just used mongoose to connect to a db (locally) at the top of the code, and write all the routes underneath. Is that bad practice?

10

Make sure that you only once open the database connection (and let it remain open). So that you don’t have to make a connection to the database every time you handle a request.

I suppose you are doing it the correct way. As long as you are only making a connection to the database once.

1 Like
11

Mongoose handles the db connection that way naturally as I understand it. I’ve only used Mongoose once, and it did make things easier in that regard.

1 Like
12

This is a great thread .

1 Like
13

@JacksonBates I took a look to your code, in particular the way you work with the db. Don’t you close it at any time right?? I read that Mongoose takes care of it, but I prefer to do it right now without it.

Thanks

14

There’s no need to close the connection with either mongo or mongoose in this use case.