views:

154

answers:

2

My career started as a hard-core functional-paradigm developer (LISP), and now I'm a hard-core .net/C# developer. Of course I'm enamored with LINQ. However, I also believe in (1) using the right tool for the job and (2) preserving the KISS principle: of the 60+ engineers I work with, perhaps only 20% have hours of LINQ / functional paradigm experience, and 5% have 6 to 12 months of such experience. In short, I feel compelled to stay away from LINQ unless I'm hampered in achieving a goal without it (wherein replacing 3 lines of O-O code with one line of LINQ is not a "goal").

But now one of the engineers, having 12 months LINQ / functional-paradigm experience, is using LINQ to objects, or at least lambda expressions anyway, in every conceivable location in production code. My various appeals to the KISS principle have not yielded any results. Therefore...

What published studies can I next appeal to? What "coding standard" guideline have others concocted with some success? Are there published LINQ performance issues I could point out? In short, I'm trying to achieve my first goal - KISS - by indirect persuasion.

Of course this problem could be extended to countless other areas (such as overuse of extension methods). Perhaps there is an "uber" guide, highly regarded (e.g. published studies, etc), that takes a broader swing at this. Anything?

LATE EDIT: Wow! I got schooled! I agree I'm coming at this entirely wrong-headed. But as a clarification, please take a look below at sample code I'm actually seeing. Originally it compiled and worked, but its purpose is now irrelevant. Just go with the "feel" of it. Now that I'm revisiting this sample a half year later, I'm getting a very different picture of what is actually bothering me. But I'd like to have better eyes than mine make the comments.

//This looks like it was meant to become an extension method...
public class ExtensionOfThreadPool
{
    public static bool QueueUserWorkItem(Action callback)
    {
        return ThreadPool.QueueUserWorkItem((o) => callback());
    }
}

public class LoadBalancer
{
    //other methods and state variables have been stripped...

    void ThreadWorker()
    {
        // The following callbacks give us an easy way to control whether
        // we add additional headers around outbound WCF calls.
        Action<Action> WorkRunner = null;

        // This callback adds headers to each WCF call it scopes
        Action<Action> WorkRunnerAddHeaders = (Action action) =>
        {
            // Add the header to all outbound requests. 
            HttpRequestMessageProperty httpRequestMessage = new HttpRequestMessageProperty();
            httpRequestMessage.Headers.Add("user-agent", "Endpoint Service");

            // Open an operation scope - any WCF calls in this scope will add the
            // headers above.
            using (OperationContextScope scope = new OperationContextScope(_edsProxy.InnerChannel))
            {
                // Seed the agent id header
                OperationContext.Current.OutgoingMessageProperties[HttpRequestMessageProperty.Name] = httpRequestMessage;

                // Activate
                action();
            }
        };

        // This callback does not add any headers to each WCF call
        Action<Action> WorkRunnerNoHeaders = (Action action) =>
        {
            action();
        };

        // Assign the work runner we want based on the userWCFHeaders
        // flag.
        WorkRunner = _userWCFHeaders ? WorkRunnerAddHeaders : WorkRunnerNoHeaders;

        // This outter try/catch exists simply to dispose of the client connection
        try
        {
            Action Exercise = () =>
            {
                // This worker thread polls a work list
                Action Driver = null;
                Driver = () =>
                {
                    LoadRunnerModel currentModel = null;
                    try
                    {
                        // random starting value, it matters little
                        int minSleepPeriod = 10;
                        int sleepPeriod = minSleepPeriod;

                        // Loop infinitely or until stop signals
                        while (!_workerStopSig)
                        {
                            // Sleep the minimum period of time to service the next element
                            Thread.Sleep(sleepPeriod);

                            // Grab a safe copy of the element list
                            LoadRunnerModel[] elements = null;
                            _pointModelsLock.Read(() => elements = _endpoints);

                            DateTime now = DateTime.Now;
                            var pointsReadyToSend = elements.Where
                                (
                                    point => point.InterlockedRead(() => point.Live && (point.GoLive <= now))
                                ).ToArray();

                            // Get a list of all the points that are not ready to send
                            var pointsNotReadyToSend = elements.Except(pointsReadyToSend).ToArray();

                            // Walk each model - we touch each one inside a lock
                            // since there can be other threads operating on the model
                            // including timeouts and returning WCF calls.
                            pointsReadyToSend.ForEach
                            (
                                model =>
                                {
                                    model.Write
                                    (
                                        () =>
                                        {
                                            // Keep a record of the current model in case
                                            // it throws an exception while we're staging it
                                            currentModel = model;

                                            // Lower the live flag (if we crash calling
                                            // BeginXXX the catch code will re-start us)
                                            model.Live = false;

                                            // Get the step for this model
                                            ScenarioStep step = model.Scenario.Steps.Current;

                                            // This helper enables the scenario watchdog if a
                                            // scenario is just starting
                                            Action StartScenario = () =>
                                            {
                                                if (step.IsFirstStep && !model.Scenario.EnableWatchdog)
                                                {
                                                    model.ScenarioStarted = now;
                                                    model.Scenario.EnableWatchdog = true;
                                                }
                                            };

                                            // make a connection (if needed)
                                            if (step.UseHook && !model.HookAttached)
                                            {
                                                BeginReceiveEventWindow(model, step.HookMode == ScenarioStep.HookType.Polled);
                                                step.RecordHistory("LoadRunner: Staged Harpoon");
                                                StartScenario();
                                            }

                                            // Send/Receive (if needed)
                                            if (step.ReadyToSend)
                                            {
                                                BeginSendLoop(model);
                                                step.RecordHistory("LoadRunner: Staged SendLoop");
                                                StartScenario();
                                            }

                                        }
                                    );
                                }
                                , () => _workerStopSig
                            );

                            // Sleep until the next point goes active. Figure out
                            // the shortest sleep period we have - that's how long
                            // we'll sleep.
                            if (pointsNotReadyToSend.Count() > 0)
                            {
                                var smallest = pointsNotReadyToSend.Min(ping => ping.GoLive);
                                sleepPeriod = (smallest > now) ? (int)(smallest - now).TotalMilliseconds : minSleepPeriod;
                                sleepPeriod = sleepPeriod < 0 ? minSleepPeriod : sleepPeriod;
                            }
                            else
                                sleepPeriod = minSleepPeriod;
                        }
                    }
                    catch (Exception eWorker)
                    {
                        // Don't recover if we're shutting down anyway
                        if (_workerStopSig)
                            return;

                        Action RebootDriver = () =>
                        {
                            // Reset the point SendLoop that barfed
                            Stagepoint(true, currentModel);

                            // Re-boot this thread
                            ExtensionOfThreadPool.QueueUserWorkItem(Driver);
                        };

                        // This means SendLoop barfed
                        if (eWorker is BeginSendLoopException)
                        {
                            Interlocked.Increment(ref _beginHookErrors);
                            currentModel.Write(() => currentModel.HookAttached = false);
                            RebootDriver();
                        }
                        // This means BeginSendAndReceive barfed
                        else if (eWorker is BeginSendLoopException)
                        {
                            Interlocked.Increment(ref _beginSendLoopErrors);
                            RebootDriver();
                        }
                        // The only kind of exceptions we expect are the 
                        // BeginXXX type. If we made it here something else bad
                        // happened so allow the worker to die completely.
                        else
                            throw;
                    }
                };

                // Start the driver thread. This thread will poll the point list
                // and keep shoveling them out
                ExtensionOfThreadPool.QueueUserWorkItem(Driver);

                // Wait for the stop signal
                _workerStop.WaitOne();

            };

            // Start
            WorkRunner(Exercise);
        }
        catch(Exception ex){//not shown}
    }
}
+6  A: 

You seem to be equating Linq to objects with greater complexity, because you assume that unnecessary use of it violates "keep it simple, stupid".

All my experience has been the opposite: it makes complex algorithms much simpler to write and read.

On the contrary, I now regard imperative, statement-based, state-mutational programming as the "risky" option to be used only when really necessary.

So I'd suggest that you put effort into getting more of your colleagues to understand the benefit. It's a false economy to try to limit your approaches to those that you (and others) already understand, because in this industry it pays huge dividends to stay in touch with "new" practises (of course, this stuff is hardly new, but as you point out, it's new to many from a Java or C# 1.x background).

As for trying to pin some charge of "performance issues" on it, I don't think you're going to have much luck. The overhead involved in Linq-to-objects itself is minuscule.

Daniel Earwicker
+11  A: 

Well, it sounds to me like you're the one wanting to make the code more complicated - because you believe your colleagues aren't up to the genuinely simple approach. In many, many cases I find LINQ to Objects makes the code simpler - and yes that does include changing just a few lines to one:

int count = 0;
foreach (Foo f in GenerateFoos())
{
    count++;
}

becoming

int count = GenerateFoos().Count();

for example.

Where it isn't making the code simpler, it's fine to try to steer him away from LINQ - but the above is an example where you certainly aren't significantly hampered by avoiding LINQ, but the "KISS" code is clearly the LINQ code.

It sounds like your company could benefit from training up its engineers to take advantage of LINQ to Objects, rather than trying to always appeal to the lowest common denominator.

Jon Skeet
I agree. The engineers need to dive into LINQ and lambda goodness. Not for everything, of course, but nothing good can come from avoiding it forever. The company will fall behind the curve, as will the employees.
Anthony Pegram
I'm sold - except that pre-existing BCL extension methods (such as Count() you demonstrated) is not the issue for me. Instead, what do you think of a logging signature that deliberately takes only lambdas instead of strings://assume ~10000 occurrences of this example throughout code...Logger.Log( () => "Hello" ); //or...Logger.Log( () => String.Format("hey {0}","there") );The purpose is to achieve a (questionable) performance boost if logging is off.
Brent Arias
@Mystagogue: Actually, I was talking to colleagues yesterday about deferring argument evaluation in logging statements. It's a good idea when there are no side effects, as it means you can conditionally log potentially expensive-to-format details, knowing that the evaluation won't take place unless detailed logging is enabled. I prefer that to `if (Logger.IsEnabled(info)) { Logger.Info(...); }` which is the common alternative.
Jon Skeet