DevX Home    Today's Headlines   Articles Archive   Tip Bank   Forums   

Results 1 to 5 of 5

Thread: Class vs structs design issues

  1. #1
    Join Date
    Oct 2005
    Posts
    173

    Class vs structs design issues

    This question is really an extension of http://forums.devx.com/showthread.php?t=153459
    Quote Originally Posted by Danny
    As a rule, when a class has something like 40 data members, it's a design gaffe so one needs to go back to the drawing board. However, ignoring this issue for a moement...
    Lets not ignore this issue. It will only be harder to fix later. I would appreciate suggestions on how to improve the class.

    I'm given a file in the following format:
    Code:
    common1, common2, ... common10
    case1_first, case1_second, ... case1_last
    case2_first, case2_second, ... case2_last
    ...
    caseX_first, caseX_second, ... caseX_last
    The 10 common parameters applies to all cases. The number of parameters from first to last in each case is also fixed. Only the number of cases X varies in each file.

    I'm treating each file as an obj and each case as an object. So the class is pretty simple, it just stores the common parameters.
    Code:
    class StressFile
    {
        double common1, ... common10;
        vector<LoadCase> loadCases;
    };
    The messy part is data for each case. Even though the actual input parameters are fixed, there are also variables that are calculated per case. So this means more data than just first to last. The extra data members are the results of manipulating the input data.
    Code:
    class LoadCase
    {
    public:
        StressFile * belongsTo;   // keeps track of which file this case belongs to
        // constructor
        LoadCase(vector<string> & casein, StressFile * parent);
        ~LoadCase(void);
        // data members
        struct BodyData{
            double alpha;
            double beta;};
        struct AccelData{
        	double rAccel, pAccel, yAccel;};
        struct LoadData{
            double LV, LS, LA;
            double PM, YM, RM;};
        struct ComponentData{
            LoadData int;
            LoadData fct;
            LoadData trt;
            LoadData not;
            LoadData compTotal;};
        struct partAData{
            double alpha, gama;
            double nz, ny, nx;
            AccelData CG;
            ComponentData pwrOnAir_AFL, pwrOnAir_LRP;
            LoadData PwrOnAirThrInc_AFL;
            LoadData PwrOnAirThrInc_LRP;
            LoadData PwrOnAirThrInc_WLE;
            LoadData PwrOnInertia_LRP;
            LoadData PwrOnTotal_LRP;
            ComponentData pwrOffAir_AFL, pwrOffAir_LRP;
            LoadData PwrOffAirWind_AFL;};
        struct partBData{
            LoadData PwrOnAirThrInc_LRP;
            LoadData PwrOnAirThrInc_WLE;
            LoadData PwrOnInertia_LRP;
            LoadData PwrOnTotal_LRP;};
        struct partABData{
            double dExt,sFric;
            ComponentData pwrOn, pwrOff;
            LoadData PwrOnAirThrInc_WLE;
            LoadData PwrOnAirThrInc_LRP;
            LoadData PwrOffAirWind_WLE;};
            
        string        conditionNumber, conditionTitle;
        double        pRate, yRate;
        partAData     partA;
        partBData     partB;
        partABData    partAB;
        BodyData      body;
        vector<InterfaceLoadData> interfaceLoads;
     
    
        // member functions
        void calcAppliedLoads();
        void calcUnitSoln();
        void LoadCaseCommon(vector<string> & casein);
        void moveLoadsFromTo(POINTLOCATION & from, Matrix<double> & fromData , POINTLOCATION & to, LoadData & toData);
    };

    Quote Originally Posted by Danny
    2) Simplify the class declaration. It really seems as if you're trying to create what is known as a God Class, a class which does too much, knows too much, and actually functions as a full blow application. For starters, you may simplify the internal data structures, leveraging them from dumb data structs to full blow classes, and then inherit from them or use them as embedded objects.
    Using structs was my way of trying to simply. Bascially they really just need to hold data. The class dosen't do a lot but does have to know and keep track of a lot of data.


    Quote Originally Posted by Danny
    3)You seem to be enamored with the notion of offset calculations (seriously, do you really need to be able to sort these objects according to every possible data member? I think I counted about 40 different variables of type double inside this class!) so you want to look into the offsetof macro. A word of caution: this macro works only for POD types. If your class virtual member functions, a virtual destructor, or base classes, the results of using offsetoff are undefined. Here as well, you need a flat data layout, without nesting types.
    Yeah, This is one requirement. Depending on what type of analysis, I will need to be able to sort by each field.



    So, any advice on how to organize my class better?
    What's the rule on when to use struct versus class?

    Thanks!

  2. #2
    Join Date
    Nov 2003
    Posts
    4,118
    OK, first of all I'd spilt the class into several smaller classes. In fact, each struct that you consider as "just holding data" should be a class per se, with constructor(s), overlaoded operators that could be used for comparisons and sorting, and setters and getters for each member. Since many members are represented as a pair of long double called LoadData, LoadData itself should be a class with the said functionality. Once every struct becomes a first class object, it will be much easier to compose a larger class from those building bricks. Notice also that the ultimate class doesn't have to contain all these data members as value objects; it could use some form of indirection, say a pointer to a struct etc.
    As for sorting: since you need to sort the data according to each data member, you need to redesign a more manageable interface. It's not enough to overload == , < etc. You also need a way of telling the object according to which members you want to sort, so you probably need an enumerator that is mapped to every field. The actual LoadCase class will not sort directly, it will merely delegate the intrinsic operations to its own smaller objects, which will in turn take care of their own private data members.

    Another possible design approach: store all the long double variables in a map that binds a case to a long double value. This way all you need to take care of is pairs of type <enum_field, long double>. When you use a map, many operations are much simpler because the map is a self sorting container. In your case, this can be a huge relief!

    To conclude, I think you need to think over the design goals as follows:
    1) what are the smallest independent data structures in your app? Make each one of them a self-suuficient class with all the necessary OO interfaces. Never ever expose data members with a public access specifier. It's very tempting to claim that "these are just dumb data containers" but at the end of the day, so is every object. In OO, you never do this. Think for example of a simple implementation detail that could topple your design if data members are public: are the data members decaled as a an array of two doubles or as two indiviudal variables? If you make them public, you force the users of this struct to assume too much about the underlying implementation.
    2) The larger class which is created by a collection of smaller, self sufficient objects, should provide an interface, but it should delegate many of the operations to the smaller classes.
    3) offset claculations etc. could work. I'm not saying that they're totally out of the question. However, they should always be implemented at the inner-most circle, i.e., as a private member function of class that contains the said data members.
    4) You have 10 different cases, each of which is matched to a variable. In some designs, it's best to use the facade pattern which essentially creates a single class for all cases, plus a flag that tells the class object which case it's representing. A classic example is charecters. Would you design 26 (52, for uppercase and lowercase) small classes to represent the letters of the alphabet or would you instead create a single class + an enumeration that tells its object which letter it's representing?
    Anyway, these are just crude ideas that should be considered while you're at the drawing board. Remember that your ultimate goal doesn't have to be a single class that knows everything and does everything; a modular design that consists of many autonomous building blocks is much more resilient to future design changes, debugging etc.
    Last edited by Danny; 06-09-2006 at 05:31 PM.
    Danny Kalev

  3. #3
    Join Date
    Oct 2005
    Posts
    173
    Ok, so let me start with LoadData.
    Code:
    class InputLoadData
    {
    private:
        double LV, LS, LA, PM, YM, RM;
    public:
        InputLoadData(double V, double S, double A, double P, double Y, double R);
        double getLV();
        double getLS();
        double getLA();
        double getPM();
        double getYM();
        double getRM();
    };
    bool operator==(InputLoadData A, InputLoadData B);
    bool operator<(InputLoadData A, InputLoadData B);
    I decided to make a child class here so that I can have read-only loads and those that can be manipulated.

    Code:
    class CalcLoadData:public InputLoadData
    {
    public:
        CalcLoadData(double V, double S, double A, double P, double Y, double R);
        CalcLoadData(CalcLoadData old);
        void setLV(double);
        void setLS(double);
        void setLA(double);
        void setPM(double);
        void setYM(double);
        void setRM(double);
        void setAll(double V, double S, double A, double P, double Y, double R);
        void clearAllData();
    };
    bool operator==(CalcLoadData A, CalcLoadData B);
    bool operator<(CalcLoadData A, CalcLoadData B);
    I'll have to chew on the sorting part some more....
    Last edited by rssmps; 06-10-2006 at 10:40 AM.

  4. #4
    Join Date
    Nov 2003
    Posts
    4,118
    A few stylistic comments: make your constructors explicit, and provide default values for constructor arguments. This will be useful when you want to zero initialize all members automatically. Also,

    CalcLoadData(CalcLoadData old);

    This ctor declaration will not work. You have to pass the argument by reference (it should probably be a reference to const).
    Finally, declare all your get functions as const:
    double getLV() const;
    Danny Kalev

  5. #5
    Join Date
    Oct 2005
    Posts
    173
    Yeah, it was late fri... not a very productive time.... LOL.

    I tried to keep all the comparison done by the object itself.

    InputLoadData.h
    Code:
    #ifndef InputLoadData_h
    #define InputLoadData_h
    #include <map>
    using namespace std;
    class InputLoadData
    {
    private:
        double LV, LS, LA, PM, YM, RM;
        map<string, int> mapObj;
    public:
        friend class CalcLoadData;
        InputLoadData(double A,double S, double V,  double P, double R, double Y);
        InputLoadData(const InputLoadData & old);
        double getLV() const;
        double getLS() const;
        double getLA() const;
        double getPM() const;
        double getYM() const;
        double getRM() const;
        bool lessThan(const InputLoadData & A, string name);
    };
    bool operator==(InputLoadData &  A, InputLoadData &  B);
    
    class LT_LoadData
    {
    public:
        LT_LoadData(string _idx):idx(_idx){};
        bool operator()(InputLoadData & A, InputLoadData & B)
        {
            return(A.lessThan(B,idx));
        };
    private:
        string idx;
    };
    #endif
    added a lessThan function to the object so that it will kno how to compare itself. It uses a map to distinguish which data is being compared.

    InputLoadData.cpp
    Code:
    #include <map>
    #include <iostream>
    #include <string>
    #include "InputLoadData.h"
    
    InputLoadData::InputLoadData(double A, double S, double V, double P, double R, double Y):LA(A),LS(S),LV(V),PM(P),RM(R),YM(Y)
    {
        mapObj["LA"]=0;
        mapObj["LS"]=1;
        mapObj["LV"]=2;
        mapObj["PM"]=3;
        mapObj["RM"]=4;
        mapObj["YM"]=5;
    };
    InputLoadData::InputLoadData(const InputLoadData & old)
    {
        LA = old.getLA();
        LS = old.getLS();
        LV = old.getLV();
        PM = old.getPM();
        RM = old.getRM();
        YM = old.getYM();
    };
    double InputLoadData::getLV() const{return LV;};
    double InputLoadData::getLS() const{return LS;};
    double InputLoadData::getLA() const{return LA;};
    double InputLoadData::getPM() const{return PM;};
    double InputLoadData::getYM() const{return YM;};
    double InputLoadData::getRM() const{return RM;};
    bool operator==(InputLoadData & A, InputLoadData & B)
    {
        if(A.getLV()==B.getLV())
        {
            if(A.getLA()==B.getLA())
            {
                if(A.getLS()==B.getLS())
                {
                    if(A.getPM()==B.getPM())
                    {
                        if(A.getYM()==B.getYM())
                        {
                            if(A.getRM()==B.getRM())
                            {
                                return true;
                            }
    	           else
                                return false;
                        }
                        else
                             return false;
                    }
                    else
                         return false;
                 }
                 else
                    return false;
              }
              else
                  return false;
          }
          else
              return false;
    };
    bool InputLoadData::lessThan(const InputLoadData & A, string name)
    {	
    	switch(mapObj[name])
    	{
    		case 0:
    			if(LA<A.getLA())
    				return true;
    			else
    				return false;
    			break;
    		case 1:
    			if(LS<A.getLS())
    				return true;
    			else
    				return false;
    			break;
    		case 2:
    			if(LV<A.getLV())
    				return true;
    			else
    				return false;
    			break;
    		case 3:
    			if(PM<A.getPM())
    				return true;
    			else
    				return false;
    			break;
    		case 4:
    			if(RM<A.getRM())
    				return true;
    			else
    				return false;
    			break;
    		case 5:
    			if(YM<A.getYM())
    				return true;
    			else
    				return false;
    			break;
    		default:
    			cout<<name<<" is not a valid sorting index selector"<<endl;
    			cout<<"must be LA, LS, LV, PM, YM, RM"<<endl;
    			exit(1);
    			return false;
    	}
    };
    CalcLoadData.h
    Code:
    #ifndef CalcLoadData_h
    #define CalcLoadData_h
    using namespace std;
    
    class CalcLoadData:public InputLoadData
    {
    public:
        CalcLoadData(double A, double S, double V, double P, double R, double Y);
        CalcLoadData(const CalcLoadData & old);
        void setLV(double lv);
        void setLS(double ls);
        void setLA(double la);
        void setPM(double pm);
        void setYM(double ym);
        void setRM(double rm);
        void setAll(double A,double S, double V, double P, double R, double Y);
        void clearAllData();
    };
    bool operator==(CalcLoadData & A, CalcLoadData & B);
    #endif
    CalcLoadData.cpp
    Code:
    #include <map>
    #include <iostream>
    #include <string>
    #include "InputLoadData.h"
    #include "CalcLoadData.h"
    
    CalcLoadData::CalcLoadData(double A, double S, double V, double P, double R, double Y):InputLoadData(A,S,V,P,R,Y){};
    CalcLoadData::CalcLoadData(const CalcLoadData & old):InputLoadData(old){};
    void CalcLoadData::setLV(double lv){LV = lv;};
    void CalcLoadData::setLS(double ls){LS = ls;};
    void CalcLoadData::setLA(double la){LA = la;};
    void CalcLoadData::setPM(double pm){PM = pm;};
    void CalcLoadData::setYM(double ym){YM = ym;};
    void CalcLoadData::setRM(double rm){RM = rm;};
    void CalcLoadData::setAll(double A,double S, double V, double P, double R, double Y)
    {
    	LA = A;
    	LS = S;
    	LV = V;
    	PM = P;
    	RM = R;
    	YM = Y;
    };
    void CalcLoadData::clearAllData(){};
    bool operator==(CalcLoadData & A, CalcLoadData & B)
    {
    	return (operator==(static_cast<InputLoadData>(A), static_cast<InputLoadData>(B)));
    };
    Last edited by rssmps; 06-12-2006 at 04:36 AM.

Similar Threads

  1. Alarm class
    By Aristo in forum Java
    Replies: 6
    Last Post: 12-05-2005, 12:07 PM
  2. Nested Class Definitions
    By evac-q8r in forum C++
    Replies: 13
    Last Post: 08-02-2005, 11:50 PM
  3. design issues
    By Sean Newton in forum Database
    Replies: 1
    Last Post: 10-23-2002, 11:14 PM
  4. Call shared method without class instantiation?
    By Robert C. Cain in forum .NET
    Replies: 3
    Last Post: 01-27-2002, 02:40 AM
  5. Performance Issues around Page Design
    By Brian in forum authorevents.mitchell
    Replies: 3
    Last Post: 10-17-2000, 04:47 PM

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •  
HTML5 Development Center
 
 
FAQ
Latest Articles
Java
.NET
XML
Database
Enterprise
Questions? Contact us.
C++
Web Development
Wireless
Latest Tips
Open Source


   Development Centers

   -- Android Development Center
   -- Cloud Development Project Center
   -- HTML5 Development Center
   -- Windows Mobile Development Center