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
function output in Excel
1 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.