2

A project that I am working on has the following code for interface example:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace Test
{
    public interface IDeviceEssentials
    {
        string Model { get; set; }
        string Manufacturer { get; set; }
        string BIOSVersion { get; set; }
        string TotalPhysicalMemory { get; set; }
        string TotalVirtualmemory { get; set; }
        string OSName { get; set; }
    }
}

every other class implementing this interface uses Automatic properties which renders the getters and setters useless so the effective implementation is reduced to

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace Test
{
    public interface IDeviceEssentials
    {
        string Model;
        string Manufacturer;
        string BIOSVersion;
        string TotalPhysicalMemory;
        string TotalVirtualmemory;
        string OSName;
    }
}

For which simply a structure or class is enough to hold the data..

The main usage of interface is polymorphism making use of liskov substitution, even though the above code is using the getter setter it is effectively reduced to variable decleration by all the classes implementing it (by using automatic property). That is there is no need i want to mock the above interface. even if it is designed as a class I can simply mock by creating new class of it.

Question are :-

  1. I think the above properties are providing data representation only. is it ok to use interface only for data representation?
  • 1
    Why would it not be fine? This is exactly what interfaces are for. Can you clarify your question, or why you feel it is not good to do this? – rmayer06 Nov 27 '14 at 23:47
  • @rmayer06 I have updated the question and explained the issue more specifically – k4vin Nov 28 '14 at 0:15
  • In C#, an interface is the only way to effect multiple inheritance. So, that is at least one legitimate reason to do this. – rmayer06 Nov 28 '14 at 1:17
  • The presence of the setters is a bit weird. This looks like the kind of data which most consumer would only read not modify. – CodesInChaos Nov 28 '14 at 15:18
  • Types in general that only contain public getters and setters are a smell. Sure, it's fine in simple DTOs and request models, but if your domain model looks like this, worry about that instead. – sara Apr 27 '16 at 17:16
7

No it isn't a code smell in itself. Having many implementations using only auto properties is a code smell - this is where you refactor common code into a base class and define it as virtual so that the extending class can override it where necessary.

every other class implementing this interface uses Automatic properties which renders the getters and setters useless

No, automatic properties do not render the interface definition useless. Your two illustrated interfaces are not the same - in fact the second one is illegal as you cannot define a field in an interface. Automatic properties still use getters and setters (they're inserted by the compiler), so using automatic properties satisfies the requirements of the interface. Simply declaring the field public string Model; in your implementation does not satisfy the interface.

You'll see from the following image that even though I used automatic properties in the implementation there are still getters and setters implemented for me:

enter image description here

If that still makes no sense, think of it this way: the interface defines a contract. If I later refactor my implementation so that I don't use auto properties then the interface guarantees that I expose both a getter and setter. Consuming code uses my implementation via its interface, not directly accessing the concrete implementation. This means I can refactor the implementation and the caller doesn't have to change at all.

2

This interface is perfectly fine. I often see this kind of interface used on an object that's responsible for data storage, to hide the data storage code from the consumer.

As long as you follow SOLID principles in your class design, it really doesn't matter what your interfaces define.

  • Is the class behind this interface only responsible for one thing?
  • Is it open for extension and closed for modification?
  • Does it follow the Liskov Substitution principle?
  • Does it follow the interface segregation principle?
  • Does it follow the dependency inversion principle?

If you can answer yes to each of these questions then your class design should be ok. The interface then becomes almost a by-product of this thought process.

  • what if every class that is implementing this interface is just using automatic properties. that is, I have 5 classes implementing only this interface and looks same.. differs just by name everything else was same.. what about code duplication.. – k4vin Nov 27 '14 at 23:56
  • 1
    @k4vin: then you create an abstract class which implements the properties, and then five classes which inherit from the common abstract class. – Arseni Mourzenko Nov 27 '14 at 23:57
1

You seem to worry about code duplication. One way around that is an abstract class as mentioned in other answers. But inheritance can have it's own problems, which is why another principle exists: use composition over inheritance.

Since IDeviceEssentials is basically a struct, you could make it a concrete class with only the automatic properties and no logic. Then have this class as a member of your other classes representing the concrete devices. If necessary, introduce an interface IDeviceEssentialsProvider.

Example (I'm not fluent in C# so you might need to tweak it):

public class DeviceEssentials
{
    string Model { get; set; }
    string Manufacturer { get; set; }
    string BIOSVersion { get; set; }
    string TotalPhysicalMemory { get; set; }
    string TotalVirtualmemory { get; set; }
    string OSName { get; set; }
}

public interface IDeviceEssentialsProvider
{
    DeviceEssentials DeviceEssentials { get; }
}

public class DeviceA implements IDeviceEssentialsProvider
{
    DeviceEssentials DeviceEssentials { get; }
}

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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