1
1
12rbah2021-08-13 10:56:19
go
12rbah, 2021-08-13 10:56:19

Is it ok to refactor if-else?

In general, I saw such a code and decided to try to refactor it. Can you rate my code?

////////начальная версия
func MultiSort(tracks []*Track, columns []string) sort.Interface {
  return customSort{
    tracks,
    func(x, y *Track) bool {
      fmt.Println(columns)
      for i := len(columns) - 1; i >= 0; i-- {<code lang="cpp">
        if columns[i] == "Title" && x.Title != y.Title {
          return x.Title < y.Title
        } else if columns[i] == "Artist" && x.Artist != y.Artist {
          return x.Artist < y.Artist
        } else if columns[i] == "Album" && x.Album != y.Album {
          return x.Album < y.Album
        } else if columns[i] == "Year" && x.Year != y.Year {
          return x.Year < y.Year
        } else if columns[i] == "Length" && x.Length != y.Length {
          return x.Length < y.Length
        }
      }
      return true
    },
  }
}

////////моя версия 
type CompRules []CompRule

type CompRule struct{
  name string
  cond bool
  result bool
}

func (d *CompRules)getRulesResult(name string)(bool,bool){
  for _, rule := range *d {
    if rule.name == name && rule.cond {
      return true, rule.result
    }
  }
  return false, false
}

func initRules(x,y *Track)*CompRules{
  return &CompRules{
    {"Title",  x.Title  != y.Title,  x.Title < y.Title},
    {"Artist", x.Artist != y.Artist, x.Artist < y.Artist},
    {"Album",  x.Album  != y.Album,  x.Album < y.Album},
    {"Year",   x.Year   != y.Year,   x.Year < y.Year},
    {"Length", x.Length != y.Length, x.Length < y.Length},
  }
}

func MultiSort(tracks []*Track, columns []string) sort.Interface {
  return customSort{
    tracks,
    func(x, y *Track) bool {
      var r = initRules(x,y)
      for i := len(columns) - 1; i >= 0; i-- {
        if ok, res := r.getRulesResult(columns[i]);ok{
          return res
        }
      }
      return true
    },
  }
}

</code>

Answer the question

In order to leave comments, you need to log in

1 answer(s)
E
Evgeny Mamonov, 2021-08-13
@12rbah

I would advise you to leave the initial version, because. if you do banchmarks, you will see that it will be faster than your implementation.
Because in the initial implementation there is no extra. function call, and yours has getRulesResult.
To call a function, you need to push the values ​​onto the stack, and then pop them out when you're done.
This will make your implementation run slower.
But this remark is true only for functions whose execution speed is critical, like sorting for example.
If it were business logic, then the priority would be on the understandability of the code in the first place, so as not to get confused later, and so that someone could then figure out the code.
But visually the first option is clearer.
I estimate simply by the time it takes to understand how the initial implementation and yours work, the initial one took less time.
When refactoring, they usually try to get either more understandable / readable / extensible code, or more productive.
But you are definitely on the right track, I like the decisions made and the approach, it's just that you have chosen a not very successful task for refactoring, the solution is initially good)
Usually, the code that needs to be refactored looks many times more incomprehensible, it can be a sheet for several monitor screens :)), and when in such a sheet there is also a huge cyclomatic complexity (the nesting of if's and for's is large) - such code is generally very difficult to understand :)
This is where refactoring comes in handy.

Didn't find what you were looking for?

Ask your question

Ask a Question

731 491 924 answers to any question