Attention all Excel Experts

[northernstar]

thought the title may attract a little attention

i have created a form which involves a lot of coding so far and i am only about halfway through completing the form.....what i have done works and appears to be ok

but i dont think i have done it in the correct or best format, would just like some advice and comments on it please

the password to get into the code is 2729.........use then forget...otherwise i will have to kill you....hahahahaha!!!!!

enough of the funny jokes

please find attached a copy of the file

thanks in advance

[Bob Phillips]

There are just too many things that we could pick up on, but tackling them in isolation is fruitless, and there is just too much.

Pick one particular item that you are not happy with, and tack;le that. Then maybe the next, the ... one stepo at a time.

PS PNE did well tonight, shame it was against my mate's team.

[tpoynton]

I didnt look at the code, but I found this article (http://vbaexpress.com/forum/showthread.php?t=9882) extremely useful for helping me get a start on making my code more efficient. Then, as I struggled with other things, I posted questions here.

[mdmackillop]

Here's a small start.
Use Select Case instead of multiple If statements
If you're going to repeat code, pass it to another sub to handle. You then only have one bit to maintain.#
eg

Private Sub CommandButton17_Click()
Select Case DecimalPlaces
Case 0
Call FixDec("0")
Case 1
Call FixDec("0.0")
'etc.
End Select
End Sub

Private Sub FixDec(Dec as String)
For i = 1 To 11
Controls("Reading" & i) = Format(Controls("Reading" & i), Dec)
Next
End Sub

[Norie]

Here's another hint.

This can be replaced,

Private Sub Reading11ba_BeforeUpdate(ByVal Cancel As MSForms.ReturnBoolean)
If DecimalPlaces = 0 Then
Reading11ba = Format(Reading11ba, "0")
End If
If DecimalPlaces = 1 Then
Reading11ba = Format(Reading11ba, "0.0")
End If
If DecimalPlaces = 2 Then
Reading11ba = Format(Reading11ba, "0.00")
End If
If DecimalPlaces = 3 Then
Reading11ba = Format(Reading11ba, "0.000")
End If
If DecimalPlaces = 4 Then
Reading11ba = Format(Reading11ba, "0.0000")
End If
If DecimalPlaces = 5 Then
Reading11ba = Format(Reading11ba, "0.00000")
End If
If DecimalPlaces = 6 Then
Reading11ba = Format(Reading11ba, "0.000000")
End If
End Sub
with this,

Private Sub Reading11ba_BeforeUpdate(ByVal Cancel As MSForms.ReturnBoolean)

If DecimalPlaces = 0 Then
Reading11ba = Format(Reading11ba, "0")
Else
Reading11ba = Format(Reading11ba, "0." & String(DecimalPlaces, "0"))
End If
End Sub

[northernstar]

thanks for all your hints and comments

not sure i understand them all (or any of them fully) but i will have another look at them and let you know

thanks again

i have another problem which i will post in a new thread

[northernstar]

even though i have been using vba for a couple of years now i and still very new to it and i dont use it all the time

i have a problem getting from a to b without going via c d e f and even sometimes g.........i nearly always get there in the end

after taking on board your comments xld the part i am really unhappy with is that i have to format a number of text boxes depending on the result of a combobox (DecimalPlaces) this will need repeating for about 50 textboxes which is a hell of alot code

would really appreciate some ideas on this

Private Sub Reading1b_BeforeUpdate(ByVal Cancel As MSForms.ReturnBoolean)
If DecimalPlaces = 0 Then
Reading1b = Format(Reading1b, "0")
End If
If DecimalPlaces = 1 Then
Reading1b = Format(Reading1b, "0.0")
End If
If DecimalPlaces = 2 Then
Reading1b = Format(Reading1b, "0.00")
End If
If DecimalPlaces = 3 Then
Reading1b = Format(Reading1b, "0.000")
End If
If DecimalPlaces = 4 Then
Reading1b = Format(Reading1b, "0.0000")
End If
If DecimalPlaces = 5 Then
Reading1b = Format(Reading1b, "0.00000")
End If
If DecimalPlaces = 6 Then
Reading1b = Format(Reading1b, "0.000000")
End If
End Sub

[Bob Phillips]

Private Sub Reading1b_BeforeUpdate(ByVal Cancel As MSForms.ReturnBoolean)

Call FormatTextBox(Me.Reading1b)
End Sub

Private Sub FormatTextBox(ByRef tb As MSForms.TextBox)

If DecimalPlaces = 0 Then
tb = Format(tb, "0")
End If
If DecimalPlaces = 1 Then
tb = Format(tb, "0.0")
End If
If DecimalPlaces = 2 Then
tb = Format(tb, "0.00")
End If
If DecimalPlaces = 3 Then
tb = Format(tb, "0.000")
End If
If DecimalPlaces = 4 Then
tb = Format(tb, "0.0000")
End If
If DecimalPlaces = 5 Then
tb = Format(tb, "0.00000")
End If
If DecimalPlaces = 6 Then
tb = Format(tb, "0.000000")
End If
End Sub


or better, incorporating Norie's previous suggestion



Private Sub Reading1b_BeforeUpdate(ByVal Cancel As MSForms.ReturnBoolean)

Call FormatTextBox(Me.Reading1b)
End Sub

Private Sub FormatTextBox(ByRef tb As MSForms.TextBox)

If DecimalPlaces = 0 Then
tb = Format(tb, "0")
Else
tb = Format(tb, "0." & String(DecimalPlaces, "0"))
End If
End Sub

[northernstar]

and i can use this code for every textbox on the form and just change the (Me.Reading1b) statement to refer to the different textboxes

i will try this while i wait for a reply

thanks again