1
\$\begingroup\$

This code is tested and works, but I am needing some guidance on simplifying these loops with non-contiguous ranges. I was thinking an array might be better, but I haven't had enough experience with Arrays to know where to start with one. Any guidance would be greatly appreciated.

Dim r As Range
Dim v
For Each r In JHACheck.Range("M7:M32")
    v = r.Value
    If v <> "" Then
        If Not r.HasFormula Then
            r.Value = Trim(v)
        End If
    End If
Next r

For Each r In JHACheck.Range("M35:M41")
    v = r.Value
    If v <> "" Then
        If Not r.HasFormula Then
            r.Value = Trim(v)
        End If
    End If
Next r

For Each r In JHACheck.Range("R35:R41")
    v = r.Value
    If v <> "" Then
        If Not r.HasFormula Then
            r.Value = Trim(v)
        End If
    End If
Next r
\$\endgroup\$
  • 1
    \$\begingroup\$ Are the ranges the same every time you run the macro? if they are you could try For Each r In JHACheck["M7:M32,M35:M41R35:R41"] It wouldn't decrease the speed but it would reduce the duplication of code. Otherwise you would need to create an array of ranges to reduce the duplication. \$\endgroup\$ – R. Roe May 1 at 22:23
  • \$\begingroup\$ Thanks. I'll give it a try when I'm back in the office tomorrow. \$\endgroup\$ – Zack E May 1 at 22:33
1
\$\begingroup\$

The small amount of data doesn't merit the use of arrays. I agree with R. Roe comment on combining the Ranges.

But first:

  • V is a very unhelpful helper variable, I would get rid of it.
  • Personally, I use r to iterate rows row, c to iterate columns and cell to iterate cells in a Range
  • Techiniquelly, the two if statements are more efficient than combining a single If statement. Realistically, it will not make a noticeable difference with your code. I would combine the Ifs and save the clutter.

Dim cell As Range
For Each cell In JHACheck.Range("M7:M32,M35:M41,R35:R41")
    If Not r.HasFormula And cell.Value <> "" Then cell.Value = Trim(cell.Value)
Next
\$\endgroup\$
  • \$\begingroup\$ Thanks! For some strange reason I was under the wrong assumption that I couldnt loop through non-contiguous cells like this. \$\endgroup\$ – Zack E May 2 at 12:55
  • \$\begingroup\$ @ZackE I think that they changed it in recent years. I'm pretty sure that in Excel 2007, we had to iterate over each cell of each area of a range, \$\endgroup\$ – TinMan May 3 at 5:01
2
\$\begingroup\$

It's clear you're performing the same operation over different ranges of cells. So I'd separate that logic in a Function which you can call with any range you like. Using arrays achieve a speed up if the ranges are large, but for now wait until you think you need it. (Needing to check the cell for .HasFormula will slow down using an array in any case.)

The code below shows the trimming logic isolated to a single function. The logic is no different from yours with the exception of more clear variable names.

Option Explicit

Sub test()
    TrimTheseCells JHACheck.Range("M7:M32")
    TrimTheseCells JHACheck.Range("M35:M41")
    TrimTheseCells JHACheck.Range("R35:R41")
End Sub

Private Sub TrimTheseCells(ByRef thisRange As Range)
    Dim cell As Range
    For Each cell In thisRange
        If Not IsEmpty(cell) Then
            If Not cell.HasFormula Then
                cell.Value = Trim(cell.Value)
            End If
        End If
    Next cell
End Sub
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.