Hi Glen,

I applied the same behavior as used in the http:* commands ;)
We can improve both.

Regards
JB

On 09/23/2011 02:07 PM, Glen Mazza wrote:
Added:
karaf/trunk/management/mbeans/http/src/main/java/org/apache/karaf/management/mbeans/http/internal/HttpMBeanImpl.java

URL:
http://svn.apache.org/viewvc/karaf/trunk/management/mbeans/http/src/main/java/org/apache/karaf/management/mbeans/http/internal/HttpMBeanImpl.java?rev=1174535&view=auto

==============================================================================

---
karaf/trunk/management/mbeans/http/src/main/java/org/apache/karaf/management/mbeans/http/internal/HttpMBeanImpl.java
(added)
+++
karaf/trunk/management/mbeans/http/src/main/java/org/apache/karaf/management/mbeans/http/internal/HttpMBeanImpl.java
Fri Sep 23 06:07:32 2011
@@ -0,0 +1,101 @@
+/*
+ private String getStateString(int type) {
+ switch(type) {
+ case WebEvent.DEPLOYING:
+ return "Deploying ";
+ case WebEvent.DEPLOYED:
+ return "Deployed ";
+ case WebEvent.UNDEPLOYING:
+ return "Undeploying";
+ case WebEvent.UNDEPLOYED:
+ return "Undeployed ";
+ case WebEvent.FAILED:
+ return "Failed ";
+ case WebEvent.WAITING:
+ return "Waiting ";
+ default:
+ return "Failed ";
+ }

JB, are you sure you want to have the same "default" value as you have
for another case (WebEvent.FAILED)? Might you want to make it a
different value ("Unknown", for example, or "Invalid" or "Error" if the
"default" case should never be occurring?)

Reason: if someone on the mailing list complains that the state is
"Failed" and they want to know why, it's harder to trace the code
because you don't know if the case is WebEvent.FAILED or default
(something else), because they both give the same text string. Whereas
if the values are different you'll know if the WebEvent actually did
fail, or somewhere else in the code, the type value wasn't being
properly set and hence the code was improperly falling into the
"default" category.

Glen


--
Jean-Baptiste Onofré
[email protected]
http://blog.nanthrax.net
Talend - http://www.talend.com

Reply via email to