How to make node js controller not so messy?

Question

i was making my nodeJs app and i realized that my controller is a little bit messy,so i tried to decompose the logic by creating various function but when i see the result, i have a feeling that something wrong with my code ????????. I mean, is it ok to use async function in another async function, or to use async functions the way i use them. Could anyone make a little code-review, i would be grateful for any help.

    import {Request, Response } from "express";
    import bcrypt from "bcrypt"
    
    import Role from "../../models/Role.js";
    import User from "../../models/User.js";
    
    export default class UserControler {
        public async createUser(req: Request, res: Response): Promise{
            try {
                async function getHashedPassword(password: any): Promise {
                    const salt: string = await bcrypt.genSalt(10);
                    const hashedPassword: string = await bcrypt.hash(password, salt);
                    
                    return hashedPassword
                };
                async function getDefaultRoleId(): Promise {
                    const [roleData, createdRole] = await Role.findOrCreate({
                        where: {
                            name: 'User',
                        },
                        defaults: {
                            name: 'User'
                        }
                    });
                    const roleId: string = roleData.getDataValue('_id');
                    
                    return roleId
                };
                async function emailExists(email: string): Promise {
                    const user = await User.findOne({
                        where: {
                            email: email,
                        }
                    });
                    if(user){
                        return true
                    } else {return false;}
                };
                
                const _emailExists: boolean = await emailExists(req.body.email)
                if(_emailExists) return res.status(400).json({exists: true});
    
                await User.create({
                    role: await getDefaultRoleId(),
                    email: req.body.email,
                    hidden: false,
                    password: await getHashedPassword(req.body.password)
                }).then((data) => {
                    res.json({data: data});
                })
            } catch (err: any) {
                res.status(400).json({error: err});
            }
        }
    
    }

Salvino Salvino · Accepted Answer · 2022-07-19T16:50:01

Firstly, I would encourage you to follow the practice of creation of services. I can see from your code that you already have the controller & models in place. The only piece you're missing are services. You can move all your logic to the service file and keep your controller as thin as possible.

Sample Implementation:

// userController.ts file
import { Request, Response } from "express";
import { emailExists } from "./userService";

export default class UserController {
  public async createUser(req: Request, res: Response): Promise {
    try {
      const { email, password } = req.body;
      const _emailExists: boolean = await emailExists(email);
      if (_emailExists) return res.status(400).json({ exists: true });

      const user = await createUser(email, password);
      res.json({ data: user });
    } catch (err: any) {
      res.status(400).json({ error: err });
    }
  }
}

Now create a userService.ts file, export the methods and then reference the required ones in your main controller file.

// userService.ts
import bcrypt from "bcrypt";
import Role from "../../models/Role.js";
import User from "../../models/User.js";

export const createUser = async (
  email: string,
  password: string
): Promise<User> => {
  return User.create({
    role: await getDefaultRoleId(),
    email,
    hidden: false,
    password: await getHashedPassword(password),
  });
};

export const getHashedPassword = async (password: any): Promise<string> => {
  const salt: string = await bcrypt.genSalt(10);
  const hashedPassword: string = await bcrypt.hash(password, salt);

  return hashedPassword;
};

export const getDefaultRoleId = async (): Promise<string> => {
  const [roleData, createdRole] = await Role.findOrCreate({
    where: {
      name: "User",
    },
    defaults: {
      name: "User",
    },
  });
  const roleId: string = roleData.getDataValue("_id");
  return roleId;
};

export const emailExists = async (email: string): Promise<boolean> => {
  const user = await User.findOne({ where: { email: email } });
  if (user) return true;
  return false;
};

How to make node js controller not so messy?

1 Answers

Sample Implementation:

References: