patternModerate
Next revision letter
Viewed 0 times
letterrevisionnext
Problem
This is a fun one! I wrote some code to automatically get the next revision letter for a file by looking at the existing letter. It works, but I think there is room for improvement as far as simplifying it or making more efficient.
Our revision scheme follows ASME Y14.35, which specifies the following rules:
is "A", and subsequent revisions increment by one letter.
One caveat is that I'm working with some legacy data, and in the past there was no consistency in how the "rev" field was filled out in the database I pull the current rev from. As a result, sometimes for a rev "-" file, the rev field will contain a "-" or a space " " or a zero-length string "" or tilde "~", etc. So the decision was made to handle all non-letters like a rev "-".
```
Option Explicit
'***NOTE: _
This module requires the following references: _
Function NextRevLetter(strCurrentRev As String) As String
'This function returns the next revision letter given an existing revision letter.
'Declare variables
Dim lngX As Long
Dim strX As String
Dim strY As String
Dim strZ As String
'First, check if we are dealing with rev A-Z or AA-ZZ
If Len(strCurrentRev) = 65 And Asc(strCurrentRev) 89 Then
'Next rev is AA. Return the updated revision and exit function
NextRevLetter = "AA"
Exit Function
End If
Else
'If we received a non letter, then the rev will be A (e.g. recieved "-")
lngX = 65
Our revision scheme follows ASME Y14.35, which specifies the following rules:
- First release is revision "-".
- First revision to a released document
is "A", and subsequent revisions increment by one letter.
- Skip letters "I", "O", "Q", "S", "X", "Z"
- After rev "Y" the next rev is "AA", and then (following the same rules still) the second letter increments for each revision (e.g. "AB", "AC", etc.) until "AY" is reached, then it goes to "BA", then "BB", etc.
- "YYY" is the maximum and feasibly should never be reached.
One caveat is that I'm working with some legacy data, and in the past there was no consistency in how the "rev" field was filled out in the database I pull the current rev from. As a result, sometimes for a rev "-" file, the rev field will contain a "-" or a space " " or a zero-length string "" or tilde "~", etc. So the decision was made to handle all non-letters like a rev "-".
```
Option Explicit
'***NOTE: _
This module requires the following references: _
Function NextRevLetter(strCurrentRev As String) As String
'This function returns the next revision letter given an existing revision letter.
'Declare variables
Dim lngX As Long
Dim strX As String
Dim strY As String
Dim strZ As String
'First, check if we are dealing with rev A-Z or AA-ZZ
If Len(strCurrentRev) = 65 And Asc(strCurrentRev) 89 Then
'Next rev is AA. Return the updated revision and exit function
NextRevLetter = "AA"
Exit Function
End If
Else
'If we received a non letter, then the rev will be A (e.g. recieved "-")
lngX = 65
Solution
The variable names
You've used recursion to solve this problem, which makes it a lot more complicated than it needs to be. A
Due to the six excluded letters, using ASCII codes to get the next letter is not as useful a strategy. A lookup table (
lngX, strX, strY, and strZ are 75% Hungarian and 25% useless. I don't have a good idea of what their purpose is. You don't have to write a comment telling me that you are declaring variables; the Dim keyword is obvious enough.You've used recursion to solve this problem, which makes it a lot more complicated than it needs to be. A
For loop that considers each character from right to left would be simpler.Due to the six excluded letters, using ASCII codes to get the next letter is not as useful a strategy. A lookup table (
alphabet in the solution below) should simplify the code. As a bonus, since InStr() for any unrecognized character would return 0, such garbage characters would map to "A". And, as it turns out, this function also happens to correctly handle "" as input, due to the way the carrying mechanism works.Public Function NextRevLetter(currentRev As String) As String
' Alphabet, skipping I O Q S X Z
Const alphabet As String = "ABCDEFGHJKLMNPRTUVWY"
NextRevLetter = currentRev
Dim i As Long
For i = Len(NextRevLetter) To 1 Step -1
Dim digit As String
digit = Mid$(NextRevLetter, i, 1)
' Increment digit: "A"->"B", "B"->"C", ..., "H"->"J", ..., "Y"->"".
' Any other garbage maps to "A".
digit = Mid$(alphabet, InStr(alphabet, digit) + 1, 1)
If Len(digit) = 0 Then
' "Y"->"" case. Reset to "A", then carry.
Mid$(NextRevLetter, i, 1) = "A"
Else
Mid$(NextRevLetter, i, 1) = digit
Exit Function
End If
Next
' Carry.
NextRevLetter = "A" & NextRevLetter
End FunctionCode Snippets
Public Function NextRevLetter(currentRev As String) As String
' Alphabet, skipping I O Q S X Z
Const alphabet As String = "ABCDEFGHJKLMNPRTUVWY"
NextRevLetter = currentRev
Dim i As Long
For i = Len(NextRevLetter) To 1 Step -1
Dim digit As String
digit = Mid$(NextRevLetter, i, 1)
' Increment digit: "A"->"B", "B"->"C", ..., "H"->"J", ..., "Y"->"".
' Any other garbage maps to "A".
digit = Mid$(alphabet, InStr(alphabet, digit) + 1, 1)
If Len(digit) = 0 Then
' "Y"->"" case. Reset to "A", then carry.
Mid$(NextRevLetter, i, 1) = "A"
Else
Mid$(NextRevLetter, i, 1) = digit
Exit Function
End If
Next
' Carry.
NextRevLetter = "A" & NextRevLetter
End FunctionContext
StackExchange Code Review Q#90765, answer score: 11
Revisions (0)
No revisions yet.