Checking Whether A Method Is Overridden

13 commentsWritten on January 11th, 2010 by
Categories: C#

One of the requirements for using Agatha’s Caching Layer is that your request types must override the Equals and the GetHashCode methods.  In order to limit the number of questions i’m going to receive about the caching layer not working correctly the amount of time people will waste on debugging when they don’t override these methods, i wanted to add a check to the initialization of Agatha which makes sure that each cacheable request type indeed overrides Equals and GetHashCode.

I actually had to think about how i could do this, but it turned out to be very simple.  The trick is basically to retrieve the MethodInfo of the method you're interested in, and then check the declaring type of that method.  If the method has been overridden, the declaring type will equal the type of the class you're checking.  If it hasn’t been overridden, the declaring type will be that of one of the base classes:

        private static bool CheckIfOverrideExists(Type type, string methodName)

        {

            var methodInfo = type.GetMethod(methodName, BindingFlags.Instance | BindingFlags.Public);

 

            if (methodInfo == null)

            {

                return false;

            }

 

            return methodInfo.DeclaringType == type;

        }

 

Not sure if there’s a better way to do this (if so, please share) but for now, this is good enough for me.

  • Leon Breedt

    A little more robust: Check for Virtual and the presence of the ReuseSlot flag.

    static bool IsMethodOverriden(Type type, string methodName)
    {
    var mi = type.GetMethod(methodName, BindingFlags.Instance | BindingFlags.Public | BindingFlags.DeclaredOnly);
    return ((mi.Attributes & MethodAttributes.Virtual) != 0) &&
    ((mi.Attributes & MethodAttributes.VtableLayoutMask) == MethodAttributes.ReuseSlot);
    }

    Also, for your use case, I believe you want to include BindingFlags.DeclaredOnly, so that it doesn’t search up the hierarchy, but maybe not…

  • Leon Breedt

    If it wasn’t clear, checking for virtual and reuse-slot pretty much verifies that the method was declared thus:

    \override RETURN-Type MethodName(…)\

    At least from my understanding :)

  • Dave H

    Thanks for posting about this, I have a similar check in a unit test for a company project to make sure people obey our domain object contract when they make new objects. It saves a lot of debugging headaches!

  • http://davybrion.com Davy Brion

    @Leon

    great stuff, didn’t know about that :)

  • Pingback: The Morning Brew - Chris Alcock » The Morning Brew #515

  • http://twitter.com/vkornov Victor Kornov

    Did you consider a role interface like ICacheable { bool Equals(); int GetHashCode();} which all cached responses must implement?

  • Steve

    @Victor

    That won’t help since System.Object already implements those methods.

  • http://davybrion.com Davy Brion

    @Victor

    Yeah, but like Steve said it wouldn’t help :)

  • http://twitter.com/vkornov Victor Kornov

    Yeah, sorry, I wan’t clear. What I really meant is… you could require this interface to be implemented to make caching more explicit. It already requires modification of the response classes. Adding an interface like that should make people think of what they are doing. In any case, you can’t do much about this whole thing. This is exactly like the case with .NET in-built hashes\dictionaries. You just have to remember.

    Btw, how this “check if Equals&GetHashCode are overridden” thing is going to work with a case of base class implementing those 2 methods? You pretty much force people to have redundant, “base” forwarding implementations then.

  • http://davybrion.com Davy Brion

    @Victor

    you do have to put an attribute on top of your request class to enable caching for it so there is some kind of explicitness (though not entirely obviously), response types don’t need to be altered in any way though.

    The only thing i’m forcing people to do is to make sure they have valid implementations of Equals and GetHashCode for _their_ situation and only if they want to use caching for that particular type of request. I agree that it’s not ideal, but it’s the only way i could come up with to make it work.

  • Dathan Bennett

    The only thing i’m forcing people to do is to make sure they have valid implementations of Equals and GetHashCode for _their_ situation and only if they want to use caching for that particular type of request.

    To be precise, what your code here does is force the user to have an override of Equals and GetHashCode on the most-derived class. If they override on a base class and then inherit from it, their classes will fail your test, even though Equals and GetHashCode may be fully functional. Is this the behavior you want?

    If not, Leon’s approach should work better for you.

  • http://davybrion.com Davy Brion

    @Dathan

    it’s a tough one… if you derive from a request class, you’re always going to add _something_ to it. That ‘something’ is pretty much always an important part of the value semantics of an instance of your request type, and as such it needs to be taken into account when doing the equality and hashcode checks in the context of caching

    i’m still not entirely sure on it though, which is why the check hasn’t been committed to the project yet

  • Pingback: Find, if a method is overridden in a derived class! « ReflectWorks