mpay24/mpay24-php

Confusing MPay24Config.php

Closed this issue · 7 comments

Implementing this in my point of view results to some issues:

  • it includes config.php which defines the constants which are in any case just defaults and can be set as arguments to the constructor
  • from the above config.php is not needed
  • currently MPay24Config relies on arguments which are specified in surtan order, but which are not documented and it's a bit tricky without reading the constructor

we should resolve/think about this before we are going to release version 4.0.0
@netbull how would you prefer it?

As I understand it config.php should kept as a alternative for Developers how are not so familiar with OOP.
It's more a businesses decision than a developer issue.

The Order of arguments is the same as in was in the MPAY24 Class, I just added the missing ones.

Documenting? Yes of corse that must be done!

But to be honest, I would completely remove the hole constructor and just use the Method's.
I only put it in for some Backward compatibility.

In the past you need to set 10 Parameter if you want to enable enableCurlLog had:

$mpay25 = new MPAY24($merchantID, $soapPassword, $test, $debug, $proxyHost, $proxyPort, $proxyUser, $proxyPass, $verfiyPeer, $enableCurlLog);

Now you have to same options, and some additional ways to configure your system, but most important you only have to set what you need:

$config = new MPay24Config('91234', '*******');
$config->setEnableCurlLog(true);

$mpay25 = new MPAY24($config);
Is this not more user friendly?

I have to use the MPAY24 more than one time in my app and I don't want to think about how many parameters I have to set if I want to enable or disable a log file.
Most important the Log Path that was not configurable without changing the Core Code (Very bad), even if its only a constant....

If there is a update I want to update the SW without changing the Core Code all the time....

sorry just submitted a PR with proposal :/

however the IDE's will not offer anything as a hint while coding and to see what can be configured you either have to guess or look at the source of the config object.

I agree with you about passing all dummy arguments if you want to skip some.

@stefanpolzer do you need to use the MPay24 object more than once in the app with different configurations ?

Not different configurations but I have to create more than one MPAY24 Object on several locations and just want to pass only the configure Object....

Debugging only some parts of the code should be possible as well.

My intention was to have backward compatibility as well for other users.
But I have the Feeling that is not longer a request...

@stefanpolzer backward compatibility would be great. But since we changed it to namespaces, everyone would need to change a little bit (other import & use namespace)

We will change our naming of the calls soon - probably also in the next release. That means that this will be a major release and everyone needs to adapt his implementation in case they want to use the new version

@tobiaslins yes import & namespace is different.

There is maybe a possibility to create a wrapper Class that will address this issue as well,
but I'm not sure if this would go to fare.

@stefanpolzer the wrapper class should be part of the app not the SDK