From 376017de467c478f109afa8a90d061f892465b5e Mon Sep 17 00:00:00 2001
From: mackenziestarr <mackestarr@gmail.com>
Date: Wed, 7 Aug 2019 17:30:44 -0400
Subject: [PATCH] refactor and address code review comments

---
 google/resource_bigtable_instance.go | 78 ++++++++--------------------
 1 file changed, 22 insertions(+), 56 deletions(-)

diff --git a/google/resource_bigtable_instance.go b/google/resource_bigtable_instance.go
index 4c118bb4a09..7f2ec5a6820 100644
--- a/google/resource_bigtable_instance.go
+++ b/google/resource_bigtable_instance.go
@@ -314,75 +314,41 @@ func expandBigtableClusters(clusters []interface{}, instanceID string) []bigtabl
 }
 
 func resourceBigtableInstanceClusterReorderTypeList(diff *schema.ResourceDiff, meta interface{}) error {
-	keys := diff.GetChangedKeysPrefix("cluster")
-	if len(keys) == 0 {
-		return nil
-	}
+	old_count, new_count := diff.GetChange("cluster.#")
 
-	oldCount, newCount := diff.GetChange("cluster.#")
-	var count int
-	if oldCount.(int) < newCount.(int) {
-		count = newCount.(int)
-	} else {
-		count = oldCount.(int)
+	// simulate Required:true and MinItems:1 for "cluster"
+	if new_count.(int) < 1 {
+		return fmt.Errorf("Error applying diff. Resource definition should contain one or more cluster blocks, got %d blocks", new_count.(int))
 	}
 
-	// simulate Required:true and MinItems:1
-	if count < 1 {
+	if old_count.(int) != new_count.(int) {
 		return nil
 	}
 
 	var old_ids []string
-	var new_ids []string
-	for i := 0; i < count; i++ {
-		old, new := diff.GetChange(fmt.Sprintf("cluster.%d.cluster_id", i))
-		if old != nil {
-			old_ids = append(old_ids, old.(string))
-		}
-		if new != nil {
-			new_ids = append(new_ids, new.(string))
-		}
-	}
+	clusters := make(map[string]interface{}, new_count.(int))
 
-	d := difference(old_ids, new_ids)
-
-	// clusters have been reordered
-	if len(new_ids) == len(old_ids) && len(d) == 0 {
-
-		clusterMap := make(map[string]interface{}, len(new_ids))
-		for i := 0; i < count; i++ {
-			_, id := diff.GetChange(fmt.Sprintf("cluster.%d.cluster_id", i))
-			_, c := diff.GetChange(fmt.Sprintf("cluster.%d", i))
-			clusterMap[id.(string)] = c
+	for i := 0; i < new_count.(int); i++ {
+		old_id, new_id := diff.GetChange(fmt.Sprintf("cluster.%d.cluster_id", i))
+		if old_id != nil && old_id != "" {
+			old_ids = append(old_ids, old_id.(string))
 		}
+		_, c := diff.GetChange(fmt.Sprintf("cluster.%d", i))
+		clusters[new_id.(string)] = c
+	}
 
-		// build a slice of the new clusters ordered by the old cluster order
-		var old_cluster_order []interface{}
-		for _, id := range old_ids {
-			if c, ok := clusterMap[id]; ok {
-				old_cluster_order = append(old_cluster_order, c)
-			}
+	// reorder clusters according to the old cluster order
+	var old_cluster_order []interface{}
+	for _, id := range old_ids {
+		if c, ok := clusters[id]; ok {
+			old_cluster_order = append(old_cluster_order, c)
 		}
+	}
 
-		err := diff.SetNew("cluster", old_cluster_order)
-		if err != nil {
-			return fmt.Errorf("Error setting cluster diff: %s", err)
-		}
+	err := diff.SetNew("cluster", old_cluster_order)
+	if err != nil {
+		return fmt.Errorf("Error setting cluster diff: %s", err)
 	}
 
 	return nil
 }
-
-func difference(a, b []string) []string {
-	var c []string
-	m := make(map[string]bool)
-	for _, o := range a {
-		m[o] = true
-	}
-	for _, n := range b {
-		if _, ok := m[n]; !ok {
-			c = append(c, n)
-		}
-	}
-	return c
-}