fac-15 / Curenetics

https://curenetics.netlify.com/
3 stars 2 forks source link

Hardcoding url in your app #56

Open emilyb7 opened 5 years ago

emilyb7 commented 5 years ago

Hello!

I've just had a really quick look, and I think this line of code means that I'm not going to be able to run the app for now...

In https://github.com/fac-15/Curenetics/blob/staging/src/Components/App/App.js

const baseUrl = "http://35.234.148.3:8090/data/trials/uk/";

I'm guessing you're planning on calling some third party API

Just an idea for how to deal with this kind of thing so that everyone can collaborate on the project...

You've probably used .env files for storing database credentials etc. This is a different use case, because we're not talking about sensitive data that we want to conceal.

But having the url for the API in a separate config file could prove to be useful, in case you want to switch out the URL for testing purposes in the future. Or if you need to avoid getting rate limited during development. Or if your internet connection is slow, etc.

Let me know if that's confusing or if you have questions/concerns!

Also, while we're here...

This line is also very difficult to read: fetch(${baseUrl}${postCode || "cm27jp"}/${distance}/${gender || "m"}/${age || "70"}/.json)

Consider making a separate function for building your url, and ideally use a library (just the standard querystring library like you have available in node.js is fine) to handle the query params.

emilyb7 commented 5 years ago

Ok, ignore my concerns, it is working fine :) I was thrown off by the .json url.

But I stand by my comments about building the url in a more easy-to-read way!

emilyb7 commented 5 years ago

ooh I've just seen you're working on a file called api.js in utils. That looks like a great way to do things. Nice work :)