Self Taught (badly) VBA Help :(

G

Grammarjunkie

Board Regular
Joined
Mar 22, 2016
Messages
86
I know Excel formulas, and I assumed VBA would be the same. It has similarities, but it definitely is its own beast. I was hoping someone could take a look at my VBA code, and just use one little piece as an example as to how to write it ... prettier? I know that's a tall order. But I know the way I have it written can't be the easiest, simplest way.

And also it causes some weird things. Like pressing Enter, instead of it going to the next cell (that in my VBA code is ".Activate"), it jumps to a random one. In one instance it jumped to the cell BEFORE it. I don't know how I accomplished that.

On any account, I'm not asking for anyone to rewrite literally all of it. (Though that would make my day.)

Just a little "VBA for Dummies." As I said, I'm self taught, so I just played around until I got it to do what I wanted. But it's probably not written the most ... logically.

Thanks for even considering it. I appreciate it so much. :)

The following is my code (it's not that long compared to what y'all do, I bet):

Code: 
Private Sub Worksheet_Change(ByVal Target As Range)

ActiveSheet.Unprotect
Application.ScreenUpdating = False

If Range("F45") = "Y" Then
    Rows("46").Hidden = False
    Range("F46").Activate
Else
    Rows("46").Hidden = True
End If

If Range("F45") = "y" Then
    Rows("46").Hidden = False
    Range("F46").Activate
Else
    Rows("46").Hidden = True
End If

If Range("F46") = "Y" Then
    Rows("47").Hidden = False
    Range("F47").Activate
Else
    Rows("47").Hidden = True
End If

If Range("F46") = "y" Then
    Rows("47").Hidden = False
    Range("F47").Activate
Else
    Rows("47").Hidden = True
End If


If Range("J3") = "TPO" Then
    Rows("14:15").Hidden = False
Else
    Rows("14:15").Hidden = True
End If

If Range("D13") = "Purchase" Then
    Rows("86").Hidden = False
Else
    Rows("86").Hidden = True
End If

If Range("F102") = "Y" Then
    Rows("103").Hidden = False
    Range("F103").Activate
Else
    Rows("103").Hidden = True
End If

If Range("F102") = "y" Then
    Rows("103").Hidden = False
    Range("F103").Activate
Else
    Rows("103").Hidden = True
End If

If (Range("D6") <> "") And (Range("E18") <> "") And (Range("D6") > Range("E18")) Then
        Rows("39:41").Hidden = False
Else
        Rows("39:41").Hidden = True
End If


If Range("E19") = "" Then
        Rows("48:52").Hidden = True
Else
        Rows("48:52").Hidden = False
End If

If Range("D13") = "Purchase" Then
    Rows("60").Hidden = False
Else
    Rows("60").Hidden = True
End If

If Range("E22") = "" Then
    Rows("53:57").Hidden = True
Else
    Rows("53:57").Hidden = False
End If

If Range("D13") = "Purchase" Then
    Rows("105:114").Hidden = False
Else
    Rows("105:114").Hidden = True
End If

If Range("J3") = "TPO" Then
    Rows("119:122").Hidden = False
Else
    Rows("119:122").Hidden = True
End If

If Range("D13") = "Purchase" Then
    Rows("128:131").Hidden = False
Else
    Rows("128:131").Hidden = True
End If

If Range("J3") = "TPO" Then
    Rows("127").Hidden = False
Else
    Rows("127").Hidden = True
End If

If Range("F96") = "Y" Then
    Rows("97:98").Hidden = False
    Range("F97").Activate
Else
    Rows("97:98").Hidden = True
End If

If Range("F96") = "y" Then
    Rows("97:98").Hidden = False
    Range("F97").Activate
Else
    Rows("97:98").Hidden = True
End If

If Range("F90") = "Y" Then
    Rows("91:96").Hidden = False
    Range("F91").Activate
Else
    Rows("91:96").Hidden = True
End If

If Range("F90") = "y" Then
    Rows("91:96").Hidden = False
    Range("F91").Activate
Else
    Rows("91:96").Hidden = True
End If

If Range("F102") = "Y" Then
    Rows("103").Hidden = False
    Range("F103").Activate
Else
    Rows("103").Hidden = True
End If

If Range("F102") = "y" Then
    Rows("103").Hidden = False
    Range("F103").Activate
Else
    Rows("103").Hidden = True
End If

If Range("F107") = "Y" Then
    Rows("108:109").Hidden = False
    Range("F108").Activate
Else
    Rows("108:109").Hidden = True
End If

If Range("F107") = "y" Then
    Rows("108:109").Hidden = False
    Range("F108").Activate
Else
    Rows("108:109").Hidden = True
End If


Application.ScreenUpdating = True
ActiveSheet.Protect


End Sub
 

Excel Facts

Create a Pivot Table on a Map
If your data has zip codes, postal codes, or city names, select the data and use Insert, 3D Map. (Found to right of chart icons).
Sort by date Sort by votes
A few things.

I see that you are using a "Worksheet_Change" event procedure. That is code that will be called EVERY time any change is made to that sheet! Are you sure ALL of it needs to run every single time ANY change is made?

Typically, Worksheet_Change event procedure use the inherent "Target" range variable. "Target" is the cell that was changed that triggered the macro to run. Often times, people will check Target to see if certain key cells were changed, and if they were, run some code. If not, exit the procedure without doing anything.

For example, here is how you would check to see if a cell in column B was changed:
Code: 
If Target.Column = 2 Then
'   Your code here
End If

Also, since what you are doing under:
Code: 
If Range("F45") = "Y" Then
is exactly the same as what is done under:
Code: 
If Range("F45") = "y" Then
you can eliminate the extra block of code and just have one like this:
Code: 
If UCase(Range("F45")) = "Y" Then

So the first thing to determine is whether you really need to be using a Worksheet_Change event procedure or a standard procedure. If you do need to use a Worksheet_Change procedure, you need to determine what cell changes should trigger what actions, and update your code accordingly.
 
Upvote 0
Joe4 said:
A few things.

I see that you are using a "Worksheet_Change" event procedure. That is code that will be called EVERY time any change is made to that sheet! Are you sure ALL of it needs to run every single time ANY change is made?

Typically, Worksheet_Change event procedure use the inherent "Target" range variable. "Target" is the cell that was changed that triggered the macro to run. Often times, people will check Target to see if certain key cells were changed, and if they were, run some code. If not, exit the procedure without doing anything.

For example, here is how you would check to see if a cell in column B was changed:
Code: 
If Target.Column = 2 Then
'   Your code here
End If

Also, since what you are doing under:
Code: 
If Range("F45") = "Y" Then
is exactly the same as what is done under:
Code: 
If Range("F45") = "y" Then
you can eliminate the extra block of code and just have one like this:
Code: 
If UCase(Range("F45")) = "Y" Then

So the first thing to determine is whether you really need to be using a Worksheet_Change event procedure or a standard procedure. If you do need to use a Worksheet_Change procedure, you need to determine what cell changes should trigger what actions, and update your code accordingly.
Click to expand...


I suppose no, I don't need it to run any time a change is made on the worksheet -- only when the cells have that change made! If F45 has Y, etc.
I just assumed we are always supposed to use worksheet_change. :confused:
 
Upvote 0
OK. Here are a few changes I would make. I would use Target (the cell that was just changed which called the VBA code) and I would use CASE statements (a little cleaner than IF...THEN statements).

Here is how that code would start (I did it for the updates to F45, F46, and J3 to start):
Code: 
Private Sub Worksheet_Change(ByVal Target As Range)

'   Check to make sure only one cell has been updated (otherwise exit)
    If Target.Count > 1 Then Exit Sub
    
    ActiveSheet.Unprotect
    Application.ScreenUpdating = False

'   Decide what to do based on which cell was updated
    Select Case Replace(Target.Address, "$", "")
    
'       Cell F45 or F46
        Case "F45", "F46"
            If UCase(Target) = "Y" Then
                Rows(Target.Row + 1).Hidden = False
            Else
                Rows(Target.Row + 1).Hidden = True
            End If
            
'       Cell J3
        Case "J3"
            If Target = "TPO" Then
                Rows("14:15").Hidden = False
            Else
                Rows("14:15").Hidden = True
            End If
            
'       *** REST OF CODE BELOW ***

            
    End Select

    Application.ScreenUpdating = True
    ActiveSheet.Protect

End Sub
 
Last edited:
Upvote 0
Joe4 said:
OK. Here are a few changes I would make. I would use Target (the cell that was just changed which called the VBA code) and I would use CASE statements (a little cleaner than IF...THEN statements).

Here is how that code would start (I did it for the updates to F45, F46, and J3 to start):
Code: 
Private Sub Worksheet_Change(ByVal Target As Range)

'   Check to make sure only one cell has been updated (otherwise exit)
    If Target.Count > 1 Then Exit Sub
    
    ActiveSheet.Unprotect
    Application.ScreenUpdating = False

'   Decide what to do based on which cell was updated
    Select Case Replace(Target.Address, "$", "")
    
'       Cell F45 or F46
        Case "F45", "F46"
            If UCase(Target) = "Y" Then
                Rows(Target.Row + 1).Hidden = False
            Else
                Rows(Target.Row + 1).Hidden = True
            End If
            
'       Cell J3
        Case "J3"
            If Target = "TPO" Then
                Rows("14:15").Hidden = False
            Else
                Rows("14:15").Hidden = True
            End If
            
'       *** REST OF CODE BELOW ***

            
    End Select

    Application.ScreenUpdating = True
    ActiveSheet.Protect

End Sub
Click to expand...

May I ask just a few more brief questions? One thing I haven't been able to figure out how to do is multiple rows without writing a whole new code. For example:
Cell D13
Case "D13"
If UCase(Target) = "Purchase" Then
Rows("86,60,105:114,128:131").Hidden = False
Else
Rows("86,60,105:114,128:131").Hidden = True
End If

Obviously that isn't how you do it, but you get the visual of what I mean. "Purchase" reveals multiple rows and sequences of rows, and I'm not sure how to set those off.


Another brief question I have is where the Range("F46").Activate fits into your kindly provided code.
So that if they type "Y", and press Enter, the cursor goes to the cell in the same column in the row revealed, rather than the row after the one just revealed.

In the first code I put, you can see how I had it. Should I keep it like that, or is there a better way to do that too?

I really appreciate your help with all this. It's so exciting to learn and be able to do this. Especially being a creative mind myself, not really one for logics and analyzing, etc. But when you share how you'd do a code, I stare at it for a bit, and I'm starting to see what it's saying so that I can write them easier and prettier in the future.
 
Upvote 0
Joe4 said:
OK. Here are a few changes I would make. I would use Target (the cell that was just changed which called the VBA code) and I would use CASE statements (a little cleaner than IF...THEN statements).

Here is how that code would start (I did it for the updates to F45, F46, and J3 to start):
Code: 
Private Sub Worksheet_Change(ByVal Target As Range)

'   Check to make sure only one cell has been updated (otherwise exit)
    If Target.Count > 1 Then Exit Sub
    
    ActiveSheet.Unprotect
    Application.ScreenUpdating = False

'   Decide what to do based on which cell was updated
    Select Case Replace(Target.Address, "$", "")
    
'       Cell F45 or F46
        Case "F45", "F46"
            If UCase(Target) = "Y" Then
                Rows(Target.Row + 1).Hidden = False
            Else
                Rows(Target.Row + 1).Hidden = True
            End If
            
'       Cell J3
        Case "J3"
            If Target = "TPO" Then
                Rows("14:15").Hidden = False
            Else
                Rows("14:15").Hidden = True
            End If
            
'       *** REST OF CODE BELOW ***

            
    End Select

    Application.ScreenUpdating = True
    ActiveSheet.Protect

End Sub
Click to expand...

Also, I tried to do it on my own, but I can't figure out how to write this code in your target case format:

Code: 
If (Range("D6") <> "") And (Range("E18") <> "") And (Range("D6") > Range("E18")) Then
        Rows("39:41").Hidden = False
Else
        Rows("39:41").Hidden = True
End If


Sorry to be a bother.
 
Upvote 0
Note that your IF statement here:
Code: 
        [COLOR=#333333]Case "D13"[/COLOR]
             [COLOR=#333333]If UCase(Target) = "Purchase" Then[/COLOR]
                 [COLOR=#333333]Rows("86,60,105:114,128:131").Hidden = False[/COLOR]
             [COLOR=#333333]Else[/COLOR]
                 [COLOR=#333333]Rows("86,60,105:114,128:131").Hidden = True[/COLOR]
             [COLOR=#333333]End If[/COLOR]
will NEVER be true, as UCASE means make the whole word UPPER case.
Try this:
Code: 
        Case "D13"
            If UCase(Target) = "PURCHASE" Then
                Range("60:60,86:86,105:114,128:131").EntireRow.Hidden = False
            Else
                Range("60:60,86:86,105:114,128:131").EntireRow.Hidden = True
            End If
Another brief question I have is where the Range("F46").Activate fits into your kindly provided code.
So that if they type "Y", and press Enter, the cursor goes to the cell in the same column in the row revealed, rather than the row after the one just revealed.

In the first code I put, you can see how I had it. Should I keep it like that, or is there a better way to do that too?
Click to expand...
I am not sure I follow what you are asking.
Are you referring to the Range("F46").Activate line that you had in your original code?
I didn't see much purpose to that. Most of the time, lines of code that are just selecting various cells are unnecessary, unless you just want the cursor to move to that cell so that when the code is finished running, it is in that cell.
 
Last edited:
Upvote 0
Code: 
Cell E19
        Case "E19"
            If UCase(Target) = "" Then
                Rows("48:52").Hidden = True
            Else
                Rows("48:52").Hidden = False
            End If

I also just realized this way doesn't work for hiding rows until a cell is filled. IE - hidden if a cell is blank. :(

I shouldn't have opened this bag of worms. I'm sorry.
 
Upvote 0
I also just realized this way doesn't work for hiding rows until a cell is filled. IE - hidden if a cell is blank. :(

I shouldn't have opened this bag of worms. I'm sorry.
Click to expand...
What that code would do is if a value is added to cell E19, it will unhide rows 48:52. If a value is removed from cell E19, it will hide rows 48:52.

It sounds like you want a "default" or a starting point where some rows are hidden from the get-go and unhidden as data is entered. If you want your whole worksheet to start out with a bunch of rows hidden, maybe you want to put that code in a Workbook_Open event, which is code that runs when a workbook is first opened.
 
Upvote 0
Joe4 said:
What that code would do is if a value is added to cell E19, it will unhide rows 48:52. If a value is removed from cell E19, it will hide rows 48:52.

It sounds like you want a "default" or a starting point where some rows are hidden from the get-go and unhidden as data is entered. If you want your whole worksheet to start out with a bunch of rows hidden, maybe you want to put that code in a Workbook_Open event, which is code that runs when a workbook is first opened.
Click to expand...


I'm at a loss. The code as it is doesn't hide or unhide anything no matter if I add to the cell or take from the cell.

I would like it start hidden though, yes. I'm attempting to figure out how to do it.
Thanks for your help, Joe.
 
Upvote 0
You must log in or register to reply here.

Similar threads

R
  • Question Question
Copy and unfilter in VBA
Replies
1
Views
173
Alex Blakenburg
A
J
  • Question Question
Combining VBA code together...need help!
Replies
3
Views
378
Fluff
Fluff
Rob_010101
  • Question Question
Mail Merge Macro - Saves to PDF/Word
Replies
1
Views
704
Rob_010101
Rob_010101
R
  • Solved
VBA Timer
2
Replies
11
Views
1K
Rana Gray
R
S
  • Question Question
Excel VBA - Sort & Get Totals - Logic
Replies
4
Views
325
STARL0RD
S

Forum statistics

Threads
1,223,053
Messages
6,169,831
Members
452,284
Latest member
TKM623

Share this page

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