Asignación "Magic Squares" -- java campo con beginner camp codereview Relacionados El problema

“Magic Squares” assignment


9
vote

problema

Español

La asignación es tomar un archivo .txt" mágico cuadrado ".txt (donde las líneas horizontales, las líneas verticales y la línea diagonal se suman al mismo número), leíjalo y pruebe que ellos son de hecho cuadrados mágicos.

¿Es algo en esta respuesta mala práctica o simplemente mal? Estoy usando un montón de List S y for Loops.

  package magicSquares;  public  MagicSquaresCormac() {     return; }  private List<List<Integer>> getNumberList(String file) throws IOException {     FileReader fr = new FileReader(file);     BufferedReader br = new BufferedReader(fr);      String line = null;     Integer num = null;     List<String> lineList = new ArrayList<>();     List<String> charList;     List<List<String>> listOfNums = new ArrayList<>();     List<Integer> numlist;     List<List<Integer>> numListList = new ArrayList<>();      while ((line = br.readLine()) != null) {         line.split(" ");         if (!line.equals("")) {             lineList.add(line);         }     }      for (String lineFromList : lineList) {         //    lineFromList.split(" ");         numlist = new ArrayList<>();         charList = Arrays.asList(lineFromList.split("\s"));         listOfNums.add(charList);         for (String character : charList) {             num = Integer.valueOf(character);             numlist.add(num);         }         numListList.add(numlist);     }      br.close();     return numListList; }  private boolean horizontalLine(String file) throws IOException {     List<List<Integer>> numListList = getNumberList(file);     boolean isMagic = true;     Integer lastSum = -1;     Integer sum;      for (List<Integer> listNum : numListList) {         sum = 0;         for (Integer i : listNum) {             sum = sum + i;         }         if (lastSum == -1)             lastSum = sum;          else if (!lastSum.equals(sum)) {             isMagic = false;         }     }      return isMagic; }  private boolean diagonalLine(String file) throws IOException {     List<List<Integer>> numListList = getNumberList(file);     boolean isMagic = true;     Integer sum;     Integer sumDiagonal = 0;      for (int j = 0; j < numListList.size(); j++) {         sumDiagonal = sumDiagonal + numListList.get(j).get(j);     }      sum = sumDiagonal;      if (!sum.equals(260)) {         isMagic = false;     }     return isMagic; }  private boolean verticalLine(String file) throws IOException {     List<List<Integer>> numListList = getNumberList(file);     boolean isMagic = true;     Integer lastSum = -1;     Integer sum;     List<Integer> tempList;     List<List<Integer>> verticalLists = new ArrayList<>();      for (int k = 0; k < numListList.size(); k++) {         tempList = new ArrayList<>();         for (int i = 0; i < numListList.size(); i++) {             tempList.add(numListList.get(i).get(k));         }         verticalLists.add(tempList);     }      for (List<Integer> listNum : verticalLists) {         sum = 0;         for (Integer i : listNum) {             sum = sum + i;         }         if (lastSum == -1)             lastSum = sum;          else if (!lastSum.equals(sum)) {            System.out.println((lastSum != sum) + "really?");             isMagic = false;         }     }     return isMagic; }  public static void main(String[] args) throws IOException {    MagicSquaresCormac magicSquaresCormac =  new MagicSquaresCormac();     String[] files = {"Mercury.txt", "Luna.txt"};     for (String file : files) {         System.out.println(file + " (horizontal lines) is magic? " + magicSquaresCormac.horizontalLine(file));         System.out.println(file + " (vertical lines) is magic? " + magicSquaresCormac.verticalLine(file));         System.out.println(file + " (diagonal line) is magic? " + magicSquaresCormac.horizontalLine(file));     } } }   
Original en ingles

The assignment is to take a "Magic Square" .txt file (where the horizontal lines, vertical lines and diagonal line all add up to the same number), read it and prove that they are in fact magic squares.

Is anything in this answer bad practice or just bad? I'm using a lot of Lists and for loops.

package magicSquares;  public  MagicSquaresCormac() {     return; }  private List<List<Integer>> getNumberList(String file) throws IOException {     FileReader fr = new FileReader(file);     BufferedReader br = new BufferedReader(fr);      String line = null;     Integer num = null;     List<String> lineList = new ArrayList<>();     List<String> charList;     List<List<String>> listOfNums = new ArrayList<>();     List<Integer> numlist;     List<List<Integer>> numListList = new ArrayList<>();      while ((line = br.readLine()) != null) {         line.split("\t");         if (!line.equals("")) {             lineList.add(line);         }     }      for (String lineFromList : lineList) {         //    lineFromList.split(" ");         numlist = new ArrayList<>();         charList = Arrays.asList(lineFromList.split("\\s"));         listOfNums.add(charList);         for (String character : charList) {             num = Integer.valueOf(character);             numlist.add(num);         }         numListList.add(numlist);     }      br.close();     return numListList; }  private boolean horizontalLine(String file) throws IOException {     List<List<Integer>> numListList = getNumberList(file);     boolean isMagic = true;     Integer lastSum = -1;     Integer sum;      for (List<Integer> listNum : numListList) {         sum = 0;         for (Integer i : listNum) {             sum = sum + i;         }         if (lastSum == -1)             lastSum = sum;          else if (!lastSum.equals(sum)) {             isMagic = false;         }     }      return isMagic; }  private boolean diagonalLine(String file) throws IOException {     List<List<Integer>> numListList = getNumberList(file);     boolean isMagic = true;     Integer sum;     Integer sumDiagonal = 0;      for (int j = 0; j < numListList.size(); j++) {         sumDiagonal = sumDiagonal + numListList.get(j).get(j);     }      sum = sumDiagonal;      if (!sum.equals(260)) {         isMagic = false;     }     return isMagic; }  private boolean verticalLine(String file) throws IOException {     List<List<Integer>> numListList = getNumberList(file);     boolean isMagic = true;     Integer lastSum = -1;     Integer sum;     List<Integer> tempList;     List<List<Integer>> verticalLists = new ArrayList<>();      for (int k = 0; k < numListList.size(); k++) {         tempList = new ArrayList<>();         for (int i = 0; i < numListList.size(); i++) {             tempList.add(numListList.get(i).get(k));         }         verticalLists.add(tempList);     }      for (List<Integer> listNum : verticalLists) {         sum = 0;         for (Integer i : listNum) {             sum = sum + i;         }         if (lastSum == -1)             lastSum = sum;          else if (!lastSum.equals(sum)) {            System.out.println((lastSum != sum) + "really?");             isMagic = false;         }     }     return isMagic; }  public static void main(String[] args) throws IOException {    MagicSquaresCormac magicSquaresCormac =  new MagicSquaresCormac();     String[] files = {"Mercury.txt", "Luna.txt"};     for (String file : files) {         System.out.println(file + " (horizontal lines) is magic? " + magicSquaresCormac.horizontalLine(file));         System.out.println(file + " (vertical lines) is magic? " + magicSquaresCormac.verticalLine(file));         System.out.println(file + " (diagonal line) is magic? " + magicSquaresCormac.horizontalLine(file));     } } } 
     
     
     

Lista de respuestas

6
 
vote

Sospecho que debe leer las matrices de archivos solo una vez y almacenarlos en una matriz entera bidimensional. Una vez que tengas eso, es trivial verificar si los cuadrados son mágicos.

Tenía en mente algo como esto:

  private static int[][] loadFile(File file) {      // Do file I/O for loading the square      return square; }  private static int getRowMagic(int[][] square, int row) {     int sum = 0;      for (int column = 0; column < square.length; ++column) {         sum += square[row][column];     }      return sum; }  private static int getColumnMagic(int[][] square, int column) {     int sum = 0;      for (int row = 0; row < square.length; ++row) {         sum += square[row][column];     }      return sum; }  private static int getDescendingDiagonalMagic(int[][] square) {     int sum = 0;       for (int i = 0; i < square.length; ++i) {         sum += square[i][i];     }      return sum; }  private static getAscendingDiagonalIsMagic(int[][] square) {     int sum = 0;      for (int i = 0; i < square.length; ++i) {         sum += square[square.length - 1 - i][i];     }      return sum; }  public static boolean isMagicSquare(int[][] square) {     int sum = getRowMagic(square, 0);      for (int i = 0; i < square.length; ++i) {         if (sum != getRowMagic(square, i) || sum != getColumnMagic(square, i)) {             return false;         }     }      return getAscendingDiagonalMagic(square) == sum &&            getDescendingDiagonalMagic(square) == sum; }   

espero que ayude.

 

I suspect that you should read the matrices from files only once and store them in a two-dimensional integer array. Once you have that, it is trivial to verify whether the squares are magic.

I had in mind something like this:

private static int[][] loadFile(File file) {      // Do file I/O for loading the square      return square; }  private static int getRowMagic(int[][] square, int row) {     int sum = 0;      for (int column = 0; column < square.length; ++column) {         sum += square[row][column];     }      return sum; }  private static int getColumnMagic(int[][] square, int column) {     int sum = 0;      for (int row = 0; row < square.length; ++row) {         sum += square[row][column];     }      return sum; }  private static int getDescendingDiagonalMagic(int[][] square) {     int sum = 0;       for (int i = 0; i < square.length; ++i) {         sum += square[i][i];     }      return sum; }  private static getAscendingDiagonalIsMagic(int[][] square) {     int sum = 0;      for (int i = 0; i < square.length; ++i) {         sum += square[square.length - 1 - i][i];     }      return sum; }  public static boolean isMagicSquare(int[][] square) {     int sum = getRowMagic(square, 0);      for (int i = 0; i < square.length; ++i) {         if (sum != getRowMagic(square, i) || sum != getColumnMagic(square, i)) {             return false;         }     }      return getAscendingDiagonalMagic(square) == sum &&            getDescendingDiagonalMagic(square) == sum; } 

Hope that helps.

 
 
3
 
vote

Constructor inútil

El Declaración return1 es innecesario como la última declaración en un constructor. En realidad, no necesita el constructor 9988776665544332 , puede eliminarlo de forma segura.

Asignaciones innecesarias

Estas asignaciones son innecesarias:

  String line = null; Integer num = null;   

declarar las cosas tan tarde como sea posible

Hablando aproximadamente, el Tiempo en vivo de una variable es el número de líneas entre la Declaración y el último uso. Para minimizar los malos accidentales (ventana de vulnerabilidad), es mejor declarar las variables lo más cerca posible de sus usos.

line debe declararse justo por encima del bucle while . Y sin asignar un valor a él, ya que siempre recibirá un valor de todos modos.

num65544336 debe declararse dentro del for bucle después de que se usa, y no necesita ser un 99887776655544338 , un simple int lo hará:

  return0  

Lo mismo ocurre con return1 , puede declararlo donde lo asignan:

  return2  

y por lo tanto para return3 también:

  return4  

Otros elementos innecesarios

Usted crea return5 y coloque valores en él, pero luego nunca se usa.

El return6 no es absolutamente nada.

Utilice el intento con recursos

En lugar de cerrar manualmente los recursos, es mejor usar intente-con- recursos .

Poniéndolo juntos

Aplicando las sugerencias anteriores, return7 se convierte en:

  return8  

Ir a más ...

Puede aplicar técnicas de refactorización similares en sus otros métodos. Dejo eso como un ejercicio para ti. Solo unas pocas sugerencias para return9 :

  • Reemplazar MagicSquaresCormac0 Con MagicSquaresCormac1 Cuando sea posible
  • Puede eliminar por completo MagicSquaresCormac2 al regresar directamente
  • MagicSquaresCormac3 se puede declarar dentro del MagicSquaresCormac4 Loop
 

Pointless constructor

The return statement is unnecessary as the last statement in a constructor. Actually you don't need the MagicSquaresCormac constructor at all, you can safely remove it.

Unnecessary assignments

These assignments are unnecessary:

String line = null; Integer num = null; 

Declare things as late as possible

Roughly speaking, the live time of a variable is the number of lines between declaration and last use. To minimize accidental misuses (window of vulnerability), it's best to declare variables as close to their uses as possible.

line should be declared just above the while loop. And without assigning a value to it, as it will always receive a value anyway.

num should be declared inside the for loop later where it's used, and it doesn't need to be an Integer, a simple int will do:

int num = Integer.valueOf(character); 

The same goes for charList, you can declare it where you assign it:

List<String> charList = Arrays.asList(lineFromList.split("\\s")); 

And so for numlist too:

List<Integer> numlist = new ArrayList<>(); 

Other unnecessary elements

You create listOfNums and put values in it, but then it's never used.

The line.split("\t"); does absolutely nothing.

Use try-with-resources

Instead of manually closing resources, it's better to use try-with-resources.

Putting it together

Applying the above suggestions, getNumberList becomes:

private List<List<Integer>> getNumberList(String file) throws IOException {     List<String> lineList = new ArrayList<>();      try (             FileReader fr = new FileReader(file);             BufferedReader br = new BufferedReader(fr);     ) {         String line;         while ((line = br.readLine()) != null) {             if (!line.equals("")) {                 lineList.add(line);             }         }     }      List<List<Integer>> numListList = new ArrayList<>();      for (String lineFromList : lineList) {         List<Integer> numlist = new ArrayList<>();         List<String> charList = Arrays.asList(lineFromList.split("\\s"));         for (String character : charList) {             int num = Integer.valueOf(character);             numlist.add(num);         }         numListList.add(numlist);     }      return numListList; } 

Going further...

You can apply similar refactoring techniques in your other methods. I leave that as an exercise for you. Just a few hints for horizontalLine:

  • Replace Integer with int when possible
  • You can completely eliminate isMagic by returning directly
  • sum can be declared inside the for loop
 
 
1
 
vote

También puede intentar usar una declaración MagicSquaresCormac5 para asegurarse de que su búquera, siempre esté cerrado, incluso en caso de una excepción.

 

You might also try using a try-with-resources statement to ensure your BufferedReader is always closed, even in the event of an Exception.

 
 

Relacionados problema

5  Colector de archivos M3U  ( M3u file collector ) 
Soy nuevo en Python y escribió este código para recopilar todos los archivos en un archivo M3U (Lista de reproducción) y copiándolos en un directorio. impo...

6  Palindrome más largo en una matriz  ( Longest palindrome in an array ) 
Soy nuevo en la programación, y creo que este código podría mejorarse. ¿Alguna sugerencia? 'done'0 ...

12  Autenticación simple en ASP.NET MVC 5  ( Simple authentication in asp net mvc 5 ) 
Estoy construyendo una aplicación ASP.NET MVC 5 y, por razones que son irrelevantes en este punto, estoy intentando construir mi propio medio para autenticar ...

1  Actualización de una página web de DRUPAL con contenido basado en texto ingresado  ( Updating a drupal web page with content based on inputted text ) 
La intención es actualizar una página web de drupal existente con contenido basado en texto ingresado en un cuadro de texto. La página existente muestra dat...

2  Pequeño bot de Twitter hecho en Python  ( Small twitter bot made in python ) 
Acabo de empezar a jugar con Python y he decidido crear un pequeño script que publique Tweets cada hora (Cronjob en Ubuntu) diciendo qué hora es. ¿No estoy ...

3  Aplanar para obtener todos los controles infantiles de cierto tipo en una UIView  ( Flatten to get all child controls of certain type in a uiview ) 
extension Array { func flatten<TResult>(transform: (T) -> TResult?, childArray: (T) -> [T]) -> [TResult] { var result = [TResult]() for ...

2  Formulario básico de comentarios de PHP  ( Basic php comment form ) 
Soy un novato de programación. He escrito este simple script PHP para ejecutar un formulario de comentarios muy básico y apreciaría cualquier comentario, espe...

1  ¿Cómo hacer que esta clase de lector de TLV se vea mejor y funciona más suave?  ( How to make this tlv reader class look nicer and work smoother ) 
Soy bastante nuevo para Java y estaba tratando de mejorar un poco mis habilidades haciendo un lector de archivos binarios. El elegido fue el tlv formato. He...

1  Script por lotes para descargar y extraer nodos.js y paquetes NPM  ( Batch script to download and extract node js and npm packages ) 
El siguiente es mi primer intento real de Scripting por lotes, y lo soy nuevo, así que no estoy seguro de si lo que he hecho es practicar mejor. El código fun...

2  Devuelve verdadero si los elementos de una matriz no contienen uno u otro  ( Return true if the elements of an array do not contain one or the other ) 
Estoy completando gradualmente los ejercicios de codificación para Java. Aquí está el uno acabo de hacer: Dada una matriz de INTS, devuelva verdadera si ...




© 2022 respuesta.top Reservados todos los derechos. Centro de preguntas y respuestas reservados todos los derechos