ParentId and Parent are inconsistent in Comment class

[zmarty]

When examining a leaf (not level 1!) Comment I noticed the ParentId is the id of the parent comment id, but the Parent field is always the Post. So ParentId is the parent comment, but Parent is the parent Post?

I am using NuGet 2.0.0-CI00028

[CrustyJew]

Try again with the latest, there was some restructuring that I think may have fixed this. It may still be an issue though that needs addressed.

[zmarty]

I just checked. This is still a problem in MyGet 2.0.0-CI00047. In the image below notice that the ParentId starts which t1, which means the parent of this comment is another comment. However, the Parent field is still of type Post.

[justcool393]

So question: should we use GetParent() instead of Parent, keeping the theme of using GetX() for things that require another request?

The Link thing may also arguably something that may have to be requested as well and moved to a GetX() format.

[CrustyJew]

@justcool393 yes, I'd say anything that needs a request and is NOT included in the JSON directly should be the GetX() format, preferably with the async suffix if it is async, which it really should be.

It is also probably worth while to be more explicit with ParentComment and ParentPost instead to avoid confusion.

[chuggafan]

@CrustyJew Considering all code that initializes comments currently already requested the post and put it through to obtain the current comment, I think that using the GetPost system would be bad as we would have already made the request and can easily be throwing the few things that are required for initial post setup. A similar case can be made for making the Post having an initialized Subreddit that fills all the data in once we want to actually use the object, and having the Subreddit fill in the only basics that we need and already have (e.g. the name of the subreddit is returned by default when you get a list of comments for a post), so I think the idea of having to use the GetX syntax a bit weird since the class should already have the basic needs to be initialized, maybe having the classes internally have a bool IsInitialized that gets set once everything other than the basics gets requested to be filled in? I don't know, I'm just throwing out some ideas here

[justcool393]

@chuggafan Is a post initialized when using /api/info? It doesn't seem to return the post within it's own metadata IIRC.

[chuggafan]

It doesn't even return a json string as far as I am seeing, which is the stupid part

[justcool393]

Well, we do get the parent_id (the parent) and the link_id, so it's possible to derive the post from that, but that, in some cases, requires another request to get the submission itself.

[chuggafan]

Oh yes, definitely, but my point is that we could have a system a la:

bool Initialized { get; private set; } = false;
public Post(string PostId, IWebAgent agent... /* everything needed to initialize, but has a special parameter that indicates to not initialize the class */) 
{

}
public Post(IWebagent agent...) // current constructor
{ 
Initialized = true;
}
public Listing<Comment> Comments {
get
{
    if(!Initialized)
    {
         // Initialize the comments here
    }
}
}

Would be something I would not disagree with doing, it would save the extra request while still allowing us to keep the relevant information. I wouldn't say do it exactly as I'm saying right now, but something along the lines that I'm saying should work fine

[CrustyJew]

@zmarty can you update and check this again. Reddit added an official implementation for this if I'm remembering right and I think it should be included now in RedditSharp.

[benwurth]

I'm going to look into possibly implementing a fix myself, but I just wanted to comment that this issue is still not fixed. My specific problem is that it breaks downloading a post's entire comment tree.