Wrapper for a transfer transaction API

I'm looking for any kind of advice, like when or where to do exception handling, the overall library structure/layout, usage of classes/partial classes, code efficiency, naming conventions, and so on and so forth.

Note: Knowledge of cryptocurrency technology would be an advantage.

The following code is the section that covers this API call. The whole wrapper can be found here. I will post code for one specific operation but would love if someone would be willing to do a full review.

Connecting to NIS:

using System;

namespace NemApi
{
public static class Connection
{
    public static string partialUri { get; set; }

    static Connection()
    {
        NisConnect();
    }

    public static void NisConnect(String nodeIP, int port)
    {
        partialUri = String.Format("http://{0}:{1}", nodeIP, port);
    }

    public static void NisConnect()
    {
        partialUri = String.Format("http://{0}:{1}", "127.0.0.1", 7890);
    }
}
}

When making any request, I need to work with JSON. For example, when sending a transaction, I populate a JSON data transfer object and then serialize it into a JSON string:

```
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace NemApi.DTOs.AccountTransactionObjects
{
public class TransactionVersionOne
{
public class Message
{
public string payload { get; set; }
public int type { get; set; }
}

public class Transaction
{
public long timeStamp { get; set; }
public long amount { get; set; }
public long fee { get; set; }
public string recipient { get; set; }
public int type { get; set; }
public long deadline { get; set; }
public Message message { get; set; }
public int version { get; set; }
public string signer { get; set; }
}

public class RootObject
{
public Transaction transaction { get; set; }
public string privat

Bigger Issues

Making Connection static is usually asking for trouble. The reason is you can't swap out the connection for something else, you can't have it run it with two different servers in the same process.

Static classes also make it very difficult to unit test code.

I recommend you make a IConnection interface that describes your Connection. Then, create a single connection and pass it down through your various constructors. It is annoying, but if it really bothers you, you could consider using an IoC container such as Ninject to handle the constructor chains for you.

I highly recommend you use enums or symbolic types for some of your constants.

Tx.type = 257;
Msg.type = encrypted ? 2 : 1;


Having numbers like that are asking for trouble. Newton JSON has a fair amount of work in handling that so you can use C#-isms and reduce your chance of failures.

I recommend deadline is a DateTimeOffset or some sort of time-based element that you convert inside your DTO class. That will reduce the chance of errors but also make it more obvious how it works. As it stands know, you need to know a lot more of the internals to figure out how to format that deadline properly. If it is an offset, use TimeSpan instead. Things like the / 1000 should be encapsulated inside the class, not in the consumers.

While your sender and recipient are strings, it would reduce the chance of errors if you wrap that in a NisPublicKey (NisPrivateKey for sendiner?) or some sort of class that indicates its purpose. Again, it will make it easier to detect errors.

Fee processing should be encapsulated. Your DTO can hold the fee and calculate it (or have a wrapper that does it).

VersionOne as a method name doesn't really scale well. I highly recommend you make your class based on one (and use numbers instead of text).

This code look wrong. The brace doesn't mean anything and gives the impression it is under the response.Content which it is not.

var content = response.Content;
    {
        var json = await content.ReadAsStringAsync();

        var model = JsonConvert.DeserializeObject(json);

        return model;
    }

Minor

You might consider using UriBuilder and Uri instead of strings. I know it is a minor thing, but the compiler will catch the three in the morning mistakes if you have type safety.

Cosmetic

If you are following Microsoft's suggested conventions, all of your properties and methods should be PascalCase. So, partialUri would be PartialUri.

Generally speaking, I would have named Connection to NisConnection and then make the method names simpler (Connect).

Consider avoiding generic class names (Message) since Intellisense will create a lot of noise since multiple packages with that name will show up.

I like one class per file, but I find it easier to work with.

NewtonJSON has the ability to convert PascalCase properties into camelCase automatically. That will let you use the conventions of your language without making things feel "off".

If you use Microsoft's conventions, variables are camelCase. So Tx would be tx (though, I would have used transaction instead).

StackExchange Code Review Q#125727, answer score: 5