volatiletech / sqlboiler

Generate a Go ORM tailored to your database schema.
BSD 3-Clause "New" or "Revised" License
6.73k stars 544 forks source link

Optimization suggestion #181

Closed nkev closed 7 years ago

nkev commented 7 years ago

What version of SQLBoiler are you using (sqlboiler --version)?

v2.5.0

I don't think you need to do this:

if err != nil {
  return errors.Wrap(err, "models: unable to update all for transactions")
}

return nil

Since errors.Wrap() returns nil if err is nil, you can just do this:

return errors.Wrap(err, "models: unable to update all for transactions")
aarondl commented 7 years ago

While this is possible the flow is much more clear in the existing snippet. The bottom one relies on conventions in the errors package which not everyone will know. Also I don't believe there's any actual gain here since the if err != nil check is either performed here, or inside the errors package meaning that there is truly no optimization, in fact we skip a function call in the current code which should be faster than the proposed code. For those reasons I'll close this without action. Thanks for reporting it, don't take my brevity as dismissal, we appreciate all input on the project :)

nkev commented 7 years ago

No problem. Perhaps "optimization" was the wrong word, I meant less code, but I see your point too.