First SignalR application (Connecting 2 clients; mobile and desktop)

Two days ago I started reading about SignalR and today I started writing a simple application. I have written the code below so far, but I was wondering if you have some tips to improve my code.

Program

• Desktop client will automatically create a random group (random) and displays the random generated code to the user.

• Mobile client has the possibility to connect with the desktop client (both clients in one group) by entering the random generated code.

• Maximum users per group: 2 (1; Desktop, 2; Mobile).

• All sessions need to be registrered in my database.

-
The session has to be removed from the database when closing it.

```
public class ExpertHub : Hub
{
// OnDisconnected
public override Task OnDisconnected(bool stopCalled)
{
if(stopCalled)
{
removeSession(Context.ConnectionId);
}
else
{
// This server hasn't heard from the client in the last ~35 seconds.
}
return base.OnDisconnected(stopCalled);
}

// Send Message To Specified GroupName
public void Send(string message, string groupName)
{
Clients.OthersInGroup(groupName).addMessage(Context.ConnectionId + ": " + message);
}

// Join Group At Desktop
public async Task JoinDesktop(string groupName)
{
if(CountGroup(groupName) a.RoomName == groupName).FirstOrDefault();

if (group == null)
return false;
else
return true;
}
}

// Count Occurences of GroupName
public int CountGroup(string groupName)
{
using (var db = new ConnectionContext())
{
int group = db.Connections.Where(a => a.RoomName == groupName).Count();
return group;
}
}

// Write A New Session To The Database
public void newSession(string groupName)
{
using (var db = new ConnectionContext())
{
var session = new Connection()

In :

if(stopCalled)
{
    removeSession(Context.ConnectionId);
}
else
{
    // This server hasn't heard from the client in the last ~35 seconds.
}

Why do you keep the else block if there's nothing in it?

You should always use brackets when there's a bug possibility. In such a case :

else
    Clients.Caller.addMessage("Something went wrong. Please refresh your browser");

should use brackets, just to make sure you don't introduce some nasty bugs.

And hey, you could reverse your condition to remove some nesting :

if(CountGroup(groupName) < 1)
{
    // Write new session to database.
    newSession(groupName);
    // Add user to group
    await Groups.Add(Context.ConnectionId, groupName);
}
else
    Clients.Caller.addMessage("Something went wrong. Please refresh your browser");

could be :

if(CountGroup(groupName) > 1)
{
    Clients.Caller.addMessage("Something went wrong. Please refresh your browser");
    return;
}

// Write new session to database.
newSession(groupName);
// Add user to group
await Groups.Add(Context.ConnectionId, groupName);

This method name isn't very explicit :

// Check if group already exists
if (CheckGroup(groupName))

Read your comment, read your method name. Does it mean the same thing? No it doesn't. In that case, CheckGroup should be GrouExists or something like that. Check is a very vague term.

CountGroup(groupName) < 2 should be GetGroupCount or something like that. Because right now your method doesn't explain enough.

That :

if (group == null)
    return false;
else
    return true;

Could be changed to return group != null;

You have some methods that don't respect naming like newSession(groupName);. It should be PascalCased, so NewSession(groupName);

Finally, your header comments should follow conventions.

// Send Message To Specified GroupName
public void Send(string message, string groupName)

You should use the triple dash style comments, which will be used by the IDE when you type your code. But overall your comments need to be more useful. For example, it's useless to state what your code does. It should state why you're doing it. If you feel like your code doesn't need to be explained, don't put any comments.

StackExchange Code Review Q#113470, answer score: 2