Recursive function to copy over an object with very specific constraints

I totally suck at recursion. I have a function that works but it also looks horribly wrong. I would love to 1) Know how to write it properly 2) Know about resources to read/study so that I don't get so stuck again.

The function is simple: given something like this:

queryConditions = {
    type: 'and',
    args: [

      // First of all: video must be published or belonging to logged in user
      { type: 'or', args: [
        { type: 'eq', args: [ 'published', true ] },
        { type: 'eq', args: [ 'userId', '#_loggedInUser#' ] },
      ]},

      // Second: filter by userId if passed
      { type: 'eq', args: [ 'userId', '#userId#'] },

      { type: 'or', args: [
        { type: 'contains', args: [ 'title', '#s#' ] },
        { type: 'contains', args: [ 'videosTags.tagName', '#s#' ] },
      ] }

    ]
  },

It will return a copy of it, but:

• If ifDefined is set and the equivalent attribute in conditionsHashisn't set, the entry is skipped

• If a "leaf" has, as second field, something in this #format#, then its content is exchanged with conditionsHash['format'].

• if or or and only have one argument, then they are eliminated and replaced by their argument That means that you can't have { type: 'or', args: [ { type: 'eq', args: [ 'published', true ] } ]} since there is only one argument in or,

I am especially uneasy about the fact that I check twice if the type is 'and' or 'or'. I need to fully check this because I need to add a feature to this: I need to add the ability to add an each parameter:

// Third: must satisfy _each_ condition of #s#, space-separated
  { type: 'each', 'value': 's', type: 'and', separator: ' ', args: [
    { type: 'or', args: [
      { type: 'contains', args: [ 'title', '#s#' ] },
      { type: 'contains', args: [ 'videosTags.tagName', '#s#' ] },
    ] }
  ] }

Which creates an entry for every space-separated string in conditionsHash['s']. That way, I can make composite queries if needed. At this

You pretty much NEED better variable names.

You've got a lot of comments in there, describing what everything does. But if you could somehow extract all this to function names and variable names then you wouldn't need all that much comments.

newCondition and actualCondition are good. arg0 and arg1 are okay. f, fc, o, osf, m, res... those don't explain anything. Those short variable names make your code hard to read, to the point where I don't actually want to read it.

The comments don't do your spacing any favor. Here it looks like the comment cuts up the if-chain...

if(  newCondition.args.length === 0 ){
          return ;
        // Only one condition returned: get rid of logical operator, add the straight condition
        } else if( newCondition.args.length === 1 ){

but it doesn't. The return statement means that there was no way for args of length 0 to go past that if statement anyway.

You'd be better off making it more explicit:

if( newCondition.args.length === 0 ){
          return;
        }

        // Only one condition returned: get rid of logical operator, add the straight condition
        if( newCondition.args.length === 1 ){

Now it reads like a separate condition.

// If it's a leaf
      } else {
        var newCondition = {};
        var f = visitQueryConditions( condition, newCondition );
        if( f !== false ) fc.args.push( newCondition );
      }

Wrap it in handleLeaf(condition, fc). Your main code should read pretty much as a state machine; "if the next node is of type A, pass it to the type A handler, if it is a type B, send it off to the type B handler", etc. Maybe get used to using different words to refer to a single "condition" and multiple "conditions". Personally, I used Condition and ConditionSet when I needed such a thing in my own code.

StackExchange Code Review Q#121962, answer score: 2