AlumnForce / coding-conventions

[DEPRECATED] Style guide & coding conventions for AlumnForce projects
Creative Commons Attribution 4.0 International
11 stars 2 forks source link

[Proposal] Avoid redundant Phpdocs and comments #9

Open nunomaduro opened 6 years ago

nunomaduro commented 6 years ago

This issue addresses the usage of comments/phpdocs.

comments/phpdocs should be avoided as much as possible by writing expressive code.

Don't use docblocks for methods that can be fully type hinted (unless you need a description).

Only add a description when it provides more context than the method/class signature itself. Use full sentences for descriptions, including a period at the end.

Part of this proposal was inspired in https://guidelines.spatie.be/code-style/laravel-php#docblocks


Class or interface headers:

Good:

<?php

namespace Stripe\Model\Configuration;

class Repository
{

Good:

/**
 * This class contains methods that give the information
 * about what env the application is working on.
 *
 * Usage:
 *     if ((new EnvironmentChecker())->inProduction()) {
 *         // Do your work knowing that you are in production
 *     } else {
 *         // Do your work knowing that you are in development
 *     }
 */
class EnvironmentChecker

Bad:

<?php

/*
 * This file is part of AlumnForce.
 *
 * (c) XXXXX <xxx@xx.com>
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */

namespace Stripe\Model\Configuration;

/**
 * @category  Stripe
 * @package   Stripe
 * @author    Nuno Maduro <nuno.maduro@mevia.fr>
 * @copyright 2016 Mevia
 * @license   http://alumnforce.org Private
 */
class Repository

Methods:

Good:

    /**
     * @return boolean <-- Good, if you can't use type-hints.
     */
    public function inProduction()
    {
        return APPLICATION_ENV === self::PRODUCTION;
    }

Good:

    public function inProduction(): boolean
    {
        return APPLICATION_ENV === self::PRODUCTION;
    }

Bad:

    /**
     * Checks if the application is in production. <-- The method name is already understandable.
     *
     * @return boolean
     */
    public function inProduction()
    {
        return APPLICATION_ENV === self::PRODUCTION;
    }

Bad:

    /**
     * @param $something // <-- No need for this since we have the type hint.
     *
     * @return boolean // <-- No need for this since we have the return hint.
     */
    public function inProduction(string $something): bool
    {
    }

Comments:

Good:

     // For single line comment.

    /**
     * Multiline comments
     * should use
     * this format
     */

Class attributes:

Good:


    /**
     * @var \Mevia\Tools\EnvironmentChecker // <-- If needed, fully qualified namespace always.
     */
    private $environmentChecker;

Good:


class CountryIsoTypeRepository implements CountryIsoTypeRepositoryContract
{
    private $app; // <-- No need for Phpdocs, the constructor is explicit.

    public function __construct(ApplicationContract $app)
    {
        $this->app = $app;
    }

Bad:


    /**
     * Holds an instance of the EnvironmentChecker. <-- No need for this.
     *
     * @var \Mevia\Tools\EnvironmentChecker
     */
    private $environmentChecker;

I think there is no need of going deeper on this proposal. You get the point. If something is not clear please write on the comments before voting.

mrDlef commented 6 years ago
class CountryIsoTypeRepository implements CountryIsoTypeRepositoryContract
{
    private $app; // <-- No need for Phpdocs, the constructor is explicit.

    public function __construct(ApplicationContract $app)
    {
        $this->app = $app;
    }
class CountryIsoTypeRepository implements CountryIsoTypeRepositoryContract
{
   /**
    * @var \Fully\Qualified\Namespace\ApplicationContract;
    */
    private $app;

    public function __construct(ApplicationContract $app)
    {
        $this->app = $app;
    }

I think the second one is more readable as we can have a lot of property on the class so it can be hard to know direclty what is $app;