Re: Tidy up beginners code
Private Sub Worksheet_Change(ByVal Target As Range)
ActiveSheet.Unprotect Password:="mwtaskings"
Application.ScreenUpdating = False
Dim Rng As Range
Dim Response As Script
Range("D8:D22,D27:D41,D46:D60,D65:D79,D84:D98").Select 'All the ranges should be ok if combined as shown here.
Selection.Rows.AutoFit
For Each Rng In Selection.Cells
Debug.Print Left(Rng, 5); Rng.RowHeight
Rng.RowHeight = Application.WorksheetFunction.Max(Rng.RowHeight, 40)
Next Rng
Application.ScreenUpdating = False
Range("A8:J22").Sort _
Key1:=Range("H8"), Order1:=xlDescending, Header:=xlGuess, _
OrderCustom:=1, MatchCase:=False, Orientation:=xlTopToBottom, _
DataOption1:=xlSortNormal
Range("A27:J41").Sort _
Key1:=Range("H27"), Order1:=xlDescending, Header:=xlGuess, _
OrderCustom:=1, MatchCase:=False, Orientation:=xlTopToBottom, _
DataOption1:=xlSortNormal
Range("A46:J60").Sort _
Key1:=Range("H46"), Order1:=xlDescending, Header:=xlGuess, _
OrderCustom:=1, MatchCase:=False, Orientation:=xlTopToBottom, _
DataOption1:=xlSortNormal
Range("A65:J79").Sort _
Key1:=Range("H65"), Order1:=xlDescending, Header:=xlGuess, _
OrderCustom:=1, MatchCase:=False, Orientation:=xlTopToBottom, _
DataOption1:=xlSortNormal
Range("A84:J98").Sort _
Key1:=Range("H65"), Order1:=xlDescending, Header:=xlGuess, _
OrderCustom:=1, MatchCase:=False, Orientation:=xlTopToBottom, _
DataOption1:=xlSortNormal
Application.ScreenUpdating = False
Dim HiddenRow&, RowRange As Range, RowRangeValue&
Const FirstRow As Long = 8
Const LastRow As Long = 98
Const FirstCol As String = "I"
Const LastCol As String = "I"
For HiddenRow = FirstRow To LastRow 'you could put the constant values directly in the loop like
'For HiddenRow = 8 To 98
Set RowRange = Range(FirstCol & HiddenRow & _
":" & LastCol & HiddenRow) ' FirstCol and LastCol are the same, and at any point in
' the loop HiddenRow is the same, so why do this? It could
'be this instead:
'Set RowRange = Range("I" & HiddenRow)
RowRangeValue = Application.Sum(RowRange.Value) ' You are summing a single cell here so:
'^ could be removed, and use this instead:
'If RowRange.Value <> 0 Then
If RowRangeValue <> 0 Then
Rows(HiddenRow).EntireRow.Hidden = False
Else
Rows(HiddenRow).EntireRow.Hidden = True
End If
Next HiddenRow
Dim myid As Long
On Error GoTo ws_exit:
Application.EnableEvents = False
If Not Intersect(Target, Me.Range("D8:D98")) Is Nothing Then
'With Target 'looks like you only use the With once here, so I'd get rid of that and drop
'Target into the IF like this:
If Target.Value <> "" Then
On Error Resume Next
myid = Evaluate(ThisWorkbook.Names("___UniqueId").RefersTo)
myid = myid + 1
ThisWorkbook.Names.Add Name:="___UniqueId", RefersTo:="=" & myid
If Target.Offset(0, 1) = "" Then
Target.Offset(0, 1).Value = "C-" & myid
End If
End If
'End With
End If
ws_exit:
Application.EnableEvents = True
Application.ScreenUpdating = True
ActiveSheet.Protect Password:="mwtaskings", _
DrawingObjects:=True, Contents:=True, Scenarios:=True, AllowFormattingRows:=True
End Sub
Display More
See comments added and play around with various ways of doing things. A lot of variables/constants for no reason, IMO.