Shortening code

[IanC]

I have the following code which disables CommandButton1 if correct
selections have not been made in all 6 comboboxes:

If Me.ComboBox1.MatchFound = False Then Me.CommandButton1.Enabled = False
If Me.ComboBox2.MatchFound = False Then Me.CommandButton1.Enabled = False
If Me.ComboBox3.MatchFound = False Then Me.CommandButton1.Enabled = False
If Me.ComboBox4.MatchFound = False Then Me.CommandButton1.Enabled = False
If Me.ComboBox5.MatchFound = False Then Me.CommandButton1.Enabled = False
If Me.ComboBox6.MatchFound = False Then Me.CommandButton1.Enabled = False

It works fine but I was wondering if it was possible to loop through each
ComboBox withouthaving to repeat the line. I'm looking for something like
"for each ComboBox in UserForm1 if MatchFound=False then disable
CommandButton1" but I don't know where to begin with it.

 

[IanC]

Thanks Harald. I was just wanting to tidy the code up a little, but if it's
going to make the code run more slowly I'll leave it as it is.

 

[GS]

I agree in principal with Harald's reply, but you could speed things up
a bit AND shorten the code like this:

With Me
.CommandButton1.Enabled = (.ComboBox1.MatchFound)
.CommandButton1.Enabled = (.ComboBox2.MatchFound)
.CommandButton1.Enabled = (.ComboBox3.MatchFound)
.CommandButton1.Enabled = (.ComboBox4.MatchFound)
.CommandButton1.Enabled = (.ComboBox5.MatchFound)
.CommandButton1.Enabled = (.ComboBox6.MatchFound)
End With

This skips the processing of the 'If' construct and assigns the value
directly to the command button.

Since the state of the button is the same as the return of MatchFound
then there's no need to query the return of MatchFound. If the state of
the button was to be the opposite of the return of MatchFound then the
code could be written like this:

<snip>
.CommandButton1.Enabled = (Not .ComboBox1.MatchFound)

HTH

 

[Martin Brown]

I have the following code which disables CommandButton1 if correct
selections have not been made in all 6 comboboxes:

If Me.ComboBox1.MatchFound = False Then Me.CommandButton1.Enabled = False
If Me.ComboBox2.MatchFound = False Then Me.CommandButton1.Enabled = False
If Me.ComboBox3.MatchFound = False Then Me.CommandButton1.Enabled = False
If Me.ComboBox4.MatchFound = False Then Me.CommandButton1.Enabled = False
If Me.ComboBox5.MatchFound = False Then Me.CommandButton1.Enabled = False
If Me.ComboBox6.MatchFound = False Then Me.CommandButton1.Enabled = False

It works fine but I was wondering if it was possible to loop through each
ComboBox withouthaving to repeat the line. I'm looking for something like
"for each ComboBox in UserForm1 if MatchFound=False then disable
CommandButton1" but I don't know where to begin with it.

You can speed it up a bit along the lines of nesting the tests

Me.CommandButton1.Enabled = False
If Me.ComboBox1.MatchFound Then
If Me.ComboBox2.MatchFound Then
If Me.ComboBox3.MatchFound Then
If Me.ComboBox4.MatchFound Then
If Me.ComboBox5.MatchFound Then
If Me.ComboBox6.MatchFound Then
Me.CommandButton1.Enabled = True
Endif
Endif
Endif
Endif
Endif
Endif

Though it might be better to use data validation in the worksheet to
prevent invalid input form ever getting this far.

Regards,
Martin Brown

 

[IanC]

Hi Garry

It hadn't occurred to me that I could set the instruction to disable
CommandButton1 directly from the MatchFound status. Makes sense really.

Thanks for that.

 

[IanC]

Hi Martin

Thanks for the pointers. I hadn't thought to nest the Ifs but I think I'm
going to go with garry's suggestion.

As for validation in the worksheet, there are 2 reasons why this wouldn't
work as things stand.
1. The data isn't written to the worksheet until CommandButton1 is clicked.
Not an insurmountable problem, but
2. Not all the data is going into the worksheet as entered. In some
circumstances the contents of 2 ComboBoxes may be combined and entered in
one cell and in other circumstances a ComboBox selection only affects the
response of worksheet code and never reaches the sheet at all.

Thanks for the suggestions.

 

[Martin Brown]

Hi Garry

It hadn't occurred to me that I could set the instruction to disable
CommandButton1 directly from the MatchFound status. Makes sense really.

Thanks for that.

However you should be aware that the code below is *not* functionally
equivalent to the code that you posted. It simplifies to:

Me.CommandButton1.Enable = Me.ComboBox6.MatchFound

You could write it as the logical AND of all the terms on the right hand
side and that might well be faster on some compilers and CPUs.
I would not like to bet on whether or not it is with VBA.

With Me
.CommandButton1.Enable = (.ComboBox1.MatchFound) And
(.Combobox2.MatchFound) And (...etc
End With

Would work as intended.

Regards,
Martin Brown

 

[IanC]

Hi Martin

However you should be aware that the code below is *not* functionally
equivalent to the code that you posted. It simplifies to:

Me.CommandButton1.Enable = Me.ComboBox6.MatchFound

Yes, I just discovered that!

You could write it as the logical AND of all the terms on the right hand
side and that might well be faster on some compilers and CPUs.
I would not like to bet on whether or not it is with VBA.

The length of time it takes is imperceptible. If there was an equivalent to
Debug.Print Now() which returned fractions of a second I could put a figure
on it, but to the human eye it's effectively instantaneous.

With Me
.CommandButton1.Enable = (.ComboBox1.MatchFound) And
(.Combobox2.MatchFound) And (...etc
End With

Would work as intended.

Indeed it does. As there's no detectable difference between the speed of
this routine and your nested if solution, I'm sticking with this option on
the basis that it's less characters and ultimately a smaller file size.

 

[GS]

IanC was thinking very hard :

Hi Martin


Yes, I just discovered that!


The length of time it takes is imperceptible. If there was an equivalent to
Debug.Print Now() which returned fractions of a second I could put a figure
on it, but to the human eye it's effectively instantaneous.



Indeed it does. As there's no detectable difference between the speed of this
routine and your nested if solution, I'm sticking with this option on the
basis that it's less characters and ultimately a smaller file size.

And so makes it more efficient code, no? What makes it so is the 'dot
processing' is minimal. 'Dot processing' requires resources for each
dot ref, which in this case is reintiating the ref to the form object
(Me) in every iteration where it's used. So Martin's suggestion is more
efficient in that the object ref (and thus reinitialization of that
ref) to CommandButton1 is only made once instead of six times. Though,
I would write his suggestion this way for clarity and readability:

With Me
.CommandButton1.Enabled = _
(.ComboBox1.MatchFound) And _
(.ComboBox2.MatchFound) And _
(.ComboBox3.MatchFound) And _
(.ComboBox4.MatchFound) And _
(.ComboBox5.MatchFound) And _
(.ComboBox6.MatchFound)
End With

 

[IanC]

Hi Garry

IanC was thinking very hard :

And so makes it more efficient code, no? What makes it so is the 'dot

Not necessarily in every case. An instruction may be more characters than
another but still quicker to execute. That said, in this case the time
involved is minimal so the absolute time is irrelevant.

processing' is minimal. 'Dot processing' requires resources for each dot
ref, which in this case is reintiating the ref to the form object (Me) in
every iteration where it's used. So Martin's suggestion is more efficient
in that the object ref (and thus reinitialization of that ref) to
CommandButton1 is only made once instead of six times. Though, I would
write his suggestion this way for clarity and readability:

With Me
.CommandButton1.Enabled = _
(.ComboBox1.MatchFound) And _
(.ComboBox2.MatchFound) And _
(.ComboBox3.MatchFound) And _
(.ComboBox4.MatchFound) And _
(.ComboBox5.MatchFound) And _
(.ComboBox6.MatchFound)
End With

Already did this. As you say it's much easier to read and I shortened it
even more by removing the parentheses.