snippetMinor
Change list separator, parse file, restore list separator to original value
Viewed 0 times
fileoriginalvalueparseseparatorlistchangerestore
Problem
Following up on @Vogel612's friendly advice from the
This will be my first task that I have completely automated via batch file scheduling, so I have included some spaces to pass error messages to the vbscript and batch file that will execute these macros. If some of the error handling seems a little excessive, it's because it is a zealous attempt by me to prevent any possible issues so that I am trusted to automate additional tasks via the scheduler in the future.
Part 1 is simply in charge of changing the list separator and quitting Excel so that Excel can be re-opened with the new list separator active.
```
Option Explicit
Private Declare Function SetLocaleInfo _
Lib "kernel32" Alias "SetLocaleInfoA" ( _
ByVal Locale As Long, _
ByVal LCType As Long, _
ByVal lpLCData As String) As Boolean
Private Declare Function GetUserDefaultLCID% Lib "kernel32" ()
Private Const LOCALE_SLIST = &HC
Private Const LOCALE_NAME_MAX_LENGTH = 85
Private Const LOCALE_NAME_USER_DEFAULT = vbNullString
'Get Locale Info
Private Declare Function GetLocaleInfoEx _
Lib "kernel32" ( _
ByVal lpLocaleName As String, _
ByVal LCType As Long, _
ByVal lpLCData As String, _
ByVal cchData As Long) As Long
Private Declare Function GetLastError Lib "kernel32" () As Long
Sub M1DelimiterSetup()
Dim lngTryAgainCtr As Long
Dim strBuffer As String
Dim strListSeparator As String
Dim lpLCData As String
Dim Long1 As Long
lngTryAgainCtr = 0
TryAgain:
lngTryAgainCtr = lngTryAgainCtr + 1
'Change delimiter to pipe
Call SetLocaleInfo(GetUserDefaultLCID(), LOCALE_SLIST, "|")
'Check to make sure setting separator as pipe worked correctly
strBuffer = String$(85, 0)
Long1 = GetLocaleInfoEx(LOCALE_NAME_USER_DEFAULT, LOCALE_SLIST, lpLCData, 0)
strListSeparator = String$(Long1, 0)
Long1 = GetLocaleInfoEx(LOCALE_NAME_USER_DEFAULT, LOCALE_SLIST, strListSeparat
vba-rubberducking chat room, I am posting my working code for open review. Thanks also to @Mat'sMug for the help!This will be my first task that I have completely automated via batch file scheduling, so I have included some spaces to pass error messages to the vbscript and batch file that will execute these macros. If some of the error handling seems a little excessive, it's because it is a zealous attempt by me to prevent any possible issues so that I am trusted to automate additional tasks via the scheduler in the future.
Part 1 is simply in charge of changing the list separator and quitting Excel so that Excel can be re-opened with the new list separator active.
```
Option Explicit
Private Declare Function SetLocaleInfo _
Lib "kernel32" Alias "SetLocaleInfoA" ( _
ByVal Locale As Long, _
ByVal LCType As Long, _
ByVal lpLCData As String) As Boolean
Private Declare Function GetUserDefaultLCID% Lib "kernel32" ()
Private Const LOCALE_SLIST = &HC
Private Const LOCALE_NAME_MAX_LENGTH = 85
Private Const LOCALE_NAME_USER_DEFAULT = vbNullString
'Get Locale Info
Private Declare Function GetLocaleInfoEx _
Lib "kernel32" ( _
ByVal lpLocaleName As String, _
ByVal LCType As Long, _
ByVal lpLCData As String, _
ByVal cchData As Long) As Long
Private Declare Function GetLastError Lib "kernel32" () As Long
Sub M1DelimiterSetup()
Dim lngTryAgainCtr As Long
Dim strBuffer As String
Dim strListSeparator As String
Dim lpLCData As String
Dim Long1 As Long
lngTryAgainCtr = 0
TryAgain:
lngTryAgainCtr = lngTryAgainCtr + 1
'Change delimiter to pipe
Call SetLocaleInfo(GetUserDefaultLCID(), LOCALE_SLIST, "|")
'Check to make sure setting separator as pipe worked correctly
strBuffer = String$(85, 0)
Long1 = GetLocaleInfoEx(LOCALE_NAME_USER_DEFAULT, LOCALE_SLIST, lpLCData, 0)
strListSeparator = String$(Long1, 0)
Long1 = GetLocaleInfoEx(LOCALE_NAME_USER_DEFAULT, LOCALE_SLIST, strListSeparat
Solution
Okay, I'll say it - variable names. It is awesome that you declared all of them with a type and used
Some of them are good where others are too cryptic or don't have inherent meaning. By describing what they are, you reduce the likelihood of future-you getting confused.
Part 1
But
Your
And why is a
You don't need to use the prefix
Also Standard VBA naming conventions have
Part 2
Again, the prefixes could be removed and the variables would be good descriptions - but why shorten
Seems like those variables could have names AND the comments could be eliminated in one fell swoop.
I have no idea what a
Option Explicit. But, the naming leaves something to be desired.Some of them are good where others are too cryptic or don't have inherent meaning. By describing what they are, you reduce the likelihood of future-you getting confused.
Part 1
Private Declare Function SetLocaleInfo is a good description, as is Locale.But
LCType - is that localeType? And lpLCData - I can't tell what type of data that should be.Your
Const are good - but why do you have a Const = vbNullString? That's already a constant.Private Declare Function GetLocaleInfoEx - what does this do? The Ex means something - why not use the full name? And lpLocaleName and cchData - what are those?And why is a
Data variable a long? Seems counter-intuitive when reading the function.You don't need to use the prefix
str or any other prefix (Hungarian Notation) if your variables have good naming, so you can drop those. Long1 - I find it bad practice to not only use a number in a variable, but also a system reserved name. What does it do? Looks like it is positionOfListSeparator, right?Also Standard VBA naming conventions have
camelCase for local variables and PascalCase for other variables and names. Your constants are great, but some of your locals could be adjusted.Part 2
Again, the prefixes could be removed and the variables would be good descriptions - but why shorten
Address to Addr - you're not paying per character so use them!Dim Object1 As Object 'Shell Application
Dim Object2 As Object 'File system object
Dim Int1 As Integer 'Input file number (system-assigned number for file management)Seems like those variables could have names AND the comments could be eliminated in one fell swoop.
Dim oXMLHTTP As Object
Dim oResp() As Byte
Dim Variant1 As Variant
Dim Variant2 As VariantI have no idea what a
Variant1 and Variant2 are when you declare them, so how would I be able to tell how they are different further down? You catch the drift by now.Code Snippets
Dim Object1 As Object 'Shell Application
Dim Object2 As Object 'File system object
Dim Int1 As Integer 'Input file number (system-assigned number for file management)Dim oXMLHTTP As Object
Dim oResp() As Byte
Dim Variant1 As Variant
Dim Variant2 As VariantContext
StackExchange Code Review Q#126146, answer score: 2
Revisions (0)
No revisions yet.