Seeking constructive feedback / code review on my first package

As part of my journey to deepen my understanding of Laravel, I decided to explore package development as an exercise. To start simple, I built a wrapper for the Logo.dev API, which provides an easy way to retrieve company logos by domain name, stock ticker, or search for brand data.

The package is available on GitHub: https://github.com/syntaxsapiens/logodev-laravel

I’m looking for constructive feedback and a code review—what’s working well and what could be improved. Any suggestions or insights are greatly appreciated!

Thank you in advance!

OussamaMater

1 year ago

Level 37

Congrats on your first package! I skimmed the code and I have a little feedback:

In the config file, add proper PHPDocs to describe what each parameter does. For example, “size” is null, but the default is 128? The format is jpg? Some comments could be helpful here. Check how spatie/laravel comments their config files.
In the facade, add the available methods in the PHPDoc, otherwise the IDE won’t be able to autocomplete them.
In the service provider, you are missing return types and type hints in the closures. You could also use arrow functions there.
In the main class itself, I see no point in having tons of comments. Back then, PHP didnt have proper types, so PHPDocs helped. Now, with modern PHP having types, you don’t need to have those comments. For the context, expressive names do the trick.
In the main class, you are missing proper array return types. It returns an array of what? keep in mind that a package will be used by clients, and knowing what they will be dealing with helps. Also if they are using phpstan on a high level, u might break it for them I personally would expect something like array<int, string>, as a return type.
The buildLogoUrl method could be refactored to use Laravel collections, something like maybe?

        $queryParams = collect([
            'size' => $this->imageSize,
            'format' => $this->imageFormat,
            'greyscale' => $this->imageGreyscale,
            'fallback' => $this->imageFallback,
            'token' => $this->publishableKey,
        ])
            ->merge($options)
           // no idea what the options array is so i can provide u correct type hints
            ->map(fn($value) => is_bool($value) ? var_export($value, true) : $value)
            ->filter()
            ->toArray();

        return $this->imgBaseUrl . $path . '?' . Arr::query($queryParams);

Overall, the naming of the API could be improved. Think about how its going to be used. LogoDev::logoByDomain() doesnt read great imo, its the logo word is repetitive, we already now thats what we will be getting, thats the service provide. It could be something like LogoDev::for($domain), this reads like englsh for me, logo dev for domain, u can tweak, its just on top of my head.

Keep the good work up man!

1 like