views:

428

answers:

11

I'm developing a windows application in C# using VS2005. Consider i need to call a non static method(Method1) which is in a class(Class1) from another class(Class2). In order to call the method, i need to create object for that class.

But my class 'Class1' has more than 1000 variables. So every time i create object for 'Class1', all the variables are getting instantiated but my method 'Method1' uses very few of those variables. So unnecessarily all other variables are getting instantianted.

How can i optimize this code, as its an over head in instantiating the variables that are not needed for the particular method which has to be called.?

Or is there any other coding standard using which i can overcome this.? Thanks in advance.

here is the list of my variables. edit:

       private string m_sPath = String.Empty;
        private string m_sModule = String.Empty;
        private string m_sType = String.Empty;

        public static string strFileSave;
        private string fileName = "", strFactoryXML = "", strPriorityXml = "", strTempXml = "";
        private string selMethod = "";
        private string selClass = "";
        private string selMethodName = "";
        private string str, str1, str2;
        public static string strFolder;
        public static string strFilePath = "";
        public static string sel1, sel2;
        private string strSetPrFileName = "";
        private string results;
        private string strClassCompare, strMethodCompare, strTestResultCompare;
        public static string CurrWorkingDirectory = Environment.CurrentDirectory;
        private System.Threading.ThreadStart m_CummulativeTimeThreadStart = null;
        private System.Threading.Thread m_CummulativeTimeThread;
        private string[] OuterDelim = null;
        private string[] InnerDelim = null;
        public static ArrayList alObjectsNotDefined;
        public static ArrayList alSystemObjUnDefined = null;
        public static string objFileName = "";
        public static string XmlFileName = "";
        //Integers
        private int nMethodCount, index = 0;
        private static int nClearFlag, nExp = 0, cell;
        public static int flag;
        private static int nMemberCount, nPrevMemberCount;
        private static int classIndex, methodIndex;
        private static int[,] arrIndex = new int[100, 100];
        private int indexThread;
        private int bFlag = 0;
        //private int nIndexClass=0,nIndexMethod=0;
        frmCreateProject cp = new frmCreateProject();


        //Boolean
        private bool crFlag = false, tvAfter = true;
        private bool bSetPriority = false;
        private bool bFilter = false;

        private bool bCr = false;
        private bool snapShotFlag = false, factoryFlag = false, tvSel = false;
        private bool bResultsFlag = false, bSummaryFlag = false;
        public static bool bClick = true, rightMouse = false;
        private bool dataError = false;
        private bool bClassFilter = false, bMethodFilter = false;//,bTestResultFilter=false;
        private bool tvFact = false;
        //private bool bAssignFactory=false;
        //private bool factoryChanges=false;
        private bool bEntered = false;
        public static bool bAssign = false;
        private bool bChangeMethFactory = false;
        private bool bExecution = false;

                public bool bModule = false;
        public bool bClass = false;
        public bool bMethod = false;

        public bool bAssemLoaded = false;         public bool bLoadClicked = false;         public static bool bAssignCustom = false;
        public static string strPath;
        //  public static  int intRowCount;
        public bool bTestGen = false;
        //Others
        private Module[] mdls = null;
        private Assembly oAssembly;
        private DataTable dtSnapshot;
        private DataTable dtFactory;
        private DataTable dtFactoryGV        private DataTable dtExceptions = new DataTable();
        private DataTable dtResults = new DataTable();
        private DataTable dtSummary = new DataTable();
        private DataRow dr;
        private DataRow drGV;
        private DataColumn dc, dc1;
        private DataColumn dcGV;
        private Type m_Type;
        private Type[] types = new Type[100];
        private XmlDocument doc = new XmlDocument();
        //private XmlDocument docTemp=new XmlDocument();
        private DataGridTextBoxColumn TextCol;
        private DataGridTextBoxColumn TextColGV;
        private object[] arguments = new object[20];
        public static bool bSupport = false;
        DataGridTableStyle ts1 = null;
        DataGridTableStyle ts2 = null        DateTime gridMouseDownTime;
        XmlElement gridElement = null;
        XmlNodeList gridList = null, gridCustom = null;

        public string testAssem;




        public string strXmlTestCase;
        public string Gen_dll_path;
        public static string curClass = "";

        public static string curMethod = "";
        public static string curTestResult = "";

        public static bool comdll = false;

        public static bool bIncludeNullValue;
        public static string strMainExcep;

        public static string cmbFactoryClassSel = "";
        public static string cmbFactoryMethodSel = "";
        public static string dataGridParameter = "";
        private static string strReferredTable = "";


        public static string strExePath = ""; 
        public static string strProjPath = "";
        public static string strFactoryType = "";
        public static string strParameterName = "", strParameterType = "";
        public static string checkFactoryClass = "";
        public static string strFileXml = "";
        public static bool bEdit = false;
        public static bool bNumArr = false;
        public string checkType = "";
        public static bool bThread = true;
        public static bool bObjectsDefined = false;
        public static bool bSystemObjDefined = false;
        public static string cmbFactoryNewMethodSel = "";

        public static bool editAuto = false;
        private System.Windows.Forms.MainMenu mainMenu1;
        private System.Windows.Forms.MenuItem mnuFile;
        public System.Windows.Forms.MenuItem mnuTest;        private System.Windows.Forms.MenuItem mnuExit;
        private System.Windows.Forms.MenuItem mnuHelp;
        private System.Windows.Forms.MenuItem mnuAbout;
        public System.Windows.Forms.ToolBar toolBar1;        private System.Windows.Forms.TabControl tcResults;
        private System.Windows.Forms.TabPage tpSummary;
        private System.Windows.Forms.TabPage tpExceptions;
        private System.Windows.Forms.DataGrid dataGridSummary;
        private System.Windows.Forms.RichTextBox rtbOutput;
        private System.Windows.Forms.DataGrid dataGridExceptions;
        private System.Windows.Forms.Splitter splitterDown;
        private System.Windows.Forms.Splitter splitterTV;
        private System.Windows.Forms.OpenFileDialog openFileDialog1;
        private System.Windows.Forms.ImageList imageList1;
        private System.Windows.Forms.ToolBarButton toolBarOpen;
        private System.Windows.Forms.ToolBarButton toolBarTest;
        private System.Windows.Forms.Panel panelResults;
        private System.Windows.Forms.TabPage tpProblems;
        private System.Windows.Forms.RichTextBox rtbProblems;
        private System.Windows.Forms.MenuItem mnuCreateProject;
        private System.Windows.Forms.MenuItem mnuOpenProject;
        private System.Windows.Forms.ToolBarButton toolBarNew;
        private System.Windows.Forms.PictureBox pictureBox1;
        protected System.Windows.Forms.ContextMenu contextMenuTest;
        private System.Windows.Forms.ToolTip toolTip1;
        private System.Windows.Forms.MenuItem mnuAssignFactory;
        private System.Windows.Forms.ContextMenu contextMenuFactory;
        private System.Windows.Forms.MenuItem mnuViewAutoFactory;
        private System.Windows.Forms.ContextMenu contextMenu1;
        private System.Windows.Forms.MenuItem mnuEditCustomFactory;
        private System.Windows.Forms.MenuItem mnuDeleteCustomFactory;
        private System.Windows.Forms.ToolBarButton toolBarPriority;
        private System.Windows.Forms.MenuItem mnuSetPriority;
        private System.Windows.Forms.ContextMenu contextMenuResults;
        private System.Windows.Forms.MenuItem mnuPerformance;
        private System.ComponentModel.IContainer components;


        public static System.Windows.Forms.TreeView tempTV        public static string classNameGV;
        public string strAppPath = "";
        public string strTempPath = "";
        public bool bDllExists;
        public string smTemp1 = "";
        public string strTestFolder = "";
        public static bool bReloadNewDll = false;
        private string dllUnderTest = "";
        private System.Threading.ThreadStart m_UpdateDBTimeThreadStart = null;
        private System.Threading.Thread m_UpdateDBTimeThread;
        private TabPage tabPage2;
        private TreeView tvTestAssemb;
        private TabPage tabPage1;
        private TreeView tv;
        private TabControl tabControl1;
        public static string Output;
        private MenuItem menuItem3;
        private MenuItem menuItem4;
        private MenuItem menuItem5;
        private MenuItem menuItem1;
        private MenuItem menuItem2;
        private TabPage tpTestGen;
        public StatusBar statusBar1;
        private TextBox txtTestGen;
        private TabPage tpTestCaseTable;
        private DataGrid dataGrid2;
        private Panel panel1;
        private Button btnGenerate;
        private Button btnSave;
        private Button btnTestCase;
        private Label label5;
        private ComboBox cmbTestCase;
        private DataGrid dataGrid1;
        private TabPage tpGlobalVariable;
        public DataGrid dataGridFactoryGV;
        private Panel panelGlobalVariable;
        private ComboBox cmbMethodNameGV;
        private Label label2;
        private Label label1;
        private ComboBox cmbClassNameGV;
        private TabPage tpFactorySettings;
        private Panel panelFactory;
        public DataGrid dataGridFactory;
        private Panel panelClassDetails;
        private Label lblClassName;
        private ComboBox cmbClassName;
        private Label lblMethodName;
        private ComboBox cmbMethodName;
        private TabPage tpSnapshot;
        private Panel panelUpdateMsg;
        private PictureBox pictureBox2;
        private Label label4;
        private DataGrid datagridSnapshot;
        public TabControl tcSnapshot;
        public StatusBar statBar;


        public ArrayList alMultiLevelTableName = new ArrayList();

        public ArrayList alExpRes;        
public ArrayList alParVal; 
        public ArrayList alTestCaseID        
public ArrayList alClassName;
        private MenuItem menuItem6        
public ArrayList alMethSig;
        public ArrayList alComments;
+7  A: 

It's hard to say what can be done to solve this particular problem. What can be said, though, is that having such a huge class is definitely a code smell. Try refactoring your classes so that the class required for operation of a method in Class2 is of a more "sane" size.

EDIT Holy cow! You're obviously trying to invoke a method on a Form-derived class. Move required logic to a third class.

Anton Gogolev
can u explain me the EDIT part a bit. what can be moved to a third class.?
pragadheesh
Move out of the form all of the code that is not about the UI. That will probably include the code you're trying to call.
John Saunders
+1 to moving the logic to a class other than the Form.
Jeromy Irvine
A: 

I don't know concrete purpose of "Class1", but if some class has more than 1000 variables, it does not "smell" good. It seems, there is some design problem.

However you can look at "Singleton" and "IOC container" design patterns, mayby it will help.

TcKs
A: 

I'd suggest splitting your class up, >1000 variables in a single class could be a code smell that the class is doing too much?

If you split out the variables needed for Method1 into a separate class, that might make life simpler.

Colin Desmond
A: 

Class1 has more than 1000 variables? That alone hints that you have some design issue with the class. Also if you need an instance of the class frequently that uses only a portion of the class you most certainly should be able to break the class into smaller parts.

What is the idea behind Class1? What does it try to achieve?

Mikko Rantanen
+1  A: 

It sounds like you need to rethink your implementation a bit. If you have a class with 1000 variables where any given instance only uses a fraction of those variables, you'd be much better served by using a property bag of some sort.

public class C1 {
  private Dictionary<string,object> m_bag = new Dictionary<string,object>();
  public string Name {
    get { return (string)m_bag["Name"]; }
    set { m_bag["Name"] = value; }
  }
  ...
}

Otherwise you'll have to declare a field for every one of your variable. If you are only using just a few of those fields for every object, you will have an incredible amount of waste in your program. This sample is wasteful because it uses a backing store but it's almost certainly less wasteful than using a small number of fields in a 1000 field class.

JaredPar
A: 

I would recommend to read more about Single Responsibility Principle (SRP).

Vadim
I am not touching that with a 40 foot barge pole!
leppie
Original poster is still working on the Nine Hundred Ninety-Nine Responsibilities Principle.
mquander
+2  A: 

It would be very rare for a well-designed class to need 1000+ variables. So it sounds like there is something wrong with the design of Class1. If you need to call that non-static method, then you need to instantiate the class and there is no way around the performance hit. So, if you can, you need to refactor.

You could:

  • Split Class1 into smaller classes which are much more focussed on a single job.
  • You could refactor the method you need to call so it does not require the instantiation of the class (Might be possible as it doesn't use much fo the class data).

Good luck.

Sam Meldrum
+1  A: 

Everyone is being really nice when they say a 1000 variables in a single class is a code smell and hint at splitting up the class. Honestly, its far worse than that.

1000 variables in a single class is indefensible.

Brian Ensink
Assuming it's handcarved code, I totally agree. Generated code, not so much.
Andrew
Ah good point on generated code.
Brian Ensink
A: 

I have 452 static readonly fields for my entire IronScheme system. Then another odd 2000 other static fields (for boxed symbols and global references).

Still only takes 100ms to load.

leppie
A: 

One way to get started on splitting this class up is to look at what fields are used in what methods.

As you described above, you'll quickly see that not all methods use all of the fields in the class. This is a sign that your class lacks internal cohesion. What you want to do is refactor this class into several classes that are more cohesive internally.

So, look for clusters of functions that operate on the same fields. These clusters are smaller classes that want to break free of the uber class you have here. As you move these clusters of methods and the fields that they use into their own individual classes look for expressive names for the concept that the cluster represents.

Many of the resulting classes will be useful on their own (and can be tested on their own). The remaining original class may end up using many of these newly created class to perform the functions it provides.

Note that this is not a totally mechanical process, but is rather a heuristic that can give you a good start on cleaning up this mess.

Rob Scott
+1  A: 

Let's assume for a second that your object classes are well designed and necessarily large and stick to the problem at hand. If you are creating an instance of Class1 to call one method and one method only, perhaps the logic belongs elsewhere.

Does the method change the members of Class1 at all? Are you reading properties of Class1 to get the output? Once you get the output is the instance of Class1 done with?

If the answers to these questions are yes then this method may not belong in Class1 at all. Try redesigning the method so that only parmeters passed are used in the logic and the return value is the only output.

Brad_Z