Либо удалите этот бесполезный экземпляр объекта класса «EligibilityImport», либо используйте его

#laravel #sonarqube #sonarscanner

Вопрос:

Я использую SonarQube для сканирования своего приложения Laravel, и ему не нравится следующий код:

 class EligibilityImportJob implements ShouldQueue
{
    use Dispatchable,
        InteractsWithQueue,
        Queueable,
        SerializesModels;

    /** @var string */
    protected $file;

    /** @var int */
    protected $mode;

    public function __construct(string $file, $mode)
    {
        $this->file = $file;
        $this->mode = $mode;
    }

    public function handle(): void
    {
        $file = $this->file;
        $mode = $this->mode;

        new EligibilityImport($file, $mode); // Doesn't like this line
    }
}
 

Это дает мне следующую ошибку: Either remove this useless object instantiation of class "EligibilityImport" or use it . Как я могу это исправить? Ниже приведен EligibilityImport класс, который импортирует или удаляет данные из базы данных и поступает из файла CSV:

 final class EligibilityImport
{
    const MODE_APPEND = 1;
    const MODE_PURGE = 2;

    const MODES = [
        self::MODE_APPEND => 'append',
        self::MODE_PURGE => 'purge'
    ];

    /** @var string */
    protected $file;

    /** @var int */
    protected $mode;

    /** @var array */
    protected $cache = [];

    public function __construct($file, $mode = self::MODE_APPEND)
    {
        $this->file = $file;
        $this->mode = $mode;

        $this->process();
    }

    protected function process()
    {
        $file = $this->file;
        $mode = $this->mode;
        $path = storage_path('app/imports/' . $file);

        if (is_file($path)) {
            $csv = Reader::createFromPath($path, 'r');
            $csv->setHeaderOffset(0);

            $records = $csv->getRecords();

            foreach ($records as $record) {
                $companyName = $record['company'] ?? null;

                if ( ! empty($companyName)) {
                    $company = $this->cache['companies'][$companyName] ?? null;

                    if (empty($company)) {
                        $company = Company::where('name', $companyName)->first();

                        if ($company !== null) {
                            $this->cache['companies'][$companyName] = $company;
                        }
                    }

                    if ($company !== null) {
                        $eligibility = null;
                        $skip = false;

                        $firstName = $record['first_name'] ?? null;
                        $lastName = $record['last_name'] ?? null;
                        $email = $record['email'] ?? null;
                        $ein = $record['ein'] ?? null;

                        if ( ! empty($email)) {
                            $eligibility = $company
                                ->eligibilities()
                                ->where('email_hash', sha1($email))
                                ->first();

                            if ($eligibility !== null) {
                                $skip = true;

                                if ($mode == self::MODE_PURGE) {
                                    $eligibility->delete();
                                }
                            }
                        }

                        if ( ! empty($ein)) {
                            $eligibility = $company
                                ->eligibilities()
                                ->where('ein_hash', sha1($ein))
                                ->first();

                            if ($eligibility !== null) {
                                $skip = true;

                                if ($mode == self::MODE_PURGE) {
                                    $eligibility->delete();
                                }
                            }
                        }

                        if ($mode == self::MODE_APPEND amp;amp; ! $skip) {

                            if ( ! empty($firstName) amp;amp; ! empty($lastName) amp;amp; ( ! empty($email) || ! empty($ein))) {
                                $eligibility = new Eligibility();
                                $eligibility->fill($record);

                                $company->eligibilities()->save($eligibility);
                            }
                        }
                    }
                }
            }

            @unlink($path);
        }
    }
}
 

Комментарии:

1. Вы правы. Сначала я ввел код из-за другой ошибки. Я обновил исходный вопрос и предоставил вам больше кода, чтобы вы могли его увидеть.

2. Спасибо. На самом деле сейчас вы опубликовали правильный код. Смотрите мой ответ ниже. 🙂 Видите, почему публикация правильного кода в форме этого MRE имеет значение?

Ответ №1:

Я вообще не использую Laravel или Сонар, но я вижу, что компилятор правильно жалуется на указанную вами строку.

 public function handle(): void
{
    $file = $this->file;
    $mode = $this->mode;

    new EligibilityImport($file, $mode); // Doesn't like this line
}
 

Он жалуется, потому что вы создаете новый экземпляр EligibilityImport , а затем сразу же отбрасываете его, что означает, что вообще нет причин создавать его в первую очередь (как говорится в сообщении компилятора). Если вы прочтете слова в

Либо удалите этот бесполезный экземпляр объекта класса «EligibilityImport», либо используйте его.

компилятор сообщает вам именно то, что я написал — либо полностью удалите эту строку кода, либо сделайте что-нибудь с экземпляром объекта, который вы создаете, например, сохраните ссылку на него.

Правильный способ создания экземпляра объекта — это форма

 someVar = new SombObject()
 

который создает новый экземпляр и сохраняет ссылку на него для использования в последующем коде. Если вам нужно использовать этот экземпляр, вам нужно сохранить эту ссылку, иначе вы ничего не сможете с ней сделать. Если вам не нужно его использовать, то полностью удалите строку кода, так как это бессмысленно (а в некоторых языках может быть утечка памяти).