Function Simpification

[vincentzack]

Could anyone help to simplify the follow code? Thanks


Function ReCent(Depth, Cover, Dia, No_Dia, IDia, No_IDia, No_Layer, Link, Layer)

If Layer = 1 Then
Cover = Cover + Dia
End If


AO = No_Dia * Dia ^ 2 * 3.14 / 4
AI = No_IDia * IDia ^ 2 * 3.14 / 4


d1 = Depth - Cover - Link - Dia / 2


d2 = Depth - Cover - Link - 2 * Dia - IDia / 2


d3 = Depth - Cover - Link - 4 * Dia - IDia / 2


d4 = Depth - Cover - Link - 6 * Dia - IDia / 2


d5 = Depth - Cover - Link - 8 * Dia - IDia / 2


d6 = Depth - Cover - Link - 10 * Dia - IDia / 2


d7 = Depth - Cover - Link - 12 * Dia - IDia / 2


d8 = Depth - Cover - Link - 14 * Dia - IDia / 2


d9 = Depth - Cover - Link - 16 * Dia - IDia / 2


d10 = Depth - Cover - Link - 18 * Dia - IDia / 2


If No_Layer = 1 Then
ReCent = d1
ElseIf No_Layer = 2 Then
ReCent = (d1 * AO + d2 * AI) / (AO + AI)
ElseIf No_Layer = 3 Then
ReCent = (d1 * AO + d2 * AO + d3 * AI) / (AO + AO + AI)
ElseIf No_Layer = 4 Then
ReCent = (d1 * AO + d2 * AO + d3 * AO + d4 * AI) / (AO + AO + AO + AI)
ElseIf No_Layer = 5 Then
ReCent = (d1 * AO + d2 * AO + d3 * AO + d4 * AO + d5 * AI) / (AO + AO + AO + AO + AI)
ElseIf No_Layer = 6 Then
ReCent = (d1 * AO + d2 * AO + d3 * AO + d4 * AO + d5 * AO + d6 * AI) / (AO + AO + AO + AO + AO + AI)
ElseIf No_Layer = 7 Then
ReCent = (d1 * AO + d2 * AO + d3 * AO + d4 * AO + d5 * AO + d6 * AO + d7 * AI) / (AO + AO + AO + AO + AO + AO + AI)
ElseIf No_Layer = 8 Then
ReCent = (d1 * AO + d2 * AO + d3 * AO + d4 * AO + d5 * AO + d6 * AO + d7 * AO + d8 * AI) / (AO + AO + AO + AO + AO + AO + AO + AI)
ElseIf No_Layer = 9 Then
ReCent = (d1 * AO + d2 * AO + d3 * AO + d4 * AO + d5 * AO + d6 * AO + d7 * AO + d8 * AO + d9 * AI) / (AO + AO + AO + AO + AO + AO + AO + AO + AI)
ElseIf No_Layer = 10 Then
ReCent = (d1 * AO + d2 * AO + d3 * AO + d4 * AO + d5 * AO + d6 * AO + d7 * AO + d8 * AO + d9 * AO + d10 * AOI) / (AO + AO + AO + AO + AO + AO + AO + AO + AO + AI)
End If


End Function

[Paul_Hossler]

Still some simplifications, but Recent_1() is a starting point

There was not test data so I just made some up

I think you have 2 typos marked in your version with ---------------------------------
I changed to what I thought you meant, but it's easy to change

I'm sure someone will come up with a way to do it one line, but I wanted to go for readability and maintainability, instead of brevity




Option Explicit
Function ReCent_1(Depth As Double, Cover As Double, Dia As Double, No_Dia As Double, IDia As Double, No_IDia As Double, No_Layer As Long, Link As Double, Layer As Double) As Double
Dim AO As Double, AI As Double
Dim d(1 To 10) As Double
Dim i As Long
Dim Denom As Double, Numer As Double

If Layer = 1 Then Cover = Cover + Dia

AO = No_Dia * Dia ^ 2 * 3.14 / 4
AI = No_IDia * IDia ^ 2 * 3.14 / 4

'Used d1 = Depth - Cover - Link - IDia / 2
For i = 1 To 10
d(i) = Depth - Cover - Link - (2 * (i - 1) * Dia) - (IDia / 2)
Next i

If No_Layer = 1 Then
ReCent_1 = d(1)

Else
For i = 1 To No_Layer - 1
Numer = Numer + d(i) * AO
Next i
Numer = Numer + d(No_Layer) * AI

Denom = (No_Layer - 1) * AO + AI

ReCent_1 = Numer / Denom
End If

End Function
Function ReCent(Depth, Cover, Dia, No_Dia, IDia, No_IDia, No_Layer, Link, Layer)
Dim AO, AI, d1, d2, d3, d4, d5, d6, d7, d8, d9, d10
If Layer = 1 Then
Cover = Cover + Dia
End If


AO = No_Dia * Dia ^ 2 * 3.14 / 4
AI = No_IDia * IDia ^ 2 * 3.14 / 4

'----------------- d1 a special case or typo??
' you have -Dia/2, rest are -IDia/2
' d1 = Depth - Cover - Link - 0 * Dia - IDia / 2
d1 = Depth - Cover - Link - IDia / 2 ' I changed to IDia
d2 = Depth - Cover - Link - 2 * Dia - IDia / 2
d3 = Depth - Cover - Link - 4 * Dia - IDia / 2
d4 = Depth - Cover - Link - 6 * Dia - IDia / 2
d5 = Depth - Cover - Link - 8 * Dia - IDia / 2
d6 = Depth - Cover - Link - 10 * Dia - IDia / 2
d7 = Depth - Cover - Link - 12 * Dia - IDia / 2
d8 = Depth - Cover - Link - 14 * Dia - IDia / 2
d9 = Depth - Cover - Link - 16 * Dia - IDia / 2
d10 = Depth - Cover - Link - 18 * Dia - IDia / 2


If No_Layer = 1 Then
ReCent = d1
ElseIf No_Layer = 2 Then
ReCent = (d1 * AO + d2 * AI) / (AO + AI)
ElseIf No_Layer = 3 Then
ReCent = (d1 * AO + d2 * AO + d3 * AI) / (AO + AO + AI)
ElseIf No_Layer = 4 Then
ReCent = (d1 * AO + d2 * AO + d3 * AO + d4 * AI) / (AO + AO + AO + AI)
ElseIf No_Layer = 5 Then
ReCent = (d1 * AO + d2 * AO + d3 * AO + d4 * AO + d5 * AI) / (AO + AO + AO + AO + AI)
ElseIf No_Layer = 6 Then
ReCent = (d1 * AO + d2 * AO + d3 * AO + d4 * AO + d5 * AO + d6 * AI) / (AO + AO + AO + AO + AO + AI)
ElseIf No_Layer = 7 Then
ReCent = (d1 * AO + d2 * AO + d3 * AO + d4 * AO + d5 * AO + d6 * AO + d7 * AI) / (AO + AO + AO + AO + AO + AO + AI)
ElseIf No_Layer = 8 Then
ReCent = (d1 * AO + d2 * AO + d3 * AO + d4 * AO + d5 * AO + d6 * AO + d7 * AO + d8 * AI) / (AO + AO + AO + AO + AO + AO + AO + AI)
ElseIf No_Layer = 9 Then
ReCent = (d1 * AO + d2 * AO + d3 * AO + d4 * AO + d5 * AO + d6 * AO + d7 * AO + d8 * AO + d9 * AI) / (AO + AO + AO + AO + AO + AO + AO + AO + AI)
ElseIf No_Layer = 10 Then
'----------------------------------------------------------- Typo you had .... d10 * AOI --- always use Option Explicit
ReCent = (d1 * AO + d2 * AO + d3 * AO + d4 * AO + d5 * AO + d6 * AO + d7 * AO + d8 * AO + d9 * AO + d10 * AI) / (AO + AO + AO + AO + AO + AO + AO + AO + AO + AI)
End If


End Function

[p45cal]

There's an error here right?:

ElseIf No_Layer = 10 Then
ReCent = (d1 * AO + d2 * AO + d3 * AO + d4 * AO + d5 * AO + d6 * AO + d7 * AO + d8 * AO + d9 * AO + d10 * AOI) / (AO + AO + AO + AO + AO + AO + AO + AO + AO + AI)
End If

edit post posting: I see Paul's spotted that possible error, I'll see what you say about the other one.
In the meantime, the following (is it really a simplification?) gives the same results in my limited testing:
Function ReCent(Depth, Cover, Dia, No_Dia, IDia, No_IDia, No_Layer, Link, Layer)
If Layer = 1 Then
Cover = Cover + Dia
End If
AO = No_Dia * Dia ^ 2 * 3.14 / 4
AI = No_IDia * IDia ^ 2 * 3.14 / 4

ReDim d(1 To 10)
d(1) = Depth - Cover - Link - Dia / 2
For i = 2 To 10
d(i) = Depth - Cover - Link - ((i - 1) * 2) * Dia - IDia / 2
Next i

If No_Layer = 1 Then
ReCent = d(1)
Else
For i = 1 To No_Layer
numerator = numerator + d(i) * IIf(i = No_Layer, AI, AO)
Next i
ReCent = numerator / ((No_Layer - 1) * AO + AI)
End If
End Function

If you can describe to us what it's actually calculating we might be able to make a simpler function from scratch - reverse engineering the code is too much for me to attempt!

ps. Paul, with code such as:
(d1 * AO + d2 * AO + d3 * AO + d4 * AO + d5 * AO + d6 * AO + d7 * AO + d8 * AO + d9 * AO
I really want to find a way to use SumProduct, then we might get closer to a one-liner

[p45cal]

You've cross posted this - do have a read of http://www.excelguru.ca/content.php?184

[Paul_Hossler]

I really want to find a way to use SumProduct, then we might get closer to a one-liner

I had started down that road a little, but found that setting up the arrays was more complicated (for me at least) than just using a more brute force approach

You have to handle the different No_Level values, and the last value in the string being AI, all others AO, Would like to see if you can do it


I'm personally not a fan of 'one liners'

Since the machine has to use the same number of cycles either way, I use the philosophy that I'm writing for the reader (most likely going to be me in 6 months), and not the computer

a. A variable called 'pageCounter' will mean more to me in 6 months than a variable called 'p', even if it required more typing

b. 10 steps on 10 lines is easier for me to follow and debug than 10 steps on 1 line

c. There's a middle ground, e.g. the typo on the d10 term was caused I'm sure by over enthusiastic copy/paste, so a loop would be less error prone so in that case shorter is better

Just my opinion

[p45cal]

Would like to see if you can do it I may try, but I won't be posting anything until we get feedback from our (cross-posting) OP.