Re: Re: Working Version of Net_Monitor - Please Review :)
| From: | bertrand Gugger | Date: | Mon, 06 Dec 2004 09:36:51 +0000 |
| Subject: | Re: Re: Working Version of Net_Monitor - Please Review :) | ||
| References: | 1 2 3 | Groups: | php.pear.dev |
| Request: | Send a blank email to pear-dev+get-34829@lists.php.net to get a copy of this message | ||
Hi Robert Peake who wrote:
Thanks for the constructive feedback so far. I am reluctant to move this to "proposal" yet as my understanding is that I can not edit the main document after it reaches this stage. You're wrong, in "proposal" state you can still edit it, even change the package's nameif you want. Hmmm, in the mean time you are proposing... So now, the smart message is in. But so long you store it in some internal opaque format, then should the API furnish finer handling of it. The resetState() method is samewhat too violent, as it will provoke the resending of all alerts. It should be possible to reset only one service for example. BTW, it's to be hoping, nobody puts a "\t" in message, thus could the getState() method have some problem... I think this save format is quite dangerous. It's not directly concerning your package, but to get it work some user interface would be usefull to handle the services and recipients lists. so even have the user (un)subscribe from itself. One technical remark: why do you need to make copy of $this->stuff in $stuff everywhere ? Most often you can directly work on $this->stuff. The same for subarrays, why make a copy ? you may use so many indexes you like as: $secondary[$j]['service'] The stateDiff() method could follow both arrays in one loop not need to double loop and reset ! Please, understand it's no offense here. I'm certainly not coding better ! Anyway I go on in my review. Bye -- bertrand Gugger (toggg)