Javascript/Typescript anagram String search

I've created a random word generator which allows me to create random words/anagrams.

Then I've created Anagram search app to find the angarams when user start typing in inbox.

Could you review this with SOLID principles, Performance, Productions level ready scenarios in mind? Please be critical

// Generate random words to be used
class RandomWordGenerator {

  private wordList = [];

  constructor() {
    this.generate().createAnagrams();
  }
  generate() {
    for (let i = 0; i  {
      return str.substr(2, str.length / 2).split('').reverse().join('')
    })
    this.wordList = [...this.wordList, ...anagrams];

    return this;
  }

  getWordList() {
    return this.wordList;
  }

}

// Create Search index
class SearchIndex {

  private searchResults: Array;
  private indexMap = {};
  private indexMapCharArray = [];

  constructor(private wordService: RandomWordGenerator) {
    this.searchResults = this.wordService.getWordList();
    this.sanitizeList().createIndex();
    console.log(this.searchResults);
  }

  sanitizeList() {
    this.searchResults = this.searchResults.filter((str) => !!str).sort();
    this.searchResults = Array.from(new Set(this.searchResults));

    return this;
  }

  createIndex() {
    this.indexMap = {};
    this.indexMapCharArray = [];

    for (let i = 0; i document.querySelector('.search-field');

    element.addEventListener('keyup', (e) => {
      let target = e.target;
      console.log(target.value, e);
      console.log(this.resultIndex.getSearchResults(target.value))
    })
  }
}

window.onload = () => {
  var el = document.getElementById('content');
  var generator = new RandomWordGenerator();

  let searchIndex = new SearchIndex(generator);
  let app = new App(searchIndex);

  app.start();

};

This is quite a nice idea for a learning project.

I created a jsfiddle to test your code. Initially I was not able to run your program, as there were a few minor bugs, most relevant:

When you use element.addEventListener, the this context of your App instance is not captured, even though you are using an arrow function. This leads to this.searchIndex being undefined when the keyup event handler is executed. I fixed this, by introducing a local variable before the handler:

const index = this.resultIndex;
element.addEventListener('keyup', (e) => {
  let target = e.target;
  console.log(target.value, e);
  console.log(index.getSearchResults(target.value))
})

You can also bind the this context to your handler explicitly, if you prefer.

element.addEventListener('keyup', ((e) => {
  let target = e.target;
  console.log(target.value, e);
  console.log(this.resultIndex.getSearchResults(target.value))
}).bind(this))

I wasn't able to transpile the arrow function with direct return of the double-negated str and changed it to

(str) => { return !!str; }

even though

str => str

does the same (with respect to the filter function.)

Coding Style

I have some remarks regarding your coding style.

• You use semicolons inconsistently. I would prefer to always use semicolons at the end of a statement.

• const vs. let: You should use const, whenever you don't intend to reassign the given variable, which is almost always the case - except string concatenations and declaration before assignment.

• implicit return types (e.g. in RandomWordGenerator::getWordList()) are not very helpful. If you make the return type explicit, i.e. getWordList() : string[], the compiler will enforce all code paths to return a value, which helps to reduce programming errors.

• implicit parameter types: the SearchIndex::getSearchResults(str) method does not specify the parameter type explicitly, but rather implicitly by naming the parameter str. Use getSearchResults(searchTerm : string) instead and two things are resolved at the same time: First your parameter type is specified, and second, your parameter name is way more relevant in the context of the usage of this method. This is a problem that occurs more often in your code, also with variables. sortedStrArr could be defined as sortedCharacters : string[] and sortedStr could be sorted : string. You could also make the member variable types explicit. indexMap should be index : {[key : string] : string[]}

• using private methods for methods that don't need to be accessible from the outside. In your code all of SearchIndex's methods, except getSearchResults could be private.

• unused code: You call document.getElementById('content'); without ever using the returned element. The member variable indexMapCharArray is reset to an empty array, but never used.

I am not a big fan of the method chaining in RandomWordGenerator either. The chaining as you implement it does not prevent the user of your code to call createAnagrams before calling createWords.

SOLID Principles

A scenario for extending your application could be to replace the random word generation with e.g. an English dictionary. I assume that in the real world use case one would also only want to find the anagrams already present in the dictionary.

Currently this is not possible without touching the SearchIndex class, which is unfavorable coupling. Also I'm not very satisfied with the fact, that SearchIndex does need to call the getWordList() method of RandomWordGenerator.

I would recommend the following signature of SearchIndex's constructor:

constructor(private wordList : string[])

This way you don't depend on the way the word list is generated to provide it to the SearchIndex anymore.

If you want to allow the user to provide multiple sources of word lists, you could have an interface for this:

interface IWordListSource {
    getWordList() : string[];
}

and make RandomWordGenerator, DictionaryWordSource or other implementations implement that interface. It would be okay to inject that interface into the SearchIndex, like constructor(private wordListSource : IWordListSource) instead of directly providing the word list, if you prefer.

Your App class is coupled to a specific document structure, i.e. the existence of an input element with the class search-field is required. I would recommend to inject this element into the App instance either when calling start() or when constructing the instance.

One should also think about, whether logging to the browser's console is the best way of outputting information to the user of your program.

StackExchange Code Review Q#156111, answer score: 4