Coding-standard: Class contains unused private method when I use array_map

[radekhubner]

I have a class like as:

class AwesomeClass {

    public function foo(): array
    {
        $a = array_map([$this, 'bar'], [1, 2, 3]);
    }

    private function bar(array $data): array
    {
        return $data;
    }
}

And I got error:

Class AwesomeClass contains unused private method bar().
(SlevomatCodingStandard.Classes.UnusedPrivateElements.UnusedMethod)

[kukulich]

@radekhubner I think you should rewrite your code to:

$a = array_map(function ($data) {
    return $this->bar($data);
}, [1, 2, 3]); 

The detection should work then.

[radekhubner]

Alright. But when I would like to use this method from more places, private is better. Yes, I can create a own class for my method. But I reported it only for Your info. Thanks for your response. 👍

[kukulich]

@radekhubner The method bar can still be private. However using closure is better so I will probably not support this old way of writing callbacks.

[Majkl578]

Agree with @kukulich, calling your private method from inside of a closure is a better approach. 👍

[Fuco1]

Can some of you explain why it is better to wrap it in a closure? The array syntax for callback seems pretty concise and clear.

[ondrejmirtes]

Because then the method call looks like a normal method call, so it can be found by an IDE or even with a fulltext search ->bar()... And the parameters are passed explicitly, so something like this cannot ever happen: https://stackoverflow.com/questions/262427/why-does-parseint-yield-nan-with-arraymap (similar example from JavaScript).

[patrickkusebauch]

Hello, I see what you are saying here, but I would welcome a little bit more clarification. Let me see if I can set-up the situation:

So right now I have these function dangling in my class:

    /**
     * @SuppressWarnings(PHPMD.UnusedPrivateMethod)
     * @see https://github.com/phpmd/phpmd/issues/502
     */
    private static function hasReacted(array $log): bool
    {
        return in_array($log['event'], [
            CampaignLogEvent::OPEN,
            CampaignLogEvent::CLICK,
            CampaignLogEvent::SKIPPED,
        ], true);
    }

    //phpcs:disable SlevomatCodingStandard.Classes.UnusedPrivateElements

    /**
     * @SuppressWarnings(PHPMD.UnusedPrivateMethod)
     * @see https://github.com/phpmd/phpmd/issues/502
     */
    private static function hasReactedReducer(int $reacted, array $log): int
    {
        return $reacted === 1 ? 1 : (int) self::hasReacted($log);
    }

And used on more than one place in the rest of the class like this:

        $reducer     = Closure::fromCallable([self::class, 'hasReactedReducer']);

and

            $samples = array_values($consideredCampaigns->map(static function(Collection $campaignLogs): int {
                return (int) ($campaignLogs->filter(Closure::fromCallable([self::class, 'hasReacted']))->count() > 0);
            })->toArray());

In the case of non-static methods, I could concede the point:

• Create the function in the constructor
• Assign it to a private class property
• Use this property in the rest of the class

But my functions are static. And I want them private, so nobody else can use them. And since I can not do this:

    private static $callback = function(){};

How would you write the code so that it would be compliant with this sniff and at the same time have only one instance of the callback function and keep it private just for the class usage?

I imagine it would be also beneficial to others coming to this issue not really being sure, how to solve the problem.

[ondrejmirtes]

This doesn't work? And if you remove static from the closure?

$samples = array_values($consideredCampaigns->map(static function(Collection $campaignLogs): int {
    return (int) ($campaignLogs->filter(Closure::fromCallable(static function (array $log): bool {
        return self::hasReacted($log);
    }))->count() > 0);
})->toArray());

[patrickkusebauch]

@ondrejmirtes, Of course, inlining the closure works. But as I said:

And used on more than one place in the rest of the class like this:

Since I do not want to duplicate the implementation of the closure (which is the whole point) I am looking for by this sniff acceptable solution that hopefully will not create closure on each class instantiation.

[patrickkusebauch]

The only idea I have is:

    /**
     * @var ?Closure
     */
    private static $closure = null;

    public function __construct()
    {
        self::$closure = self::$closure ?? Closure::fromCallable(static function(){/*implementation*/});
    }

But that looks much uglier on my opinion than the original implementation as a standalone private static function.

[github-actions[bot]]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.