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

Next revision letter

Submitted by: @import:stackexchange-codereview··
0
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:

  • 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 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 Function

Code 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 Function

Context

StackExchange Code Review Q#90765, answer score: 11

Revisions (0)

No revisions yet.