ExcelBanter

ExcelBanter (https://www.excelbanter.com/)
-   Excel Programming (https://www.excelbanter.com/excel-programming/)
-   -   Macro works but is there a better way (https://www.excelbanter.com/excel-programming/404814-macro-works-but-there-better-way.html)

Brad

Macro works but is there a better way
 
In the quest to improve myself, any suggestions on better code?
I'm not interested in error trapping because this code will only be run by me.

Sub ExportCashValues()
Dim Filename As String
Dim Numrows As Long
Dim i As Long
Dim j As Long
Dim dur As Long
Dim data As Double
Dim key1 As Range
Dim key2 As Range
Dim key3 As Range
Dim key4 As Range
Set key1 = shtCash.Range("d1:d96064")
Set key2 = shtCash.Range("e1:e96064")
Set key3 = shtCash.Range("g1:g96064")
Set key4 = shtCash.Range("h1:h96064")
Set key5 = shtCash.Range("j1:s96064")
Filename = "C:\myTempDir\cashvalu.txt"
Open Filename For Output As #1
For i = 1 To 96094
For j = 1 To 10
dur = key4(i) + j - 1
If key5(i, j) < "" Then
data = key5(i, j) / 100
Print #1, key1(i); ","; key2(i); ","; dur; ","; Format(data,
"0.00")
End If
Next j
Next i
Close #1
End Sub

Dana DeLouis

Macro works but is there a better way
 
In the quest to improve myself, any suggestions on better code?

Hi. Just some ideas.
There is no Dim statement for key5, so this suggest that you are not using
"Option Explicit" at the top of your module.
I suggest you turn this on going to Tools, Options, Editor Tab, and turn on
"Require Variable Declarations.

Key3 apparently is not used, and might be removed.

Although not required, you may want to load the array, instead of setting a
reference to the worksheet.

key1 = Range("D1:D96064")

Dim Filename As String
Filename = "C:\myTempDir\cashvalu.txt"


I like to combine these two into 1 statement.

Const Filename As String = "C:\myTempDir\cashvalu.txt"

"dur" is calculated on each loop, but may not be used depending on key5.
If you have a lot of blanks in Key5, this could be a "waste."
The 'Comma string is generated often.
Here's one of a few ideas.

Const Filename As String = "C:\myTempDir\cashvalu.txt"
Const Cm As String = "," 'Just a Comma
' etc...

For i = 1 To 96094
For j = 1 To 10
If Key5(i, j) < vbNullString Then
Print #1, _
key1(i); Cm; _
key2(i); Cm; _
key4(i) + j - 1; Cm; _
Format(Key5(i, j) / 100, "0.00")
End If
Next j
Next i

Again, these are just some ideas. :)

--
Dana DeLouis



"Brad" wrote in message
...
In the quest to improve myself, any suggestions on better code?
I'm not interested in error trapping because this code will only be run by
me.

Sub ExportCashValues()
Dim Filename As String
Dim Numrows As Long
Dim i As Long
Dim j As Long
Dim dur As Long
Dim data As Double
Dim key1 As Range
Dim key2 As Range
Dim key3 As Range
Dim key4 As Range
Set key1 = shtCash.Range("d1:d96064")
Set key2 = shtCash.Range("e1:e96064")
Set key3 = shtCash.Range("g1:g96064")
Set key4 = shtCash.Range("h1:h96064")
Set key5 = shtCash.Range("j1:s96064")
Filename = "C:\myTempDir\cashvalu.txt"
Open Filename For Output As #1
For i = 1 To 96094
For j = 1 To 10
dur = key4(i) + j - 1
If key5(i, j) < "" Then
data = key5(i, j) / 100
Print #1, key1(i); ","; key2(i); ","; dur; ",";
Format(data,
"0.00")
End If
Next j
Next i
Close #1
End Sub



Dana DeLouis

Macro works but is there a better way
 
Oops! It's been a while.
Check if using "Write #" might be better than "Print #"

Sub Demo()
Const FileName As String = "C:\Junk.txt"
Open FileName For Output As #1

Print #1, 2; ","; 4; ","; 6
Write #1, 2, 4, 6
Close
End Sub

- -
Dana DeLouis



Brad

Macro works but is there a better way
 
I caught the I didn't "Dim key5 ..."

Like the idea of the constants -

Thanks

"Dana DeLouis" wrote:

In the quest to improve myself, any suggestions on better code?


Hi. Just some ideas.
There is no Dim statement for key5, so this suggest that you are not using
"Option Explicit" at the top of your module.
I suggest you turn this on going to Tools, Options, Editor Tab, and turn on
"Require Variable Declarations.

Key3 apparently is not used, and might be removed.

Although not required, you may want to load the array, instead of setting a
reference to the worksheet.

key1 = Range("D1:D96064")

Dim Filename As String
Filename = "C:\myTempDir\cashvalu.txt"


I like to combine these two into 1 statement.

Const Filename As String = "C:\myTempDir\cashvalu.txt"

"dur" is calculated on each loop, but may not be used depending on key5.
If you have a lot of blanks in Key5, this could be a "waste."
The 'Comma string is generated often.
Here's one of a few ideas.

Const Filename As String = "C:\myTempDir\cashvalu.txt"
Const Cm As String = "," 'Just a Comma
' etc...

For i = 1 To 96094
For j = 1 To 10
If Key5(i, j) < vbNullString Then
Print #1, _
key1(i); Cm; _
key2(i); Cm; _
key4(i) + j - 1; Cm; _
Format(Key5(i, j) / 100, "0.00")
End If
Next j
Next i

Again, these are just some ideas. :)

--
Dana DeLouis



"Brad" wrote in message
...
In the quest to improve myself, any suggestions on better code?
I'm not interested in error trapping because this code will only be run by
me.

Sub ExportCashValues()
Dim Filename As String
Dim Numrows As Long
Dim i As Long
Dim j As Long
Dim dur As Long
Dim data As Double
Dim key1 As Range
Dim key2 As Range
Dim key3 As Range
Dim key4 As Range
Set key1 = shtCash.Range("d1:d96064")
Set key2 = shtCash.Range("e1:e96064")
Set key3 = shtCash.Range("g1:g96064")
Set key4 = shtCash.Range("h1:h96064")
Set key5 = shtCash.Range("j1:s96064")
Filename = "C:\myTempDir\cashvalu.txt"
Open Filename For Output As #1
For i = 1 To 96094
For j = 1 To 10
dur = key4(i) + j - 1
If key5(i, j) < "" Then
data = key5(i, j) / 100
Print #1, key1(i); ","; key2(i); ","; dur; ",";
Format(data,
"0.00")
End If
Next j
Next i
Close #1
End Sub





All times are GMT +1. The time now is 08:01 AM.

Powered by vBulletin® Copyright ©2000 - 2024, Jelsoft Enterprises Ltd.
ExcelBanter.com