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")
.
- is typically defined via
-
ElementArrayFinder
is a special object that corresponds to an array of elements- is typically defined via
element.all(by.smth(""))
or$$("css selector")
- is typically defined via
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:
'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')
'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:
- It can be misleading, suggesting that one file is usable without the other, but it’s not
- The
elementArrayFinderGetMethods
and the conditions on it inis-element-finder
violates the encapsulation ofisElementArrayFinder
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.