Budzik - przystosowanie kodu do obsługi kilku alarmów

Question

Czesc,
napisalem budzik ktory dziala jak powinien tylko wtedy, kiedy obsluguje jeden alarm. Proba obslugi kilku wydarzen jednoczesnie zakonczyla sie fiaskiem. Sypie sie wszystko co tylko moze. Moglby ktos mnie nakierowac w jaki sposob to obsluzyc? I prosilbym o jakis maly code review.

http://www.wklej.org/id/3095071/ //Main
http://www.wklej.org/id/3095072/ //Event
http://www.wklej.org/id/3095073/ //Listener
http://www.wklej.org/id/3095074/ //Stoper

@EDIT:
W startMenu() dodalem while loopa, zeby dwukrotnie ustawic alarm. Watek czeka do wybicia najblizszej ustawionej godziny, dwa razy wyswietla run() z klasy Stopper i rzuca bledami:

TRY TO RE-WRITE: "abecadlo z pieca spadlo i nie wpadlo, o ziemie huklo":
TRY TO RE-WRITE: "abecadlo z pieca spadlo i nie wpadlo, o ziemie huklo":
java.lang.ArrayIndexOutOfBoundsException: 580
	at javazoom.jl.decoder.LayerIIIDecoder.huffman_decode(Unknown Source)
	at javazoom.jl.decoder.LayerIIIDecoder.decode(Unknown Source)
	at javazoom.jl.decoder.LayerIIIDecoder.decodeFrame(Unknown Source)
	at javazoom.jl.decoder.Decoder.decodeFrame(Unknown Source)
	at javazoom.jl.player.Player.decodeFrame(Unknown Source)
	at javazoom.jl.player.Player.play(Unknown Source)
	at javazoom.jl.player.Player.play(Unknown Source)
	at Listener.run(Listener.java:34)
	at java.lang.Thread.run(Thread.java:748)
java.lang.ArrayIndexOutOfBoundsException: 580
	at javazoom.jl.decoder.LayerIIIDecoder.huffman_decode(Unknown Source)
	at javazoom.jl.decoder.LayerIIIDecoder.decode(Unknown Source)
	at javazoom.jl.decoder.LayerIIIDecoder.decodeFrame(Unknown Source)
	at javazoom.jl.decoder.Decoder.decodeFrame(Unknown Source)
	at javazoom.jl.player.Player.decodeFrame(Unknown Source)
	at javazoom.jl.player.Player.play(Unknown Source)
	at javazoom.jl.player.Player.play(Unknown Source)
	at Listener.run(Listener.java:34)
	at java.lang.Thread.run(Thread.java:748)
Exception in thread "Thread-2" java.lang.IndexOutOfBoundsException: Index: 0, Size: 0
	at java.util.ArrayList.rangeCheck(ArrayList.java:653)
	at java.util.ArrayList.get(ArrayList.java:429)
	at Listener.run(Listener.java:22)
	at java.lang.Thread.run(Thread.java:748)

Comments

Daj logi

---

Dodalem do tematu bledy ktorymi sypie.

Answer 1

Ciężko mi coś doradzić. Ale na piewrszy rzut oka pewnie masz problemy z kodem równoległym. Nigdzie nie widze synchronizacji a próbujesz używać jednego zasobóu z kilku miejsc. Bo jak w Main w 56 piszesz ze tu jest jakiś loop. To obawiam się że wywoływałeś startListening() dwa razy. A z tego co rozumiem twój kod to powinieneś wywoływać go tylko raz

Co do code review. Mam kilka uwag.
Pierwsza. Atrybuty w klasie Main nie widze potrzeby abyś miał tyle pól ponieważ w wiekszosci przypadków i tak ich nie używasz w całej klasie tylko linijke po ich stworzeniu. Dlatego możesz się ich pozbyć.

Druga Gettery i Settery. O ile do geterów nie zabardzo mam jak się przyczepić( chociaż patrz uwaga wyżej nie są potrzebne) za to settery mają ustawiać wartość a nie je inicializować. Od tego jest konstruktor. Np setEvent moim zdaniem powinno nosić nazwe createEvent i nie wpisywać eventu do pola którego nigdzie indziej nie używasz.

Trzecia. Warto ustawiać metody w kolejnosci ich wywoływania. Czyli w twoim przypadku main->startMenu->loadPlayer->i tak dalej i tak dalej. Pozwala to na czytanie klasy od góry do dołu. A teraz jest tak że musisz latać oczami po całej klasie bo nie wiesz gdzie co jest. Polecam też getery i settery dawać na sam dół bo zazwyczaj nie ma tam interesującej treści do czytania.

Comments

zgadza sie, wywoluje to dwa razy. piszac to mialem zamiar w taki sposob uaktualniac liste alarmow w klasie Listener. przychodzi Ci do glowy jakis inny, tym razem prawidlowy sposob na zrobienie tego?

---

Przecież masz liste. Jak dodasz alarm i posortujesz to nie musisz dodawać listy do wątku. On już ma na niego referencje. Tylko nie zapomnij o synchronizacji

Answer 2

W Listenerze masz pętle 

while(!alarmList.isEmpty())

Czyli dopoki nie jest pusta a ty jej w żaden sposób nie opróżniasz więc jeśli jest pusta to się nie wykona wcale a jeśli nie jest pusta to bedzie się cały czas wykonywać więc domyślam się że stąd IndexOutOfBoundException

Więc zrób to forem 

for(Date d :alarmList)
//Lub
for (int i = 0; i<alarmList.size();i++)

 

Comments

w 38 linii usunalem komentarz na alarmList.remove(0);.W mojej glowie to wygladalo tak, ze po kazdej wykonanej petli

while (alarmList.get(0).compareTo(currentTime) == 1) 

powinien zostac usuniety wykonany alarm i teraz sprawdzic warunek petli !alarmList.isEmpty(). No ale to tylko w mojej glowie. Jak to dziala w takim razie w rzeczywistosci?

---

Z listy się nic nie usuwa dopóki elementu nie usuniesz. Zrób to for'em i najpierw sprawdź czy alarmList nie jest null'em.

for(Date d : alarmlist){
while(alarmList.get(0).compareTo(d)== 1)
...

Rozumiem, że element zerowy musi być stworzony i nie ma opcji żeby go nie było skoro na sztywno o niego pytasz?