Program for a trading application

Question

I am writing a program for my trading application. Over the months the program has been growing steadily, and what is used to be a small program now I would call a medium size program (about 1000-2000 lines in total), and it will keep growing as I add more features to it. It has grown to a point where it start to get difficult to read my code.

Below is a small part of the code from my over program. It is particularly nasty because I have a lot of calculations involved. What I think the following code is especially hard to read is because there are many similarities in blocks but differ from a plus (+) or minus (-) sign, so I cannot easily chunk it to new methods (refactoring). Should I just group the code into like 20-30 lines or so together and declare a new method? Then I will have many small methods which it is only be called in one place, which I think it defeat the purpose of a new method.

I have been reading extension method or so but I don't know if it is easy to apply to my program.

public class CVB
{
    public Bar bar = new Bar();

    public class Bar
    {
        public DateTime startTime;
        public DateTime endTime;
        public double open;
        public double high;
        public double low;
        public double close;
        public double bestBid;
        public double bestAsk;
        public int volume;
        public int delta;
        public int accVolume;

        public DateTime lastPrintTime;
        public DateTime lastMarcoTime;

        public string writeLine = null;
        public List<string> outputString = new List<string>();
        public Boolean validBarWrite = false;
    }

    public Info info = new Info();

    public class Info
    {
        public string symbol = null;
        public string exchange = null;
        public string finalSymbol = null;
        public int CVBSize = 200;
        public double tickSize;
        public int largeSizeLimit = 50;
        public Boolean realtimeChart = false;
        public int realtimeChartTimer = 2000;        
    }

          // more variables not included here....

    public Boolean initialise = false;

    public List<Level2> ask = new List<Level2>();
    public List<Level2> bid = new List<Level2>();
    public List<Level2> largeSize = new List<Level2>();

    public class Level2
    {
        public double price { get; set; }
        public int size { get; set; }

        public Level2(double price, int size)
        {
            this.price = price;
            this.size = size;
        }
    }

    class SortAscending : IComparer<Level2>
    {
        public int Compare(Level2 x, Level2 y)
        {
            if (x.price > y.price) return 1;
            else if (x.price < y.price) return -1;
            else return 0;
        }
    }

    class SortDecending : IComparer<Level2>
    {
        public int Compare(Level2 x, Level2 y)
        {
            if (x.price < y.price) return 1;
            else if (x.price > y.price) return -1;
            else return 0;
        }
    }

    public static void processLine(ref Output output, ref CVB[] cvb, int c)
    {
        double bidDistance;
        double askDistance;
        int oldProcessVolume;      // use to hold the previous volume, use to calculate the overflow when the CVB limit is reached
        int temp;

        double outputPrice = output.price;

        cvb[c].bar.writeLine = null;

        //////////convert the tick file into constant volume

        if (cvb[c].initialise == false)        //dont worry about the first bar, it is bugged, the number can be comeing from anywhere, not just trade
        {
            cvb[c].bar.startTime = output.time;
            cvb[c].bar.open = output.price;
            cvb[c].bar.high = output.price;
            cvb[c].bar.low = output.price;
            cvb[c].initialise = true;
        }

        if (output.price > 0 || output.type == "Volume")
        {
            switch (output.type)
            {
                case "Trade":
                Beginning:

                oldProcessVolume = cvb[c].bar.volume;
                cvb[c].bar.volume = cvb[c].bar.volume + output.volume;

                if (output.price > cvb[c].bar.high)
                    cvb[c].bar.high = output.price;

                if (output.price < cvb[c].bar.low)
                    cvb[c].bar.low = output.price;

                cvb[c].bar.close = output.price;

                if (cvb[c].bar.bestAsk != 0 || cvb[c].bar.bestBid != 0)       //not at the start of the program
                {
                    bidDistance = output.price - cvb[c].bar.bestBid;
                    askDistance = cvb[c].bar.bestAsk - output.price;

                    if (bidDistance - askDistance > 0.00000001)            //trade closer to ask
                    {
                        cvb[c].bar.delta = cvb[c].bar.delta + output.volume;

                        if (output.volume > cvb[c].info.largeSizeLimit)
                        {
                            temp = cvb[c].largeSize.Where(x => Math.Abs(x.price - outputPrice) < 0.00000001).Sum(y => y.size); //extract the last volume

                            cvb[c].largeSize.RemoveAll(x => Math.Abs(x.price - outputPrice) < 0.00000001);

                            cvb[c].largeSize.Add(new Level2(output.price, temp + output.volume));
                        }

                        if (cvb[c].bar.volume >= cvb[c].info.CVBSize)     // volume is filled
                        {
                            output.volume = cvb[c].bar.volume - cvb[c].info.CVBSize;   // cap the volume of the bar to the size of CVB, any overflow is put into the next bar
                            cvb[c].bar.delta = cvb[c].bar.delta - output.volume;

                            temp = cvb[c].ask.Where(x => Math.Abs(x.price - outputPrice) < 0.0000001).Sum(yield => yield.size);    //extract the last size on the ask, because we want accurate bid size and ask size when the bar is finish
                            cvb[c].ask.RemoveAll(x => Math.Abs(x.price - outputPrice) < 0.00000001);
                            temp = temp - (cvb[c].info.CVBSize - oldProcessVolume);
                            cvb[c].bar.accVolume = cvb[c].bar.accVolume + (cvb[c].info.CVBSize - oldProcessVolume);

                            if (temp > 0)
                                cvb[c].ask.Add(new Level2(output.price, temp));

                            cvb[c].bar.validBarWrite = true;
                            writeToString(ref output, ref cvb, c);

                            goto Beginning;
                        }
                    }
                    if (askDistance - bidDistance > 0.00000001)             //trade closer to bid
                    {
                        cvb[c].bar.delta = cvb[c].bar.delta - output.volume;

                        if (output.volume > cvb[c].info.largeSizeLimit)
                        {
                            temp = cvb[c].largeSize.Where(x => Math.Abs(x.price - outputPrice) < 0.00000001).Sum(y => y.size);

                            cvb[c].largeSize.RemoveAll(x => Math.Abs(x.price - outputPrice) < 0.00000001);

                            cvb[c].largeSize.Add(new Level2(output.price, temp - output.volume));
                        }

                        if (cvb[c].bar.volume >= cvb[c].info.CVBSize)     // volume is filled
                        {
                            output.volume = cvb[c].bar.volume - cvb[c].info.CVBSize;
                            cvb[c].bar.delta = cvb[c].bar.delta + output.volume;

                            temp = cvb[c].bid.Where(x => Math.Abs(x.price - outputPrice) < 0.0000001).Sum(yield => yield.size);    //extract the last size on the ask
                            cvb[c].bid.RemoveAll(x => Math.Abs(x.price - outputPrice) < 0.00000001);
                            temp = temp - (cvb[c].info.CVBSize - oldProcessVolume);
                            cvb[c].bar.accVolume = cvb[c].bar.accVolume + (cvb[c].info.CVBSize - oldProcessVolume);

                            if (temp > 0)
                                cvb[c].bid.Add(new Level2(output.price, temp));

                            cvb[c].bar.validBarWrite = true;
                            writeToString(ref output, ref cvb, c);

                            goto Beginning;
                        }
                    }
                    if (Math.Abs(bidDistance - askDistance) < 0.00000001)
                    {
                        if (cvb[c].bar.volume >= cvb[c].info.CVBSize)     // volume is filled
                        {
                            output.volume = cvb[c].bar.volume - cvb[c].info.CVBSize;
                            cvb[c].bar.accVolume = cvb[c].bar.accVolume + (cvb[c].info.CVBSize - oldProcessVolume);

                            cvb[c].bar.validBarWrite = true;
                            writeToString(ref output, ref cvb, c);

                            goto Beginning;
                        }
                    }
                }    //end if (output.price != 0)
                break;

                case "Ask":

                cvb[c].ask.RemoveAll(x => Math.Abs(x.price - outputPrice) < 0.00000001);
                cvb[c].bid.RemoveAll(x => x.price >= outputPrice - 0.00000001);

                if (output.volume > 0)
                    cvb[c].ask.Add(new Level2(output.price, output.volume));
                break;

                case "BestAsk":

                cvb[c].bar.bestAsk = output.price;
                cvb[c].ask.RemoveAll(x => x.price <= outputPrice + 0.000000001);
                cvb[c].bid.RemoveAll(x => x.price >= outputPrice - 0.00000001);

                if (output.volume > 0)
                    cvb[c].ask.Add(new Level2(output.price, output.volume));
                break;

                case "Bid":

                cvb[c].bid.RemoveAll(x => Math.Abs(x.price - outputPrice) < 0.00000001);
                cvb[c].ask.RemoveAll(x => x.price <= outputPrice + 0.000000001);           //eliminate the chance the bid is higher than the old ask size

                if (output.volume > 0)
                    cvb[c].bid.Add(new Level2(output.price, output.volume));
                break;

                case "BestBid":

                cvb[c].bar.bestBid = output.price;
                cvb[c].bid.RemoveAll(x => x.price >= outputPrice - 0.00000001);
                cvb[c].ask.RemoveAll(x => x.price <= outputPrice + 0.000000001);           //eliminate the chance the bid is higher than the old ask size

                if (output.volume > 0)
                    cvb[c].bid.Add(new Level2(output.price, output.volume));
                break;

                case "Volume":

                cvb[c].bar.accVolume = output.volume;
                break;

            }   //switch (output.type)
        }

        if (cvb[c].info.realtimeChart == true && output.time > cvb[c].bar.lastPrintTime.AddMilliseconds(cvb[c].info.realtimeChartTimer))      //update the string for the last bar
        {
            cvb[c].bar.validBarWrite = false;
            writeToString(ref output, ref cvb, c);
        }

        cvb[c].bar.lastPrintTime = output.time;
    }


           // more methods below
 }

Nearly every variable I have starts with cvb[c].something.... and is is hurting my readability for the code. I came from a VB background which I have with / end with to improves my readability. There is nothing in C# that is similar to VB with / end with. I read about other threads where they use

var p = this.StatusProgressBar;
p.IsIndeterminate = false;
p.Visibility = Visibility.Visible;
...

but you can't limit the scope of p within a certain place in the function, which I don't like.

The code for write to string

public static void writeToString(ref Output output, ref CVB[] cvb, int c)
{
    string bidString = null;
    string askString = null;
    string largeSizeString = null;
    double bestAsk = 0;
    double bestBid = 0;

    double processTickSize = cvb[c].info.tickSize;

    string timeFmt = "yyyy/MM/dd HH:mm:ss.fff";

    int currentBarBidAskSize = cvb[c].minimumWallSize;
    cvb[c].wallPrice[0] = 0;

    cvb[c].bar.endTime = output.time;

    //output all the info into the text file

    SortDecending sortDecending = new SortDecending();
    cvb[c].bid.Sort(sortDecending);

    SortAscending sortAscending = new SortAscending();
    cvb[c].ask.Sort(sortAscending);
    cvb[c].largeSize.Sort(sortAscending);

    for (int i = 0; i < 10; i++)
    {
        if (i < cvb[c].ask.Count && cvb[c].ask[i].price < cvb[c].ask[0].price + (20 * cvb[c].info.tickSize))
        {
            askString = askString + "," + cvb[c].ask[i].price.ToString() + "," + cvb[c].ask[i].size.ToString();
        }
        else
        {
            askString = askString + ",0,0";      //pad it until 10 size is shown
        }
    }

    for (int i = 0; i < 10; i++)
    {
        if (i < cvb[c].bid.Count && cvb[c].bid[i].price > cvb[c].bid[0].price - (20 * cvb[c].info.tickSize))
        {
            bidString = bidString + "," + cvb[c].bid[i].price.ToString() + "," + cvb[c].bid[i].size.ToString();
        }
        else
        {
            bidString = bidString + ",0,0";     //pad it until 10 size is shown
        }
    }

    for (int i = 0; i < cvb[c].largeSize.Count; i++)
    {
        largeSizeString = largeSizeString + "," + cvb[c].largeSize[i].price.ToString() + "," + cvb[c].largeSize[i].size.ToString();
    }

    cvb[c].largeSize.Clear();

    if (cvb[c].ask.Count > 0)
        bestAsk = cvb[c].ask[0].price;

    if (cvb[c].bid.Count > 0)
        bestBid = cvb[c].bid[0].price;

    int askSum5 = cvb[c].ask.Where(x => x.price >= bestAsk && x.price < bestAsk + (5 * processTickSize)).Sum(y => y.size);
    int bidSum5 = cvb[c].bid.Where(x => x.price <= bestBid && x.price > bestBid - (5 * processTickSize)).Sum(y => y.size);

    int askSum10 = cvb[c].ask.Where(x => x.price >= bestAsk && x.price < bestAsk + (10 * processTickSize)).Sum(y => y.size);
    int bidSum10 = cvb[c].bid.Where(x => x.price <= bestBid && x.price > bestBid - (10 * processTickSize)).Sum(y => y.size);


    cvb[c].bar.writeLine = string.Format("`{0},{1},{2},{3},{4},{5},ASK:{6},BID:{7},{8:0.##},{9:0.##},{10:0.##},{11:0.##},largeSize:{12}",
                        cvb[c].bar.endTime.ToString(timeFmt),
                        cvb[c].bar.open,
                        cvb[c].bar.high,
                        cvb[c].bar.low,
                        cvb[c].bar.close,
                        cvb[c].bar.accVolume,
                        askString,
                        bidString,
                        (double)cvb[c].bar.delta / cvb[c].info.CVBSize,                                                                         //delta ratio
                        (double)Math.Log10(Math.Abs((cvb[c].bar.endTime.AddMilliseconds(10) - cvb[c].bar.startTime).TotalSeconds)),     //bar duration in seconds, TAKE LOG, when the HH:mm time has fixed, then no need to get the abs value
                        (double)Math.Sign(askSum5 - bidSum5) * (Math.Max(askSum5, bidSum5) / (Math.Min(askSum5, bidSum5) + 0.0001)) - (Math.Sign(askSum5 - bidSum5)),          //size ratio price5
                        (double)Math.Sign(askSum10 - bidSum10) * (Math.Max(askSum10, bidSum10) / (Math.Min(askSum10, bidSum10) + 0.0001)) - (Math.Sign(askSum10 - bidSum10)),
                        largeSizeString);

    if (cvb[c].info.realtimeChart == false || cvb[c].bar.validBarWrite == true)
    {
        cvb[c].bar.open = cvb[c].bar.close;
        cvb[c].bar.high = cvb[c].bar.open;
        cvb[c].bar.low = cvb[c].bar.open;
        cvb[c].bar.volume = 0;
        cvb[c].bar.delta = 0;
        cvb[c].bar.startTime = cvb[c].bar.endTime;
    }
}

Any suggestions on improving the readability of my code? Should I add more comment in my code? It looks like a complete mess for anyone who doesn't know the function of the code. I have read somewhere that you should not need to add comments on the code if your code is structure well, just a header. My code is definitely not structured well but I don't know where to start improving it. Now I understand my code, but 6 months from now it will be a complete mess for me to look at the code again.

Answer 1

You obviously don't do enough encapsulation. I see at least three classes with no behaviour (Bar, Level2 and Info), your "main" class (CVB) contains only static method... it does not seem like good OOP for me (and when we are talking C# we should mostly talk OOP).

Let's try to fix it, one thing at a time.

public static void processLine(ref Output output, ref CVB[] cvb, int c)

Why are output and cvb ref, if it's not assigned anywhere in code? It should not be. Only things that implies assignment is call to writeToString(ref output, ref cvb, c);, which should not modify anything, because it's only writing. You have not included your code, but let's assume that it's not doing any assignments inside too. So we get rid of those refs.

Next thing in here: you're passing in an array of CVBs and some c, most likely an index. But everywhere in method (with exception to calls to writeString) you're using cvb[c]. So we can safely extract it into temporary variable. Let's call it cvbEl, for the lack of best option.

var cvbEl = cvb[c];

Another "safe" extraction is to put your 0.00000001 into a (class-level) constant, which we will name Tolerance, because as far as I can see, it is a floating-point comparison tolerance.

const double Tolerance = 0.00000001;

Now I'll do some strange thing: I'll extract your call to writeString to variable. I do this only because it is the only thing that operates with cvb and c and I can not make any decision about it without seeing it first. So I'll just save entire call for later.

Action writeToStringCall = () => writeToString(ref output, ref cvb, c);

And now, thanks to this, all our code operates on cvbEl. So we can safely move it to CVB instance method, because all this processing is coupled to single instance of CVB. Let's call this method Process (of course, all the code inside it does not use cvbEl, but uses implicit this. Now, all that's left of yours processLine is:

public static void processLine(Output output, CVB[] cvb, int c)
{
    var cvbEl = cvb[c];
    Action writeToStringCall = () => writeToString(ref output, ref cvb, c);
    cvbEl.Process(output, writeToStringCall);
}

In fact, it could be an one-liner if inlined all the variables, but I leave them as is for the sake of readability.

Next thing: you should extract every case in your switch as a separate method. Look at it now:

switch (output.type)
{
    case "Trade":
        Trade(output, writeToStringCall);
        break;
    case "Ask":
        Ask(output);
        break;
    case "BestAsk":
        BestAsk(output);
        break;
    case "Bid":
        Bid(output);
        break;
    case "BestBid":
        BestBid(output);
        break;
    case "Volume":
        Volume(output);
        break;
}

In fact, repeating pattern (for every type of Output call a method passing output inside) clearly calls for some inheritance on Output side: you should have several different Output classes, subtyped after your Output.Type, with method ProcessSmth(CVB), which should have processing related to that type of output. Having that you would replace this whole switch with output.ProcessSmth(this). But I won't do that now, and instead concentrate of contents of there methods (although they are already pretty small and readable, with exception of Trade).

private void Volume(Output output)
{
    bar.accVolume = output.volume;
}

Already nothing to do here.

ask.RemoveAll(x => x.price <= outputPrice + 0.000000001)

This line is repeated three times (although I suppose that third one is a mistake), so we extract it into method, also giving it meaningful name (RemoveAllAsksLessThan). We repeat same operation for remaining RemoveAlls. Let's look what's left of these:

private void BestBid(Output output)
{
    bar.bestBid = output.price;
    RemoveAllBidsGreaterThan(output.price);
    RemoveAllAsksLessThan(output.price);

    if (output.volume > 0)
        bid.Add(new Level2(output.price, output.volume));
}

private void Bid(Output output)
{
    RemoveAllBidsEqualTo(output.price);
    RemoveAllAsksLessThan(output.price);

    if (output.volume > 0)
        bid.Add(new Level2(output.price, output.volume));
}

private void BestAsk(Output output)
{
    bar.bestAsk = output.price;
    RemoveAllBidsGreaterThan(output.price);
    RemoveAllAsksLessThan(output.price);

    if (output.volume > 0)
        ask.Add(new Level2(output.price, output.volume));
}

private void Ask(Output output)
{
    RemoveAllAsksEqualTo(output.price);
    RemoveAllBidsGreaterThan(output.price);

    if (output.volume > 0)
        ask.Add(new Level2(output.price, output.volume));
}

All that's left is Trade. Most of it happens only when bar.bestAsk != 0 || bar.bestBid != 0. Let's invert that if so we don't have to think what happens if both bests are 0 (because we simply return at that, don't leave us hanging!).

if (bar.bestAsk == 0 && bar.bestBid == 0)
    return;

(BTW, you forgot your tolerance check)

if (output.price > bar.high)
    bar.high = output.price;

You do that for every run you got thru your Beginning: label, but in fact it won't change: neither output.price nor bar.high are changed elsewhere in this method. So we can safely move this operation above Beginning:. Same goes for bar.low, bar.close, bidDistance and askDistance.

Then we convert your goto into common-knowledge while(true). To do that we simply replace Beginning with while(true), enclose all code below in loop, add return in the end, and continue instead of every goto.

Inside loop we have three ifs. If we look closely, we'll see that they are mutually exclusive. We will mark it explicitly, so we don't have to guess. If we follow this line of thought, we'll see that every time we don't hit continue inside our if we can safely return from call. So let's invert those conditions too.

What we've now come to?

public static void processLine(Output output, CVB[] cvb, int c)
{
    var cvbEl = cvb[c];
    Action writeToStringCall = () => writeToString(ref output, ref cvb, c);
    cvbEl.Process(output, writeToStringCall);
}

public void Process(Output output, Action writeToStringCall)
{
    bar.writeLine = null;

    //////////convert the tick file into constant volume

    if (initialise == false)        //dont worry about the first bar, it is bugged, the number can be comeing from anywhere, not just trade
    {
        bar.startTime = output.time;
        bar.open = output.price;
        bar.high = output.price;
        bar.low = output.price;
        initialise = true;
    }

    if (output.price > 0 || output.type == "Volume")
    {
        switch (output.type)
        {
            case "Trade":
                Trade(output, writeToStringCall);
                break;
            case "Ask":
                Ask(output);
                break;
            case "BestAsk":
                BestAsk(output);
                break;
            case "Bid":
                Bid(output);
                break;
            case "BestBid":
                BestBid(output);
                break;
            case "Volume":
                Volume(output);
                break;
        }
    }

    if (info.realtimeChart && output.time > bar.lastPrintTime.AddMilliseconds(info.realtimeChartTimer))      //update the string for the last bar
    {
        bar.validBarWrite = false;
        writeToStringCall();
    }

    bar.lastPrintTime = output.time;
}

private void Volume(Output output)
{
    bar.accVolume = output.volume;
}

private void BestBid(Output output)
{
    bar.bestBid = output.price;
    RemoveAllBidsGreaterThan(output.price);
    RemoveAllAsksLessThan(output.price);

    if (output.volume > 0)
        bid.Add(new Level2(output.price, output.volume));
}

private void Bid(Output output)
{
    RemoveAllBidsEqualTo(output.price);
    RemoveAllAsksLessThan(output.price);

    if (output.volume > 0)
        bid.Add(new Level2(output.price, output.volume));
}

private void BestAsk(Output output)
{
    bar.bestAsk = output.price;
    RemoveAllBidsGreaterThan(output.price);
    RemoveAllAsksLessThan(output.price);

    if (output.volume > 0)
        ask.Add(new Level2(output.price, output.volume));
}

private void Ask(Output output)
{
    RemoveAllAsksEqualTo(output.price);
    RemoveAllBidsGreaterThan(output.price);

    if (output.volume > 0)
        ask.Add(new Level2(output.price, output.volume));
}

private void Trade(Output output, Action writeToStringCall)
{
    var oldProcessVolume = bar.volume;      // use to hold the previous volume, use to calculate the overflow when the CVB limit is reached
    if (output.price > bar.high)
        bar.high = output.price;
    if (output.price < bar.low)
        bar.low = output.price;
    bar.close = output.price;

    var bidDistance = output.price - bar.bestBid;
    var askDistance = bar.bestAsk - output.price;

    while(true)
    {
        bar.volume = bar.volume + output.volume;

        if (bar.bestAsk == 0 && bar.bestBid == 0)
            return;

        int temp;
        if (bidDistance - askDistance > Tolerance) //trade closer to ask
        {
            bar.delta = bar.delta + output.volume;

            if (output.volume > info.largeSizeLimit)
            {
                temp = largeSize.Where(x => ToleratedEquals(x.price, output.price)).Sum(y => y.size);
                largeSize.RemoveAll(x => ToleratedEquals(x.price, output.price));
                largeSize.Add(new Level2(output.price, temp + output.volume));
            }

            if (bar.volume < info.CVBSize)
                return;

            output.volume = bar.volume - info.CVBSize;
            bar.delta = bar.delta - output.volume;

            temp = ask.Where(x => ToleratedEquals(x.price, output.price)).Sum(yield => yield.size);
            RemoveAllAsksEqualTo(output.price);
            temp = temp - (info.CVBSize - oldProcessVolume);
            bar.accVolume = bar.accVolume + (info.CVBSize - oldProcessVolume);

            if (temp > 0)
                ask.Add(new Level2(output.price, temp));

            bar.validBarWrite = true;
            writeToStringCall();

            continue;
        }

        if (askDistance - bidDistance > Tolerance) //trade closer to bid
        {
            bar.delta = bar.delta - output.volume;

            if (output.volume > info.largeSizeLimit)
            {
                temp = largeSize.Where(x => ToleratedEquals(x.price, output.price)).Sum(y => y.size);
                largeSize.RemoveAll(x => ToleratedEquals(x.price, output.price));
                largeSize.Add(new Level2(output.price, temp - output.volume));
            }

            if (bar.volume < info.CVBSize)
                return;

            output.volume = bar.volume - info.CVBSize;
            bar.delta = bar.delta + output.volume;

            temp = bid.Where(x => ToleratedEquals(x.price, output.price)).Sum(yield => yield.size);
            RemoveAllBidsEqualTo(output.price);
            temp = temp - (info.CVBSize - oldProcessVolume);
            bar.accVolume = bar.accVolume + (info.CVBSize - oldProcessVolume);

            if (temp > 0)
                bid.Add(new Level2(output.price, temp));

            bar.validBarWrite = true;
            writeToStringCall();

            continue;
        }

        if (ToleratedEquals(bidDistance, askDistance))
        {
            if (bar.volume < info.CVBSize)
                return;

            output.volume = bar.volume - info.CVBSize;
            bar.accVolume = bar.accVolume + (info.CVBSize - oldProcessVolume);

            bar.validBarWrite = true;
            writeToStringCall();

            continue;
        }
        return;
    }
}

private static bool ToleratedEquals(double x, double y)
{
    return Math.Abs(x - y) < Tolerance;
}

private void RemoveAllAsksEqualTo(double outputPrice)
{
    ask.RemoveAll(x => ToleratedEquals(x.price, outputPrice));
}

private void RemoveAllAsksLessThan(double outputPrice)
{
    ask.RemoveAll(x => x.price <= outputPrice + Tolerance);
}

private void RemoveAllBidsGreaterThan(double outputPrice)
{
    bid.RemoveAll(x => x.price >= outputPrice - Tolerance);
}

private void RemoveAllBidsEqualTo(double outputPrice)
{
    bid.RemoveAll(x => Math.Abs(x.price - outputPrice) < Tolerance);
}

With exception of Trade that still need improvement, and where you can apply same process, it seems to me that we've done pretty good work here.

But I must repeat: with exception of one operation (moving process inside CVB instance) all we've done here is a function-based refactoring. As it's already said here, we might be as well programming in C or JS. I would highly recommend you to rethink your approach into more object-oriented one.

Answer 2

There are a few things you can do here.

1. Put each class in its own file. You don't really want to have to look through all of the class definitions to get to the code you care about. Typically every public class should have its own .cs file.
2. Definitely split code out into smaller methods. Each method you write should have exactly one purpose. Take for example your processLine method - processing an entire line of text is made up of several different operations. You can split out an Initialize method, and each logical operation should go into its own method.
3. Creating methods that are only called from one place is still a good thing. If you name your methods appropriately, it means you can look at the code and just read the name of the method to know what it's doing. As it is now, we can look at your code, but we have no idea why you're adding or subtracting any two numbers. We don't know what overall operation each block of code is computing or what it is really trying to calculate - simple methods can fix that.
4. Rename your variables. The parameter 'c' is meaningless - what is 'c'? Looking through the method, you can't tell what it is or what it's used for. cvb is the same - you should give that array a meaningful name
5. If you're going to be using the same array item over and over (cvb[c] for example) it can be more readable to extract it to its own variable. Save cvb[c] in a variable with a meaningful name so you don't have to look at the bracket syntax all over.
6. Create a constant for 0.00000001 and give the const a name to describe what that number is being used for. Why are you comparing to that number? The name of your constant should answer that question.

There are probably a few other things that you could dig into, but these are where I would start.

Answer 3

Ideas:

Do create methods and call them. That does not defeat... And it's easy - just select that code and click: Refactor -> Extractmethod.

Create classes and so, your variables will not pollute other classes with their names.

You can create one method for the two parts you mentioned, which differ by +/-. Just have an extra argument for the method: an int that will either be 1 or -1...

When creating new classes - you can put them in new files, and even in new folders (just right-click the project in solution explorer...) But beware - when creating new items in those folders - the namespace will be different. (But you can change it.)

And, when everything else fails - use #region - #endregion

Answer 4

You have a lot of classes nested under another class. Why not pull these out into their own files? It will greatly simplify the parent class.

Also, a perfect example of where you should be creating other methods comes in your very large conditional block. Why not pull out the long if..else statement into a separate method? That will make the switch block much easier to read.

Answer 5

1. Please refactor out the GOTOs into normal loops with conditions! :) PS: there is a 'goto case' that should have been used here instead of adding a label under the case, but I still would not recommend using it if you are going for readability and maintainability.

2. Consider refactoring out each actionable block into a separate method, and have the method names reflect it's action (i.e. 'FindLargestTrade', 'GetPreviousOverflowVolume', 'GetTradeCap' etc..)

3. Prefer using member variables in your classes to passing variables to each method (if all your class methods can be declared static, you might as well be writing in C)