Circular dependency in utility functions

Posted on

Problem

Some Background

I’ve been developing an ESLint plugin to catch Protractor/Jasmine specific code style violations.

In Protractor, there are two important notations: ElementFinder and ElementArrayFinder.

  • ElementFinder is a special object that corresponds to a single element

    • is typically defined via element(by.smth("")) or $("css selector").
  • ElementArrayFinder is a special object that corresponds to an array of elements

    • is typically defined via element.all(by.smth("")) or $$("css selector")

And, all of these notations can be chained. ElementFinder can also be retrieved from an ElementArrayFinder via get(), first() or last() methods, e.g.:

element.all(by.className("someClass")).element(by.tagName("td"));

The Task

In multiple ESLint rules I’ve been developing so far, I needed to locate the ESLint Call Expressions that are either ElementFinder or ElementArrayFinder.

Currently, I’ve come up with two utility functions that are circularly dependent on each other:

is-element-finder.js:

'use strict'

/**
 * Checks if a given node is an ElementFinder instance.
 *
 * @fileoverview Utility function to determine if a node is an ElementFinder
 * @author Alexander Afanasyev
 */

var elementArrayFinderGetMethods = [
  'get',
  'last',
  'first'
]
var elementFinderConstructors = [
  '$',
  'element'
]

module.exports = function (node) {
  var callee = node.callee
  if (!callee) {
    return false
  }

  // handling raw $ and element
  if (elementFinderConstructors.indexOf(callee.name) > -1) {
    return true
  }

  // handling $ and element chaining
  if (callee.type === 'MemberExpression' && callee.property) {
    if (elementFinderConstructors.indexOf(callee.property.name) > -1) {
      return true
    }

    // got element finder from element array finder
    var isElementArrayFinderMethod = elementArrayFinderGetMethods.indexOf(callee.property.name) > -1
    if (isElementArrayFinderMethod && callee.object && isElementArrayFinder(callee.object)) {
      return true
    }
  }

  return false
}
var isElementArrayFinder = require('./is-element-array-finder')

is-element-array-finder.js:

'use strict'

/**
 * Checks a given CallExpression node is an ElementArrayFinder instance.
 *
 * @fileoverview Utility function to determine if a node is an ElementArrayFinder
 * @author Alexander Afanasyev
 */

module.exports = function isElementArrayFinder (node) {
  var callee = node.callee

  if (callee) {
    // handling $$ shortcut
    if (callee.name === '$$') {
      return true
    }

    // handling element.all() and nested .all()
    if (callee.property && (callee.property.name === 'all' || callee.property.name === '$$')) {
      if (callee.object) {
        if (callee.object.type === 'Identifier') {
          return callee.object.name === 'element'
        }

        if (callee.object.type === 'CallExpression') {
          return isElementFinder(callee.object) || isElementArrayFinder(callee.object)
        }
      }
    }
  }

  return false
}
var isElementFinder = require('./is-element-finder')

The argument node here is a CallExpression ESLint object.

The Problem

These two functions successfully pass all of my tests and locate ElementFinder and ElementArrayFinder instances.

But, because they both can be chained, each of the functions need to call another, which leads to a circular dependency. How would you approach removing the circular dependency? I would also appreciate any comments about the code style and readability.

Note: using Standard JS style.

I understand that there is a bit of a context in order to understand the problem, but I am open to elaborate more or add any additional information.

Solution

Circular dependency

Although I find this circular dependency fishy, I don’t know how to remove it. Maybe it’s not possible.

However, as long as the circular dependency exists,
it would be better if the two functions were defined in the same file.
I don’t like having them in separate files because:

  1. It can be misleading, suggesting that one file is usable without the other, but it’s not
  2. The elementArrayFinderGetMethods and the conditions on it in is-element-finder violates the encapsulation of isElementArrayFinder

Readability

Symmetry helps readability.
In isElementFinder you use early return on callee,
but not in isElementArrayFinder.

You could further reduce the nesting by lifting a callee.object condition to one level higher:

  if (callee.object && callee.property && (callee.property.name === 'all' || callee.property.name === '$$')) {
    if (callee.object.type === 'Identifier') {
      return callee.object.name === 'element'
    }

    if (callee.object.type === 'CallExpression') {
      return isElementFinder(callee.object) || isElementArrayFinder(callee.object)
    }
  }

The callee.property && (callee.property.name === 'all' || callee.property.name === '$$') condition could be a good candidate to extract to a helper method.

Leave a Reply

Your email address will not be published. Required fields are marked *