Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

id versus id_str #126

Closed
ghost opened this issue Aug 12, 2014 · 22 comments
Closed

id versus id_str #126

ghost opened this issue Aug 12, 2014 · 22 comments

Comments

@ghost
Copy link

ghost commented Aug 12, 2014

twit.post('favorites/create', { id: tweet.id_str }, function(err, data, response) {...}
works just fine.

twit.post('favorites/create', { id: tweet.id }, function(err, data, response) {...}
[message=Sorry, that page does not exist, code=34]

This error could be documented and/or casting numerical ids to string.

@helmus
Copy link

helmus commented Sep 10, 2014

I feel like the library might just as well remove the tweet.id property, it's useless in context of a javascript library. It only causes beginners to trip up over some very low level stuff that's insanely hard to debug if you don't understand what is going on.

@paulbjensen
Copy link

+1 - We've been bit by this. Annoying as the data is not guaranteed to be found.

@capelio
Copy link

capelio commented Nov 8, 2014

+1 as well, there's no upside to keeping it around

On Fri, Nov 7, 2014 at 10:10 AM, Paul Jensen notifications@github.com
wrote:

+1 - We've been bit by this. Annoying as the data is not guaranteed to be
found.


Reply to this email directly or view it on GitHub
#126 (comment).

C. Corey Capel
Capelio

corey@capelio.com
303.704.8187

@polarathene
Copy link

I think casting to string for this case is very unreliable, besides id_str from JSON already provides that. Agree that the property is not too useful, to avoid confusion of missing property brief documentation explaining/linking to why id field is omitted and pointing to js libs that can handle int64 manipulation if needed.

@dsalcedo
Copy link

close this.
Answer:
twit.post('favorites/create', { id: tweet.id_str }, function(err, data, response) {...}

@aymericbeaumet
Copy link

@helmus Though I agree tweet.id might be troublesome, I think it would be better to explain why it is actually better to use tweet.id_str (instead of just hiding the problem).

@helmus
Copy link

helmus commented Dec 31, 2014

I agree that an explanation is probably a better idea. Here is an idea to make the explanation more explicit:
Create a new property tweet.id_num that exposes the id as a number.
Throw with an explanation referring to tweet.id_num and tweet.id_str when a users accesses tweet.id ( trough a getter ).

This will automatically educate novices and prevent them from running into issues and it still allows access to the numeric variant in case JavaScript ever get's 64bit support.

@aymericbeaumet
Copy link

When I rely on a library to communicate with an API, I expect the results I get to be pristine. The fact that tweet.id must not be used (as JS doesn't support number > 53 bits) should obviously be documented, however IMHO the JSON sent by the Twitter API has to be left as is.

@helmus
Copy link

helmus commented Jan 2, 2015

Accessing tweet.id indicates a programming error, so an exception is in place. That's just my opinion, maybe it's a matter of taste :)

@aymericbeaumet
Copy link

Well, a one could do as you say and protect with exceptions.

Or look at the potential solutions :)

@helmus
Copy link

helmus commented Jan 2, 2015

Are you suggesting that twit should include a bigint library to allow tweet id arithmetic's by default ? Or how exactly do you see a bigint lib solving the "don't use tweet.id" problem ?

@aymericbeaumet
Copy link

Well I do not know if this is a doable solution, neither if the need actually exists. I'm just pointing the fact that JS libraries exist to handle such cases.

But after all, tweet.id_str exists and a one would easily be able to build a BigInt from this so... Let's just keep it simple. Maybe your solution is the more appropriate (to throw an exception).

Have you benchmarked the cost of defining a custom getter for so many objects?

@helmus
Copy link

helmus commented Jan 2, 2015

Have you benchmarked the cost of defining a custom getter for so many objects?

No I haven't, that might actually be a valid reason to not do it. Perhaps it is just better to document it properly.

@aymericbeaumet
Copy link

I put up some tests, and IMHO the performance cost is not worth it. Let's update the documentation 👍

@aymericbeaumet
Copy link

Let me know what you guys think: https://github.com/ttezel/twit/wiki/FAQ

/@helmus /@ttezel

@helmus
Copy link

helmus commented Jan 3, 2015

  1. props for the test, I agree that it's best to just leave the twitter response untouched
  2. don't link to w3schools
  3. If people really want to learn more about IEEE 754 in JS, maybe link in this video: https://www.youtube.com/watch?v=MqHDDtVYJRI
    it does an excellent job at explaining it with great visualizations ( available here: http://bartaz.github.io/ieee754-visualization/ )
  4. Since there is only 1 faq item, maybe this can just go in the readme with a more condensed explanation ?

@aymericbeaumet
Copy link

Those two links of yours are great, feel free to update the FAQ :)

This could totally fit in the readme by now, but I thought we could add more items to this FAQ in the future?

@deepakmani
Copy link

Hi there,

I'm using tweet.id_str property after doing:

client.get('search/tweets', {q: query, count: 100, since_id: since_id}, function(err, data, response) {
                var tweets      = data.statuses;
        var num_tweets  = tweets.length;

        for (var i = 0; i < num_tweets ; i++)
        {
            var user_name           = tweets[i].user.name;
            var text                = tweets[i].text;
            var user_screen_name    = tweets[i].user.screen_name; // twitter handle
            var profile_image_url   = tweets[i].user.profile_image_url; 
            var status_id           = Number(tweets[i].id_str);
               }
});

I'm now getting the wrong status Id, I think it was working a few weeks ago. Both tweet.id and tweet.id_str are the same if i output it to console.
All the other properties of the tweet are matching. Is anyone else seeing this?

Thanks,
Depeak

@kai-koch
Copy link

@deepakmani

// [...]
var status_id  = Number(tweets[i].id_str);
// [...]

You are casting a String of an 64-Bit Integer to a Floating-Point Number that can only hold a 53-Bit.
You there by contradict the purpose of id_str in the first place. That is holding a 64-int without loosing precision.
Use:

// [...]
var status_id  = tweets[i].id_str;
// [...]

Do not use Number. If you use tweet.id for something other than being an unique identifier, you need to use a Library that handles Big Numbers.

Every number in Javascript is a Double-Prescision Floating-Point number, this is not accurate for large numbers by default. Why that is read:
http://en.wikipedia.org/wiki/Floating_point#IEEE_754:_floating_point_in_modern_computers

@deepakmani
Copy link

Thanks! I didn't realize that there was an issue when casting. It seems like there should be a warning somewhere that indicates overflow is happening.

@ttezel
Copy link
Owner

ttezel commented Jan 16, 2015

Thanks for discussing this everyone! Love the conversation and @aymericbeaumet thanks so much for putting together the FAQ. I updated the twit readme to link to the FAQ page. Closing this issue.

@ttezel ttezel closed this as completed Jan 16, 2015
@payomdousti
Copy link

just gonna put it out there that this just burned me for a month without realizing it and i have a db full of the wrong ids now...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants