Laravel eloquent-dokumentationen vs. phpmd: "Avoid using static access to class"

[xlsson]

Jag följer Laravels dokumentation för Eloquent ORM:et och använder statisk tillgång till klassen som representerar min "books"-tabell i databasen, och får då följande valideringsfel av phpmd: Avoid using static access to class 'App\Models\Book' in method 'getAllBooks'.

Visst gör jag rätt om jag följer Laravels dokumentation? Ska jag då dra slutsatsen att felet ska ignoreras?

Konstigt nog får jag inte samma valideringsfel för min Highscore-klass, som är uppbyggd på samma sätt, (och som ligger i samma katalog).

Här är Laravels dokumentation: https://laravel.com/docs/8.x/eloquent#inserts

Så här ser min Book-klass ut:

namespace App\Models;

use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Database\Eloquent\Model;

class Book extends Model
{
    use HasFactory;

    /**
     * Indicates if the model should be timestamped.
     *
     * @var bool
     */
    public $timestamps = false;

    /**
     * Get all books from books table in database.
     *
     * @property array $books
     * @property object $bookDBObject
     * @property object $book
     * @return array $books
     */
    public function getAllBooks()
    {
        $bookDBObject = Book::all();
        $books = [];

        foreach ($bookDBObject as $book) {
            array_push($books, [
                'isbn' => $book->isbn,
                'title' => $book->title,
                'author' => $book->author,
                'image' => $book->image_url
            ]);
        }

        return $books;
    }
}

Min Highscore-klass:

namespace App\Models;

use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Database\Eloquent\Model;

class Highscore extends Model
{
    use HasFactory;

    /**
     * Indicates if the model should be timestamped.
     *
     * @var bool
     */
    public $timestamps = false;

    /**
     * Get all highscores from highscores table in database.
     *
     * @property object  $highscoresDBObject
     * @property array $highscoresArray
     * @property int $rank
     * @property array $highscore
     * @return array $highscoresArray
     */
    public function getAllHighscores()
    {

        $highscoresDBObject = Highscore::orderByDesc('score')
                                ->limit(10)
                                ->get();

        $highscoresArray = [];
        $rank = 0;

        foreach ($highscoresDBObject as $highscore) {
            $rank += 1;
            array_push($highscoresArray, [
                'rank' => $rank,
                'player' => $highscore->player,
                'score' => $highscore->score,
                'date_played' => substr($highscore->date_played, 0, 10)
            ]);
        }

        return $highscoresArray;
    }
}

[mosbth]

Det är lite udda om du får valideringsfel på ena men inte andra, det hade man gärna velat kika på och förstå om det är någon skillnad eller ej.

Laravel använder konstruktionen Class::method rätt ofta, så om phpmd klagar på de konstruktionerna i Laravel så hade jag ignorerat eller tom disablat den delen i phpmd.

I andra sammanhang hade jag försökt att jobba runt problemen med andra kodkonstruktioner, om det är möjligt.

Valideringsfelet i sin grund har att göra med att de anser man knyter koden för mycket till en specifik klass när man anropar statiska metoder på det viset.

Så här skriver phpmd i sin dokumentation:

Static access causes unexchangeable dependencies to other classes and leads to hard to test code. Avoid using static access at all costs and instead inject dependencies through the constructor. The only case when static access is acceptable is when used for factory methods.

https://phpmd.org/rules/cleancode.html

I dokumentationen kan man också se att det är möjligt att göra undantag för vissa klasser där man anser det okey att använda denna konstruktionen.

[xlsson]

Tack! Ja, Laravels upplägg verkar lite grann gå emot phpmd:s uppfattning om bra kod?

Gällande att Book genererar en varning, medan Highscore inte gör det, så har jag inte riktigt förstått varför. Här är de intressanta kodraderna, och vad som skiljer klasserna åt:

Book:

$bookDBObject = Book::all();

Highscore:

$highscoresDBObject = Highscore::orderByDesc('score')
                        ->limit(10)
                        ->get();

I Highscore använder jag alltså fler metoder i mitt statiska anrop. Jag fick faktiskt bort varningen för Book genom att skriva om anropet (se nedan), men jag förstår inte riktigt varför. Det är väl fortfarande ett statiskt anrop?

$bookDBObject = Book::orderBy('author')
                    ->get();

[mosbth]

Ja, Laravels upplägg verkar lite grann gå emot phpmd:s uppfattning om bra kod?

Njae, jag är inte riktigt säker på att man kan/bör säga så. Det går lite djupare än så. Laravel har sitt lager av "helpers" eller "syntaktiskt socker" som är tänkt att underlätta för ramverksprogrammeraren. Det behöver inte vara fel att göra så.

I sammanhanget "helpers" så kan det vara en rimlig väg. Det beror nog lite på hur hård och strikt ens syn är på objektorienterade programmerings "regler".

I sammanhanget ovan så känns det som det finns alternativa skrivsätt. Men det ser ju rätt mycket ut som "Laravels sätt".

PHPMD ger oss ett råd, normalt sett bör man inte skriva på det sättet. Men PHPMD är rådgivande och det är inte svart och vitt vad som är rätt och fel.

Det är väl fortfarande ett statiskt anrop?

Japp. det är det och båda fallen du visar är referensen till den klassen man är i så det känns som self:: är mer korrekt och inte på något sätt felaktigt utifrån en validators perspektiv.

[xlsson]

Tack! self:: löste biffen – jag kör på det skrivsättet helt enkelt.