Sobre buenas prácticas de programación

[zonahurbana]

En C++ no llegué a usar mucho clases y objetos, más que todo usaba funciones que se llamaban entre sí o que eran recursivas.

En Java, digamos que ya he usado la POO como tal. Entonces, en muchas ocasiones pude reutilizar código sin tantas complicaciones, a través de herencia y polimorfismo. Incluso creo que comencé a utilizar mejor los métodos, y a crearlos siempre que fuese conveniente.

Ahora me encuentro con una situación un tanto particular, en la que me he detenido por pensar qué es lo mejor que puede hacerse.

Tengo un método agregarProducto en una clase que gestiona un listado del mismo.
Aquí se tienen las restricciones para los atributos ingresados. Si hubo error, el producto no se agrega al listado y se devuelve una advertencia como String. Si no hubo error, el producto es agregado y se devuelve null.

Ahora he querido agregar un método modificarProducto. A éste se le pasa la posición del producto a modificar (existe un método que devuelve la posición de un producto buscándolo por su código) y los atributos que deben asignarse.

Creo que lo más correcto es copiar el código de agregarProducto pero cambiar las líneas en las que se agrega... es decir, cambiar el add por un set (el listado es un ArrayList). Así, modificar también devuelve una advertencia, y si no la hay, devuelve null.

Pero me pareció extraño copiar el código ya que no lo hacía por un buen tiempo, y pensé en lo siguiente:

1. Podría hacer que agregarProducto llame a modificarProducto y que en vez de pasarle una posición intermedia, le pase la última posición+1. Pero podría darse el caso de que no se haya reservado memoria para tal posición aún.

2. Podría hacer lo anterior pero en el método modificarProducto evaluar si la posición es mayor a la longitud actual del ArrayList. De ser así agregar (usar add), sino, modificar (usar set).

3. Que agregarProducto se mantenga tal cual pero modificarProducto llame a agregarProducto. En caso de que se haya agregado, pasar ese objeto a la posición que se quiere modificar y luego eliminar la última posición. Así modificar ya no tiene el mismo código (no tendría que copiarlo) y se modificaría la posición deseada con datos correctos. Lo malo es que estoy haciendo operaciones adicionales.

A fin de cuentas, es mejor a veces copiar el código que tratar de no hacerlo y complicarse en vano, ¿verdad?
Además, más líneas de código pueden estar más optimizadas que otras muy pocas.

A veces me pasa esto, que me lío en vano, pudiendo simplemente avanzar con el programa y no detenerme a pensarlo tanto.

Edit: Muchas gracias por su interés, espero que puedan ayudarme un poco comentando cómo creen que es conveniente hacerlo.

[1mpuls0]

Hola.

De poder si puedes, pero yo no te lo recomendaría, no sería una buena práctica.

Por ejemplo los siguientes métodos.

existeProducto(String producto) -> retorna verdadero o falso si un producto existe en la lista
obtenerIdProducto(String producto) -> retorna la clave del producto
agregarProducto(String categoria, String producto) -> agrega el producto a la lista.

Es decir las funciones deben estar creadas para su propósito puntual.

A fin de cuentas, es mejor a veces copiar el código que tratar de no hacerlo y complicarse en vano, ¿verdad?
Además, más líneas de código pueden estar más optimizadas que otras muy pocas.

Tu lo haz dicho!

A veces por querer ahorrarnos algunas líneas de código (simplemente por hacerlo más pequeño) tratando de implementar métodos o funciones dentro de otras resulta más complicado.

No sé que alcance tenga tu proyecto, pero y si en un futuro necesita modificaciones en esos módulos?, posiblemente sea más complicado realizarlo como lo haz pensado que si estuviese separado.

Saludos.

[zonahurbana]

Gracias por contestar.

En realidad es una aplicación un tanto extensa pero solo la estoy realizando a modo de práctica, es decir, no va a ser usada... pero desde ya quería escribir un código bien estructurado.

Los métodos serían:

Código (java) [Seleccionar] Expandir

    public String agregarEmpleado(String apellidos, String nombres, String DNI, String sueldoCadena, String password, boolean tipoTrabajador) {
       
        // Borramos espacios al inicio y al final con trim()
        apellidos = apellidos.trim();
        nombres = nombres.trim();
        DNI = DNI.trim();
       
        if(apellidos.isEmpty() || nombres.isEmpty() || DNI.isEmpty() || password.isEmpty())
            return "Se han encontrados datos vacíos o que sólo contienen espacios.";
       
        if(password.length()<6)
            return "La contraseña debe tener al menos 6 caracteres.";
       
        if(DNI.length() != 8)
            return "El DNI debe tener 8 dígitos.";
       
        if(buscarPorDNI(DNI) != -1)
            return "Ya existe un empleado registrado con este DNI.";
       
        Double sueldo;
        try{
            sueldo = Double.parseDouble(sueldoCadena);
        }catch(NumberFormatException e){
            return "El saldo ingresado no se corresponde con un valor numérico.";
        }
       
        empleados.add( new Empleado(apellidos, nombres, DNI, sueldo, password, tipoTrabajador) );
        return null; // Si devuelve null es porque no hubo error.
    }
   
    public String modificarEmpleado(int posicion, String apellidos, String nombres, String DNI, String sueldoCadena, String password, boolean tipoTrabajador) {
       
        // Borramos espacios al inicio y al final con trim()
        apellidos = apellidos.trim();
        nombres = nombres.trim();
        DNI = DNI.trim();
       
        if(apellidos.isEmpty() || nombres.isEmpty() || DNI.isEmpty() || password.isEmpty())
            return "Se han encontrados datos vacíos o que sólo contienen espacios.";
       
        if(password.length()<6)
            return "La contraseña debe tener al menos 6 caracteres.";
       
        if(DNI.length() != 8)
            return "El DNI debe tener 8 dígitos.";
       
        if(buscarPorDNI(DNI) != -1)
            return "Ya existe un empleado registrado con este DNI.";
       
        Double sueldo;
        try{
            sueldo = Double.parseDouble(sueldoCadena);
        }catch(NumberFormatException e){
            return "El saldo ingresado no se corresponde con un valor numérico.";
        }
       
        empleados.set( posicion, new Empleado(apellidos, nombres, DNI, sueldo, password, tipoTrabajador) );
        return null; // Si devuelve null es porque no hubo error.
    }

Cuando se agregue, los datos serán enviados desde un formulario.
Cuando se modifique, será desde una tabla y aquí puede haber un ligero problema.

Lo malo es que, si el administrador modifica un empleado en la tabla (y le modifica todos sus atributos) yo no tendría como identificar a ese empleado en todo mi ArrayList, ya que la tabla no muestra todos los empleados, sólo los que son resultados de una búsqueda (haciendo imposible que coincida la posición del empleado en la lista con su posición en el ArrayList). Creo que tendré que poner que el código no sea modificable, así tendría cómo ubicarlo siempre.

[sapito169]

Waoo ese estilo de programación es muy parecido a model driven desing (Evans)

Tengo un método agregarProducto en una clase que gestiona un listado del mismo.
Aquí se tienen las restricciones para los atributos ingresados. Si hubo error, el producto no se agrega al listado y se devuelve una advertencia como String. Si no hubo error, el producto es agregado y se devuelve null.

Es mejor cadenas vacías sobre nulls. Esto es debido a que algún programador despistado se olvide manejar los nulls y provoque que la aplicación muera de sorpresa miserablemente

Lo malo es que, si el administrador modifica un empleado en la tabla (y le modifica todos sus atributos) yo no tendría como identificar a ese empleado en todo mi ArrayList, ya que la tabla no muestra todos los empleados, sólo los que son resultados de una búsqueda (haciendo imposible que coincida la posición del empleado en la lista con su posición en el ArrayList). Creo que tendré que poner que el código no sea modificable, así tendría cómo ubicarlo siempre.

Definitivamente La segunda opción es mejor

Código (java) [Seleccionar] Expandir


import java.util.ArrayList;
import java.util.List;

public class ServicioEmpleado {
private List<Empleado> empleados = new ArrayList<>();

public String agregarEmpleado(String apellidos, String nombres, String DNI,
String sueldoCadena, String password, boolean tipoTrabajador) {

// Borramos espacios al inicio y al final con trim()
apellidos = apellidos.trim();
nombres = nombres.trim();
DNI = DNI.trim();

if (apellidos.isEmpty() || nombres.isEmpty() || DNI.isEmpty()
|| password.isEmpty())
return "Se han encontrados datos vacíos o que sólo contienen espacios.";

if (password.length() < 6)
return "La contraseña debe tener al menos 6 caracteres.";

if (DNI.length() != 8)
return "El DNI debe tener 8 dígitos.";

if (buscarPorDNI(DNI) != -1)
return "Ya existe un empleado registrado con este DNI.";

Double sueldo;
try {
sueldo = Double.parseDouble(sueldoCadena);
} catch (NumberFormatException e) {
return "El saldo ingresado no se corresponde con un valor numérico.";
}
long id = empleados.size() + 1;
empleados.add(new Empleado(id, apellidos, nombres, DNI, sueldo,
password, tipoTrabajador));
return ""; // Si devuelve null es porque no hubo error.
}

private int buscarPorDNI(String dNI) {
return 0;
}

public String modificarEmpleado(int posicion, String apellidos,
String nombres, String DNI, String sueldoCadena, String password,
boolean tipoTrabajador) {

apellidos = apellidos.trim();
nombres = nombres.trim();
DNI = DNI.trim();

if (apellidos.isEmpty() || nombres.isEmpty() || DNI.isEmpty()
|| password.isEmpty())
return "Se han encontrados datos vacíos o que sólo contienen espacios.";

if (password.length() < 6)
return "La contraseña debe tener al menos 6 caracteres.";

if (DNI.length() != 8)
return "El DNI debe tener 8 dígitos.";

if (buscarPorDNI(DNI) != -1)
return "Ya existe un empleado registrado con este DNI.";

Double sueldo;
try {
sueldo = Double.parseDouble(sueldoCadena);
} catch (NumberFormatException e) {
return "El saldo ingresado no se corresponde con un valor numérico.";
}

empleados.set(posicion, new Empleado(posicion, apellidos, nombres, DNI,
sueldo, password, tipoTrabajador));
return ""; // Si devuelve null es porque no hubo error.
}
}