Declare Variables

[gaz1959]

Hi,
I have a table with approx 44 headings and at present 7800 rows. The rows are increased by approx 30 each week with new data.

I have not declared any variables, please can you suggest what variables I should declare, and at what point in the code should the variables be mentioned.

Also where I have the code look to row 9999 (which gives me scope for the weekly added new rows, could I just make this say 99999 without causing any problems).

The code is just one of many, all the same except for the col that the 9999 is applied to, and sometimes that col has blank cells, hence the 9999.

Thanks.

Here's the code which achieves the result exactly as I need it to along with my notes:

Sub SEE_OnlySoldThisWeek()
'from a combo box choice stored in named range SUPP_Name to only show that supplier
'run from the MENU sheet

Sheets("ALLdetails").Visible = True

Sheets("ALLdetails").Select
ActiveSheet.Unprotect
Application.ScreenUpdating = False

'unhide all the columns in case any left from previous sorts
' cols only go to CL at present, D:D gives scope for more
Columns("A:DD").Select
    Selection.EntireColumn.Hidden = False
    
'remove any previous filters (must be filtered before otherwise this will bug it)
ActiveSheet.ShowAllData

    
'select the supplier full name (eg. 30 - LEWIS)col M,

    Range("M1").Select
' filter criteria & hide the arrows
    Selection.AutoFilter
        
    ActiveSheet.Range("$M$1:$M$9999").AutoFilter Field:=1, Criteria1:=Range("SUPP_Name").Value, VisibleDropDown:=False

' go to cell A1 to remove the selection of col I (must be a better way, perhaps it does not even need it)
'Range("A1").Select
'or maybe this
'ActiveWindow.ScrollRow = 2

' maybe this, col AR #44 is 2nd unprotected col ready for user input
Columns(44).Cells.SpecialCells(xlCellTypeVisible).Cells(2).Select

'=====================
'the sort and hide bit
'======================
'sort the whole table starting from PLU (H) to the end (BG) columns
'set sort criteria for col X, from row 2 to the last row, in desending order
Range("H2", Range("BG" & Rows.Count).End(xlUp).Address).Sort key1:=[X2], order1:=xlDescending, Header:=xlNo

'now hide the rows that have not sold this week <1(zero)
'look in col X from row 2 to end, then up to last used cell
' approx 30 rows added each week,at 7800 now, 9999 gives scope for a while
    Set rng = Range("X2", Range("X9999").End(xlUp))

        For Each cel In rng

        If cel.Value < 1 Then 'if cell is less than 1, ie none sold this week

            cel.EntireRow.Hidden = True  'hide the row

         'if greater than 1, ie some sold this week, leave it shown and go to next row
        End If

    Next cel 'keep going until all rows checked
    
 'hide columns to only show those needed for this report
 
ActiveSheet.Columns("K:W").Select
Selection.EntireColumn.Hidden = True
' CL is last col as BF-CI are only heading at present
ActiveSheet.Columns("Y:CL").Select
Selection.EntireColumn.Hidden = True
    
'scroll to the top
ActiveWindow.ScrollRow = 2
ActiveWindow.ScrollColumn = 1

ActiveSheet.Protect

'hide the menu sheet
Sheets("MENU").Visible = False

Application.ScreenUpdating = True
End Sub

 

[kelly mort]

Maybe

Sub SEE_OnlySoldThisWeek()
 Dim cel As Long
Dim rng As Long

 

[MARK858]

Sub SEE_OnlySoldThisWeek()
Dim cel As Range, Rng As Range
Sheets("ALLdetails").Visible = True

and

Also where I have the code look to row 9999 (which gives me scope for the weekly added new rows, could I just make this say 99999 without causing any problems).

Set Rng = Range("X2", Range("X" & Rows.Count).End(xlUp))

If you mean the last row with data in column X.

and the same for M

Range("M1:M" & Range("M" & Rows.Count).End(xlUp).Row).AutoFilter Field:=1, Criteria1:=Range("SUPP_Name").Value, VisibleDropDown:=False

 

[MARK858]

Or

lr = Cells.Find("*", , xlValues, xlPart, xlByRows, xlPrevious, False).Row
Range("M1:M" & lr).AutoFilter Field:=1, Criteria1:=Range("SUPP_Name").Value, VisibleDropDown:=False
' rest of code inbetween
Set Rng = Range("X2", Range("X" & lr))

which would make your declarations...

Sub SEE_OnlySoldThisWeek()
Dim cel As Range, Rng As Range, lr As Long
Sheets("ALLdetails").Visible = True

 

[gaz1959]

Thanks to everyone who replied, just after I posted the question I was unable to log back in, site maintenance, and have been away from the PC since. I will be able to test out your suggestions sometime tomorrow, so apologies for not replying before.
Thanks again

 

[gaz1959]

Mark,
Thanks for your suggestion which i have incorporated, it all works a lot better and faster, so thanks very much.
There is still one part of the code that does take forever to complete, I think that it is checking the all of the rows, instead of what I was hoping for, just the filtered rows, I've marked it thus:
'++++++++++
'remove any rows that have zero sales, this part does the job, but takes ages to complete.
'+++++++++

Sub SEE_OnlySoldThisWeek789()

'from a combo box choice stored in named range SUPP_Name to only show that supplier
'run from the MENU sheet


Dim cel As Range, Rng As Range, lr As Long


Sheets("ALLdetails").Visible = True

Sheets("ALLdetails").Select
ActiveSheet.Unprotect
Application.ScreenUpdating = False

'this unhides all cols and finds the supplier
'MAR858
lr = Cells.Find("*", , xlValues, xlPart, xlByRows, xlPrevious, False).Row
Range("M1:M" & lr).AutoFilter Field:=1, Criteria1:=Range("SUPP_Name").Value, VisibleDropDown:=False


'++++++++++
'remove any rows that have zero sales, this part does the job, but takes ages to complete.
'+++++++++

Set Rng = Range("X2", Range("X" & lr)) 'X is the qty sold col


For Each cel In Rng

If cel.Value < 1 Then 'if cell is less than 1, ie none sold this week

cel.EntireRow.Hidden = True 'hide the row

'if greater than 1, ie some sold this week, leave it shown and go to next row
End If

Next cel 'keep going until all rows checked

' no problems on hiding whatever column is needed

ActiveSheet.Columns("K:W").Select
Selection.EntireColumn.Hidden = True
' CL is last col as BF-CI are only heading at present
ActiveSheet.Columns("Y:CL").Select
Selection.EntireColumn.Hidden = True

'scroll to the top
ActiveWindow.ScrollRow = 2
ActiveWindow.ScrollColumn = 1

ActiveSheet.Protect

'hide the menu sheet
Sheets("MENU").Visible = False

Application.ScreenUpdating = True
End Sub

 

[Roxxien]

If you want to be sure to declare your variable correctly, simply put "Option explicit" at the top of your modules.

 

[MARK858]

Why can't you autofilter for >0 as well as Range("SUPP_Name").Value rather than looping through the range hiding rows?

Also why does the sort need to take place after the Range("SUPP_Name").Value filter and not before?

You also need to stop using select and selection as it slows down the code.

If you want to be sure to declare your variable correctly, simply put "Option explicit" at the top of your modules.

Option Explicit doesn't make you declare them correctly, it just forces you to declare the variables.

For instance in the OP's post you could just do

Dim cel, Rng, lr

Which Option Explicit will be happy with even though you are declaring them all as Variant.

 

[gaz1959]

Hi, I've added option explicit as Roxxien suggested, and changed the order around (I think I have done it right). No reason for the sort to take place before or after, whichever works best. I have tried the >0 which is what i would prefer to do, but really do not undertand how to add 2 sorts, I tried adding a second line and got in all sorts of troubles.
Here is my modified code, thanks again to all.

Option Explicit

Sub SEE_OnlySoldThisWeek789()

'from a combo box choice stored in named range SUPP_Name to only show that supplier
'run from the MENU sheet

'+++++++
Dim cel As Range, Rng As Range, lr As Long
'++++++++

Sheets("ALLdetails").Visible = True

Sheets("ALLdetails").Select
ActiveSheet.Unprotect
Application.ScreenUpdating = False

lr = Cells.Find("*", , xlValues, xlPart, xlByRows, xlPrevious, False).Row

'++++++++++
'remove any rows that have zero sales, this part does the job, but takes ages to complete.
'+++++++++

Set Rng = Range("X2", Range("X" & lr)) 'X is the qty sold col


For Each cel In Rng

If cel.Value < 1 Then 'if cell is less than 1, ie none sold this week

cel.EntireRow.Hidden = True 'hide the row

'if greater than 1, ie some sold this week, leave it shown and go to next row
End If

Next cel 'keep going until all rows checked

'this unhides all cols and finds the supplier
'MAR858

Range("M1:M" & lr).AutoFilter Field:=1, Criteria1:=Range("SUPP_Name").Value, VisibleDropDown:=False
' no problems here

ActiveSheet.Columns("K:W").Select
Selection.EntireColumn.Hidden = True
' CL is last col as BF-CI are only heading at present
ActiveSheet.Columns("Y:CL").Select
Selection.EntireColumn.Hidden = True

'scroll to the top
ActiveWindow.ScrollRow = 2
ActiveWindow.ScrollColumn = 1

ActiveSheet.Protect

'hide the menu sheet
Sheets("MENU").Visible = False

Application.ScreenUpdating = True
End Sub

 

[MARK858]

Possibly (untested)...

Sub SEE_OnlySoldThisWeek789()

    'from a combo box choice stored in named range SUPP_Name to only show that supplier
    'run from the MENU sheet

    '+++++++
    Dim cel As Range, Rng As Range, lr As Long
    '++++++++

    Sheets("ALLdetails").Visible = True

    With Sheets("ALLdetails")
        .Unprotect
        Application.ScreenUpdating = False

        lr = .Cells.Find("*", , xlValues, xlPart, xlByRows, xlPrevious, False).Row

        .Range("H2", .Range("BG" & Rows.Count).End(xlUp).Address).Sort key1:=.Range("H2"), order1:=xlDescending, Header:=xlNo

        '++++++++++
        'remove any rows that have zero sales, this part does the job, but takes ages to complete.
        '+++++++++

        With .Range("M1:CL" & lr)
            .AutoFilter Field:=1, Criteria1:= Range("SUPP_Name").Value
            .AutoFilter Field:=12, Criteria1:=">0", VisibleDropDown:=False
        End With

        'scroll to the top
        Application.Goto .Range("A2")

        .Protect
    End With

    'hide the menu sheet
    Sheets("MENU").Visible = False

    Application.ScreenUpdating = True
End Sub