Ayende @ Rahien

It's a girl

Challenge: Modifying execution approaches

In RavenDB, we had this piece of code:

internal T[] LoadInternal<T>(string[] ids, string[] includes)
            if(ids.Length == 0)
                return new T[0];

            Debug.WriteLine(string.Format("Bulk loading ids [{0}] from {1}", string.Join(", ", ids), StoreIdentifier));
            MultiLoadResult multiLoadResult;
            JsonDocument[] includeResults;
            JsonDocument[] results;
            var sp = Stopwatch.StartNew();
            var startTime = DateTime.Now;
            bool firstRequest = true;
                IDisposable disposable = null;
                if (firstRequest == false) // if this is a repeated request, we mustn't use the cached result, but have to re-query the server
                    disposable = DatabaseCommands.DisableAllCaching();
                using (disposable)
                    multiLoadResult = DatabaseCommands.Get(ids, includes);

                firstRequest = false;
                includeResults = SerializationHelper.RavenJObjectsToJsonDocuments(multiLoadResult.Includes).ToArray();
                results = SerializationHelper.RavenJObjectsToJsonDocuments(multiLoadResult.Results).ToArray();
            } while (
                AllowNonAuthoritiveInformation == false &&
                results.Any(x => x.NonAuthoritiveInformation ?? false) &&
                sp.Elapsed < NonAuthoritiveInformationTimeout
                (DateTime.Now - startTime) < NonAuthoritiveInformationTimeout

            foreach (var include in includeResults)

            return results

And we needed to take this same piece of code and execute it in:

  • Async fashion
  • As part of a batch of queries (sending multiple requests to RavenDB in a single HTTP call).

Everything else is the same, but in each case the marked line is completely different.

When we had only one additional option, I choose the direct approach, and implement it using;

public Task<T[]> LoadAsync<T>(string[] ids)
    return AsyncDatabaseCommands.MultiGetAsync(ids)
        .ContinueWith(task => task.Result.Select(TrackEntity<T>).ToArray());

You might notice a few differences between those approaches. The implementation behave, most of the time, the same, but all the behavior for edge cases is wrong. The reason for that, by the way, is that initially the Load and LoadAsync impl was functionality the same, but the Load behavior kept getting more sophisticated, and I kept forgetting to also update the LoadAsync behavior.

When I started building support for batches, this really stumped me. The last thing that I wanted to do is to either try to maintain complex logic in three different location or have different behaviors depending if you were using a direct call, a batch or async call. Just trying to document that gave me a headache.

How would you approach solving this problem?


08/02/2011 09:22 AM by

Without having seen the rest of the code, the description of the problem screems 'strategy pattern' to me. If I assume that LoadInternal is used by all three implementations, settings a delegate (which is called on the marked line) could do the job.

Whether this is the correct solutions depends ofcourse on more than just the problem you've try to solve.

08/02/2011 09:49 AM by

Besides using a delegate as Dave recommended, I would consider having just an async version and using a blocking wait around the call in the synchronous case. That makes for 99% code sharing.

08/02/2011 11:26 AM by

Use lazy enumerable, like in the result of Future from NHibernate. Address the internal choice with a strategy, passing the rest of the code, which must be executed as a some-kind-of continuation. During iteration IEnumerable different actions can occur (the enumerable is produced via strategy, for instance AsynchGetStrategy : IGetStrategy). What do you think about it?

Duarte Nunes
08/02/2011 04:03 PM by
Duarte Nunes

Off the top of my head, I would have something like

IEnumerator LoadInternal<T, M>(string id, string[] includes, 
                                                     Func<string, string[], M> cmd,
                                                     Func<M, MultiLoadResult> unwrapper);

And pump the enumerator either sync or asynchronously. I guess you can sophisticate this a bit to be able to return a typed IEnumerator. (This smells kind of monady).

Chris Wright
08/02/2011 11:34 PM by
Chris Wright

I'm surprised that the strategy pattern is worthy of a name. It's just method object + inheritance, no?

Alex Simkin
08/03/2011 11:26 AM by
Alex Simkin

@Chris Wright

Not only this pattern got a name, it even got two names. Strategy and State patterns are two names for the same programming construct.

Comments have been closed on this topic.