ManagementView API question/comment

[ozizka]

Hi,

regarding ManagementView#getMatchingDeploymentName( String regex ):

1) Shouldn't the name be ...Names?

2) What's the point of throwing an exception in case of zero names found? Why not just reply with an empty set? Now when I just want to check remotely whether or not something is deployed, I have to:

public boolean isDeployed( String name ){
try {
 Set<String> names = currentProfileView.getMatchingDeploymentName( name );
 if( matchingNames.size() > 0 )
 return true;
} catch( Exception ex ){
 if( ex.getCause() != null && ex.getCause().getCause() != null &&
 ex.getCause().getCause() instanceof NoSuchDeploymentException ){
 return false;
 } else {
 throw ex;
 }
}

instead of

return currentProfileView.getMatchingDeploymentName( name ).size() > 0;

3) The same applies to getDeployment( String name ) - why not just return null?

Perhaps there are some tech reasons to do it this ugly way, but I would like it to have the API more user-friendly.

Thanks,
Ondra

[ozizka]

According to the following legacy code which worked with AS 5 snapshots, has this behavior changed?

managedDeployment = currentProfileView.getDeployment(deployment);
return managedDeployment != null;

[ozizka]

legacy code which worked

Actually, it might have not work, because the deployable name changed too, so it started throwing the ex.

Anyway, I understand that the common case is that the developer expects the deployment to be in place rather than not, but even in that case, throwing an exception in case of no matches is easy enough to let it up to the caller, opposed to the handling of the exception. Or, a findMatchingDeploymentNames() alternative could be available.

[emuckenhuber]

1) Shouldn't the name be ...Names?

We actually use this as a regular expression and do Patter.compile(regex) - so the naming should be fine.

2) What's the point of throwing an exception in case of zero names found? Why not just reply with an empty set? Now when I just want to check remotely whether or not something is deployed, I have to:

public boolean isDeployed( String name ){
try {
 Set<String> names = currentProfileView.getMatchingDeploymentName( name );
 if( matchingNames.size() > 0 )
 return true;
} catch( Exception ex ){
 if( ex.getCause() != null && ex.getCause().getCause() != null &&
 ex.getCause().getCause() instanceof NoSuchDeploymentException ){
 return false;
 } else {
 throw ex;
 }
}

instead of

return currentProfileView.getMatchingDeploymentName( name ).size() > 0;

Yes, i agree - in this case throwing an exception does not make much sense, returning an empty list would be better to use.

3) The same applies to getDeployment( String name ) - why not just return null?

Perhaps there are some tech reasons to do it this ugly way, but I would like it to have the API more user-friendly.

Hmm, getDeployment(String name) should throw an Exception though. It's not intended to be used without previous actions like getDeploymentNames() - otherwise you'll never know which deployments are deployed. Usually when getDeployment(...) throws an except it would mean that the client and the management view are out of sync.

We definitely are going to review the interfaces to make them more user-friendly and try to include some feedback we got so far.

Thanks,
Emanuel