AmitKumarDas / Decisions

Apache License 2.0
10 stars 3 forks source link

Go: Working with Lists #198

Open AmitKumarDas opened 5 years ago

AmitKumarDas commented 5 years ago

Let us try to understand below code & then try to see if we can better this code.

type Conditions []apis.CStorVolumeCondition

// AddCondition appends the new condition to existing conditions
func (c Conditions) AddCondition(cond apis.CStorVolumeCondition) []apis.CStorVolumeCondition {
    c = append(c, cond)
    return c
}

// DeleteCondition deletes the condition from conditions
func (c Conditions) DeleteCondition(cond apis.CStorVolumeCondition) []apis.CStorVolumeCondition {
    newConditions := []apis.CStorVolumeCondition{}
    for _, condObj := range c {
        if condObj.Type != cond.Type {
            newConditions = append(newConditions, condObj)
        }
    }
    return newConditions
}

// UpdateCondition updates the condition if it is present in Conditions
func (c Conditions) UpdateCondition(cond apis.CStorVolumeCondition) []apis.CStorVolumeCondition {
    for i, condObj := range c {
        if condObj.Type == cond.Type {
            c[i] = cond
        }
    }
    return c
}
// caller looks like
cv.Status.Conditions =  cstorvolume.
  Conditions(copyCV.Status.Conditions).
  UpdateCondition(condition{...})

Few Observations:

Few Questions:

AmitKumarDas commented 5 years ago

Let us try to answer these questions:

The final question that we still have is whether we can improve this code?

AmitKumarDas commented 5 years ago

In this section, let us try to understand the aspects that can improve this code further. In my opinion, we as programmers should think from the perspective of decoupling iterations from conditions. In other words, if we happen to see a code that has conditional logic like if else within some loop constructs like for loop, there is a chance to improve this logic. Let me rewrite the above code to prove this point.

type Conditions []apis.CStorVolumeCondition

type CondPredicate func(apis.CStorVolumeCondition) bool

func TypeDoesNotMatch(type string) CondPredicate {
  return func(c apis.CStorVolumeCondition) bool {
    return type != c.Type
  }
}

func TypeMatch(type string) CondPredicate {
  return func(c apis.CStorVolumeCondition) bool {
    return type == c.Type
  }
}

func (c Conditions) Filter(p CondPredicate) []apis.CStorVolumeCondition {
  filtered := []apis.CStorVolumeCondition{}
  for _, cond := range c {
    if p(cond) {
      filtered = append(filtered, cond)
    }
  }
  return filtered
}

func (c Conditions) Find(p CondPredicate) (apis.CStorVolumeCondition, bool) {
  for _, cond := range c {
    if p(cond) {
      return cond, true
    }
  }
  return apis.CStorVolumeCondition{}, false
}

func (c Conditions) IsTypeAvailable(type string) bool {
  _, found := c.Find(TypeMatch(type))
 return found
}

// Add appends the given condition to this list
func (c Conditions) Add(cond apis.CStorVolumeCondition) []apis.CStorVolumeCondition {
    if c.IsTypeAvailable(cond.Type) {
      // do not add if already present
      return c
    }
    c = append(c, cond)
    return c
}

// Delete deletes the given condition from the list
func (c Conditions) Delete(cond apis.CStorVolumeCondition) []apis.CStorVolumeCondition {
    return c.Filter(TypeDoesNotMatch(cond.Type))
}

// Update updates the given condition from this list
func (c Conditions) Update(cond apis.CStorVolumeCondition) []apis.CStorVolumeCondition {
    filtered := c.Delete(cond)
    return append(filtered, cond)
}
AmitKumarDas commented 5 years ago

Since we have gone through above iterations, let us answer our original questions:

Let us try to answer these questions: