I have details of key decisions made by a business in a worksheet Decision Record.My data is laid out as follows:I have written some code to create a timeline of these decisions using an XY scatter chart, like this one:The code works fine, but as with most of the code I write, it doesn't look tidy to me. Perhaps it's because of the long with statement, which seems like it could be simplified, maybe with a separate with statement for the DataLabels method.Is this good practice? Is there something else I should be doing to tidy this up?Sub UpdateDecisionTimelineChart()    Dim scount As Integer    Dim labelrotation As Integer    Dim c As Range    ActiveSheet.ChartObjects(chtDecisionTimeline).Activate    scount = 0        For Each c In Range(Worksheets(Decision Record).Range(C7), Worksheets(Decision Record).Range(C7).End(xlDown))        scount = scount + 1        ActiveChart.SeriesCollection.NewSeries        With ActiveChart.SeriesCollection(scount)            .Name = ='Decision Record'! & c.Offset(0, 1).Address            .XValues = ='Decision Record'! & c.Address            .Values = ='Decision Record'! & c.Offset(0, -1).Address            .MarkerStyle = 8            .MarkerSize = 7            .MarkerBackgroundColor = RGB(228, 10, 56)            .MarkerForegroundColor = -2            .Format.Line.Visible = False            .ApplyDataLabels            .DataLabels.ShowValue = False            .DataLabels.ShowSeriesName = True            .DataLabels.Position = xlLabelPositionAbove            .DataLabels.Orientation = -45        End With        NextEnd Sub
Excel VBA to create a visual timeline of events using an XY scatter chart
vba;excel
Your variable names could use some tidying up-Instead of c why not record as they are in the Decision Recordlabelrotation should be labelRotation - Standard VBA naming conventions have camelCase for local variables and PascalCase for other variables and names.Same goes for scount - sCount or even recordCountIntegers - integers are obsolete. According to msdn VBA silently converts all integers to long.Be sure to avoid things like .Activate - it just slows the code down by needing to fiddle with the spreadsheet while doing everything else behind the scenes. There's a good question on StackOverflow addressing this - https://stackoverflow.com/questions/10714251/how-to-avoid-using-select-in-excel-vba-macros .Dim timelineChart As Chart    Set timelineChart = ActiveSheet.ChartObjects(chtDecisionTimeline)    ...        timelineChart.SeriesCollection.NewSeries        With timelineChart.SeriesCollection(scount)Speaking of structure, your spacing is a little off - your with block is the same level as your for block. Try pushing the loop back so alignment is correct -Option ExplicitSub UpdateDecisionTimelineChart()    Dim sCount As Long    Dim labelrotation As Long    Dim record As Range    Dim timelineChart As Chart    Set timelineChart = ActiveSheet.ChartObjects(chtDecisionTimeline)    sCount = 0    For Each record In Range(Worksheets(Decision Record).Range(C7), Worksheets(Decision Record).Range(C7).End(xlDown))        sCount = sCount + 1        timelineChart.SeriesCollection.NewSeries        With timelineChart.SeriesCollection(sCount)            .Name = ='Decision Record'! & record.Offset(0, 1).Address            .XValues = ='Decision Record'! & record.Address            .Values = ='Decision Record'! & record.Offset(0, -1).Address            .MarkerStyle = 8            .MarkerSize = 7            .MarkerBackgroundColor = RGB(228, 10, 56)            .MarkerForegroundColor = -2            .Format.Line.Visible = False            .ApplyDataLabels            .DataLabels.ShowValue = False            .DataLabels.ShowSeriesName = True            .DataLabels.Position = xlLabelPositionAbove            .DataLabels.Orientation = -45        End With    NextEnd SubThe object model for the series object doesn't really specify any of the attributes' defaults, so I think your With block is pretty clean. You might want to create some variables or something like -Dim decisionRecord As StringdecisionRecord = ='Decision Record'!        .Name = decisionRecord & record.Offset(0, 1).Address        .XValues = decisionRecord & record.Address        .Values = decisionRecord & record.Offset(0, -1).AddressYour xlDown could be fixed like this -Dim decisionRecordSheet As WorksheetSet decisionRecordSheet = Worksheets(Decision Record)Dim lastRow As LonglastRow = decisionRecordSheet.Cells(Rows.Count, 3).End(xlUp).RowDim recordRange As RangeSet recordRange = decisionRecordSheet.Range(Cells(7, 3), Cells(lastRow, 3))...For Each record In recordRangeWith all these variables using the sheet, why not use the CodeName - Worksheets have a CodeName property - View Properties window (F4) and the (Name) field can be used as the worksheet name. This way you can avoid Sheets(mySheet) and instead just use mySheet.Unless you need it to be Public you should make it privatePrivate Sub UpdateDecisionTimelineChart()Also, I don't see you ever using the labelRotation variable, so unless that part was removed, you don't need it.