I would like to get some feedback on a programming task I use to teach object-oriented design in an intro to programming course with C#. I happen to like this task a lot but since I designed it myself I am not sure I see its flaws. In addition I sometimes get to interview developers for junior positions and I am planning to use this task as an interview question and therefore it is essential that I get more people to look at it.
The task is for C# but it is practically the same in Java and will probably fit quite well in other class-based object-oriented languages. Here it is:
You have several things you need to model
Building
- Area
- Height
- Color
House
- Rooms
- Area (sum of area of all rooms)
- Height
- Color
- Owner
Person
- Name
Room
- Area
- Color
Bathroom
- Area
- Color
Kitchen
- Area
- Color
Bedroom
- Area
- Color
You are required to choose the appropriate types (using string for color is fine) and design classes in such a way that one can instantiate an array of buildings and print their areas, heights and colors. You should use good OOP practices like encapsulation and define proper relationships between the types.
Go ahead do it, I will wait. If you are brave post your solution in the comments.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
Here is my solution
class Building
{
private double area;
public virtual double Area
{
get { return area; }
}
public double Height { get; private set; }
public string Color { get; private set; }
protected Building(double height, string color)
{
Height = height;
Color = color;
}
public Building(double area, double height, string color)
{
this.area = area;
Height = height;
Color = color;
}
}
class House : Building
{
public Person Owner { get; private set; }
private Room[] rooms;
public House(Room[] rooms, double height, string color, Person owner)
: base(height, color)
{
this.rooms = rooms;
Owner = owner;
}
public override double Area
{
get
{
double area = 0;
for (int i = 0; i < rooms.Length; i++)
{
area += rooms[i].Area;
}
return area;
}
}
}
class Person
{
public string Name { get; private set; }
public Person(string name)
{
Name = name;
}
}
class Room
{
public double Area { get; private set; }
public string Color { get; private set; }
public Room(double area, string color)
{
Area = area;
Color = color;
}
}
class Bathroom : Room
{
public Bathroom(double area, string color)
: base(area, color)
{
}
}
class Bedroom : Room
{
public Bedroom(double area, string color)
: base(area, color)
{
}
}
class Kitchen : Room
{
public Kitchen(double area, string color)
: base(area, color)
{
}
}
Client code
Kitchen kitchen = new Kitchen(10, "red");
Bedroom bedroom = new Bedroom(20, "red");
Bathroom bathroom = new Bathroom(5, "red");
Room[] rooms = new Room[] { kitchen, bedroom, bathroom};
Person pesho = new Person("Pesho");
House house = new House(rooms, 5, "black", pesho);
Building skyscraper = new Building(100, 500, "blue");
Building[] buildings = new Building[] { house, skyscraper };
foreach (Building building in buildings)
{
Console.WriteLine("Building area: {0}, building color: {1}, building height: {2}", building.Area, building.Color, building.Height);
}
I give this task after explaining inheritance and overriding, before talking about interfaces and before generics and collections. This is why Rooms is an array and not an IEnumerable. Of course in an interview there is high chance that the candidate may use more advanced collections but an array is also fine in my opinion if it is not public. The lesson stresses out that inheritance is an "is a" relationship while composition is a "has a" relationship. I think this task is good for teaching for the following reasons:
- It demonstrates the "is a" vs. "has a" thing quite clearly. There is no debate if a house is a building unlike in many real world cases where the distinction between "is a" and "has a" is not clear at all.
- A common mistake is to use inheritance just because some properties of one object are present in another object. Many people fall in the trap of trying to make Room a base class for Building just because of these properties.
- There is a need to override the Area property. Also in House Area is read only.
- It demonstrates the value of immutability and encapsulation. I try to teach the students not to write public setters or even expose properties before they need them. People who make the Area setter public in Building or insist on adding setters to all properties have a hard time thinking of a meaningful setter for Area in the House class.
- We can demonstrate encapsulation by not making Rooms public. After all it is not needed to complete the actual assignment.
- We can demonstrate how we call base constructors and pass default values for arguments. We can even make one constructor protected to avoid polluting the public interface.
So why do I think this is a good interview question?
- It is a question that involves writing code and is not very time consuming.
- The traps are still there. For example I would consider making Room a base class of Building a failure.
- If the candidate makes Rooms public I can ask if this violates encapsulation and how it can be fixed (I would expect to see the property either made private or made an IEnumerable at this point).
- I can ask about validating input and throwing appropriate exceptions which is not included in the code above since in the course we have not introduced exceptions at this point.
- If the candidate has a setter that is not needed for Area I can ask why this is bad and how it can be fixed.
- I can ask what we can do about the area parameter in the House constructor?
Additional questions give the opportunity to test if the candidate understands a greater range of concepts even if he did not include it into the initial solution. Missing argument validation? That is fine but do you know how to add it if asked to? Bad public interface for your class (Area setter that does nothing)? That is OK but what if I ask you to fix it?
I would really like to hear your feedback on this one. Are there any glaring holes? Does my solution suck? What do you think?