2

I´ve dealing with Checkstyle for a while and know what Magic Numbers are, however I´ve never encountered it this way, its just a Switch for assigning data, but the cases are marked as magic numbers, and I cant make those final or anything like that (that I know). The program I created gets a random number (int from 0 to 199) and uses It in the switch, not only that but the 200 in the ra.nextInt(200); is also "magic".

My questions is how do i prevent the cases from beeing magic numbers without having to declare 200 new variables?

This is my code:

public Basura selectB(final Basura b) {

        private Random ra = new Random();

        ran = ra.nextInt(200);

        switch (ran) {
            case 0: b.setName("Funko Roto");
                b.setClasification("PL"); break;
            case 1: b.setName("Bateria de Laptop");
                b.setClasification("B"); break;
            case 2: b.setName("Sobras de Comida");
                b.setClasification("O"); break;
            case 3: b.setName("Clips de Oficina");
                b.setClasification("M"); break;
            case 4: b.setName("Colcha Deshilachada");
                b.setClasification("T"); break;
            case 5: b.setName("Disco DVD");
                b.setClasification("B"); break;

             ...

            default:b.setName("ERROR");
                b.setClasification("E"); break;
        }

        return b;
    }

2 Answers2

1

You could externalize this list of things into a text file, and have the program read the file contents into an array. You can put it at a hard-coded location in the code, or configure a location, or you can put it in the classpath (src/main/resources if you're using maven conventions) and read it with Class.getResourceAsStream.

Store the classification on the same line as the name, separated by a delimiter, like:

Sobras de Comida|O
Colcha Deshilachada|T

etc. The array length is the value that gets passed into nextInt.

The contents of the file get loaded into an array. Get rid of the switch and use the random value to index into the array to get your string. Then you can split the string and set the fields on the pojo you're returning.

With this approach:

  • There are no magic numbers.

  • It's easier to update the list.

  • You can delete a bunch of repetitive low-value code.

Nathan Hughes
  • 94,330
  • 19
  • 181
  • 276
0

I tried the answer sugested and it worked, heres the code I ended up with

public class DBAnalisis {


        private Random ra = new Random();

        private int ran = 0;

        private final int doscientos = 200;


    public Basura selectB(final Basura b) throws IOException {

        ran = ra.nextInt(doscientos);


        String linea;


        String[] basu = new String[2];


        BufferedReader dataBse = null;


        ArrayList<String> list = new ArrayList<String>();


        URL direcc = getClass().getResource("DB.txt");
        FileReader dB =
                new FileReader(direcc.getPath());

                try {

                    dataBse = new BufferedReader(dB);
                    // Lectura del fichero
                    while ((linea = dataBse.readLine()) != null) {
                        list.add(linea);
                    }
                } catch (Exception e) {
                    e.printStackTrace();
                }

                dB.close();

                String[] myArray = new String[list.size()];
                list.toArray(myArray);


                //basu = myArray[ran].split("|", 1); no funciona

                 int index = myArray[ran].indexOf("|");
                 basu[0] = myArray[ran].substring(0, index);
                 basu[1] = myArray[ran].substring(index + 1);
                 basu[0].trim();
                 basu[1].trim();

                 b.setName(basu[0]);
                 b.setClasification(basu[1]);

                return b;


    }
}

The DB.txt is the file that cointains each string that was in the switch Its divided like this: NAME|CLASS