Use of Guid as a session identifer

[jgilbert2017]

I believe Guid.NewGuid() is a pretty heavy weight call.

From what I can tell, the only use of the RequestId (which is currently a Guid) is to be used as a key into a hash table.

For this purpose new object() would suffice or probably sealed class RequestId {} for type safety.

[justcoding121]

So that would mean that we will the pointer (memory address) to an object as a key. Right?

[jgilbert2017]

in a sense yes, but since this isn't c/c++ (and memory addrsses that objects occupy can change), to be precise:
hash tables rely on 2 methods declared in System.Object: .Equals() and .GetHashCode()

var obj = new object();
var hashCode = obj.GetHashCode(); // returns a hashcode based on the object reference
obj.Equals(otherObj) -> only returns true if the references are the same.

Hope that is clear!

[justcoding121]

@honfika LGTM. What do you think?

[justcoding121]

Actually using just a long/int starting from negative max should be fine too. Unless of course the long integer reaches the max limit which is very unlikely. Just to be safe we can reset to to -ive max if that does happen. :-)

We would need to use an interlocked long counter may be to prevent duplicates.

[jgilbert2017]

are you sure we even need an id?

• if you need to hash why not just hash off the WebSession object?
• alternatively, if the API user needs to associate data on a per WebSession basis, how about just adding a public object UserData { get; set; } property to the WebSession. this avoids issues with object lifetimes that the hashing approach creates

[justcoding121]

We had complaints about not having Id in past. Specifically because without an Id there is no connection between request & response event WebSession objects since they happen asynchronously. May be because of lack of examples. HashSet<WebSession> Dictionary<WebSession, CustomData> would actually do the job if user wants to keep track of it. I am okay with removing the property completely.

If we did keep the RequestId property I think int is the most simplest and yet performance wise good choice (much better than a Guid). With objects as RequestId it is little bit costlier to create an object.

Using an extra property to keep user stuff, I am not supporting that. It gets little dirty. If user wants to keep custom data with a session they can always use a Dictionary<WebSession, CustomData>.

[jgilbert2017]

I didn't realize the request and response WebSession objects were different.

The guid is created in the WebSession (HttpWebClient) ctor.

Can you please explain how the request/response is matched right now if they are different?

[justcoding121]

They are the same objects per Http request/response session. But since events happen asynchronously we cannot predict the sequence they get fired, that's what I was mentioning a few users had difficulty understanding. A sample flow is below, requests/response handlers can get called in random order as you can see.

Request A gets fired (WebSession A)
Request B gets fired (WebSession B)
Request B gets response (WebSession B)
Request A gets response (WebSession A)

So as you can see in the example in home readme we use RequestId explicitly, but like I said actually a HashSet<WebSession> Dictionary<WebSession, CustomData> would do the job.

[jgilbert2017]

So yeah... i don't see the need for an id. If you need to hash, just use the WebSession object as the key. I don't really understand the use case for a HashSet of WebSessions. But are you sure you are opposed to the UserData property? I can definitely envision a case for this (storing API user state associated with the request) and the hash table approach is going to be prone to memory leaks.

[justcoding121]

Would'nt using a Dictionary<WebSession, CustomData> satisfy the need? Add item to list once a new session arrives, remove when its over. @honfika Your thoughts would be appreciated for using an additional object property called UserData.

[jgilbert2017]

The Dictionary approach would work but if for some reason (lets say an obscure bug) you missed the callback that indicates "its over" you wind up leaking WebSession objects which I suspect are pretty big. It also saves you an extra hash lookup (which is synchronized so a potential bottleneck).

[honfika]

I think an object UserData property is a good idea. I don't mind the Id or the type of the Id.. I don't need it anyway:)

[justcoding121]

Okay. Lets remove the Id property then and add UserData object.