Should I opt for code repetition or consolidation with api service - JS

Question

I'm working on a large CMS system where a particular module and its submodules take advantage of the same backend API. The endpoint is exactly the same for each submodule aside from its "document type".

So a pattern like this is followed:

api/path/v1/{document-type}

api/path/v1/{document-type}/{id}

api/path/v1/{document-type}/{id}/versions

As time goes on the number of modules that use this API grows and I am left with many, many redundant api services that implement 7 CRUD methods:

getAllXs() {...}
getX(id) {...}
getXVersion(id, versionId) {...}

etc...

with an individual method looking like this

getAllXs() {
    let endpoint = BASE.URL + ENDPOINTS.X;
    let config = ...
    return http.get(endpoint, config)
        .then(response => response.data);
        .catch(...);

}

Where X would be the name of a particular Document Type.

I came to a point where I decided to make a single service and do something like this:

const BASE_URL = window.config.baseUrl + Const.API_ENDPOINT;
const ENDPOINTS = {
  "W": "/v1/W/",
  "X": "/v1/X/",
  "Y": "/v1/Y/",
  "Z": "/v1/Z/",
}

getAllDocuments(docType, config={}) {
  let endpoint = BASE_URL + ENDPOINTS[docType];
  return http.get(endpoint, config)
        .then(response => response.data);
        .catch(...);
}
...other methods

Where a type is specified and a mapped endpoint is used to build the path.

This reduces all of the document api services down to one. Now this is more concise code wise, but obviously now requires an extra parameter and the terminology is more generic:

getAllXs() --> getAllDocuments()

and it's a bit less 'idiot-proof'. What makes me insecure about the current way it is written is that there are 6 modules that use this API and the same 7 methods in each service.

The questions I keep asking myself are:

• Am I bordering anti-pattern with the dynamic functions?

• What if I had 10+ modules using the same API?

Answer 1

Your question made me think of a common Object Relational Mapping design problem.

There are no single source of truth when it comes to design, but if your recognize an ORM in what you are building and value object oriented design principles, I have some inspiration for you.

Here's an over simplification of my own vanilla ES6 ORM I have used on many projects (reusing your code snippets for relatability). This deisgn is inspired by heavy ORM frameworks I have used in other languages.

class ORM {
   constructor() {
      this.BASEURL = window.config.baseUrl + Const.API_ENDPOINT
      this.config = {foo:bar} // default config
   }

   getAll() {
      let endpoint = this.BASEURL + this.ENDPOINT
      return http.get(endpoint, this.config)
       .then(response => response.data)
       .catch(...)
   }

   get(id) {
      // ...
   }
}

And examples of extension of that class (notice the one with a special configuration)

class XDocuments extends ORM {
   static endpoint = '/XDocument/'

   constuctor() {
      super()
   }

   otherMethod() {
      return 123
   }
}

class YDocuments extends ORM {
   static endpoint = '/YDocument/'

   constuctor() {
      super()
   }

   getAll() {
      this.config = {foo:not_bar}
      super.getAll()
   }
}

Since you are specifically asking if this is bordering anti-patterns. I would suggest reading about SOLID and DRY principles, and ORM designs in general. You will also find code smells about global constants, but this would only be true if you are in a window context. I see you are already on the right path trying to avoid code duplication smell and the shotgun surgery smell. :-)

Good luck and don't hesitate to ask further questions and add additional details in comments!

Answer 2

If you'd provide more of your original version of code, that'd be more points to point to.

At the moment I just can say that the DRY principle is generally a good idea (not always but... well it's a complicated topic). There's a lot of articles about DRY. Google it.

You're afraid that your code became more complex. Don't be. In my experience, novice programmers fail miserably exactly because they discard the DRY principle. And only after a while, when they become stronger they start to fail at the KISS principle. And only one extra argument in addition to the 3 that already there doesn't add much to the complexity.

Am I bordering anti-pattern with the dynamic functions?

To be "dynamic" - that's the reason functions exist

What if I had 10+ modules using the same API?

Exactly! 10+ more chances to make a typo, to forget something, to misread, etc and 10+ more work to do if you'll need to change something. That is if you don't DRY your code.

^{PS. And the name getAllDocuments is actually better than getAllDocType1s if that's the real name from your original code.}

Answer 3

While I share most of x00's answer, I would take into account how static your endpoints really.

Is there no chance module "X" can change any of its endpoints definitions? For instance, you need to pass one more query param. Are your modules all exactly the same with no room for change?

If the answer is no, there you go. To make that simple change you would have to refactor your whole code base (if you would implement it the way you propose, that is).

If the answer is yes, well, I see no reason for you to not implement your proposed dynamic functions. Personally, I would lean towards a service that my modules extend and use, just in case I do want to make minimal changes to them. For instance:

class MyGenericService {
  constructor() {
    this.url = window.config.baseUrl;
  }

  async getAllDocuments(config) {
    return http.get(this.url, config)
      .then(response => response.data);
      .catch(...);
  };

  // ...and so on
}

This allows my code to scale and be modifiable, with just one takeaway which is you would need to maintain one file per module, that has something like this:

class XService extends MyGenericService {
  constructor() {
    this.url = window.config.baseUrl + '/v1/x';
  }
}

If maintaining this extra files is too much overhead, you could receive the endpoint's URL in the constructor, on the MyGenericService, and you would just need to do stuff like this in your controllers:

const myXService = new MyGenericService('/v1/x');
const myYService = new MyGenericService('/v1/y');
// ...or it could use your endpoint url mapping
// I don't really know how is your code structured, just giving you ideas

There you have some options, hope it helps!