From 8991d8e512894db2e2d7ad97529cf3a197c56a04 Mon Sep 17 00:00:00 2001
From: Richard Downer <richard.downer@cloudsoftcorp.com>
Date: Thu, 5 Jan 2012 09:51:05 +0200
Subject: [PATCH] Address review comments: use guava checkArgument(), and use
 switch instead of if for zone network type

---
 .../CloudStackComputeServiceAdapter.java      | 57 ++++++++++---------
 1 file changed, 31 insertions(+), 26 deletions(-)

diff --git a/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/strategy/CloudStackComputeServiceAdapter.java b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/strategy/CloudStackComputeServiceAdapter.java
index b7dcf41a5b..28f3b8dde5 100644
--- a/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/strategy/CloudStackComputeServiceAdapter.java
+++ b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/strategy/CloudStackComputeServiceAdapter.java
@@ -126,35 +126,40 @@ public class CloudStackComputeServiceAdapter implements
       CloudStackTemplateOptions templateOptions = template.getOptions().as(CloudStackTemplateOptions.class);
 
       DeployVirtualMachineOptions options = displayName(name).name(name);
-      if (zone.getNetworkType() == NetworkType.ADVANCED) {
-         // security groups not allowed.
-         // at least one network must be given to CloudStack,
-         // but jclouds will try to autodetect an appropriate network if none given.
-         if (templateOptions.getSecurityGroupIds().size() > 0) {
-            throw new IllegalArgumentException("security groups cannot be specified for locations (zones) that use advanced networking");
-         }
-         if (templateOptions.getNetworkIds().size() > 0) {
-            options.networkIds(templateOptions.getNetworkIds());
-         } else {
-            if (networks.size() == 0) {
-               throw new IllegalArgumentException("please setup a network for zone: " + zoneId);
+      switch(zone.getNetworkType()) {
+
+         case BASIC:
+            // both security groups and networks are optional, and CloudStack will
+            // use the zone/user's default network/security group if none given
+            if (templateOptions.getSecurityGroupIds().size() > 0) {
+               options.securityGroupIds(templateOptions.getSecurityGroupIds());
             }
-            Network defaultNetworkInZone = Iterables.getFirst(filter(networks.values(), and(defaultNetworkInZone(zoneId), supportsStaticNAT())), null);
-            if(defaultNetworkInZone == null) {
-               throw new IllegalArgumentException("please choose a specific network in zone " + zoneId + ": " + networks);
+            if (templateOptions.getNetworkIds().size() > 0) {
+               options.networkIds(templateOptions.getNetworkIds());
+            }
+            break;
+
+         case ADVANCED:
+            // security groups not allowed.
+            // at least one network must be given to CloudStack,
+            // but jclouds will try to autodetect an appropriate network if none given.
+            checkArgument(templateOptions.getSecurityGroupIds().isEmpty(), "security groups cannot be specified for locations (zones) that use advanced networking");
+            if (templateOptions.getNetworkIds().size() > 0) {
+               options.networkIds(templateOptions.getNetworkIds());
             } else {
-               options.networkId(defaultNetworkInZone.getId());
+               checkArgument(!networks.isEmpty(), "please setup a network for zone: " + zoneId);
+               Network defaultNetworkInZone = Iterables.getFirst(filter(networks.values(), and(defaultNetworkInZone(zoneId), supportsStaticNAT())), null);
+               if(defaultNetworkInZone == null) {
+                  throw new IllegalArgumentException("please choose a specific network in zone " + zoneId + ": " + networks);
+               } else {
+                  options.networkId(defaultNetworkInZone.getId());
+               }
             }
-         }
-      } else if(zone.getNetworkType() == NetworkType.BASIC) {
-         // both security groups and networks are optional, and CloudStack will
-         // use the zone/user's default network/security group if none given
-         if (templateOptions.getSecurityGroupIds().size() > 0) {
-            options.securityGroupIds(templateOptions.getSecurityGroupIds());
-         }
-         if (templateOptions.getNetworkIds().size() > 0) {
-            options.networkIds(templateOptions.getNetworkIds());
-         }
+            break;
+
+         default:
+            throw new IllegalStateException("Zone networking type is unrecognized");
+
       }
 
       if (templateOptions.getIpOnDefaultNetwork() != null) {