I am relatively new to javascript ES6, the code I've written is working fine. However I am curious if this would be the best approach. I am trying to write a reusable module that I can use in multiple projects. On top of an API I've written a simple module to geocode adresses in the Netherlands. The suggest function will provide related adresses to a specific query. The lookup function provides information about a specific adress.
class Geocoder {
// Class to geocode adresses to coordinates or find related adresses
// suggest(), lookup() both return a promise
// geocoder.lookup('adr-54e85361e27f833dd8331fd85bc46ac6').then((data) => {console.log(data)});
constructor() {
this.url = "https://geodata.nationaalgeoregister.nl/locatieserver/v3/";
}
async suggest(query) {
let url = `${this.url}suggest?q=${query}`;
return await this._get(url);
}
async lookup(id) {
let url = `${this.url}lookup?id=${id}`;
return await this._get(url);
}
_get(url) {
return new Promise ((resolve, reject) => {
fetch(url).then((response) => {
return response.json();
}).then((json) => {
resolve(json);
}).catch((err) => {
reject(err);
})
})
}
}
1 Answer 1
Using Promise
when async
/await
is supported is a sign of an anti-pattern. The only case you ever manually build promise in an environment that supports async
/await
is when the asynchronous operation cannot be written linearly/when you can't await
(e.g. resolve
must be called in a callback).
// You cannot really "await" this kind of API, hence the wrapper Promise.
foo() {
return new Promise((resolve, reject){
someAPIThatOnlyDoesCallbacks((err, data) => {
if (err) reject(err)
else resolve(data)
})
})
}
But in your case, fetch
returns a promise, response.json()
also returns a promise. You can just return the promise generated by the whole fetch
operation itself.
_get(url) {
return fetch(url).then(r => r.json())
}
Next, consider using Url
and UrlSearchParams
to construct your url. This way, the url is constructed correctly.
async suggest(url, param1, param2, ....){
// Create the url
const url = new URL(url)
// Append the values
url.searchParams.append('param1', param1)
url.searchParams.append('param2', param2)
// Construct the url
return await this._get(url.toString())
}
On to other things, there's currently nothing in your class that is instance-specific (the base url is hardcoded, params are supplied at call time). You can simply export a bunch of stateless functions from the module instead of having to instantiate an instance.
const getJSON = url => fetch(url).then(r => r.json())
const geocoderUrl = "https://geodata.nationaalgeoregister.nl/locatieserver/v3/"
export const suggest = async (query) {
const url = new URL('suggest', geocoderUrl)
url.searchParams.append('q', query)
return await getJSON(url.toString());
}
export const lookup = async(id) => {
const url = new URL('lookup', geocoderUrl)
url.searchParams.append('id', id)
return await getJSON(url.toString());
}
-
\$\begingroup\$ Is it possible to avoid code duplication in lookup and suggest functions? \$\endgroup\$Stexxe– Stexxe2018年09月17日 11:02:32 +00:00Commented Sep 17, 2018 at 11:02
Explore related questions
See similar questions with these tags.
this._get(something)
overfetch(something)
? You're just saving that.then(response => response.json())
. 2) what do you need aclass
for? I don't see any encapsulated state, two exported plain functions will work pretty well. 3). Whatquery
parameter is? I'm lazy to go over geodata.nationaalgeoregister.nl specs, why don't you accept anobject
instead of an already encoded query string? \$\endgroup\$