[Java] synchronized HashTable geeft ConcurrentModificationEx

[Kman]

'lo allemaal

Ik ben momenteel bezig met het maken van een serverapplicatie waarin meerdere threads gebruik maken van data uit een object. Om concurrency problemen te voorkomen wordt de code welke toegang tot de data wil hebben in een synchronized blok gezet.

Like so:

code:

public class Data

private HashMap users = new HashMap();

[..]
public synchronized User getUser(String id)
 {
        synchronized(this.users)
        {
            if (this.users.containsKey(id))
            {
                return (User) this.users.get(id);
            }
            else
            {
                return null;
            }
        }
    }
    
    public synchronized void removeUser(String id)
    {
        synchronized(this.users)
        {
            if (this.users.containsKey(id))
            {
                this.users.remove(id);
            }
        }
    }
    
    public synchronized void broadcastToUsers(String message)
    {
      synchronized(this.users)
      {
        Set keySet = this.users.keySet();
        Iterator keySetIterator = keySet.iterator();

        while( keySetIterator.hasNext() )
        {
            String currKey = (String)keySetIterator.next();
            User currUser= (User)this.users.get( currKey );
            currUser.sendMessage( message );
        }
      }
    }

Als test heb ik een set clients verbonden met de server, waarvan een paar de server zwaar gaan spammen met berichten. Als ik, terwijl er gespamt wordt (en broadcastToUsers dus hard bezig is met het verwerken van de opdrachten) een client laat disconnecten (waardoor removeUser wordt aangeroepen) dan geeft broadcastToUsers een ConcurrentModificationException.

Op zich is dat logisch als de users HashMap door meerdere processen wordt gebruikt/veranderd, maar dit zou dus niet moeten kunnen vanwege het gebruik van synchronized. Het lijkt erop dat tijdens het doorlopen van de users in broadcastToUsers er toch een proces toegang tot removeUser heeft gekregen, terwijl er op het object Users een lock zit.

Heeft iemand een idee hoe dit mogelijk is?

[Alarmnummer]

Het zou goed moeten gaan.. dus ik denk toch dat er iets naar buiten is gelekt. Weet je zeker dat het die users is? En check of je iets van die hashmap naar buiten lekt... values/keyset/iterator van de hashmap lekken is al voldoende om dat probleem te veroorzaken.. dus post ook de rest ff.

opmerkingen
1) waarom doe je een synchronized voor de methode call en synchronized voor die users? 1 is voldoende. Persoonlijk prefereer ik de interne synchronized. Ik wil niet dat andere objecten op mijn object een lock kunnen krijgen.

2) waarom doe je

code:

if (this.users.containsKey(id))
            {
                this.users.remove(id);
            }

ipv
this.users.remove(id)?

en

code:

synchronized(this.users)
        {
            if (this.users.containsKey(id))
            {
                return (User) this.users.get(id);
            }
            else
            {
                return null;
            }
        }

return this.users.get(id)

3) waarom zet je overal die this ervoor? Dit is onnodig. Ik hou zelf liever van een prefix (zelf gebruik ik _)

[ Voor 18% gewijzigd door Alarmnummer op 05-03-2005 23:14 ]

[Gert]

Die prefix is net zo onnodig als this. dat is gewoon persoonlijk.

Ik weet niet of het wat oplevert maar moet die map niet ook synchornized zijn? Collections.synchronizedMap(new HashMap());

[Alarmnummer]

Gert schreef op zaterdag 05 maart 2005 @ 23:11:
Die prefix is net zo onnodig als this. dat is gewoon persoonlijk.

Slechte persoonlijke mening. Hoe lang programmeer je al?

voorbeeld van het gevaar...

Dit is goed..

code:

void setVoornaam(String voornaam){
    this.voornaam = voornaam;
}

Dit heb ik vroeger maar al te vaak fout gehad.

code:

setVoornaam(String voornaam){
    voornaam = voornaam;
}

Nu heb je het probleem niet meer en schept enorm veel duidelijkheid. Aan de prefix kun je achterhalen wat voor 'type' veld het is.

code:

setVoornaam(String voornaam){
    _voornaam = voornaam;
}

En hoe wou jij het probleem oplossen als je niet gebruik wilt maken van een prefix of this... een extra variable naam bedenken die net beetje anders is dan degene waar je naar toe wilt schrijven? Krijg je van die onnodig complexe systemen...

Ik weet niet of het wat oplevert maar moet die map niet ook synchornized zijn?

Je programmeert nog niet zo lang merk ik

Collections.synchronizedMap(new HashMap());

Je kunt die hashmap daar ook uitstekend mee synchronized maken, alhoewel je dan net zo goed een hashtable has kunnen nemen. maar de synchronisatie die de TS gebruikt gaat verder dan alleen atomic calls. Hij creeert met zijn synchronized blok een ondeelbare eenheid van werk.. die meerdere calls lang kan zijn. Ik bouw met enige regelmaat mulitthreaded applicaties en daarbij is mijn behoefte om standaard synchronized structuren zoals hashtable, vector, of die Collections.synchronizedXXX volledig verdwenen. Aangezien je meestal toch meerdere calls nodig bent in een ondeelbare actie, en je dan toch al zelf een synchronized blok gaat plaatsen, kan je net zo goed afzien van een standaard synchronized structuur.

[ Voor 14% gewijzigd door Alarmnummer op 05-03-2005 23:28 ]

[Gert]

Mijn ide vertelt mij gewoon als ik this. vergeet. Je kan net zo goed de _ vergeten.
Daarnaast vind ik het bij functie aanroepen duidelijker te zien of het om een eigen functie gaat of een die door overerving beschikbaar is.

Dat mensen niet met synchronizatie werken betekend niet 123 dat ze weinig ervaring hebben, het ligt er maar aan wat je werk gebied is.

[ Voor 24% gewijzigd door Gert op 05-03-2005 23:29 ]

[Alarmnummer]

Gert schreef op zaterdag 05 maart 2005 @ 23:27:
Mijn ide vertelt mij gewoon als ik this. vergeet. Je kan net zo goed de _ vergeten.

Nee.. want je hebt nog steeds geen oplossing gegeven voor wanneer er een naamsconflict is tussen een lokale parameter en een klasse veld.

Daarnaast vind ik het bij functie aanroepen duidelijker te zien of het om een eigen functie gaat of een die door overerving beschikbaar is.

?

[raoulduke]

Alarmnummer schreef op zaterdag 05 maart 2005 @ 23:33:
[...]

Nee.. want je hebt nog steeds geen oplossing gegeven voor wanneer er een naamsconflict is tussen een lokale parameter en een klasse veld.

[...]

?

Begrijp ik het nu verkeerd? De volgende code is perfect valide en compileert ook zonder problemen:

code:

public class Test
{
    private String _t;

    public Test(String t) {
        t = t;
    }
}

Het gebruik van een prefix heeft in dit geval toch geen toegevoegde waarde? Je komt alleen achter het probleem als je een IDE hebt die je waarschuwt, of als je heel je code regel voor regel naloopt. In dit opzicht is het gebruik van "this" of een prefix toch hetzelfde? Kan je misschien iets uitgebreider uitleggen wat je bedoelde met bovenstaande quote, want ik wil het graag weten!

[Kman]

En weer even on topic

Het lijkt erop dat ik een manier heb gevonden wat de synchronized blokken omzeilt (en wat dus de exception veroorzaakte). Beetje tricky om uit te leggen, maar here goes:

De server maakt gebruik van Java's NIO (New I/O) Selectors, Keys en Channels voor netwerkcommunicatie. Deze communicatie wordt gedaan in 1 thread. Vanuit deze netwerkcommunicatie thread kwam de broadcastToUsers call (waardoor de lock ging naar het object welke in de thread draaide). Vanuit broadcastToUsers werd een sendMessage call gegeven naar dat User object. Dat user object bevatte een verwijzing naar de netwerkcommunicatie thread welke weer de 'verwijder mij uit de connectie lijst van de selector' opdracht gaf, en aan deze opdracht zat weer een call naar het data object om de user te verwijderen. Dus uiteindelijk heb ik hiermee een soort circle-of-calls gemaakt waardoor de communicatie thread alsnog de userlijst kon aanpassen terwijl hier nog doorheen gelopen werd.

De fout kwam dus vanuit een onprettige codestructuur, hoewel dit voor een deel wel nodig is om met NIO te kunnen werken.

Een herziening van de code structuur en wat meer checks in de communicatie thread zal dit oplossen. Het synchronized block werkte dus gewoon goed, het werd simpelweg omzeild

Wat betreft this. .. zelf gebruik ik 't inderdaad om parameters van klasse vars te onderscheiden. Klassevars een _ als prefix geven kan dan net zo goed, net wat je prettiger vindt denk ik?

Wel bedankt trouwens voor de suggesties!

[Alarmnummer]

raoulduke schreef op zaterdag 05 maart 2005 @ 23:42:
[...]


Begrijp ik het nu verkeerd? De volgende code is perfect valide en compileert ook zonder problemen:

code:

public class Test
{
    private String _t;

    public Test(String t) {
        t = t;
    }
}

Het gebruik van een prefix heeft in dit geval toch geen toegevoegde waarde?

Compiletechnisch voegt het niet veel toe. Maar ik zie in 1 oogopslag wat voor type variabele ik heb.

Je komt alleen achter het probleem als je een IDE hebt die je waarschuwt, of als je heel je code regel voor regel naloopt.

Mijn persoonlijke ervaring is dat je met this als prefix veel vaker een this vergeet dan als je een _ of m_ gebruikt.

In dit opzicht is het gebruik van "this" of een prefix toch hetzelfde? Kan je misschien iets uitgebreider uitleggen wat je bedoelde met bovenstaande quote, want ik wil het graag weten!

Het komt ook op hetzelfde neer. Maar ik vind werken met this veel foutgevoeliger en verder heb je nu 5 karakters extra per var naam nodig. Ik maar 1.

[Alarmnummer]

KiLLiNGMAN schreef op zaterdag 05 maart 2005 @ 23:45:
En weer even on topic

Het lijkt erop dat ik een manier heb gevonden wat de synchronized blokken omzeilt (en wat dus de exception veroorzaakte). Beetje tricky om uit te leggen, maar here goes:

De server maakt gebruik van Java's NIO (New I/O) Selectors, Keys en Channels voor netwerkcommunicatie. Deze communicatie wordt gedaan in 1 thread. Vanuit deze netwerkcommunicatie thread kwam de broadcastToUsers call (waardoor de lock ging naar het object welke in de thread draaide). Vanuit broadcastToUsers werd een sendMessage call gegeven naar dat User object. Dat user object bevatte een verwijzing naar de netwerkcommunicatie thread welke weer de 'verwijder mij uit de connectie lijst van de selector' opdracht gaf, en aan deze opdracht zat weer een call naar het data object om de user te verwijderen. Dus uiteindelijk heb ik hiermee een soort circle-of-calls gemaakt waardoor de communicatie thread alsnog de userlijst kon aanpassen terwijl hier nog doorheen gelopen werd.

De fout kwam dus vanuit een onprettige codestructuur, hoewel dit voor een deel wel nodig is om met NIO te kunnen werken.

Hou deze regels in de gaten:

Always lock during updates to object fields.

Always lock during access of possibly updated object fields.

Never lock when invoking methods on other objects.

Een herziening van de code structuur en wat meer checks in de communicatie thread zal dit oplossen. Het synchronized block werkte dus gewoon goed, het werd simpelweg omzeild

Er zijn (uiteraard) verschillende oplossingen. Maar een oplossing die ik zelf graag mag gebruiken is dat van de reactor pattern (zie patterns of software architecture). Je hebt 1 centrale queue waar messages (commando`s) geplaatst kunnen worden. Achter deze queue zit 1 thread die op een semafoor ligt te maffen. Alle threads mogen messages in de message queue plaatsen, maar er is maar 1 thread aanwezig om ze af te handelen. Je kunt dan een groot deel van je datastructuren bij die thread neergooien en je hebt dan de veiligheid dat je bijna niets aan concurrency control hoeft te doen. Hou er wel rekening mee dat je achter die queue geen multithreaded systeem meer hebt.. het is ineens sequentieel geworden... Misschien dat dit een oplossing voor je probleem kan zijn.... maarja... er zijn nog wel 10.000 andere oplossingen.

[Verwijderd]

Alarmnummer schreef op zaterdag 05 maart 2005 @ 23:33:
Nee.. want je hebt nog steeds geen oplossing gegeven voor wanneer er een naamsconflict is tussen een lokale parameter en een klasse veld.
?

Ik schrijf liever werkende Beans in plaats van dat onzinnige geprefix...

[zeroxcool]

Alarmnummer schreef op zondag 06 maart 2005 @ 00:01:
Hou er wel rekening mee dat je achter die queue geen multithreaded systeem meer hebt.. het is ineens sequentieel geworden... Misschien dat dit een oplossing voor je probleem kan zijn.... maarja... er zijn nog wel 10.000 andere oplossingen.

Alsof een systeem met maar één processor écht multithreaded is .

[Alarmnummer]

Verwijderd schreef op zondag 06 maart 2005 @ 14:15:
[...]
Ik schrijf liever werkende Beans in plaats van dat onzinnige geprefix...

Hmm tja.. ik programmeer al heel wat jaartjes. en ik heb al heel wat stijlen geprobeerd. Door een prefix is mijn code informatiever. En verder kan ik in 1 oogopslag zien of het een lokale var is, of een member.. Dat vind ik persoonlijk wel vrij sterke argumenten om het wel te gebruiken. Een prefix en this. zijn in mijn ogen even veel waard, alleen vind ik de this. fout gevoeliger en vervelender om te lezen. Dus vandaar de _.

Trouwens.. jouw antwoord raak kant nog wel... aub je mening ff onderbouwen ipv wat neer te hoesten.

[ Voor 3% gewijzigd door Alarmnummer op 06-03-2005 15:38 ]

[Alarmnummer]

ZeRoXcOoL schreef op zondag 06 maart 2005 @ 14:33:
[...]
Alsof een systeem met maar één processor écht multithreaded is .

Ja en wat wil je hiermee zeggen? Dat je geen concurrency problematiek hebt in een computer met maar 1 cpu? Terug naar de 1e klas met jou.

[ Voor 6% gewijzigd door Alarmnummer op 06-03-2005 15:39 ]

[Verwijderd]

Alarmnummer schreef op zondag 06 maart 2005 @ 15:34:
Trouwens.. jouw antwoord raak kant nog wel... aub je mening ff onderbouwen ipv wat neer te hoesten.

Omdat het conventie is je "privates" te gebruiken als naam voor je methodes. Dus raakt wel degelijk kant, en wel degelijk wal. Leebaarheid is ook niet interessant voor die paar regeltjes code in een setter...

[Alarmnummer]

Verwijderd schreef op zondag 06 maart 2005 @ 15:43:
[...]

Omdat het conventie is je "privates" te gebruiken als naam voor je methodes.

Dit snap ik niet.

Dus raakt wel degelijk kant, en wel degelijk wal. Leebaarheid is ook niet interessant voor die paar regeltjes code in een setter...

Geef eens een voorbeeld, want ik snap niet wat je bedoelt.

[Verwijderd]

Dergelijke conventies:
http://homepage.mac.com/l...nal/java/jguide.html#N644

[Alarmnummer]

Verwijderd schreef op zondag 06 maart 2005 @ 16:01:
Dergelijke conventies:
http://homepage.mac.com/l...nal/java/jguide.html#N644

Hmm... ik zie de relatie tussen die getters/setter specificatie en de prefix van klasse velden niet. (Ik vind je antwoord dus nog steeds kant nog wal raken)

[Verwijderd]

Mag je vinden. Don't really care actually

[Alarmnummer]

Verwijderd schreef op zondag 06 maart 2005 @ 16:06:
Mag je vinden. Don't really care actually

Hmm tja.. Jij geeft antwoord op iets dat geloof ik niet werd gevraagd. Ik vraag om opheldering en jij komt nog een keer met iets aanzetten dat nog steeds geen relatie heeft met die prefix. Geef dan gewoon geen antwoord.. scheelt jou en mij tijd.

[Verwijderd]

Gedraag je gewoon eens niet zo vreselijk arogant/betweterig... Dan heb ik geen zin meer motivatie te geven.

[Jrz]

Dus vanuit sendMessage() roep je removeUser aan?
Dat kan niet, je moet dan keySetIterator.remove() gebruiken..

btw, die rant over code stijlen.. prefix of niet maakt niet veel uit.. Je IDE zal als het goed is een local var een ander kleurte geven, en warnings geven bij code als t=t.

Ikzelf gebruik geen _ prefix

[ Voor 49% gewijzigd door Jrz op 06-03-2005 16:18 ]

[flowerp]

KiLLiNGMAN schreef op zaterdag 05 maart 2005 @ 23:45:
En weer even on topic

Het lijkt erop dat ik een manier heb gevonden wat de synchronized blokken omzeilt (en wat dus de exception veroorzaakte). Beetje tricky om uit te leggen, maar here goes:

Wat je dus eigenlijk dus hebt is het volgende patroon:

Java:

public void removeUser(String id)  {
        synchronized( users )  {
                users.remove(id);
        }
}

public void broadcastToUsers(String message) {
      synchronized(this.users)  {
        Set keySet = this.users.keySet();
        Iterator keySetIterator = keySet.iterator();

        while( keySetIterator.hasNext() )  {
            String currKey = (String)keySetIterator.next();
            User currUser= (User)this.users.get( currKey );
            
            // chain of calls
            // problematic part:
            removeUser(id);
        }
      }
}

Het hele NIO verhaal heeft er mischien niet zoveel mee te maken. Ook je statement dat het user object een referentie heeft naar je broadcasting thread lijkt me niet te klopppen. Namelijk, je thread kan maar 1 ding tegelijk doen, en die kun je niet via een andere thread opeens nog wat laten doen.

Waar het om gaat is dat je het User object aanroept in de context van je huidige thread. Dit user object heeft een referentie naar het Data object in kwestie en roept daar de removeUser op aan. Dit heeft dus eigenlijk niks met threads te maken; het zijn alleen call stacks die niks met threads doen. Sla die stacks plat en je hebt feitelijk de volgende code:

Java:

public void broadcastToUsers(String message) {
        Set keySet = this.users.keySet();
        Iterator keySetIterator = keySet.iterator();

        while( keySetIterator.hasNext() )  {
            String currKey = (String)keySetIterator.next();
            User currUser= (User)this.users.get( currKey );
            // doesn't work:
            users.remove(id);
        }
}

Wat betreft this. .. zelf gebruik ik 't inderdaad om parameters van klasse vars te onderscheiden. Klassevars een _ als prefix geven kan dan net zo goed, net wat je prettiger vindt denk ik?

Ik twijfel hierover. een reden om geen this te gebruiken is dat je je code makkelijker kan refactoren/hergebruiken. In veel gevallen is het voor de werken van een stuk code helemaal niet belangrijk of een variable object scope of method scope heeft. Bij het refactoren kom ik er nog wel eens achter dat iets helemaal niet object wide beschikbaar hoeft te zijn en geef ik deze direct aan een method mee (bijvoorbeeld). Als overal this voor stond kan ik dat met de hand gaan veranderen.

Als je prefixen gebruikt is dat toch makkelijker (hoewel dan de prefix conventie weer niet klopt na refactoren). Vroeger gebruikte ik uitsluitend prefixen, tegenwoordig ben ik daar toch een beetje vanaf gestapt.

Een andere toepassing van prefixen (bv m_) zie je vaak om onderscheid te maken tussen data in de huidige classe en die van de super classes. Dit kun je dan natuurlijk alleen voor private data gebruiken, anders klopt de conventie niet meer bij de volgende class die van jou inherrit. Je zit dan met je public / protected data die geen prefix gebruikt, en je private data die dat wel doet: niet acceptabel (voor mij), en daarom gebruik ik het maar helemaal niet.

Overigens @TS:

Waarom zet jij je accoladed zo raar onder elkaar? Dit is tegen de java conventies (zie sun website), en geeft optisch een beetje raar beeld. Omdat je zo veel regels krijgt waar telkens in kolommen haakjes recht onder elkaar staan, lijkt het net op de code 'huilt'. Ik weet dat andere mensen dit ook wel eens doen, maar het ziet er (imho) niet echt uit...

[Alarmnummer]

Verwijderd schreef op zondag 06 maart 2005 @ 16:13:
Gedraag je gewoon eens niet zo vreselijk arogant/betweterig.

Hmm tja.. daar heb je wel een beetje gelijk in. Maar check verder hoe jij dit topic binnenkomt... getuigt ook niet van bijster veel respect. Dus vandaar mijn afbrandend antwoord..

[Jrz]

Die accolades doe ik ook zo. Veel overzichtelijker..

[alienfruit]

Normaal gesproken maak ik niet gebruik van zulke variable namen zoals Alarmnummer, maar bij. setVoorname( String iVoornaam ); etc. Waarbij i aangeeft dat een incoming variable is, zo dus ook o voor outgoing. Maar goed meer een kwestie van smaak.

[flowerp]

Jrz schreef op zondag 06 maart 2005 @ 16:16:
Dus vanuit sendMessage() roep je removeUser aan?
Dat kan niet, je moet dan keySetIterator.remove() gebruiken..

Nou, het gaat mischien te ver om zo zonder meer te roepen dat het niet kan. De naam sendMessage suggereert toch een beetje dat het User object in een andere thread draait. Vanuit een andere thread mag je zeer zeker wel een call naar removeUser doen. Die execute dan namelijk in de context van die andere thread. Omdat de feitelijke code in een synchronized block staat, zal die andere thread netjes op zijn beurt wachten.

Dat het nu fout gaat komt omdat sendMessage helemaal niet naar een andere thread gaat, maar gewoon in de huidige blijft draaien. Dan werkt synchronized natuurlijk niet. Deze KAN ook niet werken in die situatie, want anders had je een deadlock. Anders gezegd, de iteratie uit het code voorbeeld van de TS zal pas verder gaan nadat sendMessage terugkomt.

btw, die rant over code stijlen.. prefix of niet maakt niet veel uit.. Je IDE zal als het goed is een local var een ander kleurte geven, en warnings geven bij code als t=t.
Ikzelf gebruik geen _ prefix

Niet als rant bedoelt hoor Maar het is eigenlijk meestal niet de IDE die een warning geeft maar de compiler. Je idee lijkt me wel correct overigens: je kunt met tools warnings laten geven voor 'name hiding'.

[Alarmnummer]

flowerp schreef op zondag 06 maart 2005 @ 16:18:
[...]


Wat je dus eigenlijk dus hebt is het volgende patroon:

Java:

public void removeUser(String id)  {
        synchronized( users )  {
                users.remove(id);
        }
}

public void broadcastToUsers(String message) {
      synchronized(this.users)  {
        Set keySet = this.users.keySet();
        Iterator keySetIterator = keySet.iterator();

        while( keySetIterator.hasNext() )  {
            String currKey = (String)keySetIterator.next();
            User currUser= (User)this.users.get( currKey );
            
            // chain of calls
            // problematic part:
            removeUser(id);
        }
      }
}

Ik zou er voor zorgen dat je een copy gaat maken van de structuur die je lockt terwijl je calls naar buiten doet. Je zult dan wel iets vriendelijkere code moeten maken aangezien je niet perfect kunt voorspellen of iets wel of niet aanwezig is.

code:

public void broadcastToUsers(String message) {
    Map clonedUsers;      
    synchronized(this.users)  {
        clonedUsers =  new Hashmap(this.users);
    }
        
    Set keySet = clonedUsers.keySet();
    Iterator keySetIterator = keySet.iterator();

    while( keySetIterator.hasNext() )  {
        String currKey = (String)keySetIterator.next();
        User currUser= (User)clonedUsers.get( currKey );
            
        // chain of calls
        // problematic part:
        removeUser(id);
    }
}

Als je trouwens 100% controle wilt hebben (bv voor zoiets als boekingen), dan zou ik toch voor een zwaarde synchronisatie oplossing gaan waarbij je beter kunt voorspellen in welke toestand het systeem zich bevind. De Reactor vind ik persoonlijk erg handig daarvoor. Maar in veel gevallen mag het wel iets 'ruimer' zijn.

[ Voor 214% gewijzigd door Alarmnummer op 06-03-2005 16:44 ]

[flowerp]

Jrz schreef op zondag 06 maart 2005 @ 16:21:
Die accolades doe ik ook zo. Veel overzichtelijker..

Zoals gezegd, het gaat tegen de java conventies in. En waarom is het overzichtelijker? (laat staan VEEL overzichtelijker?)

Ik denk juist dat het andersom is. Een accolade sluti in 1ste instantie een blok af die een betekenis heeft. Dat de ene accolade een andere afsluit is leuk, maar accolade - accolade zegt me niet zoveel. Ik zie veel liever direct WAT een accolade afsluit. Met een accolade onder de naam van het blok is dat toch net even sneller zichtbaar. Bijvoorbeeld:

code:

while ( Jrz het licht nog niet heeft gezien ) {
     moeten mensen Jrz overtuigen
}

Het ziet meteen dat het blok een while is. Vergelijk met:

code:

while ( Jrz het licht nog niet heeft gezien )
{
     moeten mensen Jrz overtuigen
}

In de eerste instantie zie je nu aanleen 2 haakjes onder elkaar. Je oog moet even omhoog springen om te zien dat het een while is. -zo- groot is het verschil natuurlijk niet, dus de stelling dat het niet de conventie is voor Java vind ik sterker.

[Nick_S]

Ook nog een leuke truukje, waar je geen prefix, speciale IDE of zo nodig hebt. Zorg dat je compiler gaat klagen als het fout doet.

Als je dit tikt:

code:

private String voornaam;

public void setVoornaam(final String voornaam) {
voornaam = voornaam;
}

terwijl je dit:

code:

private String voornaam;

public void setVoornaam(final String voornaam) {
this.voornaam = voornaam;
}

bedoelde en je probeert te compileren, krijg je een compiler fout. Ik maak daarom mijn argumenten ook altijd final. Heeft me al een paar keer een hoop tijd gescheeld.

[flowerp]

Nick_S schreef op zondag 06 maart 2005 @ 17:08:
Ook nog een leuke truukje, waar je geen prefix, speciale IDE of zo nodig hebt. Zorg dat je compiler gaat klagen als het fout doet.

Inderdaad ja. Iets soortgelijks wordt in C++ gedaan met het const keyword. Op de een of andere reden zie je het daar wat vaker gebruikt worden dan in Java.

[Jrz]

flowerp schreef op zondag 06 maart 2005 @ 16:40:
[...]


Zoals gezegd, het gaat tegen de java conventies in. En waarom is het overzichtelijker? (laat staan VEEL overzichtelijker?)

Ik denk juist dat het andersom is. Een accolade sluti in 1ste instantie een blok af die een betekenis heeft. Dat de ene accolade een andere afsluit is leuk, maar accolade - accolade zegt me niet zoveel. Ik zie veel liever direct WAT een accolade afsluit. Met een accolade onder de naam van het blok is dat toch net even sneller zichtbaar. Bijvoorbeeld:

code:

while ( Jrz het licht nog niet heeft gezien ) {
     moeten mensen Jrz overtuigen
}

Het ziet meteen dat het blok een while is. Vergelijk met:

code:

while ( Jrz het licht nog niet heeft gezien )
{
     moeten mensen Jrz overtuigen
}

In de eerste instantie zie je nu aanleen 2 haakjes onder elkaar. Je oog moet even omhoog springen om te zien dat het een while is. -zo- groot is het verschil natuurlijk niet, dus de stelling dat het niet de conventie is voor Java vind ik sterker.

Je ziet duidelijker waar iets begint / eindigt.. Je ziet meer blokvormen..
In C/C++ deed ik het op het begin op de andere manier, maar ben daarna overgestapt hierop.
Nooit meer teruggegaan

[Verwijderd]

Dan zul je het nog moeilijk gaan krijgen als je voor je beroep in teams moet gaan coden. Aan conventies wordt toch wel zwaar aan gehecht. Ik denk dat de seniors not-amused zijn als jij in een groot project ff lekker alle haakjes tegen de conventies in gaat zitten te zetten.

[Alarmnummer]

Verwijderd schreef op maandag 07 maart 2005 @ 13:18:
Dan zul je het nog moeilijk gaan krijgen als je voor je beroep in teams moet gaan coden. Aan conventies wordt toch wel zwaar aan gehecht. Ik denk dat de seniors not-amused zijn als jij in een groot project ff lekker alle haakjes tegen de conventies in gaat zitten te zetten.

Zo lang er niet onnodig veel extra haakjes in staan... is een reformat (zit wel in meeste ide`s) je vriend. Ik vind het persoonlijk niet zo`n ramp als ze een kut layout gebruiken... het is tenslotte zo aangepast.

Ik heb meer problemen met brakke code constructies, te veel code, onduidelijke code, of gewoon foute/slechte code. En uiteraard... overgedesignde code... helemaal een drama

[ Voor 24% gewijzigd door Alarmnummer op 07-03-2005 14:33 ]

[Kman]

flowerp schreef op zondag 06 maart 2005 @ 16:18:
[...]
Het hele NIO verhaal heeft er mischien niet zoveel mee te maken. Ook je statement dat het user object een referentie heeft naar je broadcasting thread lijkt me niet te klopppen. Namelijk, je thread kan maar 1 ding tegelijk doen, en die kun je niet via een andere thread opeens nog wat laten doen.

[...]

Even om het verhaal compleet te maken:

sendMessage van het user object vroeg aan de netwerkcommunicatiethread om een bericht te versturen. Wanneer dit gebeurde naar een user wie in de tussentijd de verbinding had verbroken, ontstond er een IOException. Als reactie op die exception werd (nog steeds vanuit de netwerkcommunicatiethread) vervolgens removeUser van het Data object aangeroepen (want user was niet meer verbonden en hoorde daarom niet meer in users array). Hierdoor kon de netwerkcommunicatiethread zowel broadcastUsers als removeUser door elkaar callen. Daar kwam dus de fout vandaan.

Wat betreft de hele discussie over code style.. er is blijkbaar vraag voor een apart topic hierover?

[ Voor 78% gewijzigd door Kman op 07-03-2005 20:55 . Reden: ontmixen van tegenwoordige tijd en verleden tijd ]

[Alarmnummer]

Heb je mijn reply over het clonen van die usershashmap wel gezien? Daarmee los je jouw probleem goed mee op.

[Verwijderd]

KiLLiNGMAN schreef op maandag 07 maart 2005 @ 20:36:
[...]
Even om het verhaal compleet te maken:

sendMessage van het user object vroeg aan de netwerkcommunicatiethread om een bericht te versturen. Wanneer dit gebeurde naar een user wie in de tussentijd de verbinding had verbroken, ontstond er een IOException. Als reactie op die exception werd (nog steeds vanuit de netwerkcommunicatiethread) vervolgens removeUser van het Data object aangeroepen (want user was niet meer verbonden en hoorde daarom niet meer in users array). Hierdoor kon de netwerkcommunicatiethread zowel broadcastUsers als removeUser door elkaar callen. Daar kwam dus de fout vandaan.

Ik denk dat deze toelichting het hele gebeuren wel helemaal verklaard. Het is een gemakkelijk te maken fout die je snel over het hoofd ziet. De essentie van de uitleg van Leeko blijft staan: terwijl je aan het ittereren bent roep je code aan die in dezelfde thread in 1 van z'n controll paths een mutatie aan de hashtable doet. Of dat controll path nu een exception handler is, of een if statement of whatever maakt niks uit. De code staat er en zal ooit kunnen worden aangeroepen.

De oplossing die Alarmnummer dan aandraagt lijkt me een perfecte oplossing. Heb je deze ook toegepast?

Wat betreft de hele discussie over code style.. er is blijkbaar vraag voor een apart topic hierover?

Die is er geloof ik wel al vaker geweest. In dit topic is het echter wel lekker toegepast. Accolades onder elkaar vind ik ook verschrikkelijk. Je ziet het ook zelden in boeken, tutorials en dergelijke. Ik dacht dat het wel algemeen aangenomen was dat het bad-coding style was. Gelukkig kan elke IDE van visual studio tot en met eclipse het met 1 druk op de knop recht zetten

[Jrz]

Verwijderd schreef op maandag 07 maart 2005 @ 13:18:
Dan zul je het nog moeilijk gaan krijgen als je voor je beroep in teams moet gaan coden. Aan conventies wordt toch wel zwaar aan gehecht. Ik denk dat de seniors not-amused zijn als jij in een groot project ff lekker alle haakjes tegen de conventies in gaat zitten te zetten.

Bestaande conventies neem ik altijd over.. Maar mijn eigen projecten / nieuwe projecten doe ik op mijn manier, en de andere ontwikkelaars zullen daar naar moeten luisteren.

[Verwijderd]

Je zult het zelf wel het beste weten, maar echt een profesionele uitstraling geeft het niet. (hoeft natuurlijk ook helemaal niet als je geen pro bent, maar gewoon lekker voor jezelf aan het rommelen bent )

Als je wel pro bent komt het een beetje over als "zodra niemand me wat kan maken ga ik lekker slordig coden", beetje als iemand die op zijn rij examen keurig rijdt en zodra ie zijn papiertje heeft opeens als een malloot gaat rijden en alle regels aan z'n laars lapt.

Maar goed, laten we de haakjes discussie maar niet verder zetten

[Kman]

Alarmnummer schreef op maandag 07 maart 2005 @ 21:40:
Heb je mijn reply over het clonen van die usershashmap wel gezien? Daarmee los je jouw probleem goed mee op.

Verwijderd schreef op maandag 07 maart 2005 @ 23:51:
[...]
De oplossing die Alarmnummer dan aandraagt lijkt me een perfecte oplossing. Heb je deze ook toegepast?
[...]

Yup, bedankt daarvoor trouwens

[Jrz]

Verwijderd schreef op dinsdag 08 maart 2005 @ 11:18:
Je zult het zelf wel het beste weten, maar echt een profesionele uitstraling geeft het niet. (hoeft natuurlijk ook helemaal niet als je geen pro bent, maar gewoon lekker voor jezelf aan het rommelen bent )

Als je wel pro bent komt het een beetje over als "zodra niemand me wat kan maken ga ik lekker slordig coden", beetje als iemand die op zijn rij examen keurig rijdt en zodra ie zijn papiertje heeft opeens als een malloot gaat rijden en alle regels aan z'n laars lapt.

Als dat zo op jouw overkomt, dan is dat zo. Maar een accolade-stijl heeft absoluut niet met 'slordig coden' te maken.

Verwijderd schreef op dinsdag 08 maart 2005 @ 11:18:
Maar goed, laten we de haakjes discussie maar niet verder zetten

ok

[Kwistnix]

Alarmnummer schreef op zondag 06 maart 2005 @ 16:09:
[...]

Hmm tja.. Jij geeft antwoord op iets dat geloof ik niet werd gevraagd. Ik vraag om opheldering en jij komt nog een keer met iets aanzetten dat nog steeds geen relatie heeft met die prefix. Geef dan gewoon geen antwoord.. scheelt jou en mij tijd.

Wat ik denk dat hij bedoelt.
Als je een member aanroept van een bepaalde Bean dan wordt intern de getter/setter methode aangeroepen, maar dan moet getter/setter wel dezelfde naam heeft als de member. Je prefixt de member met een _ maar dat doe je met methode naam van de getter/setter doorgaans niet. In dat geval werkt je Bean dus niet naar behoren.

Ik ga er trouwens niet vanuit dat ik je nu iets nieuws vertel hoor

[Verwijderd]

FallenAngel666 schreef op dinsdag 08 maart 2005 @ 22:22:
[...]


Wat ik denk dat hij bedoelt.
Als je een member aanroept van een bepaalde Bean dan wordt intern de getter/setter methode aangeroepen, maar dan moet getter/setter wel dezelfde naam heeft als de member. Je prefixt de member met een _ maar dat doe je met methode naam van de getter/setter doorgaans niet. In dat geval werkt je Bean dus niet naar behoren.

Ik ga er trouwens niet vanuit dat ik je nu iets nieuws vertel hoor

Dat doe je zeer zeker wel. Het is namelijk klinkklare onzin wat jij hier loopt te verkondigen!

De naam van je data member heeft GEEN ENKELE RELATIE met de naam van je getter/setter. Voor reflection (het mechanisme waarmee java beans kunnen werken), telt alleen de method signature. Op geen enkele manier whatshowever zal er enige inspectie van de method body plaatsvinden. Het is ten eerste theoretisch onmogelijk om te bewijzen welke data member de body uiteindelijk gaat gebruiken, en ten tweede zou dat gewoon onzin zijn.

Ook is het in de java bean specificatie NIET opgenomen dat dmv reflectie willekeurige data members aangesproken kunnen worden met de bean syntax. Er moet een method bestaan met de juiste signature, anders werkt het niet. Andersom werkt het ook niet, het aanspreken van een public data member zal niet leiden tot een aanroep van een getter/setter. In C# heb je daar wel een mechanisme voor, in standaard Java niet.

Waar het overigens wel wat uitmaakt (maar wat compleet hier los van staat), is dat sommige IDEs (bv Eclipse) getters en setters kunnen genereren aan de hand van geselecteerde data members. Door gebruik te maken van prefixen wordt deze automatische generatie een beetje lelijk. Maar nogmaals, dat staat compleet los van wat jij beweerd.

Terug naar klas 1 met jou dus!

[ Voor 6% gewijzigd door Verwijderd op 08-03-2005 22:54 ]

[Kwistnix]

Verwijderd schreef op dinsdag 08 maart 2005 @ 22:51:
[...]


Dat doe je zeer zeker wel. Het is namelijk klinkklare onzin wat jij hier loopt te verkondigen!

De naam van je data member heeft GEEN ENKELE RELATIE met de naam van je getter/setter. Voor reflection (het mechanisme waarmee java beans kunnen werken), telt alleen de method signature. Op geen enkele manier whatshowever zal er enige inspectie van de method body plaatsvinden. Het is ten eerste theoretisch onmogelijk om te bewijzen welke data member de body uiteindelijk gaat gebruiken, en ten tweede zou dat gewoon onzin zijn.

Ook is het in de java bean specificatie NIET opgenomen dat dmv reflectie willekeurige data members aangesproken kunnen worden met de bean syntax. Er moet een method bestaan met de juiste signature, anders werkt het niet. Andersom werkt het ook niet, het aanspreken van een public data member zal niet leiden tot een aanroep van een getter/setter. In C# heb je daar wel een mechanisme voor, in standaard Java niet.

Waar het overigens wel wat uitmaakt (maar wat compleet hier los van staat), is dat sommige IDEs (bv Eclipse) getters en setters kunnen genereren aan de hand van geselecteerde data members. Door gebruik te maken van prefixen wordt deze automatische generatie een beetje lelijk. Maar nogmaals, dat staat compleet los van wat jij beweerd.

Terug naar klas 1 met jou dus!

Ja mag ook normaal reageren hoor.
Zo is het mij verteld, dus ik weet niet beter.
Maar bedankt voor jouw uitleg hoor, nu mag je aan je gefrustreerde persoonlijkheid gaan sleutelen.

[Verwijderd]

FallenAngel666 schreef op dinsdag 08 maart 2005 @ 23:19:
[...]

Ja mag ook normaal reageren hoor.
Zo is het mij verteld, dus ik weet niet beter.
Maar bedankt voor jouw uitleg hoor, nu mag je aan je gefrustreerde persoonlijkheid gaan sleutelen.

Lachuh joh! Geef nou maar gewoon toe: je was een beetje dom
Ik hoop dat je het nu wel een beetje begrijpt.

_{henk gaat maar aan zijn gefrustreerde persoonlijkheid sleutelen}

[Kwistnix]

Verwijderd schreef op dinsdag 08 maart 2005 @ 23:46:
[...]


Lachuh joh! Geef nou maar gewoon toe: je was een beetje dom
Ik hoop dat je het nu wel een beetje begrijpt.

_{henk gaat maar aan zijn gefrustreerde persoonlijkheid sleutelen}

Als je een gebrek aan kennis, of liever een teveel aan onjuiste informatie, dom wilt noemen was mijn reactie behoorlijk dom ja
Ik begrijp nu hoe het zit vanwege jouw duidelijk uitleg, maar je had het wel iets tactischer kunnen brengen. Ga maar gauw sleutelen dan!