Skip to content
This repository has been archived by the owner on Jan 1, 2019. It is now read-only.

[Proposal] Avoid redundant Phpdocs and comments #9

Open
nunomaduro opened this issue Mar 7, 2018 · 1 comment
Open

[Proposal] Avoid redundant Phpdocs and comments #9

nunomaduro opened this issue Mar 7, 2018 · 1 comment

Comments

@nunomaduro
Copy link
Contributor

nunomaduro commented Mar 7, 2018

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.

@nunomaduro nunomaduro changed the title [WIP] Avoid redundant Phpdocs and comments [Proposal] Avoid redundant Phpdocs and comments Mar 27, 2018
@mrDlef
Copy link
Member

mrDlef commented Mar 28, 2018

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;

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

No branches or pull requests

2 participants