patternMinor
VBA Data Conversion (random printout -> CSV file)
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
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):
-
You could add some error handling logic:
Right after your declarations:
And at the end of the code:
-
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)
-
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_handlerAnd 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 SubOn Error GoTo error_handlerExit Sub
error_handler:
'code to handle the error for example:
MsgBox "There was an error: " & Err.Description
End SubContext
StackExchange Code Review Q#8767, answer score: 3
Revisions (0)
No revisions yet.