17

This might be a simple/basic OOP question, but I still cannot figure out how to solve it. I had the following problem during an interview : make an UML class diagram and write the basic code for a "smart"phone which contains the functionalities of a telephone and of an mp3 player. We've got with the following (accepted) solution :

class Telephone 
{
    public string name { get; set; }

    public Telephone()
    {
        name = "name telephone";
    }
}

class MP3 
{
    public string name { get; set; }

    public MP3()
    {
        name = "name mp3";
    }
}

And the "smart"phone class :

class TelephoneMP3 
{
    public Telephone tel;
    public MP3 mp3;

    public TelephoneMP3()
    {
        tel = new Telephone();
        mp3 = new MP3();
    }
}

As you can see, we have a composition relation between the TelephoneMP3 and the Telephone/MP3 classes.

But, using this code, the TelephoneMP3 is not a Telephone and the TelephoneMP3 is not an MP3 either, which is not logical. So, what changes should I make in order to make this valid ? For example, this kind of test :

if (telMp3 is Telephone)
{
    Console.WriteLine("TelephoneMP3 is telephone");
}           
if (telMp3 is MP3)
{
    Console.WriteLine("TelephoneMP3 is mp3");
}

The modifications could be made using the following remarks :

  1. Telephone / MP3 / TelephoneMP3 must remain classes (all 3 of them)
  2. I could add interfaces / other classes if necessary
  3. TelephoneMP3 must not duplicate all the functionalities from a Telephone / MP3 (for example during an inheritance from an interface where the TelephoneMP3 will be obliged to write the code from all the interface's members)

Thank you in advance

Tzah Mama
  • 1,547
  • 1
  • 13
  • 25
IonR
  • 173
  • 1
  • 5
  • 2
    What exactly is it you want to achieve? Do you want `TelephoneMP3` to return true on `is MP3` and `is Telephone`? – Yuval Itzchakov Jun 30 '14 at 08:00
  • yes, this is the goal – IonR Jun 30 '14 at 08:03
  • Guess At the end,You want TelephoneMp3 to be either Mp3 or an Telephone,but not both? – Rangesh Jun 30 '14 at 08:04
  • I don't think this is possible I'm afraid. `TelephoneMP3` can't be an `MP3` _and_ a `Telephone` at the same time. It is possible using interfaces or with design patterns which will add functionality to either the `MP3` or the `Telephone`. But having a class that is both? – Tzah Mama Jun 30 '14 at 08:06

11 Answers11

35

Since C# doesn't support multiple inheritance, consider using interfaces instead:

public interface Phone{ ... }
public interface Mp3{ ... }

public class Telephone : Phone{ ... }
public class Mp3Player : Mp3{ ... }
public class Smartphone : Phone, Mp3{ ... }

This way Smartphone is both Phone and Mp3. If you are in need to write a method which operates on a Telephone, use the Phone interface instead. This way you'll be able to pass either Telephone or Smartphone as an argument.

Bartek Maraszek
  • 1,404
  • 2
  • 14
  • 31
  • 19
    note that a common naming standard in C# for interfaces is using the prefix `I`. – default Jun 30 '14 at 08:27
  • The OP asks: `TelephoneMP3 must not duplicate all the functionalities from a Telephone / MP3` (I assume he/she means duplicate the code). C# only allows inheritance from one base class so Smartphone must contain an internal instance of either Mp3 player or a telephone (or even both). – Ben Jun 30 '14 at 08:36
  • 27
    @Default, but there are legal ramifications using an `IPhone`. ;) – crthompson Jun 30 '14 at 21:13
18

There are some good answers here. The answers which say to use interfaces are good, and that's what the interviewer is likely looking for. However, I would consider simply denying the premise that the "is-a-kind-of" relationship being satisified is a good idea. Rather, I would consider using a service provider organization:

public interface ITelephone { ... }
internal class MyTelephone : ITelephone { ... }
public interface IMusicPlayer { ... }
internal class MyPlayer : IMusicPlayer { ... }
public interface IServiceProvider
{
  T QueryService<T>() where T : class;
}

internal class MyDevice : IServiceProvider
{
  MyTelephone phone = new MyTelephone();
  MyPlayer player = new MyPlayer();
  public T QueryService<T>() where T : class
  {
      if (typeof(T) == typeof(ITelephone)) return (T)(object)phone;
      if (typeof(T) == typeof(IPlayer)) return (T)(object)player;
      return null;
  }
}

Now a caller has a MyDevice in hand via its IServiceProvider interface. You ask it

ITelephone phone = myDevice.QueryService<ITelephone>();

and if phone is non-null, then the device can act like a phone. But

myDevice is ITelephone

is false. The device is not a phone, it knows how to find you something that acts like a phone.

For more in this vein, study plug-in architectures such as MAF.

Luca Cremonesi
  • 144
  • 2
  • 3
  • 13
Eric Lippert
  • 647,829
  • 179
  • 1,238
  • 2,067
  • Why is queryservice generic? – adrianm Jun 30 '14 at 22:06
  • @adrianm: What would you prefer it to be? – Eric Lippert Jul 01 '14 at 03:34
  • It was just a subtle joke around the discussion if a generic method which do different things depending on the type argument should really be generic. – adrianm Jul 01 '14 at 07:18
  • 2
    @adrianm: I confess that this is in some sense an abuse of the generic mechanism; one prefers generic methods to be *generic*. That is, to be able to handle all types equally well. I have said many times in the past that if you are making a type test on a generic type parameter, you're probably doing something wrong. This would be a possible violation of that guideline. The alternative is to have the method take a `Type` and return `object`, which seems hardly better. – Eric Lippert Jul 01 '14 at 13:53
  • @EricLippert, can you share some links about MAF, I can't seem to find good documentation on it. – Eugene S. Jul 22 '14 at 03:51
17

It is almost similar to the other answers, but..
I think it has the best accuracy regarding the inheritance hierarchy.

internal class Program
{
    private static void Main(string[] args)
    {
        var telephone = new Telephone();
        Console.WriteLine(telephone.Name);
        telephone.OutboundCall("+1 234 567");
        Console.WriteLine("Am I a Telephone?                 {0}", telephone is Telephone);
        Console.WriteLine("Am I a MP3?                       {0}", telephone is MediaPlayer3);
        Console.WriteLine("Am I a Smartphone?                {0}", telephone is Smartphone);
        Console.WriteLine("Do I Have Telephone Capabilities? {0}", telephone is ITelephone);
        Console.WriteLine("Do I Have MP3 Capabilities?       {0}", telephone is IMediaPlayer3);
        Console.WriteLine();

        var mp3 = new MediaPlayer3();
        Console.WriteLine(mp3.Name);
        mp3.PlaySong("Lalala");
        Console.WriteLine("Am I a Telephone?                 {0}", mp3 is Telephone);
        Console.WriteLine("Am I a MP3?                       {0}", mp3 is MediaPlayer3);
        Console.WriteLine("Am I a Smartphone?                {0}", mp3 is Smartphone);
        Console.WriteLine("Do I Have Telephone Capabilities? {0}", mp3 is ITelephone);
        Console.WriteLine("Do I Have MP3 Capabilities?       {0}", mp3 is IMediaPlayer3);
        Console.WriteLine();

        var smartphone = new Smartphone();
        Console.WriteLine(smartphone.Name);
        smartphone.OutboundCall("+1 234 567");
        smartphone.PlaySong("Lalala");
        Console.WriteLine("Am I a Telephone?                 {0}", smartphone is Telephone);
        Console.WriteLine("Am I a MP3?                       {0}", smartphone is MediaPlayer3);
        Console.WriteLine("Am I a Smartphone?                {0}", smartphone is Smartphone);
        Console.WriteLine("Do I Have Telephone Capabilities? {0}", smartphone is ITelephone);
        Console.WriteLine("Do I Have MP3 Capabilities?       {0}", smartphone is IMediaPlayer3);

        Console.ReadKey();
    }

    public interface IDevice
    {
        string Name { get; }
    }

    public interface ITelephone : IDevice
    {
        void OutboundCall(string number);
    }

    public interface IMediaPlayer3 : IDevice
    {
        void PlaySong(string filename);
    }


    public class Telephone : ITelephone
    {
        public string Name { get { return "Telephone"; } }

        public void OutboundCall(string number)
        {
            Console.WriteLine("Calling {0}", number);
        }
    }

    public class MediaPlayer3 : IMediaPlayer3
    {
        public string Name { get { return "MP3"; } }

        public void PlaySong(string filename)
        {
            Console.WriteLine("Playing Song {0}", filename);
        }
    }

    public class Smartphone : ITelephone, IMediaPlayer3
    {
        private readonly Telephone telephone;
        private readonly MediaPlayer3 mp3;

        public Smartphone()
        {
            telephone = new Telephone();
            mp3 = new MediaPlayer3();
        }

        public string Name { get { return "Smartphone"; } }

        public void OutboundCall(string number)
        {
            telephone.OutboundCall(number);
        }

        public void PlaySong(string filename)
        {
            mp3.PlaySong(filename);
        }
    }

}

Program Output:

Telephone
Calling +1 234 567
Am I a Telephone?                 True
Am I a MP3?                       False
AM I a Smartphone?                False
Do I Have Telephone Capabilities? True
Do I Have MP3 Capabilities?       False

MP3
Playing Song Lalala
Am I a Telephone?                 False
Am I a MP3?                       True
AM I a Smartphone?                False
Do I Have Telephone Capabilities? False
Do I Have MP3 Capabilities?       True

Smartphone
Calling +1 234 567
Playing Song Lalala
Am I a Telephone?                 False
Am I a MP3?                       False
AM I a Smartphone?                True
Do I Have Telephone Capabilities? True
Do I Have MP3 Capabilities?       True
G.Y
  • 6,042
  • 2
  • 37
  • 54
6

I think this interview question is not (as should be all interview questions) about the challenge itself. The coding exercise of merging two classes via composition could be answered with a textbook. This challenge is a subtle trick question, and I propose that the point is to get you to discuss why. At least that's what I would want from my interviewees.


This test:

if(telMp3 is Telephone && telMp3 is MP3) {

...is the real problem. Why must you meet this criteria? This test completely voids the purpose of building objects out of composition. It demands that the objects are implemented in a specific way. It shows that the existing class implementations are already tightly coupled to the codebase (if they cannot be done away with). These requirements mean that SOLID principles were not followed, because you can't just fulfill the methods of a base type, you have to actually be the base type. That's no good.


As other answers have said, the solution would be to use interfaces. Then you can pass your object to any method that requires the interface. This kind of usage would require a test like so:

if (telMp3 is IPhone && telMp3 is IMp3) {

...but you can't do that, because of the limitation of your challenge. That means that out there in the rest of your code, people have been writing methods that explicitly depend on the specific types Telephone and MP3. That is the real problem.


In my opinion, the correct answer to this challenge is to say that the codebase fails the test. The specific fallout in your challenge is irrelvant; you need to change the requirements of the challenge before you can solve it properly. An interviewee would who recognize this fact would pass the test with flying colors.

Seth Battin
  • 2,811
  • 22
  • 36
5

You can use explicit interface implemenations as well to limit usage of the shared variable Name. That way you would have to cast to the interface to access it. You can still have public properties / methods from the interface.

Composition is still used, but the SmartPhone has control over the implementations of their properties / methods.

To me, this would be the easiest implementation to work with, because I rarely want to use both the implementation from the mp3player and the phone, but rather one of them. Also, I still have full control over what happens when the interfaces methods are called on the SmartPhone.

class User
{
    void UseSmartPhone(SmartPhone smartPhone)
    {
        // Cannot access private property 'Name' here
        Console.WriteLine(smartPhone.Name);

        // Cannot access explicit implementation of 'IMp3Player.Play'
        smartPhone.Play();

        // You can send the phone to the method that accepts an IMp3Player though
        PlaySong(smartPhone);

        // This works fine. You are sure to get the Phone name here.
        Console.WriteLine(((IPhone)smartPhone).Name);

        // This works fine, since the Call is public in SmartPhone.
        smartPhone.Call();
    }

    void CallSomeone(IPhone phone)
    {
        phone.Call();
    }

    void PlaySong(IMp3Player player)
    {
        player.Play();
    }
}

class SmartPhone : IPhone, IMp3Player
{
    private Phone mPhone;
    private Mp3Player mMp3Player;

    public SmartPhone()
    {
        mPhone = new Phone();
        mMp3Player = new Mp3Player();
    }

    public void Call()
    {
        mPhone.Call();
    }

    string IPhone.Name
    {
        get { return mPhone.Name; }
    }

    string IMp3Player.Name
    {
        get { return mMp3Player.Name; }
    }

    void IMp3Player.Play()
    {
        mMp3Player.Play();
    }
}

class Mp3Player
{
    public string Name { get; set; }

    public void Play()
    {
    }
}

class Phone
{
    public string Name { get; set; }

    public void Call()
    {
    }
}

interface IPhone
{
    string Name { get; }
    void Call();
}

interface IMp3Player
{
    string Name { get; }
    void Play();
}
default
  • 11,485
  • 9
  • 66
  • 102
2

How about this solution:

public interface ITelephone
{
    string Name{get;}
    void MakeCall();
}

public interface IMp3
{
    string Name { get; }
    void Play(string filename);
}

public abstract class BaseTelephone : ITelephone
{
    public virtual string Name { get { return "Telephone"; } }

    void MakeCall()
    {
        // code to make a call.
    }
}

public class MyMp3Player : IMp3
{
    public string Name { get { return "Mp3 Player"; } }

    public void Play(string filename)
    {
        // code to play an mp3 file.
    }
}

public class SmartPhone : BaseTelephone, IMp3
{
    public override string Name { get { return "SmartPhone"; } }

    private IMp3 Player { get { return _Player; } set { _Player = value; } }
    private IMp3 _Player = new MyMp3Player();

    public void Play(string filename) { Player.Play(filename); }
}

This way the smart phone can also be an Mp3 player, but internally it has an Mp3 player that it uses to play the music. The internal player can be swapped for a new one (eg. upgrade) by using the SmartPhone Player property.

The code for the phone is only written once, in the base phone class. The code for the Mp3 player is only written once also - in the MyMp3Player class.

Ben
  • 3,241
  • 4
  • 35
  • 49
2

Use the strategy pattern (used some shortcuts below, you'll get the gist).

public class Device {

  private List<App> apps;

  public Device() {

    this.apps = new List<App>();

    this.apps.Add(new Mp3Player());
    this.apps.Add(new Telephone());

  }

}

public class Mp3Player implements App {...}
public class Telephone implements App {...}
public interface App {...}

Disclaimer: my native tongue is PHP, forgive me any non C# coding standards etc etc.

Bart
  • 433
  • 3
  • 19
2

You could use implicit casting

class TelephoneMP3 
{
    public Telephone tel;
    public MP3 mp3;

    public TelephoneMP3()
    {
        tel = new Telephone();
        mp3 = new MP3();
    }

    public static implicit operator Telephone(TelephoneMP3 telemp3) {
        return telemp3.tel;
    }

    public static implicit operator MP3(TelephoneMP3 telemp3) {
        return telemp3.mp3;
    }

}

It won't pass the exact test that you proposed, but you can do

var teleMp3 = new TelephoneMP3();
Telephone t = teleMp3;
Jonny
  • 2,509
  • 23
  • 42
  • +1 Although the fact that the is operator doesn't work for this limits the usefulness of this solution. – Taemyr Jul 01 '14 at 11:54
1

You are trying to model a product hierarchy, in which a given product may have its own specific properties, as well as being composed of standard sub-products. This is indeed an example of the composition pattern. I suggest introducing a base interface for any product component, then creating specific interfaces for telephone, MP3 player and smartphone products.

In the traditional composition pattern each node may contain an arbitrary list of components to which subcomponents can be added or removed, however in your data model it appears more useful for each specific type of product to specify its precise children, then provide a generic method to iterate over them. This allows for specific (sub)components of a specified type/interface to be easily queryable throughout the product hierarchy.

I've also introduced an interface for a GPS product since all new phones contain built-in GPS receivers -- just to illustrate how to work with recursive hierarchies of components.

public interface IProductComponent
{
    string Name { get; set; }

    IEnumerable<IProductComponent> ChildComponents { get; }

    IEnumerable<IProductComponent> WalkAllComponents { get; }

    TProductComponent UniqueProductComponent<TProductComponent>() where TProductComponent : class, IProductComponent;
}

public interface ITelephone : IProductComponent
{
    IGps Gps { get; }
}

public interface IMp3Player : IProductComponent
{
}


public interface IGps : IProductComponent
{
    double AltitudeAccuracy { get; }
}

public interface ISmartPhone : IProductComponent
{
    ITelephone Telephone { get; }

    IMp3Player Mp3Player { get; }
}

These interfaces could then be implemented by a parallel set of classes:

public abstract class ProductComponentBase : IProductComponent
{
    string name;

    protected ProductComponentBase(string name)
    {
        this.name = name;
    }

    #region IProductComponent Members

    public string Name
    {
        get
        {
            return name;
        }
        set
        {
            name = value;
        }
    }

    public virtual IEnumerable<IProductComponent> ChildComponents
    {
        get
        {
            return Enumerable.Empty<IProductComponent>();
        }
    }

    public IEnumerable<IProductComponent> WalkAllComponents
    {
        get
        {
            yield return this;
            foreach (var child in ChildComponents)
            {
                foreach (var subChild in child.WalkAllComponents)
                    yield return subChild;
            }
        }
    }

    public TProductComponent UniqueProductComponent<TProductComponent>() where TProductComponent : class, IProductComponent
    {
        TProductComponent foundComponent = null;
        foreach (var child in WalkAllComponents.OfType<TProductComponent>())
        {
            if (foundComponent == null)
                foundComponent = child;
            else
                throw new Exception("Duplicate products found of type " + typeof(TProductComponent).Name);
        }
        return foundComponent;
    }

    #endregion
}

public class Telephone : ProductComponentBase, ITelephone
{
    IGps gps = new Gps();

    public Telephone()
        : base("telephone")
    {
    }

    #region ITelephone Members

    public IGps Gps
    {
        get
        {
            return gps;
        }
    }

    #endregion

    IEnumerable<IProductComponent> BaseChildComponents
    {
        get
        {
            return base.ChildComponents;
        }
    }

    public override IEnumerable<IProductComponent> ChildComponents
    {
        get
        {
            if (Gps != null)
                yield return Gps;
            foreach (var child in BaseChildComponents)
                yield return child;
        }
    }
}

public class Gps : ProductComponentBase, IGps
{
    public Gps()
        : base("gps")
    {
    }

    #region IGps Members

    public double AltitudeAccuracy
    {
        get { return 100.0; }
    }

    #endregion
}

public class TelephoneMP3 : ProductComponentBase, ISmartPhone
{
    ITelephone telephone;
    IMp3Player mp3Player;

    public TelephoneMP3()
        : base("TelephoneMP3")
    {
        this.telephone = new Telephone();
        this.mp3Player = new MP3();
    }

    IEnumerable<IProductComponent> BaseChildComponents
    {
        get
        {
            return base.ChildComponents;
        }
    }

    public override IEnumerable<IProductComponent> ChildComponents
    {
        get
        {
            if (Telephone != null)
                yield return Telephone;
            if (Mp3Player != null)
                yield return Mp3Player;
            foreach (var child in BaseChildComponents)
                yield return child;

        }
    }

    #region ISmartPhone Members

    public ITelephone Telephone
    {
        get { return telephone; }
    }

    public IMp3Player Mp3Player
    {
        get { return mp3Player; }
    }

    #endregion
}

public class MP3 : ProductComponentBase, IMp3Player
{
    public MP3()
        : base("mp3Player")
    {
    }
}

As new product component types are added (or subclassed), they override the "ChildComponents" of their parent and return their domain specific children.

Having done this, you can (recursively) query the product hierarchy for components of a given type for your use. For instance:

var accuracy = smartPhone.UniqueProductComponent<IGps>().AltitudeAccuracy

or

bool hasPhone = (component.UniqueProductComponent<ITelephone>() != null)

This combination of generalization and composition avoids duplicating code while making explicit the type of sub-components that should be found in any given product. It also avoids the burden of making all higher-level products proxy the interfaces of their standard children, passing all calls on to them.

dbc
  • 104,963
  • 20
  • 228
  • 340
1

Contrary to all the other answers I am quite confident the way this question is asked makes it impossible. The reason is the following :

You explicitly state

But, using this code, the TelephoneMP3 is not a Telephone and the TelephoneMP3 is not an MP3 either, which is not logical. So, what changes should I make in order to make this valid ?

Seeing the word "is" makes me immediately think of the "is" operator. I immediately assume that this is what you really want.

You then go on later to say the following:

Telephone / MP3 / TelephoneMP3 must remain classes (all 3 of them)

Well sure we can do the following:

interface ITelephone { }
class Telephone
{
    public string name { get; set; }

    public Telephone()
    {
        name = "name telephone";
    }
}
interface IMP3 { }

class MP3 : IMP3
{
    public string name { get; set; }

    public MP3()
    {
        name = "name mp3";
    }
}

class TelephoneMP3 : ITelephone, IMP3
{
    public Telephone tel;
    public MP3 mp3;

    public TelephoneMP3()
    {
        tel = new Telephone();
        mp3 = new MP3();
    }
}

But we still have one problem. The word "is". Since we must keep the classes TelephoneMP3, Telephone and MP3 and C# does not support multiply inheritance it simply is not possible.

To Illustrate my point:

public class Program
{
    static void Main(string[] args)
    {
        TelephoneMP3 t = new TelephoneMP3();
        Console.WriteLine((t is TelephoneMP3)? true:false);
        Console.WriteLine((t is ITelephone) ? true : false);
        Console.WriteLine((t is IMP3) ? true : false);
        Console.WriteLine((t is Telephone) ? true : false);
        Console.WriteLine((t is MP3) ? true : false);
        Console.ReadLine();
    }
}

This will give you

True

True

True

False

False

In other words TelephoneMP3 "is" an ITelephone. TelephoneMP3 "is" an IMP3; however, it is not possible for a TelephoneMP3 to be both a MP3 and a Telephone.

Community
  • 1
  • 1
Aelphaeis
  • 2,593
  • 3
  • 24
  • 42
-5

C# does not support multiple inheritance, you need to use interfaces and abstract class for common implementations , You can do the follwing :

Edit : I have add more details to my answer

abstract class BaseDevice 
{
    public string name { get; set; }

    public void Print()
    {
        Console.WriteLine("{0}", name );
    }
}

public interface IPhone
{
   void DoPhone();
}


public interface IMP3
{
    void DoMP3();
}

class Telephone :BaseDevice , IPhone
 {
     public Telephone()
     {
          name = "name telephone";
     }
 }

 class MP3 : BaseDevice , IMP3
 {
      public MP3()
      {
          name = "name mp3";
      }
 }

class telMp3 : BaseDevice , IMP3, IPhone
{
     private Telephone _tel;
     private MP3 _mp3; 

     public telMp3()
      {
          name = "name telMp3";
      }
     public  void DoPhone()
     {
         _tel.DoPhone();
     }
     public  void DoMP3()
     {
         _mp3.DoMP3();
     }


}
Shachaf.Gortler
  • 5,655
  • 14
  • 43
  • 71
  • 2
    Thank you, but I am not quite sure about this, because in this case we will have a Telephone is an MP3, and an MP3 is a Telephone, which shouldn't be true ... – IonR Jun 30 '14 at 08:00
  • 21
    How is this answer acceptable? Telephone and Mp3 have a common base class which has a name property, how is this proper inheritance? Mp3 and Telephone don't derive from the same parent. One is a `Phone` and one is a `Player`. Both IMp3 and IPhone are empty interfaces just for the sake of passing the `is` test. I would definitely not accept this answer in a job interview. – Yuval Itzchakov Jun 30 '14 at 08:13
  • 2
    _in telMp3 calls to IMP3 sould be routed to _mp3, and calls to IPhone should be routed to _tel_ How's that exactly? – Tzah Mama Jun 30 '14 at 08:19