2
\$\begingroup\$

I want to parse a string to an array of object with following those rules:

  • parse line by line
  • if line starts with "*" this is a title
  • if line starts with "\s**" this is a subdivision associated with the previous title
  • all entries are trimmed and no "*" character should stay
  • example : {title: "someval", subitems: ["somevale"]}

This what I ended with:

const str = `
* Level 1
  ** Level 1.1
  ** Level 1.2
  ** Level 1.3
* Level 2
* Level 3
* Level 4
* Level 5
* Level 6
  ** Level 6.1
  ** Level 6.2
* Level 7
`;

const parse = (str) => {
  const lines = str.split('\n')
  let obj = [];
  for (let i = 0; i < lines.length; i++) {
    if (!lines[i].includes("**")) {
      obj.push({title: lines[i].replace("* ", "")})
    }
    if (lines[i].includes("**") && typeof obj[obj.length-1] !== "undefined") {
      if (!obj[obj.length-1].subitems){
        obj[obj.length-1].subitems = []
      }
      obj[obj.length-1].subitems.push(lines[i].replace("** ", "").trim())
    }
  }
  return obj.filter(el => el.title !== "")
}
\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

Style

  • Use constants for variables that do not change.

    You have the array obj that only has items added to it. It's reference never changes and thus it should be a constant.

  • Use descriptive names. obj does not describe what the array holds, nor does it indicate that it is an array. A better name would be titles

  • If you don't need the array index use a for of loop rather than a for;; loop.

  • Always keep expressions and statements simple. Examples...

    • The clause typeof obj[obj.length-1] !== "undefined" can just be obj[obj.length-1] as undefined will evaluate to false

    • You filter out empty titles. The clause el.title !== "" can be just el.title as an empty string evaluate to false.

      Additionally: You don't need the filter you can check for empty lined as you add the item. Also this makes adding sub titles ambiguous. If you expect empty lines what happens to subtitles that follow the empty line?

  • Be consistent with your style. You use semicolons randomly. Use them everywhere if you do it just once.

Bugs?

You state that

  • if line starts with "*" this is a title

    However your code will interpret any line that does not contain "**" as a title?

  • if line starts with "\s**" this is a subdivision associated with the previous title

    However your code will parse any line that contains '"**"' without the leading "\s" and in any position.

  • all entries are trimmed and no "*" character should stay

    But you do not trim white spaces from titles.

To fix

  • Use regular expressions to locate matching lines.

    For titles use /^\*(?!\*)/

    For sub titles use /\s\*{2}/

  • Use String.trim when formatting the title.

Rewrite

The rewrite uses regExp to test lines (to match the requirements as you stated in your question). The parser is encapsulated in a function to reduce messing up the global scope. There are slight changes.

  • Empty lines are ignored
  • Non empty Sub titles are added to the previous title (if there is one)

const testMenu = `* Level 1
 ** Level 1.1
 ** Level 1.2
** This is not a sub title as there is no \\s**
 ** Level 1.3
This line is not marked as a menu line and is not added
* Level 2
*
The above line is an empty title and ignored
 ** This is added to title level 2 skipping the ignored empty title
 **
The above is an empty sub title and is ignored.
* Level 3`;

const menuParser = (() => {
    const TITLE = /^\*(?!\*)/, SUBTITLE = /\s\*{2}/;
    return (text) => {
        var title;
        const titles = [];
        for (const line of text.split("\n")) {
            const cleaned = line.replace(/\*/g, "").trim();
            if (cleaned) {
                if (TITLE.test(line)) {          
                    titles.push(title = {title: cleaned});
                } else if (SUBTITLE.test(line) && title) {
                    (title.subitems ?? (title.subitems = [])).push(cleaned);
                }
            }
        }
        return titles;
    }
})();
console.log(menuParser(testMenu));

\$\endgroup\$
1
  • \$\begingroup\$ I noticed you moved * Level to the same line as where you put the opening quote for the template string, to avoid the extra new line. One thing I like to do is to put a `` right after the opening quote, then do a new line. That will cause it to ignore the newline character. \$\endgroup\$ Mar 1, 2022 at 22:15
0
\$\begingroup\$
  • Use for-of instead of the C-style for loop you're currently using.
  • Be consistent with your programming style. If you're going to use const, use it everywhere (i.e. make obj a const declaration). If you're going to use semicolons, use them everywhere. If you're going to use double quotes, use them everywhere.
  • If you want to check if something is specifically undefined, you can just do x === undefined. The whole typeof thing is just overkill, and harkens back to when people were being a little over-protective about the fact that it used to be possible for someone to change the value of undefined to be something else. This isn't possible anymore in modern JavaScript, so you can just compare against undefined and call it a day. Though, in this particular scenario, an even better option would be to just do obj.length > 0, because what you're really trying to do is check if the array is empty or not.
  • You're currently have a pattern like this in your code: if (condition) { ... } if (!condition && anotherCondition) { ... }. This can simply be replaced with an if-else: if (condition) { ... } else if (anotherCondition) { ... }.
  • There's a newer .at() function available that lets you easily grab the last element of an array. If your target platforms support it (most up-to-date platforms do), or if you polyfill, you can go ahead and use this function.
  • Naming could be improved, especially with your array named obj. As @blindman67 suggested, something like "titles" would work better.

Here's what an initial rewrite would look like when I implement the above suggestions.

const parse = (str) => {
  const titles = [];
  for (const line of str.split("\n")) {
    if (!line.includes("**")) {
      titles.push({title: line.replace("* ", "")});
    } else if (titles.length > 0) {
      if (!titles.at(-1).subitems) {
        titles.at(-1).subitems = [];
      }
      titles.at(-1).subitems.push(line.replace("** ", "").trim());
    }
  }
  return titles.filter(el => el.title !== "");
};

Now for some more structural changes

  • Consider always including the "subitems" array in your final data structure, even when it's empty. Right now, you're simply omitting subitems when it's empty, which just makes more work for yourself and anyone who uses this data structure, because they have to always check if the array exists before they can use it.
  • As far as I can tell, the final .filter() is only there to handle blank lines. Why not just handle the blank lines in the for loop itself, by continue-ing if the line is empty?
  • Previously, you wrote logic to handle the possibility of the input string starting with a sub-topic, by making sure at least one parent topic existed in obj before going into your if branch (effectively ignoring the bad line). But, wouldn't it make more sense to just throw an error if this condition failed?
  • The algorithm itself could be a little more expressive. @blindman67's use of regular expressions would be an optimal way to implement this sort of thing, as the regular expressions will make sure you never incorrectly interpret bad input as good input. I'm going to showcase another option that isn't quite as rigid as @blindman67's solution, but should work ok if you really think this function won't run across bad input (which, judging by how you originally implemented this, this seems to be the case). I'm going to go the route of simply ignoring the padding on the left, and counting the number of * that are left-aligned, to know if we're dealing with a heading or subheading.
const parse = (str) => {
  const titles = [];
  for (const lineWithPadding of str.split("\n")) {
    const line = lineWithPadding.trim();
    if (line === "") continue;

    if (line.startsWith("* ")) {
      titles.push({title: line.slice("* ".length).trim(), subitems: []});
    } else if (line.startsWith("** ")) {
      const lastTitle = titles.at(-1)
      if (!lastTitle) {
        throw new Error('Can not start with a subtopic');
      }
      lastTitle.subitems.push(line.slice("** ".length).trim());
    } else {
      throw new Error(`Found a line that did not start with "* " or "** ": ${line}`);
    }
  }
  return titles;
};

Full example:

const str = `
* Level 1
  ** Level 1.1
  ** Level 1.2
  ** Level 1.3
* Level 2
* Level 3
* Level 4
* Level 5
* Level 6
  ** Level 6.1
  ** Level 6.2
* Level 7
`;

const parse = (str) => {
  const titles = [];
  for (const lineWithPadding of str.split("\n")) {
    const line = lineWithPadding.trim();
    if (line === "") continue;

    if (line.startsWith("* ")) {
      titles.push({title: line.slice("* ".length).trim(), subitems: []});
    } else if (line.startsWith("** ")) {
      const lastTitle = titles.at(-1)
      if (!lastTitle) {
        throw new Error('Can not start with a subtopic');
      }
      lastTitle.subitems.push(line.slice("** ".length).trim());
    } else {
      throw new Error(`Found a line that did not start with "* " or "** ": ${line}`);
    }
  }
  return titles;
};

console.log(parse(str));

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.