patternjavascriptMinor
Refactoring an Object Walker utility function in Javascript
Viewed 0 times
refactoringjavascriptfunctionutilitywalkerobject
Problem
I'm refactoring some code, essentially a dot-notation walker for objects. I've abstracted things quite a bit, I'm to the point any more and I begin to loose readability.
It all hinges on this one function objectWalker, which does the actual object traversals. I can control what happens by passing in different callbacks to handle key points in the processes.
Pretty simple, and here are some of the callbacks i can send in.
```
dotWalker.onEnd = function(context, idx, dotPathss, prevContext) {
if (idx === 0) {
return dotWalker.objectHas(context, dotPathss[idx]) ? context[dotPathss[idx]] : undefined;
} else {
return prevContext[dotPathss[idx]];
}
}
dotWalker.onEndAssigner = function(value){
return function(context, idx, dotPathss, prevContext) {
return prevContext[dotPathss[idx]] = value, prevContext[dotPathss[idx]];
}
}
dotWalker.enRouteMapper = function(context, idx, dotPathss, prevContext) {
return context[dotPathss[idx]] = {}, context[dotPathss[idx]];
}
dotWalker.enRoute = function(context, idx, dotPathss, prevContext) {
return dotWalker.objectHas(context, dotPathss[idx]) ? context[dotPathss[idx]] : undefined ;
}
dotWalker.on
It all hinges on this one function objectWalker, which does the actual object traversals. I can control what happens by passing in different callbacks to handle key points in the processes.
function objectWalker (paths, obj, onEnd, enRoute, onEach) {
idx = -1,
nextContext = prevContext = obj;
const len = paths ? paths.length : 0;
const lastStep = len ? len - 1 : 0;
while (nextContext != null && ++idx < len) {
if (onEach) nextContext = onEach(nextContext, idx, paths, prevContext);
if (idx !== lastStep) {
prevContext = nextContext;
nextContext = (!dotWalker.objectHas(nextContext, paths[idx])) ? enRoute(nextContext, idx, paths, prevContext) : nextContext[paths[idx]];
} else {
prevContext = paths.length === 1 ? nextContext : prevContext[paths[idx - 1]];
return onEnd(nextContext, idx, paths, prevContext );
}
if(!nextContext) return nextContext;
}
return nextContext;
}Pretty simple, and here are some of the callbacks i can send in.
```
dotWalker.onEnd = function(context, idx, dotPathss, prevContext) {
if (idx === 0) {
return dotWalker.objectHas(context, dotPathss[idx]) ? context[dotPathss[idx]] : undefined;
} else {
return prevContext[dotPathss[idx]];
}
}
dotWalker.onEndAssigner = function(value){
return function(context, idx, dotPathss, prevContext) {
return prevContext[dotPathss[idx]] = value, prevContext[dotPathss[idx]];
}
}
dotWalker.enRouteMapper = function(context, idx, dotPathss, prevContext) {
return context[dotPathss[idx]] = {}, context[dotPathss[idx]];
}
dotWalker.enRoute = function(context, idx, dotPathss, prevContext) {
return dotWalker.objectHas(context, dotPathss[idx]) ? context[dotPathss[idx]] : undefined ;
}
dotWalker.on
Solution
First of all you originally add the tag functional-programming to your question.
Your code looks not so functional to me, for a couple for reasons.
First of all you design an object dotWalker, that have nothing to do with functional programming. You need just functions!
I know you apply the module path, to contain your code, but instead of keeping the private stuff private you expose all.
This is another point about your design.
I know that you ask just for the pathFinder method, but I found some issue in all the code, so I just point it out.
First of all the module name. As it is not a function, you should use a name that shown this.
You did good in the comment: DotWalker with the first letter as capital.
If dotWalker is your module, you should define it as a plain object:
And then attack the functions you want to export outside the module:
You should not return the result of something else.
If you need to define an objectWalker then:
You define 2 times this function:
So the last defined wins.
About your pathFinder method:
What is wrong with proper names?
c: wolkerObject or originalObject
pd: pathDelimiter?
nd: namespaceDelimiter?
So, why don't put this constants as internals? I mean in your module closure, but outside the function, like:
If you need defaults values and then allow to change the values with configuration options:
And in your constructor or initializer function:
And then you just use pathDelimiter and namespaceDelimiter in your code.
Another point very important here is: avoid to override function parameter.
In complex application, if a function override the value of a parameter, it is often a drive to an hell situation in debugging.
Connecting this to functional programming, functions should take an input and provide some output and never change the input.
If you need for some reason, you should document on the API, or make a local copy of the value where you do your changes.
The following code appear a little confused to me.
The return of this method should be true, why you use !!?
I'll move this condition, after the check isNamespacedDotPath, as if this is fals, you don't have a chance that your last char is ':'.
And I think you should handle this in the else part too.
I think the point here is to get the namespace as it is the starting point to walk through the path.
You could write functions that parse the namespace and the paths to break up the code and make it much readable.
```
paths = paths[0] === ''
Your code looks not so functional to me, for a couple for reasons.
First of all you design an object dotWalker, that have nothing to do with functional programming. You need just functions!
I know you apply the module path, to contain your code, but instead of keeping the private stuff private you expose all.
This is another point about your design.
I know that you ask just for the pathFinder method, but I found some issue in all the code, so I just point it out.
First of all the module name. As it is not a function, you should use a name that shown this.
You did good in the comment: DotWalker with the first letter as capital.
const dotWalker = function (dotPaths, context, end, enRoute, onEach){
end = end || dotWalker.onEnd;
enRoute = enRoute || dotWalker.enRoute;
var { context, dotPaths } = dotWalker.pathFinder( dotPaths || '', context);
return objectWalker(dotPaths, context, end, enRoute, onEach);
}If dotWalker is your module, you should define it as a plain object:
const DotWalker = {};And then attack the functions you want to export outside the module:
DotWalker.init = function () {
// ... the stuff goes here
}You should not return the result of something else.
If you need to define an objectWalker then:
DotWalker.objectWalker = function (dotPaths, context, end, enRoute, onEach) {
return theObjectWalker; // return what you need
}You define 2 times this function:
dotWalker.assign = function (dotPath, value, context) {
return dotWalker(dotPath, context, dotWalker.onEndAssigner(value), dotWalker.enRouteMapper);
}So the last defined wins.
About your pathFinder method:
dotWalker.pathFinder = function(paths, c, namespace, pd, nd) {What is wrong with proper names?
c: wolkerObject or originalObject
pd: pathDelimiter?
nd: namespaceDelimiter?
pd = pd || '.';
nd = nd || ':';So, why don't put this constants as internals? I mean in your module closure, but outside the function, like:
const pathDelimiter = '.';
const namespaceDelimiter = ':';If you need defaults values and then allow to change the values with configuration options:
const defaultPathDelimiter = '.';
const defaultNamespaceDelimiter = ':';
let pathDelimiter;
let namespaceDelimiter;And in your constructor or initializer function:
pathDelimiter = opts['pathDelimiter'] || defaultPathDelimiter;
namespaceDelimiter = opts['namespaceDelimiter'] || defaultNamespaceDelimiter;And then you just use pathDelimiter and namespaceDelimiter in your code.
Another point very important here is: avoid to override function parameter.
In complex application, if a function override the value of a parameter, it is often a drive to an hell situation in debugging.
Connecting this to functional programming, functions should take an input and provide some output and never change the input.
If you need for some reason, you should document on the API, or make a local copy of the value where you do your changes.
The following code appear a little confused to me.
const lastChar = paths[paths.length - 1];
const isNamespacedDotPath = paths.indexOf(nd) > -1;
if ( !!dotWalker.objectHas(c, 'ns') ) {The return of this method should be true, why you use !!?
if ( lastChar === nd ) { // "label:" -> namespaced but no dotPaths
namespace = c[paths.slice(0, -1)], paths = undefined;
}I'll move this condition, after the check isNamespacedDotPath, as if this is fals, you don't have a chance that your last char is ':'.
And I think you should handle this in the else part too.
if ( isNamespacedDotPath ) { // "label:some.sub.path" -> namespaced but with dotPaths
paths = paths.split(nd), namespace = c.ns[paths[0]], paths = paths.slice(1)[0].split(pd);
}
if(!namespace){
paths = paths.split(pd), namespace = c.selected || c.ns.root;
}
} else { /* Attempting to use namespaceses on simple object */
if ( isNamespacedDotPath ) {
paths = paths.split(nd);
if( !c || !dotWalker.objectHas(c, paths[0])){
namespace = undefined, paths = [];
} else {
namespace = c[paths[0]], paths = paths.slice(1).join('.').split(pd);
}
}
if (typeof paths === 'string' && paths.indexOf(pd) > -1) {
paths = paths.split(pd);
}
if ( lastChar === nd){
paths = '';
}
namespace = namespace || c;
}I think the point here is to get the namespace as it is the starting point to walk through the path.
const contextArray = paths.split(ns);
if (contextArray.length !== 2 && paths.indexOf(ns) == -1) {
// no namespace provided
} else if (contextArray.length !== 2) {
// namespace only
} else { // namespace and paths
}You could write functions that parse the namespace and the paths to break up the code and make it much readable.
```
paths = paths[0] === ''
Code Snippets
const dotWalker = function (dotPaths, context, end, enRoute, onEach){
end = end || dotWalker.onEnd;
enRoute = enRoute || dotWalker.enRoute;
var { context, dotPaths } = dotWalker.pathFinder( dotPaths || '', context);
return objectWalker(dotPaths, context, end, enRoute, onEach);
}const DotWalker = {};DotWalker.init = function () {
// ... the stuff goes here
}DotWalker.objectWalker = function (dotPaths, context, end, enRoute, onEach) {
return theObjectWalker; // return what you need
}dotWalker.assign = function (dotPath, value, context) {
return dotWalker(dotPath, context, dotWalker.onEndAssigner(value), dotWalker.enRouteMapper);
}Context
StackExchange Code Review Q#160452, answer score: 2
Revisions (0)
No revisions yet.