Question

I understand how to work with interfaces and explicit interface implementation in C#, but I was wondering if it's considered bad form to hide away certain members that would not be used frequently. For example:

public interface IMyInterface
{
    int SomeValue { get; set; }
    int AnotherValue { get; set; }
    bool SomeFlag { get; set; }

    event EventHandler SomeValueChanged;
    event EventHandler AnotherValueChanged;
    event EventHandler SomeFlagChanged;
}

I know in advance that the events, while useful, would very rarely be used. Thus, I thought it would make sense to hide them away from an implementing class via explicit implementation to avoid IntelliSense clutter.

Was it helpful?

Solution

Yes, it's generally bad form.

Thus, I thought it would make sense to hide them away from an implementing class via explicit implementation to avoid IntelliSense clutter.

If your IntelliSense is too cluttered, your class is too big. If your class is too big, it's likely doing too many things and/or poorly designed.

Fix your problem, not your symptom.

OTHER TIPS

Use the feature for what it was intended for. Reducing namespace clutter is not one of those.

Legitimate reasons aren't about "hiding" or organization, they are to resolve ambiguity and to specialize. If you implement two interfaces that define "Foo", the semantics for Foo might differ. Really not a better way to resolve this except for explicit interfaces. The alternative is to disallow implementing overlapping interfaces, or declaring that interfaces cannot associate semantics (which is just a conceptual thing anyway). Both are poor alternatives. Sometimes practicality trumps elegance.

Some authors/experts consider it a kludge of a feature (the methods aren't really part of the type's object model). But like all features, somewhere, someone needs it.

Again, I would not use it for hiding or organization. There are ways to hide members from intellisense.

[System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)]

I might be a sign of messy code, but sometimes the real world is messy. One example from .NET framework is ReadOnlyColection which hides the mutating setter [] ,Add and Set.

I.E ReadOnlyCollection implements IList< T>. See also my question to SO several years ago when i pondered this.

It's good form to do so when the member is pointless in context. For example, if you create a readonly collection that implements IList<T> by delegating to an internal object _wrapped then you might have something like:

public T this[int index]
{
  get
  {
     return _wrapped[index];
  }
}
T IList<T>.this[int index]
{
  get
  {
    return this[index];
  }
  set
  {
    throw new NotSupportedException("Collection is read-only.");
  }
}
public int Count
{
  get { return _wrapped.Count; }
}
bool ICollection<T>.IsReadOnly
{
  get
  {
    return true;
  }
}

Here we've got four different cases.

public T this[int index] is defined by our class rather than the interface, and hence is of course not an explicit implementation, though note that it does happen to be similar to the read-write T this[int index] defined in the interface but is read-only.

T IList<T>.this[int index] is explicit because one part of it (the getter) is perfectly matched by the property above, and the other part will always throw an exception. While vital to someone accessing an instance of this class through the interface, it is pointless to someone using it through a variable of the class's type.

Similarly because bool ICollection<T>.IsReadOnly is always going to return true it is utterly pointless to code that is written against the class's type, but could be vital to that using it through the interface's type, and therefore we implement it explicitly.

Conversely, public int Count is not implemented explicitly because it could potentially be of use to someone using an instance through its own type.

But with your "very rarely used" case, I would lean very strongly indeed toward not using an explicit implementation.

In the cases where I do recommend using an explicit implementation calling the method through a variable of the class's type would either be an error (attempting to use the indexed setter) or pointless (checking a value that will always be the same) so in hiding them you are protecting the user from buggy or sub-optimal code. That's significantly different to code you think is just likely to be rarely used. For that I might consider using the EditorBrowsable attribute to hide the member from intellisense, though even that I would be weary of; people's brains already have their own software for filtering out what doesn't interest them.

Licensed under: CC-BY-SA with attribution
scroll top