[Java] Loginverificatie ontwerp

[Glimi]

Inleiding:
Ik ben bezig met ontwerpen van een applicatie waarin verschillende types mensen kunnen inloggen ( mentoren, leerlingen en decanen ). Al deze mensen hebben verschillende rechten.

Nu moeten deze personen natuurlijk inloggen en aan de hand van hun login & pass moet hun status bepaald worden ( leerling of mentor? )
Nu ben ik dus opzoek naar een mooie OO oplossing hiervoor.


Situatie:

Verschillende soorten mensen kunnen inloggen in de applicatie: Mentoren, leerlingen en Decanen, dus hiervoor heb ik verschillende classes gemaakt.



Nu vind ik inloggen in de applicatie niet echt een actie van de personen zelf, maar van de applicatie, dus heb ik hiervoor ook een class gemaakt. Deze class kan teruggeven of de persoon toegang heeft en een request voor toegang doen. Hier even wat code van die class om het te verduidelijken :

code:

public class ToegangControleur {
    
    private String  naam = "";
    private String  wachtwoord = "";
    private boolean     toegang = false;

    // Default constructor, het aangemaakte zal geen waardes bevatten; 
    public ToegangControleur() {
    }
    
    //Constructor welke het aangemaakte een naam geeft
    public ToegangControleur( String naam ) throws NullPointerException{
     
      //We willen geen nullwaarde als invoer
      if( naam == null ){
            throw new NullPointerException("Naam kan niet null zijn");
      }else{
        this.naam = naam;
      }
    }
    
    public ToegangControleur( String naam, String wachtwoord ) throws NullPointerException{
     
      //We willen geen nullwaarde als invoer
      if( naam == null ){
            throw new NullPointerException("Naam kan niet null zijn");
      //We willen geen nullwaarde als invoer
      }else if( wachtwoord == null ){
            throw new NullPointerException("Wachtwoord kan niet null zijn");
      }else{
        this.naam    = naam;
        this.wachtwoord = wachtwoord;
      }
    }

    public String getNaam( ){
        return naam;
    }
    
    public String getWachtwoord( ){
        return wachtwoord;
    }
    
    public boolean getToegang( ){
      return toegang;
    }
    
    public void setNaam( String naam ){
      
      if( naam == null ){
            throw new NullPointerException("Naam kan niet null zijn");
      }else{
        this.naam = naam;
      }
    }
    
    public void setWachtwoord( String wachtwoord ){
      
      if( wachtwoord == null ){
            throw new NullPointerException("Wachtwoord kan niet null zijn");
      }else{
        this.wachtwoord = wachtwoord;
      }
    }
    
    public ???? setToegang( ){
        
      // Deze moet nog gecoded worden
    }
}

probleem
Het gaat om dit request. Als hij vraagt om toegang, verbind hij met de db (dit moet nog via een class) en query't de db of de persoon toegang heeft en welk level dan wel niet.
Maar hoe weet de applicatie nou welk soort object hij moet gaan aanmaken ( evt. terugkrijgt )

Ik had de volgende oplossingen bedacht:

- De methode kan een object returnen van het juiste type. Dit zou ik een mooie oplossing vinden, immers dan zit alle logica in de ToegangControleur class. Echter hoe weet de applicatie dan wel object hij gaat returnen?
- De methode returned een string. Met die string kan de applicatie de juiste handel aan gaan maken, maar dan moet het wel als een string in de db opgeslagen worden, wat ik niet ontzettend mooi vind ( ivm zpellingsfouten )
- Een andere oplossing waar ik maar niet op kan komen.

Wat zou het mooiste zijn? Hoe doen jullie dit allemaal (want inloggen in een applicatie is toch een doodnormale zaak)? zit er een gruwelijke fout in het ontwerp? Is mijn codingstyle klote?

Google en de search hebben me in ieder geval niet kunnen helpen, maar ik vermoed dat het toch niet zo moeilijk moet zijn.
Alvast bedankt eenieder die de moeite neemt om een reply te typen.

[wasigh]

Je laat je methode een object van het type persoon retourneren, Maar je maakt wel het object van het jsuite type aan.

daarna kun je met instanceof kijken of de persoon een docent of een leerling is.

[D2k]

inhoudelijk ff nix
maar wel over je coding style

code:

public class ToegangControleur 
{
    
    private String  naam = "";
    private String  wachtwoord = "";
    private boolean     toegang = false;
    
    // Default constructor, het aangemaakte zal geen waardes bevatten; 
    public ToegangControleur() 
    {
    }
    
    //Constructor welke het aangemaakte een naam geeft
    public ToegangControleur( String naam ) throws NullPointerException
    {
    
        //We willen geen nullwaarde als invoer
        if( naam == null )
        {
              throw new NullPointerException("Naam kan niet null zijn");
        }
        else
        {
                this.naam = naam;
        }
    }
    
    public ToegangControleur( String naam, String wachtwoord ) throws NullPointerException
    {
    
        //We willen geen nullwaarde als invoer
        if( naam == null )
        {
              throw new NullPointerException("Naam kan niet null zijn");
            //We willen geen nullwaarde als invoer
        }
        else if( wachtwoord == null )
        {
              throw new NullPointerException("Wachtwoord kan niet null zijn");
        }
        else
        {
                this.naam    = naam;
                this.wachtwoord = wachtwoord;
        }
    }
    
    public String getNaam( )
    {
            return naam;
    }
    
    public String getWachtwoord( )
    {
            return wachtwoord;
    }
    
    public boolean getToegang( )
    {
        return toegang;
    }
    
    public void setNaam( String naam )
    {
        if( naam == null )
        {
              throw new NullPointerException("Naam kan niet null zijn");
        }
        else
        {
                this.naam = naam;
        }
    }
    
    public void setWachtwoord( String wachtwoord )
    {
    
        if( wachtwoord == null )
        {
              throw new NullPointerException("Wachtwoord kan niet null zijn");
        }
        else
        {
                this.wachtwoord = wachtwoord;
        }
    }
    
    public ???? setToegang( )
    {
        // Deze moet nog gecoded worden
    }
}

zo zie ik het graag

[Glimi]

Op maandag 01 april 2002 15:08 schreef wasigh het volgende:
Je laat je methode een object van het type persoon retourneren, Maar je maakt wel het object van het jsuite type aan.

daarna kun je met instanceof kijken of de persoon een docent of een leerling is.

Dus in ToegangControleur:

code:

    public Persoon setToegang( ){ // nog ff mooiere naam zoeken want deze klopt niet
        
        //Vrolijke query enzo!

        switch( rechtenVeldUitDeDB ){
      
             case 1:
                 Leerling returnLeerling = new Leering( info uit de db );
                 return returnLeerling;
             case 2:
                 Mentor returnMentor = new Mentor( info uit db );
                  return returnMentor;
             }
    }

Maar als ik dan in de MainClass (die het object dus 'ontvangt') ontvang en controleer op het type class in een if, dan ben ik de instantie toch kwijt? Immers ik heb nog geen object om het in op te slaan.
Maar je zal vast gelijk hebben, dus ik ben even googelen

[edit] Bedankt D2k Ik zal in het vervolg goed letten op tabs ( en m'n haakjes anders zetten )

[Alarmnummer]

Op maandag 01 april 2002 15:14 schreef D2k het volgende:
inhoudelijk ff nix
maar wel over je coding style
zo zie ik het graag

Ik gebruik dezelfde stijl als Glimi en die bevalt me uitstekend. Ik heb vroeger altijd jouw stijl gebruikt, maar je bent dan onnodig veel regels kwijt.

[D2k]

Op maandag 01 april 2002 16:20 schreef Alarmnummer het volgende:

[..]

Ik gebruik dezelfde stijl als Glimi en die bevalt me uitstekend. Ik heb vroeger altijd jouw stijl gebruikt, maar je bent dan onnodig veel regels kwijt.

onnodig? tegenwoordig maakt dat echt nix meer uit hoor

[Alarmnummer]

code:

public void setNaam(String naam){
   if(naam == null){
     throw new NullPointerException("naam can`t be null");
   }
   _naam = naam;
}

public void setNaam(String naam)
{
    if(naam == null)
    {
     throw new NullPointerException("naam can`t be null");  
    }
    _naam = naam;
}

Ik ben de bovenste veel handiger omdat je niet zoveel ruimte verspilt met een regel met alleen een accolade. Het is trouwens de sun huis stijl (niet dat dat veel uitmaakt).

Het is trouwens iets persoonlijk. Je moet doen wat je het beste vind werken. En als iemand zijn layout me niet aanstaat, dan gooi ik er gewoon een code beautifier overheen

[D2k]

Op maandag 01 april 2002 16:27 schreef Alarmnummer het volgende:
Ik ben de bovenste veel handiger omdat je niet zoveel ruimte verspilt met een regel met alleen een accolade. Het is trouwens de sun huis stijl (niet dat dat veel uitmaakt).

Het is trouwens iets persoonlijk. Je moet doen wat je het beste vind werken. En als iemand zijn layout me niet aanstaat, dan gooi ik er gewoon een code beautifier overheen

ach ja dat kan :
maar hij sprong ook nog eens niet consequent in
en das wel kwalijk

[Glimi]

Op maandag 01 april 2002 16:27 schreef Alarmnummer het volgende:

code:

public void setNaam(String naam){
   if(naam == null){
     throw new NullPointerException("naam can`t be null");
   }
   _naam = naam;
}
[knip]

* Glimi noteert, klassevars laten beginnen met een _

[wasigh]

Op maandag 01 april 2002 16:38 schreef Glimi het volgende:

[..]

* Glimi noteert, klassevars laten beginnen met een _

dat vindt ik zo lelijk

[Alarmnummer]

Op maandag 01 april 2002 15:08 schreef wasigh het volgende:
Je laat je methode een object van het type persoon retourneren, Maar je maakt wel het object van het jsuite type aan.

daarna kun je met instanceof kijken of de persoon een docent of een leerling is.

Je kan ook gebruik maken van een visitor

code:

public interface PersoonVisitor{
    public void visitDocent(Docent docent);
    public void visitLeerling(Leerling leerling);
}

public class PersoonVisitorAdapter implements PersoonVisitor{
    public void visitDocent(Docent docent){}
    public void visitLeerling(Leerling leerling){}
}

abstract public class Persoon{
    
    .... alle methoden en variablen.

    
    abstract public void accepts(PersoonVisitor persoonVisitor);    
}

public class Docent extends Persoon{
    
    public void accepts(PersoonVisitor persoonVisitor){
        if(persoonVisitor == null){
            throw new NullPointerException("persoonVisitor can`t be null");     
        }   
        persoonVisitor.visitDocent(this);
    }   
}

public class Leerling extends Persoon{
    
    public void accepts(PersoonVisitor persoonVisitor){
        if(persoonVisitor == null){
            throw new NullPointerException("persoonVisitor can`t be null");     
        }   
        persoonVisitor.visitLeerling(this);
    }   
}


class StrafwerkGever extends PersoonVisitorAdapter{
    public void visitLeerling(Leerling leerling){
        if(leerling == null){
            throw new NullPointerException("leerling can`t be null");       
        }   
        
        leerling.geefStrafwerk();   
    }
}

Persoon persoon = ....
persoon.accepts(new StrafwerkGever());

Vooral handig als je een grote hoeveelheid soorten classen hebt.

Op maandag 01 april 2002 16:53 schreef wasigh het volgende:

[..]

dat vindt ik zo lelijk

Wat heb je erop tegen? Op welke manier maak je onderscheid tussen je member variablen en je locale variablen? Of maak je geen verschil? Ik vind het echt super!

[wasigh]

Niet hij weer met zijn visitor aargh!!

[Alarmnummer]

Ik zou trouwens maar 1 keer doe toeganscontroleur aanmaken (eventueel een Singleton) en daar een factory method

code:

Persoon haalPersoon(String loginName, String password){
}

In plaatsen. Het heeft geen zin om iedere keer een nieuwe toegangscontroleur aan te maken.

tips:
zorg ervoor dat je geen wildgroei van constructoren krijgt. Een extreem goeie regel: KISS (keep it simple stupid )
Hou je object zo eenvoudig mogelijk en probeer er ook zo weinig mogelijk setters in te zetten (dit ivm constistentie).

[Glimi]

Op maandag 01 april 2002 17:08 schreef Alarmnummer het volgende:
Ik zou trouwens maar 1 keer doe toeganscontroleur aanmaken (eventueel een Singleton) en daar een factory method

code:

Persoon haalPersoon(String loginName, String password){
}

In plaatsen. Het heeft geen zin om iedere keer een nieuwe toegangscontroleur aan te maken.

Helemaal begrepen en logisch ook.

tips:
zorg ervoor dat je geen wildgroei van constructoren krijgt. Een extreem goeie regel: KISS (keep it simple stupid )
Hou je object zo eenvoudig mogelijk

Hmzjah, hoewel het moeilijk is , is het wel slim om te doen.

en probeer er ook zo weinig mogelijk setters in te zetten (dit ivm constistentie).

Waarom? Je hoeft ze niet allemaal public te maken, maar doordat ik voor elke classvar een setter en een getter maak, houd ik die logica op één plek. Maarja in deze class staan ze allemaal public en dat hoeft iid niet

[Alarmnummer]

Het is helemaal geen gek idee om alle instellingen naar variablen via een setter te laten verlopen, maar het publiekelijk maken is een ware nachtmerrie.. Met name het onderhoud ervan, want als het in je systeem zit dan kom je er niet altijd meer vanaf En soms wordt het door die extra setters lastiger om je object ten alle tijden consistent te houden.

een tip:
wees streng en hou je api zo simpel mogelijk.

[Glimi]

Ten eerste: sorry voor de kick, maar ik wou graag mijn oplossing posten voor onze search. Owjah, wou ook nog even laten zien dat ik geluisterd heb Het is dus een uitwerking van de post van wasigh. Ook heb ik nog wat gehakt in de code.

Dit is de base class voor de verschillende soorten mensen die kunnen inloggen met verschillende soorten rechten

code:

public class Persoon {

    // De klasse variabelen
    protected int          _leeftijd   = 0;
    protected String        _naam    = "";
    
    /** Constructor van de Persoon class
     *
     *  Input : Een string met de naam van de persoon
     *      Een positieve integer met de leeftijd van de persoon (in jaren)
     *
     *  Output: Een volledig geinitialiseerd Persoon.
     *
     *  Note:   De constructor throwt een NullPointerException mocht de input nullwaardes bevatten  
     */ 
    public Persoon( String naam, int leeftijd ) throws NullPointerException{
      
        if( naam == null ){         
            throw new NullPointerException("Naam kan niet null zijn");
             
        }else{
            
            _naam    = naam;
            _leeftijd   = leeftijd;
            
        }
    }

    /** Setter van de leeftijd variable
     *
     *  Input:  Een positieve integer met de leeftijd van de persoon
     *
     *  Output: De leeftijd van de persoon wordt de inputwaarde
     *
     *  Note:   Deze methode is alleen voor intern gebruik beschikbaar
     **/
    protected void setLeeftijd( int leeftijd ){
      
        _leeftijd = leeftijd;
    }
    
    /** Setter van de naam variabele
     *
     *  Input:  Een String met de naam van de persoon
     *
     *  Output: De naam van de persoon wordt de inputwaarde
     *
     *  Note:   Deze methode kan een NullpointerException throwen, mocht de input null zijn
     *      Deze methode is alleen voor intern gebruik beschikbaar
     **/
    protected void setNaam( String naam ) throws NullPointerException{
      
        if( naam == null){          
            throw new NullPointerException("Naam kan niet null zijn");

        }else{
            _naam = naam;

        }
    }
   
    /** Getter van de leeftijd variabele
     *
     *  Output: return de leeftijdswaarde van de persoon ( een positieve integer )
     **/
    public int getLeeftijd( ){
        return _leeftijd;
        
    }
   
    /** Getter van de naam variabele
     *
     *  Output: return de naam van de persoon ( een String )
     **/
    public String getNaam( ){     
        return _naam;      
        
    }
     
    /** String representatie van het object
     *
     *  Output: Een String vertegenwoordiging van het persoon object
     **/
    public String toString( ){
        return( "Persoon " + getNaam( ) + " is " + getLeeftijd( ) );
        
    }
}

Van de class Persoon leid ik dan een Leerling af. Een leerling heeft het minste rechten in deze applicatie. Mentor is soortgelijk, dus post ik deze niet.

code:

public class Leerling extends Persoon {

    protected int    _leerlingnummer = 0;
    protected String    _klas        = "";
    
    /** Nu ff geen zin in commentaar :)
     */
    public Leerling( String naam, int leeftijd, int leerlingnummer, String klas ){
   
        super( naam, leeftijd );
        
        if( klas == null ){
            throw new NullPointerException("Klas kan niet null zijn");
        
        }else{          
            _leerlingnummer     = leerlingnummer;
            _klas          = klas;
            
        }       
    }
    
    public int getLeerlingnummer( ){    
      
        return _leerlingnummer;
        
    }
    
    public String getKlas( ){
      
        return _klas;
        
    }
    
    protected void setLeerlingnummer( int leerlingnummer ){
        
        _leerlingnummer = leerlingnummer;
      
    }
    
    protected void setKlas( String klas ){
      
        if( klas == null ){         
            throw new NullPointerException("Klas kan niet null zijn");
        
        }else{          
            _klas = klas;
            
        }
    }

    public String toString( ){     
      
        return( "Leerling #" + getLeerlingnummer() );
        
    }
}

Nu komt de class toegangControleur. Deze klass query't de db en kijkt of de ingevulde gegevens genoeg zijn voor toegang. Is dat zo, dan wordt een object gemaakt met de juiste rechten ( leerling of mentor )

code:

    public ToegangControleur( String naam, String wachtwoord ) throws NullPointerException{
     
        //We willen geen nullwaarde als invoer
        if( naam == null ){
              throw new NullPointerException("Naam kan niet null zijn");
        //We willen geen nullwaarde als invoer
        }else if( wachtwoord == null ){
              throw new NullPointerException("Wachtwoord kan niet null zijn");
        }else{
            _naam    = naam;
            _wachtwoord = wachtwoord;
        }
    }
    
    //standaard getter
    public String getNaam( ){
      
        return _naam;
        
    }
    
    //standaard getter
    public String getWachtwoord( ){
      
        return _wachtwoord;
        
    }
    
    //standaard getter
    public boolean getToegang( ){
      
        return _toegang;
        
    }
    
    //standaard setter, alleen protected omdat we het niet voor iedereen bereikbaar willen laten zijn
    protected void setNaam( String naam ){
      
        if( naam == null ){
            throw new NullPointerException("Naam kan niet null zijn");
        }else{
            _naam = naam;
        }
    }
    
    //standaard setter, alleen protected omdat we het niet voor iedereen bereikbaar willen laten zijn
    protected void setWachtwoord( String wachtwoord ){
      
        if( wachtwoord == null ){
            throw new NullPointerException("Wachtwoord kan niet null zijn");
        }else{
            _wachtwoord = wachtwoord;
        }
    }
    
    //standaard setter, alleen protected omdat we het niet voor iedereen bereikbaar willen laten zijn
    protected void setToegang( boolean toegang ){
        
        _toegang = toegang;
    }
    
    // Hier gaan we aan het object vragen of men toegang heeft met de ingevulde gegevens
    // Als dat zo is returnen we een persoonobject van de juiste classe (mentor, leerling ed.)
    public Persoon requestToegang( ){
      
        //Hier komen allemaal interessante Query's die kijken of de ww's kloppen en welke 
        //status de persoon heeft (mentor of leerling, of helemaal geen toegang!
        
        
        if( deDbZegtDatIeToegangHeeft == true ){
            
            setToegang( true );
            
            if( dbZegtDatHetEenMentorIs == true ){
              
              Mentor returnMentor = new Mentor( naam, leeftijd, klas );
              return( returnMentor );
              
            }else{ // Als het geen mentor is, dan is het een leerling ( want die heeft de minste rechten )
              
              Leerling returnLeerling = new Leerling( naam, leeftijd, leerlingnummer, klas );
              return( returnLeerling );
            }
            
        }else{ //Ow geen toegang? Dan geen toegang dude!
            
            return null;
        }
        
         return null;
    }
      
}

Hieronder de implementatie hoe we kunnen checken wat de toegangsControleur teruggaf (wat voor'n object), zodat we die kunnen gebruiken in de rest van de app.

code:

      ToegangControleur toegangControle = new ToegangControleur("henk", "wachtwoord");
      Persoon notCheckedPerson = toegangControle.requestToegang();
      
      if( notCheckedPerson == null ){
            System.out.println("Uhm geen toegang");
      
      }else if( notCheckedPerson instanceof Leerling ){
            System.out.println("Leerling!");
      }else{
            System.out.println("Geen Leerling!");
      }

Dank voor uw aandacht

[Alarmnummer]

Het ziet er erugh goed uit, goeie layout, veel checks. Ik heb er even naar gekeken met een behoorlijk streng oog, en ik heb wel een aantal punten gevonden waarover ik iets had op te merken. Ik hoop dat je het kan warderen als opbouwende kritiek.

-voor persoon zou ik een abstracte class maken omdat er bij jou geen personen aangemaakt kunnen worden.

-geen protected variablen gebruiken omdat je niet kan garanderen dat een overerfende class er goed mee omgaat.

-een string gebruiken voor een klas is mwah. Je kan beter een class klas maken ofzo, omdat je met een string niet veel kan doen en omdat je een stukje redundantie in je data krijgt. Als de klas heet:"klas 4" en deze klas moet later heten klas 4a dan heb je een probleem omdat door het hele systeem door te voeren, want je zult overal de string klas4 moeten vervangen. (en keuzes maken op strings is altijd beetje eng).

-is het bij de toegangscontrolleur nodig dat iedereen zijn password na afloop kan uitlezen? Dit ivm andere programmeurs die ook eventueel aan dit project mee werken en wel eens andere leuke dingen met deze gegevens konden doen. Probeer pas getters en setters te maken als het nodig is. Hierdoor blijft je api eenvoudiger en veiliger.

-if(deDbZegtDatIeToegangHeeft == true) vervang ik meestal tot: if(deDbZegtDatIeToegangHeeft) omdat dit veel duidelijker is. (Dit geld voor een aantal)

-ik zou ook dit 'return( returnMentor );' vervangen door 'return returnMentor; omdat die haakjes geen toegevoegde waarde hebben en ik in eerste instantie het gevoel had dat ik met een functie te maken had ipv een return value. Hierdoor is je code minder snel te begrijpen.

-ik zou ook geen ongebruikte tijdelijke variable maken, je code wordt hierdoor weer langer, dus minder goed te begrijpen.
dus ipv

code:

    Mentor returnMentor = new Mentor( naam, leeftijd, klas );
    return( returnMentor );

code:

    return new Mentor( naam, leeftijd, klas );

-ik zou ook maar 1 return statement gebruiken aan het einde van de methode. Hierdoor kan je ook beter zien wat er allemaal gebeurd. Ik zou hiervoor de lokale variable naam:'result' gebruiken (dus mij iedere functie).

-je weet dat alle mentoren en leerlingen door de Toegangscontrolleur worden gemaakt, dus eigelijk weet je dat als je een niet null waarde terug krijgt dat diegene toegang had en bij een null niet. Ik zou die toegang variable gewoon laten vervallen, want dat scheelt je weer een methode (dus complexiteit) die niets toevoegd aan je ontwerp. Je zou er eventueel ook voor kunnen kiezen om Leerling en Mentor in dezelfde package te plaatsen en ze een friendly (dus geen) access modifier te geven in de constructor. Hierdoor kunnen alleen objecten in dezelfde package ze aanmaken (de ToegangsControleur bv).

-Ik neem aan dat je nog een aantal argumenten moet opnemen in je: 'public Persoon requestToegang( )' methode omdat je anders niet kan opgeven wie je wilt ophalen. Ik zou daarvoor een primare sleutel gebruiken, een sofiNr bv.

-als we kijken naar het volgende:

code:

      Persoon notCheckedPerson = toegangControle.requestToegang();
      
      if( notCheckedPerson == null ){
            System.out.println("Uhm geen toegang");
      
      }else if( notCheckedPerson instanceof Leerling ){
            System.out.println("Leerling!");
      }else{
            System.out.println("Geen Leerling!");
      }

Dan zie ik hier iets vrij lelijks staan. Vooral als je meerde klassen begint te krijgen dan kan dit een hele akelige if else contstructie gaan worden. Dit kan je oplossen met het polymorfisme (dus bij persoon een methode 'drukSoort' af ofzo en die bij Mentor en Leerling imlementeren. Maar als je je object niet wilt vervuilenen met allerlei handige functies kan je ook het visitor design pattern hier voor gebruiken.

vb:

code:

public interface PersoonVisitor{
    public void visit(Mentor mentor);
    public void visit(Leerling leerling);
}

public class PersoonVisitorAdapter implements PersoonVisitor{
    public void visit(Mentor mentor){};
    public void visit(Leerling leerling){};
}

abstract public class Persoon{
     ...
     
    abstract public void visit(PersoonVisitor persoonVisitor);
}

public class Mentor extends Persoon{
    ...
    public void visit(PersoonVisitor persoonVisitor){
        if(persoonVisitor == null){
            throw new NullPointerException("persoonVisitor can`t be null");         
        }   
        persoonVisitor.visit(this);
    }
}

public class Leerling extends Persoon{
    ...
    public void visit(PersoonVisitor persoonVisitor){
        if(persoonVisitor == null){
            throw new NullPointerException("persoonVisitor can`t be null");         
        }   
        persoonVisitor.visit(this);
    }
}


en dan kan je nu een soort afdrukker maken:
class SoortAfdrukken implements PersoonVisitor{
    public void visit(Mentor mentor){
        if(mentor == null){
            throw new NullPointerException("mentor can`t be null");     
        }
        System.out.println("het is een vervelende mentor"); 
    }
    
    public void visit(Leerling leerling){
        if(leerling == null){
            throw new NullPointerException("leerling can`t be null");       
        }       
        System.out.println("het is een leerling!!!!");
    }
}

een aanroepen met:
notCheckedPerson.accepts(new SoortAfdrukker());

Hierdoor kan je veel nieuwe functionaliteit toevoegen aan je objecten zonder dat je objecten vervuild raken door allerlei handige functies en het is veel veiliger dan een dikke if else reeks op basis van instanceof. Je kunt de visitor design pattern ook perfect gebruiken en complexe object verzamelingen (trees bv) om daar nieuwe functionaliteit aan toe te voegen. Ik ben dus zwaar verliefd op de visitor design pattern, zie signature

Maar afgezien deze punten van kritiek ziet je code er goed uit.

[edit] typo`s en foutje.

[Glimi]

Op zaterdag 06 april 2002 10:15 schreef Alarmnummer het volgende:
Het ziet er erugh goed uit, goeie layout, veel checks. Ik heb er even naar gekeken met een behoorlijk streng oog, en ik heb wel een aantal punten gevonden waarover ik iets had op te merken. Ik hoop dat je het kan warderen als opbouwende kritiek.

Natuurlijk! Ik ben alleen maar blij met kritiek. Ik wil je dan ook danken dat je me zo veel geholpen hebt tot nu toe!

-voor persoon zou ik een abstracte class maken omdat er bij jou geen personen aangemaakt kunnen worden.

Dat is zeker een goed idee ja. Alleen krijg ik dan problemen met het opvangen in de mainclass ( de 'persoon' die geretourneerd wordt door toegangControleur ) en moet ik gaan opvangen in een Object, of zie ik dat verkeerd?

-geen protected variablen gebruiken omdat je niet kan garanderen dat een overerfende class er goed mee omgaat.

Is er wel een manier om dit te garanderen? Ik zal er iig nog even naar kijken

-een string gebruiken voor een klas is mwah. Je kan beter een class klas maken ofzo, omdat je met een string niet veel kan doen en omdat je een stukje redundantie in je data krijgt. Als de klas heet:"klas 4" en deze klas moet later heten klas 4a dan heb je een probleem omdat door het hele systeem door te voeren, want je zult overal de string klas4 moeten vervangen. (en keuzes maken op strings is altijd beetje eng).

Hier heb je 200% gelijk. Ik zal het gelijk gaan veranderen.

-is het bij de toegangscontrolleur nodig dat iedereen zijn password na afloop kan uitlezen? Dit ivm andere programmeurs die ook eventueel aan dit project mee werken en wel eens andere leuke dingen met deze gegevens konden doen. Probeer pas getters en setters te maken als het nodig is. Hierdoor blijft je api eenvoudiger en veiliger.

Ik wil eigenijk wel overal getters en setters voor maken, zodat ik die logica in die methode kan houden, en ook bijvoorbeeld makkelijk kan wisselen van variablenamen. Maar ik moet idd beter gaan opletten wat ik toegangelijk maak voor wie.
Trouwens ik begin er aan te denken om die toegansControleur class weer (jah nog een keer) om te gooien. Opslaan van wachtwoorden en loginnames is gewoon niet wat je wilt imho. Dus het lijkt me beter om hem te initializen met een db connection en hem dan 2 public methodes te geven.

code:

public Persoon requestToegang( String loginnaam, String password ){ }

public boolean getToegang( ){ }

Dan wel met implementatie iig

-if(deDbZegtDatIeToegangHeeft == true) vervang ik meestal tot: if(deDbZegtDatIeToegangHeeft) omdat dit veel duidelijker is. (Dit geld voor een aantal)

Dit doe ik normaal ook (en waarom nu niet dan?). Ik had even dit als pseudocode in elkaar gehangen (zie je hopelijk wel aan die var naam en wou even duidelijk maken dat het een boolean was.

-ik zou ook dit 'return( returnMentor );' vervangen door 'return returnMentor; omdat die haakjes geen toegevoegde waarde hebben en ik in eerste instantie het gevoel had dat ik met een functie te maken had ipv een return value. Hierdoor is je code minder snel te begrijpen.

Hmmmz, als jij het zegt. Ik zie het meestal wel, maar dat komt ook omdat de IDE return zo'n mooi kleurtje geeft

-ik zou ook geen ongebruikte tijdelijke variable maken, je code wordt hierdoor weer langer, dus minder goed te begrijpen.
dus ipv

code:

    Mentor returnMentor = new Mentor( naam, leeftijd, klas );
    return( returnMentor );

code:

    return new Mentor( naam, leeftijd, klas );

Understood, is een gewoonte van me die ik ooit heb aangeleerd, maar nu ik hem gewoon als 1 liner zie, vind ik hem ook wat mooier

-ik zou ook maar 1 return statement gebruiken aan het einde van de methode. Hierdoor kan je ook beter zien wat er allemaal gebeurd. Ik zou hiervoor de lokale variable naam:'result' gebruiken (dus mij iedere functie).

Het is inderdaad een slechte stijl. Er moet één entry zijn en één exit. Maar soms coded het wel lekker als je aan het begin al bij een check al ziet dat bijv de linked list geen entry's bevat, om dan direct 0 te returnen bij een NULL waarde op de headPointer als je functie de lengte van de list moet leveren ( uhm ff een C++ voorbeeld )

Maar in dit verhaal had het makkelijk met 1 return gekunt, zal het zo fixxen

-je weet dat alle mentoren en leerlingen door de Toegangscontrolleur worden gemaakt, dus eigelijk weet je dat als je een niet null waarde terug krijgt dat diegene toegang had en bij een null niet. Ik zou die toegang variable gewoon laten vervallen, want dat scheelt je weer een methode (dus complexiteit) die niets toevoegd aan je ontwerp. Je zou er eventueel ook voor kunnen kiezen om Leerling en Mentor in dezelfde package te plaatsen en ze een friendly (dus geen) access modifier te geven in de constructor. Hierdoor kunnen alleen objecten in dezelfde package ze aanmaken (de ToegangsControleur bv).

Nu je het zegt Maak er dan maar 1 public functie van in ToegangsControleur

Over die friendly modifier, dat is een goed punt. Ik wil niet dat iemand opeens een Mentor gaat aanmaken ergens en dan een boel rechten extra krijgt.

-Ik neem aan dat je nog een aantal argumenten moet opnemen in je: 'public Persoon requestToegang( )' methode omdat je anders niet kan opgeven wie je wilt ophalen. Ik zou daarvoor een primare sleutel gebruiken, een sofiNr bv.

Jupz als ik m'n ontwerp omgooi, zoals hierboven omschreven komen er een login en ww bij. Dat moet voldoende zijn om de persoon te indentificeren (daar zorgt de db dan maar voor ). Het luistert ook allemaal niet _te_ nauw, omdat het maar voor school is ( uhm huiswerk vriendin, 6de VWO )

-als we kijken naar het volgende:

code:

      Persoon notCheckedPerson = toegangControle.requestToegang();
      
      if( notCheckedPerson == null ){
            System.out.println("Uhm geen toegang");
      
      }else if( notCheckedPerson instanceof Leerling ){
            System.out.println("Leerling!");
      }else{
            System.out.println("Geen Leerling!");
      }

Dan zie ik hier iets vrij lelijks staan. Vooral als je meerde klassen begint te krijgen dan kan dit een hele akelige if else contstructie gaan worden. Dit kan je oplossen met het polymorfisme (dus bij persoon een methode 'drukSoort' af ofzo en die bij Mentor en Leerling imlementeren.

Ik zie niet in wat het verschil zou zijn tussen de check op een string, en op de class. Tevens is het _niet_ zo'n heel groot probleem, omdat er in totaal 3 classes komen. Plus dat het relatief simpel moet zijn. Verder wordt _dit_ al een enorm probleem om uit te leggen aan m'n miep, het visitor design pattren zou onmogelijk zijn. Sorry.

Maar als je je object niet wilt vervuilenen met allerlei handige functies kan je ook het visitor design pattern hier voor gebruiken.

vb:

code:

public interface PersoonVisitor{
    public void visit(Mentor mentor);
    public void visit(Leerling leerling);
}

public class PersoonVisitorAdapter implements PersoonVisitor{
    public void visit(Mentor mentor){};
    public void visit(Leerling leerling){};
}

abstract public class Persoon{
     ...
     
    abstract public void visit(PersoonVisitor persoonVisitor);
}

public class Mentor extends Persoon{
    ...
    public void visit(PersoonVisitor persoonVisitor){
        if(persoonVisitor == null){
            throw new NullPointerException("persoonVisitor can`t be null");         
        }   
        persoonVisitor.visit(this);
    }
}

public class Leerling extends Persoon{
    ...
    public void visit(PersoonVisitor persoonVisitor){
        if(persoonVisitor == null){
            throw new NullPointerException("persoonVisitor can`t be null");         
        }   
        persoonVisitor.visit(this);
    }
}


en dan kan je nu een soort afdrukker maken:
class SoortAfdrukken implements PersoonVisitor{
    public void visit(Mentor mentor){
        if(mentor == null){
            throw new NullPointerException("mentor can`t be null");     
        }
        System.out.println("het is een vervelende mentor"); 
    }
    
    public void visit(Leerling leerling){
        if(leerling == null){
            throw new NullPointerException("leerling can`t be null");       
        }       
        System.out.println("het is een leerling!!!!");
    }
}

een aanroepen met:
notCheckedPerson.accepts(new SoortAfdrukker());

Hierdoor kan je veel nieuwe functionaliteit toevoegen aan je objecten zonder dat je objecten vervuild raken door allerlei handige functies en het is veel veiliger dan een dikke if else reeks op basis van instanceof. Je kunt de visitor design pattern ook perfect gebruiken en complexe object verzamelingen (trees bv) om daar nieuwe functionaliteit aan toe te voegen. Ik ben dus zwaar verliefd op de visitor design pattern, zie signature

Maar het ziet er zeker super mooi uit. Ik beloof je bij deze plechtig dat ik het goed zal gaan bestuderen en in mijn volgende (eigen) project zal proberen te implementeren.

Maar afgezien deze punten van kritiek ziet je code er goed uit.

[edit] typo`s en foutje.

Danku Jij trouwens ook heel erg bedankt voor het typen van deze enorme (lieve) reply

[Alarmnummer]

Op zaterdag 06 april 2002 11:48 schreef Glimi het volgende:

[..]

Natuurlijk! Ik ben alleen maar blij met kritiek. Ik wil je dan ook danken dat je me zo veel geholpen hebt tot nu toe!

Gelukkig

Dat is zeker een goed idee ja. Alleen krijg ik dan problemen met het opvangen in de mainclass ( de 'persoon' die geretourneerd wordt door toegangControleur ) en moet ik gaan opvangen in een Object, of zie ik dat verkeerd?

Persoon p = toegangsControlleur.requestToegang(sofinr); ofzo..

Aangezien iedereen van het basis type Persoon is, kan je hem ook in persoon opvangen.

Is er wel een manier om dit te garanderen? Ik zal er iig nog even naar kijken

Als je protected getters en setters gebruikt ipv protected variablen dan kan jij er eventueel met controles/extra functionaliteit nog even tussenin komen.
bv

code:

private static int maxLeeftijd = 0;

protected setLeeftijd(int leeftijd){
    if(leeft>maxLeeftijd){
        maxLeeftijd = leeftijd; 
    }
    _leeftijd = leeftijd;
}

Je hebt nu 1 centrale plek om het te onderhouden (setLeeftijd) en als je dan iets wil controleren dan kan je het op 1 plek doen. Dit zou je niet zo makkelijk voor elkaar krijgen als je protected variablen gaat gebruiken. En je zou zelfs kunnen zeggen dat protected variablen juist geen encapsulation bevorderen.

Als je wilt dat er hele bijzondere controles zijn die niet in setLeeftijd bij persooon meer kunnen (zie onderstaande voorbeeld voor duidelijkheid) en jij wilt toch je common entrance point houden dan zou je ook kunnen gaan voor de template method design pattern.

code:

class Persoon{
    private static int maxLeeftijd = 0; 
    
    private int leeftijd;

    final protected void setLeeftijd()throws IllegalArgumentException{
        checkLeeftijd();
        
        if(leeft>maxLeeftijd){
            maxLeeftijd = leeftijd; 
        }
        _leeftijd = leeftijd;
    
    }

    abstract protected void checkLeeftijd(int leeftijd)
}

class Mentor{

    //onzinnige controle
    protected void checkLeeftijd(int leeftijd){
        if(leeftijd>getVoornaam().length()){
            throw new IllegalArgumentException("leeftijd can`t be greater than lenght voornaam");       
        }
    }
}

class Leerling{

    //onzinnige controle
    protected void checkLeeftijd(int leeftijd){
        if(leeftijd%5==0){
            throw new IllegalArgumentException("leeftijd can`t be divideable by 5");        
        }
    }
}

en hier is even wat nutteloze tekst om de quote van de code te onderscheiden

Ik wil eigenijk wel overal getters en setters voor maken, zodat ik die logica in die methode kan houden, en ook bijvoorbeeld makkelijk kan wisselen van variablenamen.

Dat is goed (ik doe het zelf (nog niet)). Kan nog komen dat ik mij in de toekomst hier ook nog toe bekeer

Maar ik moet idd beter gaan opletten wat ik toegangelijk maak voor wie.

Tip: wees zo onvriendelijk mogelijk, dus zo weinig mogelijk publieke methodes en zo veel mogelijk checks. Uiteindelijk zal je code daardoor een stuk duidelijker worden. En je kan altijd nog een methode publieker maken maar niet andersom.

vb van een vriendelijke methode, waardoor je object inconsistent kan worden.

code:

public void setLeeftijd(int leeftijd){
    _leeftijd = leeftijd; <= leeftijd kan niet kleiner zijn dan 0, maar geen checks ed.
}

en dit is helemaal een leuke want je merkt niet dat er iets is fout gegaan.

code:

public void setLeeftijd(int leeftijd){
    if(leeftijd <0){
        _leeftijd = 0;  
    }else{
        _leeftijd = leeftijd;   
    }
}

dit is een betere versie:

code:

public void setLeeftijd(int leeftijd){
    if(leeftijd < 0){
        throw new IllegalArgumentException("leeftijd can`t be smaller than 0, leeftijd = "+leeftijd);   
    }   
    _leeftijd = leeftijd;
}

Hierdoor merk je meteen als er iets foutgaat (en dat wil je echt) en hierdoor blijft je object altijd consistent.

Trouwens ik begin er aan te denken om die toegansControleur class weer (jah nog een keer) om te gooien. Opslaan van wachtwoorden en loginnames is gewoon niet wat je wilt imho. Dus het lijkt me beter om hem te initializen met een db connection en hem dan 2 public methodes te geven.

code:

public Persoon requestToegang( String loginnaam, String password ){ }

public boolean getToegang( ){ }

Dan wel met implementatie iig

Ik zie nog steeds niet in wat de toegevoegde waarde is van die getToegang methode.

vb

code:

Persoon persoon = ToegangControlleur.requestToegang("Jan","wachtwoord");
if(persoon == null){
    //geen toegang dus
}else{
    //wel toegang dus.
}

en hier is nog meer nutteloze tekst om de quote van de code te onderscheiden

Dit doe ik normaal ook (en waarom nu niet dan?). Ik had even dit als pseudocode in elkaar gehangen (zie je hopelijk wel aan die var naam en wou even duidelijk maken dat het een boolean was.

ff voor de duidelijkheid: if(man==true) vs if(man)

Ik doe het zelf niet omdat je een conditie onnodig complexer wordt want je moet tenslotte 1 extra == operator overzien, en dus is dus onnodig toegevoegde complexiteit.

Hmmmz, als jij het zegt. Ik zie het meestal wel, maar dat komt ook omdat de IDE return zo'n mooi kleurtje geeft

Ik doe het (ook nog maar recent) omdat je nu op een centrale plek (einde) uit je methode gaat. Je zou ook nog eventueel extra checks ed (bv post condities) heel eenvoudig kunnen toevoegen. En dan kan met meerdere returns veel lastiger. Voor hele korte procedures waag ik me er ook wel eens aan, want ik heb een enorme hekel aan al dat inspringen.

Het is inderdaad een slechte stijl. Er moet één entry zijn en één exit. Maar soms coded het wel lekker als je aan het begin al bij een check al ziet dat bijv de linked list geen entry's bevat, om dan direct 0 te returnen bij een NULL waarde op de headPointer als je functie de lengte van de list moet leveren ( uhm ff een C++ voorbeeld )

Ik doe het ook wel eens hoor

Jupz als ik m'n ontwerp omgooi, zoals hierboven omschreven komen er een login en ww bij. Dat moet voldoende zijn om de persoon te indentificeren (daar zorgt de db dan maar voor ). Het luistert ook allemaal niet _te_ nauw, omdat het maar voor school is ( uhm huiswerk vriendin, 6de VWO )

Jouw vriendin die krijgt java programmeren op het vwo? Hmmm... ben ik even te vroeg geboren! En wat sinds wanneer loop jij huiswerk van je vriendin te maken? Volgens mij heeft ze je wel goed onder de duim

Ik zie niet in wat het verschil zou zijn tussen de check op een string, en op de class. Tevens is het _niet_ zo'n heel groot probleem, omdat er in totaal 3 classes komen. Plus dat het relatief simpel moet zijn. Verder wordt _dit_ al een enorm probleem om uit te leggen aan m'n miep, het visitor design pattren zou onmogelijk zijn. Sorry.

Misschien is een polymorfistische oplossing dan op zijn plek zoals ik in mijn 1e reply had gezet. Dus een abstracte getSoort methode in Persoon en die geimplementeerd in Mentor en Student. Maar als je er verder niets meer mee doet dan is het denk ik niet echt nodig.

Maar het ziet er zeker super mooi uit. Ik beloof je bij deze plechtig dat ik het goed zal gaan bestuderen en in mijn volgende (eigen) project zal proberen te implementeren.

Als je er naar gaat kijken dan moet je ook opzoek gaan naar guides. Anders moet je maar even op mijn site kijken daar staan een hele lading handige links en oa ook een online design patterns boek voor java. Er komen binnenkort nog een aantal andere links over design patterns bij op. Als je het echt leuk gaat vinden (design patterns zijn echt fantastisch... een hele nieuwe abstractie laag erbij) dan moet je voor 'Design Patterns: Elements of Reusable Object Oriented Software" van GoF gaan. Echt een vette aanrader.

Danku Jij trouwens ook heel erg bedankt voor het typen van deze enorme (lieve) reply

Ik vind het irritant als niemand je iets kan leren, en loop je maar allerlei eigen conventies te maken en eigelijk 10.000 keer het wiel opnieuw uit te vinden. Ik heb gelukkig veel gehad een martin en ben verder ook veel aan het lezen over refactoring en design patterns om mijn code kwaliteit maar te verbeteren. Hierdoor hoef ik me minder bezig te houden met allerlei triviale dingen en kan ik me echt bezig houden met het daadwerkelijke probleem ipv de problemen van de prog taal. Kortom: graag gedaan