i'm working on a school project, we're building an Electronic Grade book, it's just a project to learn better OOP and UML nothing of real. And we got blocked in a strange bug we worked for a lot of time trying to solve it, but we couldn't find a solution for it. The function that read the students names, date, class, and so on. Has a strange bug when we debug it, it read all the info and save to the class array without any problem but when we run it, some students are not added to the class although the output seems to be non-deterministic. I did find a way to overcome this bug by adding a thread sleep of 1 ms in the function and it worked. But we have no idea why and what's the problem, it just makes no sense. We worked for like 10 hours searching for a reason and a real solution.
This is the function:
/**
* Import the studend list from a text file and load them into an array
* @param nomeFile file name of the studend
* @param classi array name of all the classes
* @param nclassi number of classes
* @return number of student imported
* @throws FileNotFoundException, IOException problemi con la lettura del file
* IndexOutOfBoundsException problemi di inserimento dell'alunno nella classe specificata
* Exception problemi con la data di nasciita dello studente
*/
public static int importaIscrizioniAlunni(String nomeFile, Classe[] classi, int nclassi) throws FileNotFoundException,IOException,IndexOutOfBoundsException,Exception {
BufferedReader br = null;
String riga = "";
int iscrizioni=0;
// Carico le iscrizioni effettuate dagli alunni dal file
try
{
br = new BufferedReader(new FileReader(nomeFile));
//PROBLEM STARTS
while ((riga = br.readLine()) != null)
{
String[] campi = riga.split(",");
// campi[1] => cognome
// campi[2] => nome
// campi[3] => sesso
// data nascita: campi[4] => giorno campi[5] => mese campi[6] => anno
// campi[7] => classe campi[8] => sezione
Studente nuovo = new Studente(campi[0], campi[1], campi[2], campi[3].charAt(0), Integer.valueOf(campi[4]), Integer.valueOf(campi[5]), Integer.valueOf(campi[6]));
Classe classe_scelta = new Classe(Integer.parseInt(campi[7]), campi[8].charAt(0), null);
//print the new student and new class using toString just for debug purposes
System.out.println("input: " nuovo .toString() " " classe_scelta.toString());
for(int i = 0; i < nclassi; i )
{
//trova la classe
if(classi[i].equals(classe_scelta))
{
try
{
classi[i].aggiungi(nuovo);
//print the new student and new class just for debug purposes
System.out.println("Iscrizione effettuata: " nuovo.getCognome() " " nuovo.getNome() " " classi[i].getSezione() " " classi[i].getPercorso().getannoCorso() "\n\n\n");
break;
}
catch(IndexOutOfBoundsException e)
{
throw new IndexOutOfBoundsException();
}
}
}
iscrizioni ;
//Thread.sleep(1); Solve the bug but it's not the best solution we couldn't find what is causing the bug
//print the class using toString just for debug purposes
for(int i = 0; i < nclassi; i )
{
System.out.println(classi[i]);
}
}
//PROBLEM ENDS
}
catch (FileNotFoundException e)
{
throw new FileNotFoundException();
} catch (IOException e) {
throw new IOException("Errore di accesso al file");
} catch (Exception e) {
throw new Exception("Errore generico durante importazione alunni (probabile data non valida nel file)");
}finally {
if (br != null) {
try {
br.close();
} catch (IOException e) {
throw new IOException("Errore durante chiusura del file");
}
}
}
return iscrizioni;
}
I'm sorry if the variables are in Italian but due to the fact of being a team school project, we had to adapt for everyone, as you can see this method read the student find his class and add the student to it.
The "aggiungi" method which means add:
public void aggiungi(Studente nuovo) throws IllegalArgumentException
{
if (this.numeroStudenti 1 <= MAX_STUDENTI)
{
this.elenco.add(nuovo);
nuovo.classe = this;
nuovo.assegnaMatricola();
numeroStudenti ;
}
else
throw new IllegalArgumentException();
}
We add the student to the internal container which is a Set, and we then set the class of the student to be this one, then we assign the student ID and finally we increase the student number.
Any help is very appreciated, we're getting mad because the code is working when we debug it, and if you need more information on the code or even the full repository just ask for it. Thanks in advice.
We tried do debug it several time and it works, we tried to use several output for debugging purpose, we did try the single add function, we checked if the problem was created by reading the file and it doesn't seems to be, we also saw that if we put the break point while debugging after the method call the problem still persist and everything is working as it should.
This is an example of the output in run mode:
This is the text file of the students:
RSSMRA00R05A519L,Rossi,Mario,m,10,5,2000,3,A
VRDLGU99A01E423D,Verdi,Luigi,m,1,1,1999,3,A
BNCPLA00S63D848E,Bianchi,Paola,f,23,11,2000,3,A
CRLNRE99D45E458M,Neri,Carlo,m,5,4,1999,4,A
GLIRTT99D45D852Z,Rossetto,Giulia,f,2,2,1998,4,A
RSORSO99D45D852K,Rosa,Rosa,f,25,4,1998,4,A
GLLDRA99E04A182Y,Gialli,Dario,m,4,5,1999,4,A
VLIVLI98C43A182B,Viola,Viola,f,3,3,1998,4,B
RSSMRC98M14A182A,Rossi,Marco,m,14,8,1998,4,B
And this is the output:
input: Matricola: 0, Rossi, Mario Classe 3^A
Registration completed: Rossi Mario A 3
classes:
Classe 3^A
1: Matricola: 1, Rossi, Mario
Classe 4^A
Classe 4^B
input: Matricola: 0, Verdi, Luigi Classe 3^A
Registration completed: Verdi Luigi A 3
classes:
Classe 3^A
1: Matricola: 1, Rossi, Mario
2: Matricola: 2, Verdi, Luigi
Classe 4^A
Classe 4^B
input: Matricola: 0, Bianchi, Paola Classe 3^A
Registration completed: Bianchi Paola A 3
classes:
Classe 3^A
1: Matricola: 1, Rossi, Mario
2: Matricola: 2, Verdi, Luigi
Classe 4^A
Classe 4^B
input: Matricola: 0, Neri, Carlo Classe 4^A
Registration completed: Neri Carlo A 4
classes:
Classe 3^A
1: Matricola: 1, Rossi, Mario
2: Matricola: 2, Verdi, Luigi
Classe 4^A
1: Matricola: 4, Neri, Carlo
Classe 4^B
input: Matricola: 0, Rossetto, Giulia Classe 4^A
Registration completed: Rossetto Giulia A 4
classes:
Classe 3^A
1: Matricola: 1, Rossi, Mario
2: Matricola: 2, Verdi, Luigi
Classe 4^A
1: Matricola: 4, Neri, Carlo
Classe 4^B
input: Matricola: 0, Rosa, Rosa Classe 4^A
Registration completed: Rosa Rosa A 4
classes:
Classe 3^A
1: Matricola: 1, Rossi, Mario
2: Matricola: 2, Verdi, Luigi
Classe 4^A
1: Matricola: 4, Neri, Carlo
Classe 4^B
input: Matricola: 0, Gialli, Dario Classe 4^A
Registration completed: Gialli Dario A 4
classes:
Classe 3^A
1: Matricola: 1, Rossi, Mario
2: Matricola: 2, Verdi, Luigi
Classe 4^A
1: Matricola: 4, Neri, Carlo
2: Matricola: 7, Gialli, Dario
Classe 4^B
input: Matricola: 0, Viola, Viola Classe 4^B
Registration completed: Viola Viola B 4
classes:
Classe 3^A
1: Matricola: 1, Rossi, Mario
2: Matricola: 2, Verdi, Luigi
Classe 4^A
1: Matricola: 4, Neri, Carlo
2: Matricola: 7, Gialli, Dario
Classe 4^B
1: Matricola: 8, Viola, Viola
input: Matricola: 0, Rossi, Marco Classe 4^B
Registration completed: Rossi Marco B 4
classes:
Classe 3^A
1: Matricola: 1, Rossi, Mario
2: Matricola: 2, Verdi, Luigi
Classe 4^A
1: Matricola: 4, Neri, Carlo
2: Matricola: 7, Gialli, Dario
Classe 4^B
1: Matricola: 8, Viola, Viola
Just a remainder the output is non-deterministic as far as i know so another run will give a different output also some time after a lot of try it will give a right answer. As you can see not all students are being registered.
CodePudding user response:
Thanks for the MCVE. Without it, it would have been impossible to find out what was wrong, because the problems are in parts of the code you did not show.
The problem starts with the fact that the user base class Utente extends GregorianCalendar
. A user is not a calendar. Maybe a user has or uses one or more calendars. If your class is about OOP principles, you may want to study the differences between generalisation (inheritance) vs. composition vs. aggregation vs. association.
The rather technical problem with Utente
extending GregorianCalendar
is that its parent class Calendar
already implements Comparable<Calendar>
. So if you want to override compareTo
, you cannot just override compareTo(Object)
like you tried (but commented out), or even compareTo(Studente)
in order to achieve sorting in a TreeSet
, but you need to override compareTo(Calendar)
. The other two variants will never be called.
Background: Due to type erasure, You cannot implement the same interface twice for different generic types, i.e. something like Studente extends Utente implements Comparable<Studente>
will yield a compile error "java.lang.Comparable cannot be inherited with different arguments: <scuola.Studente> and <java.util.Calendar>"
.
The consequence of the above is that, the way your code looks now on GitHub, your TreeSet<Studente>
will be sorted as a set of Calendar
objects. But Utente
does not initialise the parent calendar instance by calling super(...)
in its own constructors, i.e. the calendar will be uninitialised, because only its default constructor with no arguments will be called. But that one is inherited by Object
and not overridden by any calendar or user class, i.e. Calendar.compareTo
, when noticing that no time is set for itself, will simply always use System.currentTimeMillis()
. The results are hard to predict and seem non-deterministic, even though of course they are perfectly deterministic, but dependent on when compareTo
is called for which Studente
object, and which ones are on the left and right hand sides of the comparison.
Therefore, because Calendar.compareTo(Calendar)
is inadequate for class Studente
, the TreeSet
will have inconsistent sorting and e.g. A < B and B < C does not reliably mean A < C. It could even be that you get A < B one time and B < A at the same time. Your TreeSet
, if you simply print its size or print its toString()
value to the console, will show the correct size and the correct set of Studente
elements. But when iterating over it like you do in the Classe.toString()
method, you will see strange results due to the bogus ordering, because it upsets the set iterator. By the way, I would recommend a toString
method printing only a simple string without line breaks. If you want to print something with multiple lines, you should do so in a utility method.
So you have several choices:
Implement
compareTo(Calendar)
inStudente
(maybe also inUtente
, if you need it there). But then you need to make sure that it also does something meaningful for otherCalendar
instances passed in or at least throws meaningful exceptions if something other than aStudent
is passed in. But like I said, a student or user is not a calendar, so this feels awfully wrong.Remove
extends GregorianCalendar
fromUtente
and addimplements Comparable<Studente>
toStudente
instead. Then implement acompareTo(Student)
method which is consistent withequals(Object)
. Because yourequals
method compares first by ID (matriculo
), then by last name (cognome
) and first name (nome
), you should do the same incompareTo
and not use some other criterion likemediaVoti()
there. If you want to sort by average votes (whatever that means in your context), you can still do so later usingCollections.sort(List, Comparator)
or so, using a custom comparator.Do not implement
Comparable
at all, use a normal, unsorted set and sort students on demand usingCollections.sort(List, Comparator)
.
I am showing you no. 2, so you can learn something about Comparable
and compareTo
:
public class Utente /*extends GregorianCalendar*/ {
// ...
}
public class Studente extends Utente implements Comparable<Studente> {
// ...
/**
* Confronta per matricola, cognome, nome
*/
@Override
public int compareTo(Studente altroStudente) {
if (this.equals(altroStudente))
return 0;
int result = Integer.compare(matricola, altroStudente.matricola);
if (result != 0)
return result;
result = cognome.compareTo(altroStudente.cognome);
if (result != 0)
return result;
return nome.compareTo(altroStudente.nome);
}
// ...
}
This solution also is not perfect, because it will throw exceptions if first or last names are null
. But I guess for your simple solution this is acceptable.
There are many other smaller and bigger problems in your application, but I know you are all just starting, and I think all in all you have achieved a lot together as a group.