How could you prevent controller callback methods from becoming too big? about bulletproof-nodejs HOT 10 OPEN

Ah I get it! Makes complete sense then, I just had this thinking that a service was only meant to retrieve/do things with database layer stuff. But the service layer is meant to do any logic for that particular endpoint and, like you said, if the service gets to big, then follow the facade pattern. Makes sense to me!

It seems I was really just under thinking how the services need to be used, I should use them to their full extant.

Thanks!

from bulletproof-nodejs.

Hi @HappyZombies

The controller should only have 1 call to a service that handles all the business logic required for that endpoint.

I know that a search endpoint is one of the most complicated ones because you pass several filters.

If you need to call several services, you that may run into circular dependency injections.
To avoid that, you could try to apply the Facade Pattern (read this related issue #7)

Essentially is just a service that aggregates several services.

Let's see an example, suppose you have a service that gets Venues/Places from third-party providers (google places, facebook places graph, yelp, etc) and your own database.
Then, using the twitter api you get all twits near a place.

import PlaceService from './PlaceService'
import GooglePlaceService from './GoogleAPIService'
import FBPlacesGraphService from './FBPlacesService'
import YelpService from './YelpService'
import TwitterService from './TwitterService'

class SearchService {
 constructor(
   private placeService: PlaceService,
   private googlePlaceService: GooglePlaceService,
   private fbPlacesGraphService: FBPlacesGraphService,
   private yelpService: YelpService,
   private twitterService: TwitterService
 ) {}
 
  async  public Search(filters, location) {
    const allPlaces = await Promise.all([this.searchInFacebook(filters, location), this.searchInGoogle(filters, location), this.searchInYelp(filters, location), this.searchInOwnDB])     

    const filteredPlaces = await allPlaces.map(this.removeDuplicates)

    const twitts = await this.getTwitsNearALocation(location)
    
    const placesWithTwits = await this.mergeAllTogether(filteredPlaces, twits);
    return result;
   }

   private getTwitsNearALocation(location) {

   }
   private searchInFacebook(filters, location){ 

   }

   private searchInGoogle(filters, location){ 

   }

   private searchInYelp(filters, location){

   }
   private searchInOwnDB(filters, location){

   }

   private mergeAllTogether(places, twits) {

   }

   private removeDuplicates(places) {

   }
}

What do you think?

from bulletproof-nodejs.

I tried splitting code like this but I get an error saying this is undefined inside the class. Any idea what may cause this? Tried logging this in the constructor function and still showed as undefined.
Even tried using .bind() in the router while calling the controller but didn't work.

from bulletproof-nodejs.

Hey I can help. This has happened to me before. If you’re using regular Node (no typescript) make sure your routes callback are using arrow functions. Could you give an example of what your code looks like?

from bulletproof-nodejs.

I used .bind() and it is now working. I think when passing the controller as a middleware the context for this inside the controller class comes as undefined. My current setup is
index.js -- Using koa-router

const FundController = require('./controllers');
const CreateFundFacade = require('./services/create-fund');
const SearchFundFacade = require('./services/search-fund');
const FundService = require('./services');

// Injecting the stuff here. DB models are directly imported, not using injections now
const createFundFacade = new CreateFundFacade();
const searchFundFacade = new SearchFundFacade();
const fundService = new FundService(createFundFacade, searchFundFacade);
const fundController = new FundController(fundService);

router.post('/', validateSession, fundController.createFund.bind(fundController));

My controller

class FundController {
  constructor(FundService){
      this.fundService = FundService;
   }

  async createFund(ctx) {
    try {
      const result = await this.fundService.createFund(ctx.request.body);
      return ctx.send(200, {
        message: 'Fund has been added successfuly',
        fundID: result.fundID
      });

    } catch (err) {
      console.log('Error at create fund');
      console.log(err);
      return ctx.send(500, 'Something went wrong');
    }
  };

  module.exports = FundController;
}

The createFundFacade

class CreateFundFacade {

  createFund(data) {
    return new Promise(async (resolve, reject) => {
      try {
        const {
          // lot of data
        } = data;
        const fund = new FundModel({
          // create mongoDB object from data
        });
        resolve(await fund.save());

      } catch (err) {
        reject(err);
      }
    });
  };
};
module.exports = CreateFundFacade;

And finally the service

class FundService {

  constructor(CreateFundFacade, SearchFundFacade){
    this.createFundFacade = CreateFundFacade;
    this.searchFundFacade = SearchFundFacade;
  }

  createFund(body) {
    return this.createFundFacade.createFund(body);
  };

  getFundByID(body) {
    return this.searchFundFacade.getFundByID(body);
  };

  getFundsByInvestorID(investorID) {
    return this.searchFundFacade.getFundsByInvestorID(investorID);
  };
};

module.exports = FundService;

Not sure if I am doing this right. Couldn't find any solid examples for creating the facade pattern

from bulletproof-nodejs.

Yup, using bind will help with the ctx from koa. Also, personal opinion, but I don't seperate out the controllers since I don't want to pass the ctx to different classes, I want to keep it all contained. You can see how I did it in my personal project for context . Again personal opinion! In order to handle different errors I have a "ApiError" class that extends Error, and simply call next() whenever I throw it

from bulletproof-nodejs.

from bulletproof-nodejs.

@varunagarwal315 I would have my services instance inside the controller function, that way a new instance per request is created, it is crucial if you use any kind of state inside the class.

Here is more information about it...

https://stackoverflow.com/questions/47664079/node-js-express-and-object-instantiation-inside-outside-route-endpoints

from bulletproof-nodejs.

from bulletproof-nodejs.

@HappyZombies Hey in this pattern have you tried using arrow functions inside the controller and service class instead of defining the functions. Eg

async doSomething(req, res, next) { }

becomes

doSomething = async (req, res, next) => { }

This allows us to avoid using bind() in the router file. It seems a bit cleaner but read in a few stackoverflow answers that it is advised to avoid using arrow functions in any class. Thoughts?

from bulletproof-nodejs.