views:

247

answers:

6

Please ignore language syntax, I want to discuss only OOPS here.


I will present here 2 code snippets, each of which is a sample of Composition (if I'm not wrong).

Problem Statement: I have an object which keeps the stats of an entity in the system. Let those stats be:

  1. Name
  2. LastName
  3. Salary

*these fields can be anything, I just took these for example. so please dont think in terms of what field is necessary or not. Assume all three are necessary.

I created a class which corresponds to these fields:

public class Stats{
field Name;
field LatsName;
field Salary;
}

Now I came across a situation where i want to have information about a person across time period. Lets see at a point in the workflow system requires that it be presented with information about a person for all three stages

  1. When he was child.
  2. When he was young.
  3. when he got retired.

Point to note here is that name and lastName won't change, only salary might change. Because of this, I thought that I can perhaps create a class that can use existing object 'stats'.

I'm goin to present two solutions , Please suggest which one is better and why.

Code Sample 1

public class CompositeStats{
  //Adding three more properties and hinding the one already existing.
  private objStats= new Stats();
  public field FirstName{
    get{return objStats.Name;}
    set{objStats.Name=value;}
  }
  public field LastName{
    get{return objStats.LastName;}
    set{objStats.LastName=value;}
  }

  public field SalaryChild{get;set;}
  public field SalaryYoung{get;set;}
  public field SalaryRetired{get;set;}
}

In the above sample code, I did not expose the original field of salary but have created 3 new for each time span.

Code Sample 2

public class CompositeStats{
  private objStatsChild= new Stats();
  private objStatsYoung= new Stats();
  private objStatsRetired= new Stats();

  public field FirstName{
    get{return objStatsChild.Name;}
    set{objStatsChild.Name=value;}
  }

  public field LastName{
    get{return objStatsChild.LastName;}
    set{objStatsChild.LastName=value;}
  }

  public field SalaryChild{
    get{return objStatsChild.Salary;}
    set{objStatsChild.Salary=value;}
  }

  public field SalaryYoung{
    get{return objStatsYoung.LastName;}
    set{objStatsYoung.LastName=value;}
  }

  public field SalaryRetired{
    get{return objStatsRetired.LastName;}
    set{objStatsRetired.LastName=value;}
  }

}
+1  A: 

Let us consider one more usage:

var dictStages = new Dictionary<Stage,double>();

Here, Stage is an enum: Child, Adult, Retired

In your class add the properties

CurrentStage - this will relect the latest stage from the dictStage
CurrentSalary - this will reflect the salary for CurrentStage.

Although, you might want to expose some methods that can return Salary for a particular stage.

+1  A: 

I think its obvious that first one is a better choice.

Reason: 1. Performance:

 private objStatsChild= new Stats();
private objStatsYoung= new Stats(); 
private objStatsRetired= new Stats();

Why would you create three instances, when you actually need value from one of the instaces only.

  1. Bad Design. You are creating three instances of "Stats",and while returning the name you are picking one of them (at random, atleast there is not logic to select one of them).

3.If all you want to do is to hide "Salary" in stats, then it is more of a case of inheritance, with "Salary" removed from parent class.

Amby
The reason "salary" is in base class because....some wheere deep deep in the system this "stats" object is used independently which will make use of all the three fields.
SilverHorse
+1  A: 

I think something like this would be better all around. It allows you to create new life stages without changing your object model.

Also, if a person has not reached a stage yet, there will be no information stored for it. A property called "CurrentStage" could pick the latest stage in the list.

public class Stage{
  object BeginStage;  // Probably a DateTime or similar class.
  object EndStage;    // Probably a DateTime or similar class.
  object Salary;
}

public class Stats{
  object Name;
  object LastName;
  IEnumerable<Stage> Stages;
}

You could also use this variation.

public enum Stage {
  Child,
  Young,
  Retired,
}

public class StageSalary {
  Stage Stage;
  object Salary;
}

public class Stats{
  object Name;
  object LastName;
  List<StageInfo> Stages;
}
John Fisher
and how to represnt a snap shot of a single instance......when some one says hey give me "what you got for young age" then how you wouuld represnt that info. My 'Stat' class i intented for that use...and very general one.
SilverHorse
SilverHorse, Did my recent edit answer your question?
John Fisher
+1  A: 

Check out a design pattern called "Facade", it allows you to have a single class structure that inherits from multiple interfaces, and allowing data to only represent the structure that you categorized them as.. i.e. hide / ignore fields for "child" that are only used for "retired" etc.

Jan de Jager
A: 

I would refactor your code to something like this:

class Person {
   field Name;
   field LastName;
}

enum PersonAge {
   Child, Young, Retired
}

class Stage {
   public PersonAge Age { get; set; }
   field Salary;
}

class PersonStats {
   public Person Person { get; set; }
   public IList<Stage> Stages { get; set; }
}

Does it make sense?

bruno conde
Good but not relevant to my question. I presented two sample and that too around "composition".
SilverHorse
Reason for downvote please ...
bruno conde
I would appreciate if you will help me with composition. and downvote because it didnt helped."answer was not helpful" you want me to revert it ?
SilverHorse
A: 

The 'classic' "IsA", "HasA" thinking will always help steer you in the right direction:

A "Person" "HasA" "Collection of Data", which your information from his life stages. Today you say "three" stages but of course you would never design to such a limited view of the problem ... right?

so you have
class Person {
[vector or list or your favorite collection type] Data;
Name;
LastName;
};

The snapshot you ask about "HasA" reference(s) to the Person, and a specifier for which piece of data you want in the snapshot:

class PersonInfoSnapshot {
[reference to] Person;
/* "reference to Person.Data" could mean anything that allows retrieval of the data that you need */ [reference to] Person.Data;
};

Details of specific types, visibility and so forth left as an exercise.