2
\$\begingroup\$

I had a job interview take home test where I was asked a series of questions on object inheritance. In addition to answering the questions, I created the following application to demonstrate inheritance among different objects.

The first piece of code are different shape classes. Some inherit from an IShape interface.

The second piece of code is a console application that uses these classes in different inheritance scenarios. The console.write tells the use case.

I would like feedback if my implementation looks correct.

Class definitions

using System;
using OOP.Interfaces;

namespace OOP.Models
{
    public class Circle : IShape
    {
        public void Draw()
        {
            Console.WriteLine("Circle object's draw method\n");
        }
    }

    public class Rectangle : IShape
    {
        public void Draw()
        {
            Console.WriteLine("Rectangle object draw method\n");
        }
    }

    public class Square : Rectangle
    {
        public void Draw()
        {
            Console.WriteLine("Square object draw method\n");
        }
    }

}

Console App

using System;
using OOP.Interfaces;
using OOP.Models;

namespace OOP_Calling_Client
{
  public class Program
  {
    static void Main(string[] args)
    {
        Console.WriteLine("This is a demo console app that shows examples of OOP inheritance through a series of shape classes and interfaces.");
        Console.WriteLine("*********************************\n");

        DrawCircle();
        DrawRectangle();
        DrawSquare();
        AssignDifferentTypesToSameInstanceVariable();
        Console.Read();
    }

    private static void DrawCircle()
    {
        IShape circle = new Circle();
        circle.Draw();
        Console.WriteLine("Object of type circle stored in a variable of type IShape\n");
        Console.WriteLine("*********************************\n");
    }

    private static void DrawRectangle()
    {
        Console.WriteLine("Object of type rectangle stored in a variable of type rectangle\n");
        Rectangle rectangle = new Rectangle();
        rectangle.Draw();
        Console.WriteLine("*********************************\n");
    }

    private static void DrawSquare()
    {
        Console.WriteLine("Object of type square stored in a variable of type rectangle. Rectangle is derived from square\n");
        Rectangle rectangle = new Square();
        rectangle.Draw();
        Console.WriteLine("*********************************\n");
    }

    private static void AssignDifferentTypesToSameInstanceVariable()
    {
        Console.WriteLine("Instance variable holds a type of circle, and then a type of rectangle. The draw methods change\n");
        IShape shape = new Circle();
        Console.WriteLine("The object variable is of type IShape,\nand is holding an instance of a Circle.\n");
        shape.Draw();
        Console.WriteLine("The object variable is of type IShape,\nbut is now holding an instance of a Rectangle.\n");
        shape = new Rectangle();
        shape.Draw();
    }
  }
}
\$\endgroup\$
0

3 Answers 3

8
\$\begingroup\$

Shadowing

There is some shadowing going on between Rectangle and Square.

Rectangle implements IShape, and Square inherits from Rectangle.

As it stands, Square is hiding Rectangles Draw method. This is evident from the following compiler warning:

Warning 1 'ConsoleApplication1.Square.Draw()' hides inherited member 'ConsoleApplication1.Rectangle.Draw()'. Use the new keyword if hiding was intended.

This can pose a problem in the following scenario:

  Square s = new Square();
  s.Draw(); //Square method called

  Rectangle r = new Square();
  r.Draw(); //Rectangle method called!

As you see, this breaks inheritance and will only call the method on Square if the variable is a Square.

Instead, you might want to make the method virtual on Rectangle:

public class Rectangle : IShape
{
    public virtual void Draw()
    {
        Console.WriteLine("Rectangle object draw method\n");
    }
}

public class Square : Rectangle
{
    public override void Draw()
    {
        Console.WriteLine("Square object draw method\n");
    }
}

This will ensure that the correct method is always called.

Line breaks

There is no need to add \n when calling Console.WriteLine. It will add a line break automatically.

Moreover, \n is not the correct method of adding a line break in .NET. Use Environment.NewLine instead.

Misc.

The order of the final print in your example is wrong. It prints before actually assigning shape = new Rectangle().

\$\endgroup\$
1
  • \$\begingroup\$ Thanks Rotem. Your response was very helpful. I see my error with shadowing now that you pointed it out. \$\endgroup\$ Oct 13, 2015 at 21:09
3
\$\begingroup\$

no, this isn't exactly correct. Your demonstration exhibits whether a type implements IShape.

The part that seems correct is that you can get the IShape interface from Rectangle type (IShape shape = new Rectangle()), because Shape implements IShape and Rectange inherits Shape.

However, Rectangle does not implement IShape, its base does.. so IShape shape = new Rectangle(); shape.Draw(); will be equivalent to ((Shape)shape).Draw() not ((Rectangle)shape).Draw().

Additionally, you're overloading Shape.Draw by defining Rectangle.Draw, so your sample doesn't depict any inherited methods that satisify to usage..

To feature inheritance, you demonstrate the implicit and explicit castings as follows:

List<Shape> shapes = new List<Shape>(new Shape[] { new Shape(), new Rectangle()});

Then iterate over the List as so:

shapes.ForEach(shape => shape.Draw()); //uses Shape.Draw (base) of Rectangle

For the explicit casting example...

List<Rectangle> rectInstances = shapes.OfType(Rectangle).Cast<Rectangle>().ToList();
rectInstances.ForEach( rect => {
    rect.Draw(); //uses Rectangle.Draw()
});

Additionally,

IShape.Draw is improper.. Shapes don't draw. they are drawable; to be drawable indicates a context in which they are to be drawn on.

Interfaces can inherit other interfaces.. you should consider demonstrating that concept as well.

Class & Interface Definitions

public interface IShape { } // at minimum: IShape is a decorator; it may be composed of 3 or more points

public interface IRectangle : IShape { } //IRectangle is a shape; with 4 points, 4 even sides.

public interface IDrawable { void Draw(IDrawableContext context); void Draw(); }

public interface IDrawableShape : IShape, IDrawable { }

public interface IDrawableContext
{
    System.IO.TextWriter Debugger { get; set; } //assign Console.Out to this..
}

public class DrawableContext : IDrawableContext
{
    public System.IO.TextWriter Debugger { get; set; }
}

public class Shape : IDrawableShape
{
    protected internal IDrawableContext drawContext { get; set; }

    public Shape(IDrawableContext context)
    {
        this.drawContext = context;
    }

    public void Draw(IDrawableContext context)
    {
        context.Debugger.WriteLine(value: "Drawing Shape");
    }

    public void Draw()
    {
        this.drawContext.Debugger.WriteLine(value:  "Drawing Shape");
    }
}

public class Rectangle : Shape, IRectangle
{
    public Rectangle(IDrawableContext context) : base(context) { }

}

public class RectangleExplicit : Rectangle, IDrawableShape
{
    public RectangleExplicit(IDrawableContext context) : base(context) {}

    public new void Draw(IDrawableContext context)
    {
        context.Debugger.WriteLine(value: "Draw Rectangle (Explicit)");
    }

    public new void Draw()
    {
        base.drawContext.Debugger.WriteLine(value: "Draw Rectangle (Explicit)");
    }
}

public class RectangleImplicit : Rectangle
{
    public RectangleImplicit(IDrawableContext context) : base(context) { }

    public new void Draw(IDrawableContext context)
    {
        context.Debugger.WriteLine(value: "Draw Rectangle (Implicit)");
    }

    public new void Draw()
    {
        base.drawContext.Debugger.WriteLine(value: "Draw Rectangle (Implicit)");
    }
}

Program

class Program
{
    static void Main(string[] args)
    {
        IDrawableContext ContextOne = new DrawableContext() {
            Debugger = Console.Out
        };

        var shapes = new Shape[] {
            new Shape(ContextOne),
            new RectangleExplicit(ContextOne),
            new RectangleImplicit(ContextOne)
        };

        var shapelist = new List<Shape>(shapes);

        Console.WriteLine(value: "...using `List<Shape>`:");

        shapelist.ForEach(shape => {
            // don't use 'shape.drawContext.Debugger.Write(value:  "Test");'
            shape.Draw();
            shape.Draw(ContextOne); // RectangleImplicit and RectangleExplicity were implicitly casted to Shape in the construction of the array, so this iteration will be equivalent to ((Shape)shape).Draw(), 
        });


        Console.WriteLine(value: "...using `.OfType<IRectangle>().Cast<Rectangle>().ToList<Rectangle>`:");

        shapelist.OfType<IRectangle>().Cast<Rectangle>().ToList<Rectangle>().ForEach(shape => {
            // don't use 'shape.drawContext.Debugger.Write(value:  "Test");'
            shape.Draw();
            shape.Draw(ContextOne); // Rectangle does not define .Draw(), so here we are using the method of it's base (Shape); Shape.Draw()
        });

        Console.WriteLine(value: "...using `.OfType<RectangleExplicit>().Cast<RectangleExplicit>().ToList<RectangleExplicit>`:");

        shapelist.OfType<RectangleExplicit>().Cast<RectangleExplicit>().ToList<RectangleExplicit>().ForEach(shape => {
            // don't use 'shape.drawContext.Debugger.Write(value:  "Test");'
            shape.Draw();
            shape.Draw(ContextOne); // RectangleExplicit.Draw
        });

        Console.WriteLine(value: "...using `.OfType<RectangleImplicit>().Cast<RectangleImplicit>().ToList<RectangleImplicit>`:");

        shapelist.OfType<RectangleImplicit>().Cast<RectangleImplicit>().ToList<RectangleImplicit>().ForEach(shape => {
            // don't use 'shape.drawContext.Debugger.Write(value:  "Test");'
            shape.Draw();
            shape.Draw(ContextOne); // RectangleImplicit.Draw
        });

        Console.WriteLine(value: "...using `List<IDrawableShape>`:");

        // now we're looking for the first type (in the inheritance chain) that implements IDrawableShape;


        var ishapes = new IDrawableShape[] {
            new Shape(ContextOne),
            new Rectangle(ContextOne),
            new RectangleExplicit(ContextOne),
            new RectangleImplicit(ContextOne)
        };

        var ishapelist = new List<IDrawableShape>(ishapes);

        ishapelist.ForEach(shape => {               
            shape.Draw();
            shape.Draw(ContextOne); 
        });

        ConsoleKey key;
        Console.WriteLine(value: "Press [Enter] to proceed.");
        do
        {
            key = Console.ReadKey().Key;                
        } while (key != ConsoleKey.Enter);
    }
}

Output

...using `List<Shape>`:
Drawing Shape
Drawing Shape
Drawing Shape
Drawing Shape
Drawing Shape
Drawing Shape
...using `.OfType<IRectangle>().Cast<Rectangle>().ToList<Rectangle>`:
Drawing Shape
Drawing Shape
Drawing Shape
Drawing Shape
...using `.OfType<RectangleExplicit>().Cast<RectangleExplicit>().ToList<RectangleExplicit>`:
Draw Rectangle (Explicit)
Draw Rectangle (Explicit)
...using `.OfType<RectangleImplicit>().Cast<RectangleImplicit>().ToList<RectangleImplicit>`:
Draw Rectangle (Implicit)
Draw Rectangle (Implicit)
...using `List<IDrawableShape>`:
Drawing Shape
Drawing Shape
Drawing Shape
Drawing Shape
Draw Rectangle (Explicit)
Draw Rectangle (Explicit)
Drawing Shape
Drawing Shape
Press [Enter] to proceed.

A shorter demonstration... define an instance of the interface of the type that inherits all types, and explicitly cast ...

    ...
      IDrawable drawableInstance = new RectangleExplicit(ContextOne);

      drawableInstance.Draw();
      ((Shape)drawableInstance).Draw();
      ((Rectangle)drawableInstance).Draw();
      ((RectangleExplicit)drawableInstance).Draw();

      Console.WriteLine(value: "Press [Enter] to proceed.");
      do
      {
          key = Console.ReadKey().Key;                
      } while (key != ConsoleKey.Enter);
    ...

Output

Draw Rectangle (Explicit)
Drawing Shape
Drawing Shape
Draw Rectangle (Explicit)
Press [Enter] to proceed.
\$\endgroup\$
1
  • \$\begingroup\$ as a side consideration... this sample uses two draw methods as to encorporate (and hopefully distinquish) two methodologies... \$\endgroup\$ Oct 13, 2015 at 17:18
1
\$\begingroup\$

Your code doesn't benefit from the use of inheritance. You can remove all references to IShape without losing anything. If you had an illustrator class that accepted IShape objects, then you'd have something but it feels forced.

I tend to avoid inheritance. They say we should prefer composition over inheritance. One thing I look for in my code is switch statements and enums. I have found that if I replace them with polymorphism, my code becomes cleaner. I can add the equivalent of switch-cases without changing existing code.

Example using switch statements:

enum FileType { xlsx, xml, csv }

public void GenerateFile(DataType data, FileInfo dest, FileType type)
{
    switch(type)
    {
        case FileType.xlsx:
            GenerateXlsx(data, dest);
            break;
        case FileType.xml:
            GenerateXml(data, dest);
            break;
        case FileType.csv:
            GenerateCsv(data, dest);
            break;
        default:
            throw new NotImplementedException();
    }
}

In this case, your code supports three FileTypes and your switch statement controls which method to call based on your FileType parameter. If you add a FileType "dat" to your enum, you have to add a case to each of your switch statements. This is error-prone if you have many switch cases as adding the "dat" value to the enum could be a breaking change. If you don't have access to source code that uses these enums your problem is exasperated.


Example using inheritance:

public abstract class FileGenerator
{
    void GenerateFile(DataType data, FileInfo dest)
    {
        byte[] fileContents = this.OnGenerateFile(data);
        File.Write(dest.FullName, fileContents);
    }

    protected abstract byte[] OnGenerateFile(DataType data);
}

public class FileGeneratorXlsx : FileGenerator
{
    protected override byte[] OnGenerateFile(DataType data)
    {
        //code to generate bytes
    }
}

The benefits to inheritance here are that when you deal with a FileGenerator type, you don't have to use a switch statement containing a case for every type of FileGenerator. You might use a Factory like this:

public void GenerateDataFile(IFileGeneraterFactory fileGeneratorFactory, DataType data)
{
    FileGenerator fileGenerator = fileGeneratorFactory.Create();
    fileGenerator.GenerateFile(data);
}

Notice that you have no conditionals here. You don't care how the FileGenerator type is implemented, only that it has a GenerateFile(DataType) method. Your factory may even return an object that didn't exist when the abstract class FileGenerator was first implemented. An object in a different assembly and namespace. All you care is that this object satisfies the contract stating that it has a GenerateFile(DataType) method.

Note that I wrote this example in a way that you couldn't easily switch from using an abstract class to an interface. I've been asked the difference in job interviews before.

\$\endgroup\$
1
  • \$\begingroup\$ Oddly enough, I look for enums and switches in my code and replace them with polymorphism... \$\endgroup\$
    – RubberDuck
    Oct 13, 2015 at 23:50

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.