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

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. I'm quite disapointed to realize that all alerts are allways sent to all users. I had imagined the user are registered pro service... So you wrote:

This is now possible using Net_Monitor::resetHostState(). This works for both a host (all host results are reset) or a host and service combination (just that host/service combination is reset).

If I right understand, it's to be called after net_monitor object creation and just before checkall(). Then I can achieve the same simply not passing anymore that host/service to checkall(). It will still be in last state array, but as no more in current state array, it will produce only alert ok if the option is set, and then will disappear.

The stateDiff() method could follow both arrays in one loop not need to double loop and reset !

By using a series of flags, the primary array now no longer needs to be reset -- data is copied directly to the return array. For the secondary array, because it is nested within the primary loop, it can actually be more efficient to act destructively on this array and then go back to change the message and code components before putting them in the return array. This is because acting destructively on the array diminishes the array size, decreasing the size of the secondary loop every time a duplicate is found.

Anyway, the total number of operations is still in order(N**2) It's only divided by 2 approximatively.

Given that a common scenario will be that a variety of services are down and stay down through the course of multiple calls to Net_Monitor, there will be a lot of duplicates and therefore removing them as soon as they are found and then going back for the ones that are unique will actually be faster than trying to copy them out to a second array.

I still think a single follow up of both arrays is much more efficient. (that would need to have them sorted host/service) 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) Then I can ensure it will be much more efficient and readable because you will use the PHP native associative array indexing to retrieve the corresponding last state. You don't need to loop anymore to find an element. 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. I understand, it's quite a lot to change, but I believe it could save you time and nerves in the future when your package is released. It's just now that the stuff is organized, once released is too late. If you prefer, or my explanations are not understable, I can send you a draft of the changed monitor.php I mean. Don't hesitate to ask me. A witch's proverb: "Doing and undoing is never nothing doing." à+ -- bertrand Gugger (toggg)