HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

The Stack Exchange Editor Toolkit

Submitted by: @import:stackexchange-codereview··
0
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) {

Solution

Your mod collection is not declared in a DRY manner, this

//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 how Array.push works ?



-
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 for

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 );
}

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.