Converting a number into Hungarian text

Question

I'd like to convert some integer number from a cell between 0 and 1,000,000,0000 into Hungarian text. I've found already a solution, a VBA function.

How can be improved this solution?

Function num2txthu(cellvalue As Double)
'------------------------------------------------------------------------------
'VBA-function, ami Excel-ben használható.
'Egy cella tartalmát (0 és 1 milliárd közötti egész számot) konvertálja szöveggé.
'
'Virág Imre megoldása alapján, köszönettel! Kissé módosítva, tesztelve.
'(http://www.adatkerteszet.hu/2014/08/osszeg-betuvel-szamok-atirasa-szovegge-keplettel/)
'
'[email protected]
'
'Példák
'           A1 A2    =num2txthu(A1)
' ------------ ----------------------------------------------------------------
'            1 egy
'           10 tíz
'           19 tizenkilenc
'           20 húsz
'           25 huszonöt
'        1 000 ezer
'        1 999 ezerkilencszázkilencvenkilenc
'        2 001 kétezer-egy
'        3 016 háromezer-tizenhat
'       47 563 negyvenhétezer-ötszázhatvanhárom
'      100 000 százezer
'    1 100 000 egymillió-egyszázezer
'    7 001 530 hétmillió-egyezer-ötszázharminc
'    7 491 530 hétmillió-négyszázkilencvenegyezer-ötszázharminc
'    7 490 530 hétmillió-négyszázkilencvenezer-ötszázharminc
'   10 000 000 tízmillió
'  999 999 999 kilencszázkilencvenkilencmillió-kilencszázkilencvenkilencezer-kilencszázkilencvenkilenc
'1 000 000 000 egymilliárd
' ------------ ----------------------------------------------------------------
'           A1 A2    =CONCATENATE("*",UPPER(MID(num2txthu(A1),1,1)),MID(num2txthu(A1),2,200),"*")
' ------------ ----------------------------------------------------------------
'         4672 *Négyezer-hatszázhetvenkettõ*
'         4911 *Négyezer-kilencszáztizenegy*
'------------------------------------------------------------------------------

Dim v_arr(100)
v_number = cellvalue
v_length = Len(v_number)
v_number_name = ""
v_group_name = ""
v_num2txthu = ""

'It will then reevaluate whenever the workbook changes (if your calculation is set to automatic).
Application.Volatile

'szám feltagolása
If v_number > 1000000000 Then
    num2txthu = "#Túl nagy szám! (jó: pozitív egész szám 0 és 1milliárd között)"
    Exit Function
End If
If Fix(v_number) = v_number Then
    For pos = v_length To 1 Step -1
        v_arr(pos) = Val(Mid(v_number, v_length - (pos - 1), 1))
    Next pos
Else
    num2txthu = "#Nem egész szám! (jó: pozitív egész szám 0 és 1milliárd között)"
    Exit Function
End If

'nulla külön kezelve
If v_number = 0 Then num2txthu = "nulla": Exit Function

' pos - position in v_arr
' felosztás a csoportoknak (egyesek, tízesek, százasok) megfelelöen
For pos = v_length To 1 Step -1
    Select Case pos
    Case 24, 21, 18, 15, 12, 9, 6, 3
        GoSub l_group_100s
    Case 23, 20, 17, 14, 11, 8, 5, 2
        GoSub l_group_10s
    Case 22, 19, 16, 13, 10, 7, 4, 1
        GoSub l_group_1s
    End Select
    GoSub l_group_names

    'az eredmény kiegészítése a csoporton belül létrehozott szöveggel
    v_num2txthu = v_num2txthu & v_number_name & v_group_name

    'a csoporton belüli szöveg inicializálása
    v_number_name = ""
    v_group_name = ""
Next pos

'eredmény visszaadása
num2txthu = v_num2txthu

Exit Function

'------------------------------------------------------------------------------
l_group_100s:
    Select Case v_arr(pos)
    Case 1
        'száz, százezer vs. ezeregyszáz, egymillióegyszázezer
        If (v_number >= 100 And v_number < 200) Or (v_number >= 100000 And v_number < 200000) Then
            v_number_name = "száz"
        Else
            v_number_name = "egyszáz"
        End If
    Case 2
        v_number_name = "kétszáz"
    Case 3
        v_number_name = "háromszáz"
    Case 4
        v_number_name = "négyszáz"
    Case 5
        v_number_name = "ötszáz"
    Case 6
        v_number_name = "hatszáz"
    Case 7
        v_number_name = "hétszáz"
    Case 8
        v_number_name = "nyolcszáz"
    Case 9
        v_number_name = "kilencszáz"
    End Select
Return

'------------------------------------------------------------------------------
l_group_10s:
    Select Case v_arr(pos)
    Case 1
        If v_arr(pos - 1) = 0 Then v_number_name = "tíz" Else v_number_name = "tizen"
    Case 2
        If v_arr(pos - 1) = 0 Then v_number_name = "húsz" Else v_number_name = "huszon"
    Case 3
        v_number_name = "harminc"
    Case 4
        v_number_name = "negyven"
    Case 5
        v_number_name = "ötven"
    Case 6
        v_number_name = "hatvan"
    Case 7
        v_number_name = "hetven"
    Case 8
        v_number_name = "nyolcvan"
    Case 9
        v_number_name = "kilencven"
    End Select
Return

'------------------------------------------------------------------------------
l_group_1s:
    Select Case v_arr(pos)
    Case 1
        If pos = 1 Then v_number_name = "egy"
        If pos = 3 Then 'száz vs. ezeregyszáz
            If v_number < 200 Then
                v_number_name = ""
            Else
                v_number_name = "egy"
            End If
        End If
        If pos = 4 Then 'ezer vs. tizenegyezer
            If v_number < 2000 Then
                v_number_name = ""
            Else
                v_number_name = "egy"
            End If
        End If
        If pos = 6 Then 'százezer vs. egymillióegyszázezer
            If v_number < 200000 Then
                v_number_name = ""
            Else
                v_number_name = "egy"
            End If
        End If
        If pos = 7 Then v_number_name = "egy" 'egymillió and tizenegymillió
        If pos = 10 Then v_number_name = "egy" 'egymilliárd
        If pos = 13 Then v_number_name = "egy"
    Case 2
        If pos = 1 Then v_number_name = "kettõ" Else v_number_name = "két"
    Case 3
        v_number_name = "három"
    Case 4
        v_number_name = "négy"
    Case 5
        v_number_name = "öt"
    Case 6
        v_number_name = "hat"
    Case 7
        v_number_name = "hét"
    Case 8
        v_number_name = "nyolc"
    Case 9
        v_number_name = "kilenc"
    End Select
Return

'------------------------------------------------------------------------------
l_group_names:
    Select Case pos
    Case 10
        If Modulo(v_number, 1000000000) > 0 Then
            v_group_name = "milliárd-"
        Else
            v_group_name = "milliárd"
        End If
    Case 7
        If Modulo(v_number, 1000000) > 0 Then
            v_group_name = "millió-"
        Else
            If v_number >= 1000000000 And Modulo(v_number, 1000000) = 0 Then
                v_group_name = ""
            Else
                v_group_name = "millió"
            End If
        End If
    Case 4
        If v_number <= 2000 Then
            v_group_name = "ezer"
        Else
            If Modulo(v_number, 1000000) - Modulo(v_number, 1000) = 0 Then
                v_group_name = ""
            Else
                If Modulo(v_number, 1000) > 0 Then
                    v_group_name = "ezer-"
                Else
                    v_group_name = "ezer"
                End If
            End If
        End If
    End Select
Return

End Function


Function Modulo(a, b)
    'Modulo = a - (b * (a \ b))
    Modulo = a Mod b
End Function

Answer 1

We'll start with variables. Variables not defined -

v_number = cellvalue
v_length = Len(v_number)
v_number_name = ""
v_group_name = ""
v_num2txthu = ""
pos

Variables defined poorly

Dim v_arr(100)

All variables should be defined and given a type. Now for variable names - I have no idea what they are supposed to be. v_arr is an array of the letter "v" (sorry, I'm not being snarky, I promise)? Good variables names are great for readers of code. Let me take a guess at these

Dim v_arr(100) -         Dim numbersForConversion() as Long
v_number = cellvalue -   Dim numberToConvert as Double
v_length = Len(v_number) Dim  numberToConvertLength as string
v_number_name = ""       Dim numberName as string
v_group_name = ""        Dim groupName as string
v_num2txthu = ""         Dim numberAsText as string
pos                      Dim positionInNumber as long

Why is v_arr(100? If you can only work with numbers under 1,000,000,000 - your array only needs to be 10. Better yet Redim numbersForConversion(numberToConvertLength).

Now the code will be a lot easier to read. Well, I can't read Hungarian, so I assume it will be easier to read. If not, pick your name in whatever language you'd like.

Function Modulo(a, b) could be Public Function(byVal numberValue as long, byVal divisor as long)

On the same note Function num2txthu(cellvalue as double) is breaking a lot of conventions. You shouldn't have a digit in a function name - it's just bad practice. There's also no reason, as mentioned above, to name it num2txthu when you could easily name it Public Function ConvertNumberToHungarian(byVal targetNumber as Double.

Code organization - none of the code is indented from the left - you should always start your code indented so labels can be seen easily - which you do have labels. I honestly can't tell where the function ends and the next one begins, are there more than two functions? There's an Exit Function all the way to the left that confuses me.

Comments - I can't really comment because I can't read what they say, but they look to me like they are just saying what's happening instead of why something is happening. Always put comments where you deviate from the obvious way of doing something and it would be difficult to unwrap without spending a lot of time on it.

Speaking of labels, they have pretty poor names too - l_group_100s, l_group_10s:, l_group_1s:, l_group_names:. They could be more descriptive like HundredsPosition, TensPosition or whatever the Hungarian words are for those numbers.

---

I'm going to try to tackle the logic, but it may take a while.

I really like the use of the Fix() function - I've never actually seen it. Same goes for Val() - but I don't see the reason to use it for an actual number:

The Microsoft Excel VAL function accepts a string as input and returns the numbers found in that string.

Oh, you're using Mid to get a string and returning it as a number. That seems like a pretty roundabout way of doing that, but I can't think of a better way.

I'd shift the check for 0 to above the check for integer, because 0 will pass the check for integer.

Does this function account for negative numbers? Because the first check - If v_number > 1000000000 Then would pass for -2,000,000,000 and I think the length might get wonky and might overflow or type mismatch in the array.

Might want to check if ABS(v_number) > 1000000000 and then figure out how to use negative numbers. You'd probably only have to change one case if you converted it to positive and made it negative at the end.

I get that Function Modulo is a refactoring of the code, but all it does is v_number mod #### so why break it out? I mean v_number mod ## is shorter than Modulo(v_number, ##) Maybe someone else can help me understand that.

The use of GoSub ... Return is interesting as well. It might actually make more sense to move the labels to their own functions and just use the Public Function(byval # as long) as string to send the digits to get the text. By the looks of it those labels are already refactored to include several types of cases, so breaking them out would make sense. Plus you wouldn't need to carry pos all the way through and back every time.