Beginning programmer seeks cleaner code...

[voydangel]

I was hoping someone could explain to me how to make this code more simple, clean and/or streamlined. It works perfectly as written, but it seems to me there must be a shorter or faster way to do this (i.e.: less lines of code)
Any advice is appreciated.

Thanks in advance.

PS: how do I make this code block show up in a smaller (scrolling) "box" so that my code laden posts dont take up so much real estate?

Public Class Form1

    Private Sub btnTax_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles btnTax.Click
        btnTaxRate.Visible = Not btnTaxRate.Visible
        btnTaxExtra.Visible = Not btnTaxExtra.Visible
        btnAssignLabor.Hide()
        btnAssignArmy.Hide()
        btnGiveGold.Hide()
        btnGiveFood.Hide()
        btnGiveItems.Hide()
        btnTrainTroops.Hide()
        btnTrainPrince.Hide()
        btnTrainSelf.Hide()
        btnTrainPeople.Hide()
        btnTradeA.Hide()
        btnTradeB.Hide()
        btnTradeC.Hide()
        btnPeopleSwitch.Hide()
        btnPeopleSelect.Hide()
        btnPeopleDemote.Hide()
        btnPeoplePromote.Hide()
        btnPeopleMarry.Hide()
        btnPeopleDraft.Hide()
        btnViewState.Hide()
        btnViewRuler.Hide()
        btnViewAlly.Hide()
        btnViewPrince.Hide()
        btnViewChild.Hide()
        btnViewStock.Hide()
        btnSpyView.Hide()
        btnSpyWreak.Hide()
        btnSpyKill.Hide()
        btnSpySearch.Hide()
    End Sub

    Private Sub btnAssign_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles btnAssign.Click
        btnTaxRate.Hide()
        btnTaxExtra.Hide()
        btnAssignLabor.Visible = Not btnAssignLabor.Visible
        btnAssignArmy.Visible = Not btnAssignArmy.Visible
        btnGiveGold.Hide()
        btnGiveFood.Hide()
        btnGiveItems.Hide()
        btnTrainTroops.Hide()
        btnTrainPrince.Hide()
        btnTrainSelf.Hide()
        btnTrainPeople.Hide()
        btnTradeA.Hide()
        btnTradeB.Hide()
        btnTradeC.Hide()
        btnPeopleSwitch.Hide()
        btnPeopleSelect.Hide()
        btnPeopleDemote.Hide()
        btnPeoplePromote.Hide()
        btnPeopleMarry.Hide()
        btnPeopleDraft.Hide()
        btnViewState.Hide()
        btnViewRuler.Hide()
        btnViewAlly.Hide()
        btnViewPrince.Hide()
        btnViewChild.Hide()
        btnViewStock.Hide()
        btnSpyView.Hide()
        btnSpyWreak.Hide()
        btnSpyKill.Hide()
        btnSpySearch.Hide()
    End Sub

    Private Sub btnGive_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles btnGive.Click
        btnTaxRate.Hide()
        btnTaxExtra.Hide()
        btnAssignLabor.Hide()
        btnAssignArmy.Hide()
        btnGiveGold.Visible = Not btnGiveGold.Visible
        btnGiveFood.Visible = Not btnGiveFood.Visible
        btnGiveItems.Visible = Not btnGiveItems.Visible
        btnTrainTroops.Hide()
        btnTrainPrince.Hide()
        btnTrainSelf.Hide()
        btnTrainPeople.Hide()
        btnTradeA.Hide()
        btnTradeB.Hide()
        btnTradeC.Hide()
        btnPeopleSwitch.Hide()
        btnPeopleSelect.Hide()
        btnPeopleDemote.Hide()
        btnPeoplePromote.Hide()
        btnPeopleMarry.Hide()
        btnPeopleDraft.Hide()
        btnViewState.Hide()
        btnViewRuler.Hide()
        btnViewAlly.Hide()
        btnViewPrince.Hide()
        btnViewChild.Hide()
        btnViewStock.Hide()
        btnSpyView.Hide()
        btnSpyWreak.Hide()
        btnSpyKill.Hide()
        btnSpySearch.Hide()
    End Sub

    Private Sub btnTrain_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles btnTrain.Click
        btnTaxRate.Hide()
        btnTaxExtra.Hide()
        btnAssignLabor.Hide()
        btnAssignArmy.Hide()
        btnGiveGold.Hide()
        btnGiveFood.Hide()
        btnGiveItems.Hide()
        btnTrainTroops.Visible = Not btnTrainTroops.Visible
        btnTrainPrince.Visible = Not btnTrainPrince.Visible
        btnTrainSelf.Visible = Not btnTrainSelf.Visible
        btnTrainPeople.Visible = Not btnTrainPeople.Visible
        btnTradeA.Hide()
        btnTradeB.Hide()
        btnTradeC.Hide()
        btnPeopleSwitch.Hide()
        btnPeopleSelect.Hide()
        btnPeopleDemote.Hide()
        btnPeoplePromote.Hide()
        btnPeopleMarry.Hide()
        btnPeopleDraft.Hide()
        btnViewState.Hide()
        btnViewRuler.Hide()
        btnViewAlly.Hide()
        btnViewPrince.Hide()
        btnViewChild.Hide()
        btnViewStock.Hide()
        btnSpyView.Hide()
        btnSpyWreak.Hide()
        btnSpyKill.Hide()
        btnSpySearch.Hide()
    End Sub

    Private Sub btnTrade_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles btnTrade.Click
        btnTaxRate.Hide()
        btnTaxExtra.Hide()
        btnAssignLabor.Hide()
        btnAssignArmy.Hide()
        btnGiveGold.Hide()
        btnGiveFood.Hide()
        btnGiveItems.Hide()
        btnTrainTroops.Hide()
        btnTrainPrince.Hide()
        btnTrainSelf.Hide()
        btnTrainPeople.Hide()
        btnTradeA.Visible = Not btnTradeA.Visible
        btnTradeB.Visible = Not btnTradeB.Visible
        btnTradeC.Visible = Not btnTradeC.Visible
        btnPeopleSwitch.Hide()
        btnPeopleSelect.Hide()
        btnPeopleDemote.Hide()
        btnPeoplePromote.Hide()
        btnPeopleMarry.Hide()
        btnPeopleDraft.Hide()
        btnViewState.Hide()
        btnViewRuler.Hide()
        btnViewAlly.Hide()
        btnViewPrince.Hide()
        btnViewChild.Hide()
        btnViewStock.Hide()
        btnSpyView.Hide()
        btnSpyWreak.Hide()
        btnSpyKill.Hide()
        btnSpySearch.Hide()
    End Sub

    Private Sub btnPeople_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles btnPeople.Click
        btnTaxRate.Hide()
        btnTaxExtra.Hide()
        btnAssignLabor.Hide()
        btnAssignArmy.Hide()
        btnGiveGold.Hide()
        btnGiveFood.Hide()
        btnGiveItems.Hide()
        btnTrainTroops.Hide()
        btnTrainPrince.Hide()
        btnTrainSelf.Hide()
        btnTrainPeople.Hide()
        btnTradeA.Hide()
        btnTradeB.Hide()
        btnTradeC.Hide()
        btnPeopleSwitch.Visible = Not btnPeopleSwitch.Visible
        btnPeopleSelect.Visible = Not btnPeopleSelect.Visible
        btnPeopleDemote.Visible = Not btnPeopleDemote.Visible
        btnPeoplePromote.Visible = Not btnPeoplePromote.Visible
        btnPeopleMarry.Visible = Not btnPeopleMarry.Visible
        btnPeopleDraft.Visible = Not btnPeopleDraft.Visible
        btnViewState.Hide()
        btnViewRuler.Hide()
        btnViewAlly.Hide()
        btnViewPrince.Hide()
        btnViewChild.Hide()
        btnViewStock.Hide()
        btnSpyView.Hide()
        btnSpyWreak.Hide()
        btnSpyKill.Hide()
        btnSpySearch.Hide()
    End Sub

    Private Sub btnView_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles btnView.Click
        btnTaxRate.Hide()
        btnTaxExtra.Hide()
        btnAssignLabor.Hide()
        btnAssignArmy.Hide()
        btnGiveGold.Hide()
        btnGiveFood.Hide()
        btnGiveItems.Hide()
        btnTrainTroops.Hide()
        btnTrainPrince.Hide()
        btnTrainSelf.Hide()
        btnTrainPeople.Hide()
        btnTradeA.Hide()
        btnTradeB.Hide()
        btnTradeC.Hide()
        btnPeopleSwitch.Hide()
        btnPeopleSelect.Hide()
        btnPeopleDemote.Hide()
        btnPeoplePromote.Hide()
        btnPeopleMarry.Hide()
        btnPeopleDraft.Hide()
        btnViewState.Visible = Not btnViewState.Visible
        btnViewRuler.Visible = Not btnViewRuler.Visible
        btnViewAlly.Visible = Not btnViewAlly.Visible
        btnViewPrince.Visible = Not btnViewPrince.Visible
        btnViewChild.Visible = Not btnViewChild.Visible
        btnViewStock.Visible = Not btnViewStock.Visible
        btnSpyView.Hide()
        btnSpyWreak.Hide()
        btnSpyKill.Hide()
        btnSpySearch.Hide()
    End Sub

    Private Sub btnSpy_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles btnSpy.Click
        btnTaxRate.Hide()
        btnTaxExtra.Hide()
        btnAssignLabor.Hide()
        btnAssignArmy.Hide()
        btnGiveGold.Hide()
        btnGiveFood.Hide()
        btnGiveItems.Hide()
        btnTrainTroops.Hide()
        btnTrainPrince.Hide()
        btnTrainSelf.Hide()
        btnTrainPeople.Hide()
        btnTradeA.Hide()
        btnTradeB.Hide()
        btnTradeC.Hide()
        btnPeopleSwitch.Hide()
        btnPeopleSelect.Hide()
        btnPeopleDemote.Hide()
        btnPeoplePromote.Hide()
        btnPeopleMarry.Hide()
        btnPeopleDraft.Hide()
        btnViewState.Hide()
        btnViewRuler.Hide()
        btnViewAlly.Hide()
        btnViewPrince.Hide()
        btnViewChild.Hide()
        btnViewStock.Hide()
        btnSpyView.Visible = Not btnSpyView.Visible
        btnSpyWreak.Visible = Not btnSpyWreak.Visible
        btnSpyKill.Visible = Not btnSpyKill.Visible
        btnSpySearch.Visible = Not btnSpySearch.Visible
    End Sub

    Private Sub btnMove_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles btnMove.Click
        btnTaxRate.Hide()
        btnTaxExtra.Hide()
        btnAssignLabor.Hide()
        btnAssignArmy.Hide()
        btnGiveGold.Hide()
        btnGiveFood.Hide()
        btnGiveItems.Hide()
        btnTrainTroops.Hide()
        btnTrainPrince.Hide()
        btnTrainSelf.Hide()
        btnTrainPeople.Hide()
        btnTradeA.Hide()
        btnTradeB.Hide()
        btnTradeC.Hide()
        btnPeopleSwitch.Hide()
        btnPeopleSelect.Hide()
        btnPeopleDemote.Hide()
        btnPeoplePromote.Hide()
        btnPeopleMarry.Hide()
        btnPeopleDraft.Hide()
        btnViewState.Hide()
        btnViewRuler.Hide()
        btnViewAlly.Hide()
        btnViewPrince.Hide()
        btnViewChild.Hide()
        btnViewStock.Hide()
        btnSpyView.Hide()
        btnSpyWreak.Hide()
        btnSpyKill.Hide()
        btnSpySearch.Hide()
    End Sub

    Private Sub btnPolicy_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles btnPolicy.Click
        btnTaxRate.Hide()
        btnTaxExtra.Hide()
        btnAssignLabor.Hide()
        btnAssignArmy.Hide()
        btnGiveGold.Hide()
        btnGiveFood.Hide()
        btnGiveItems.Hide()
        btnTrainTroops.Hide()
        btnTrainPrince.Hide()
        btnTrainSelf.Hide()
        btnTrainPeople.Hide()
        btnTradeA.Hide()
        btnTradeB.Hide()
        btnTradeC.Hide()
        btnPeopleSwitch.Hide()
        btnPeopleSelect.Hide()
        btnPeopleDemote.Hide()
        btnPeoplePromote.Hide()
        btnPeopleMarry.Hide()
        btnPeopleDraft.Hide()
        btnViewState.Hide()
        btnViewRuler.Hide()
        btnViewAlly.Hide()
        btnViewPrince.Hide()
        btnViewChild.Hide()
        btnViewStock.Hide()
        btnSpyView.Hide()
        btnSpyWreak.Hide()
        btnSpyKill.Hide()
        btnSpySearch.Hide()
    End Sub

    Private Sub btnTreaty_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles btnTreaty.Click
        btnTaxRate.Hide()
        btnTaxExtra.Hide()
        btnAssignLabor.Hide()
        btnAssignArmy.Hide()
        btnGiveGold.Hide()
        btnGiveFood.Hide()
        btnGiveItems.Hide()
        btnTrainTroops.Hide()
        btnTrainPrince.Hide()
        btnTrainSelf.Hide()
        btnTrainPeople.Hide()
        btnTradeA.Hide()
        btnTradeB.Hide()
        btnTradeC.Hide()
        btnPeopleSwitch.Hide()
        btnPeopleSelect.Hide()
        btnPeopleDemote.Hide()
        btnPeoplePromote.Hide()
        btnPeopleMarry.Hide()
        btnPeopleDraft.Hide()
        btnViewState.Hide()
        btnViewRuler.Hide()
        btnViewAlly.Hide()
        btnViewPrince.Hide()
        btnViewChild.Hide()
        btnViewStock.Hide()
        btnSpyView.Hide()
        btnSpyWreak.Hide()
        btnSpyKill.Hide()
        btnSpySearch.Hide()
    End Sub

    Private Sub btnWar_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles btnWar.Click
        btnTaxRate.Hide()
        btnTaxExtra.Hide()
        btnAssignLabor.Hide()
        btnAssignArmy.Hide()
        btnGiveGold.Hide()
        btnGiveFood.Hide()
        btnGiveItems.Hide()
        btnTrainTroops.Hide()
        btnTrainPrince.Hide()
        btnTrainSelf.Hide()
        btnTrainPeople.Hide()
        btnTradeA.Hide()
        btnTradeB.Hide()
        btnTradeC.Hide()
        btnPeopleSwitch.Hide()
        btnPeopleSelect.Hide()
        btnPeopleDemote.Hide()
        btnPeoplePromote.Hide()
        btnPeopleMarry.Hide()
        btnPeopleDraft.Hide()
        btnViewState.Hide()
        btnViewRuler.Hide()
        btnViewAlly.Hide()
        btnViewPrince.Hide()
        btnViewChild.Hide()
        btnViewStock.Hide()
        btnSpyView.Hide()
        btnSpyWreak.Hide()
        btnSpyKill.Hide()
        btnSpySearch.Hide()
    End Sub
End Class

 

[RiverGuy]

Private Sub HideAllButtons()
  For Each c As Control In SomeContainer.Controls 'SomeContainer could be your form (Me), or a Panel, or whatever contains your buttons
    If TypeOf(c) Is Button Then
      Ctype(c, Button).Hide()
    End If
  Next
End Sub

The code example above would loop through a container, and hide all of the buttons contained within. You could create a similar routine and call it from your click events.

 

[Frink]

Hallo,

Is this implementing some sort of Tab control?
You seem to be displaying similarly named controls when a button is pressed.
If each set of controls can go on a separate tab page you could then just show/hide the tab page.

Alternatively write a Sub:

Private Sub SetButtonsVisibility(ByVal vstrType As String)
  SetButtonVisibility btnTaxRate, (vstrType="Tax")
  SetButtonVisibility btnTaxExtra, (vstrType="Tax")
  SetButtonVisibility btnAssignLabor, (vstrType="Assign")
  SetButtonVisibility btnAssignArmy, (vstrType="Assign")
  SetButtonVisibility btnGiveGold, (vstrType="Give")
  etc.
End Sub

Private Sub SetButtonVisibility(ByRef rctlButton As Control, ByVal vblnVisible As Boolean)
On Error Resume Next
  If vblnVisible Then
    If Not rctlButton.Visible Then rctlButton.Visible = True
  Else
    If Not rctlButton.Visible Then rctlButton.Hide()
  End If
  If Err Then MsgBox "Error calling SetButtonVisibility(" & rctlButton.Name & ", " & IIf(vstrVisible, "True", "False") & ")", vbExclamation
End Sub

Then replace all your btn<Type>_Click code with
SetButtonVisibility <Type>
e.g. in btnTax_Click use SetButtonVisibility "Tax",
in btnAssign_Click use SetButtonVisibility "Assign"

There are probably better ways of doing this, but this is better than what you had.

- Frink

 

[Frink]

Hallo again,

You could replacing the:
SetButtonVisibility btnTaxRate, (vstrType="Tax")
statements in my code with RiverGuy's code, replacing:
Ctype(c, Button).Hide()
with
SetButtonVisibility (c, c.Name Like "btn" & vstrType & "*")

Something like that anyway. You'll just have to make sure all the buttons you want to hide are called btn<Type>..., as yours seem to be so far.

- Frink

 

[voydangel]

Ok, so a bit more explanation is in order.

On a single panel container I have 12 Buttons. We will call these the "parent" buttons. These parent buttons must always be enabled, visible, and able to be clicked upon. When you click one of said parent buttons, it causes some "child" buttons to become visible.

I only want one set of child buttons visible at any given time. So if you click on a prent button, and then a second parent button, the children of the 1st parent button should become hidden, and if you click a parent button a second time, it should hide its child buttons, thereby making only the parent buttons visible again.

Effectively this is nearly identical functionality to regular old "file | edit | view" type menus at the top of your app window (only one menu shows at a time & clicking on a menu title a second time closes the menu), but because all the buttons are in various positions on the form, I cant use a menu control.

@ Frink: I have implemented your code as suggested, I see where it has the potential to make the code block much more elegant, but upon execution, clicking on a parent button shows its children correctly, but no matter what I do I can not hide children buttons with your code.

 

[voydangel]

Oh - update after a bit of playing:

Private Sub SetButtonVisibility(ByRef rctlButton As Control, ByVal vblnVisible As Boolean)
        On Error Resume Next
        If vblnVisible Then
            rctlButton.Visible = Not rctlButton.Visible
        Else
            If Not rctlButton.Visible Then rctlButton.Hide()
        End If
        If Err.Number <> 0 Then MsgBox("Error calling SetButtonVisibility(" & rctlButton.Name & ", " & IIf(vblnVisible, "True", "False") & ")", vbExclamation)
    End Sub

50% Working now - I can click a second time on the parent button to have it hide its child buttons, but clicking on a second parent button does not hide all other child buttons. It just shows its own in addition to any other children that were visible. Getting close though...

 

[voydangel]

and by removing Not:

If Not rctlButton.Visible Then rctlButton.Hide()

->

If rctlButton.Visible Then rctlButton.Hide()

it works as intended.

Now I just need to adapt the code to work for the other panels and controls etc that the children button show when you click on them =P

Thanks all.

 

[DaveInIowa]

If you set the Tag property of each button that is capable of being toggled to 'ChildMenu', then you could use the following code and not need a separate subroutine to handle each top level button's click event:

    Private Sub Form1_Load(ByVal sender As Object, ByVal e As System.EventArgs) Handles Me.Load

        AddHandler btnTax.Click, AddressOf btnPseudoMenu_Click
        AddHandler btnAssign.Click, AddressOf btnPseudoMenu_Click
        AddHandler btnGive.Click, AddressOf btnPseudoMenu_Click
        AddHandler btnTrain.Click, AddressOf btnPseudoMenu_Click
        AddHandler btnTrade.Click, AddressOf btnPseudoMenu_Click
        AddHandler btnPeople.Click, AddressOf btnPseudoMenu_Click
        AddHandler btnView.Click, AddressOf btnPseudoMenu_Click
        AddHandler btnSpy.Click, AddressOf btnPseudoMenu_Click
        AddHandler btnMove.Click, AddressOf btnPseudoMenu_Click
        AddHandler btnPolicy.Click, AddressOf btnPseudoMenu_Click
        AddHandler btnTreaty.Click, AddressOf btnPseudoMenu_Click
        AddHandler btnWar.Click, AddressOf btnPseudoMenu_Click

    End Sub

    Private Sub btnPseudoMenu_Click(ByVal sender As System.Object, ByVal e As System.EventArgs)

        Dim btnNameMatch = DirectCast(sender, Button).Name & "*"

        For Each ctrl As Control In Me.Controls
            If (TypeOf ctrl Is Button AndAlso ctrl.Tag = "ChildMenu") Then
                If (ctrl.Name Like btnNameMatch) Then
                    ctrl.Visible = Not ctrl.Visible
                Else
                    ctrl.Hide()
                End If
            End If
        Next

    End Sub

 

[bboffin]

If I've followed this correctly an even simpler approach is to place each set of 'child buttons' on separate Panel components all sharing the same location and size. By default initially all these panels are not visible.

Then when a parent button is clicked you simply make the required panel visible and call its BringToFront method.

This avoids all the looping and makes for much cleaner code.

Regards

Bob Boffin

 

[VB4Life]

I am with Bob, and if you have several groups that need the same location you can stack the panels on the same location and use the BringToFront() property of the panel you want to show.