Re: Re: Working Version of Net_Monitor - Please Review :)

Hi Bertrand, bertrand Gugger wrote:

Hi Robert, I've been further checking the code. As I allready told, it's extremely laborious and difficult to read as you permanently never use the original variable but make a copy of it and further a copy of the copy ... If I was a voter, it would be a big condition.

Interesting. The whole purpose of $result = $this->_result is actually *greater* readability (except where it is necessary to not operate on the class array directly). The fact that you find a shorter, more mnemonic variable less readable is pretty amazing to me. More amazing that you threaten to withhold a vote based on such a minor point of style, even though you are not a Pear developer.

I'm quite disapointed to realize that all alerts are allways sent to all users. I had imagined the user are registered pro service... I still think a single follow up of both arrays is much more efficient.

The end user can control who gets alerts based on how they call Net_Monitor, i.e. based on the values of the $alerts array they use.

In fact, as you now serialize the result array, a much more efficient and simpler way to make it is to have the result as an associative array and no more flat. Something looking like $this->_result = array( 'foo.example.com'=>array('SMTP'=>(service code)

       'DNS'=>(service code)),

'bar.example.com'=>array('HTTP'=>(service code),

       'FTP'=>(service code),'DNS'=>(service code)));

(it's organized as the _services array)

You forget the message component in this proposed redesign. Following your thinking it would be: $this->_result = array( 'foo.example.com' => array('SMTP' => array('message' => 'OK', 'code' => '200'), 'DNS' => array('message' => 'Service Unavailable', code => '0')), ... ) Which is slightly less legible and slightly more efficient for searches that involve host/service. However, while this package currently favors host/service searching, in the future I could see the need to sort by error code, message, or some new component I have not yet discovered will be useful. If the arrays were restructured as you propose, it would be significantly more difficult to sort the structure you are proposing based on anything but host (first) and service (second). For this reason and since this package is in the early stages of growth, I think I will stick to the slightly more flat, slightly more readable, slightly more flexible, and slightly less specialized array structure.

BTW, how are this service codes defined ? I think it's the service's responsability. Just perhaps the OK value should be fixed to 200 as I can read in stateDiff() Is it not dangerous ? You should have an OK value per service type.

As stated in the proposal, 200 is always OK. 0 is a general problem. -1 is undefined. Everything else is service-defined (e.g. 404 is "Page Not Found" for HTTP and HTTPS). -RP