Coding Standards

Yii 2.0 General Discussions
1

Just wondering, will there be a set coding standard for version 2.0?

The current Yii codebase largely follows Symfony 1 coding standards, with a few small exceptions.

The Symfony 2 coding standards look terrible, as does their reasoning for coming up with a new standard in the first place.

For one, they make you use K&R style instead of Allman style.

According to Wikipedia, the justification for K&R style is this:

"because in the original C language, argument types needed to be declared on the subsequent line, whereas when no arguments were necessary, the opening brace would not appear in the same line with the function declaration"

In other words, the main justification for this style, is whitespace-sensitivity in some outdated language specification.

As for Allman style:

"Advantages of this style are that the indented code is clearly set apart from the containing statement by lines that are almost completely whitespace, improving readability, and the closing brace lines up in the same column as the opening brace, making it easy to find matching braces"

What do you know, there is actual practical reasoning for this style.

The new coding standard page for Symfony 2 states:

“Remember that the main advantage of standards is that every piece of code looks and feels familiar, it’s not about this or that being more readable.”

The new standard is definitely not more readable, they’re right about that much - but if they wanted it to “look and feel familiar”, they should have stuck with their own existing standards, shouldn’t they?




if (true === $dummy)

What exactly is the argument for writing all your comparisons backwards now? Code isn’t hard enough to read as it is?? I don’t get it.

I hope you stick with existing Yii standards - it’s the only code standard that ever made any sense to me.

2

For Yii 1 there is no coding standard set. We’re following a standard most code is written with. I guess the style itself came from Prado framework.


if (true === $dummy)

This one is called Yoda Condition because of spelling. The goal is to prevent accidental assignment in conditions:


if ($dummy = true)

I don’t like it because of Yoda-style.

Personally I’m for coding standards and for some changes (spaces in comparisons, brackets for one-line else etc.) for Yii 2 as well as for publishing actual rules we’re following.

3

I would prefer spaces between assignments, concats etc.




$var.'text'.$more.implode(...)


$var . 'text' . $more . implode(...)

Tend to find it easier to notice a mistype.

4

Me too. Spaces are beautiful. :)

However, if the majority of Yii developers doesn’t use spaces between operators, then I would rather have consistency

5

Anyway, coding standards isn’t the topic we need to focus too much. There will be change to correct these when we’ll have alpha version.

6

FWIW - I dislike spaces in concatenation strings, because it is harder to read when everything is spread across the page like that. The brain takes it all in at once and sorts it all out. When you space it out I have to look at and process each piece individually.

7

Mindplay, thank you for the link to "Symfony 1 coding standards". I definitely like that one e.g. indentation: "Never use tabulations in the code. Indentation is done by steps of 2 spaces".

I think this is a reasonable compromise between (interpreted language) performance and code readability. Of course this was more important in the ages of so called structured (procedural) programming where the standard eight column tab spacing was totally out of question (my opinion).

Besides that I have to say that I really like the compact style of the current framework core (but personally I put spaces around assignments and operators).

In general e.g for contributed code I support braces on separate lines but I don’t think braces should surround single statements. That is the kind of code we may choose to write as consultants, in order to avoid possible maintenance mistakes in the future, by less experienced personnel. But it really makes the code look ugly and less transparent.

Neverthless, K&R style braces may sometimes be good for small code snippets to not having them look more complicated than they are.

Just my 2 ören ;)

/Tommy

8

Using braces even for one liners has one big "+1" - many code re-formatters do not align such constructs without the braces.

Second - braces tend to show that this is individual block of code under condition or loop.

As for tabs vs spaces - I personally use tabs and don’t like spaces because many editors treat those space idents, well, as spaces. Navigation through code with arrows becomes tedious. Tabs, on the other hand, almost every IDE or feature-full editor are handled very well and advanced navigation helps very well (usually it’s called something like “Smart tabs” or alike). For example in PHPStorm this advanced fast navigation works only with tabs. Use spaces and it becomes much harder to navigate and edit the code (not writing new code, but there are few caveats too).

I know only one IDE that did transparent space conversion to tab-like navigation - Zend Studio 5.5 and lower. It had a feature "Treat spaces as tabs" and "Tab size [ X ]" made those spaces work like tabs of specified width in editor, but the file was formatted with spaces, where 1 virtual tab was X spaces.

Why not just use common size tab (4 spaces) and use the rule that aligning of the levels, aligned arrays and so on takes 1 tab. Code looks good and everyone can make tab the size they like - be it 2, 6 or 8 spaces.

To illustrate what I mean:




	/**

	 * @param $step

	 * @return void

	 */

	final public function actionCheckProxy()

	{

		if ($this->account->proxy_active && count($this->account->proxyLists)) {

			foreach ($this->account->proxyLists as $proxy) {

				if ($proxy->active == 'no') {

					continue;

				}

				$options = array(

					CURLOPT_URL             => 'https://example.com/checkproxy?time='.urlencode(microtime(true)),

					CURLOPT_RETURNTRANSFER  => true,

					CURLOPT_FOLLOWLOCATION  => true,

					CURLOPT_CONNECTTIMEOUT  => 15,

					CURLOPT_TIMEOUT         => 15,

					CURLOPT_SSL_VERIFYHOST  => false,

					CURLOPT_SSL_VERIFYPEER  => false,

					CURLOPT_PROXYTYPE       => CURLPROXY_HTTP,

					CURLOPT_HTTPPROXYTUNNEL => false,

					CURLOPT_PROXY           => $proxy->host,

					CURLOPT_PROXYPORT       => $proxy->port,

					CURLOPT_PROXYAUTH       => CURLAUTH_BASIC,

					CURLOPT_PROXYUSERPWD    => $proxy->user.':'.$proxy->password

				);

				$ch = curl_init();

				curl_setopt_array($ch, $options);

				$content = curl_exec($ch);

				$errno = curl_errno($ch);

				$error = curl_error($ch);

				curl_close($ch);

				unset($ch);


				$x = strpos($content, $proxy->host);


				if ($errno !== 0) {

					Yii::log(

						$content.' '.$proxy->host.' '.var_export($x, true).PHP_EOL

							.$errno.' - '.$error.PHP_EOL

							.print_r(getReadableCurlOptions($options), true), 

						'error'

					);

					$proxy->active = 'no';

					$proxy->update(array('active'));


					$message = sprintf(

						'Connectivity check of the proxy has failed for transaction %s.<br>

							Proxy '.$proxy->host.' has not responded for account '.$this->account->login.'<br>

							Proxy was disabled',

						$this->trans->id

					);

					if ($errno !== 0) {

						$message .= '<br>cURL error was recorded: '.$errno.' - '.$error;

					}

	

					$emails = Params::multiple('supportnotify');

					if (is_array($emails) && count($emails) > 0) {

						try {

							foreach ($emails as $email) {

								$this->_sendMail($email, sprintf($message, $this->trans->id), 'Proxy down alert!','admin_notify');

							}

						} catch (Exception $e) {

						}

					}

				} else {

					$this->proxy = $proxy;

					if ($this->current_step != null && isset($this->steps[$this->current_step])) {

						

					}

					break;

				}

			}

		}

		if ($this->proxy instanceof ProxyList) {

			Operations::addEntry($this->trans, 'Using proxy '.$this->proxy->host.' for the requests');

		} elseif ($this->account->proxy_active) {

			Operations::addEntry($this->trans, 'Proxy use flag is active, but no active proxy available');

		} else {

			Operations::addEntry($this->trans, 'Proxy usage is disabled');

		}

	}

All this code has been aligned with 4 spaces tab. Here is an actual screenshot: https://www.file.lu/imageh/0/4/xvu48cj5kkqm.png

9

Always, always and always uses spaces instead of tabs!!

If PHPStorm does not handle it, it’s badly written. The author should fix it. :)

NetBeans does handle it seamlessly. I can’t tell the difference.

By the way: Tabs Are Evil ;)

Religious war? You bet!

What really matters is that whatever you use, be consistent.

I prefer spaces, and editors which knows how to handle them.

Most can be configured to handle it, if they don’t by default.

10

A big "-1" here. And those code formatters are garbage.

Actually, I quite like the Zend Framework coding standard, with the exception of braces for single line statements ;)

11

There is one "great" +1 point in favor of braces for oneliners… debugging!

I don’t know for other debuggers but in Netbeans with xdebug you cannot debug the oneliner if there are no braces around it… you cannot even set a brakpoint there…

12

In my working practice I haven’t seen any code formatting tools that has handled one liners without braces. They left them as is.

Second - maybe you have used to writing code like that and reading, but for people who write code with braces Yii core code is just f*****g hard to read. Especially places like: if (cond) do stuff;

As for tabs vs spaces - the link you gave - the reasoning is based on the coding style. There alignment is done like that:




$result = a(someLongArgument1,

            someLongArgument2,

            someLongArgument3);

Yep, in this case tabs will break things. But thats C style code.

What I prefer is style like this:




$result = a(

    someLongArgument1,

    someLongArgument2,

    someLongArgument3

);

It’s closer to JavaScript style. Who of us doesn’t work with JS daily? I bet almost everyone. It’s much easier to work with similar code style.

And consider code like this:




$model = MyModelName::model()->scope()->findAll(array('condition' => 'somecondition',

                                                      'order' => 'field ASC',

                                                      'limit' => 1));

I say, bullshit. In IDE it looks nice. Then open it in nano in console… Or MC editor. Not to say that it just reads not really well.

Apply this to a multi-level array defined in code and it will look ugly.




$model = MyModelName::model()

    ->scope()

    ->findAll(

        array(

            'condition' => 'somecondition',

            'order' => 'field ASC',

            'limit' => 1

        )

    );


$somedata = functionCall(

    array(

        'some' => 1,

        'some2' => 2,

        'some3' => 3,

    )

);

In this code style tabs do not break anything. Indentation always uses 1 tab. And no mater is tab 2 spaces or 16 - it will always be indented as much as 1 tab.

13

Braces exist in order to group multiple lines, no multiple lines, no braces.

About code formatters, I never noticed issues with single line statements (maybe because I correctly format most of my code from the beginning), but if your code formatters are broken and can’t handle single line statements, I think that you should search for better formatters :) I.E. It’s not really a PHP IDE, but I remember that the Visual Studio code formatter does correctly handle single line statements.

Did you try the Netbeans code formatter?

14

I am glad that you are not in charge of the new Yii coding standards, Ekerazha - you are a bit extreme! :lol:

I tend to use braces even for single liners for at least three reasons:

They makes it foolproof because if you ever need an extra line in the same scope, the braces are already there.

They make it visually clearer, flow and scope.

They aid in debugging - (NetBeans and others).

Just because you can get away with something, doesn’t mean that you should do it.

I use it for the same reason I am (trying to) use the Yoda notation when I can:


if(null === $somevar) {

  doStuff()

}

Simply because it’s safer that way.

15

Can’t you add them when/if you add extra lines? :D In my opinion, readability is given by indentation, not braces (indeed, I find that code with an abuse of braces is less readable). However, in the end it’s probably a matter of taste.

Braces exist in order to group multiple lines, no multiple lines, no braces. That’s my creed :D

16

Really, haven’t you ever had to write code under pressure when fixing something bad and I needs to be done like yesterday and you do it at 01 PM in the night? (a little extreme, but I had to do something similar at least a dozen times because of changes in external API or urgent changes in the business logic). I bet you haven’t :) That’s the time when forgetting to add braces is a common mistake and you sit, debug and slap yourself with something when you find out it’s those f*****g braces you have missed to add.

I’m all +1 with jacmoe - foolproof, visual appearance and debugging.

And I hate when people are trying to press on with code standards from other languages. They just don’t reflect the way PHP is and how we write code.

I had to deal with many code styles and I have to say that many of them are just too formal and make you to actually make effort to follow the code style. And no IDE auto formatting is flawless - they all have some caveats that break your dreamworld (I’m writing a big bug report on PHPStorm abilities of code formatting witch just doesn’t want to do some things and doesn’t look at the scope of the code at all resulting is some really weird formatting - and PHPStorm is one of the best IDE out there).

And I have to add that code should be easy readable. Frameworks is Open Source, so people look at it’s code. Following guidelines like not using braces for one liners, single-line function formatting in one line, using ternary operator too much and other similar suggestions are just evil. It makes code less readable and sometimes cryptic. It’s not only you who is going to read that code - it will be thousandths, tens of thousands of other people and many have their views on how the code should be written. So it should be as much to academic as possible with the thought of the language and common practices in mind.

17

Interesting, but this has nothing to do with Yii :D

18

It has - it’s good practice to follow framework in coding standards.

And someone has to develop Yii too ;)

19

I think you should leave this discussion.

The core team should be able to code the way they know better and are more productive, focusing on what really matters.

I would agree with this discussion if the currect code was not well written or readable. But it is.

You wouldn’t like someone coming to your work and telling you how you should write your code, would you ?

20

Code is easy enough to get wrong as it is.

That syntax makes it just a bit harder to make mistakes.

Consider this:


if(true = $dummy)

Error!

But this:


if($dummy = true)

No error! But most probably wrong.

I wouldn’t like it at first, but people are being told how to write their code all the time.

That’s what a Coding Standard means.

The code in Yii is well written and readable.

Doesn’t mean that it can’t get better.

Any discussion about coding standards and conventions is both interesting and worth the while.

IMO.

Case overruled. :)