Search for longest substring in alphabetical order in a given string

Question

I would like some help with a review of a program that I wrote to identify the longest continuous sub-string that is in alphabetical order within a string. This is a JavaScript adaptation of an exercise originally written in Python. I am new to JavaScript. I would like any help identifying ways that this program could be written more efficiently, specifically if there are techniques or conventions in JavaScript that could improve this program.

One thing that was different between Python and JavaScript was that I had to split the string into an array in order to perform the comparison of characters. I saw another example where this was the technique to assess the strings in JavaScript.

Is there a better way in JavaScript to assess strings when looking for components other than creating an array?

The second part of the program that I would like help with is the way that the search executes immediately, instead of waiting for the call to the function with the button. I left the label in the code to show that the alphabetical sub-string in the place holder language is immediately identified, instead of waiting for a call to the function with the button.

What should I have done to make the function wait for a new search string?

// test string azcbobobegghakl, should yield beggh
function findSubStr() {
  document.getElementById("main").style.visibility = "visible";
  var string = document.getElementById("enterString").value;
  var lastSub = [],
    longestSub = [];
  var Arr = string.split('');
  for (var i = 0; i < Arr.length; i++) {
    if (Arr[i] >= lastSub.slice(-1)) {
      lastSub = lastSub + Arr[i];
      if (lastSub.length > longestSub.length) {
        longestSub = lastSub;
      }
    } else {
      lastSub = Arr[i];
    }
  }
  document.getElementById("main").innerHTML = longestSub;
}

console.log(findSubStr());

#start {
  top: 25%;
  left: 40%;
  position: absolute;
}
#main {
  top: 50%;
  left: 45%;
  font-size: 1.5em;
  position: absolute;
  visibility: hidden;
}
#enterString {
  color: gray;
}

<div id="start">
  <input id="enterString" value="Please enter string">
  <button id="search" onclick="findSubStr()">Sub-String Search</button>
</div>
<div id="main"></div>

A pen on CodePen.

Answer 1

You don't have to split a string into an Array, you can work on a String directly.

Note that I have removed you inline onclick handler. https://en.wikipedia.org/wiki/Unobtrusive_JavaScript

How your ES3 code probably should have looked

var main = document.getElementById('main');
main.style.visibility = 'visible';
var enterString = document.getElementById('enterString');
// test string azcbobobegghakl, should yield beggh
function findSubStr() {
  var string = enterString.value;
  var lastSub = '';
  var longestSub = '';
  for (var index = 0; index < string.length; index += 1) {
    var character = string.charAt(index);
    if (character >= lastSub.slice(-1)) {
      lastSub += character;
      if (lastSub.length > longestSub.length) {
        longestSub = lastSub;
      }
    } else {
      lastSub = character;
    }
  }
  if (main.firstChild) {
    main.firstChild.nodeValue = longestSub;
  } else {
    main.appendChild(document.createTextNode(longestSub));
  }
  return false;
}

document.getElementById('search').onclick = findSubStr;

#start {
  top: 25%;
  left: 40%;
  position: absolute;
}
#main {
  top: 50%;
  left: 45%;
  font-size: 1.5em;
  position: absolute;
  visibility: hidden;
}
#enterString {
  color: gray;
}

<div id="start">
  <input id="enterString" placeholder="Please enter string">
  <button id="search">Sub-String Search</button>
</div>
<div id="main"></div>

How it could have looked in ES5

var forEach = Function.prototype.call.bind(Array.prototype.forEach);
var main = document.getElementById('main');
main.style.visibility = 'visible';
var enterString = document.getElementById('enterString');
// test string azcbobobegghakl, should yield beggh
function findSubStr() {
  var lastSub = '';
  var longestSub = '';
  forEach(enterString.value, function(character) {
    if (character >= lastSub.slice(-1)) {
      lastSub += character;
      if (lastSub.length > longestSub.length) {
        longestSub = lastSub;
      }
    } else {
      lastSub = character;
    }
  });
  main.textContent = longestSub;
}

document.getElementById('search').addEventListener('click', findSubStr, false);

#start {
  top: 25%;
  left: 40%;
  position: absolute;
}
#main {
  top: 50%;
  left: 45%;
  font-size: 1.5em;
  position: absolute;
  visibility: hidden;
}
#enterString {
  color: gray;
}

<div id="start">
  <input id="enterString" placeholder="Please enter string">
  <button id="search">Sub-String Search</button>
</div>
<div id=main "></div>

How it could have looked in ES6

const main = document.getElementById('main');
main.style.visibility = 'visible';
const enterString = document.getElementById('enterString');
// test string azcbobobegghakl, should yield beggh
function findSubStr() {
  let lastSub = '';
  let longestSub = '';
  for (let character of enterString.value) {
    if (character >= lastSub.slice(-1)) {
      lastSub += character;
      if (lastSub.length > longestSub.length) {
        longestSub = lastSub;
      }
    } else {
      lastSub = character;
    }
  }
  main.textContent = longestSub;
}

document.getElementById('search').addEventListener('click', findSubStr, false);

#start {
  top: 25%;
  left: 40%;
  position: absolute;
}
#main {
  top: 50%;
  left: 45%;
  font-size: 1.5em;
  position: absolute;
  visibility: hidden;
}
#enterString {
  color: gray;
}

<div id="start">
  <input id="enterString" placeholder="Please enter string">
  <button id="search">Sub-String Search</button>
</div>
<div id="main"></div>

There are surely other ways that you could have tackled this, but essentially there is nothing wrong with your approach.

If you have any particular questions about the changes/code posted, then just comment and I will expand in the areas that most need it.

Answer 2

The algorithm is pretty good. It is similar in spirit to the Python solution that I came up with independently.

Unfortunately, lastSub = lastSub + Arr[i]; is rather inefficient, as you end up reallocating and copying all of lastSub just to append one letter. lastSub.push(Arr[i]) would be much better.

document.getElementById("main").innerHTML = longestSub; is weird — you're setting it to an array of characters rather than a string.

Avoid mixing calculations and browser interaction in the same function. You should design the function so that it accepts a string and returns a string.

function findSubStr(string) {
    var lastSub = [], longestSub = [];
    …
    return longestSub.join('');
}

document.getElementById('search').bind('click', function() {
    document.getElementById("main").innerText = findSubStr(document.getElementById("enterString").value);
});

You should set innerText rather than innerHTML, when the string isn't actually HTML. Otherwise, you risk HTML injection.

As @Xotic750 did, but didn't explicitly mention, you should use <input placeholder=…> to set the placeholder text.