Forms RFC: InputFilter ready to test

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

Forms RFC: InputFilter ready to test

weierophinney
Administrator
Hey, all --

I've finished work on the first of several components specified in the
Forms RFC: the InputFilter.

Work is here:

    https://github.com/weierophinney/zf2/tree/feature/forms

(clone my repo, and use the feature/forms branch)

This work includes the following objects:

 * Input (representing a single input, which composes filter and
   validation chains, some metadata, and the value)
 * InputFilter (an aggregation of Input and InputFilter objects)
 * Factory (a build for Input and InputFilter objects, based on
   array/Traversable specifications)

The best documentation at this point is the unit tests, as they show the
various use cases and patterns I was attempting to solve. There are a
few use cases/issues I identified later:

 * If the data does not have a key for a given input, and it's required
   and cannot be empty, should any messages be set in the input? If so,
   where and how?
 * Should the factory allow passing default values for inputs it
   creates?
 * Should the factory allow setting defaults to apply to all inputs
   (such as filters, validators, required/allow_empty flags, etc.)?
 * Zend_Filter_Input allowed a "fields" metadata command on an input
   that allowed indicating that the value to be evaluated should be a
   composite of several values. As an example, a "date" input might be
   the composite of "year", "month", and "day". Should I make this
   possible?

My next stop on the Forms train is the form objects themselves. Once
those are in place, I'll start working on a builder for forms and input
filters based on annotations. Finally, I'll work on view helpers.

Any testing/feedback you can provide on the InputFilter component would
be greatly appreciated!

--
Matthew Weier O'Phinney
Project Lead            | [hidden email]
Zend Framework          | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc
Reply | Threaded
Open this post in threaded view
|

Re: Forms RFC: InputFilter ready to test

Tomáš Fejfar
I've just read the thread about naming conventions :)

$input->setAllowEmpty(true);
$this->assertTrue($input->allowEmpty());

vs

$input->setValue('bar');
$this->assertEquals('bar', $input->getValue());

It's missing "getAllowEmpty". From my understanding with allowEmpty() (without "get") present should be 

$input->allowEmpty(true); //set
$this->assertTrue($input->allowEmpty(/* default=null */));  //get

My rule of thumb: where is set should be get, otherwise empty parameter returns current value

On Wed, Apr 4, 2012 at 11:58 PM, Matthew Weier O'Phinney <[hidden email]> wrote:
Hey, all --

I've finished work on the first of several components specified in the
Forms RFC: the InputFilter.

Work is here:

   https://github.com/weierophinney/zf2/tree/feature/forms

(clone my repo, and use the feature/forms branch)

This work includes the following objects:

 * Input (representing a single input, which composes filter and
  validation chains, some metadata, and the value)
 * InputFilter (an aggregation of Input and InputFilter objects)
 * Factory (a build for Input and InputFilter objects, based on
  array/Traversable specifications)

The best documentation at this point is the unit tests, as they show the
various use cases and patterns I was attempting to solve. There are a
few use cases/issues I identified later:

 * If the data does not have a key for a given input, and it's required
  and cannot be empty, should any messages be set in the input? If so,
  where and how?
 * Should the factory allow passing default values for inputs it
  creates?
 * Should the factory allow setting defaults to apply to all inputs
  (such as filters, validators, required/allow_empty flags, etc.)?
 * Zend_Filter_Input allowed a "fields" metadata command on an input
  that allowed indicating that the value to be evaluated should be a
  composite of several values. As an example, a "date" input might be
  the composite of "year", "month", and "day". Should I make this
  possible?

My next stop on the Forms train is the form objects themselves. Once
those are in place, I'll start working on a builder for forms and input
filters based on annotations. Finally, I'll work on view helpers.

Any testing/feedback you can provide on the InputFilter component would
be greatly appreciated!

--
Matthew Weier O'Phinney
Project Lead            | [hidden email]
Zend Framework          | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc

Reply | Threaded
Open this post in threaded view
|

Re: Forms RFC: InputFilter ready to test

weierophinney
Administrator
-- Tomáš Fejfar <[hidden email]> wrote
(on Thursday, 05 April 2012, 04:31 PM +0200):

> I've just read the thread about naming conventions :)
>
> $input->setAllowEmpty(true);
> $this->assertTrue($input->allowEmpty());
>
> vs
>
> $input->setValue('bar');
> $this->assertEquals('bar', $input->getValue());
>
> It's missing "getAllowEmpty". From my understanding with allowEmpty() (without
> "get") present should be
>
> $input->allowEmpty(true); //set
> $this->assertTrue($input->allowEmpty(/* default=null */));  //get
>
> My rule of thumb: where is set should be get, otherwise empty parameter returns
> current value

For boolean values, we have a history of having an explicit setter, and
then a natural language accessor. Thus, "setAllowEmpty" and "allowEmpty"
-- the former is used to set the value, the latter usually in
conditionals:

    if ($input->allowEmpty()) {
        // do something if it allows empty values
    }

Similarly, with the "required" flag, "setRequired" and "isRequired":

    if ($input->isRequired()) {
        // do something if the input is required
    }

Using this approach ensures we're not overloading a method to achieve
multiple behaviors (e.g.: allowEmpty() behaving differently if a value
is passed vs with no value).

I think this is much more readable and understandable than an explicit
"getter" method for boolean flags.


> On Wed, Apr 4, 2012 at 11:58 PM, Matthew Weier O'Phinney <[hidden email]>
> wrote:
>
>     Hey, all --
>
>     I've finished work on the first of several components specified in the
>     Forms RFC: the InputFilter.
>
>     Work is here:
>
>        https://github.com/weierophinney/zf2/tree/feature/forms
>
>     (clone my repo, and use the feature/forms branch)
>
>     This work includes the following objects:
>
>      * Input (representing a single input, which composes filter and
>       validation chains, some metadata, and the value)
>      * InputFilter (an aggregation of Input and InputFilter objects)
>      * Factory (a build for Input and InputFilter objects, based on
>       array/Traversable specifications)
>
>     The best documentation at this point is the unit tests, as they show the
>     various use cases and patterns I was attempting to solve. There are a
>     few use cases/issues I identified later:
>
>      * If the data does not have a key for a given input, and it's required
>       and cannot be empty, should any messages be set in the input? If so,
>       where and how?
>      * Should the factory allow passing default values for inputs it
>       creates?
>      * Should the factory allow setting defaults to apply to all inputs
>       (such as filters, validators, required/allow_empty flags, etc.)?
>      * Zend_Filter_Input allowed a "fields" metadata command on an input
>       that allowed indicating that the value to be evaluated should be a
>       composite of several values. As an example, a "date" input might be
>       the composite of "year", "month", and "day". Should I make this
>       possible?
>
>     My next stop on the Forms train is the form objects themselves. Once
>     those are in place, I'll start working on a builder for forms and input
>     filters based on annotations. Finally, I'll work on view helpers.
>
>     Any testing/feedback you can provide on the InputFilter component would
>     be greatly appreciated!
>    
>     --
>     Matthew Weier O'Phinney
>     Project Lead            | [hidden email]
>     Zend Framework          | http://framework.zend.com/
>     PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc
>
>

--
Matthew Weier O'Phinney
Project Lead            | [hidden email]
Zend Framework          | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc
Reply | Threaded
Open this post in threaded view
|

Re: Forms RFC: InputFilter ready to test

Tomáš Fejfar
inline

On Sat, Apr 7, 2012 at 10:09 PM, Matthew Weier O'Phinney <[hidden email]> wrote:
-- Tomáš Fejfar <[hidden email]> wrote
(on Thursday, 05 April 2012, 04:31 PM +0200):
> I've just read the thread about naming conventions :)
>
> $input->setAllowEmpty(true);
> $this->assertTrue($input->allowEmpty());
>
> vs
>
> $input->setValue('bar');
> $this->assertEquals('bar', $input->getValue());
>
> It's missing "getAllowEmpty". From my understanding with allowEmpty() (without
> "get") present should be
>
> $input->allowEmpty(true); //set
> $this->assertTrue($input->allowEmpty(/* default=null */));  //get
>
> My rule of thumb: where is set should be get, otherwise empty parameter returns
> current value

For boolean values, we have a history of having an explicit setter, and
then a natural language accessor. Thus, "setAllowEmpty" and "allowEmpty"
-- the former is used to set the value, the latter usually in
conditionals:

   if ($input->allowEmpty()) {
       // do something if it allows empty values
   }

Similarly, with the "required" flag, "setRequired" and "isRequired":

   if ($input->isRequired()) {
       // do something if the input is required
   }
That was my point actually. The problem with allowEmpty is that it starts with a verb, so that it can't be named appropriately isSomething. 

the setter/getter should be named (set|get)Property.
But when the value is boolean it should be (set|is)Property. 

But in case of allowEmpty it's (set|null)Property. Using same logic as for allowEmpty the "required" syntax would be setIsRequired / isRequired. 

Maybe it can be solved by simple renaming to EmptyAlowed => setEmptyAllowed / isEmptyAllowed. Maybe variable starting with verb sould be discouraged principle :)

Hope I clarified that :)

Using this approach ensures we're not overloading a method to achieve
multiple behaviors (e.g.: allowEmpty() behaving differently if a value
is passed vs with no value).

I think this is much more readable and understandable than an explicit
"getter" method for boolean flags.
I 100% agree, but that was not the point (see above).  


> On Wed, Apr 4, 2012 at 11:58 PM, Matthew Weier O'Phinney <[hidden email]>
> wrote:
>
>     Hey, all --
>
>     I've finished work on the first of several components specified in the
>     Forms RFC: the InputFilter.
>
>     Work is here:
>
>        https://github.com/weierophinney/zf2/tree/feature/forms
>
>     (clone my repo, and use the feature/forms branch)
>
>     This work includes the following objects:
>
>      * Input (representing a single input, which composes filter and
>       validation chains, some metadata, and the value)
>      * InputFilter (an aggregation of Input and InputFilter objects)
>      * Factory (a build for Input and InputFilter objects, based on
>       array/Traversable specifications)
>
>     The best documentation at this point is the unit tests, as they show the
>     various use cases and patterns I was attempting to solve. There are a
>     few use cases/issues I identified later:
>
>      * If the data does not have a key for a given input, and it's required
>       and cannot be empty, should any messages be set in the input? If so,
>       where and how?
>      * Should the factory allow passing default values for inputs it
>       creates?
>      * Should the factory allow setting defaults to apply to all inputs
>       (such as filters, validators, required/allow_empty flags, etc.)?
>      * Zend_Filter_Input allowed a "fields" metadata command on an input
>       that allowed indicating that the value to be evaluated should be a
>       composite of several values. As an example, a "date" input might be
>       the composite of "year", "month", and "day". Should I make this
>       possible?
>
>     My next stop on the Forms train is the form objects themselves. Once
>     those are in place, I'll start working on a builder for forms and input
>     filters based on annotations. Finally, I'll work on view helpers.
>
>     Any testing/feedback you can provide on the InputFilter component would
>     be greatly appreciated!
>
>     --
>     Matthew Weier O'Phinney
>     Project Lead            | [hidden email]
>     Zend Framework          | http://framework.zend.com/
>     PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc
>
>

--
Matthew Weier O'Phinney
Project Lead            | [hidden email]
Zend Framework          | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc

Reply | Threaded
Open this post in threaded view
|

Re: Forms RFC: InputFilter ready to test

weierophinney
Administrator
-- Tomáš Fejfar <[hidden email]> wrote
(on Sunday, 08 April 2012, 06:51 PM +0200):

> On Sat, Apr 7, 2012 at 10:09 PM, Matthew Weier O'Phinney <[hidden email]>
> wrote:
>     -- Tomáš Fejfar <[hidden email]> wrote
>     (on Thursday, 05 April 2012, 04:31 PM +0200):
>     > I've just read the thread about naming conventions :)
>     >
>     > $input->setAllowEmpty(true);
>     > $this->assertTrue($input->allowEmpty());
>     >
>     > vs
>     >
>     > $input->setValue('bar');
>     > $this->assertEquals('bar', $input->getValue());
>     >
>     > It's missing "getAllowEmpty". From my understanding with allowEmpty()
>     (without
>     > "get") present should be
>     >
>     > $input->allowEmpty(true); //set
>     > $this->assertTrue($input->allowEmpty(/* default=null */));  //get
>     >
>     > My rule of thumb: where is set should be get, otherwise empty parameter
>     returns
>     > current value
>
>     For boolean values, we have a history of having an explicit setter, and
>     then a natural language accessor. Thus, "setAllowEmpty" and "allowEmpty"
>     -- the former is used to set the value, the latter usually in
>     conditionals:
>
>        if ($input->allowEmpty()) {
>            // do something if it allows empty values
>        }
>
>     Similarly, with the "required" flag, "setRequired" and "isRequired":
>
>        if ($input->isRequired()) {
>            // do something if the input is required
>        }
>
> That was my point actually. The problem with allowEmpty is that it starts with
> a verb, so that it can't be named appropriately isSomething.

Not all booleans need to start with "is". "allowEmpty" is natural
language -- "does $input allow empty?" (well, technically, should be
"does $input allow empty values?" but I think we can get away without
the "values"). Similarly, "hasSuchAndSuch" is fine for a boolean.

> the setter/getter should be named (set|get)Property.
> But when the value is boolean it should be (set|is)Property.

I disagree, for the reasons stated above. "is" is not the only "boolean"
question we can ask. There are cases when it's inaccurate verbiage.

> But in case of allowEmpty it's (set|null)Property. Using same logic as for
> allowEmpty the "required" syntax would be setIsRequired / isRequired.

Why? The verbiage is different, even though we're still asking a boolean
question here.

> Maybe it can be solved by simple renaming to EmptyAlowed => setEmptyAllowed /
> isEmptyAllowed. Maybe variable starting with verb sould be discouraged
> principle :)

Again, I disagree.

--
Matthew Weier O'Phinney
Project Lead            | [hidden email]
Zend Framework          | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc
Reply | Threaded
Open this post in threaded view
|

Re: Forms RFC: InputFilter ready to test

H Glenn Hatfield
On Apr 9, 2012, at 8:21 AM, Matthew Weier O'Phinney wrote:
> Not all booleans need to start with "is". "allowEmpty" is natural
> language -- "does $input allow empty?" (well, technically, should be
> "does $input allow empty values?" but I think we can get away without
> the "values"). Similarly, "hasSuchAndSuch" is fine for a boolean.

I think the culprit is that 'is' and 'has' at the beginning of a phrase connote a question, while 'allow' can be used as either a question or command:
"Allow Empty!" vs "Allow Empty?"

We would normally distinguish the two phrases based off of inflection, which is not possible (yet) in code. Maybe 'allowsEmpty' could work?

H Hatfield
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: Forms RFC: InputFilter ready to test

Tomáš Fejfar
In reply to this post by weierophinney
Last try, I promise! ;) More inline. 

On Mon, Apr 9, 2012 at 4:21 PM, Matthew Weier O'Phinney <[hidden email]> wrote:
Not all booleans need to start with "is". "allowEmpty" is natural
language -- "does $input allow empty?" (well, technically, should be
"does $input allow empty values?" but I think we can get away without
the "values"). Similarly, "hasSuchAndSuch" is fine for a boolean.
I'm afraid you missed my point - which is consistency. I'll try that again. 

For boolean member variables we use is/has/allow/can. It's used in form:

setter - set + VariableName
getter - is/has/allow + VariableName

Let's set VariableName = "Required". 

Than it's setRequired / isRequired

Now let's set VariableName = 'Items'

Than it's setItems / hasItems

now let's set VariableName = 'AllowEmpty'

That's setAllowEmpty / allowAllowEmpty ?!

The naming does not follow the "verb + variableName" pattern. 

> the setter/getter should be named (set|get)Property.
> But when the value is boolean it should be (set|is)Property.

I disagree, for the reasons stated above. "is" is not the only "boolean"
question we can ask. There are cases when it's inaccurate verbiage.
Agreed. As i state above - can, allow and others are good for boolean questions too. But the variables themselves MUST NOT contain a verb. 

Bad example off the top of my head - method that return if something can be translated (let's say I can't come up with isTranslatable for the sake of the example): 

canBeTranslated() -> i expect the setter to be the getter minus the verb preppended with "set" = setBeTranslated. 

I think that every variable can be rewritten so that it does not start with a verb - as does this example = translatable - setTranslatable / isTranslatable

> But in case of allowEmpty it's (set|null)Property. Using same logic as for
> allowEmpty the "required" syntax would be setIsRequired / isRequired.

Why? The verbiage is different, even though we're still asking a boolean
question here.

Let my rephrase my argument here - isRequired => return the value of $this->_required. What value does allowEmpty return? I'd guess $this->_empty? Or not? Are "is" and "has" some kind of "lesser" words, so they are ommited from variable names and "allow" and "can" are better so they can stay? :))

> Maybe it can be solved by simple renaming to EmptyAlowed => setEmptyAllowed /
> isEmptyAllowed. Maybe variable starting with verb sould be discouraged
> principle :)

Again, I disagree.

--
Matthew Weier O'Phinney
Project Lead            | [hidden email]
Zend Framework          | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc

Reply | Threaded
Open this post in threaded view
|

Re: Forms RFC: InputFilter ready to test

weierophinney
Administrator
-- Tomáš Fejfar <[hidden email]> wrote
(on Monday, 09 April 2012, 11:58 PM +0200):

> Last try, I promise! ;) More inline.
>
> On Mon, Apr 9, 2012 at 4:21 PM, Matthew Weier O'Phinney <[hidden email]>
> wrote:
>
>     Not all booleans need to start with "is". "allowEmpty" is natural
>     language -- "does $input allow empty?" (well, technically, should be
>     "does $input allow empty values?" but I think we can get away without
>     the "values"). Similarly, "hasSuchAndSuch" is fine for a boolean.
>
> I'm afraid you missed my point - which is consistency. I'll try that again.
>
> For boolean member variables we use is/has/allow/can. It's used in form:
>
> setter - set + VariableName
> getter - is/has/allow + VariableName
>
> Let's set VariableName = "Required".
>
> Than it's setRequired / isRequired
>
> Now let's set VariableName = 'Items'
>
> Than it's setItems / hasItems
>
> now let's set VariableName = 'AllowEmpty'
>
> That's setAllowEmpty / allowAllowEmpty ?!
>
> The naming does not follow the "verb + variableName" pattern.
>
>
>     > the setter/getter should be named (set|get)Property.
>     > But when the value is boolean it should be (set|is)Property.
>
>     I disagree, for the reasons stated above. "is" is not the only "boolean"
>     question we can ask. There are cases when it's inaccurate verbiage.
>
> Agreed. As i state above - can, allow and others are good for boolean questions
> too. But the variables themselves MUST NOT contain a verb.
>
> Bad example off the top of my head - method that return if something can be
> translated (let's say I can't come up with isTranslatable for the sake of the
> example):
>
> canBeTranslated() -> i expect the setter to be the getter minus the verb
> preppended with "set" = setBeTranslated.
>
> I think that every variable can be rewritten so that it does not start with a
> verb - as does this example = translatable - setTranslatable / isTranslatable
>
>
>     > But in case of allowEmpty it's (set|null)Property. Using same logic as
>     for
>     > allowEmpty the "required" syntax would be setIsRequired / isRequired.
>
>     Why? The verbiage is different, even though we're still asking a boolean
>     question here.
>
>
> Let my rephrase my argument here - isRequired => return the value of $this->
> _required. What value does allowEmpty return? I'd guess $this->_empty? Or not?

Honestly, still not seeing the issue here.

    if ($input->allowEmpty()) {
        // branch logic
    }

I don't see the issue with setAllowEmpty() in this case; it reads fine
in terms of English. "Allow" in this case is qualifying the condition
"Empty"; neither works by itself. Together, they can be used as a
predicate + subject.

> Are "is" and "has" some kind of "lesser" words, so they are ommited from
> variable names and "allow" and "can" are better so they can stay? :))

I'm not saying they're "lesser" words; I'm saying that in the case of
flag properties, they're unnecessary verbiage, but necessary when it
comes to the method used to indicate their value. "required" as a
property name is understandable; "isRequired" as a method name indicates
via the name what sort of return you're expecting.

--
Matthew Weier O'Phinney
Project Lead            | [hidden email]
Zend Framework          | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc
Reply | Threaded
Open this post in threaded view
|

Re: Forms RFC: InputFilter ready to test

leedavis
My take..

AllowEmpty is ambiguous to me. Sounding like it could be either an instruction (set this to allow empty) or a question (does this allow empty?). 

I think the method name should be more explicit of the expected behaviour. Maybe AllowsEmpty?

On Mon, Apr 9, 2012 at 11:14 PM, Matthew Weier O'Phinney <[hidden email]> wrote:
-- Tomáš Fejfar <[hidden email]> wrote
(on Monday, 09 April 2012, 11:58 PM +0200):
> Last try, I promise! ;) More inline.
>
> On Mon, Apr 9, 2012 at 4:21 PM, Matthew Weier O'Phinney <[hidden email]>
> wrote:
>
>     Not all booleans need to start with "is". "allowEmpty" is natural
>     language -- "does $input allow empty?" (well, technically, should be
>     "does $input allow empty values?" but I think we can get away without
>     the "values"). Similarly, "hasSuchAndSuch" is fine for a boolean.
>
> I'm afraid you missed my point - which is consistency. I'll try that again.
>
> For boolean member variables we use is/has/allow/can. It's used in form:
>
> setter - set + VariableName
> getter - is/has/allow + VariableName
>
> Let's set VariableName = "Required".
>
> Than it's setRequired / isRequired
>
> Now let's set VariableName = 'Items'
>
> Than it's setItems / hasItems
>
> now let's set VariableName = 'AllowEmpty'
>
> That's setAllowEmpty / allowAllowEmpty ?!
>
> The naming does not follow the "verb + variableName" pattern.
>
>
>     > the setter/getter should be named (set|get)Property.
>     > But when the value is boolean it should be (set|is)Property.
>
>     I disagree, for the reasons stated above. "is" is not the only "boolean"
>     question we can ask. There are cases when it's inaccurate verbiage.
>
> Agreed. As i state above - can, allow and others are good for boolean questions
> too. But the variables themselves MUST NOT contain a verb.
>
> Bad example off the top of my head - method that return if something can be
> translated (let's say I can't come up with isTranslatable for the sake of the
> example):
>
> canBeTranslated() -> i expect the setter to be the getter minus the verb
> preppended with "set" = setBeTranslated.
>
> I think that every variable can be rewritten so that it does not start with a
> verb - as does this example = translatable - setTranslatable / isTranslatable
>
>
>     > But in case of allowEmpty it's (set|null)Property. Using same logic as
>     for
>     > allowEmpty the "required" syntax would be setIsRequired / isRequired.
>
>     Why? The verbiage is different, even though we're still asking a boolean
>     question here.
>
>
> Let my rephrase my argument here - isRequired => return the value of $this->
> _required. What value does allowEmpty return? I'd guess $this->_empty? Or not?

Honestly, still not seeing the issue here.

   if ($input->allowEmpty()) {
       // branch logic
   }

I don't see the issue with setAllowEmpty() in this case; it reads fine
in terms of English. "Allow" in this case is qualifying the condition
"Empty"; neither works by itself. Together, they can be used as a
predicate + subject.

> Are "is" and "has" some kind of "lesser" words, so they are ommited from
> variable names and "allow" and "can" are better so they can stay? :))

I'm not saying they're "lesser" words; I'm saying that in the case of
flag properties, they're unnecessary verbiage, but necessary when it
comes to the method used to indicate their value. "required" as a
property name is understandable; "isRequired" as a method name indicates
via the name what sort of return you're expecting.

--
Matthew Weier O'Phinney
Project Lead            | [hidden email]
Zend Framework          | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc

Reply | Threaded
Open this post in threaded view
|

Re: Forms RFC: InputFilter ready to test

DeNix
On 16.04.2012 15:49, Lee Davis wrote:
My take..

AllowEmpty is ambiguous to me. Sounding like it could be either an instruction (set this to allow empty) or a question (does this allow empty?). 

I think the method name should be more explicit of the expected behaviour. Maybe AllowsEmpty?
for me if it answers yes/no question it's good enought

allowEmpty - yes allow / no don't allow
throwException - yes throw
useDefaultAdapter - yes use
returnResult - yes return

and so on

On Mon, Apr 9, 2012 at 11:14 PM, Matthew Weier O'Phinney <[hidden email]> wrote:
-- Tomáš Fejfar <[hidden email]> wrote
(on Monday, 09 April 2012, 11:58 PM +0200):
> Last try, I promise! ;) More inline.
>
> On Mon, Apr 9, 2012 at 4:21 PM, Matthew Weier O'Phinney <[hidden email]>
> wrote:
>
>     Not all booleans need to start with "is". "allowEmpty" is natural
>     language -- "does $input allow empty?" (well, technically, should be
>     "does $input allow empty values?" but I think we can get away without
>     the "values"). Similarly, "hasSuchAndSuch" is fine for a boolean.
>
> I'm afraid you missed my point - which is consistency. I'll try that again.
>
> For boolean member variables we use is/has/allow/can. It's used in form:
>
> setter - set + VariableName
> getter - is/has/allow + VariableName
>
> Let's set VariableName = "Required".
>
> Than it's setRequired / isRequired
>
> Now let's set VariableName = 'Items'
>
> Than it's setItems / hasItems
>
> now let's set VariableName = 'AllowEmpty'
>
> That's setAllowEmpty / allowAllowEmpty ?!
>
> The naming does not follow the "verb + variableName" pattern.
>
>
>     > the setter/getter should be named (set|get)Property.
>     > But when the value is boolean it should be (set|is)Property.
>
>     I disagree, for the reasons stated above. "is" is not the only "boolean"
>     question we can ask. There are cases when it's inaccurate verbiage.
>
> Agreed. As i state above - can, allow and others are good for boolean questions
> too. But the variables themselves MUST NOT contain a verb.
>
> Bad example off the top of my head - method that return if something can be
> translated (let's say I can't come up with isTranslatable for the sake of the
> example):
>
> canBeTranslated() -> i expect the setter to be the getter minus the verb
> preppended with "set" = setBeTranslated.
>
> I think that every variable can be rewritten so that it does not start with a
> verb - as does this example = translatable - setTranslatable / isTranslatable
>
>
>     > But in case of allowEmpty it's (set|null)Property. Using same logic as
>     for
>     > allowEmpty the "required" syntax would be setIsRequired / isRequired.
>
>     Why? The verbiage is different, even though we're still asking a boolean
>     question here.
>
>
> Let my rephrase my argument here - isRequired => return the value of $this->
> _required. What value does allowEmpty return? I'd guess $this->_empty? Or not?

Honestly, still not seeing the issue here.

   if ($input->allowEmpty()) {
       // branch logic
   }

I don't see the issue with setAllowEmpty() in this case; it reads fine
in terms of English. "Allow" in this case is qualifying the condition
"Empty"; neither works by itself. Together, they can be used as a
predicate + subject.

> Are "is" and "has" some kind of "lesser" words, so they are ommited from
> variable names and "allow" and "can" are better so they can stay? :))

I'm not saying they're "lesser" words; I'm saying that in the case of
flag properties, they're unnecessary verbiage, but necessary when it
comes to the method used to indicate their value. "required" as a
property name is understandable; "isRequired" as a method name indicates
via the name what sort of return you're expecting.

--
Matthew Weier O'Phinney
Project Lead            | [hidden email]
Zend Framework          | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc


Reply | Threaded
Open this post in threaded view
|

Re: Forms RFC: InputFilter ready to test

Artur Bodera
2012/4/16 Denis Portnov <[hidden email]>
throwException - yes throw

Throw it now ?
Or throw it in the future? (AKA setThrowException)



-- 
      __
     /.)\   +48 695 600 936
     \(./   [hidden email]