Private Sub vs Public Sub and Modules (Having issues)

NewYears1978

Board Regular
Joined
Feb 25, 2014
Messages
107
So I've read about Public, Private and that Sub = Public Sub
I've seen projects with some Subs in their on modules and such but I still cannot quite understand when and where to put them.

Here is one example, I have the following sub located in my form (TradeRouteForm) code section

Code:
Private Sub LoadColors()


    Dim oCtrl As Control
    
         For Each oCtrl In Me.Controls
        If TypeName(oCtrl) = "TextBox" Or TypeName(oCtrl) = "Label" Or TypeName(oCtrl) = "Frame" Then
            oCtrl.BorderColor = ActiveWorkbook.Colors(Worksheets("Options").Range("A4").Value)
        End If
    Next oCtrl


    
    For Each oCtrl In Me.Controls
        If TypeName(oCtrl) = "Label" Then
            If oCtrl.Tag <> "ColorSquares" Then
                oCtrl.ForeColor = ActiveWorkbook.Colors(Worksheets("Options").Range("B4").Value)
            End If
        End If
    Next oCtrl


    


    For Each oCtrl In Me.Controls
        If TypeName(oCtrl) = "TextBox" Then
            If oCtrl.Tag <> "OptionBoxes" Then
                oCtrl.ForeColor = ActiveWorkbook.Colors(Worksheets("Options").Range("C4").Value)
            End If
        End If
    Next oCtrl


  
    For Each oCtrl In Me.Controls
        If TypeName(oCtrl) = "Label" Then
            If oCtrl.Tag = "Buttons" Or oCtrl.Tag = "RouteButtons" Or oCtrl.Tag = "OptionButtons" Then
                oCtrl.ForeColor = ActiveWorkbook.Colors(Worksheets("Options").Range("D4").Value)
            End If
        End If
    Next oCtrl


End Sub


I figured it might be nice to move it to it's own module or a module that contains many of my other Subs already.
So I made a module named "ModuleLoadColors" and pasted it in there with some changes (.Me had to be changed since not located in my form code)

Code:
Public Sub LoadColors()


    Dim oCtrl As Control
    
         For Each oCtrl In TradeRouteForm.Controls
        If TypeName(oCtrl) = "TextBox" Or TypeName(oCtrl) = "Label" Or TypeName(oCtrl) = "Frame" Then
            oCtrl.BorderColor = ActiveWorkbook.Colors(Worksheets("Options").Range("A4").Value)
        End If
    Next oCtrl


    
    For Each oCtrl In TradeRouteForm.Controls
        If TypeName(oCtrl) = "Label" Then
            If oCtrl.Tag <> "ColorSquares" Then
                oCtrl.ForeColor = ActiveWorkbook.Colors(Worksheets("Options").Range("B4").Value)
            End If
        End If
    Next oCtrl


    


    For Each oCtrl In TradeRouteForm.Controls
        If TypeName(oCtrl) = "TextBox" Then
            If oCtrl.Tag <> "OptionBoxes" Then
                oCtrl.ForeColor = ActiveWorkbook.Colors(Worksheets("Options").Range("C4").Value)
            End If
        End If
    Next oCtrl


  
    For Each oCtrl In TradeRouteForm.Controls
        If TypeName(oCtrl) = "Label" Then
            If oCtrl.Tag = "Buttons" Or oCtrl.Tag = "RouteButtons" Or oCtrl.Tag = "OptionButtons" Then
                oCtrl.ForeColor = ActiveWorkbook.Colors(Worksheets("Options").Range("D4").Value)
            End If
        End If
    Next oCtrl


End Sub

No errors are thrown but the code doesn't work..I can't think of any other reason why.

I also thought maybe it wasn't calling the sub correctly where I have it being called elsewhere so I changed "LoadColors" to "Call LoadColors" and also tried "Call TradeRouteForm.LoadColors"


I hope I have explained this well enough..it's been throwing me off ever since I started learning excel. (I have Googled and read a ton of pages about Private vs Public also..but that didn't help)


Also, I added a random msgbox in there to see if the sub was calling and it is indeed..it just isn't doing what it should (read data in cells and pass the values)
 
Last edited:

Excel Facts

Copy a format multiple times
Select a formatted range. Double-click the Format Painter (left side of Home tab). You can paste formatting multiple times. Esc to stop
I agree it should work. I cannot see anything wrong with the code. As a quick test, can you paste in back in the UserForm's code module to confirm it still works from there.


I figured it might be nice to move it to it's own module or a module that contains many of my other Subs already.

I know you do some of these things just to learn, but I would keep it located in the userform's module because it's coded to work with that one userform.
 
Upvote 0
I agree it should work. I cannot see anything wrong with the code. As a quick test, can you paste in back in the UserForm's code module to confirm it still works from there.


I know you do some of these things just to learn, but I would keep it located in the userform's module because it's coded to work with that one userform.

Yes sir, I pasted it back and it worked fine. It appears to be working in the module as well however it just doesn't do what it should, oddly.
I don't have a problem leaving it I just have had such bad luck with modules and when all the code is in the forms code things get messy. Tonight I have been comment codes and putting things in a more functional order (Like, Initialize, Activate and then buttons in order they appear on the form etc) and this one thing about Modules and Sub's and Privates was throwing me lol.


A secondary question, does the ORDER of things here actually matter? Code in the userform I mean, does the order it is matter? I notice sometimes when double clicking a new button it puts it at the top of all code..and I never an tell if that's intentional or what.
 
Upvote 0
A secondary question, does the ORDER of things here actually matter? Code in the userform I mean, does the order it is matter? I notice sometimes when double clicking a new button it puts it at the top of all code..and I never an tell if that's intentional or what.

The order of the procedures doesn't matter.

TIP:You can consolidate all the IF code blocks within one For-Next loop.

Code:
[color=darkblue]Public[/color] [color=darkblue]Sub[/color] LoadColors()
    
    [color=darkblue]Dim[/color] oCtrl [color=darkblue]As[/color] Control
    
    [color=darkblue]For[/color] [color=darkblue]Each[/color] oCtrl [color=darkblue]In[/color] TradeRouteForm.Controls
        
        [color=darkblue]If[/color] TypeName(oCtrl) = "TextBox" [color=darkblue]Or[/color] TypeName(oCtrl) = "Label" [color=darkblue]Or[/color] TypeName(oCtrl) = "Frame" [color=darkblue]Then[/color]
            oCtrl.BorderColor = ActiveWorkbook.Colors(Worksheets("Options").Range("A4").Value)
        [color=darkblue]End[/color] [color=darkblue]If[/color]
        
        [color=darkblue]If[/color] TypeName(oCtrl) = "Label" [color=darkblue]Then[/color]
            [color=darkblue]If[/color] oCtrl.Tag <> "ColorSquares" [color=darkblue]Then[/color]
                oCtrl.ForeColor = ActiveWorkbook.Colors(Worksheets("Options").Range("B4").Value)
            [color=darkblue]ElseIf[/color] oCtrl.Tag = "Buttons" [color=darkblue]Or[/color] oCtrl.Tag = "RouteButtons" [color=darkblue]Or[/color] oCtrl.Tag = "OptionButtons" [color=darkblue]Then[/color]
                oCtrl.ForeColor = ActiveWorkbook.Colors(Worksheets("Options").Range("D4").Value)
            [color=darkblue]End[/color] [color=darkblue]If[/color]
        [color=darkblue]ElseIf[/color] TypeName(oCtrl) = "TextBox" [color=darkblue]Then[/color]
            [color=darkblue]If[/color] oCtrl.Tag <> "OptionBoxes" [color=darkblue]Then[/color]
                oCtrl.ForeColor = ActiveWorkbook.Colors(Worksheets("Options").Range("C4").Value)
            [color=darkblue]End[/color] [color=darkblue]If[/color]
        [color=darkblue]End[/color] [color=darkblue]If[/color]
        
    [color=darkblue]Next[/color] oCtrl
    
[color=darkblue]End[/color] [color=darkblue]Sub[/color]
 
Upvote 0
Well that's helpful, thanks :)
Always good to shorten/condense/consolidate code! (which is what I am trying to do right now anyways)

Thanks :) I guess I will just leave the Sub in the form, maybe some day I will figure out why it doesn't work the other way.


AlphaFrog, thanks again for always helping out :)
 
Upvote 0
Well this isn't going to make sense but when I condensed the code, it stopped working (no errors just no results, same as when it was in module)

Check it out here, the two codesets together, I comment one out for the other. Old way works, new way doesn't. Weird.

Code:
'Private Sub LoadColors()
'
'    Dim oCtrl As Control
'
'    For Each oCtrl In TradeRouteForm.Controls
'        If TypeName(oCtrl) = "TextBox" Or TypeName(oCtrl) = "Label" Or TypeName(oCtrl) = "Frame" Then
'            oCtrl.BorderColor = ActiveWorkbook.Colors(Worksheets("Options").Range("A4").Value)
'        End If
'
'         If TypeName(oCtrl) = "Label" Then
'            If oCtrl.Tag <> "ColorSquares" Then
'                oCtrl.ForeColor = ActiveWorkbook.Colors(Worksheets("Options").Range("B4").Value)
'            End If
'        End If
'
'        If TypeName(oCtrl) = "TextBox" Then
'            If oCtrl.Tag <> "OptionBoxes" Then
'                oCtrl.ForeColor = ActiveWorkbook.Colors(Worksheets("Options").Range("C4").Value)
'            End If
'        End If
'
'        If TypeName(oCtrl) = "Label" Then
'            If oCtrl.Tag = "Buttons" Or oCtrl.Tag = "RouteButtons" Or oCtrl.Tag = "OptionButtons" Then
'                oCtrl.ForeColor = ActiveWorkbook.Colors(Worksheets("Options").Range("D4").Value)
'            End If
'        End If
'    Next oCtrl
'
'End Sub
Private Sub LoadColors()


    Dim oCtrl As Control


         For Each oCtrl In Me.Controls
        If TypeName(oCtrl) = "TextBox" Or TypeName(oCtrl) = "Label" Or TypeName(oCtrl) = "Frame" Then
            oCtrl.BorderColor = ActiveWorkbook.Colors(Worksheets("Options").Range("A4").Value)
        End If
    Next oCtrl




    For Each oCtrl In Me.Controls
        If TypeName(oCtrl) = "Label" Then
            If oCtrl.Tag <> "ColorSquares" Then
                oCtrl.ForeColor = ActiveWorkbook.Colors(Worksheets("Options").Range("B4").Value)
            End If
        End If
    Next oCtrl






    For Each oCtrl In Me.Controls
        If TypeName(oCtrl) = "TextBox" Then
            If oCtrl.Tag <> "OptionBoxes" Then
                oCtrl.ForeColor = ActiveWorkbook.Colors(Worksheets("Options").Range("C4").Value)
            End If
        End If
    Next oCtrl




    For Each oCtrl In Me.Controls
        If TypeName(oCtrl) = "Label" Then
            If oCtrl.Tag = "Buttons" Or oCtrl.Tag = "RouteButtons" Or oCtrl.Tag = "OptionButtons" Then
                oCtrl.ForeColor = ActiveWorkbook.Colors(Worksheets("Options").Range("D4").Value)
            End If
        End If
    Next oCtrl


End Sub
 
Upvote 0
I don't know what's going on.

Maybe add this at the end of the procedure.
Me.Repaint


If that doesn't help, comment out parts of the procedure see if there is one section that is the offender.
 
Upvote 0

Forum statistics

Threads
1,213,487
Messages
6,113,938
Members
448,534
Latest member
benefuexx

We've detected that you are using an adblocker.

We have a great community of people providing Excel help here, but the hosting costs are enormous. You can help keep this site running by allowing ads on MrExcel.com.
Allow Ads at MrExcel

Which adblocker are you using?

Disable AdBlock

Follow these easy steps to disable AdBlock

1)Click on the icon in the browser’s toolbar.
2)Click on the icon in the browser’s toolbar.
2)Click on the "Pause on this site" option.
Go back

Disable AdBlock Plus

Follow these easy steps to disable AdBlock Plus

1)Click on the icon in the browser’s toolbar.
2)Click on the toggle to disable it for "mrexcel.com".
Go back

Disable uBlock Origin

Follow these easy steps to disable uBlock Origin

1)Click on the icon in the browser’s toolbar.
2)Click on the "Power" button.
3)Click on the "Refresh" button.
Go back

Disable uBlock

Follow these easy steps to disable uBlock

1)Click on the icon in the browser’s toolbar.
2)Click on the "Power" button.
3)Click on the "Refresh" button.
Go back
Back
Top