Restart Citrix session using locally stored application

I have a Windows forms application which we deployed in my firm for use on thin clients. The app lives locally on each user's machine, but pretty much all of the work the users do is over a Citrix session.

One of the options (the main option) available to the user is the ability to restart their own session. I recently attempted to completely redesign this area of the program as version which is currently live is very messy despite the fact that it works quite reliably.

A brief outline:

A list of servers is retrieved from AD. It check if an appdata file (created by the logon script) is present which contains the server name. If not, or if it contains the wrong server, then it iterates through all servers and queries them through the CMD until the server is found. It then updates the session with these details. If there is no server (for example, halfway through a restart), then it opens Outlook (published) and checks if the user is logged on against our server list, then displays a loading screen until "lync.exe" is running.

The session reset is initiated inside a background worker. This keeps the UI responsive and allows for a loading GIF (on page 10):

BackgroundWorker Reset

```
public void backgroundReset_DoWork(object sender, DoWorkEventArgs e)
{
// Allow page change when whilst on a thread other than the Form's
pageControl1.Invoke(new Action(() =>
{
pageControl1.SelectedIndex = 10;
}));

// _currentsession is initialised right at the beginning, in a separate backgroundWorker (if there is no session, one is created and assigned to _currentsession)
Session oldsession = _currentSession;
Session newsession = oldsession.Reset();

e.Result = newsession;
}

public void backgroundReset_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e)
{
Session newsession = (Session)e.Result;

if (newsession.ID == null)
{
// No session - re

Auto-Implemented Properties

If you're using any C# 3.0 or higher, you can use an Auto-Implemented Property, rather than manually declaring private property variables and the getters and setters. If you use Auto-Implemented Properties, then this chunk of code:

#region Fields
private string _username;
private string _pcname;
private int? _id; // the ? allows it to be nullable ... to make checking easier
private string _server;
#endregion

#region Properties
public string UserName
{
    get { return _username; }
    set { _username = value; }
}
public string PCName
{
    get { return _pcname; }
    set { _pcname = value; }
}
public int? ID
{
    get { return _id; }
    set { _id = value; }
} 
public string ServerName
{
    get { return _server; }
    set { _server = value; }
}
#endregion

Can become the following much shorter and much cleaner looking piece of code:

public string UserName { get; set; }
public string PCName { get; set; }
public int? ID { get; set; }
public string ServerName { get; set; }

You can also remove the #region Properties as well since there's no real need to be able to expand and minimize four lines of code.

Nitpicks

You have a lot of unnecessary comments in your code. The best example I can find was probably this one:

//////////////////////////////////////////////////////////// Reset

There are quite a few things wrong with this:

• The function it resides above is itself named "Reset".

• It contains an excessive amount of forward slashes, distracting whoever may be trying to read the code.

There are many more comments in your code like this one, and I'm sure you'll be able to pick them out and improve upon them.

If you really want to have a useful comment, I'd highly recommend using an XML Documentation Comment. A typical XML Documentation Comment looks something like this:

/// 
/// Describe your property, function or class here.
/// 
/// Make a remark about your property, function or class here, if needed.
/// Describe your parameter(s) here and below, if needed.

I've also noticed that you've used string literals in some places for path strings, but in other places you just write a normal string with escaped backslashes, like you did here:

private static string AppDataCitrix = AppData + "\\Citrix\\SelfService\\";

Whether you use string literals for paths or choose not to, just remember the important rule of be-consistent.

When I was first looking at your code, I came across these two lines:

Session oldsession = _currentSession;
Session newsession = oldsession.Reset();

There is no need for the oldsession variable in this scenario. It's just extraneous code. You could write these two lines as one:

Session newSession = _currentSession.Reset();

StackExchange Code Review Q#118058, answer score: 3