Hola Sniper, te indico las cosas a mejorar (que hay bastantes) en este código:
Clase CantanteFamoso
public void setNombre() { nombre = entrada(); }
La idea está bien y demuestra que tienes cierta soltura en el manejo de código. Sin embargo, aunque como idea pueda entenderse, desde el punto de vista de la programación no es algo adecuado. Oirás a muchos programadores hablar del principio KIS ó Keep it Simple. Otros lo alargar como principio KISS ó Keep it Simple Stupid (esto es más grosero). El asunto es que el código debe mantenerse simple.
El código que has planteado es un método que en realidad no hace otra cosa que llamar a otro método. Hay varias cosas que no son adecuadas en el planteamiento hecho: por un lado, ¿para qué tener un método que únicamente llama a otro? ¿No sería más lógico tener el código directamente sin tener que estar llamándolo?
Por otro lado contraviene otro principio básico de la programación: no inventar de nuevo la rueda. Este principio lo que dice es que hay muchas cosas que los programadores usan y alcanzan el nivel de "convención" entre los programadores, es decir, que todos los programadores trabajan igual porque es algo digamos que todo el mundo acepta.
Un método set, de acuerdo con el convenio tácito entre todos los programadores, es un método que recibe un parámetro para establecer el valor de un atributo. En el método que has planteado no se recibe un parámetro, por tanto va contra el convenio de la comunidad de programadores. No significa que esté mal, pero sí que estás yendo en contra de unos principios básicos de la programación que darían lugar a que cualquier programador que mirara tu código no lo valorara positivamente, es decir, transmitirías que no te atienes al convenio y eso genera rechazo. Tu jefe, o tu profesor, o quien fuera, te dirían... que no, que está bien tratar de innovar cuando se pueda, pero que eso lo corrijas.
En este otro método:
public void setDiscoConMasVentas()
{
discoConMasVentas = entrada();
}
Tenemos una repetición (también llama al mismo método). En algunas ocasiones, por ser necesario, se repite código. Pero en general también un buen programador mirará con mala cara la repetición de código.
Luego tenemos este método:
public String entrada()
{
Scanner escaner = new Scanner(System.in);
return escaner.nextLine();
}
Tampoco encaja bien en la filosofía de la orientación a objetos. En POO cada objeto tiene sus responsabilidades delimitadas. ¿Es lógico que un objeto CantanteFamoso tenga un método que sirva para pedir, de forma genérica, una entrada de teclado? Más bien no, un objeto CantanteFamoso debe limitarse a almacenar la información de un cantante famoso y a tener métodos para gestionar y hacer cosas con dicha información. Pero entre sus responsabilidades no estará, normalmente, el pedir datos al usuario. Esta responsabilidad se debería delegar en otra clase, que podría denominarse EntradaTeclado ó GestorEntradasUsuario ó ... que sería un objeto cuyas responsabilidades, de forma lógica, incluirían el que pida una entrada de teclado.
En programación orientada a objetos tenemos objetos, cada uno con sus tareas... el programa se construye creando objetos y haciendo que interaccionen entre ellos.
Clase ListaCantantesFamosos
Tenemos el constructor:
public ListaCantantesFamosos(String nombre, String discoConMasVentas)
{
listaDeCantantesF = new ArrayList<CantanteFamoso>();
addObjetoCantante(nombre, discoConMasVentas);
}
Si esto lo mira un programador no entenderá lo que tratas de hacer, o pensará que estás tratando de inventar la rueda. Un constructor sirve para inicializar los atributos. En este caso la tarea la cumple la primera línea. Si tuviéramos un constructor que recibe parámetros, el parámetro a recibir debería ser un ArrayList, precisamente porque ese es el atributo de la clase. No se atiene a la convención que en esta clase se reciban los datos de un elemento de un ArrayList y se añadan al mismo. Esto no es el objetivo de un constructor. Aquí estás mezclando ideas de un método con ideas de un constructor. La solución es hacer las cosas lo más simples posibles.
public ListaCantantesFamosos()
{
listaDeCantantesF = new ArrayList<CantanteFamoso>();
}
Con esto resuelto: más sencillo, tenemos que escribir menos, más elegante, y nos atenemos a la convención de la comunidad de programadores.
Aquí hay otra cosa que en cierta manera llama la atención:
int i = 0;
for(CantanteFamoso datos : listaDeCantantesF)
{
System.out.println(i + ": " + datos.getNombre() + " - " + datos.getDiscoConMasVentas()); i++;
}
Un for extendido es un bucle que no usa de índice para su recorrido. Sin embargo estás introduciendo el índice i. De esta manera se mezcla un bucle que no usa índice con un índice: extraño, no es que esté mal, pero llama la atención.
Si queremos usar un índice, lo lógico es usar un bucle con índice, algo así como:
for (int i=0; i<listaDeCantantesF.size(); i++) {
System.out.println(i + ": " + listaDeCantantesF.obtenerObjetoEnPosicion(i).getNombre() + " - " + listaDeCantantesF.obtenerObjetoEnPosicion(i).getDiscoConMasVentas());
}
Aquí otra cosilla a comentar:
CantanteFamoso objeto;
objeto = listaDeCantantesF.get(posicion);
objeto.setNombre();
Podemos escribir:
listaDeCantantesF.get(posicion).setNombre();
Es decir, condensar tres líneas en una.
La clase con el método main no la he revisado, simplemente decirte que debes acortarla y repensarla. El método:
private static String entradaNombre()//Crea una entrada personalizada para el campo nombre de un objeto tipo Cantante
{
System.out.println("Nombre: ");
return entrada();
}
¿Es un método para manipular el campo nombre de un objeto Cantante? No debe estar ahí ¿Por qué? Porque la responsabilidad de los campos es de las clases, por tanto la clase con el método main no tiene por qué tener un método para hacer algo que es responsabilidad de un objeto Cantante.
Veo más métodos con el mismo problema ¿están en la clase donde está la responsabilidad?
Ha sido un poco extenso, espero que al menos ayude a sacar algo positivo. Saludos
