Simplify Expression of if else in golang-CodePudding

i have logic module for head, last, page, limit and offset filter.

such as :

../example/param if no query, return all value

../example/param?limit=10 return the first 10 value

..example/param?offset=3 return value from number 4 to 23 (default limit is 20)

..example/param?limit=15&offset=2 return value 3 to 17

../example/param?page=2&limit=5 return value 6 to 10

../example/param?head=15 return first 15 value

../example/param?last=15 return last 15 value

../example/param?head=8&last=4 return error

../example/param?page=3&offset=4 return error

../example/param?page=2&limit=5&offset=6 return error

i have default parameter like a :

var defaultPageParameter = PageParameter{
Limit:  20,
Offset: 0,
Page: 1,
Head: 0,
Last: 0,
}

limit := pagePara.Limit
offset := pagePara.Offset
page := pagePara.Page
head := pagePara.Head
last := pagePara.Last

defaultPage := defaultPageParameter.Page
defaultLimit := defaultPageParameter.Limit
defaultOffset := defaultPageParameter.Offset
paginate := (page*limit)-limit

i have write some expression if else, but this code really to much, I want to simplify code.

if offset != defaultOffset && limit != defaultLimit && page != defaultPage && head != 0 && last != 0{
    return make([]bson.M, 0), errors.New("can't show all queries")
    //if there are all query
} else if offset != defaultOffset && limit != defaultLimit && page != defaultPage {
    return make([]bson.M, 0), errors.New("can't show all queries")
    //if there are all query
} else if offset != defaultOffset && limit == defaultLimit && page != defaultPage{
    return make([]bson.M, 0), errors.New("can't merge page and offset")
    //if combine page & offset
} else if head != 0 && last != 0 {
    return make([]bson.M, 0), errors.New("can't merge head and last")
    //if combine head & last
} else if limit <=0 || page <= 0{
    return make([]bson.M, 0), errors.New("BSON field value must be >= 0, actual value -20")
    //if limit & page value smaller than 0
}else if offset == defaultOffset && limit != defaultLimit && page == defaultPage {
    opt.SetLimit(limit)
    //if just limit
} else if offset != defaultOffset && limit == defaultLimit && page == defaultPage{
    opt.SetSkip(offset).SetLimit(limit)
    //if just offset
} else if offset == defaultOffset && limit == defaultLimit && page != defaultPage{
    opt.SetSkip(paginate).SetLimit(defaultLimit)
    //if just page
} else if offset == defaultOffset && limit != defaultLimit && page != defaultPage{
    opt.SetSkip(paginate).SetLimit(limit)
    //if limit & page
} else if offset != defaultOffset && limit != defaultLimit && page == defaultPage{
    opt.SetSkip(offset).SetLimit(limit)
    //if offset & limit
} else if head == 0 && last != 0{
    opt.SetLimit(last).SetSort(bson.M{"$natural": -1})
    //if just last
} else if head != 0 && last == 0{
    opt.SetSkip(0).SetLimit(head)
    //if just head
} else {
    options.Find()
    //else return default
}

this code refer to notes above, my code really to much expression, can anyone simplify?

CodePudding user response：

It's hard to simplify the whole thing and give you the finished code without knowing a bit more of the surrounding code.

But I have a couple suggestions for you which I think you can apply to make it a lot easier for you to simplify the logic yourself.

Suggestion #1

One issue I see here which I believe is making the logic more confusing is the usage of basic types (int) for the input variables (offset, limit, page, head, last).

The problem is that you are comparing input variables with default values (called "zero values" in Go) such as 0 or defaultLimit when what you are actually trying to check is whether the user provided these inputs or not.

My suggestion is to make these input variables pointers (*int) so that you can instead compare them with nil to more easily and clearly check if the user sent these inputs or not. I think this will make the logic much more readable.

For example, offset & limit you might be able to simplify to something like this:

if offset != nil {
  opt.SetSkip(*offset)
}

if limit != nil {
  opt.SetLimit(*limit)
}

Suggestion #2

I would also suggest handling errors differently.

Check error cases first. Return an error if the input is invalid.

Then handle success cases after that if there was no error. This is a pretty common way of writing code in Golang (and other languages too).

Something like this:

// in the function from your post:

// check for error cases
err := checkError(offset, limit, page, head, last)
if err != nil {
  return make([]bson.M, 0), err
}

// handle success (non error) cases
if offset == defaultOffset && limit != defaultLimit && page == defaultPage {
    opt.SetLimit(limit)
    //if just limit
} else if // ... the rest of the success cases ...


// ... elsewhere in the same file (or package) ...

func checkError(offset, limit, page, head, last int) error {
  if offset != defaultOffset && limit != defaultLimit && page != defaultPage && head != 0 && last != 0{
      return errors.New("can't show all queries")
      //if there are all query
  } else if offset != defaultOffset && limit != defaultLimit && page != defaultPage {
      return errors.New("can't show all queries")
      //if there are all query
  } else if offset != defaultOffset && limit == defaultLimit && page != defaultPage{
      return errors.New("can't merge page and offset")
      //if combine page & offset
  } else if head != 0 && last != 0 {
      return errors.New("can't merge head and last")
      //if combine head & last
  } else if limit <=0 || page <= 0{
      return errors.New("BSON field value must be >= 0, actual value -20")
      //if limit & page value smaller than 0
  }

  return nil
}