3

I just wrote a function and I'm not sure if it is a good idea.

The motivation is, I'd like to store some internal bookkeeping data along with my business data in one big happy javascript object. When that object is consumed, I would like to strip the internal variables. As a convention, my internal variables take the form _internalNotes whereas my business variables don't start with an underscore.

Anyway, this is the function. It works as intended, but is this a terrible pattern?

export const stripInternals = (obj) => {
  const props = {}
  Object.keys(obj).forEach(key => {
    if (key.slice(0,1) !== '_') props[key] = obj[key]
  })
  return props
}
  • I'd put all your "secret" data into a single object within shape, then just delete it. But neither way seems very secure. – user949300 Jan 28 '17 at 0:40
  • What are those secrets? Where does that code run? It isin't clear why you have to do this without more contextual information. – plalx Jan 28 '17 at 1:24
  • I don't know what "terrible" means. If you are asking if this pattern is consistent with a certain requirement, it is best to be as explicit and precise about that requirement as possible. I gather there is some vague security reason for this....? – John Wu Jan 28 '17 at 1:29
  • Sorry, that 'secret' nonsense was a distraction. It's not secret I was just trying to show that it is not part of the core object. I will update the question... – prauchfuss Jan 28 '17 at 5:02
4

First, I'll state my opinion. This is a bad idea. Many languages don't support adding/removing fields like this, and they get along just fine. There are also nasty performance consequences to doing this. Really, though, for me this is just very hacky and relies on implicit rules which, if violated (and they can easily be violated), will result in completely bizarre behavior.

So what should you do instead?

You want some "internal" meta-data around some business data. So just do that: simply "convert" the business data into an object { internals: ..., business_data: ... } (or you can have the "internal" fields directly). Your stripInternals function can then simply be replaced by (obj) => obj.business_data. This will be much more explicit and simple and will likely perform better. If you handle all the wrapping/unwrapping yourself, this will never cause surprises, i.e. this will work correctly no matter what the "business data" is.

  • I think this is right. The fact that this can't be achieved in most languages is I think what bothered me about it. But javascript makes a feature of using variable names in ways that impact execution, e.g., the new es6 object syntax const color = 'green'; const obj = {color}; . It can certainly be abused! Also your optimization point is a good one. – prauchfuss Jan 29 '17 at 21:05
1

When applied to a framework, the pattern you're using is generally called Convention over Configuration. It is commonly accepted to be a useful approach to simplifying the interaction between the framework code and the user code (although there are some dissenters).

The question you should be asking, though, is whether your task is really difficult enough that writing framework-style code like this is actually appropriate. Is there not a simpler thing you could be doing that would work instead? Why do you need this code?

  • 1
    The expression isn't just "do the simplest thing that could possiby work." It has a condition (the second line on the page, no less), "if you're not sure what to do yet." – svidgen Jan 28 '17 at 13:58
  • @svidgen - yes, but it's pretty clear that the OP isn't sure what to do, otherwise he wouldn't be asking this question... – Jules Jan 29 '17 at 18:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.