Skip to main content
Code Review

Return to Answer

replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link
added 1003 characters in body
Source Link

I've reviewed a couple of validation + data-model related questions, so I thought I'd add some of my (lengthy) reviews here, too. For completeness' sake:

I've reviewed a couple of validation + data-model related questions, so I thought I'd add some of my (lengthy) reviews here, too. For completeness' sake:

added 1003 characters in body
Source Link

Yes and no. This class has a clear task within the application: it is passed around, carrying data. It represents a number of values that belong together. It's a model. Having data containers isn't just OK, it's good practice. You can use type-hinting, and doc-blocks to make your code easier to use.
You call it a glorified struct, well: in C, a FILE * is just a struct, too, but you rarely use it as such: There's an entire API that handles the nitty-gritty for you, in the same way that you can use this model to validate the data you are setting.

Code that requires this data, either to store it in a DB, or present it to the client side in a view, can then be written with a clear contract:

public function showRecipient(Recipient $recipient)
{
 //if an instance is passed, its email address is expected to be set and valid
}

But how do you best make sure when, and where to validate the email address:

Don't. If you are processing data that belongs to, or is part of the dataset that makes up a Recipient, the thing to do is to wrap this data in the appropriate container: a Recipient instance.
If you validate the email elsewhere in your code, you are repeating yourself. Simply pour it into an object, that makes sure the data is valid, and pass that object around. Whatever values you extract from it (through getters), is guaranteed to be validated.

The validation should Not be done in the constructor. I can construct an instance, and re-assign/reset the properties to a particular value, bypassing the constructor's validation.
A separate class, then? No. Really... why would you centralize all data validation/sanitation? Such a class would be either an all-static monster, or you'd initialize all validation code, every time you want to validate a single value.

I'd make my properties protected, and use setters. These setter methods are tailored to the type of property they are setting, and have each setter sanitizecan perform the necessary sanitation and validate its particular bit of datavalidation. Simple.

Yes and no. This class has a clear task within the application: it is passed around, carrying data. It represents a number of values that belong together. It's a model. Having data containers isn't just OK, it's good practice. You can use type-hinting, and doc-blocks to make your code easier to use.
You call it a glorified struct, well: in C, a FILE * is just a struct, too, but you rarely use it as such: There's an entire API that handles the nitty-gritty for you, in the same way that you can use this model to validate the data you are setting

Don't. If you are processing data that belongs to, or is part of the dataset that makes up a Recipient, the thing to do is to wrap this data in the appropriate container: a Recipient instance.

The validation should Not be done in the constructor. I can construct an instance, and re-assign/reset the properties to a particular value, bypassing the constructor's validation. I'd use setters, and have each setter sanitize and validate its particular bit of data.

Yes and no. This class has a clear task within the application: it is passed around, carrying data. It represents a number of values that belong together. It's a model. Having data containers isn't just OK, it's good practice. You can use type-hinting, and doc-blocks to make your code easier to use.
You call it a glorified struct, well: in C, a FILE * is just a struct, too, but you rarely use it as such: There's an entire API that handles the nitty-gritty for you, in the same way that you can use this model to validate the data you are setting.

Code that requires this data, either to store it in a DB, or present it to the client side in a view, can then be written with a clear contract:

public function showRecipient(Recipient $recipient)
{
 //if an instance is passed, its email address is expected to be set and valid
}

But how do you best make sure when, and where to validate the email address:

Don't. If you are processing data that belongs to, or is part of the dataset that makes up a Recipient, the thing to do is to wrap this data in the appropriate container: a Recipient instance.
If you validate the email elsewhere in your code, you are repeating yourself. Simply pour it into an object, that makes sure the data is valid, and pass that object around. Whatever values you extract from it (through getters), is guaranteed to be validated.

The validation should Not be done in the constructor. I can construct an instance, and re-assign/reset the properties to a particular value, bypassing the constructor's validation.
A separate class, then? No. Really... why would you centralize all data validation/sanitation? Such a class would be either an all-static monster, or you'd initialize all validation code, every time you want to validate a single value.

I'd make my properties protected, and use setters. These setter methods are tailored to the type of property they are setting, and can perform the necessary sanitation and validation. Simple.

Source Link
Loading
lang-php

AltStyle によって変換されたページ (->オリジナル) /