Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I'm new to the functional programming concepts in C#, but I have some experience with higher order functions via Haskell, Scala, Python and Ruby. I'm currently going through an old .NET 2.0 codebase with a LOT of duplication in it. Most of it is simple "copy/paste" with a few variables changed. I'm trying to condense as much of this as possible.

A chunk of code within a routine I'm working on now goes a bit like this, for example:

if (PostProcess.UsesFirstPass)
{
    w.WriteLine(V1 + " = 0" + " !! SET FOO");
    w.WriteLine(V2 + " = 0" + " !! SET BAR");
    w.WriteLine(V3 + " = 0" + " !! SET BAZ");
    if (PostProcess.IsSpecial){
        w.WriteLine(V3 + " = 20" + " !! INCREASE BAZ");
    }
    w.WriteLine(V4 + " = 0" + " !! SET QUUX");
}
if (PostProcess.UsesSecondPass)
{
    w.WriteLine(V5 + " = 0" + " !! SET FOO");
    w.WriteLine(V6 + " = 0" + " !! SET BAR");
    w.WriteLine(V7 + " = 0" + " !! SET BAZ");
    if (PostProcess.IsSpecial){
        w.WriteLine(V7 + " = 20" + " !! INCREASE BAZ");
    }
    w.WriteLine(V8 + " = 0" + " !! SET QUUX");
}
if (PostProcess.UsesFinalPass)
{
    w.WriteLine(V9 + " = 0" + " !! SET FOO");
    w.WriteLine(V10 + " = 0" + " !! SET BAR");
    w.WriteLine(V11 + " = 0" + " !! SET BAZ");
    if (PostProcess.IsSpecial){
        w.WriteLine(V11 + " = 20" + " !! INCREASE BAZ");
    }
    w.WriteLine(V12 + " = 0" + " !! SET QUUX");
}

Where any of the "V"s you see in there is a string variable defined earlier. It's part of a fairly large method. I could write another method called, say, WriteVariables() and pass in the required info and call it three times, but it only ever applies to this section of method. I think this would be a place where writing an anonymous inner function would make sense.

I did it to test it out, and it seems to work fine. Now I have a single delegate that looks something like:

private delegate void WriteCutVars(string V1, 
    string V2, string V3, string V4, CuttingParameters cutParam);

// snip //

WriteCutVars WriteVars = (Va, Vb, Vc, Vd) => {
    w.WriteLine(Va + " = 0" + " !! SET FOO");
    w.WriteLine(Vb + " = 0" + " !! SET BAR");
    w.WriteLine(Vc + " = 0" + " !! SET BAZ");
    if (PostProcess.IsSpecial){
        w.WriteLine(Vc + " = 20" + " !! INCREASE BAZ");
    }
    w.WriteLine(Vd + " = 0" + " !! SET QUUX");
}
if (Process.UsesFirstPass) WriteVars(V1, V2, V3, V4);
if (Process.UsesSecondPass) WriteVars(V5, V6, V7, V8);
if (Process.UsesFinalPass) WriteVars(V9, V10, V11, V12);

My main questions:

  • Is this a proper use of inner functions?
  • Is this acceptable when I'm the only CS graduate on a team of mostly mechanical engineers?
share|improve this question
    
This might be problem with only this example. But why not use normal method? –  Euphoric Aug 10 '12 at 20:16
    
@Euphoric I guess that's what I'm trying to figure out. I could use one, but the object is already so bloated and huge, and I'd like to keep the scope of the function as local as possible to prevent additional classwide clutter. –  KChaloux Aug 10 '12 at 20:25
    
C# is OOP language. Not functional language. There should be nothing like "local function scope". If class is bloated, then fix that class. Don't try to bypass the problem. –  Euphoric Aug 10 '12 at 20:29
    
@Euphoric Fixing the class is what I'm trying to do. Unfortunately, the schedule they have me on doesn't give me time to completely rewrite it right now, and it's not my call to do a complete overhaul :( Just trying to clean up redundancy where I can so that it's more manageable when the bigger overhaul eventually comes a-knocking. –  KChaloux Aug 10 '12 at 21:00

3 Answers 3

up vote 6 down vote accepted

Your implementation seems reasonable to me.

However, if most of the other people developing the software have limited advanced programming experience you might want to use a regular function instead of an anonymous one since it will be more obvious what's going on.

When you're the senior developer and the rest of your team is at a significantly lower skill level; making sure they understand how your code works and can easily modify it if needed is an important consideration. Since your team mostly consists of non-programmers this is a larger consideration, since for programming techniques they probably congregate closer to the Use What I Know end of the scale than the Learn New Things Everyday end.

Breaking the repeated logic out and removing the copy/pasting should be simple to explain the benefits of. More advanced techniques like anonymous functions or linq would be a harder sell since they don't give major and obvious functional/maintenance improvements. The hardware engineers I've worked with in the past mostly only learned C in college since it was the language of choice for the micro-controllers in the gadgets they were building. With only a handful of classes their skills were concentrated on practical basic programming only, and not anything advanced.

share|improve this answer
    
Something might make a difference. The ultimate intent of the original designers is for me to take over maintenance and development of the project completely (I'm a junior programmer). The others want to move away from it. –  KChaloux Aug 10 '12 at 20:02
    
@KChaloux if you're taking primary ownership of the code you've got more latitude to update things. Unless you're confident the hardware people won't end up poking around in the code themselves when things don't behave as expected I'd still try to avoid doing anything that blows their minds. And despite your lack of experience (by years of work); you're the de facto senior developer. In terms of formal training most of your colleagues are probably the equivalent of first year CS students even if the school of hard knocks has since cured them of some of the worst bad student habits. –  Dan Neely Aug 10 '12 at 20:20
    
My direct supervisor, despite not having a CS degree, has actually been entrenched in the software side of things for the better half of a decade now. So while he may not have a degree in this particular field, he's knowledgeable enough at least. The other guys who write software here less so, but I'm not sure if they'll touch this code much. –  KChaloux Aug 10 '12 at 20:23

This is not a higher-order function, so it should be easily understandable to any C# programmer.

Although instead of using a custom delegate type, you could've used Action<string, string, string, CuttingParameters>.

But I think a normal method would have been fine here too, and is probably more idiomatic. Even if you won't use it anywhere else.

share|improve this answer
    
+1 For telling me about Action. Even if I end up not using the anonymous function, it's nice to know about. –  KChaloux Aug 10 '12 at 20:09

The key question is whether to use a "normal" private function or a strictly local function and the answer should stem from the same decision-making you'd use with a private field vs. a local variable: Is this thing useful anywhere but this function? In this case, it would appear not, so your approach seems correct.

There is an issue with using advanced functional techniques in C#, but C# is evolving to have more and more functional features, so you ought not avoid all functional techniques only because they may still be unfamiliar. I would say that this would actually be the type of use that would be good in a codebase where people were relatively unfamiliar with functional programming, since it is very straightforward in both utility and implementation.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.