kenany / gosugamers-match

Parse a match page from GosuGamers
MIT License
1 stars 0 forks source link

Prevents reading 'length' of undefined #8

Closed the-lay closed 9 years ago

the-lay commented 9 years ago

Otherwise the script throws error on parsing live matches. Also, it's better to set to null, instead of deleting. Not just because of performance, but it's easier to have constant properties and have values or null.

kenany commented 9 years ago

Thanks for the patch, but I can't seem to reproduce this issue. I just tried parsing a live CS:GO game, a live Dota2 game, and a live LoL game and an error is never thrown. What game were you seeing this issue for?

constant properties

Yeah, this sounds better actually. I just wanted the smallest possible Object to be returned but if it's more convenient we can set to null instead.

the-lay commented 9 years ago

I'll recheck when I get home from office, but I'm pretty sure I had this issue yesterday. I'll try to provide testcase this evening. I'll also fix the test (so TravisCI doesn't bark)

In an ideal world, there should be some kind of configuration for this. Maybe an object that specifies what script should try to parse. For example, if there was rounds: true in configuration, script should try to parse rounds and return rounds property either as null or as an array, but if there was rounds: false then don't include rounds property at all.

the-lay commented 9 years ago

Sorry, I've mistaken, not on live matches, but on upcoming matches, the ones that were not played yet. Example test case: http://www.gosugamers.net/counterstrike/tournaments/5502-starladder-starseries-season-12/1441-group-stage-i/5505-group-c/matches/63859-dat-team-vs-titan-cs throws:

    if (!ret.rounds.length) {
                   ^
TypeError: Cannot read property 'length' of undefined

on Windows 8.1, with v0.10.34 Node.JS.

kenany commented 9 years ago

upcoming matches

Makes sense considering I never parse those :smile:

I've merged these changes as https://github.com/KenanY/gosugamers-match/compare/cdcd4d5d363c8273ce44ed47be0cef4c373d6210...a45764a3702aa946e9d366615c8d7ef14a71bd39. I took the liberty of splitting each individual change (the fix, setting rounds to null, and the team name update) to its own commit.

kenany commented 9 years ago

Also thanks for the patches :) I've added you as a collaborator so we can review and land each other's PRs from now on should any more be needed.

the-lay commented 9 years ago

Thanks for the add! Hopefully I will add something useful :)

kenany commented 9 years ago

Released 2.0.2 :)