ZF2 API standards and conventions
Locked
 
|
Hi all,
Not sure if it's the right time, because everything is a work in progress right now, but anyway... As a heavy ZF1 user I often stuble upon inconsistencies in API between components so I would like to raise the question of API standards and conventions There are several issues I see -- component configuration As far as I uderstand using Zend\Stdlib\Options is advised but not requred so many components will still be using ZF1 approach, several issues I see here - setConfig() vs setOptions() there should be strict rule for naming method that configures instance and what it accepts (array, Zend\Config etc) - singular vs plural (i.e. setParam/Option/Value() vs setParams/Options/Values) there should be a rule to define how such setters behave. It's either class has both setParam($name, $value) that could accept only name value pairs and setParams(array $params) or just one setParam() that could accept both name/value pair and array right now, you have to check the docs or code all the time, to make sure how method behaves - options array keys there are still few components that use camelCase keys (Zend\Queue\Adapter, Zend\Locale\Format ) So I think lowercase + underscore keys should be a strict rule also developers should separate meaningfull prefixes, i.e. 'ssl_transport' vs 'ssltransport' -- setting adapter, storage, transport etc methods like setAdapter() behaves differently between components -- class naming conventions There is no convention on how Factory, Service etc classes should be named so you can have Zend\Uri\UriFactory and Zend\Config\Factory -- i18n Some components return english messages without clear way to set message in different language i.e. Zend\Captcha\Dumb returns plain english message I think there should be a standard that defines how all non-exeptional messages could be set or the way to inject Transalator (Translatable interface ?) That's basically it so far Sorry for long post and bad english Best regards Denis |
|
On 4 Apr 2012, at 03:44, Denis Portnov wrote: > Hi all, > > Not sure if it's the right time, because everything is a work in progress right now, but anyway... > > As a heavy ZF1 user I often stuble upon inconsistencies in API between components > so I would like to raise the question of API standards and conventions > There are several issues I see In addition to the items Denis raised, there's also a situation where sometimes the getter is named xxx() rather than getXxx() and there doesn't seem to be a predictable rule to use to work out which one it's likely to be without going to the source. Regards, Rob... |
Re: ZF2 API standards and conventions
|
In reply to this post by DeNix
Am 4. April 2012 04:44 schrieb Denis Portnov <[hidden email]>:
> Hi all, > > Not sure if it's the right time, because everything is a work in progress > right now, but anyway... > > As a heavy ZF1 user I often stuble upon inconsistencies in API between > components > so I would like to raise the question of API standards and conventions > There are several issues I see > > -- component configuration > As far as I uderstand using Zend\Stdlib\Options is advised but not requred > so many components will still be using ZF1 approach, > several issues I see here > > - setConfig() vs setOptions() > there should be strict rule for naming method that configures instance and > what it accepts (array, Zend\Config etc) > > - singular vs plural (i.e. setParam/Option/Value() vs > setParams/Options/Values) > there should be a rule to define how such setters behave. > It's either class has both > setParam($name, $value) that could accept only name value pairs > and setParams(array $params) > or just one > setParam() that could accept both name/value pair and array > > right now, you have to check the docs or code all the time, to make sure how > method behaves > > - options array keys > there are still few components that use camelCase keys (Zend\Queue\Adapter, > Zend\Locale\Format ) > So I think lowercase + underscore keys should be a strict rule > also developers should separate meaningfull prefixes, i.e. 'ssl_transport' > vs 'ssltransport' > > -- setting adapter, storage, transport etc > methods like setAdapter() behaves differently between components > > -- class naming conventions > There is no convention on how Factory, Service etc classes should be named > so you can have Zend\Uri\UriFactory and Zend\Config\Factory > > > -- i18n > Some components return english messages without clear way to set message in > different language > i.e. Zend\Captcha\Dumb returns plain english message > I think there should be a standard that defines how all non-exeptional > messages could be set > or the way to inject Transalator (Translatable interface ?) > > That's basically it so far > Sorry for long post and bad english > > Best regards > Denis Hi all, perhabs we should define how things *should* work, we can discuss the new definitions and perhabs do a short poll on the decision. Next we should enforce everybody to update their component or let other contributors take over that work. currently i got even some time to help out here. Regards Sascha-Oliver Prolic |
Re: ZF2 API standards and conventions
Administrator
|
In reply to this post by DeNix
-- Denis Portnov <[hidden email]> wrote
(on Wednesday, 04 April 2012, 06:44 AM +0400): > Not sure if it's the right time, because everything is a work in > progress right now, but anyway... > > As a heavy ZF1 user I often stuble upon inconsistencies in API > between components > so I would like to raise the question of API standards and conventions > There are several issues I see > > -- component configuration > As far as I uderstand using Zend\Stdlib\Options is advised but not requred > so many components will still be using ZF1 approach, > several issues I see here This has been accepted, and is a requirement; the problem is that we haven't gone through all classes to refactor them to use this approach yet. > - setConfig() vs setOptions() > there should be strict rule for naming method that configures > instance and what it accepts (array, Zend\Config etc) There should be only one (if any), setOptions(), and it should accept the Options class for the given component. setConfig() should be eliminated in all cases. > - singular vs plural (i.e. setParam/Option/Value() vs > setParams/Options/Values) > there should be a rule to define how such setters behave. > It's either class has both > setParam($name, $value) that could accept only name value pairs > and setParams(array $params) > or just one > setParam() that could accept both name/value pair and array > > right now, you have to check the docs or code all the time, to make > sure how method behaves There's a reason for having both singular _and_ plural variants. The singular version is for setting a single param/option/value/whatever. The plural is for setting them en masse. I'd argue that we should refactor anywhere we're _not_ seeing this pattern being used to use it. Overloading a method to accept either pairs or sets is unintuitive for users. > - options array keys > there are still few components that use camelCase keys > (Zend\Queue\Adapter, Zend\Locale\Format ) > So I think lowercase + underscore keys should be a strict rule > also developers should separate meaningfull prefixes, i.e. > 'ssl_transport' vs 'ssltransport' This is, again, the goal; we simply haven't gone through all components and refactored them to follow the guideline. > -- setting adapter, storage, transport etc > methods like setAdapter() behaves differently between components Can you give some specific examples, please? And indicate what behavior you feel should be standard? > -- class naming conventions > There is no convention on how Factory, Service etc classes should be named > so you can have Zend\Uri\UriFactory and Zend\Config\Factory Agreed. Could you propose what you feel should be standard? > -- i18n > Some components return english messages without clear way to set > message in different language > i.e. Zend\Captcha\Dumb returns plain english message > I think there should be a standard that defines how all > non-exeptional messages could be set > or the way to inject Transalator (Translatable interface ?) During an IRC meeting a week or two ago, we agreed that translation should happen at the closest possible place to output, and should be _explicit_ -- i.e., not auto-magical. As such, you _should_ be able to grab the message from the captcha and pass it directly to the translator. If some classes make this difficult, they should be refactored to make it possible. -- 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 |
Re: ZF2 API standards and conventions
|
On Wed, Apr 4, 2012 at 4:24 PM, Matthew Weier O'Phinney <[hidden email]> wrote:
$obj->events() // ->attach(...)
or |
|
|
In reply to this post by weierophinney
On 04.04.2012 18:24, Matthew Weier O'Phinney wrote:
> -- Denis Portnov<[hidden email]> wrote > (on Wednesday, 04 April 2012, 06:44 AM +0400): >> -- component configuration >> As far as I uderstand using Zend\Stdlib\Options is advised but not requred >> so many components will still be using ZF1 approach, >> several issues I see here > This has been accepted, and is a requirement; the problem is that we > haven't gone through all classes to refactor them to use this approach > yet. for me the problem is how far this goes if I say have ComponentX, ComponentX/Storage, ComponentX/Adapter should I have ComponentXOptions, ComponentX/StorageOptions, ComponentX/AdapterOptions ? > >> - setConfig() vs setOptions() >> there should be strict rule for naming method that configures >> instance and what it accepts (array, Zend\Config etc) > There should be only one (if any), setOptions(), and it should accept > the Options class for the given component. setConfig() should be > eliminated in all cases. yet there are many methods with singular name that accept both name/value and array > >> - singular vs plural (i.e. setParam/Option/Value() vs >> setParams/Options/Values) >> there should be a rule to define how such setters behave. >> It's either class has both >> setParam($name, $value) that could accept only name value pairs >> and setParams(array $params) >> or just one >> setParam() that could accept both name/value pair and array >> >> right now, you have to check the docs or code all the time, to make >> sure how method behaves > There's a reason for having both singular _and_ plural variants. The > singular version is for setting a single param/option/value/whatever. > The plural is for setting them en masse. > > I'd argue that we should refactor anywhere we're _not_ seeing this > pattern being used to use it. Overloading a method to accept either > pairs or sets is unintuitive for users. >> -- setting adapter, storage, transport etc >> methods like setAdapter() behaves differently between components > Can you give some specific examples, please? And indicate what behavior > you feel should be standard? what I'm talking is something like setAdapter($adapter), where $adapter could be 1) instance 2) string - adapter name from Adapter namespace 3) string - any class name, that implements Adapter interface > >> -- class naming conventions >> There is no convention on how Factory, Service etc classes should be named >> so you can have Zend\Uri\UriFactory and Zend\Config\Factory > Agreed. Could you propose what you feel should be standard? I think it should be discussed along with Interface/Abstract naming I personally prefer extra folding, so fo me it's Zend\Uri\Factory in Zend\Uri\Factory.php Zend\Something\Storage\Factory in Zend\Something\Storage\Factory.php etc but again, it's up to discussion bottom line is, should I create API conventions section in CS wiki ? |
|
|
In reply to this post by akrabat
On 04.04.2012 9:45, Rob Allen wrote:
> On 4 Apr 2012, at 03:44, Denis Portnov wrote: > > In addition to the items Denis raised, there's also a situation where > sometimes the getter is named xxx() rather than getXxx() and there > doesn't seem to be a predictable rule to use to work out which one > it's likely to be without going to the source. Regards, Rob... agree, My view is that xxx() returns instance and getXxx() returns php type Denis |
Re: ZF2 API standards and conventions
|
In reply to this post by DeNix
Hello All,
<snip> >>> - singular vs plural (i.e. setParam/Option/Value() vs >>> setParams/Options/Values) >>> there should be a rule to define how such setters behave. >>> It's either class has both >>> setParam($name, $value) that could accept only name value pairs >>> and setParams(array $params) >>> or just one >>> setParam() that could accept both name/value pair and array >>> >>> right now, you have to check the docs or code all the time, to make >>> sure how method behaves >> >> There's a reason for having both singular _and_ plural variants. The >> singular version is for setting a single param/option/value/whatever. >> The plural is for setting them en masse. >> >> I'd argue that we should refactor anywhere we're _not_ seeing this >> pattern being used to use it. Overloading a method to accept either >> pairs or sets is unintuitive for users. > > I completely agree I'm not certain on this 100%; mainly just because it might be unintuitive to have: $class->name('dude') vs. $class->setNames(array('dude', 'baconator', 'foo', 'bar')) and then be able to get them all the time through $class->name(); In this case it might be better just to have $class->names(array('dude')); with $class->names() returning the actual value. >>> -- setting adapter, storage, transport etc >>> methods like setAdapter() behaves differently between components >> >> Can you give some specific examples, please? And indicate what behavior >> you feel should be standard? > > what I'm talking is something like setAdapter($adapter), where $adapter > could be > 1) instance > 2) string - adapter name from Adapter namespace > 3) string - any class name, that implements Adapter interface We should completely get away from any string based versions unless it is a factory IMO; with DI you should always be passing in an instance anyway. Additionally; instances make it much easier to be able to control how things are loaded. This additionally does save us performance on CPU clock cycles; while not major, it does have overhead when it is happening all the time everywhere. >>> -- class naming conventions >>> There is no convention on how Factory, Service etc classes should be >>> named >>> so you can have Zend\Uri\UriFactory and Zend\Config\Factory >> >> Agreed. Could you propose what you feel should be standard? > > I think it should be discussed along with Interface/Abstract naming > I personally prefer extra folding, so fo me it's > > Zend\Uri\Factory in Zend\Uri\Factory.php > Zend\Something\Storage\Factory in Zend\Something\Storage\Factory.php etc > > > but again, it's up to discussion > > bottom line is, should I create API conventions section in CS wiki ? I think this would be good to start adding this to the coding standards document for conventions. Regards, Mike |
|
This post has NOT been accepted by the mailing list yet.
In reply to this post by weierophinney
Are this rules listed somewhere in one place? It would be nice to have them in a single document, maybe next to this one: http://zendframework.com/wiki/display/ZFDEV2/Coding+Standards.
|
|
|
In reply to this post by Mike Willbanks
On 04.04.2012 21:32, Mike Willbanks wrote:
> Hello All, > > <snip> > >>>> - singular vs plural (i.e. setParam/Option/Value() vs >>>> setParams/Options/Values) >>>> there should be a rule to define how such setters behave. >>>> It's either class has both >>>> setParam($name, $value) that could accept only name value pairs >>>> and setParams(array $params) >>>> or just one >>>> setParam() that could accept both name/value pair and array >>>> >>>> right now, you have to check the docs or code all the time, to make >>>> sure how method behaves >>> There's a reason for having both singular _and_ plural variants. The >>> singular version is for setting a single param/option/value/whatever. >>> The plural is for setting them en masse. >>> >>> I'd argue that we should refactor anywhere we're _not_ seeing this >>> pattern being used to use it. Overloading a method to accept either >>> pairs or sets is unintuitive for users. >> I completely agree > I'm not certain on this 100%; mainly just because it might be > unintuitive to have: > $class->name('dude') vs. $class->setNames(array('dude', 'baconator', > 'foo', 'bar')) and then be able to get them all the time through > $class->name(); > > In this case it might be better just to have > $class->names(array('dude')); with $class->names() returning the > actual value. I ment methods when you set name/value pair or array of name/value pairs i.e. in current Http\Client we have: public function setParameterPost(array $post) { $this->getRequest()->post()->fromArray($post); return $this; } to me intuitive way is to have something like: public function setParameterPost($paramName, $paramValue) { } public function setParametersPost(array $parameters) { } >> what I'm talking is something like setAdapter($adapter), where $adapter >> could be >> 1) instance >> 2) string - adapter name from Adapter namespace >> 3) string - any class name, that implements Adapter interface > We should completely get away from any string based versions unless it > is a factory IMO; with DI you should always be passing in an instance > anyway. Additionally; instances make it much easier to be able to > control how things are loaded. This additionally does save us > performance on CPU clock cycles; while not major, it does have > overhead when it is happening all the time everywhere. Regards, Denis |
RE: ZF2 API standards and conventions
|
I really, really, yes I mean REALLY hate the lack of get and set prefixes. Here are some reasons why:
- The main reason is that the method name does not reflect what the method does. // Is a HTTP POST request done here? $result = $this->getRequest()->post(); - It is not clear if setting or getting is allowed (!!!). - The function gets bigger than it should be since it now has functionality of the getter and setter at the same time. - Dealing with private methods will be confusing. Public method A sets values by using multiple private methods, $this->setPrivateA, $this->setPrivateB? Is it suddenly allowed to use "set" again? - I don't know about you but I work (and think) like this: 1. I want to get a POST variable from the request. 2. First I need to get the request. I type: "get" since I want to get it. 3. My IDE shows me all the getters but unfortunately I don't see getRequest... there is getFrontController though. 4. I first get the front controller since I know that I can get the request with the front controller. 5. Now I type "$frontController->get" and my IDE neatly shows all the getters again. Nice, getRequest is in it. Done. In my opinion method names should describe what the method does and be readable at the same time by using nouns, adjectives and verbs. Denis wants to name a method "setParametersPost" while I would name it: "setPostParameters". Some examples of correct named methods: - isAlphaVisible(); - setAlphaVisibility(); - setCanHide(); - canHide(); - getTitle(); - setTitle(); - isEditable(); - setEditable(); - hide(); - show(); Booleans (flags) should be retrieved by asking a question: "can I hide this?", "is this visible?", "is this field editable?" Actions often only need a verb: "$teaPot->show();", "$teaPot->hide();", "$teaPot->pour()" Setting variables should be seen as: "I want to set the POST parameters" (setPostParameters) and not: "I want to set the parameters of type POST" (setParametersPost) Just my 2 cents... > Date: Thu, 5 Apr 2012 04:42:10 +0400 > From: [hidden email] > To: [hidden email] > CC: [hidden email] > Subject: Re: [zf-contributors] ZF2 API standards and conventions > > On 04.04.2012 21:32, Mike Willbanks wrote: > > Hello All, > > > > <snip> > > > >>>> - singular vs plural (i.e. setParam/Option/Value() vs > >>>> setParams/Options/Values) > >>>> there should be a rule to define how such setters behave. > >>>> It's either class has both > >>>> setParam($name, $value) that could accept only name value pairs > >>>> and setParams(array $params) > >>>> or just one > >>>> setParam() that could accept both name/value pair and array > >>>> > >>>> right now, you have to check the docs or code all the time, to make > >>>> sure how method behaves > >>> There's a reason for having both singular _and_ plural variants. The > >>> singular version is for setting a single param/option/value/whatever. > >>> The plural is for setting them en masse. > >>> > >>> I'd argue that we should refactor anywhere we're _not_ seeing this > >>> pattern being used to use it. Overloading a method to accept either > >>> pairs or sets is unintuitive for users. > >> I completely agree > > I'm not certain on this 100%; mainly just because it might be > > unintuitive to have: > > $class->name('dude') vs. $class->setNames(array('dude', 'baconator', > > 'foo', 'bar')) and then be able to get them all the time through > > $class->name(); > > > > In this case it might be better just to have > > $class->names(array('dude')); with $class->names() returning the > > actual value. > Your example is more about setting/getting some entity properties > I ment methods when you set name/value pair or array of name/value pairs > i.e. in current Http\Client we have: > > public function setParameterPost(array $post) > { > $this->getRequest()->post()->fromArray($post); > return $this; > } > > to me intuitive way is to have something like: > > public function setParameterPost($paramName, $paramValue) > { > } > > public function setParametersPost(array $parameters) > { > } > >> what I'm talking is something like setAdapter($adapter), where $adapter > >> could be > >> 1) instance > >> 2) string - adapter name from Adapter namespace > >> 3) string - any class name, that implements Adapter interface > > We should completely get away from any string based versions unless it > > is a factory IMO; with DI you should always be passing in an instance > > anyway. Additionally; instances make it much easier to be able to > > control how things are loaded. This additionally does save us > > performance on CPU clock cycles; while not major, it does have > > overhead when it is happening all the time everywhere. > I'm also leaning towards setAdapter/Storage/whatever accepting instance only > > > Regards, > Denis |
Re: ZF2 API standards and conventions
|
I might generally agree, but what about objects who's primary purpose
and API is intended to be fluent in nature, for example: $where = new Zend\Db\Sql\Where(); $where->equalTo('id', 1)->OR->equalTo('id', 2); Thoughts? -ralph On 4/5/12 1:34 PM, Walter Tamboer wrote: > I really, really, yes I mean REALLY hate the lack of get and set > prefixes. Here are some reasons why: > > - The main reason is that the method name does not reflect what the > method does. > // Is a HTTP POST request done here? > $result = $this->getRequest()->post(); > > - It is not clear if setting or getting is allowed (!!!). > > - The function gets bigger than it should be since it now has > functionality of the getter and setter at the same time. > > - Dealing with private methods will be confusing. Public method A sets > values by using multiple private methods, $this->setPrivateA, > $this->setPrivateB? Is it suddenly allowed to use "set" again? > > - I don't know about you but I work (and think) like this: > 1. I want to get a POST variable from the request. > 2. First I need to get the request. I type: "get" since I want to get it. > 3. My IDE shows me all the getters but unfortunately I don't see > getRequest... there is getFrontController though. > 4. I first get the front controller since I know that I can get the > request with the front controller. > 5. Now I type "$frontController->get" and my IDE neatly shows all the > getters again. Nice, getRequest is in it. Done. > > In my opinion method names should describe what the method does and be > readable at the same time by using nouns, adjectives and verbs. Denis > wants to name a method "setParametersPost" while I would name it: > "setPostParameters". > > Some examples of correct named methods: > - isAlphaVisible(); > - setAlphaVisibility(); > - setCanHide(); > - canHide(); > - getTitle(); > - setTitle(); > - isEditable(); > - setEditable(); > - hide(); > - show(); > > Booleans (flags) should be retrieved by asking a question: "can I hide > this?", "is this visible?", "is this field editable?" > Actions often only need a verb: "$teaPot->show();", "$teaPot->hide();", > "$teaPot->pour()" > Setting variables should be seen as: "I want to set the POST parameters" > (setPostParameters) and not: "I want to set the parameters of type POST" > (setParametersPost) > > Just my 2 cents... > > > > > Date: Thu, 5 Apr 2012 04:42:10 +0400 > > From: [hidden email] > > To: [hidden email] > > CC: [hidden email] > > Subject: Re: [zf-contributors] ZF2 API standards and conventions > > > > On 04.04.2012 21:32, Mike Willbanks wrote: > > > Hello All, > > > > > > <snip> > > > > > >>>> - singular vs plural (i.e. setParam/Option/Value() vs > > >>>> setParams/Options/Values) > > >>>> there should be a rule to define how such setters behave. > > >>>> It's either class has both > > >>>> setParam($name, $value) that could accept only name value pairs > > >>>> and setParams(array $params) > > >>>> or just one > > >>>> setParam() that could accept both name/value pair and array > > >>>> > > >>>> right now, you have to check the docs or code all the time, to make > > >>>> sure how method behaves > > >>> There's a reason for having both singular _and_ plural variants. The > > >>> singular version is for setting a single param/option/value/whatever. > > >>> The plural is for setting them en masse. > > >>> > > >>> I'd argue that we should refactor anywhere we're _not_ seeing this > > >>> pattern being used to use it. Overloading a method to accept either > > >>> pairs or sets is unintuitive for users. > > >> I completely agree > > > I'm not certain on this 100%; mainly just because it might be > > > unintuitive to have: > > > $class->name('dude') vs. $class->setNames(array('dude', 'baconator', > > > 'foo', 'bar')) and then be able to get them all the time through > > > $class->name(); > > > > > > In this case it might be better just to have > > > $class->names(array('dude')); with $class->names() returning the > > > actual value. > > Your example is more about setting/getting some entity properties > > I ment methods when you set name/value pair or array of name/value pairs > > i.e. in current Http\Client we have: > > > > public function setParameterPost(array $post) > > { > > $this->getRequest()->post()->fromArray($post); > > return $this; > > } > > > > to me intuitive way is to have something like: > > > > public function setParameterPost($paramName, $paramValue) > > { > > } > > > > public function setParametersPost(array $parameters) > > { > > } > > >> what I'm talking is something like setAdapter($adapter), where > $adapter > > >> could be > > >> 1) instance > > >> 2) string - adapter name from Adapter namespace > > >> 3) string - any class name, that implements Adapter interface > > > We should completely get away from any string based versions unless it > > > is a factory IMO; with DI you should always be passing in an instance > > > anyway. Additionally; instances make it much easier to be able to > > > control how things are loaded. This additionally does save us > > > performance on CPU clock cycles; while not major, it does have > > > overhead when it is happening all the time everywhere. > > I'm also leaning towards setAdapter/Storage/whatever accepting > instance only > > > > > > Regards, > > Denis |
Re: ZF2 API standards and conventions
|
This post has NOT been accepted by the mailing list yet.
In reply to this post by Walter Tamboer
Completely agree you on this one Walter.
Thanks On Thu, Apr 5, 2012 at 1:35 PM, Walter Tamboer [via Zend Framework Community] <[hidden email]> wrote:
|
Re: ZF2 API standards and conventions
|
In reply to this post by Walter Tamboer
Only one thing about the quoted - the good thing about setParameterPost is "importance order". I want to set. What I want to set? Parameter. What kind of parameter? Post.
That way the list in IDE is progressively shortened keeping all the possible matches.
On Thu, Apr 5, 2012 at 8:34 PM, Walter Tamboer <[hidden email]> wrote:
|
Re: ZF2 API standards and conventions
|
|
This post has NOT been accepted by the mailing list yet.
Personally i find setPostParameter() far more comfortable and logical to use, to me setParametersPost is backwards. The way i look at things is i'm working with a post and i want to set some parameter for it, hence setPostParameter(). so basically the rule would be set/get <main object> <action/var/etc..>
Thanks, Toby On Thu, Apr 5, 2012 at 2:52 PM, Tomáš Fejfar [via Zend Framework Community] <[hidden email]> wrote: Only one thing about the quoted - the good thing about setParameterPost is "importance order". I want to set. What I want to set? Parameter. What kind of parameter? Post. |
|
|
In reply to this post by Tomáš Fejfar
On 05.04.2012 23:52, Tomáš Fejfar wrote:
Only one thing about the quoted - the good thing about setParameterPost is "importance order". I want to set. What I want to set? Parameter. What kind of parameter? Post.The problem with this exact example is that setParameterPost($name, $vulue) and setParametersPost(array $params) is hard to distinct where setPostParam($name, $value) and setPostParams(array $params) is easear to distinct visually |
|
2012/4/5 Denis Portnov <[hidden email]>:
> On 05.04.2012 23:52, Tomáš Fejfar wrote: > > Only one thing about the quoted - the good thing about setParameterPost is > "importance order". I want to set. What I want to set? Parameter. What kind > of parameter? Post. > > That way the list in IDE is progressively shortened keeping all the possible > matches. > > On Thu, Apr 5, 2012 at 8:34 PM, Walter Tamboer <[hidden email]> > wrote: >> >> Setting variables should be seen as: "I want to set the POST parameters" >> (setPostParameters) and not: "I want to set the parameters of type POST" >> (setParametersPost) > > The problem with this exact example is that > setParameterPost($name, $vulue) and setParametersPost(array $params) is hard > to distinct > where > setPostParam($name, $value) and setPostParams(array $params) is easear to > distinct visually Maybe you want addPostParam() -- I'm guessing there can be multiple. |
Re: ZF2 API standards and conventions
|
Administrator
|
In reply to this post by Walter Tamboer
-- Walter Tamboer <[hidden email]> wrote
(on Thursday, 05 April 2012, 08:34 PM +0200): > I really, really, yes I mean REALLY hate the lack of get and set prefixes. Here > are some reasons why: > > - The main reason is that the method name does not reflect what the method > does. > // Is a HTTP POST request done here? > $result = $this->getRequest()->post(); I can see this point. > - It is not clear if setting or getting is allowed (!!!). I think the fact that a "setPost()" method is in the API makes this a bit clearer... > - The function gets bigger than it should be since it now has functionality of > the getter and setter at the same time. Um, no. There's a setter for passing in the value object. It's only used for retrieval. One reason for using this pattern is because we want to expose an object property without allowing the ability to set it to an arbitrary value. Ideally, we could do simply this: $request->post->get('some-var', 'default-value'); but because PHP does not have first-class properties (yet), this isn't possible. (Making it public would allow setting it to *any* value, which could lead to issues.) That said, perhaps we need to revisit this, if it's really causing such confusion. > - Dealing with private methods will be confusing. Public method A sets values > by using multiple private methods, $this->setPrivateA, $this->setPrivateB? Is > it suddenly allowed to use "set" again? I don't quite understand this point. Can you clarify, please? > - I don't know about you but I work (and think) like this: > 1. I want to get a POST variable from the request. > 2. First I need to get the request. I type: "get" since I want to get it. > 3. My IDE shows me all the getters but unfortunately I don't see getRequest... > there is getFrontController though. > 4. I first get the front controller since I know that I can get the request > with the front controller. > 5. Now I type "$frontController->get" and my IDE neatly shows all the getters > again. Nice, getRequest is in it. Done. This is going to sound snarky, but it's not meant to be. What version of the framework are you using, exactly? We don't have a "front controller" in ZF2 at this time, so I'm really unsure how the above would play out. Your action controllers have a "getRequest" method already (as well as getResponse). Are you talking about retrieving the POST variables? Because that more directly relates to the problem you originally outlined, and there I can see the issue -- how do you get the POST variables? * I have the request object * I type "get" and look for a "Post" object.. and don't see it * What should I look for? In this case, I see the issue -- you retrieve it with "post()" -- which you obviously won't find if you start typeing "get". > In my opinion method names should describe what the method does and be readable > at the same time by using nouns, adjectives and verbs. Denis wants to name a > method "setParametersPost" while I would name it: "setPostParameters". Why have "Parameters" in the name at all? why not "setPost", "setQuery", etc? Basically, what exact use case are you trying for? * Do you want to set the variable container that represents the POST variables? * Do you want to set values in the POST variable container? They're different actions, and I'd expect different workflows for them. One question, if the second action is what you want, is if we should have a method that proxies this within the request -- so that instead of this: $request->getPost()->set(...) you would do something like: $request->setParametersPost(...) This may be more convenient in terms of API, though it means that the Request object then has a _ton_ of extra behavior to allow such proxies. Again, however, I'm open to doing so, if we can demonstrate that it aids developer understanding of the API or simplifies usage. > Some examples of correct named methods: > - isAlphaVisible(); > - setAlphaVisibility(); > - setCanHide(); > - canHide(); > - getTitle(); > - setTitle(); > - isEditable(); > - setEditable(); > - hide(); > - show(); > > Booleans (flags) should be retrieved by asking a question: "can I hide this?", > "is this visible?", "is this field editable?" I completely agree here (and just responded as such in another thread). I consider this more usable. > Actions often only need a verb: "$teaPot->show();", "$teaPot->hide();", > "$teaPot->pour()" I agree here, as wel. > Setting variables should be seen as: "I want to set the POST parameters" > (setPostParameters) and not: "I want to set the parameters of type POST" > (setParametersPost) The problem here is that it potentially makes the API less discoverable and predictable. The reason being that in the request object, we have a number of parameter containers -- for the Query parameters, POST, cookies, etc. As such, having consistent naming may aid discovery and education -- setParameters*() vs set*Parameters(). Regardless, we need to make it consistent -- but also ensure we understand the ramifications of changing this, and how it might impact the API in terms of bloat. > > Date: Thu, 5 Apr 2012 04:42:10 +0400 > > From: [hidden email] > > To: [hidden email] > > CC: [hidden email] > > Subject: Re: [zf-contributors] ZF2 API standards and conventions > > > > On 04.04.2012 21:32, Mike Willbanks wrote: > > > Hello All, > > > > > > <snip> > > > > > >>>> - singular vs plural (i.e. setParam/Option/Value() vs > > >>>> setParams/Options/Values) > > >>>> there should be a rule to define how such setters behave. > > >>>> It's either class has both > > >>>> setParam($name, $value) that could accept only name value pairs > > >>>> and setParams(array $params) > > >>>> or just one > > >>>> setParam() that could accept both name/value pair and array > > >>>> > > >>>> right now, you have to check the docs or code all the time, to make > > >>>> sure how method behaves > > >>> There's a reason for having both singular _and_ plural variants. The > > >>> singular version is for setting a single param/option/value/whatever. > > >>> The plural is for setting them en masse. > > >>> > > >>> I'd argue that we should refactor anywhere we're _not_ seeing this > > >>> pattern being used to use it. Overloading a method to accept either > > >>> pairs or sets is unintuitive for users. > > >> I completely agree > > > I'm not certain on this 100%; mainly just because it might be > > > unintuitive to have: > > > $class->name('dude') vs. $class->setNames(array('dude', 'baconator', > > > 'foo', 'bar')) and then be able to get them all the time through > > > $class->name(); > > > > > > In this case it might be better just to have > > > $class->names(array('dude')); with $class->names() returning the > > > actual value. > > Your example is more about setting/getting some entity properties > > I ment methods when you set name/value pair or array of name/value pairs > > i.e. in current Http\Client we have: > > > > public function setParameterPost(array $post) > > { > > $this->getRequest()->post()->fromArray($post); > > return $this; > > } > > > > to me intuitive way is to have something like: > > > > public function setParameterPost($paramName, $paramValue) > > { > > } > > > > public function setParametersPost(array $parameters) > > { > > } > > >> what I'm talking is something like setAdapter($adapter), where $adapter > > >> could be > > >> 1) instance > > >> 2) string - adapter name from Adapter namespace > > >> 3) string - any class name, that implements Adapter interface > > > We should completely get away from any string based versions unless it > > > is a factory IMO; with DI you should always be passing in an instance > > > anyway. Additionally; instances make it much easier to be able to > > > control how things are loaded. This additionally does save us > > > performance on CPU clock cycles; while not major, it does have > > > overhead when it is happening all the time everywhere. > > I'm also leaning towards setAdapter/Storage/whatever accepting instance only > > > > > > Regards, > > Denis -- 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 |
RE: ZF2 API standards and conventions
|
|
Hi,
> > - It is not clear if setting or getting is allowed (!!!). > > I think the fact that a "setPost()" method is in the API makes this a > bit clearer... > A bit yes. But post() vs setPost() still does not tell me whether or not I can retrieve the post parameters especially since post is also a verb in this example (which indicates that an action is done). > > - The function gets bigger than it should be since it now has functionality of > > the getter and setter at the same time. > > Um, no. There's a setter for passing in the value object. It's only used > for retrieval. > > One reason for using this pattern is because we want to expose an object > property without allowing the ability to set it to an arbitrary value. > Ideally, we could do simply this: > > $request->post->get('some-var', 'default-value'); > > but because PHP does not have first-class properties (yet), this isn't > possible. (Making it public would allow setting it to *any* value, which > could lead to issues.) > > That said, perhaps we need to revisit this, if it's really causing such > confusion. > :-) https://wiki.php.net/rfc/propertygetsetsyntax I hope this will make it. What I mean with a function being bigger than it should be is this: function myMagic( $value = null ) { if ( $value === null ) { // do lots of calculations // // do more calculations // // and even more // return $calculationResult; } else { // do lots of calculations using $value // // do more calculations using $value // // and even more using $value // // set value 1 // set value 2 // set value 3 } } Actually, comparing this to properties I realize that I'm contradicting my self a bit. I think what I'm trying to say is that properties indicate that you change a value while a getter and setter could emphasise that there could be (expensive) logic behind it. Anyway, my previous point still stands I think. Properties in C# work because the IDE (Visual Studio) helps a lot so you immediately see whether getting and setting is allowed. http://stackoverflow.com/questions/4948816/getters-setters-and-properties-best-practices-java-vs-c-sharp > > - Dealing with private methods will be confusing. Public method A sets values > > by using multiple private methods, $this->setPrivateA, $this->setPrivateB? Is > > it suddenly allowed to use "set" again? > > I don't quite understand this point. Can you clarify, please? My bad, I should have explained this in more detail. If we take my code example from above (myMagic) you see that the setter sets three values. How would you name those methods? I was assuming that it would start with "set" but that might not be the case. Would it be "$this->_value1( $calculatedValue1 );" or "$this->value1( $calculatedValue1 );"? > > - I don't know about you but I work (and think) like this: > > 1. I want to get a POST variable from the request. > > 2. First I need to get the request. I type: "get" since I want to get it. > > 3. My IDE shows me all the getters but unfortunately I don't see getRequest... > > there is getFrontController though. > > 4. I first get the front controller since I know that I can get the request > > with the front controller. > > 5. Now I type "$frontController->get" and my IDE neatly shows all the getters > > again. Nice, getRequest is in it. Done. > > This is going to sound snarky, but it's not meant to be. What version > of the framework are you using, exactly? We don't have a "front > controller" in ZF2 at this time, so I'm really unsure how the above > would play out. > I'm using the latest code. The example was just ... an example :) I lack imagination so I just came up with a scenario from ZF1. > > In my opinion method names should describe what the method does and be readable > > at the same time by using nouns, adjectives and verbs. Denis wants to name a > > method "setParametersPost" while I would name it: "setPostParameters". > > Why have "Parameters" in the name at all? why not "setPost", "setQuery", > etc? Basically, what exact use case are you trying for? > Good point. The only reason why is because "post" is also a verb. > > Setting variables should be seen as: "I want to set the POST parameters" > > (setPostParameters) and not: "I want to set the parameters of type POST" > > (setParametersPost) > > The problem here is that it potentially makes the API less discoverable > and predictable. The reason being that in the request object, we have a > number of parameter containers -- for the Query parameters, POST, > cookies, etc. As such, having consistent naming may aid discovery and > education -- setParameters*() vs set*Parameters(). Regardless, we need > to make it consistent -- but also ensure we understand the ramifications > of changing this, and how it might impact the API in terms of bloat. > Consistency is key in an API, definitely. Regards, Walter |
Re: ZF2 API standards and conventions
|
Administrator
|
-- Walter Tamboer <[hidden email]> wrote
(on Sunday, 08 April 2012, 01:23 PM +0200): > > > - It is not clear if setting or getting is allowed (!!!). > > > > I think the fact that a "setPost()" method is in the API makes this a > > bit clearer... > > A bit yes. But post() vs setPost() still does not tell me whether or not I can > retrieve the post parameters especially since post is also a verb in this > example (which indicates that an action is done). Right -- this is one of those cases where the object ($_POST) is also a verb, and thus confusion. <snip> > What I mean with a function being bigger than it should be is this: > > function myMagic( $value = null ) > { > if ( $value === null ) > { > // do lots of calculations > // > // do more calculations > // > // and even more > // > return $calculationResult; > } > else > { > // do lots of calculations using $value > // > // do more calculations using $value > // > // and even more using $value > // > // set value 1 > // set value 2 > // set value 3 > } > } Right -- and we've been gradually getting rid of patterns such as these (look at the various "plugin()/broker()" methods -- most of them are single-behavior methods at this time). The consensus is exactly what you're pointing out -- too much behavior in the method, based on what parameters you pass -- makes them too difficult to learn. > Actually, comparing this to properties I realize that I'm contradicting my self > a bit. I think what I'm trying to say is that properties indicate that you > change a value while a getter and setter could emphasise that there could be > (expensive) logic behind it. Anyway, my previous point still stands I think. > Properties in C# work because the IDE (Visual Studio) helps a lot so you > immediately see whether getting and setting is allowed. > http://stackoverflow.com/questions/4948816/ > getters-setters-and-properties-best-practices-java-vs-c-sharp I'd like to find a happy medium in here somehow if we can. Most setters and getters we have, particularly in the request object, are not performing any logic -- they're simply ensuring we have the appropriate object (via typehinting) in the property. This was the original idea behind methods like "broker()", "events()", etc -- we can assume the objects are present, and we want public access to them -- but we don't want to allow setting them to arbitrary values, only objects of certain types. > > > - Dealing with private methods will be confusing. Public method A > > > sets values by using multiple private methods, $this->setPrivateA, > > > $this->setPrivateB? Is it suddenly allowed to use "set" again? > > > > I don't quite understand this point. Can you clarify, please? > > My bad, I should have explained this in more detail. If we take my code example > from above (myMagic) you see that the setter sets three values. How would you > name those methods? I was assuming that it would start with "set" but that > might not be the case. Would it be "$this->_value1( $calculatedValue1 );" or > "$this->value1( $calculatedValue1 );"? It really depends on what you're doing. If you're simply setting properties, you may be able to get away with simple assignment: $this->value1 = $calculatedValue1; Alternately, you could move the calculation into a protected method in order to make the method simpler: $this->calculateValue1($some, $values, $from, $current, $scope); The naming would be very much based on what you're trying to accomplish. When it comes to non-public methods, I'm not sure we need to enforce a specific naming scheme, so long as the names generally indicate the purpose. <snip> > > > In my opinion method names should describe what the method does > > > and be readable at the same time by using nouns, adjectives and > > > verbs. Denis wants to name a method "setParametersPost" while I > > > would name it: "setPostParameters". > > > > Why have "Parameters" in the name at all? why not "setPost", "setQuery", > > etc? Basically, what exact use case are you trying for? > > Good point. The only reason why is because "post" is also a verb. Again, as I said before, we have murky territory here, because of our PHP legacy. The object is encapsulating $_POST. :-/ Good discussion -- and thanks for the clarifications. -- 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 |
«
Return to ZF Contributor
|
1 view|%1 views
Loading...
| Powered by Nabble | Edit this page |