0

I'm working on the backend for a web app using node-postgres and I'm curious as to what others think. I need basic insert queries for a users table, products table, and other tables to come. I don't want redundant code but I also don't want to slack on readability. Would it be better to have several similar query functions:

NOTE: createValueString() just creates a parameter string ($1, $2, $3)

async function createUser(user) {
  const client = await pool.connect();
  const userKeys = Object.keys(user).join(",");
  const userValues = Object.values(user);
  try {
    const { rows } = await client.query(
      `
        INSERT INTO users(${userKeys})
        VALUES (${createValueString(user)})
        RETURNING *;
    `,
      userValues
    );
    console.log(`USER CREATED`, rows);
    return rows;
  } catch (error) {
    throw error;
  } finally {
    client.release();
  }
}

async function createProduct(product) {
  const client = await pool.connect();
  const productKeys = Object.keys(product).join(",");
  const productValues = Object.values(product);
  try {
    const { rows } = await client.query(
      `
        INSERT INTO products(${productKeys})
        VALUES (${createValueString(product)})
        RETURNING *;
    `,
      productValues
    );
    console.log(`PRODUCT CREATED`, rows);
    return rows;
  } catch (error) {
    throw error;
  } finally {
    client.release();
  }
}

OR would it be better to create one dynamic function:

async function insertQuery(tableName, item){
    const client = await pool.connect();
    const itemKeys = Object.keys(item).join(",");
    const itemValues = Object.values(item);
    try {
      const { rows } = await client.query(
        `
          INSERT INTO ${tableName}(${itemKeys})
          VALUES (${createValueString(itemValues)})
          RETURNING *;
      `,
        itemValues
      );
      console.log(`ITEM CREATED`, rows);
      return rows;
    } catch (error) {
      throw error;
    } finally {
      client.release();
    }
}
  • Basically this is a question inviting opinion-based answers (I don't think there's an objective way to assess which is better, since it depends purely on your use-case and methodology); I would suggest that this *looks* like you're inserting variables into a statement, rather than using [parameterised queries](https://stackoverflow.com/a/4712113/82548), which itself is usually considered unsafe. I've linked to an answer that addresses PHP, but I think the same concerns are relevant to Node. – David Thomas Jan 28 '22 at 20:20

1 Answers1

1

You could potentially have both. Generally, you want code to be fairly clear on what code does what so insertQuery(); doesn't exactly shout this creates a user. Where as, createUser(); obviously does.

What you can have is one function such as executeQuery(sql, params); like your second idea with dynamic functions. As a result, don't have redundant/repeating code.

Simply pass in the SQL string and parameters. This works and makes more sense with stored procedures which is what I recommend you do.

const executeQuery = (sql, params) => {
// sql = "Call stored_procedure_name(parameter list)";
// params = [name, age, etc]
// Perform query code ..
// pool being a configured database connection object.
pool.query(sql, params);
}

Then, have the distinct functions you require re-use this.

const createUsers = () => {
  const sql = "Call createUserProcedure(params)";
  const params = [name, age, etc];
  const res = executeQuery(sql, params);
}

Mysql version:

const sql = "CALL CreateUser(?,?);";
const params = [name,age,etc];
const res = await executeQuery(sql, params);
Tony
  • 436
  • 1
  • 9
  • 17