mavoweb / treecle

WIP. A toolbox for hierarchical JS objects.
https://treecle.mavo.io
MIT License
6 stars 0 forks source link

what to do when parent pointers aren't set #26

Closed adamjanicki2 closed 5 months ago

adamjanicki2 commented 6 months ago

There are several functions that require parent pointers to be set to function properly. As of now, we handle cases where parent pointers are not set in an ad-hoc way, particularly, either by failing silently or by throwing an error, but we need to standardize the procedure for a parent not being set across the library.

There are a few options of how to handle cases like these:

  1. Throw an error: the benefits of throwing errors is two-fold: it let's the author know quickly that they are not properly using Treecle, and it also lowers our codebase complexity as we don't have to try and consider special cases where parent pointers are not set.
  2. Fail silently: another option is to have a function that requires parent pointers to just return and not do anything, which I see as harmful to authors since it will be harder to debug.
  3. Console warning: something we've also discussed in the past is making a custom warn function which runs console.warn in dev environments, which could warn authors that parent pointers are not set without throwing an error.
  4. Try to set parent pointers ourself: this option is interesting because on one hand, it would be ideal since the authors won't have to worry about error handling or forgetting to set parent pointers. The only issue with this is we do not possess the ability to set relevant parent pointers from a given function if we do not have the root of the tree, which means this wouldn't be a standard across the whole library.

My personal preference would be either option 1 or 3, and more specifically, building an internal function called checkParentPointers(node) which would standardize what would happen when no parent pointers are set. Any thoughts here @LeaVerou ?

LeaVerou commented 6 months ago

I think: a) If we have a root, set parent pointers (I believe I have already implemented that). That's one of the benefits of having a Treecle instance with a root! b) If we don't have a root AND we don't have parent pointers, throw.

Is there any use cases where the parent pointers are just auxiliary? Eveyrthing I can think of either doesn't need them at all, or really needs them. And if it's the latter, it seems that 2 or 3 are insufficient.

adamjanicki2 commented 6 months ago

I definitely agree with throwing in the case where there is no root/any parent pointers. I did think of a case we'll have on our hands soon enough: the generic findPath function which takes in a start node and an end node, and returns a path from start to end. Parent pointers would really help this function, because you could just walk up the tree from end, but they're not necessary. What should we do in that case? Would a warning be helpful?

LeaVerou commented 6 months ago

Good point, I'm not sure. This brings up another question: should we set parent pointers whenever we have the chance? E.g. walk() could do it, this method could do it, etc. I'm leaning towards yes, since there are no side effects from setting them.

adamjanicki2 commented 6 months ago

Good point, I'm not sure. This brings up another question: should we set parent pointers whenever we have the chance? E.g. walk() could do it, this method could do it, etc. I'm leaning towards yes, since there are no side effects from setting them.

I'm good with this. Looping back to the design, what do you think about this: We have an internal function in utils called something like assertParentPointers(node) which checks if parents exist for the node, and if they don't, it throws an error. Since parents already handles checking for root, we don't have to worry about checking that manually since it will already be checked automatically. What do you think of that?

adamjanicki2 commented 6 months ago

We can also add a second function warnParentPointers which checks for parent pointers and then uses console.warn in dev environments, and we could use this in functions that don't require parent pointers, but would help

LeaVerou commented 6 months ago

SGTM. I would use a single function, and pass in a parameter about the severity of missing parents.

LeaVerou commented 6 months ago

I think the warning can wait though.

adamjanicki2 commented 6 months ago

SGTM. I would use a single function, and pass in a parameter about the severity of missing parents.

Sounds good!