HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Stack Exchange websockets wrapper

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
exchangewebsocketswrapperstack

Problem

I've been writing a small library that allows for easy querying of the Stack Exchange websockets.

I'm going to add an enum to replace the manual SiteId at every RequestParameter so you can let that one slide (unless you have something else in mind).

A few thoughts I have:

-
RequestParameters defines its ResponseDataType as JsonConverter while in reality I would prefer it to be DataConverter (which Java has). Can I go more specific? (I don't want the caller to have to define it)

-
Even though it's internal, I feel like having a DataConverter field in my RequestParameters is questionable. I am delegating the way I handle the request to the request itself. Granted, it can't be modified in any way but are there better options?

Program.cs

```
namespace Console
{
internal class Program
{
private static void Main(string[] args)
{
//ActiveQuestions();
NewestQuestionsByTag();

System.Console.ReadKey();
}

#region ActiveQuestions

private static void ActiveQuestions()
{
var settings = new ActiveQuestionsRequestParameters
{
SiteId = "155"
};
var socket = new StackSocket("wss://qa.sockets.stackexchange.com", settings);
socket.OnSocketReceive += OnActiveQuestionsDataReceived;
socket.Connect();
}

private static void OnActiveQuestionsDataReceived(object sender, SocketEventArgs e)
{
var data = e.Response.Data as ActiveQuestionsData;
if (data == null)
{
return;
}

System.Console.WriteLine("{0} - {1}", "Title", data.TitleEncodedFancy);
System.Console.WriteLine("{0} - {1}", "Tags", string.Join(", ", data.Tags));
System.Console.WriteLine("{0} - {1}", "Last activity", data.LastActivityDate);
System.Console.WriteLine("{0}", data.ApiSiteParameter);

Solution

I will look at this again in a couple of days or maybe even later today, but here is something I saw right at the end of the question

WebSocketReceiveResult response;

            while (true)
            {
                response =
                    await _socket.ReceiveAsync(new ArraySegment(temporaryBuffer), CancellationToken.None);
                temporaryBuffer.CopyTo(buffer, offset);
                offset += response.Count;
                temporaryBuffer = new byte[BufferSize];

                if (response.EndOfMessage)
                {
                    break;
                }
            }


you can get rid of that pesky if statement if you write the while loop as a do while and use the condition response.EndOfMessage

WebSocketReceiveResult response;

Do
{
    response =
        await _socket.ReceiveAsync(new ArraySegment(temporaryBuffer), CancellationToken.None);
        temporaryBuffer.CopyTo(buffer, offset);
        offset += response.Count;
        temporaryBuffer = new byte[BufferSize];
} While(!response.EndOfMessage)


I don't like this

if (data == null)
{
    return;
}


it's just looks blah.

you should at least let this print to the Console that the data was empty, both places that you used this if the data isn't null you print out information to the console, why wouldn't you want to know that the data was null? Print it out that the data was null.

I was just looking at this

private static void OnActiveQuestionsDataReceived(object sender, SocketEventArgs e)
    {
        var data = e.Response.Data as ActiveQuestionsData;
        if (data == null)
        {
            return;
        }

        System.Console.WriteLine("{0} - {1}", "Title", data.TitleEncodedFancy);
        System.Console.WriteLine("{0} - {1}", "Tags", string.Join(", ", data.Tags));
        System.Console.WriteLine("{0} - {1}", "Last activity", data.LastActivityDate);
        System.Console.WriteLine("{0}", data.ApiSiteParameter);
        System.Console.WriteLine(data.QuestionUrl);
        System.Console.WriteLine();
    }


and I think that if you aren't going to write anything to the console then you should restructure this so that it looks like this instead

private static void OnActiveQuestionsDataReceived(object sender, SocketEventArgs e)
    {
        var data = e.Response.Data as ActiveQuestionsData;
        if (data != null)
        {
            System.Console.WriteLine("{0} - {1}", "Title", data.TitleEncodedFancy);
            System.Console.WriteLine("{0} - {1}", "Tags", string.Join(", ", data.Tags));
            System.Console.WriteLine("{0} - {1}", "Last activity", data.LastActivityDate);
            System.Console.WriteLine("{0}", data.ApiSiteParameter);
            System.Console.WriteLine(data.QuestionUrl);
            System.Console.WriteLine();
         }
    }


this is straight forward what is going on.

I know a lot of people don't like using negative conditionals in their if statement, but if you aren't doing anything on Null other than exiting the method this is the cleanest way to do that.

Code Snippets

WebSocketReceiveResult response;

            while (true)
            {
                response =
                    await _socket.ReceiveAsync(new ArraySegment<byte>(temporaryBuffer), CancellationToken.None);
                temporaryBuffer.CopyTo(buffer, offset);
                offset += response.Count;
                temporaryBuffer = new byte[BufferSize];

                if (response.EndOfMessage)
                {
                    break;
                }
            }
WebSocketReceiveResult response;

Do
{
    response =
        await _socket.ReceiveAsync(new ArraySegment<byte>(temporaryBuffer), CancellationToken.None);
        temporaryBuffer.CopyTo(buffer, offset);
        offset += response.Count;
        temporaryBuffer = new byte[BufferSize];
} While(!response.EndOfMessage)
if (data == null)
{
    return;
}
private static void OnActiveQuestionsDataReceived(object sender, SocketEventArgs e)
    {
        var data = e.Response.Data as ActiveQuestionsData;
        if (data == null)
        {
            return;
        }

        System.Console.WriteLine("{0} - {1}", "Title", data.TitleEncodedFancy);
        System.Console.WriteLine("{0} - {1}", "Tags", string.Join(", ", data.Tags));
        System.Console.WriteLine("{0} - {1}", "Last activity", data.LastActivityDate);
        System.Console.WriteLine("{0}", data.ApiSiteParameter);
        System.Console.WriteLine(data.QuestionUrl);
        System.Console.WriteLine();
    }
private static void OnActiveQuestionsDataReceived(object sender, SocketEventArgs e)
    {
        var data = e.Response.Data as ActiveQuestionsData;
        if (data != null)
        {
            System.Console.WriteLine("{0} - {1}", "Title", data.TitleEncodedFancy);
            System.Console.WriteLine("{0} - {1}", "Tags", string.Join(", ", data.Tags));
            System.Console.WriteLine("{0} - {1}", "Last activity", data.LastActivityDate);
            System.Console.WriteLine("{0}", data.ApiSiteParameter);
            System.Console.WriteLine(data.QuestionUrl);
            System.Console.WriteLine();
         }
    }

Context

StackExchange Code Review Q#51304, answer score: 6

Revisions (0)

No revisions yet.