[RESOLVED] How can I refactor this code?

[wolf99]

I have some code that works fine but I currently have the same code three times, I'd like to refactor to a single function that is called from three places instead.

The current code has access to a List(Of Sample) where a Sample object contains a sample reading for three axes x, y & z . The code calculates the mean average of the samples and subtracts this from each sample before then calculating the RMS (root mean square) of a given axis from the sample List.

This code is executed separately for each axis e.g. just the X axis, as follows

Public Function CalcRmsX() As Double
    Dim Rms As Double = 0
    Dim Mean As Double = 0
    Dim Count As UInteger = _samples.Count()
    If Count > 0 Then
        Mean = _samples.Sum(Function(smpl) smpl.X) 'Take the sum of each sample
        Mean = Mean / Count 'Divide for the mean
        For Each smpl As Sample In _samples 'Subtract the mean from each sample to remove any bias
            smpl.X = smpl.X - Mean
        Next
        Rms = _samples.Sum(Function(smpl) smpl.X ^ 2) 'Take the sum of the square of each sample
        Rms = Rms / Count 'Divide for the mean square
        Rms = Math.Sqrt(Rms) 'Take the sqare root of the mean square
    End If
    Return Rms
End Function

I'd like to end up with a function where a function call would instead look as follows
[HIGHLIGHT=vb]
Dim RmsX As Double = CalcRms(X) 'Refactor to pass the axis as an argument like this
Dim RmsX As Double = CalcXRms() 'Rather than calling a different function per axis like this
[HIGHLIGHT=vb]

But my VB.NET foo is not strong enough to know how to change the function to make a function argument (the axis) select an object member (the Sample member). Is this possible?

Thanks, T

[Evil_Giraffe]

You can pass in an object that represents a function taking a Sample and return a Double. I know you know how to do this because you've already done it in your code, although it's possible you didn't notice yourself doing so.

If you turned that function into a variable, and then used the variable throughout your method where you are currently writing smpl.X, and then make the variable a parameter, you will push the definition of that functoin out of this method, allowing you to vary what you pass in to select either .X, .Y or .Z.

Hope that points you in the right direction...

[#Destiny]

I think Evil_Giraffe is right.

[wolf99]

Hi Evil_Giraffe, #Destiny

True dat, I asked the internet how to `.Sum` and adapted what I found, but now you say it I see your point! I can pass a delegate (similar to function pointer right?) or lambda (same thing but "inline"?) that returns the same member from every sample so I end up with something along the lines of (modifying my OP code):

Add: Delegate Function fooDelegate(s As Sample) As Double 'Return type would suit assignment to the member for ln#9?
 
ln #1: Public Function CalcRms(selector As fooDelegate) As Double 'Not sure about arg type
 
ln #6: Mean = _samples.Sum(selector ()) 'Not sure about the arg to selector ?
 
ln #8,9,10: selector () = selector () - Mean 'Again, not sure about args to selector or assigning to selector return value
 
ln #11: Rms = _samples.Sum(selector () ^ 2) 'Again with the args to selector

Then an example call would be: Dim RmsX As Double = CalcRms(Function(smpl) smpl.X)

On the right track? If so, arg should I pass to the delegate when it is used? Or is it picked up from the object that .Sum is being called on? What about ln #9 where there is no sum, is that selector (_samples)?

Thanks!!



EDIT:

I'm messing around with the following for the first bit, but no joy just yet:

Delegate Function myDelegateType(s As Sample) As Double
 
Public Function CalcRms(selector As myDelegateType) As Double
    Dim Mean As Double = _samples.Sum(selector) 'VS doesn't like this, I've tried several permutations
    '...
End Function
 
'e.g. Call: CalcRms(Function(smpl) smpl.X)

[Evil_Giraffe]

A few points.

1) You don't need to define an explicitly named delegate for this, you can represent it as a function that takes a Sample and returns a Double, there is the Function(Of T1, TResult) that you can use:

Public Function CalcRms(selector As Function(Of Sample, Double)) As Double

2) You can pass selector directly to Sum

Mean = _samples.Sum(selector)

3) For line 9, you can't use your selector function to write to that property. It isn't returning the property itself, but the value of it, so you need a slightly different approach. Here's where I'd suggest the use of .Select() - what this function does is take an IEnumerable of a given type (in this case you'd want it to be your _samples array) and a function from the input enumerable type to some other type, it then returns an IEnumerable of that other type, that when you enumerate it gives you the result of applying that function to each of the elements from the input IEnumerable. Confused? Perhaps an example is in order:

An IEnumerable should be thought of as a sequence of values. You might have them in an array, a List, or they may not be stored anywhere and calculated as needed. All you can do with a sequence is walk its elements one by one. You might have an array of integers, for example (whoops this may not be valid VB syntax):

Dim input () As Integer = { 1, 2, 3, 4, 5 }

You can access any of the integers in your array by index. That same array can be treated as an IEnumerable(Of Integer). When you treat it like that, you can't access the fourth element directly, all you can do is access all the elements in order. You can only go 1 -> 2 -> 3 -> 4 -> 5.

However, there are lots of functions defined that work on IEnumerables and turn them into something else. You've already used one: Sum(). What Sum does is walk those elements one-by-one (it can't break the rules either) and adds the numbers up as it goes. It is defined to only work on numeric types for that reason.
Select() is a little different, but a more fundamental building block of working with sequences. Say we've got a function from integer to integer that simply multiplies the input by two:

Dim multiplyByTwo As Function(Of Integer, Integer) = Function(x) x * 2

You can pass that to Select and it will give you a new sequence back that is the result of applying that function to every element of your input:
multiplyByTwo(1) -> multiplyByTwo(2) -> multiplyByTwo(3) -> multiplyByTwo(4) -> multiplyByTwo(5)
= 2 -> 4 -> 6 -> 8 -> 10

Crucially, because it returns you another sequence, you can apply more transformation functions to the sequence you got back. Want to know the Sum() of your new sequence? Easy:

input.Select(multiplyByTwo).Sum()

This should give you an inkling that you don't actually need to write to the .X property when you subtract the Mean. You can simply create a new sequence of Doubles where the Mean has been subtracted away.


It should further give you an inkling that maybe you don't necessarily need to tie the method that calculates RMS to the concept of your Sample...
If you can take a sequence of Samples, and from that produce a sequence of Doubles that represent the X property of the samples... why does a function dedicated to calculating the RMS of a sequence of Doubles need to know about Samples?


What I hope you picked up from the number of times I used the word is that getting your head around the idea of a sequence opens up some very powerful ways of expressing your intent.

[wolf99]

Brilliant Evil_giraffe, thank you for your detailed and well thought post!

I now have the following code:

Public Function CalcRms(Of TSource)(input As List(Of TSource), selector As Func(Of TSource, Double), subtractDcComponent As Boolean) As Double
    Dim Mean As Double = 0
    Dim Rms As Double = 0
    Dim Count As UInteger = input.Count()
    Dim Values As List(Of Double)
    If Count > 0 Then
        values = New List(Of Double)(input.Select(selector)) 'Create a list of doubles using the selector function
        If subtractDcComponent Then
            Mean = Values.Sum() / Count 'Divide the sum of the selected values for the mean
            Values = Values.Select(Function(vlu) vlu - Mean) 'Subtract the mean from each selected value to remove any bias
        End If
        Rms = Values.Sum(Function(vlu) vlu ^ 2) / Count 'Divide the sum of the values squared for the mean square
        Rms = Math.Sqrt(Rms) 'Take the square root of the mean square
    End If
    Return Rms
End Function
 
' Example call
CalcRms(Of Sample) (_samples, (Function(smpl) smpl.X))

However I get an exception at ln #10, I guess this is because I am trying to assign to the values list as it is being read? Should be able to resort back to a simple for loop here though

For i As Integer = 0 To Values.Count -1
                Values(i) = Values(i) - Mean
            Next

[Evil_Giraffe]

The exception is because you are trying to assign an IEnumerable(Of Double) to a variable that holds a List(Of Double). You could convert the sequence to a list by calling ToList on it, but why bother? You only use those values as a sequence further on, so you can keep it as a sequence. What I'd consider doing is take the selector function out of this method and make the caller responsible for providing a sequence of Doubles:

CalcRMS(_samples.Select(Function(smpl) smpl.X))

Your signature can then become:

Public Function CalcRMS(input As IEnumerable(Of Double)) As Double

Next, think about what you're calculating. First you need to know how many items there are in the sequence. You've got this already:

Dim count = input.Count()

Then you calculate the mean of the sequence. That is the sum of the sequence divided by the count. You've created a List of the values to calculate this on. Even when you were taking responsibility for selecting the value, this wasn't necessary - you don't need a list, it is all defined in terms of the sequence. You can re-enumerate this sequence without worry because you know it's an array or list or whatever of Samples and that the projection function to get the .X is just a property access.

Dim mean = input.Sum() / count

That's all you need!
Now, the next bit you can do a couple of ways that are equivalent, it's up to you which looks neater to you. You can either use .Select to generate a sequence of the squares and then Sum that sequence, OR you can pass the function make the squares to the Sum function:

Dim meanSquare = input.Select(Function(x) (x - mean) ^ 2).Sum() / count

OR

Dim meanSquare = input.Sum(Function(x) (x - mean) ^ 2) / count

As you can see, there is no need to be writing any new lists or altering properties on Samples or anything like that, it is pure functions returning values.
You can even break up the steps of subtracting the mean and squaring if it made more sense to you:

input.Select(Function(x) x - mean)
     .Select(Function(x) x ^ 2)
     .Sum()

[wolf99]

More light dawns!

Public Function CalcRms(input As IEnumerable(Of Double), removeDcComponent As Boolean) As Double
    Dim Count As UInteger = input.Count()
    Dim MeanSquare As Double = 0
    Dim Rms As Double = 0
    If Count > 0 Then
        If removeDcComponent Then
            Dim Mean As Double = input.Sum() / Count 'Sum all input values and divide for the mean
            MeanSquare = input.Sum(Function(v) (v - Mean) ^ 2) / Count 'Subtract mean from input values and calculate mean square
        Else
            MeanSquare = input.Sum(Function(v) v ^ 2) / Count 'Calculate mean square of input values
        End If
        Rms = Math.Sqrt(MeanSquare) 'Take square root of mean squre for root mean square
    End If
    Return Rms
End Function
 
'Example call:
CalcRms(_samples.Select(Function(smpl) smpl.X), True)

Wow, that's a lot simpler looking! Thanks again!!

PS: This has also helped me on my snails-pace investigation of delegate and lambdas! B)