New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3.0 RFC: Function notation #4499

Closed
davidyell opened this Issue Sep 5, 2014 · 92 comments

Comments

Projects
None yet
@davidyell
Contributor

davidyell commented Sep 5, 2014

TL;DR
Abandon the _protected() and __private() convention.

Proposal
Cake has been in development for a long time now and when it started it supported very old PHP versions. When PHP4 was around there were no visibility keywords, which is why a convention of prefixing methods was adopted.

I think that this old convention should be dropped in the next major version of the framework.

Reasoning
At CakeFest 2014 @jameswatts said in his community keynote that we should be working to help market the framework and push it within the community with colleagues, friends and peers. I feel that having a new major version of the framework still using a convention which was born in PHP4 will give nay-sayers good ammunition to keep on with their 'Cake still uses php4 conventions'.

The newer frameworks are being sold to developers on the software engineering patterns that they implement and how they solve problems. The next major version of the framework has done lots to address this angle of promoting the framework, so it seems odd to me that such an old convention should still exist in the new major version of the framework.

Pros

  • Keeps the framework modern.
  • Takes away ammunition from nay-sayers.
  • A major release to the framework creates a solid break from old conventions.
  • IDEs have moved on since back-in-the-day, so devs will see method visibility.
  • Less to type 😉

Cons

  • Will be lots of change for no functional benefit.
  • Will cause plugin developers to change any existing 3.x plugins in development.
  • Sniffs need updating.

Thoughts
I am interested to discuss this issue as I really feel that it is a simple change which will really push the framework on as perceived by potential users.

@dereuromark dereuromark added this to the Future milestone Sep 5, 2014

@dereuromark dereuromark added the RFC label Sep 5, 2014

@dereuromark

This comment has been minimized.

Member

dereuromark commented Sep 5, 2014

I am not against this change, but I think we should maybe do this for 4.0 or the next major version as for 3.0 almost in RC this would probably create quite a lot of trouble now.
There are already enough major breaking changes in there so that this additional overhead might be better added for another major version change.
This allows us to focus on getting 3.0 now ready to ship.

That said it would probably be possible to address some of the method renaming via Upgrade app commands. But probably only a few, not all.

@thaJeztah

This comment has been minimized.

thaJeztah commented Sep 5, 2014

+100

In general, I'm hoping that CakePHP will adopt PSR-1/PSR-2 coding standards, after all, CakePHP is a voting member. Working with CakePHP alongside other libraries (including our own) now requires me to switch coding-styles (both mentally and in my IDE).

@ADmad

This comment has been minimized.

Member

ADmad commented Sep 5, 2014

I prefer the _ prefix for better readability. There is no need to make any change in the core method.

The only thing that's probably needed is to not force users to follow the same in their apps. Controller::_privateMethod() should be updated to not treat methods prefixed with underscore as "private".

@ADmad

This comment has been minimized.

Member

ADmad commented Sep 5, 2014

@thaJeztah CakePHP already follows PSR-1 and will NEVER in the forseeable future won't follow PSR-2 due to it's stupid indentation with spaces requirement.

Edit: You are not forced to follow CakPHP's coding standards in your app code.

@thaJeztah

This comment has been minimized.

thaJeztah commented Sep 5, 2014

due to it's stupid indentation with spaces requirement.

@ADmad please can we keep out childish comments, no need to call something stupid if you don't agree with it

I worked with tabs in the past, but modern IDEs can treat space-indents just as easily as tabs. Switching from tabs to spaces saved us a lot of formatting mistakes, for example if an (external) developer used incorrect tab-settings (which is not directly obvious in all cases).

Additionally, most IDEs support PSR-1/2 out of the box, CakePHP coding-standards built-in is a lot less common.

But I don't want to pollute this issue with (personal) preference too much. I've tried to explain the advantages for me as a user (interoperability) and leave the decision making to the CakePHP maintainers.

@lorenzo

This comment has been minimized.

Member

lorenzo commented Sep 5, 2014

@thaJeztah the coding standard is only used internally, there is absolutely no need to follow it in your pp if you don't agree with it

@thaJeztah

This comment has been minimized.

thaJeztah commented Sep 5, 2014

@lorenzo I fully understand that and really didn't want to go down this path, I was just hoping CakePHP to switch to a more common coding-style. Tabs vs. Spaces even isn't my top-most priority here (having the PhpDoc-comments not-indented, for example, is probably higher).

Lets put this to rest, I've added my 2c.

@lorenzo

This comment has been minimized.

Member

lorenzo commented Sep 5, 2014

@thaJeztah The cost / benefit relation for me is too low. I have plenty thing in my mind that I'd rather spend my time on. That's why most of the cake team decided to move on this spaces vs tabs debate

@dereuromark

This comment has been minimized.

Member

dereuromark commented Sep 5, 2014

Switching from tabs to spaces saved us a lot of formatting mistakes, for example if an (external) developer used incorrect tab-settings (which is not directly obvious in all cases).

The opposite is true, only tabs can really resolve all the issues here - and sane IDE settings, of course.
But that is totally off topic here. Please stick to the actual issue, regarding the convention for protected and private method visibility.

Anyway. It seems there is not a too big vote for changing it as of now. But maybe for future versions we can take another look.

@rchavik

This comment has been minimized.

Member

rchavik commented Sep 5, 2014

I'm leaning towards -1. But will not fuss over this too much if this gets passed.

@markstory

This comment has been minimized.

Member

markstory commented Sep 5, 2014

I'm ok with re-indenting the doc blocks. If that is a contentious/problematic point. My only request is we do it after the merges to 2.x are complete, as I don't want to make the already difficult merges even more difficult.

As for dropping the _, it would be a pretty big BC break at this point. I am happy to change the sniff rules to ignore _ and update the docs.

@davidyell

This comment has been minimized.

Contributor

davidyell commented Sep 5, 2014

there is absolutely no need to follow it in your pp if you don't agree with it

This is true, but misses the key point for me. The whole point is that the framework gives style guide / key / impetus to the developer for how to form their code. So by maintaining this convention in the core it is more likely to filter through to developers code.

The second point, and the one that I feel is more important, is that as long as this convention exists inside the code in the core it's valid to say that CakePHP 3 is using a PHP4 coding convention which is something I'm not cool with, and I don't think other people should be either.

@markstory commented whilst I was typing, so just to address that, I'm not aware of the technical implications of it within the core. Which is why I targeted this discussion at the 3.x version.

@rchavik

This comment has been minimized.

Member

rchavik commented Sep 5, 2014

@markstory i'm 👎 on reindenting the doc block

@dereuromark

This comment has been minimized.

Member

dereuromark commented Sep 5, 2014

@rchavik That is also off topic here :) Mark did just a remark on the side. No need to vote on it.

@rchavik

This comment has been minimized.

Member

rchavik commented Sep 5, 2014

i know. i voted because the mark brought it up and i have to keep my reputation as an early dissenter. :)

@ADmad

This comment has been minimized.

Member

ADmad commented Sep 5, 2014

@josegonzalez

This comment has been minimized.

Member

josegonzalez commented Sep 5, 2014

👎 as it'll make merges to/from 2.x difficult. 👍 If we punt on this till 4.0.

@thaJeztah

This comment has been minimized.

thaJeztah commented Sep 5, 2014

@ADmad I really don't know if you're just kidding around or giving me 'the finger', if you just did, thanks for making me feel welcome in the CakePHP community.

@markstory

This comment has been minimized.

Member

markstory commented Sep 5, 2014

@josegonzalez Another option is to wait until the 3.0.0-RC*. By that point the value in merging with 2.x is pretty low.

@lorenzo

This comment has been minimized.

Member

lorenzo commented Sep 5, 2014

@thaJeztah he has a very dark humor, but he is definitely trolling

@josegonzalez

This comment has been minimized.

Member

josegonzalez commented Sep 5, 2014

@thaJeztah I apologize for @ADmad's rude responses. I believe he is just poking fun, but it's still no excuse.

@markstory

This comment has been minimized.

Member

markstory commented Sep 5, 2014

One option for _ prefixed methods that hasn't come up is not making more. The compatibility implications with removing the existing ones is tricky, but we could not make more.

@thaJeztah

This comment has been minimized.

thaJeztah commented Sep 5, 2014

@josegonzalez @lorenzo thanks. Usually I have no problem recognizing jokes, but this time, I had a hard time. I feel bad because this discussion is heading completely in the wrong direction while I only wanted to suggest that removing the _ is a good start and I would like to see PSR-1/2 in some future.

Never expected that only mentioning them would bring such trouble.

@dereuromark

This comment has been minimized.

Member

dereuromark commented Sep 5, 2014

@thaJeztah That is pouring gas in the fire. Be careful with mentioning PSR stuff around people like to "look beyond" :) And your "childish comments" remark was also quite the blow. Calling spaces a stupid indentation is neither wrong nor childish per se - to say the least (he merely meant with it that it is an inferior way of doing it these days) ;)
Let's move on.

I am still for doing this at once - in 4.0 - to avoid more tripping stones from 2.x to 3.x. The list is already huge. Until then we can probably all live with the convention, I guess.
The core currently has 569 protected methods in cake3.0.

@ionas

This comment has been minimized.

Contributor

ionas commented Sep 5, 2014

The "stupid call" was against PSR-2 - so not too personal, I hope?

On topic / Off topic: Is there any risk of name clashing of functions and actions in controller domain?
I do as well like the prefix for readability (mostly for controller domain though, to differentiate between actions and non-actions)

p.s.: seems it will either be in 3.0 or 4.0 (no 3.1 - good call)

@ravage84

This comment has been minimized.

Member

ravage84 commented Sep 6, 2014

👎 I like the underscore for readability. Since I don't know the world of tommorow (4.0), I have no opinion about implementing it then or not.

@dereuromark even if we would be in RC, not taking the chance to improve CakePHP before going into stable, after which we are mostly limited in changing stuff, would be a loss.

http://en.wikipedia.org/wiki/Technical_debt

@Phally

This comment has been minimized.

Contributor

Phally commented Sep 7, 2014

👎

I've tried to ditch the underscored stuff for my own code in the past and got all confused. It really helps.

@Diamons

This comment has been minimized.

Diamons commented Nov 21, 2014

protected function MyProtected() { } is a lot more readable than function __MyProtected() { } and follows conventions with other OOP languages such as Java, C++, etc. I say remove the underscore, its easy to miss too.

We're on the cusp of releasing a brand new CakePHP version; it's better to release something that's fully polished and perfect than delaying features until the next version for the sake of maintaining compatibility for applications that are built on a beta.

@dereuromark

This comment has been minimized.

Member

dereuromark commented Nov 21, 2014

@Diamons __MyProtected is wrong anyway (would be private which we don't use). It is _myProtected. And most here would state the opposite regarding readability - as shown above. I don't think we need to warm it up now all over again.

@Diamons

This comment has been minimized.

Diamons commented Nov 21, 2014

I think that further exemplifies my point then. I don't think a single underscore should have the power to make that big of a difference just because of how many problems it can cause and how many can be avoided by simply being explicit by writing the word protected.

private function myPrivate
protected function myProtected
__myProtected
_myProtected

When you're scanning code very quickly or looking through someone else's library or plugin, the first two give you the right information much quicker and leave less room for error.

It seems the major obstacle is that 3.0 is so far along, but it's better to release a polished product for the general public than something that was built half-polished to support mature projects that are built on a non-final release.

@dereuromark

This comment has been minimized.

Member

dereuromark commented Nov 21, 2014

@Diamons Did you read the remarks above your comments? Specifically $this->foo() vs $this->_foo() which in itself disproves your point a 100%. Your argument is flawed.

@Diamons

This comment has been minimized.

Diamons commented Nov 21, 2014

Generally when using a class or library you read the interface or header file before using the methods in that class. It doesn't "disprove" my point "a 100%", it simply means that based on your experiences with programming you've done things one way that work for you while disregarding what programmers using other languages do.

In addition there is no need for your condescending remarks. There's a lot better ways to express your disagreement without the need for comments such as "which in itself disproves your point a 100%. Your argument is flawed". If you want to contribute to an open source project, you should keep in mind there will be people from all areas of the field expressing their opinions based on their own experiences and your own opinion is not more important than any other.

I've edited your comment below to show you how you can express yourself more clearly without coming across as condescending:

@Diamons Did you read the remarks above your comments? Specifically $this->foo() vs $this->_foo()

@dereuromark

This comment has been minimized.

Member

dereuromark commented Nov 21, 2014

I did not mean to be condescending.
I am merely stating that you are twisting the arguments above completely. When looking at the code at first glance, the meaning of what I try to tell you should be visible to you. You cannot distinguish public $this->foo() from protected $this->foo(), but public $this->foo() from protected $this->_foo().
And this seemed to help a lot of (experienced as well as not so experienced) programmers so far and cannot/shouldn't be discarded.

Maybe we understood each other here wrong. We (me and others above) have been talking about the method/property call, not its declaration. But even the declaration is a tiny little bit more clear and verbose this way.

@davidyell

This comment has been minimized.

Contributor

davidyell commented Nov 21, 2014

I still maintain that readability is a poor reason against a change.

@dereuromark

This comment has been minimized.

Member

dereuromark commented Nov 21, 2014

I still maintain that readability is a poor reason against a change.

@davidyell Why would you say that? Readability is one of the most important arguments there are. Why else would we bother with coding standards? The PHP threads don't care about that, neither do any other non-human actors involved. Readability is to make code better understandable for developers (the human actor) and thus reduce errors in code and help to speed up developing in general.

@davidyell

This comment has been minimized.

Contributor

davidyell commented Nov 21, 2014

I say it because it's how I feel. I think the majority of people here are in favour and it just seems to be resistance from the core alone. Please don't take my comments as not understanding, I do. I just think there is something important here which has been missed which is the marketability of the framework.

As a point I raised in my initial proposal. If we want to push the framework on and to compete with the likes of Symfony and Laravel, we need to make some changes to help make the framework marketable. We currently don't market the framework on it's design patterns despite there being lots implemented. The PHP community is moving forward now, faster than ever, and holding onto outdated concepts, however helpful I do not think is productive from a marketing perspective.

I understand this is an aside, as marketing the framework within the PHP community is another issue entirely.

However marketing the framework is all about perception. Having visited my local user group to do a talk about CakePHP I found that pretty much all of the people there thought that CakePHP was old, out of date, had too much magic and isn't following the current PHP community curve. 3.x does address some of these areas. Yet, he we are talking about maintaining something from PHP4 purely because it's easier to read? I do not think it's a good enough argument, because the obvious counter argument is that my IDE shows me function visibility when I use the function. So the whole problem of readability is mute if it just comes down to how you code. Surely the integrity of the code base comes before being able to read something which could be dependant on your choice of coding tool?

I am generalising here somewhat because there have been some excellent points made in this discussion, but the point I've seen raised the most is readability. Apologies to anyone with a point I've missed out.

In summary for me, I'm not a fan of using a PHP 4 convention when I'm coding in PHP 5.6. So for me all my user code will use the regular PHP visibility keywords instead of underscores. I'll just have to accept that like @jtyost2 I'll have to adapt my tool set to deal with the anomaly.

What happens to users who are new to CakePHP and those who are contributing to the core, I don't know. I guess @ADmad will just close their PR within the first 5 minutes 💥

@HavokInspiration

This comment has been minimized.

Member

HavokInspiration commented Nov 21, 2014

Just like for the locale directory location, I stand in the middle of both sets of arguments.
While I understand the readibility one (I personnaly feel that it's easier to read), I also understand what @davidyell states about marketability. When I tell other developers that I use CakePHP, I have the feeling that I live in a cave (again for the reason @davidyell stated) and because "well, the cool kids use Laravel or Symfony" (I know this is dumb but you get the idea).

I'm not going to rewrite the same things you, in my opinion, very well covered.

Let's say for a minute that the convention is changed and every underscored protected method becomes unprefixed (and I understand that some of the core features are built around this, notably what @lorenzo outlined (#4499 (comment)) but let's imagine).
Wouldn't it possible to document the breaking change in the blog post that announces the RC (this is a rhetorical question) as well as provide a tool that would help people to mutate their code to match the new convention, in the same way the Upgrade Shell helps going from one version to the other ? (I never used the shell myself but from my understanding, this is its purpose).
I know it's easier said than done and maybe the timeline is short but wouldn't something like this possible (after having adressed the Entities issues) ?

This way, the framework would go one step further towards standards that are used in most of the "new" stuff - it would also give one less topic for people to bitch about and we wouldn't have to wait until 4.0 to adopt this standards.

Edit : final thoughts and typos

@markstory

This comment has been minimized.

Member

markstory commented Nov 21, 2014

@HavokInspiration Yes it would not be a hard upgrade to remove _ prefixed methods. That is the kind of thing the upgrade shell excels at.

@dereuromark

This comment has been minimized.

Member

dereuromark commented Nov 21, 2014

@markstory I think it will be almost impossible to do. Too many $this->_foo() calls across too many files in core, app and plugin code to be able to automate that with a reasonable amount of accuracy.
You cannot use regexp, as you don't know the scope of the methods in their classes. They could just as well be user land methods or completely other methods in a totally difference class/scope just being named the same.
And we are talking thousands of replacements in larger code bases.
We are assuming here we can change all at once across all code, whatever source. Which might not be possible.

What if there are already public methods there with the same name as well? Creating collisions then even. I know a lot of cases where get() internally uses _get() etc.

The dimensions of this undertaking soon go ballistic IMO.

Upgrading applications on a functional level is already hell. I know from experience how long this can take. I really don't think we should make it any harder, especially in this dimension, prior to 4.0.
The upgrade process between 3 and 4 will most likely be not that big a deal anymore as 2 to 3 now is.
Which brings us back to the very first comment, which states exactly the same.

@HavokInspiration

This comment has been minimized.

Member

HavokInspiration commented Nov 21, 2014

@dereuromark When you put it that way...
And I didn't thought of "composed" method name (I don't know if that is called that way, I'm talking about methods which name is built from a variable). They are a few in the core code.

Well, I'm out of solutions except waiting for 4.0 (or doing it for the core and force the hand of userland code to update to fit the new standard and have their methods match the new core methods names which could create quite a mess).

Either way, it's fine by me. Plus, with what @dakota said about PSR-2, it's not as if the standard strictly forbid method prefixed with a _

Edited

@oldskool

This comment has been minimized.

Contributor

oldskool commented Nov 21, 2014

👎 I actually prefer the prefixes. It gives a clear visual representation of the method/property visibility. I also use this a lot with similarly named properties. (Like protected $_debug in my "main class" vs private $__debug in an extending class).

@markstory

This comment has been minimized.

Member

markstory commented Nov 21, 2014

@ceeram That is a reasonable option for sure. We could also stop making more prefixed methods and fix the ones we have incrementally.

@htstudios

This comment has been minimized.

Contributor

htstudios commented Nov 21, 2014

If you want to help with marketability visit my cake bake look upgrade RfC, help with crud v4 (scaffold and RAD are huge for marketability), docs, examples, tutorials, community plugins (like asset dispatcher, search, bootstrap) and docs or port croogo and quickapps to 3.x.

Each of those will help tons more.

I really don't want inconsistency - 2nd to that private function __foo() might be not much better to read but $bar = $this->__foo() is!

@jameswatts

This comment has been minimized.

Contributor

jameswatts commented Nov 22, 2014

I've stayed out of this discussion until now, given I was somewhat a match to @davidyell's fire. In fact, I completely agree with David's important point on marketability. Given my involvement in the core and the community, as well as my exposure to other developers and projects through CakeDC, I come across the situation he described at his meetup a lot. Other developers pick out these "legacy" aspects of CakePHP as a sign of it's old PHP4 agenda, usually with the objective of promoting something else, and most of the time to someone not proficient enough in programming (a client) to judge the argument (hence, the other thing wins). You can fight and discuss it all you like, while it still remains another point of notoriety to deflect. If you work professionally in a company which focuses on a single in-house product, you're probably not exposed to this harsh reality. And let's all agree, at least, that part of CakePHP's popularity is driven by the commercial ecosystem that has grown around it. If we're only considering the framework as a fun tool to build an app which manages your music collection, then it's going to be hard to see eye to eye on this.

Following standards which have been agreed upon by a large group of fellow framework developers means that we align ourselves with the general consensus. Even though I say this, while totally objecting to using spaces over tabs. Go figure, but I'd argue that indentation has nothing to do with interoperability, my reasons here.

However, this is a double edged sword. I use vim (queue dramatic music). Little do I know of visibility when I call a method, so the convention is useful to me, i.e, I am in favor of it, at least in my userland code. I would also be one to rest on the argument that a successful use case is superior to an opinionated convention. But, as David pointed out, there's a bigger picture to also consider. If we get stuck in our ways, we do risk the possibility of becoming black sheep (no racial pun intended). Although there's no technical loss either way, it (_ and __) seems to been generally seen in the PHP community as a poor design decision (absolutely no data to back that up, it's just an impression, so I could be wildly wrong). Maybe someone does have data on it, if so, please share.

Reality is that everyone's opinion here is valuable, as we're the end users of this awesome framework. But some things are bigger than an opinion, and which have a stronger consequence. I'm also therefore in favor of being able to talk about and discuss CakePHP with other framework developers without having to touch upon these issues, you can bet I'm happy that the arguments about damn arrays are over and done with. I also think that there's no excuse to not take advantage of this upcoming major release to make the hard changes.

CakePHP must be a tool which works for developers, but I also feel it should aim to be competitive, and the way it's viewed and considered in the PHP community at large is a part of that. At the end of the day, if the developers were to bail, there would be no more project. The more we row against the tide, the more effort it will be to stay afloat.

@ADmad

This comment has been minimized.

Member

ADmad commented Nov 22, 2014

I personally would be very sad to see us making decisions based on people's invalid / misplaced perceptions.

Marketability is definitely important but so it sticking of our opinions / decisions which are backed by facts / logic.

@justinyost

This comment has been minimized.

Contributor

justinyost commented Nov 22, 2014

CakePHP for sure has value in having opinions (convention over configuration and strong opinions are all good things). However there is also value in knowing when recognizing that going against the rest of the group isn't good.

Loadsys internally doesn't agree with the core team's decision regarding tabs vs. spaces, underscored functions, etc - but at the same time Cake can have their standard and we can have ours and everything plays nicely.

Marketability has some value outside of pure logic and regardless of the logic of some coding standard there may be value in having a standard that appeals to people because they think it looks better (regardless if people think there is enough evidence one way or the other). In other marketing and the "selling" of CakePHP to people outside of that community sometimes involves appeals that aren't always driven by logic.

Sometimes I have to sell CakePHP in a way that goes like this: so yeah that is still done that way but you don't have to deal with it because you'll probably never see it because who sees the protected _ functions until you do see them. Part of this is things like I don't know why a decision is made sometimes (either out of my ignorance or lack of knowledge not trying to blame the core team here) so I have muddle through about how I don't know why some decision was made even though "everybody else does it this other way".

One of Cake 3's goal as I understood it was to try adopting some of the broader standards of the PHP community to deal with this sort of thing.

I would totally get the waiting to deal with this till Cake4 that's very reasonable.

TLDR - CakePHP having opinions is good, Marketing matters and is partially driven by things other than logic, Cake3 said it was going to adopt broader standards to limit some of this sort of conflict.

Minor edits made.

@rchavik

This comment has been minimized.

Member

rchavik commented Nov 22, 2014

You guys, I still need help with croogo 3. It does not even boot up.

@bravo-kernel

This comment has been minimized.

Contributor

bravo-kernel commented Nov 24, 2014

Stripped to bare minimum to prevent TLDR.

First of all... I am no expert, hate spaced-tabs, follow every Cake convention and have never regretted it once. On the other hand I strongly empathize with @jameswatts: tough decisions should be made if it benefits the long-term strategy of the framework.

In that light I 👍 adopting all of the (most commonly used) standards since I believe it will benefit:

  • the selection process of potential/future framework-users
  • lead to more (next generation) CakePHP end-users in the long run
  • hopefully another 10 (dominant) years of the framework

The above based on the unfortunate behavior of natural selection processes where e.g. a top-5 of PHP frameworks is first sanitized during a very short time-period, generally based on that instant "like" or instant "next!!!" feeling. Everything feeling off mostly likely ending up in the latter category. Only the remaining candidates becoming subject of further in-depth investigation (where Cake would come out good naturally). IMHO it would be a shame to lose a generation of potential users due to "some tabs".

barbie

If changing things, however illogical they might seem, means the framework will outlive all others then by all means... break all the things. I will even start using spaces.

@burzum

This comment has been minimized.

Member

burzum commented Nov 27, 2014

I like the _ and __, like @jameswatts already pointed out when you're not using a fancy IDE you won't know the visibility of a method call on an object just by looking at it. Even if your IDE supports it, you usually have to hover over it first.

The only people that will complain about this are people wo don't like CakePHP any way and try to come up with something and babble about this being php4ish. Given the fact this is a pretty invalid argument I wouldn't care much about them. Haters will always hate. And you can still ignore that convention in your app.

I agree as well with @ADmad about the tabs. Stick to tabs. Tabs are thought for indention, (http://en.wikipedia.org/wiki/Tab_key) tabs can be configured in most editors to match X spaces. So I don't see a single reason to go for spaces except you love to hammer your space key on your keyboard a lot... (Yes, I know some editors can replace a tab press with spaces.)

Also there is the phrase "Millions of flies can't be wrong: Shit tastes." Just because thousands of people like spaces doesn't make them right from a technical point of view.

@ravage84

This comment has been minimized.

Member

ravage84 commented Dec 3, 2014

It really feels bad to write the following lines, but I feel it is necessary, unfortunately.
This time, it is personal.

About two years ago I wanted/needed to choose a PHP framework and I was sure this decision would have an influence on my career. I checked the usual suspects, including CakePHP.

The reasonable and sane principles, ideas and conventions behind CakePHP were the main reasons why I chose it. This especially includes not adopting PSR-2.

Yes, PSR-2 is a commonly accepted and agreed standard, but being a “commonly accepted and agreed standard” doesn’t make it any good. The opposite, I simply call it absurd, even though the basic intentions behind were honest.

You may think what you want about me, but I feel very strong about this, even though being "just" two years with the project and actively contributing to it.

That’s why I make it official:

If the core team decides to adopt the full PSR-2 standard – including space-indentation – I will cease contributing to each and every CakePHP related repo/branch, which adopts this standard effective immediately.

Well, as it seems the core team has already decided on this behind closed doors:
cakephp/migrations#27

I thought decisions about CakePHP are not made just by the core team only and are discussed openly?
I'm very disappointed.

Adopting a coding standard for the sake of others, being just standard-compliant and/or for marketing reasons instead of ourselves is a total betrayal.

And I will not support this absurdity.

It is a very sad day for me... 😞

@jippi

This comment has been minimized.

Member

jippi commented Dec 3, 2014

where is the popcorn?

👍

@josegonzalez

This comment has been minimized.

Member

josegonzalez commented Dec 3, 2014

@ravage84 We are sorry to see you go, but a bit of background (take this all as unofficial, and note the fact that said PR is still a PR, and not in master. We're still discussing it in the core.):

We had long internal discussions - both on IRC and the private mailing list - about whether to move to PSR-2 or not. Most people were against because of personal preference - I am among those - but we also realize that this is a personal preference. For large changes such as this, there is always an internal vote as to how we should approach something. Sometimes we don't make it a public vote, not because your opinions don't matter, but because we are ever-so-slightly more informed about how CakePHP community - and PHP Community at large - works as a whole. CakePHP is a framework composed of quite a diverse set of developers - we have developers on every contintent but Antarctica, and across most major timezones - so believe me when I say that people were very much up in arms about this. Rather than have to deal with the discussion about how:

  • (1.3) CakePHP uses Simpletest instead of PHPUnit
  • (2.x) CakePHP uses arrays instead of objects for their lame "ORM"
  • (2.x) CakePHP supports PHP 5.2 and doesn't use namespaces
  • (3.x beta) CakePHP sucks because all other projects use spaces and K&R-style brackets

We made - difficult and time-consuming - changes to the framework. Marketing is one reason why we make changes. But another large reason is the core developers being tired of having petty arguments about how one solution is right over another, and worse, having to justify why we still do it the way we do when the majority of the PHP community does it another way.

We can quite easily stick to our ways, but why be stubborn? This is a framework-level change from PSR-2, proposed by the PHP-FIG. Thats the PHP Framework Interop Group, of which we are a voting member. The FIG has decided - as a whole, and with dissenting members - that this is the standard to be used by framework developers in their code. If you would like to write your own code with 8 tabs and brackets starting after 3 spaces, go ahead. Its your code.

The CakePHP framework code - and all the official libraries - will be moved to PSR-2. If you see this as a betrayal of your sensibilities, we apologize, but at the end of the day you are free to write your code with whatever style you want.

I personally had this very same argument at work 3 years ago, and we decided on 2 spaces (now 4, because PSR-2). There are many technical reasons why tabs are nicer, but nicer still is having a convention across an entire language, and even nicer is having that same convention across multiple languages (we deploy 8 languages in production). Not relevant to this discussion, but just showing you that I - as the author of the first PSR-2 pull request - hate space-based indentation as well. Golang format ftw.

Note: One reason I do like it is every other package I write code for or contribute to uses PSR-2 for styling changes, and I have a style-checker enabled in my editor. Switching just for a single project is always a pain in the ass, but I currently do it when writing code for CakePHP core (however not often that is). This is one less thing I need to worry about configuring in my editor, so thats nice.

@cakephp cakephp locked and limited conversation to collaborators Dec 3, 2014

@burzum

This comment has been minimized.

Member

burzum commented Dec 3, 2014

@josegonzalez I'm as well very disappointed about this. Why is this even discussed here if it already has been decided?

Like I said before, just that millions of flies say shit tastes doesn't make it any better. Same with spaces vs tabs. Tabs are thought for this purpose (indention) spaces are not. Period. Now let the pro-spaces I'm-to-lazy-and-proud-to-change-my-year-old-wrong-formatting faction come up with a technical valid counter argument for spaces. "Just because most use them" is clearly not a valid one. Period. Haters will always hate and I bet a few dollar on the fact that the haters will find something else to complain about. Will you agree to the next silly thing based on some haters who never would use Cake any way as well then?

Why not turning this silly discussion against them? Put something in the FAQ like "We're using tabs because they we're invented for that purpose and think that is the right way to do. We don't agree to the majority of using spaces without a solid technical reason." Followed by a Wikipedia (or a better reference) link to the page that explains tabs.

Also tabs vs spaces is not a code breaking change. I don't get it why this has not been made optional in this fancy standard. IMHO a sub group of the PSR group is just trying to enforce their convenience on others. Spaces vs tabs won't make any code less good "interoperable". It's a game about political power and influence and clearly not based on a sane decision based on technical arguments.

@ceeram

This comment has been minimized.

Member

ceeram commented Dec 3, 2014

This change is about something that our targeted audience, PHP developers, prefer, not about spaces or tabs being right or wrong. Also a change that suits both our "conventions over configurations" and "community driven" philosophy very well.

@lorenzo

This comment has been minimized.

Member

lorenzo commented Dec 23, 2014

Closing as actions are being taken towards adopting all of PSR-2

@lorenzo lorenzo closed this Dec 23, 2014

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.