bertrand Gugger [2005-04-23 12:15 UTC] Perhaps difficult to achieve with your organization, but basing a package on globals should be prohibited.
Here, as an example, how make a script using different providers or gateways based on dynamic choices ?
Also your oo structure is awesome and should be more documented.
I don't really get the point of independant "factories" for each command.
What is the goal here ?
Use only static methods ?
Based on globals ?
Sorry, if I missed something...
Ian Eure [2005-04-23 18:20 UTC] As with Net_SMPP_Client, GLOBALS are only used for data here, and it's to make the PDU instances themselves more lightweight.
As for the question re: different providers, that's outside the scope of this package. This package only generates and parses SMPP PDUs, and does not care where they may be destined.
I'm not sure what you mean - there aren't independent factories for each command; there's one factory method which gives you an instance of the PDU type you request. You call the factory method statically, which is how factories are typically implemented.
Perhaps you can restate your question more clearly.
bertrand Gugger [2005-04-24 12:58 UTC] I mean I don't see the interest of your static calls which will allways redo the same thing. More dangerous: the static hide the sharing of data thru globals.
Your example of client is tricky about that: direct and via object "static" calls to finally access the same global structure.
The includes chains are awfull.
I've the feeling it's done to be "OO correct" but that result is worse as a "flat" script.
I would encourage you to build a real class containing the config and abandon this false factory idea.
Sorry, if my frankness hurts... it's more that I like to use such a package !
2 more little things:
* your directory structure is not right, check CS
* don't use unpack() without giving the length of items: PHP4.2.2 is buggy in that case.
Hope that serves.
bertrand Gugger [2005-06-10 12:27 UTC] This comment is also concerning the client proposal.
Constants name should be NET_SMPP_XXX (ESME_XXX in PDU.php)
Could you present your Net_SMS implementation, as maintainer of it I did not get a copy and anyway a practical example of Net_SMPP's (and client) use would be meaningfull.
The factories should do: $obj =& new $class(...); return $obj;
instead of: return new $class(...);
Btw, I've a strange feeling about a class containing its own static factory, but you're not alone to do that ...
I tested out _unpack() on PHP4.2.2 , seems ok.
Generally, I still not understand this over complicated structure, or if it's due, I would say it is quite difficult to check :), could you produce some schema/explanations ?
How is the vendor set in Client ?
Sorry, I'm missing something certainly.
Philippe Jausions [2005-07-19 22:09 UTC] Beside the class naming issue, I think you should do some benchmarking of the $_defs array vs. a switch() implementation of it.
Usually switch it the faster then array creation and if statement. But it may not in your case or it may make things more difficult. The investigation *may* bring performance increase.
-Philippe
|
