tastejs / TasteMusic

Empowering your MDD (Music Driven Development) - Music recommendations by JavaScripters
https://github.com/tastejs/TasteMusic/issues/1
178 stars 13 forks source link

Comment Parser #12

Open Glavin001 opened 10 years ago

Glavin001 commented 10 years ago

TODO:

Glavin001 commented 10 years ago

I do not think that the comment_parser should deal with the DOM as it does currently with it's htmlList element.

vasilionjea commented 10 years ago

Bugs: https://github.com/tastejs/TasteMusic/blob/master/app/scripts/comments_parser.js#L23 https://github.com/tastejs/TasteMusic/blob/master/app/scripts/comments_parser.js#L27

It should be url.indexOf(domains) > -1 instead of url.indexOf(domains) > 1.

Glavin001 commented 10 years ago

@vasilionjea after PR #13 is merged, would you be able to assist me in breaking up the comments_parser?

I am thinking:

Player

Tracks (originally comment_parser)

What should we include in the Track object? Potentially:

{
"user": "GitHub username of the user who created the Comment.",
"url": "The URL as parsed from the original Comment.",
"dateAdded": "Date the Comment was added.",
"comment": "Should we store a reference to the original Comment?"
}

What are your thoughts?

vasilionjea commented 10 years ago

As far as removing the htmlList, yes I agree that it has nothing to do with parsing comments and should be done on the outside.

Now on CommentsParser itself, I'm not sure what else we could separate. From your comment above I think you want to rename CommentsParser to Track or I think you're saying you want CommentsParser to instantiate new Track objects once the Track constructor exists with properties like the JSON above in your comment.

Something to keep in mind in general is that not all the URLs will be turned into Tracks because most likely we won't integrate with all the APIs from all sources. The URLs that won't become Tracks should instead be provided in another tab as anchor links. Basically this was the idea behind returning a result object with hashed links: https://github.com/Glavin001/TasteMusic/blob/master/app/scripts/comments_parser.js#L13

The other key in the result hash would be for the anchor links.