Some refactor to make the code cleaner

[ecastro98]

It would also be able to be used with composer, the package manager for php.

[qaisjp]

I've deleted commit 0205974 & fixed some other bugs to make the commit history cleaner. You will need to git fetch and then git reset origin/feature/refactor --hard so that you get the updated repo.

[ecastro98]

For some reason, WIP check dissapeared. Do not merge while 'WIP' is in title.

[ecastro98]

More and more changes >:D @qaisjp @Deltanic

[qaisjp]

I gave it a read, this looks great!

Whilst I cannot judge whether or not the code is idiomatic (I am not a PHP developer), I couldn't find any obvious issues 👍

My only comment would be that I am not sure if supporting the following syntax is a good idea:

$response = $mta->someResource->callableFunction($arg1, $arg2, $arg3, ...);

My concern is that, if we add more ways to interact with a server, the possibility of clashing with existing resources might prevent us from adding more methods.

What do you think? It is totally your call whether or not to keep the functionality.

[ecastro98]

We have MtaService class where it happens the real interaction with the server. The magic method (in Resource class) will execute the method call from the service so if we add more ways to interact with a server, it should happen here with no problems.

Plus, it also adds some DX.

Although, I will remove the magic getter from Mta class and keep $mta->getResource('someResource') to get a resource because it restrict us to create public properties that could be important in the future.

So the final ways to call a resource function will be:

$response = $mta->getResource('someResource')->call('callableFunction', $arg1, $arg2, $arg3, ...);
// or
$response = $mta->getResource('someResource')->callableFunction($arg1, $arg2, $arg3, ...);

[qaisjp]

Although, I will remove the magic getter from Mta class and keep $mta->getResource('someResource') to get a resource because it restrict us to create public properties that could be important in the future.

This is actually what I was referring to. I didn't even notice the $res->callableFunction(...) shorthand!

I notice Resource.php has this:

public function getName()
{
    return $this->name;
}

If there's a callable function getName, wouldn't you be required to use $resource->call('getName', ...) instead?

I agree it's nice to have the shorthand, but imo there's the same issue here. Maybe a nice way around that would be to support $res->call->callableFunction(...) or $res->rpc->callableFunction(...) instead?

[ecastro98]

Agree. I will add it tomorrow.

[qaisjp]

Oh, and one more thing, the license.

It looks like all of this is original work, so the license should be completely up to you.

I'd recommend either:

• GNU AGPL v3 (choosealicense.com), or
• MIT (choosealicense.com)

[ecastro98]

@qaisjp What about now?

[qaisjp]

Go for merge!