[asterisk-dev] Change in testsuite[master]: stasis: set a channel variable on websocket disconnect error

[Ashley Sanders (Code Review)]

Ashley Sanders has posted comments on this change.

Change subject: stasis: set a channel variable on websocket disconnect error
......................................................................


Patch Set 1:

(3 comments)

https://gerrit.asterisk.org/#/c/18/1/tests/rest_api/applications/stasisstatus/ari_client.py
File tests/rest_api/applications/stasisstatus/ari_client.py:

Line 148:     def on_channelcreated(self, message):
> Ah, I missed the getattr() call in on_ws_event(). I'm going to blame gerrit
That's a fair point. I, too, was hijacked by Gerrit on another comment O.o.


https://gerrit.asterisk.org/#/c/18/1/tests/rest_api/applications/stasisstatus/observable_object.py
File tests/rest_api/applications/stasisstatus/observable_object.py:

Line 84:             del self.__registrar[event][:]
> Ah, thanks for the enlightenment!
:)


Line 143:     def __validate(self, **kwargs):
> I wasn't suggesting getting rid of the method, just rewriting its body.
On the topic of YAGNI, XP co-founder Ron Jeffries has written, "Always implement things when you actually need them, never when you just foresee that you need them."

In the case of the on_channeldialplan handler in ari_client.py, that was a clear example of YAGNI. I had no usages of the handler; it was adding bloat; it would have added strain on maintaining the system; there is no way to predict if it will ever be needed or used.

In this case, however, I am using this logic, in two places. I contend that this is not a case of YAGNI, because YAGNI, by its very definition, describes code that is added in anticipation of future requirements.

The fact that the method was written in a generic fashion does not in and of itself make it a victim of code bloat by over-generalization. Python as a language already provides the constructs, args and kwargs, as a means of injecting an arbitrary number of arguments into a method. This is handy when you need to decouple the implementation details of disparate methods that use a common method, such as a method to print a variable amount of data, or as in this case, a method that uses one set of business logic to validate a variable number of parameters.


-- 
To view, visit https://gerrit.asterisk.org/18
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0f7dadfd429bd30e9f07a531f47884d8c923fc13
Gerrit-PatchSet: 1
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Ashley Sanders <asanders at digium.com>
Gerrit-Reviewer: Ashley Sanders <asanders at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-HasComments: Yes