Sending a Torrent magnet link using the Deluge JSON API

I have built this script that sends a magnet link to the client. As I never ever had any proper programming lessons I would want to ask you if this approach is good, if I am missing something, There are parts of the script that are handled off-side, link checking, login credentials etc. What I have been thinking is that I could maybe create a JSONobject that will be accessible overall and for each request I would just set its fields and make the request. Or is my approach with methods generating the JSONobject and later turning it into a string good enough?

```
namespace TorrentSwitch.managers.Deluge
{

class Deluge
{

public static CookieAwareWebClient c;
public static string URL;

public static void InitializeWebClient()
{
c = new CookieAwareWebClient();
c.Encoding = Encoding.UTF8;
c.Headers.Set("content-type", "application/json");
}

public static void GetURL(Settings currentClient)
{
URL = "http://" + currentClient.hostname + ":" + currentClient.port + "/json";

}

private static byte[] BuildRequest(string method, string parameter, bool isTorrentRequest = false)
{
JArray jarrayObj = new JArray();
jarrayObj.Add(parameter);

if (isTorrentRequest)
{
JObject torrentExtraObject = new JObject();
jarrayObj.Add(torrentExtraObject);
}

JObject X = new JObject(
new JProperty("method", method),
new JProperty("params", jarrayObj),
new JProperty("id", "1"));

string json = JsonConvert.SerializeObject(X, Formatting.None);
byte[] request = Encoding.ASCII.GetBytes(json);

return request;
}

private static byte[] SendRequest(string method, string parameter, bool IsTorrentRequest = false)
{
byte[] response = c.UploadData(URL, "POST", BuildRequest(method, parameter, IsTorrentRequest));

return response;

Regarding your naming (especially in Settings): C# public members should always be named PascalCase (first letter of each word capitalized).

Next, you have:

public static CookieAwareWebClient c;
public static string URL;

Generally, you should never have any public fields (a field is written like the two items above, no get; set; etc.), you should always make public fields a property instead. Just as well, c should be renamed Client or another meaningful name.

public static CookieAwareWebClient Client { get; set; }
public static string URL { get; set; }

In the following case:

if (currentClient.label != String.Empty)

It's probably better to use if (!string.IsNullOrEmpty(currentClient.label)) here. It can be a cheaper call (sometimes, not always) but conveys the same meaning.

You use an enum (which is great):

public enum ClientType
{
    uTorrent,
    Deluge,
    Transmission
}

But you violate naming conventions. C# enum naming is PascalCase for the enum name and all values. (So uTorrent would be UTorrent or Utorrent.)

If you have access to C#6.0, the following:

public static List users = new List();

Becomes:

public static List Users { get; } = new List();

If not, then you simply need:

private static List _users = new List();
public static List Users { get { return _users; } }

Regarding:

public static void removeUser(string alias)
{
    users.Remove(GetByAlias(alias));
}

Again, public member naming dictates it should be RemoveUser instead.

In C# a namespace should follow PascalCase naming conventions:

namespace TorrentSwitch.torrent_clients

Should be:

namespace TorrentSwitch.TorrentClients

If you have access to C#6.0 (Visual Studio 2015+) then some methods which are a single, simple return can become an expression-bodied member, example:

public static Settings GetByAlias(string alias)
{
    //Will return the first Settings where the alias set in that class matches the alias you provied. 
    //Can also return null if none is existent.
    return users.FirstOrDefault(t => t.alias == alias);
}

To:

//Will return the first Settings where the alias set in that class matches the alias you provied. 
//Can also return null if none is existent.
public static Settings GetByAlias(string alias) =>
    users.FirstOrDefault(t => t.alias == alias);

I tend to leave at least one space after the double-slash in my comments:

//Will return the first Settings where the alias set in that class matches the alias you provied.

To:

// Will return the first Settings where the alias set in that class matches the alias you provied.

It makes them easier to read, especially in large blocks.

You can make use of var in a great deal many places of your code.

You have an example here:

public static void Loop()
{
    foreach (var setting in users)
    {
        //Loop them here and access them using setting.variable
        Debug.WriteLine(setting.alias);
    }
}

Anywhere the type is a given, you can replace with var:

private static byte[] SendRequest(string method, string parameter, bool IsTorrentRequest = false)
{
    byte[] response = c.UploadData(URL, "POST", BuildRequest(method, parameter, IsTorrentRequest));

    return response;
}

To:

private static byte[] SendRequest(string method, string parameter, bool IsTorrentRequest = false)
{
    var response = c.UploadData(URL, "POST", BuildRequest(method, parameter, IsTorrentRequest));

    return response;
}

Which can then become:

private static byte[] SendRequest(string method, string parameter, bool IsTorrentRequest = false) =>
    c.UploadData(URL, "POST", BuildRequest(method, parameter, IsTorrentRequest));

With C#6.0. There are other examples, I challenge you to find them all. :)

Your code doesn't look too bad, you should fix your whitespace though. Generally a maximum of one line where it's necessary.

Also: I apologize if anything came off harsh, I mean no rudeness I just happen to be really tired (reviewing at 01:45AM may not be a good idea) and I think this is a very good start. Hopefully you explore more with C# and become a great programmer in it! :)

StackExchange Code Review Q#152974, answer score: 6