Exceptions gooien in validatiefuncties?

[Verwijderd]

Hoi,

Stel je hebt een systeem met entiteiten (BreadRoll, CandyFloss, ik noem maar wat) en je wil de velden van die dingen controleren. Daartoe maak je per veld een validatie-object, bijvoorbeeld BreadRollWeightValidator met daarin de functie isValid().

Zou je uit isValid() een exception gooien als het mis is of zou je zoals ik nu doe eerder een FieldState object teruggeven? Een FieldState object is in feite een soort exceptie. Het heeft een $message veld en een $valid boolean veld.

Voordeel van FieldStates is vooral dat de client programmeur niet hoeft te try-catchen en direct iets in handen heeft waar-ie iets mee kan in de interface. Hij zou stuk voor stuk de velden van een BreadRoll kunnen valideren en de FieldStates verzamelen in een array die dan doorgegeven wordt naar de display laag. Of ik zou een functie kunnen aanbieden waarmee je in 1 keer een hele BreadRoll mee kan valideren, die dan een array met FieldStates teruggeeft.

De vraag komt denk ik neer op of je foute invoer van een gebruiker/client programmeur ziet als business as usual of als inderdaad een uitzondering die een exception verdient.

[RobIII]

Stel dat je exceptions gaat gooien. Dan zal de isValid functie nooit een false geven (dus feitelijk krijg je een true of een exception). Beetje vreemde manier van werken if you ask me

Verder: Exceptions zijn just that: zaken die min-of-meer niet horen op te treden. Exceptions gebruik je niet om normale flow in je programmatuur te regelen. Een exception throw je als je niet (meer) weet hoe een bepaalde state af te handelen en je geen kant meer uit kan. Je zegt feitelijk "Ik weet het niet meer Ik hoop maar dat iemand anders het voor me op lost".

Programs that use exceptions as part of their normal processing suffer from all the readability and maintainability problems of classic spaghetti code.

Code Complete (ik meen Code Compete 2) heeft een goed hoofdstuk ofzo over exceptions. Zeker eens het bekijken waard.

[ Voor 42% gewijzigd door RobIII op 03-02-2010 16:55 ]

[HuHu]

De validator geeft alleen true/false terug, dat is waar hij voor bedoelt is. Als je vervolgens een functie hebt die de data gebruikt en op dat moment blijkt dat het niet valid is, dan gooi je een Exception.

Kort voorbeeld:

code:

data = invoer gebruiker
if (isValid(data)) :
  doeIetsMetData(data)
else :
  toonMeldingAanGebruiker
endif

data2 = invoer van gebruiker
doeIetsMetData(data2)

In het eerste geval zou er nooit een Exception mogen komen bij onjuiste data, maar een foutmelding. In het tweede geval komt er wel een Exception als de data onjuist is.

[jbdeiman]

@huhu
Je wilt dat die alleen true/ false teruggeeft, maar: Je wilt soms wel iets meer gedetailleerde meldingen hebben, bijvoorbeeld:
- username is te kort
- username mag geen $ teken bevatten
- username mag niet langer zijn dan zoveel tekens.

Met alleen true/ false is dit op zich mogelijk, als je verschillende functies voor de lengte e.d. gebruikt hoor, maar you get the point denk ik..

[Verwijderd]

@RobIII: duidelijk.

@Huhu: Da's ook duidelijk, met een kanttekening: die validators kunnen verschillende messages genereren bij verschillende fouten. Het $gewicht van een broodje kan fout zijn omdat het negatief is (-16) of omdat het geen integer is (BLA). En een $displayName kan te lang zijn of vreemde tekens bevatten (H#@_!!).

Dus ik wil uit die validator wel ook een specifieke message teruggeven, vandaar dat FieldState object. Dat is in feite een boolean met een berichtje erbij.

/ Wat jbdeiman al zegt dus

[RobIII]

jbdeiman schreef op woensdag 03 februari 2010 @ 16:58:
@huhu
Je wilt dat die alleen true/ false teruggeeft, maar: Je wilt soms wel iets meer gedetailleerde meldingen hebben,

Dan moet je de validatiefunctie geen true/false laten returnen maar een ValidationResult ofzo. En niet zomaar met exceptions gaan smijten. Voor mijn part leid je daar weer een aantal objecten van af; een ValidationOKResult en een ValidationErrorResult en van die laatste leid je dan weer een ValidationMinLengthError, ValidationMaxLengthError en ValidationInvalidCharError etc. af.

[ Voor 24% gewijzigd door RobIII op 03-02-2010 17:03 ]

[muksie]

Je kunt een centraal object gebruiken voor het bijhouden van de foutmeldingen.

Je maakt bijvoorbeeld een ValidationResult klasse aan, met een addError methode. Je validators krijgen vervolgens allemaal hetzelfde ValidationResult object waar ze evt. foutmeldingen aan toevoegen. Bevat het ValidationResult object aan het eind geen errors, dan is de waarde geldig.

EDIT: Voortaan wat sneller zijn.

[ Voor 4% gewijzigd door muksie op 03-02-2010 17:03 ]

[whoami]

RobIII schreef op woensdag 03 februari 2010 @ 17:00:
[...]

Dan moet je de validatiefunctie geen true/false laten returnen maar een ValidationResult ofzo. En niet zomaar met exceptions gaan smijten. Voor mijn part leid je daar weer een aantal objecten van af; een ValidationOKResult en een ValidationErrorResult en van die laatste leid je dan weer een ValidationMinLengthError, ValidationMaxLengthError en ValidationInvalidCharError etc. af.

Samen met je eerste post helemaal hetgeen ik hier ook zou gepost hebben, maar ik was te laat.
Hmm, nu zie ik je edit ivm het inheriten.
Ik zou gewoon één 'ValidationResult' class maken die er bv zo uitziet:

code:

public class ValidationResult
{

     private readonly ReadOnlyCollection<string> _brokenRules;

    public ValidationResult( ICollection<string> brokenRules )
    { 
         _brokenRules = brokenRules;
    }

    public BrokenRules { get { return _brokenRules; } }

    public bool IsValid { get { return _brokenRules.Count == 0; } }
    
}

[ Voor 28% gewijzigd door whoami op 03-02-2010 17:09 ]

[Verwijderd]

Nou, lijkt me duidelijk dan Die ValidationResult heet bij mij dus een FieldState, maar dat mag de pret niet drukken. Bedankt.

[RobIII]

whoami schreef op woensdag 03 februari 2010 @ 17:04:
[...]
Hmm, nu zie ik je edit ivm het inheriten.
Ik zou gewoon één 'ValidationResult' class maken die er bv zo uitziet:

Dat kan ook. Boeie. Punt is dat een bool isValid() niet erg nuttig is als je meer info wil. Hoe je die info dan communiceert is een implementatiedetail.

[Grijze Vos]

Je kunt toch gewoon meerdere validators op een object gooien? .NET doet dat ook. Als een veld een geldig mail adres moet bevatten dan maak je een RequiredFieldValidator en een RegularExpressionValidator, die beiden dezelfde input valideren.

[whoami]

Grijze Vos schreef op woensdag 03 februari 2010 @ 18:32:
Je kunt toch gewoon meerdere validators op een object gooien? .NET doet dat ook. Als een veld een geldig mail adres moet bevatten dan maak je een RequiredFieldValidator en een RegularExpressionValidator, die beiden dezelfde input valideren.

Tja, maar als je het met die .NET controls doet, zit je al op je UI laag.

[HuHu]

whoami schreef op woensdag 03 februari 2010 @ 17:04:
[...]

Samen met je eerste post helemaal hetgeen ik hier ook zou gepost hebben, maar ik was te laat.
Hmm, nu zie ik je edit ivm het inheriten.
Ik zou gewoon één 'ValidationResult' class maken die er bv zo uitziet:

code:

public class ValidationResult
{

     private readonly ReadOnlyCollection<string> _brokenRules;

    public ValidationResult( ICollection<string> brokenRules )
    { 
         _brokenRules = brokenRules;
    }

    public BrokenRules { get { return _brokenRules; } }

    public bool IsValid { get { return _brokenRules.Count == 0; } }
    
}

Ik zou zelf liever gaan voor de volgende opzet:

PHP:

class Validator {
  protected $_errors;

  public function isValid($data) {
    $this->_errors = array();
    if (/* iets met data*/) {
      return true;
    } else {
      $this->_errors[] = 'Data is onjuist volgens die-en-die regel.';
      return false;
  }

  public function getErrors() {
    return $this->_errors;
  }
}

Dan kun je één validator gebruiken om meerdere dingen te valideren. Je hoeft dan niet steeds een nieuwe te constructen. Op het moment dat isValid() false terug geeft, roep je getErrors() aan. Je kunt ook nog een getMessages() maken, waarbij de getErrors error-id's terug geeft en de getMessages() geeft (vertaalde) foutmeldingen.

[mithras]

jbdeiman schreef op woensdag 03 februari 2010 @ 16:58:
@huhu
Je wilt dat die alleen true/ false teruggeeft, maar: Je wilt soms wel iets meer gedetailleerde meldingen hebben, bijvoorbeeld:
- username is te kort
- username mag geen $ teken bevatten
- username mag niet langer zijn dan zoveel tekens.

Met alleen true/ false is dit op zich mogelijk, als je verschillende functies voor de lengte e.d. gebruikt hoor, maar you get the point denk ik..

Dat zijn juist dingen die je niet in een BreadRollWeightValidator moet controleren. Als je hierop wilt controleren, zal je daarvoor validators moeten maken en die chainen. Iets wat imho wel gebruikelijk is (zie in de thread ook dat .NET het ondersteunt, ZF iig ook).

Voor een username zou je een validator kunnen maken stringLength (min=x,max=y) die gekoppeld is aan een character validator (valid='a-zA-Z0-9') bijvoorbeeld. Dan heb je jou bovenstaande gevallen al afgehandeld

Verwijderd schreef op woensdag 03 februari 2010 @ 17:00:
die validators kunnen verschillende messages genereren bij verschillende fouten. Het $gewicht van een broodje kan fout zijn omdat het negatief is (-16) of omdat het geen integer is (BLA). En een $displayName kan te lang zijn of vreemde tekens bevatten (H#@_!!).

Allereerst, zorg dat je een juiste chain maakt van validators. Natuurlijk check je per validator of je input wel geldig is (als in: een integer checken waar je een string krijgt kan niet). Dáárvoor zou ik je dan mogelijk een exception kunnen gebruiken. Maar nog steeds is het niet de mooiste manier om te doen

Hoe ZF het doet is een zeer to-the-point manier. Een NotEmpty validator bijv:

PHP:

public function isValid($value)
{
    if (!is_null($value) && !is_string($value) && !is_int($value) && !is_float($value) &&
        !is_bool($value) && !is_array($value)) {
        $this->_error(self::INVALID);
        return false;
    }

    $this->_setValue($value);
    if (is_string($value)
        && (('' === $value)
            || preg_match('/^\s+$/s', $value))
    ) {
        $this->_error(self::IS_EMPTY);
        return false;
    } elseif (is_int($value) && (0 === $value)) {
        return true;
    } elseif (!is_string($value) && empty($value)) {
        $this->_error(self::IS_EMPTY);
        return false;
    }

    return true;
}

Door de class constants zie je ook waarom het mis ging en kan je dat vervolgens weer netjes vertalen e.d.

[YopY]

Wat tot nu toe aangedragen is lijkt veel op hoe dit in het Spring framework (Java) gebeurt, zie hier, met een paar kleine punten:

* Spring voegt een 'isSupported' methode toe, zodat alle properties van een object naar de toegekende validators gestuurd kunnen worden (zonder typesafety e.d.). Dit is qua configuraties en handwerk om validatie toe te voegen makkelijker.

* Spring gebruikt een 'visitor' pattern om de foutmeldingen in op te slaan.

Spring haalt alle velden van een object door alle geregistreerde Validators, die elk een of meerdere foutmeldingen toevoegen aan het Errors object. Als één Validator een waarde afkeurt, zal dit uit het Errors object af te leiden zijn, en een foutbericht teruggeven (dat gekoppeld is aan dat veld).

En omdat er geen exceptions gegooid worden, kunnen de foutmeldingen netjes afgehandeld worden, kan bij elk veld dat de gebruiker invult een foutmelding weergegeven worden, en kan het opnieuw ingevuld worden.

Zelf zou ik wel een exception gooien indien de fout niet door een gebruiker gemaakt of gefixed kan worden, maar het gewoon verkeerd gebruik van de API betreft.