patternjavascriptMinor
The Stack Exchange Editor Toolkit
Viewed 0 times
thestackexchangetoolkiteditor
Problem
I recently joined this GitHub project based on a user script that offers a powerful editing tool to Stack Exchange Editors.
My Goal:
In the past few days, I've spent many hours improving the code, but today I re-wrote the whole thing into a completely new infrastructure. Here was my objective as written in a discussion on the repo:
Right now the method in which editing is done is sort of jumping in
and out of functions. This is a note to convert the entire project
into more of a pipeline infrastructure. This is important for
exensibility; We decide to add something new (like me working on spell
checking), we just add a layer to the pipeline that grabs / modifies
the content at some point on it's way to being output, and flows it
right back into the line for further processing.
Mini Model:
Here is the working concept model I built to try it before implementing:
`//overwrite console log just to output into stack snippets (horrid practice)
console.log = function (input){
document.body.innerHTML += (input + "
");
}
//define our namespace
var App = {};
//place item data in one place
//this keeps in mind multi question inline editing support
App.items = [{
title: 'title text here',
body: 'body here',
summary: 'summary will go here'
}, {
//second edit item if multiple on page and so on
}];
//dummy plain string data to make this work in a test
var dummyData = ("I'm just a string, but I should be an object containing sets of edit items.");
//define modules in one place
App.pipeMods = {
edit: function (data) {
return (data + " Edited!");
},
omitCode: function (data) {
return (data + " Code omitted!")
},
checkSpelling: function (data) {
return (data + " Spelling Checked!")
}
}
//define order in which mods affect data
App.order = [
"omitCode",
"edit",
"checkSpelling"];
//wa-la sexy, simple, extendable code infrastructure:
App.pipe = function (data, mods, order) {
My Goal:
In the past few days, I've spent many hours improving the code, but today I re-wrote the whole thing into a completely new infrastructure. Here was my objective as written in a discussion on the repo:
Right now the method in which editing is done is sort of jumping in
and out of functions. This is a note to convert the entire project
into more of a pipeline infrastructure. This is important for
exensibility; We decide to add something new (like me working on spell
checking), we just add a layer to the pipeline that grabs / modifies
the content at some point on it's way to being output, and flows it
right back into the line for further processing.
Mini Model:
Here is the working concept model I built to try it before implementing:
`//overwrite console log just to output into stack snippets (horrid practice)
console.log = function (input){
document.body.innerHTML += (input + "
");
}
//define our namespace
var App = {};
//place item data in one place
//this keeps in mind multi question inline editing support
App.items = [{
title: 'title text here',
body: 'body here',
summary: 'summary will go here'
}, {
//second edit item if multiple on page and so on
}];
//dummy plain string data to make this work in a test
var dummyData = ("I'm just a string, but I should be an object containing sets of edit items.");
//define modules in one place
App.pipeMods = {
edit: function (data) {
return (data + " Edited!");
},
omitCode: function (data) {
return (data + " Code omitted!")
},
checkSpelling: function (data) {
return (data + " Spelling Checked!")
}
}
//define order in which mods affect data
App.order = [
"omitCode",
"edit",
"checkSpelling"];
//wa-la sexy, simple, extendable code infrastructure:
App.pipe = function (data, mods, order) {
Solution
Your mod collection is not declared in a DRY manner, this
has the function names twice. I would simply declare
I would consider defining the actual code outside of the array declaration.
Then
For the rest of the code, from a quick once over:
-
This part is very painful:
The constant repetition of an overly long
//define modules in one place
App.pipeMods = {
edit: function (data) {
return (data + " Edited!");
},
omitCode: function (data) {
return (data + " Code omitted!")
},
checkSpelling: function (data) {
return (data + " Spelling Checked!")
}
}
//define order in which mods affect data
App.order = [
"omitCode",
"edit",
"checkSpelling"];has the function names twice. I would simply declare
pipeMods as an array which has an inherent order.App.modules = [
function edit (data) {
return (data + " Edited!");
},
function omitCode (data) {
return (data + " Code omitted!")
},
function checkSpelling (data) {
return (data + " Spelling Checked!")
}
];I would consider defining the actual code outside of the array declaration.
Then
//Voila
App.pipe = function (data, modules) {
modules.forEach( function( module ){
data = module(data);
});
return data;
}For the rest of the code, from a quick once over:
App.globals.numReasons = 0;seems suspicious, you should not need it. Perhaps look up howArray.pushworks ?
-
This part is very painful:
for (var z = 0; z < App.globals.reasons.length; z++) {
//check that summary is not getting too long
if (data[0].summary.length < 200) {
//capitalize first letter
if (z === 0) {
data[0].summary += App.globals.reasons[z][0].toUpperCase() + App.globals.reasons[z].substring(1);
//post rest of reasons normally
} else {
data[0].summary += App.globals.reasons[z];
}
//if it's not the last reason
if (z !== App.globals.reasons.length - 1) {
data[0].summary += "; ";
//if at end, punctuate
} else {
data[0].summary += ".";
}
}
}The constant repetition of an overly long
App.globals.reasons and data[0].summary and a lack of ternary operator usage make this too hard to read. I would go fordata[0].summary = generateSummary( App.globals.reasons );
function generateSummary( reasons ){
var summary = reasons.join('; ');
summary = summary.charAt(0).toUpperCase() + summary.slice(1) + '.';
return summary.slice( 0, 200 );
}Code Snippets
//define modules in one place
App.pipeMods = {
edit: function (data) {
return (data + " Edited!");
},
omitCode: function (data) {
return (data + " Code omitted!")
},
checkSpelling: function (data) {
return (data + " Spelling Checked!")
}
}
//define order in which mods affect data
App.order = [
"omitCode",
"edit",
"checkSpelling"];App.modules = [
function edit (data) {
return (data + " Edited!");
},
function omitCode (data) {
return (data + " Code omitted!")
},
function checkSpelling (data) {
return (data + " Spelling Checked!")
}
];//Voila
App.pipe = function (data, modules) {
modules.forEach( function( module ){
data = module(data);
});
return data;
}for (var z = 0; z < App.globals.reasons.length; z++) {
//check that summary is not getting too long
if (data[0].summary.length < 200) {
//capitalize first letter
if (z === 0) {
data[0].summary += App.globals.reasons[z][0].toUpperCase() + App.globals.reasons[z].substring(1);
//post rest of reasons normally
} else {
data[0].summary += App.globals.reasons[z];
}
//if it's not the last reason
if (z !== App.globals.reasons.length - 1) {
data[0].summary += "; ";
//if at end, punctuate
} else {
data[0].summary += ".";
}
}
}data[0].summary = generateSummary( App.globals.reasons );
function generateSummary( reasons ){
var summary = reasons.join('; ');
summary = summary.charAt(0).toUpperCase() + summary.slice(1) + '.';
return summary.slice( 0, 200 );
}Context
StackExchange Code Review Q#68473, answer score: 4
Revisions (0)
No revisions yet.