Workdiary::get() incorrectly overrides Client::get()

[Sarke]

PHP Warning with message 'Declaration of 
Upwork\API\Routers\Workdiary::get($company, $username, $date, $params = Array)
should be compatible with 
Upwork\API\Client::get($url, $params = Array)'

in vendor/upwork/php-upwork/src/Upwork/API/Routers/Workdiary.php:81

Suggest renaming the method to getByCompany or getByCompanyAndUsername.

Or alternatively, don't extend Client. The calls are being made to $this->_client and not $this, so I don't see the reason for it.

[mnovozhylov]

Hello @Sarke ,

appreciate your comment!

First of all, it's not an error but a warning that you can safely ignore. PHP adds it because of its nature and it's up to you display or hide such warnings in your application.
Second, we will take a look into this more closely and check what we can do in the future versions.

It would be also good to know what version of PHP you use, I suspect it's below 7.1 but a confirmation is needed from your side.

[Sarke]

I never said it was an error, I know it's a warning, and I know I can silence it. However, suggesting I turn off warnings is not exactly what I was after here. As the package maintainer, you can deal with it as you wish.

My PHP version is 7.0.18, but I suspect the warning will be the same for 7.1.

[mnovozhylov]

@Sarke , appreciate your help and it's clear that you would like to have a clear solution without the warning as soon as possible. At the same time, the suggestion not to extend the class will cause the fatal error like "Cannot access parent:: when current class scope has no parent".
On the other hand, changing the method name makes the library backward incompatible and requires changes on the user side.

Thus, we might implement 3d solution and it would take some time before new version appears.

Thank you for your understanding and patience!

[bodva]

hi @mnovozhylov
In PHP 7.1, it is an Error, so I can't ignore it.
In any way, it's a really bad practice to name methods in this way, if you extending the class.

[mnovozhylov]

@bodva , thank you for the feedback! Taking into account that the library was written long time ago when PHP 7 wasn't even planned, having get methods was normal practice, although, I agree that it violates modern practices. And it will be addressed accordingly in the new major version.
As far as, the library works well with 7.3 and 7.4, the warning shouldn't be a blocker as of now.

[bodva]

@mnovozhylov Do you have information about when a new version is planned?