Separate validation from metadata retrieval

[muffinresearch]

Fixes #496

[kumar303]

I would be ok with this but a method that depends on another is always a sign of clunkiness to me. The question to ask is: would you ever need to get metadata without validating the manifest first? I think the answer is no. If that's true, you could change the class to this:

export default class ManifestJSONParser {

  constructor(jsonString, collector) {
    this.parsedJSON = JSON.parse(jsonString);
    this.collector = collector;
    this.isValid = this._validate();
  }
  ...
}

The only downside is it conceals the fact that some potentially costly validation is taking place inside the constructor. I think the upside of making the getMetaData() function more easy to work with is worth the trade-off though.

r+wc

[muffinresearch]

It's all swings and roundabouts!

I was working on the basis that the constructor shouldn't do too much, but I see what you're saying and I think you're right - it's probably a touch cleaner your way on balance.

[muffinresearch]

@kumar303 ok this is ready for another look.

[kumar303]

could be let manifestParser...

[muffinresearch]

I'm tending to use let when needed. Inside a function like this there's no difference between var and let.

[muffinresearch]

...and even then I forget to sometimes - old habits die hard.

[kumar303]

Whatever works. I've been trying to always use let unless var is needed. My rationale is that a code refactor might introduce a new block (a line down from your closure, for example) that could leak the variable.

[kumar303]

r+wc