API with sequelize and Express: verification in controller

Question

I'm building my API for my app web. I'm using Sequelize and Express. I have created models with Sequelize and I'm writing for the moment the different controller and end-point.

My question is: Do I need to do data checks before my methods from Sequelize? I ask this question because the methods from Sequelize use data check too inside.

For the moment I have that. But... Do I really need to do checks given that they are also done inside Sequelize?

`exports.editUser = (req, res) => {
    let userId = parseInt(req.params.id);

    if (!userId) {
        return res.status(400).json({ message: "Missing ID parameter" })
    }

    User.findOne({ where: { id: userId }, raw: true })
        .then(user => {
            if (user === null) {
                return res.status(404).json({ message: "This user does not exist !" })
            }

            User.update(req.body, { where: { id: userId } })
                .then(user => res.json({ message: "User updated" }))
                .catch(err => res.status(500).json({ message: "Error BDD", error: err }))
        })
        .catch(err => res.status(500).json({ message: "Error BDD", error: err }))
}`

Can I write only that ?

`exports.editUser = (req, res) => {
    let userId = parseInt(req.params.id);

    if (!userId) {
        return res.status(400).json({ message: "Missing ID parameter" });
    }

    User.update(req.body, { where: { id: userId } })
        .then(user => res.json({ message: "User updated" }))
        .catch(err => res.status(500).json({ message: "Error BDD", error: err }));
}`

Answer

I think your question is more on good programming practices, my advice is to do your data checks which allows you more control and certainty on what your application is doing or processing instead of leaving it all to Sequelize. In the first piece of code:

`exports.editUser = (req, res) => {
  let userId = parseInt(req.params.id)

  if (!userId) {
    return res.status(400).json({ message: 'Missing ID parameter' });
  }

  User.findOne({ where: { id: userId }, raw: true })
    .then((user) => {
      if (user === null) {
        return res.status(404).json({ message: 'This user does not exist !' });
      }

      User.update(req.body, { where: { id: userId } })
        .then((user) => res.json({ message: 'User updated' }))
        .catch((err) => res.status(500).json({ message: 'Error BDD', error: err }));
    })
    .catch((err) => res.status(500).json({ message: 'Error BDD', error: err }));
};`

The flow of code is straightforward and allows you more control, return's appropriate message and avoid's unexpected error's. It check's if the userID is the right data type, checks if userID is supplied and most importantly check's if a user exists before proceeding with the user update.

The second piece of code which i referenced below has a few pitfalls, it assumes that the userId exists and thus results with your system executing a piece of code unnecessarily and then throw an error in the case that the specified ID does not exist (which is something you know would happen).

`exports.editUser = (req, res) => {
  let userId = parseInt(req.params.id);

  if (!userId) {
    return res.status(400).json({ message: "Missing ID parameter" });
  }

  User.update(req.body, { where: { id: userId } })
    .then(user => res.json({ message: "User updated" }))
    .catch(err => res.status(500).json({ message: "Error BDD", error: err }));
}`

This is a classic example of something called Technical Debt, where you intentionally or strategically leave out certain functions of your code which you know you need, either as a shortcut or to meet deadlines. This will have consequences in the future and will require code refactoring. For more reading on Technical Debt, read more here.

This answer was originally posted on Stack Overflow. You can find the full discussion here