Renaming ASMX [WebMethod] parameter preserving compatibility

Sometimes people change their mind and feel better when a parameter named green gets its name changed according its meaning in code, e.g. isProceedingAllowed. We all know that this is a kind of simplest refactoring steps improving code readablity. Why not to do it? It shouldnt break anything, it can be done even manually because parameter name scope is local to the method and if we keep rule of short and overviewable methods then it is a simple task.
But we can make a big mistake.
Renaming a parameter of an old-fashioned ASMX webservice method will break the SOAP contract resulting in magic problems!

Here is a demo code:

[WebMethod("A fancy method doing extraordinary things with its parameter")]
public string CheckSMS(string strSMS)
{
    // an undiscloseable thing happen here...
}

If I would rename the parameter from Hungarian notation to smsNumber that parameter value would be always null until the callers refresh their access code or proxies to reflect change in WSDL we triggered via this rename. And these callers may spread around the whole world or simply sit at one hardly moving client, so the refresh wont be possible in acceptable time (people live only 70-80 years).

That null can be really anoying. Your webmethod may have other parameters which all get their right values from the caller, but the renamed one gets nothing. Without any signs of error. In not-so-often-used situation You may already forget about the rename and only get the issue ticket about some missbehaving service which throws NullReferenceExceptions with deep stacktraces in Your business logic (eh, You didnt read my article about Method implementation pattern? That exception must be thrown at all public entrypoints!). After checking the caller that it is really transmits that parameter value, and after checking that Your code really forwards it well You may think about communication related things.

The asmx trace can be switched on quickly by adding the following in web.config:

<configuration>
<system.diagnostics>
    <trace autoflush="true" />
    <sources>
        <source name="System.Web.Services.Asmx">
            <listeners>
                <add name="AsmxTraceFile" 
                     type="System.Diagnostics.TextWriterTraceListener" 
                     initializeData="asmxtrace.log" 
                     traceOutputOptions="LogicalOperationStack, DateTime, Timestamp, ProcessId, ThreadId" />
            </listeners>
        </source>
    </sources>
    <switches>
        <add name="System.Web.Services.Asmx" value="Verbose"  />
    </switches>
</system.diagnostics>
</configuration>

In log file You will notice the warning below:

System.Web.Services.Asmx Warning: 0 : Az adott kontextusban egy nem várt <strSMS xmlns='http://tempuri.org/'>..</strSMS> elem szerepel. A várt elemek: http://tempuri.org/:smsNumber.

Yes, it is in hungarian because the given locale there, but try to figure out, okay? 🙂

So how can we handle this?
We can always instruct XmlSerializer to use an alias name:

public string CheckSMS([XmlElement("strSMS")]string smsNumber)

But the result dont helps to much: our webmethod will be okay with the new name, but all the callers may use only the obsolete one. No fallbacks, cannot use multiple names for a parameter.
There is a XmlChoiceIdentifierAttribute which – according to its AttributeTargets may be used on parameters too, but I couldnt figure out how. All examples are about classes and their members not method parameters.

It would be nice to have an attribute which we could use like this even multiple times (because we cannot fix our always changing mind; naming is a BIG problem…):

[ParameterNameChangedSoapExtension("smsNumber", "strSMS")]
[ParameterNameChangedSoapExtension("smsNumber", "SMS")]
public string CheckSMS(string smsNumber)

Here came the SoapExtensions in the picture.
You can chain Your code into SOAP request-response (yes, must into both of them) processing.
Searching the net for SoapExtension You will find some examples and quickly realise that they are almost the same: how to log the contents of incoming and outgoing SOAP messages. But we need to modify those messages which isnt well documented. Even MSDN article named “Altering the SOAP Message Using SOAP Extensions” has only that logging sample which is the root of all other samples I mentioned before I think.

This is the reason of this post. It took hours to figure out how it works and how can I achive my goal here.
The results must be shared. Here comes the code:

    public class ParameterNameChangedSoapExtension : SoapExtension
    {
        private Stream streamChainedAfterUs = null;
        private Stream streamChainedBeforeUs = null;

        private const int STREAMBUFFERSIZE = 65535;

        private ParameterNameChangedSoapExtensionAttribute parameterNameChangedSoapExtensionAttribute = null;

        public override Stream ChainStream(Stream stream)
        {
            if (stream == null)
            {
                throw new ArgumentNullException("stream");
            }
            
            Stream ret = null;

            this.streamChainedBeforeUs = stream;
            this.streamChainedAfterUs = new MemoryStream();
            
            ret = this.streamChainedAfterUs;

            return ret;
        }

        public override object GetInitializer(Type serviceType)
        {
            throw new NotSupportedException();
        }

        public override object GetInitializer(LogicalMethodInfo methodInfo, SoapExtensionAttribute attribute)
        {
            if (attribute == null)
            {
                throw new ArgumentNullException("attribute");
            }
            
            object ret = attribute;
            return ret;
        }

        public override void Initialize(object initializer)
        {
            if (initializer == null)
            {
                throw new ArgumentNullException("initializer");
            }
            
            parameterNameChangedSoapExtensionAttribute = initializer as ParameterNameChangedSoapExtensionAttribute;

            // sanity
            if (parameterNameChangedSoapExtensionAttribute == null)
            {
                throw new InvalidOperationException(String.Format("initializer must be of type {0}, but its a {1}!", typeof(ParameterNameChangedSoapExtensionAttribute), initializer.GetType()));
            }
        }

        public override void ProcessMessage(SoapMessage message)
        {
            if (message == null)
            {
                throw new ArgumentNullException("message");
            }
            
            switch(message.Stage)
            {
                case SoapMessageStage.BeforeSerialize:
                    break;
                case SoapMessageStage.AfterSerialize:
                    // no business here; we are just part of chain so must participate well
                    streamChainedAfterUs.Position = 0;
                    Copy(streamChainedAfterUs, streamChainedBeforeUs);
                    break;
                case SoapMessageStage.BeforeDeserialize:
                    // here are we doing the magic!
                    UpdateMessage(message);
                    streamChainedAfterUs.Position = 0;
                    break;
                case SoapMessageStage.AfterDeserialize:
                    break;
                default:
                    throw new NotImplementedException(message.Stage.ToString());
            }
        }

        private void UpdateMessage(SoapMessage message)
        {
            // get the original raw msg
            var soapMsgAsString = ReadOriginalSoapMessage();
            var soapMsgRootNode = XElement.Parse(soapMsgAsString);

            // extract namespace info
            var callDescriptorNode = FindCallDescriptorNode(soapMsgRootNode, message.MethodInfo.Name);
            var ns = callDescriptorNode.Name.Namespace;

            // look for parameter named obsolete
            var originalNameWeLookFor = ns + parameterNameChangedSoapExtensionAttribute.OriginalParameterName;

            var nodeWithOriginalName = callDescriptorNode.Elements().FirstOrDefault(i => i.Name == originalNameWeLookFor);
            if (nodeWithOriginalName != null)
            {
                // found, lets replace!
                var nodeWithCurrentName = new XElement(ns + parameterNameChangedSoapExtensionAttribute.CurrentParameterName, nodeWithOriginalName.Value);
                nodeWithOriginalName.AddAfterSelf(nodeWithCurrentName);
                nodeWithOriginalName.Remove();
            }

            // write what we had or what we made from it
            WriteResultSoapMessage(soapMsgRootNode.ToString());
        }

        private XElement FindCallDescriptorNode(XElement soapMsgRootNode, string methodName)
        {
            XElement ret = null;
            
            var soapBodyName = soapMsgRootNode.Name.Namespace + "Body";
            var soapBodyNode = soapMsgRootNode.Elements().First(i => i.Name == soapBodyName);

            ret = soapBodyNode.Elements().First(i => i.Name.LocalName == methodName);

            return ret;
        }

        private void WriteResultSoapMessage(string msg)
        {
            streamChainedAfterUs.Position = 0;
            using (var sw = new StreamWriter(streamChainedAfterUs, Encoding.UTF8, STREAMBUFFERSIZE, true))
            {
                sw.Write(msg);
            }
        }

        private string ReadOriginalSoapMessage()
        {
            string ret = null;

            using (var sr = new StreamReader(streamChainedBeforeUs, Encoding.UTF8, false, STREAMBUFFERSIZE, true))
            {
                ret = sr.ReadToEnd();
            }

            return ret;
        }

        private void Copy(Stream from, Stream to)
        {
            using (var sr = new StreamReader(from, Encoding.UTF8, false, STREAMBUFFERSIZE, true))
            {
                using (var sw = new StreamWriter(to, Encoding.UTF8, STREAMBUFFERSIZE, true))
                {
                    var content = sr.ReadToEnd();
                    sw.Write(content);
                }
            }
        }
    }

First You must understand the thing about ChainStream method and the streams around it. All samples mention them as oldStream and newStream but that isnt correct. Our extension fits inside other extensions in some order. Our input is the output of an other extension or the framework itself and our output will be input for yet another one. And there is a twist: in response processing these steps occure in reverse: the stream which was output before becomes our input now and vica versa! So the old/new or input/output distinction is very misleading thats why I call them streamChainedAfterUs and streamChainedBeforeUs.
According to this usage we should always care about our stream position so the next processing entity can find it at right place.

Between public methods the ProcessMessage is the important one. All others are described in documentation and has kind of infrastructure rules.
As I mentioned before when we write a SoapExtension we must participate in both of request and response processing. Thats the reason of the simple Copy step in ProcessMessage implemetation.

The solution to our problem can be found in UpdateMessage method. We parse the SOAP message, we look for obsolete parameter name and replace it with the current one if found. Thats all.
As a result our webmethod has a “good” parameter name (at the time of this writing, hehehe), generates WSDL with that name, BUT accepts calls with all the obsolete names too!

At the end here is the attribute code to have a full solution:

    [AttributeUsage(AttributeTargets.Method, AllowMultiple=true)]
    public class ParameterNameChangedSoapExtensionAttribute : SoapExtensionAttribute
    {
        public override Type ExtensionType
        {
            get { return typeof(ParameterNameChangedSoapExtension); }
        }

        public override int Priority { get; set; }

        public string CurrentParameterName { get; private set; }
        public string OriginalParameterName { get; private set; }

        public ParameterNameChangedSoapExtensionAttribute(string currentParameterName, string originalParameterName)
        {
            if (String.IsNullOrEmpty(currentParameterName))
            {
                throw new ArgumentNullException("currentParameterName");
            }
            if (String.IsNullOrEmpty(originalParameterName))
            {
                throw new ArgumentNullException("originalParameterName");
            }

            this.CurrentParameterName = currentParameterName;
            this.OriginalParameterName = originalParameterName;
        }
    }

Jesus, still reading? This is the 280th line! 🙂

Empty enumerables initialized to null by DefaultModelBinder

As I mentioned before the ASP.NET MVC5’s DefaultModelBinder has some quirks. The actual one I met some days ago is the following.

public class MyViewModel
{
    public IEnumerable<int> IntList { get; set; }
}

What happens, when You call Your method with this JSON request?

{ IntList:[] }

I would like to find an empty IEnumerable<int> instance in IntList, but I will found null there.
Why? Because the DefaultModelBinder initializes my empty collection to null.

What You can do to avoid this is to write a custom model binder for this:

public class EmptyEnumerableCapableDefaultModelBinder:DefaultModelBinder
{
    public override object BindModel(ControllerContext controllerContext, ModelBindingContext bindingContext)
    {
       object ret = base.BindModel(controllerContext, bindingContext);

       WorkaroundEmptyEnumerablesInitializedToNullByDefaultModelBinder(ret, controllerContext, bindingContext);

       return ret;
    }
}

private void WorkaroundEmptyEnumerablesInitializedToNullByDefaultModelBinder(object model, ControllerContext controllerContext, ModelBindingContext bindingContext)
{
    if (model != null)
    {
        // workaround case when there is an IEnumerable<> member and there is a "member":[] in request
        // but default binder inits member to null

        var candidateList = bindingContext.PropertyMetadata
            .Where(kvp => bindingContext.PropertyFilter(kvp.Key))
            .Where(kvp => TypeHelper.IsSubclassOf(kvp.Value.ModelType, typeof(IEnumerable<>)))
            .Where(kvp => !bindingContext.ValueProvider.ContainsPrefix(kvp.Key)).ToArray();
        if (candidateList.Any())
        {
            if (!controllerContext.HttpContext.Request.ContentType.StartsWith("application/json"))
            {
                throw new NotImplementedException(controllerContext.HttpContext.Request.ContentType);
            }

            var json = GetJSONRequestBody(controllerContext);

            foreach(var candidate in candidateList)
            {
                var emptyEnumerablePattern = String.Format("\"{0}\":[],", candidate.Key);
                if (json.Contains(emptyEnumerablePattern))
                {
                    var pd = bindingContext.ModelType.GetProperty(candidate.Key);
                    var anEmptyArray = Array.CreateInstance(pd.PropertyType.GetGenericArguments()[0], 0);
                    pd.SetValue(model, anEmptyArray);
                }
            }
        }
    }
}

private string GetJSONRequestBody(ControllerContext controllerContext)
{
    string ret = null;

    var inputStream = controllerContext.HttpContext.Request.InputStream;
    inputStream.Position = 0;

    using (var sr = new StreamReader(inputStream, controllerContext.HttpContext.Request.ContentEncoding, false, 1024, true))
    {
        ret = sr.ReadToEnd();
    }

    return ret;
}

The point is to check the inputs on DefaultModelBinder’s null result whether it missed an empty enumerable on binding.
The valueProviders available in context are useless because they simply dont contain our IntList value. Instead I check our
target viewmodel for possible candidates and check their values directly in request. If an empty value found in request I
replace the binder result with an empty array instance which fits into IEnumerable<T> place.

KeyValuePair<,> capable ASP.NET MVC5 model binder

Once upon a day I created a viewmodel class:

public class MyViewModel
{
    public List<KeyValuePair<string, int>> MyList { get; set; }
}

I wanted to use it as a parameter in my MVC action method. The wonderful model binding feature of MVC allows me to do that and it seemed to be working without error.
I got the exact number of key value pairs in my list property but the Key and Value props were always null and 0. I repeat: without any error!

After checking DefaultModelBinder’s source I realized that it will never work: KeyValuePair<,> is a struct, so assigning to variable means a copy and it’s members are readonly so can be set only during construction. The logic in DefaultModelBinder is different: it creates the model objects, handles them over via variable assignations, evaluates their member values and then assigns those values to members. There is a workaround implemented inside related to Dictionary<,>, but it’s logic isn’t reusable for my situation because the programmer didn’t intended to allow that (private methods) and the logic there is a bit smells for me.

There are solutions on the net, but those I found suffer from one common problem: they evaluate Key and Value on their onnw, which skips some goods of model binding, e.g. validation and model state propagation. Not too good.

Here comes my solution. 🙂

First I created a new default model binder which in case of KeyValuePair<,> model type calls my BindModelViaKeyValuePairSubstitute from BindModel method but leaves all other things handled by original implementation.

public class KeyValuePairCapableDefaultModelBinder:DefaultModelBinder
{
    public override object BindModel(ControllerContext controllerContext, ModelBindingContext bindingContext)
    {
       object ret = null;

       if (TypeHelper.IsSubclassOf(bindingContext.ModelType, typeof(KeyValuePair<,>)))
       {
           ret = BindModelViaKeyValuePairSubstitute(controllerContext, bindingContext);
       }
       else
       {
           ret = base.BindModel(controllerContext, bindingContext);
       }

       return ret;
    }
}

I created a substitute class which overcomes the limitations: not a struct and has writable members.
To make the trick transparent to model binding the substitute class must contain members of same name and type as the KeyValuePair<,> we want to handle.


        /// <summary>
        /// KeyValuePair substitute.
        /// </summary>
        private class KeyValuePairSubstitute<TKey, TValue> : KeyValuePairSubstituteBase
        {
            public TKey Key { get { return (TKey)KeyAsObject; } set { KeyAsObject = value; } }
            public TValue Value { get { return (TValue)ValueAsObject; } set { ValueAsObject = value; } }

            public KeyValuePairSubstitute()
            {
                // set values to defaults to avoid NullReferenceExceptions when trying to get
                // an uninitialized null value from a generic type which cannot stand that (e.g. int).
                this.Key = default(TKey);
                this.Value = default(TValue);
            }
        }
        
        /// <summary>
        /// Base class for KeyValuePair substitute to allow access to generic values in handy way.
        /// </summary>
        private class KeyValuePairSubstituteBase
        {
            public object KeyAsObject { get; set; }
            public object ValueAsObject { get; set; }
        }

Now my BindModelViaKeyValuePairSubstitute is trivial.
The logic here is to let DefaultModelBinder bind our substitute object instead of a KeyValuePair<,> and then instantiate a KeyValuePair<,> from that object’s content.

        /// <summary>
        /// Default BindModel call doesnt handle KeyValuePair well, because it is a struct and has readonly props.
        /// It will return an instance with default values without any error!
        /// </summary>
        private object BindModelViaKeyValuePairSubstitute(ControllerContext controllerContext, ModelBindingContext bindingContext)
        {
            object ret = null;

            var keyValuePairSubstituteGeneric = typeof(KeyValuePairSubstitute<,>).MakeGenericType(bindingContext.ModelType.GetGenericArguments());

            var kvpBindingContext = new ModelBindingContext()
            {
                ModelMetadata = ModelMetadataProviders.Current.GetMetadataForType(null, keyValuePairSubstituteGeneric),
                ModelName = bindingContext.ModelName,
                ModelState = bindingContext.ModelState,
                ValueProvider = bindingContext.ValueProvider
            };

            var keyValuePairSubstitute = (KeyValuePairSubstituteBase)base.BindModel(controllerContext, kvpBindingContext);

            ret = Activator.CreateInstance(bindingContext.ModelType, keyValuePairSubstitute.KeyAsObject, keyValuePairSubstitute.ValueAsObject);
            return ret;
        }

The last step: the new model binder should be registered in Application_Start as usual:

            ModelBinders.Binders.DefaultBinder = new CustomModelBinder();

That’s all. You have bindable and validable KeyValuePair<,>s now!

Danger of IEnumerables

IEnumerables and IEnumerable<T>s are a good thing:

  • They allow returning set of values with a minimum contract and behaviour promise. You may alter the underlying data structure later to virtually anything, because nobody was able to use Your retval in a way You didnt mention. For example if You used a List instead Your retval consumers may add to or remove items from it and become coupled to that retval type. See next too.
  • They allow returning unchangeable “lists”. Did You ever hunted a bug where Your retval instance was containing values which wasnt created by Your method?
  • They may be lazy. You shouldnt known that Your retval consumers how want to use Your data. You may have a resource eater mapping process to run on 1000s of items, but the consumer may only want the Firts() item!
  • LINQ. Just the whole thing uses IEnumerable‘s features and is returning something of this type.
  • Etc. There could be a lot of things.

So I tend to be using these as a retval in every place where the only thing I want to return multiple instances of something.

Now the 50cent question: will this test run green?

 

[TestMethod]
public void MyTestMethod()
{
    IEnumerable<MyClass> result = GetMyClassList();
    Assert.AreSame(result.First(), result.First());
}

Yes? Are You sure? Sure! The first item of an IEnumerable will always be the same!
Or not?
Lets see GetMyClassList‘s implementation:

 

public IEnumerable<MyClass> GetMyClassList()
{
    IEnumerable<MyClass> ret = new MyClass[] { new MyClass(1) };
    return ret;
}

Yes, in case of this implementation the test becomes green.
But how about this:


public IEnumerable<MyClass> GetMyClassList()
{
    IEnumerable<MyClass> ret = null;
 
    var someSourceList = new int[] { 1 };
    ret = someSourceList.Select(i => new MyClass(i));
 
    return ret;
}

Now the test became red!

Why?

Because IEnumerable promises only a sequential access to items.

To items they contain.

In the first case these are items of a fixed array.

But in the second case the items are the values returned by a LINQ projection, which contains a mapping body.

When I call First() twice the IEnumerable‘s enumerator can only be recreated or Reset() and start over the evaluation. So the new MyClass(i) will run again and again resulting in different instances and failing test. And the resource friendly lazy evaluation may become shortly really bad too…

There is nothing new in the above, but in my head the parts of the explanation didnt connect to each other before.

But wait a minute! Is this meaning that when I use an IEnumerable I should known about its creation method?!?! This would break the basics or OOP!

No, I shouldnt known anything about it, just remember: IEnumerable only promises a sequential access to items!

When I consume it in a way that dont requires Reset() or enumerator recreation I need no extra steps:


var firstItem = result.First();

But when the consumption method results in multi-enumeration I should “fix” its items via explicit enumeration, for example:


var fixedResult = result.ToArray();

That allows You to use IEnumerable in a way it was designed and saves some ugly moments of You valuable life. 🙂

Misleading message

What You would do if You get the following result after running a unit test?

Assert.AreEqual failed. Expected:<2016.04.27. 8:22:52>. Actual:<2016.04.27. 8:22:52>.

My head was full with abstractions waiting to be coded, but the above result brings me in
unexpected state. Everything suspended and my eyes were scanning the two values character by
character repeatedly to find out what is the difference? Nothing!
A quick debug revealed that the values differ in milliseconds which are not shown in the message.

But what a misleading message! Maybe the difference should be emphasized somehow!
Because I lost my concentration, my flow, etc.
It was the same when You cannot work quietly because somebody always coming to You and asks
something. Anything. Applying the 8th point of the great article I found on Joel’s blog years before
to this situation: such messages are breaking down productivity and should be avoided.

A bug hunting story

Today I found a bug. It was so interesting that I decided to write a longer post here about it.
I created a strip down solution with the only classes and methods I need to demonstrate the bug. This is the reason if the story wont seem too realistic.

A long long time ago I need a dictionary to store some integers with a key which was based on a string but has some other features (not shown here). So I created MyKey class for this:

[Serializable]
public class MyKey
{
    private string key = null;

    public MyKey(string key)
    {
        if (key == null)
        {
            throw new ArgumentNullException("key");
        }

        this.key = key;
    }

    private int? hashCode = null;
    public override int GetHashCode()
    {
        int ret = 0;

        if (hashCode == null)
        {
            hashCode = this.key.GetHashCode();
        }

        ret = hashCode.Value;

        return ret;
    }

    public override bool Equals(object obj)
    {
        bool ret = false;

        MyKey other = obj as MyKey;
        if (other != null)
        {
            ret = Equals(other);
        }

        return ret;
    }

    public bool Equals(MyKey other)
    {
        bool ret = false;

        if (other != null)
        {
            if (this.hashCode == other.hashCode)
            {
                if (this.key == other.key)
                {
                    ret = true;
                }
            }
        }

        return ret;
    }

    public override string ToString()
    {
        string ret = String.Concat("\"", key, "\"");
        return ret;
    }
}

It was used happily like this:

// create data
var data = new Dictionary&lt;MyKey, int>();
data[new MyKey("alma")] = 1;

Later I wrote some code to persist these data via serialization.
Everything was working like a charm.

// serialize and save it
var serializedData = Serializer.Serialize(data);
SaveToFile(serializedData);

...

// load and deserialize data
var serializedData = LoadFromFile();
var data = Serializer.Deserialize(serializedData);

There was a usecase when after deserialization some of the values in data must be changed:

// as in deserialized data
var specificKey = new MyKey("alma");
if (data[specificKey] == 1) // a KeyNotFoundException occures here!
{
    data[specificKey] = 2;
}

KeyNotFoundException? I was sure that there should be a value in all of data instances with the given key! Lets see in QuickView:

There is an “alma” key!
Let’s comment out the line causing the exception and check data after the expected value modification to “2”:

Much more interesting isnt it?
I quickly put all the data creation, serialization, deserialization code into one unit test to have a working chunk of code I can use for bug hunting:

[TestMethod]
public void TestMethod1()
{
    var d = new Dictionary<mykey, int="">();
    d[new MyKey("alma")] = 1;

    var serialized = Serializer.Serialize(d);

    var data = Serializer.Deserialize(serialized);

    var specificKey = new MyKey("alma");
    {
        data[specificKey] = 2;
    }
}

But in the unit test everything was working! I simply cant reproduce the bug in such a way.
But when running App1, which was creating and serializing the data and running App2 which was deserializing and modifying it the bug always presents itself.
How can be a duplicate key in a Dictionary<,>? MyKey‘s implemetation, especially the Equals() override is so trivial that it cannot allow two instances created from
same string to be not equal.

But wait a minute!

How can the hashCode’s differ?!?!?!

Yes. A quick search on the net answers everything. MSDN clearly describes in a big “Important” box:

The hash code itself is not guaranteed to be stable. Hash codes for identical strings can differ across versions of the .NET Framework and across platforms (such as 32-bit and 64-bit) for a single version of the .NET Framework. In some cases, they can even differ by application domain.

As a result, hash codes should never be used outside of the application domain in which they were created, they should never be used as key fields in a collection, and they should never be persisted.

App1 was running in x86 and App2 in x64 environment. Thats why the string hashcodes differ.

The fix is really easy. Just turn off hashCode calculation optimalization for serialization:

[Serializable]
public class MyKey
{
   ...

   [NonSerialized]
   private int? hashCode = null;
   ...
}

Now hashCode will be recalculated once in all runtime environments.

I never thought about the possibility of unstable hashcodes.
I hope I am not the only man in the world with such wasteful brain.

CLR20r3 FileLoadException and Why Keep Settings from .config?

Today one of my colleagues tried to start a newer version of one of or .Net tools on her Win7 computer which was just copied from deployment share.

The app didn’t started, only Windows Error Reporting was doing something on the systray. In the Eventlog there was an error 22 with CLR20r3 and mentioning a FileLoadException. P4’s value: PresentationFramework. Short check about installed frameworks, etc.: everything seemed to be fine. Nothing useful was found in generated WER file either.

The tool was running well on other’s machine. What happened with her’s?

In the app’s .config file there were custom app settings in the appSettings section that’s why we keeped the original file from previous installation. The problem was that in .config there was an assemblyBinding section with a bindingRedirect too:

<configuration>
  <runtime>
    <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
      <dependentAssembly>
        <assemblyIdentity name="AnAssembly" publicKeyToken="123123123123" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-1.0.33334.0" newVersion="1.0.33334.0" />
      </dependentAssembly>
     ...

The new code had a newer version of the AnAssembly which wasnt used because of the bindingRedirect above! The new .config had the updated version numbers, but we overwrite it with the previous version of file because we would like to keep the correct appSettings values. It is very handy to use the appSettings section in the .config but it is a bad idea because of framework configuration is keeped in the same place. Keeping it separately seems better idea.

PS.: What about PresentationFramework in the P4 value? Completely missleading info…

LINQ to Object: Skip() performance

Assume You have a List of 5000000 items. You want to do some paging so You need e.g. 100 items from offset 300000. Check these two implementations:

        public List<T> GetItems1<T>(List<T> source, int offset, int count)
        {
            List<T> ret = new List<T>();

            for (int i = offset; i < offset + count; i++)
            {
                ret.Add(source[i]);
            }

            return ret;
        }
        public List<T> GetItems2<T>(List<T> source, int offset, int count)
        {
            List<T> ret = source.Skip(offset).Take(count).ToList();

            return ret;
        }

What do You think, which performs better? You may say the second one of course. The first one indexes the source list and calls Add() method count times. The second simply enumerates once till offset then returns count items as a new List with the possibility of internal item addition optimalization. At least that were what I think.

But I was wrong!

The second implementation always slower. The magnitude depends on the offset value but it is always slower!

offset GetItems1 GetItems2
0 43 65
10000 59 729
100000 44 5162
1000000 42 52057
3000000 44 147608

The reason is inside the implemetation details of List and IEnumerable.Skip(). The first one knows where to find the nth item, the second one should enumerate to it. The conclusion as one of my colleagues pointed out: use as specialized tools as You can.

The code which I used for result above:

    [TestClass]
    public class Test
    {
        private int cycles = 100;
        private int offset = 0;
        private int count = 100;

        [TestMethod]
        public void perftest1()
        {
            var l = GetTestData();

            var sw = new Stopwatch();

            double r = 0;
            for (int i = 0; i < cycles; i++)
            {
                sw.Reset();
                sw.Start();
                var result = GetItems1(l, offset, count);
                sw.Stop();
                r += sw.ElapsedTicks;
            }

            Assert.Fail((r / cycles).ToString());
        }


        [TestMethod]
        public void perftest2()
        {
            var l = GetTestData();

            var sw = new Stopwatch();

            double r = 0;
            for (int i = 0; i < cycles; i++)
            {
                sw.Reset();
                sw.Start();
                var result = GetItems2(l, offset, count);
                sw.Stop();
                r += sw.ElapsedTicks;
            }

            Assert.Fail((r / cycles).ToString());
        }


        public List<T> GetItems1<T>(List<T> source, int offset, int count)
        {
            ...
        }


        public List<T> GetItems2<T>(List<T> source, int offset, int count)
        {
            ...
        }

        private List<int> GetTestData()
        {
            List<int> ret = new List<int>();

            for (int i = 0; i < 5000000; i++)
            {
                ret.Add(i);
            }

            return ret;
        }
    }

NSubstitute’s unexpected default return value

NSubstitute is a great tool when You are writing unit tests for Your product which was designed with dependency injection in mind.

Let’s see it:

    public interface IWorker
    {
        object GetResult();
    }

    [TestMethod]
    public void ExecuterTest_WorkerCalledOrNot()
    {
        var workerSubstitute = Substitute.For<IWorker>();

        var executerToTest = new Executer(workerSubstitute);

        // we test here
        executerToTest.Execute();

        workerSubstitute.Received(1).GetResult();
    }

In the code above I was not interested in the real value of the worker’s result. I wanted only to know whether the worker’s GetResult method gets called or not. But all of my workers do some hard work, so for this test I dont want to instantiate them and implicitli convert my unit test into integration test. So I create a substitute via NSubstitute for the given interface and gave that to my Executer.

The substitute tries to be as neutral as it can be. All void methods return asap, all non void methods return the default value for the return type. Naturally You can modify that behaviour and explicitly tell the substitute what to do when it’s methods get called, so You can for example redirect all database calls to in memory data structures during tests if You created Your DAL layer with DI in mind. And meanwhile the subtitute collects data about its use, so we can ask it whether it was called or not and with what arguments, etc.

But today I found got something unexpected thing.

Change a bit the example above to demonstrate it:

    public interface IWorker
    {
        TRet GetResult<TRet>();
    }

    [TestMethod]
    public void SubstituteReturnValueTest()
    {
        var workerSubstitute = Substitute.For<IWorker>();

        // test #1
        var r1 = workerSubstitute.GetItem<int>();
        Assert.AreEqual(default(int), r1);

        // test #2
        var r2 = workerSubstitute.GetItem<List<int>>();
        Assert.AreEqual(default(List<int>), r2);

        // test #3
        var r3 = workerSubstitute.GetItem<IList<int>>();
        Assert.AreEqual(default(IList<int>), r3);
    }

This will fail at Assert of test #3 because r3 wont be null as You would expect but an instance of “Castle.Proxies.IList`1Proxy”!

I think it is a bug but may be a result of some design decisions which priorized some functionality (retval is an instance of some interface which is wrapped around with a subtitute created on the fly) over coherency.

So be careful 🙂

Method implementation pattern

During the development of many projects I tried to standardize the outlook of methods to help anybody to distinguish between some parts of them.
Check the code below:

        public int SaveNewPartner(Partner partner, Operator modifier)
        {
            if (partner != null)
            {
                if (modifier != null)
                {
                    if (partner.ID == null)
                    {
                        if (!SanityCheck(partner))
                        {
                            throw new ApplicationException("partner failed sanity check");
                        }

                        partner.ModificationTime = DateTime.Now;
                        partner.Modifier = modifier;
                        return Save(partner);
                    }
                    else
                    {
                        throw new InvalidOperationException("Already saved!"); //LOCSTR
                    }
                }
                else
                {
                    throw new ArgumentNullException("partner");
                }
            }
            else
            {
                throw new ArgumentNullException("partner");
            }
        }

I have problems with this code, and if I have them maybe others who meet it later in our project will have too. If You should determine which part is its business functionality it will be probably a shot in the dark. I really need to understand the whole method before I can point out how it works because parameter checking, control flow, exit point mixed with business part.

That’s why I wrote all my methods by following a pattern:

  1. check parameters
  2. define return value
  3. do business functionality
  4. return retval

Here is the rewritten method:

        public int SaveNewPartner(Partner partner, Operator modifier)
        {
            if (partner == null)
            {
                throw new ArgumentNullException("partner");
            }
            if (modifier == null)
            {
                throw new ArgumentNullException("modifier");
            }

            int ret = 0;

            if (partner.ID == null)
            {
                throw new InvalidOperationException("Already saved!"); //LOCSTR
            }
            if (!SanityCheck(partner))
            {
                throw new ApplicationException("partner failed sanity check");
            }

            partner.ModificationTime = DateTime.Now;
            ret = Save(partner);

            return ret;
        }

In lines 3-10 the parameter check occures. There should be parameter checking in each public entrypoint of our class (methods, properties). I check all the parameters I use in the given method directly or in private members I call from here. It is not necessary to check params which are simly handled to other public methods (we may not know all the constraints about these, it’s not our business). I neither check the business validity here (line 3 vs. line 14).

In line 12 I define the return value, which I gave always the name ‘ret’. So if You check any line of the method You can clearly identify where the retval is set and You dont need to scroll anywhere to determine what is the retval variable.

In lines 14-24 placed the business logic. All extremalities are closed asap, so no long lasting ifs and unnecessarily deep indentations happen.

In line 26 we return from here. No other inline returns in the method body so the control flow is clear: we enter at the beginning and exit at the end.