In Part III we looked at providing better diagnostic assistance for the SetProperty method on the ObservableItem class by asserting the relevant assumptions. Unfortunately this approach doesn't help third party users of our data model unless we give them access to debug builds. What we can do is modify the approach to match what the Windows Forms team did for detecting illegal cross-thread calls and use exceptions instead of asserts. Essentially this means the default behaviour will be to throw an exception only when the debugger is attached, providing third parties with the opportunity to see exceptions when they are debugging. This is a reasonable compromise because we want to detect incorrect usage of the SetProperty method but we don’t want to unnecessarily penalise the release build performance. The following static property indicates whether or not we want to verify the arguments to the SetProperty method.
private static bool s_verifySetPropertyArguments = Debugger.IsAttached; [EditorBrowsable(EditorBrowsableState.Advanced)] public static bool VerifySetPropertyArguments { get { return s_verifySetPropertyArguments; } set { s_verifySetPropertyArguments = value; } }
The default value is determined by the Debugger.IsAttached property but it can be changed to perform verification even when a debugger is not attached. This provides a limited amount of control over the behaviour of the ObservableItem class, plus it also makes it easier to test the verification code. I’ve used the EditorBrowsableAttribute class with the EditorBrowsableState.Advanced value to hide the property from IntelliSense when advanced members are not enabled in the code editor. This is a reasonable approach because the default value is sufficient for all but the most advanced scenarios. The SetPropertyInternal method can now use the property to determine whether or not to run the verification code, which is no longer confined to debug builds.
[MethodImpl(MethodImplOptions.NoInlining)] private void SetPropertyInternal<T>( string propertyName, ref T field, T value, IEqualityComparer<T> comparer) { if (VerifySetPropertyArguments) { VerifySetPropertyInternalArguments(propertyName, comparer); } if (!comparer.Equals(field, value)) { field = value; OnPropertyChanged(new PropertyChangedEventArgs(propertyName)); } }
The VerifySetPropertyInternalArguments method no longer uses the ConditionalAttribute class although it does still have some asserts.
[MethodImpl(MethodImplOptions.NoInlining)] private void VerifySetPropertyInternalArguments<T>( string propertyName, IEqualityComparer<T> comparer) { Debug.Assert( VerifySetPropertyArguments, "Invalid operation.", "This method should only be called when the 'VerifySetPropertyArguments' " + "property returns a value of true."); Debug.Assert( comparer != null, "Argument null.", "Argument 'comparer' should not be a null reference."); var type = GetType(); var propertyType = typeof(T); PropertyInfo property = type.GetProperty(propertyName, propertyType); if (property == null) { throw new ArgumentException( string.Format( CultureInfo.InvariantCulture, "The argument does not represent the name of a property on this type with " + "the expected return type. One or more of the following values is wrong.\n\n" + "Type: {0}\nProperty name: {1}\nProperty type: {2}", type.FullName, propertyName, propertyType.FullName), "propertyName"); } else { MethodBase expectedSetMethod = property.GetSetMethod(); var frame = new StackFrame(3); MethodBase actualSetMethod = frame.GetMethod(); if (actualSetMethod != expectedSetMethod) { throw new ArgumentException( string.Format( CultureInfo.InvariantCulture, "The argument does not match the name of the property being set.\n\n" + "Expected property set method: {0}\nActual property set method: {1}", expectedSetMethod, actualSetMethod), "propertyName"); } } } }
The first assert ensures the method is being called only when necessary and the second assert has been retained from Part III because there should always be a comparer. The key difference is that we now throw an ArgumentException when detecting incorrect usage. If the expected property cannot be found you’ll see the following exception details:
If the wrong property setter is being called you’ll see the following exception details:
You’ve probably noticed the MethodImplAttribute class on the methods above with the MethodImplOptions.NoInlining value. Even though the default behaviour is for the verification code to only run under the debugger, we can’t assume that will always be the case. This means we don’t know if the jitter has applied optimisations, such as method inlining, which could cause the verification code to produce false positives. Using the NoInlining value prevents a method from being inlined, ensuring that the verification code performs as expected.
The next issue to address is that the SetPropertyInternal method can call unknown code. In Part V we'll look at how this can occur, why it's a problem and how we can fix it.
0 comments:
Post a Comment