Hacker News new | ask | show | jobs
by meheleventyone 481 days ago
That code is a few lines maintaining a mapping though, changing it is not going to be hard and all the mappings are captured explicitly which makes it even more straightforward versus shaving off lines of code by making some of them implicit (e.g. by seeing a pattern in the mapping and using an if statement to capture them all). I'd really love to know what you think it should be to be more maintainable!
1 comments
npteljes 481 days ago
Something like this would pass a code review, while the original would not:

  function createObject(value, id, nominal)
      return {
          value = value,
          id = id,
          nominal = nominal
      }
  end
  
  local objects = {
      createObject(100, 1, "nominal1"),
      createObject(200, 2, "nominal2"),
      createObject(300, 3, "nominal3"),
      createObject(400, 4, "nominal4")
  }
Still far from ideal, but I'm not that familiar with LUA either. And I do acknowledge that bad code can work, and heck, even I write them sometimes. My original point is that it has downsides and risks, and so I don't consider it fine. I have dreaded to touch such code (mine and others) many times, and made mistakes because of past laziness like that, many times.
link
meheleventyone 480 days ago
This is not equivalent though so you fail my code review. :D

For starters its clear the code in the example expects self.base.value to already hold a value. So constructing an object blind isn't necessarily what's happening here, it's possible an existing object is even being decorated with extra fields. So your suggestion isn't preserving the semantics.

Secondly for the face cards there is an additional field being set which is not happening in your case.

That's what I mean when I said earlier that we can't just look at a piece of code contextless and say it's bad. Your refactor is definitely neater if we're initializing homogenous objects but that's an incorrect assumption on your part. It also comes with trade-offs for example it's not obvious anymore what any of the values relate to without going to the function constructing the object. Not the biggest thing in the world but if this is the only place in the codebase doing this then I'd argue that it probably isn't worth it.

link