2
\$\begingroup\$

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);
            })    
        })
    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Maybe code is in an early stage then...let me ask before answering: 1) what's the added value of this._get(something) over fetch(something)? You're just saving that .then(response => response.json()). 2) what do you need a class for? I don't see any encapsulated state, two exported plain functions will work pretty well. 3). What query parameter is? I'm lazy to go over geodata.nationaalgeoregister.nl specs, why don't you accept an object instead of an already encoded query string? \$\endgroup\$ – Adriano Repetti Sep 10 '18 at 12:33
  • \$\begingroup\$ @AdrianoRepetti 1) Indeed, I did that to save a bit of code in order to have cleaner and more readable code in all projects where I use this module. 2) You are right, I don't need class. I guess a good use case for class are multiple instances of something which hold their own separate state? 3) The parameter is a string, I don't really understand what you mean. That I take an object as input and create a string of it in my suggest / lookup function? \$\endgroup\$ – Titsjmen Sep 10 '18 at 13:33
3
\$\begingroup\$

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());
}
\$\endgroup\$
  • \$\begingroup\$ Is it possible to avoid code duplication in lookup and suggest functions? \$\endgroup\$ – Stexxe Sep 17 '18 at 11:02

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.