Turn logged data into fancy plot

Question

I'm logging data from an embedded platform over UART. This gets saved to a log file and I want to process the saved log with Excel.

I have no idea whether what I did was even remotely a good solution. My VBA experience so far consists mainly of code only used once. However, since I'll have to use this for every measurement series I conduct it's probably a good idea to clean it up. Not in the least because I can't stand ugly code, I'm about to expand on it and colleagues may want to use it in the near future as well.

Example input:

00032
00016
00016
00016
00032
00064
00080
00096
00112
00128
00160
00192

The size of the input can be thousands of values. All leading zeroes get dropped as intended. Values can be negative.

Option Explicit
Public Sub Stats()
'
' Stats Macro
' Give stats to column A and draw chart with trendline.
'
' Keyboard Shortcut: Ctrl+Shift+M
'
 Columns("A:A").Select
 Selection.FormatConditions.AddColorScale ColorScaleType:=3
 Selection.FormatConditions(Selection.FormatConditions.Count).SetFirstPriority
 Selection.FormatConditions(1).ColorScaleCriteria(1).Type = _
 xlConditionValueLowestValue
 With Selection.FormatConditions(1).ColorScaleCriteria(1).FormatColor
 .Color = 7039480
 .TintAndShade = 0
 End With
 Selection.FormatConditions(1).ColorScaleCriteria(2).Type = _
 xlConditionValuePercentile
 Selection.FormatConditions(1).ColorScaleCriteria(2).Value = 50
 With Selection.FormatConditions(1).ColorScaleCriteria(2).FormatColor
 .Color = 8711167
 .TintAndShade = 0
 End With
 Selection.FormatConditions(1).ColorScaleCriteria(3).Type = _
 xlConditionValueHighestValue
 With Selection.FormatConditions(1).ColorScaleCriteria(3).FormatColor
 .Color = 8109667
 .TintAndShade = 0
 End With
 ActiveSheet.Shapes.AddChart2(227, xlLine).Select
 ActiveChart.SetSourceData Source:=Range("Sheet1!$A:$A")
 ActiveChart.FullSeriesCollection(1).Trendlines.Add Type:=xlMovingAvg, Period _
 :=2, Forward:=0, Backward:=0, DisplayEquation:=0, DisplayRSquared:=0, _
 Name:="2 per. Mov. Avg. (Series1)"
 ActiveChart.ClearToMatchStyle
 ActiveChart.ChartStyle = 233
 ActiveChart.FullSeriesCollection(1).Trendlines(1).Select
 With Selection.Format.Line
 .Visible = msoTrue
 .ForeColor.ObjectThemeColor = msoThemeColorAccent2
 .ForeColor.TintAndShade = 0
 .ForeColor.Brightness = -0.25
 .Transparency = 0
 End With
 Range("C1").Select
 ActiveCell.FormulaR1C1 = "Average:"
 Range("D1").Select
 ActiveCell.FormulaR1C1 = "=AVERAGE(C[-3])"
 Range("C2").Select
 ActiveCell.FormulaR1C1 = "Minimum:"
 Range("D2").Select
 ActiveCell.FormulaR1C1 = "=MIN(C[-3])"
 Range("C3").Select
 ActiveCell.FormulaR1C1 = "Maximum:"
 Range("D3").Select
 ActiveCell.FormulaR1C1 = "=MAX(C[-3])"
 Range("C4").Select
 ActiveCell.FormulaR1C1 = "Median:"
 Range("D4").Select
 ActiveCell.FormulaR1C1 = "=MEDIAN(C[-3])"
End Sub

Result looks like this:

Screenshot of result

I'm mainly worried about the lack of being generic. While ActiveCell and ActiveSheet and ActiveChart work wonders, I haven't figured out how to apply the same logic everywhere. There are still values like Sheet1 which should be set automatically for the current sheet/object/etc..

Answer 1

The golden rule of using the Macro Recorder:

The code is more what you'd call "guidelines" than actual rules.

It's there to tell you which objects/properties/methods to look at. Not to write your code for you.

---

Step 1: Clean up all the implicit references (Selection, Active*), indenting and line continuations

When the Recorder spits out this code:

Thing.Select
Selection.DoThing
Selection.Child.DoThing
Selection.Child.Child.Property = _
 Value

You re-write it like so:

Thing.DoThing
Thing.Child.DoThing
Thing.Child.Child.Property = Value

And then like so:

With Thing
 .DoThing
 With .Child
 .DoThing
 .Child.Property = Value
 End With
End With

And then you go and put Thing in a proper variable so you can apply it to any generic ThingObject:

Dim targetThing as ThingObject
Set targetThing = Thing
With targetThing 
 .DoThing
 With .Child
 .DoThing
 .Child.Property = Value
 End With
End With

Applying to your code:

Dim targetCol As Range
Set targetCol = Columns("A:A")
With targetCol
 .FormatConditions.AddColorScale ColorScaleType:=3
 .FormatConditions(.FormatConditions.Count).SetFirstPriority
 .FormatConditions(1).ColorScaleCriteria(1).Type = xlConditionValueLowestValue
End With
With targetCol.FormatConditions(1).ColorScaleCriteria(1).FormatColor
 .Color = 7039480
 .TintAndShade = 0
End With
With targetCol.FormatConditions(1).ColorScaleCriteria(2)
 .Type = xlConditionValuePercentile
 .value = 50
 With .FormatColor
 .Color = 8711167
 .TintAndShade = 0
 End With
End With
With targetCol.FormatConditions(1).ColorScaleCriteria(3)
 .Type = xlConditionValueHighestValue
 With .FormatColor
 .Color = 8109667
 .TintAndShade = 0
 End With
End With
Dim targetSheet As Worksheet
Set targetSheet = ActiveSheet
Dim targetChart As Chart
Set targetChart = targetSheet.AddChart2(227, xlLine)
targetChart.SetSourceData source:=Range("Sheet1!$A:$A")
targetChart.FullSeriesCollection(1).Trendlines.Add Type:=xlMovingAvg, Period:=2, Forward:=0, Backward:=0 _
, DisplayEquation:=0, DisplayRSquared:=0, Name:="2 per. Mov. Avg. (Series1)"
targetChart.ClearToMatchStyle
targetChart.ChartStyle = 233
With targetChart.FullSeriesCollection(1).Trendlines(1).Format.Line
 .Visible = msoTrue
 .ForeColor.ObjectThemeColor = msoThemeColorAccent2
 .ForeColor.TintAndShade = 0
 .ForeColor.Brightness = -0.25
 .Transparency = 0
End With
Range("C1").FormulaR1C1 = "Average:"
Range("D1").FormulaR1C1 = "=AVERAGE(C[-3])"
Range("C2").FormulaR1C1 = "Minimum:"
Range("D2").FormulaR1C1 = "=MIN(C[-3])"
Range("C3").FormulaR1C1 = "Maximum:"
Range("D3").FormulaR1C1 = "=MAX(C[-3])"
Range("C4").FormulaR1C1 = "Median:"
Range("D4").FormulaR1C1 = "=MEDIAN(C[-3])"

and already it's much clearer how things are structured and what's going on. It's also clearly separated into 3 distinct operations (Add conditional Formatting, Add Chart, Add Summary Stats) which should be refactored into separate Subs.

---

Step 2: Initial Refactoring

Moving the 3 stages to separate subs. Tweaked the naming. Made AddLineChart a function which returns the chart object after creation for further manipulation (if desired). Made the summary stat ranges relative to the topLeftCell so you can move the table around by changing just one range declaration:

Option Explicit
Public Sub Stats()
 Dim dataRange As Range
 Set dataRange = Columns("A:A")
 AddConditionalFormatting dataRange
 Dim targetSheet As Worksheet
 Set targetSheet = ActiveSheet
 Dim targetChart As Chart
 Set targetChart = CreateLineChart(dataRange, targetSheet)
 Dim summaryStartCell As Range
 Set summaryStartCell = targetSheet.Range("C1")
 AddSumaryStats summaryStartCell '/ Moved the range declarations to be relative to base cell for portability.
End Sub
Public Sub AddConditionalFormatting(ByRef dataRange As Range)
 With dataRange
 .FormatConditions.AddColorScale ColorScaleType:=3
 .FormatConditions(.FormatConditions.Count).SetFirstPriority
 .FormatConditions(1).ColorScaleCriteria(1).Type = xlConditionValueLowestValue
 End With
 With dataRange.FormatConditions(1).ColorScaleCriteria(1).FormatColor
 .Color = 7039480
 .TintAndShade = 0
 End With
 With dataRange.FormatConditions(1).ColorScaleCriteria(2)
 .Type = xlConditionValuePercentile
 .value = 50
 With .FormatColor
 .Color = 8711167
 .TintAndShade = 0
 End With
 End With
 With dataRange.FormatConditions(1).ColorScaleCriteria(3)
 .Type = xlConditionValueHighestValue
 With .FormatColor
 .Color = 8109667
 .TintAndShade = 0
 End With
 End With
End Sub
Public Function CreateLineChart(ByRef dataRange As Range, ByRef targetSheet As Worksheet) As Chart
 Dim newChart As Chart
 Set newChart = targetSheet.AddChart2(227, xlLine)
 With newChart
 .SetSourceData source:=dataRange
 .FullSeriesCollection(1).Trendlines.Add Type:=xlMovingAvg, Period:=2, Forward:=0, Backward:=0 _
 , DisplayEquation:=0, DisplayRSquared:=0, Name:="2 per. Mov. Avg. (Series1)"
 .ClearToMatchStyle
 .ChartStyle = 233
 End With
 With newChart.FullSeriesCollection(1).Trendlines(1).Format.Line
 .Visible = msoTrue
 With .ForeColor
 .ObjectThemeColor = msoThemeColorAccent2
 .TintAndShade = 0
 .Brightness = -0.25
 End With
 .Transparency = 0
 End With
 Set CreateLineChart = newChart
End Function
Public Sub AddSummaryStats(ByRef baseCell As Range)
 Dim rowOffset As Long
 rowOffset = 0
 baseCell.Offset(rowOffset, 0).FormulaR1C1 = "Average:"
 baseCell.Offset(rowOffset, 1).FormulaR1C1 = "=AVERAGE(C[-3])"
 rowOffset = rowOffset + 1
 baseCell.Offset(rowOffset, 0).FormulaR1C1 = "Minimum:"
 baseCell.Offset(rowOffset, 1).FormulaR1C1 = "=MIN(C[-3])"
 rowOffset = rowOffset + 1
 baseCell.Offset(rowOffset, 0).FormulaR1C1 = "Maximum:"
 baseCell.Offset(rowOffset, 1).FormulaR1C1 = "=MAX(C[-3])"
 rowOffset = rowOffset + 1
 baseCell.Offset(rowOffset, 0).FormulaR1C1 = "Median:"
 baseCell.Offset(rowOffset, 1).FormulaR1C1 = "=MEDIAN(C[-3])"
End Sub

And now we've successfully split our task into 3 small, distinct, operations that can be further analysed/improved/refactored as necessary.

Answer 2

Avoid using Active*

You can use Worksheets collection to access a specified Worksheet object represents a worksheet.

Worksheets("YOUR SHEET NAME")

This can be useful to do explicit job, without changing the active sheet. The same way for ActiveCell, ActiveChart and ActiveChart, you can always access the target object by the collection.

Use With rather than Selection

Aside from error-raising macro problems, a recorded macro is less efficient, because it mimics all the mouseclicks and keystrokes (every cough and camera flash) that occurred while the recording was taking place. A recorded macro clicks on every object to select it, then performs an action on the selection:

Columns("A:A").Select
Selection.FormatConditions.AddColorScale ColorScaleType:=3
Selection.FormatConditions(Selection.FormatConditions.Count).SetFirstPriority

If you have two or more property or method statements that work on the same object, wrap them in a With / End With block:

With Worksheets("Sheet1").Columns("A")
 ' read / write object's properties here
 ' e.g. add new color scale rule
 .FormatConditions.AddColorScale ColorScaleType:=3
End With

This block can increase the code efficiency and readability.

Check the existence of target object

The macro record is powerful, but it's more difficult to understand whether something exists already. For example, we may not want to apply 10 rules of color scale to the column A.

' drop color scale if exists
For Each fc In .FormatConditions
 If fc.Type = xlColorScale Then
 fc.Delete
 End If
Next

Here's just an example, there must be better logic for treating existing object.

---

As for your script, here's my suggest edits.

Public Sub Stats2()
'
' Stats Macro
' Give stats to column A and draw chart with trendline.
'
' Modified by Mincong HUANG
'
 ' declarations
 Dim wsName As String
 Dim chartName As String
 Dim srcRange As Range
 ' initialization
 wsName = "Sheet1"
 chartName = "Fancy Chart"
 Set srcRange = Worksheets(wsName).Columns("A")
 ' set format conditions at target column
 With srcRange
 Debug.Print .Address
 ' drop color scale if exists
 For Each fc In .FormatConditions
 If fc.Type = xlColorScale Then
 fc.Delete
 End If
 Next
 ' add new color scale
 With .FormatConditions
 ' add
 .AddColorScale ColorScaleType:=3
 With .Item(.Count)
 .SetFirstPriority
 ' advanced setting (optional)
 ' Optional because these values are the default setting
 ' but you can add other desired color
 .ColorScaleCriteria(1).Type = xlConditionValueLowestValue
 .ColorScaleCriteria(1).FormatColor.Color = 7039480
 .ColorScaleCriteria(1).FormatColor.TintAndShade = 0
 .ColorScaleCriteria(2).Type = xlConditionValuePercentile
 .ColorScaleCriteria(2).FormatColor.Color = 8711167
 .ColorScaleCriteria(2).FormatColor.TintAndShade = 0
 .ColorScaleCriteria(2).Value = 50
 .ColorScaleCriteria(3).Type = xlConditionValueHighestValue
 .ColorScaleCriteria(3).FormatColor.Color = 8109667
 .ColorScaleCriteria(3).FormatColor.TintAndShade = 0
 End With
 End With
 End With
 ' drop log chart if exists
 Debug.Print Worksheets(wsName).ChartObjects.Count & " charts"
 For Each c In Worksheets(wsName).ChartObjects
 If c.Name = chartName Then
 c.Delete
 End If
 Next
 ' add log chart
 With Worksheets(wsName).Shapes.AddChart2(227, xlLine)
 .Chart.Parent.Name = chartName
 .Chart.HasTitle = True
 .Chart.ChartTitle.Text = chartName
 .Chart.SetSourceData Source:=srcRange
 .Chart.FullSeriesCollection(1).Trendlines.Add _
 Type:=xlMovingAvg, _
 Period:=2, _
 Forward:=0, _
 Backward:=0, _
 DisplayEquation:=0, _
 DisplayRSquared:=0, _
 Name:="2 per. Mov. Avg. (Series1)"
 .Chart.ClearToMatchStyle
 .Chart.ChartStyle = 233
 With .Chart.FullSeriesCollection(1).Trendlines(1).Format.Line
 .Visible = msoTrue
 .ForeColor.ObjectThemeColor = msoThemeColorAccent2
 .ForeColor.TintAndShade = 0
 .ForeColor.Brightness = -0.25
 .Transparency = 0
 End With
 End With
 ' release memory
 Set srcRange = Nothing
End Sub

By the way, I'm using Excel 2016. So I'm not sure the code compatibility. Please tell me if there's any error.

---

References

• Peltier Tech Blog - Quick Excel Chart VBA Examples
• Peltier Tech Blog - Naming an Excel Chart
• MSDN - Worksheets Object (Excel)

• Mast: Worksheets("YOUR SHEET NAME") How is that an improvement? I'm trying to get rid of such calls. Changing the active sheet seems like a more maintainable and easier to extend approach, avoiding code duplication in the process. Don't get me wrong, I appreciate the suggestion, but I fail to see why it's better.
• Mincong Huang: Effectively, it depends the usage. I consider it as an improvement, because by precising the target sheet name, this sub Stats can take care multiple sheets. For example, if there're many log, you may want to use a sheet Dashboard to show charts, which referenced log data from sheets log 1, log 2, log 3. But if log and chart are shown together, ActiveSheet is better.
• Mast: Valid points, I'll have to figure out the final use-case and act accordingly.

Answer 3

I've found Sheet1 only once in your code:
Range("Sheet1!$A:$A"),
You can replace it to
ActiveSheet.Columms(1)

(Or, to make your code even more general, working also in cases where original data is not in first column: Selection.EntireColumn)

As @MincongHuang has suggested you can do more with With clauses, they can even be embedded in each other.