Unnecessary methods in ClientInterface?

[sagikazarmark]

I am working with your package for a long time now, so I have some experience with it. I created a few clients implementing ClientInterface and I hardly ever used methods other than call and multicall. The rest of methods seems to me an implementation detail, which might be useful in the default client, but makes implementing the interface harder. So I am suggesting a BC breaking and a non BC breaking solution to make things cleaner:

1. Remove all other methods from the interface, optionally create a child interface or an abstract class. (BC breaking)
2. Create a CallableInterface which includes these two methods, extend it in ClientInterface. (non BC breaking)

What do you think? (It also follows the single responsibility principle by splitting into a callable and a configurable interface)

[lstrojny]

@sagikazarmark very fair criticism of the interface, I wholeheartedly agree. On the options: I feel extending another interface makes most sense, I for now struggle with the name.

[sagikazarmark]

Callable and Configurable interface are probably more generic.

[sagikazarmark]

Further thinking about it: I think that the main interface should be the one containing call and multicall. So it should be ClientInterface. Of course, it is a BC breaking change.

[lstrojny]

I would do something like this:

interface CallClientInterface
{
    public function call(string $method, array $arguments): mixed;
}

interface MulticallClientInterface
{
    public function multicall(): MulticallInterface;
}

interface ClientInterface extends CallClientInterface, MulticallClientInterface
{
}

interface HandlerInterface
{
   public function onError(callable $handler): HandlerInterface;
   public function onSuccess(callable $handler): HandlerInterface;
}

interface MulticallInterface extends HandlerInterface
{
    public addCall(string $methodName, array $params = [], callable $onSuccess = null, callable $onErrors = null): MulticallInterface;
    public function execute(): array;
}

What do you think?

[sagikazarmark]

Not sure I understand the reason of having a separate handler interface. Other than that it looks good.

[lstrojny]

Yeah, good point. Maybe calling it MulticallBuilderInterface and moving all of them into the same interface is the nicer option. At the end it is a builder alike problem that we are dealing with.

[sagikazarmark]

Yeah, builder sounds good.

[lstrojny]

@sagikazarmark can you have a look?

[sagikazarmark]

Looks good.

[lstrojny]

Merged into develop