CODEWITHSUNDEEP
c#refactoringsimplify
RatMonkey
RatMonkey

Reputation: 1

Too much code in a class, but not sure how to go about reducing it

I have written an application for use at work to check whether multiple POS device/s (computers) are online and replying to a network ping and then colour a corresponding rectangle either red or green depending on the ping response. It has 10 'lines' each capable of displaying a master server and upto 60 pos devices and a text box that can be populated with a site number. Then when you click go it will read 2 .ini files to find the sites IP scheme and the count of devices and their addresses so that each device can be pung (or pinged lol). I have attached a screenshot of the application, which might help explain what I have made?!

I can then use my application to see if the computers at one or more sites are online and, for example, if I run remote upgrades to the computers I can watch them all stop replying (box goes red) and start again (box goes green again) then I can easily tell once all of the updates have completed and the computers have all rebooted. My problem is that my code is (IMO) too long and may not be as efficient as it could be (60k lines!).

For any or all of the 10 lines the code behind each line is identical, apart from object and variable names etc. (i.e. the first 'reply' rectangle is called 'rectSt1Pos1' (which means Rectangle Shape-Store1-Pos1), then 'rectSt1Pos2' and so on and so forth upto '...Pos60', and repeated for all 10 lines (the last being 'rectSt10Pos60')). This has meant I have had to draw 610 individual boxes (as I wasn't too confident drawing them dynamically), and I'm happy with that, but I have done things like initialising all my variables at the start of the class (without assigning values) but that's over 1200 lines just to have a success and fail reply counters for each possible device that could be pung (10 sites, each with a master server, plus max 60 POS per site, one Success and 1 fail counter per device = 1220 int values to store the ping counts!)

OK, so that's hopefully enough with the application explanation, and as for my problem I have, i.e. my code for the application (most of which is in a single class (the only other class I have is one to read the ini files)) has just about reached 60,000 lines (yes sixty-thousand!!). This is basically 10 times the size I think it should be and I'm sure I should be able to reduce this, but I have no idea of how I can start to even think about refactoring the code!!.

Also, I think I should be able to perform all of the operations for each line in a generic class or method that can be called, but I'm not sure how I can do that as each lines rectangles and addresses etc. are different and would need to be updated cross-thread (I;'m ok with the cross-threading ;))

So in summary, does anyone have any advice on how to reduce code size?, or is it maybe ok to have a class with 60,000+ lines?, the application when compiled is about 1.8Mb and uses approximately 40Mb of memory, 15-25 threads and 400-1100 handles... Is that an acceptable overhead?

Any advice, opinions or help with this would be greatly appreciated. I can also post code snippets if it will help and as the Screen shot has not been posted (as I don't have enough reputation points apparently!) I can host that somewhere else if need be.

Many thanks for your time, please ask if anything is not too clear (but don't say all of it haha!)

Cheers,

Dan.

Upvotes: 0

Views: 229

Answers (2)

ChrisW
ChrisW

Reputation: 56083

You need to use subroutines:

  • One "handle line" subroutine
  • One "handle device" subroutine

Call the "handle line" subroutine 10 times.

Call the "handle device" subroutine 60 times per line.

Don't copy-and-paste the same code: use extract method around the lines which are copy-and-pasted, so that they only appear once (in the subroutine) instead of being copied.

Something like this:

class Device
{
  // device properties here
  int deviceState;
}

class Line
{
  // line properties here
  int lineState;
  // devices associated with this line
  Device[] devices;
  // constructor
  Line()
  {
    devices = new Device[60];
  }
}

class Main
{
  List[] lines;
  Main()
  {
    lines = new Lines[10];
    for(int i = 0; i < 10; ++i)
      HandleLine(i, line[i]);
  }
  void HandleLine(int lineNumber, Line line)
  {
    // get line status
    line.status = getLineStatus(lineNumber);
    // handle devices on this line
    for(int i = 0; i < 60; ++i)
      HandleDevice(lineNumber, i, line.devices[i]);
  }
  void HandleDevice(int lineNumber, int deviceNumber, Device device)
  {
    // get device status
    device.status = getDeviceStatus(lineNumber, deviceNumber);
  }
}

The above is pseudo-code to illustrate helper classes (Line and Device). Using C# you might want to use System.Collections.Generic.List instead of arrays.

Also, the HandleLine functionality could be a method of Line instead of Main. It could even be the constructor of the Line class.

Upvotes: 2

Christophe Roussy
Christophe Roussy

Reputation: 16999

60,000+ lines in one file ! Use methods/functions/subroutines/macros to abstract the input and output.

Also think about using loops instead of repeating the same command if the parameters differ with some logic (+1 each time for example).

Upvotes: 1

Related Questions