Function that querys database and then renders a view

Question

Im new at backend development and I'm noticing that my code is very messy and difficult to read. I know this doesn't have a single correct answer but id like to know how I can make my code tidier when I have so many queries and const definitions. Thank you!

I have this piece of code from a function that retrieves data from db and then renders a view. Im trying not to use ORMs so I can learn SQL better.

show_consorcio_controllers.render_show_consorcio = async (req, res) => {
    const id_consorcio = req.params.id_consorcio;
    const user_id = req.session.passport.user;
    const consorcios = await connection.query("SELECT id FROM consorcios WHERE id_consorcio = ?", id_consorcio);
    const id_admin_del_consorcio = consorcios[FIRST_POSITION].id;

    if(id_admin_del_consorcio === user_id) {
        const consorcio_info = await connection.query("SELECT * FROM consorcios WHERE id_consorcio = ? and id = ?", [id_consorcio, user_id]);
        const consorcio = consorcio_info[FIRST_POSITION];
        const orden_ingresos_egresos = consorcio.order_egresos_ingresos_by;

        let fecha_de_muestra_final = moment(consorcio.fecha_de_muestra_final).format("YYYY/MM/DD");
        let fecha_de_muestra_inicial = moment(consorcio.fecha_de_muestra_inicial).format("YYYY/MM/DD");

        const user = await connection.query("SELECT nombre FROM usuarios WHERE id = ?", user_id);
        const {nombre} = user[FIRST_POSITION];

        const expensas = await connection.query("SELECT unidad_funcional, mes_periodo, anio_periodo, id_expensa, id_consorcio, titulo, monto_total, fecha FROM expensas WHERE id_consorcio = ? AND id_admin = ?", [id_consorcio, user_id]);
        const { ingresos, egresos } = await show_consorcio_controllers.get_ingresos_egresos(orden_ingresos_egresos, id_consorcio, fecha_de_muestra_inicial, fecha_de_muestra_final, user_id);
        const proveedores = await connection.query("SELECT id, nombre, apellido FROM proveedores WHERE id_admin = ?", user_id);

        show_consorcio_controllers.normalize_dates(ingresos);
        show_consorcio_controllers.normalize_dates(egresos);

        const unidades_funcionales = await connection.query("SELECT * FROM unidades_funcionales WHERE id_consorcio = ? AND id_admin = ?", [id_consorcio, user_id]);

        fecha_de_muestra_final = moment(consorcio.fecha_de_muestra_final).format("DD/MM/YYYY");
        fecha_de_muestra_inicial = moment(consorcio.fecha_de_muestra_inicial).format("DD/MM/YYYY");

       res.render("consorcio/show_consorcio", {expensas, unidades_funcionales, proveedores, id: user_id, nombre, id_consorcio, ingresos, egresos, orden_ingresos_egresos, fecha_de_muestra_inicial, fecha_de_muestra_final, consorcio});
    } else {
        res.send("No esta permitido el acceso a esta pagina");
    }
};

Answer 1

Test for invalid state up front

Eliminating branching simplifies. The high level reads in a more consistant general abstraction level.

if (! id_admin_del_consorcio === user_id) {
   res.send("No esta permitido el acceso a esta pagina");
   return;
}

// all that code ...

res.render("consorcio/show_consorcio", {expensas, unidades_funcionales, proveedores, id: user_id, nombre, id_consorcio, ingresos, egresos, orden_ingresos_egresos, fecha_de_muestra_inicial, fecha_de_muestra_final, consorcio});

---

Create Consorcios object

Very confusing: Currently there are two variables named "consorcios" but in different scopes. Then everything seems to have "consorcios" in the name. Make an object to manage that. Consorcios object properties should not have "consorcios" in their names.

• Pass res, req as parameters
• Make the queries into Object methods. Probably still need to pre-populate these.
  • Pre populate in the constructor. Necessary call order is guaranteed and all in one place. And the rest of the code will be cleaner.
  • pass method parameters so we do not need to read all that SQL.
• Make object methods for the variables.
  • fecha_de_muestra_final
  • fecha_de_muestra_inicial
  • {nombre} ... possibly does not need to be a formal Object
  • { ingresos, egresos } ... possibly does not need to be a formal Object
  • that inner scope const consorcio
  • ... and so on.
• Call the methods when they are used. Don't pre-assign a separate variable.