HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

VBA Data Conversion (random printout -> CSV file)

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
randomconversionfilecsvprintoutvbadata

Problem

I have reviewed the FAQ and I believed it was yes to all 5 q's of "What questions are on-topic for this site?"

Basically, the code below is used to perform the operation shown in the picture. I'm wondering what I can improve in the attached code to clean it up for future reference. Thanks in advance!

```
Sub Button1_Click()

Dim orgFilename As String
Dim temp As String
Dim strarray(3) As String
Dim vert(4) As String
Dim vert2(3) As String
Dim newFilename As String
Dim numRows As Integer
Dim i As Integer
Dim j As Integer
Dim k As Integer
Dim segCount As Integer
Dim vertex(3, 100) As Double

Dim oldwb As Workbook
Dim newwb As Workbook

orgFilename = Application.GetOpenFilename(FileFilter:="All files (.), .", Title:="Please select a file")
If orgFilename = "False" Then Exit Sub
Workbooks.OpenText Filename:=orgFilename, _
Origin:=950, StartRow:=1, DataType:=xlDelimited, TextQualifier:= _
xlDoubleQuote, ConsecutiveDelimiter:=True, Tab:=True, Semicolon:=False, _
Comma:=False, Space:=True, Other:=False, FieldInfo:=Array(Array(1, 1), _
Array(2, 1), Array(3, 1)), TrailingMinusNumbers:=True
Set oldwb = ActiveWorkbook
Set newwb = Workbooks.Add

oldwb.Activate
Cells(5, 1).Select
numRows = Cells(5, 1).End(xlDown).Row

' Parse through data
segCount = 0
j = 1
For i = 5 To numRows
If Cells(i, 1) <> "VRTX" And segCount <> 0 Then
For k = 1 To segCount - 1
newwb.Worksheets("Sheet1").Cells(j, 1) = "GLINE"
With newwb.Worksheets("Sheet1")
.Cells(j, 2) = vertex(1, k)
.Cells(j, 3) = vertex(3, k)
.Cells(j, 4) = vertex(2, k)
.Cells(j, 5) = vertex(1, k + 1)
.Cells(j, 6) = vertex(3, k + 1)
.Cells(j, 7) = vertex(2, k + 1)
End With
j = j + 1
Next k
segCount = 0
ElseIf Cells(i, 1) = "VRTX" Then
' Save vertices to save an endpoint
vertex(1, segCount + 1) = C

Solution

Your code does not look bad overall. Here are my 2 cents:

-
To improve readability, I would split your procedure in sub procedures / functions (does not make a difference if this is the only function in the module, but makes a difference in larger projects). The main procedure could look like this for example (simplified):

Sub main()
  openPlFile
  readPlFile
  writeCsvFile
  saveCsvFile
End Sub


-
You could add some error handling logic:

Right after your declarations:

On Error GoTo error_handler


And at the end of the code:

Exit Sub
error_handler:
  'code to handle the error for example:
  MsgBox "There was an error: " & Err.Description
End Sub


-
If performance is an issue there are a few steps to improve the code:

a. You can turn off ScreenUpdating and turn Calculation to manual (don't forget to turn them back on before exiting AND in the error_handler block if there is one).

b. If you want to improve performance further, you could use arrays instead of reading from / writing to the spreadsheet directly in a loop (see http://www.avdf.com/apr98/art_ot003.html)

c. If you want to improve performance even more, you could read and write the files directly without opening / creating workbooks.

d. I generally avoid using Integer as they use the same memory space as Longs anyway (on 32 or 64 bit systems) - VBA simply applies different overflow rules. Declaring Longs instead of Integers can result in a slight performance gain (not so much in your case, but can be useful when declared in a loop for example)

Code Snippets

Sub main()
  openPlFile
  readPlFile
  writeCsvFile
  saveCsvFile
End Sub
On Error GoTo error_handler
Exit Sub
error_handler:
  'code to handle the error for example:
  MsgBox "There was an error: " & Err.Description
End Sub

Context

StackExchange Code Review Q#8767, answer score: 3

Revisions (0)

No revisions yet.