Commit 0824e1c1 by Alexander Makarov

Removed old reviews and code style (it's on github wiki now)

parent b9a83551
Yii2 code standard
==================
This code standard is used for all the Yii2 core classes and can be applied to
your application in order to achieve consistency among your team. Also it will
help in case you want to opensource code.
PHP file formatting
-------------------
### General
- Do not end file with `?>` if it contains PHP code only.
- Do not use `<?`. Use `<?php` instead.
- Files should be encoded in UTF-8.
- Any file that contains PHP code should end with the extension `.php`.
- Do not add trailing spaces to the end of the lines.
#### Indentation
All code must be indented with tabs. That includes both PHP and JavaScript code.
#### Maximum Line Length
We're not strictly limiting maximum line length but sticking to 80 characters
where possible.
### PHP types
All PHP types and values should be used lowercase. That includes `true`, `false`,
`null` and `array`.
### Strings
- If string doesn't contain variables or single quotes, use single quotes.
~~~
$str = 'Like this.';
~~~
- If string contains single quotes you can use double quotes to avoid extra escaping.
- You can use the following forms of variable substitution:
~~~
$str1 = "Hello $username!";
$str2 = "Hello {$username}!";
~~~
The following is not permitted:
~~~
$str3 = "Hello ${username}!";
~~~
### String concatenation
Add spaces around dot when concatenating strings:
~~~
$name = 'Yii' . ' Framework';
~~~
When string is long format is the following:
~~~
$sql = "SELECT *"
. "FROM `post` "
. "WHERE `id` = 121 ";
~~~
### Numerically indexed arrays
- Do not use negative numbers as array indexes.
Use the following formatting when declaring array:
~~~
$arr = array(3, 14, 15, 'Yii', 'Framework');
~~~
If there are too many elements for a single line:
~~~
$arr = array(
3, 14, 15,
92, 6, $test,
'Yii', 'Framework',
);
~~~
### Associative arrays
Use the following format for associative arrays:
~~~
$config = array(
'name' => 'Yii',
'options' => array(
'usePHP' => true,
),
);
~~~
### Classes
- Classes should be named using `CamelCase`.
- The brace should always be written on the line underneath the class name.
- Every class must have a documentation block that conforms to the PHPDoc.
- All code in a class must be indented with a single tab.
- There should be only one class in a single PHP file.
- All classes should be namespaced.
- Class name should match file name. Class namespace should match directory structure.
~~~
/**
* Documentation
*/
class MyClass extends \yii\Object implements MyInterface
{
// code
}
~~~
### Class members and variables
- When declaring public class members specify `public` keyword explicitly.
- Variables should be declared at the top of the class before any method declarations.
- Private and protected variables should be named like `$_varName`.
- Public class members and standalone variables should be named using `$camelCase`
with first letter lowercase.
- Use descriptive names. Variables such as `$i` and `$j` are better not to be used.
### Constants
Both class level constants and global constants should be named in uppercase. Words
are separated by underscore.
~~~
class User {
const STATUS_ACTIVE = 1;
const STATUS_BANNED = 2;
}
~~~
It's preferable to define class level constants rather than global ones.
### Functions and methods
- Functions and methods should be named using `camelCase` with first letter lowercase.
- Name should be descriptive by itself indicating the purpose of the function.
- Class methods should always declare visibility using `private`, `protected` and
`public` modifiers. `var` is not allowed.
- Opening brace of a function should be on the line after the function declaration.
~~~
/**
* Documentation
*/
class Foo
{
/**
* Documentation
*/
public function bar()
{
// code
return $value;
}
}
~~~
Use type hinting where possible:
~~~
public function __construct(CDbConnection $connection)
{
$this->connection = $connection;
}
~~~
### Function and method calls
~~~
doIt(2, 3);
doIt(array(
'a' => 'b',
));
doIt('a', array(
'a' => 'b',
));
~~~
### Control statements
- Control statement condition must have single space before and after parenthesis.
- Operators inside of parenthesis should be separated by spaces.
- Opening brace is on the same line.
- Closing brace is on a new line.
- Always use braces for single line statements.
~~~
if ($event === null) {
return new Event();
} elseif ($event instanceof CoolEvent) {
return $event->instance();
} else {
return null;
}
// the following is NOT allowed:
if(!$model)
throw new Exception('test');
~~~
### Switch
Use the following formatting for switch:
~~~
switch ($this->phpType) {
case 'string':
$a = (string)$value;
break;
case 'integer':
case 'int':
$a = (integer)$value;
break;
case 'boolean':
$a = (boolean)$value;
break;
default:
$a = null;
}
~~~
### Code documentation
- Refer ot [phpDoc](http://phpdoc.org/) for documentation syntax.
- Code without documentation is not allowed.
- All class files must contain a "file-level" docblock at the top of each file
and a "class-level" docblock immediately above each class.
- There is no need to use `@return` if method does return nothing.
#### File
~~~
<?php
/**
* @link http://www.yiiframework.com/
* @copyright Copyright (c) 2008 Yii Software LLC
* @license http://www.yiiframework.com/license/
*/
~~~
#### Class
~~~
/**
* Component is the base class that provides the *property*, *event* and *behavior* features.
*
* @include @yii/docs/base-Component.md
*
* @author Qiang Xue <qiang.xue@gmail.com>
* @since 2.0
*/
class Component extends \yii\base\Object
~~~
#### Function / method
~~~
/**
* Returns the list of attached event handlers for an event.
* You may manipulate the returned [[Vector]] object by adding or removing handlers.
* For example,
*
* ~~~
* $component->getEventHandlers($eventName)->insertAt(0, $eventHandler);
* ~~~
*
* @param string $name the event name
* @return Vector list of attached event handlers for the event
* @throws Exception if the event is not defined
*/
public function getEventHandlers($name)
{
if (!isset($this->_e[$name])) {
$this->_e[$name] = new Vector;
}
$this->ensureBehaviors();
return $this->_e[$name];
}
~~~
#### Comments
- One-line comments should be started with `//` and not `#`.
- One-line comment should be on its own line.
Yii application naming conventions
----------------------------------
Other library and framework standards
-------------------------------------
It's good to be consistent with other frameworks and libraries whose components
will be possibly used with Yii2. That's why when there are no objective reasons
to use different style we should use one that's common among most of the popular
libraries and frameworks.
That's not only about PHP but about JavaScript as well. Since we're using jQuery
a lot it's better to be consistent with its style as well.
Application style consistency is much more important than consistency with other frameworks and libraries.
- [Symfony 2](http://symfony.com/doc/current/contributing/code/standards.html)
- [Zend Framework 1](http://framework.zend.com/manual/en/coding-standard.coding-style.html)
- [Zend Framework 2](http://framework.zend.com/wiki/display/ZFDEV2/Coding+Standards)
- [Pear](http://pear.php.net/manual/en/standards.php)
- [jQuery](http://docs.jquery.com/JQuery_Core_Style_Guidelines)
\ No newline at end of file
Alex's Code Review, 2011.11.12
==============================
Overall hierarchy
------------------
Generally is OK. Like that `Object` and `Component` are now separated.
I've generated 2 diagrams under `docs/` to see it better as a whole.
> The purpose of separating `Object` from `Component` is to make `Object`
> a super-light base class that supports properties defined by getter/setters.
> Note that `Component` is a bit of heavy because it uses two extra member
> variables to support events and behaviors.
Object
------
### property feature
Is it OK that `canGetProperty` and `canSetProperty` will return `false` for real
class members?
> Added $checkVar parameter
### callbacks and expressions
We're using 5.3. What's the reason to support `eval()` in `evaluateExpression` if
we have anonymous functions? Is that for storing code as string inside of DB (RBAC)?
If we're going to get rid of `eval()`, cosider remaning method to something about callback.
If not then we definitely need to use anonymous functions in API docs and the guide
where possible.
> The purpose of evaluateExpression() is to provide a way of evaluating a PHP expression
> in the context of an object. Will remove it before release if we find no use of it.
>> mdomba:
>> As eval() is controversial, and anonymous functions can replace all Yii 1 usage of eval()
>> how about removing it from the beginning and add it only if we find it necessary.
>> This way we would not be tempted to stick with eval() and will be forced to first try to find alternatives
### Object::create()
#### `__construct` issue
Often a class doesn't have `__construct` implementation and `stdClass` doesn't have
default one either but Object::create() always expects constructor to be
defined. See `ObjectTest`. Either `method_exists` call or `Object::__construct` needed.
> Added Object::__construct.
#### How to support object factory like we do with CWidgetFactory?
~~~
class ObjectConfig
{
public function configure($class)
{
$config = $this->load($class);
// apply config to $class
}
private function load($class)
{
// get class properties from a config file
// in this method we need to walk all the
// inheritance hierarchy down to Object itself
return array(
'property' => 'value',
// …
);
}
}
~~~
Then we need to add `__construct` to `Object` (or implement `Initalbe`):
~~~
class Object
{
public function __construct()
{
$conf = new ObjectConfig();
$conf->configure($this);
}
}
~~~
This way we'll be able to set defaults for any object.
> The key issue here is about how to process the config file. Clearly, we cannot
> do this for every type of component because it would mean an extra file access
> for every component type
#### Do we need to support lazy class injection?
Currently there's no way to lazy-inject class into another class property via
config. Do we need it? If yes then we can probably extend component config to support
the following:
~~~
class Foo extends Object
{
public $prop;
}
class Bar extends Object
{
public $prop;
}
$config = array(
'prop' => array(
'class' => 'Bar',
'prop' => 'Hello!',
),
);
$foo = Foo::create($config);
echo $foo->bar->prop;
// will output Hello!
~~~
Should it support infinite nesting level?
> I don't think we need this. Foo::$prop cannot be an object unless it needs it to be.
> In that case, it can be defined with a setter in which it can handle the object creation
> based on a configuration array. This is a bit inconvenient, but I think such usage is
> not very common.
### Why `Event` is `Object`?
There's no need to extend from `Object`. Is there a plan to use `Object` features
later?
> To use properties defined via getter/setter.
Behaviors
---------
Overall I wasn't able to use behaviors. See `BehaviorTest`.
### Should behaviors be able to define events for owner components?
Why not? Should be a very good feature in order to make behaviors customizable.
> It's a bit hard to implement it efficiently. I tend not to support it for now
> unless enough people are requesting for it.
### Multiple behaviors can be attached to the same component
What if we'll have multiple methods / properties / events with the same name?
> The first one takes precedence. This is the same as we do in 1.1.
### How to use Behavior::attach?
Looks like it is used by `Component::attachBehavior` but can't be used without it.
Why it's public then? Can we move it to `Component?`
> It's public because it is called by Component. It is in Behavior such that
> it can be overridden by behavior classes to customize the attach process.
Events
------
Class itself looks OK. Component part is OK as well but I've not tested
it carefully. Overall it seems concept is the same as in Yii1.
### Event declaration: the on-method is mostly repetitive for every event. Should we choose a different way of declaring events?
Maybe. People complained previously about too many code for event declaration.
### Should we implement some additional event mechanism, such as global events?
Why use two different implementations in a single application?
Exceptions
----------
- Should we convert all errors, warnings and notices to exceptions?
> I think not. We used to do this in early versions of 1.0. We found sometimes
> very mysterious things would happen which makes error fixing harder rather than
> easier.
Coding style
------------
See `docs/code_style.md`.
\ No newline at end of file
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment