2

My class currently has two constructors, which are overloads:

public CustomRangeValidationAttribute(string value) {}

and

public CustomRangeValidationAttribute(object value) {}

this appears to be working correctly: When I call the method using a string the first constructor is called, when I use different values, for example an integer or boolean, the second constructor is called.

I assume there is a rule to force specific type matches into the more specific overload, preventing 

var c = new CustomRangeValidationAttrubute("test");

from calling the object-overload.

Is this "safe code", or should (or can) the code be improved? I have a nagging feeling this is not the best practice.

Improve this question
8
  • Depends, what you are trying to achieve? Commented Jul 24, 2018 at 9:40
  • I need to be sure there are absolutely no risks of the wrong constructor getting called. And if that's not the case, it still feels like a bad practice somehow. Commented Jul 24, 2018 at 9:43
  • If the object based method will do something fundamentally different/wrong if it happens to be called with a string, that's probably a logical error in creating overloads in the first place, but nothing to do with the calling code. Commented Jul 24, 2018 at 9:44
  • 2
    Just keep in mind that the overload will be selected at compile time so if you have an object reference which is actually a string, the 2nd overload will be called (unless you cast the object to a string). Commented Jul 24, 2018 at 9:44
  • 1
    @mjwills - difficult to do with constructors, since you can't run logic before chaining (in C#) Commented Jul 24, 2018 at 10:36

4 Answers 4

Reset to default
3

You have two overloads which only vary in the reference types and there's a hierarchical relationship between the reference types, such that one may be cast to the other.

In such a circumstance, you really ought to make sure that the code behaves the same logically when the broader overload is selected but the reference turns out to be of the more derived type1,2. That is where to focus your attention. Of course, if you can stick by this rule, often it'll turn out that the more derived overload isn't required and can just be special-cased within the broader method.

1Especially because, as vc74 points out in a comment, overload resolution (generally, ignoring dynamic) is done at compile time based on compile-time types3.

2And this fits the same broad principal for overloads. Don't have overloads where which one is selected leads to logically different results. If you're exhibiting different behaviours, don't give them the same name (for constructors, that may mean splitting into two separate classes, possibly with a shared base class, if that's what you intend to do)

3I appreciate that this is for an attribute and so you're expecting only compile-time to be relevant, but I'd still hew to the general principal here, where possible.

Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

2

Once there is overload with signature of more derived type the compiler will always choose most concrete type you provide.

That being said, unless someone does new CustomRangeValidationAttrubute((object)"test") if you pass string to CustomRangeValidationAttrubute always constructor with string in it's parameter will be chosen.

About if this is bad practice, I can't tell for sure if I don't see your specific use case, just keep in mind every value type you pass to new CustomRangeValidationAttrubute(object) will be boxed and this is bad as it puts pressure to the GC and whats more you will loose type safety.

Improve this answer

Comments

0

The way I resolved this code was by moving the overloads to a new abstract class with separate methods instead of the the original constructors:

public CustomRangeValidationStringAttribute(string value) {}

public CustomRangeValidationGenericAttribute(object value) {}

In the two classes inheriting from this new base class each use their own method, creating two different attributes to choose from, [CustomRangeValidationString] and [CustomRangeValidationGeneric].

Improve this answer

Comments

-1

You could use a generic class.

See the documentation

class YourClass<T> 
{
    public YourClass(T value){}
}
Improve this answer

1 Comment

Thought I didn't state it explicitly, my class is an attribute, inheriting from RemoteAttribute and unfortunately generics are not allowed in attribute classes.

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.