473,395 Members | 1,468 Online
Bytes | Software Development & Data Engineering Community
Post Job

Home Posts Topics Members FAQ

Join Bytes to post your question to a community of 473,395 software developers and data experts.

How to Clean Up This Code

124 100+
How can I clean up this code? It works but it resembles a plate of spaghetti. Any advice is welcome. The goal of the function is to parse strOrig into Expected Results. Of course strOrig will vary, but will always be in the same format.


Expand|Select|Wrap|Line Numbers
  1. Function ParseFolders()
  2.  
  3. Dim strOrig As String
  4. Dim strSemi As String
  5. Dim intSemi As Integer, intSemiBegDoc As Integer
  6. Dim strPipe As String
  7. Dim intPipe As Integer
  8. Dim arTag() As String
  9. Dim arField() As String
  10. Dim arSemi() As Integer
  11. Dim arPipe() As Integer
  12. Dim intLoopCount As Integer, intSecondLoopCount As Integer
  13. Dim strBegDoc As String, strFolder As String
  14. Dim i As Integer, j As Integer, intFieldLen As Integer
  15. Dim strFullString() As String, strDelim As String
  16.  
  17. strDelim = "[delim]"
  18.  
  19. strSemi = ";"
  20. strPipe = "|"
  21.  
  22. strOrig = "AB000001;|Folder01|Tag01|Tag02|;|Folder02|Tag01|Tag03|;"
  23.  
  24. ' Expected results: AB000001;Folder01[delim]Tag01;Folder01[delim]Tag02;Folder02[delim]Tag01;Folder02[delim]Tag03
  25.  
  26.  
  27. intSemiBegDoc = InStr(1, strOrig, strSemi)
  28. strBegDoc = Left(strOrig, InStr(intSemiBegDoc, strOrig, strSemi) - 1) & strSemi
  29.  
  30. intSemi = intSemiBegDoc
  31.  
  32. Do While intSemi < Len(strOrig)
  33.     intSemi = InStr(intSemi + 1, strOrig, strSemi)
  34.     intLoopCount = intLoopCount + 1
  35. Loop
  36.  
  37. ReDim arField(intLoopCount - 1)
  38. ReDim arSemi(intLoopCount)
  39. ReDim arField(intLoopCount - 1)
  40.  
  41. intSemi = intSemiBegDoc
  42.  
  43. arSemi(0) = intSemiBegDoc
  44. intSecondLoopCount = 1
  45.  
  46. Do While intSemi < Len(strOrig)
  47.     intSemi = InStr(intSemi + 1, strOrig, strSemi)
  48.     arSemi(intSecondLoopCount) = intSemi
  49.     intSecondLoopCount = intSecondLoopCount + 1
  50. Loop
  51.  
  52. Dim intUbound As Integer
  53. intUbound = UBound(arSemi())
  54.  
  55. For i = 0 To intUbound - 1
  56.     If i < intUbound Then
  57.         arField(i) = Mid(strOrig, arSemi(i) + 1, (arSemi(i + 1) - arSemi(i)))
  58.     Else
  59.         arField(i) = Mid(strOrig, arSemi(i - 1) + 1, arSemi(i))
  60.     End If
  61. Next
  62.  
  63. 'Testing for pipes in fields************
  64.  
  65. Dim strTest As String
  66.  
  67. ReDim strFullString(UBound(arField()))
  68.  
  69.  
  70. For j = 0 To UBound(arField())
  71.  
  72.     strTest = arField(j)
  73.  
  74.     intLoopCount = 0
  75.     intPipe = 1
  76.  
  77.     Do While intPipe < Len(strTest) - 1
  78.         intPipe = InStr(intPipe + 1, strTest, strPipe)
  79.         intLoopCount = intLoopCount + 1
  80.     Loop
  81.  
  82.     ReDim arPipe(intLoopCount - 1)
  83.     intUbound = UBound(arPipe())
  84.  
  85.     intLoopCount = 0
  86.  
  87.     intPipe = 1
  88.     Do While intPipe < Len(strTest) - 1
  89.         intPipe = InStr(intPipe + 1, strTest, strPipe)
  90.         arPipe(intLoopCount) = intPipe
  91.         intLoopCount = intLoopCount + 1
  92.     Loop
  93.  
  94.     ReDim arTag(intLoopCount - 2)
  95.     intUbound = UBound(arTag())
  96.  
  97.     strFolder = Left(strTest, arPipe(0))
  98.  
  99.     For i = 0 To intUbound
  100.         If i < intUbound Then
  101.             arTag(i) = Mid(strTest, arPipe(i), (arPipe(i + 1) - arPipe(i) + 1))
  102.         Else
  103.             arTag(i) = Mid(strTest, arPipe(i), Len(strTest) - arPipe(i))
  104.         End If
  105.     Next
  106.  
  107.     If j = 0 Then
  108.         strFullString(j) = strBegDoc
  109.     End If
  110.  
  111.     For i = 0 To intUbound
  112.         strFullString(j) = strFullString(j) & strFolder & strDelim & arTag(i) & strSemi
  113.     Next i
  114. Next j
  115.  
  116. strTest = ""
  117. For i = 0 To UBound(strFullString())
  118.  
  119.     strTest = strTest & strFullString(i)
  120. Next i
  121.  
  122. Debug.Print strTest
  123.  
  124. End Function
  125.  
  126.  
  127.  
  128.  
Sep 17 '13 #1

✓ answered by zmbd

Just because you've done a lot of work on the other code... and it has been awhile since I've played with arrays:

In a standard module:
Expand|Select|Wrap|Line Numbers
  1. Option Compare Database
  2. Option Explicit
  3.  
  4. Sub splitstring(zstrIn As String)
  5.     Dim zstrArray_pass1() As String
  6.     Dim zstrArray_pass2() As String
  7.     Dim zstrtemp As String
  8.     Dim zstrLeader As String
  9.     Dim zstrParsed As String
  10.     Dim zv As Variant 'this is for the length check
  11.     Dim zi As Integer 'this is for outer loop
  12.     Dim zx As Integer 'this is for inner loop
  13.     Dim zr As Integer 'first pass upper bound
  14.     Dim zc As Integer 'second pass upper bound
  15.     '
  16.     'Parse the initial string based on the semicolon
  17.     zstrArray_pass1 = Split(zstrIn, ";")
  18.     zr = UBound(zstrArray_pass1)
  19.     '
  20.     For zi = 0 To zr
  21.         '
  22.         'Parse each cell in the initial array for the pipe character
  23.         zstrArray_pass2() = Split(zstrArray_pass1(zi), Chr$(124))
  24.         zc = UBound(zstrArray_pass2)
  25.         '
  26.         'Parse the string from the cell by adding it to the string based on length
  27.         For zx = 0 To zc
  28.             If zc > 0 Then
  29.                 'check for the value from the cell; setup for null value by zero length string
  30.                 zstrtemp = zstrArray_pass2(zx) & ""
  31.                 If Len(zstrtemp) Then
  32.                     zv = InStr(1, zstrtemp, "folder")
  33.                     If zv > 0 Then
  34.                         zstrLeader = zstrtemp
  35.                     Else
  36. '>change the deliminator here... I used the percent-sign
  37.                         zstrParsed = zstrParsed & zstrLeader & "%" & zstrtemp & ";"
  38.                     End If
  39.                 End If
  40.             Else
  41.                 'then this should be the the only value in the cell so append it to the string and clear
  42.                 zstrParsed = zstrArray_pass2(0) & ";"
  43.             End If
  44.         Next zx
  45.         '
  46.         'clear the second array for the next cell from the first pass
  47.         ReDim zstrArray_pass2(0)
  48.     Next zi
  49.     Debug.Print String(20, "-")
  50.     Debug.Print zstrParsed
  51.     Debug.Print String(20, "-")
  52. End Sub
In the immediate window (press <ctrl><g>) enter your example:
Expand|Select|Wrap|Line Numbers
  1. splitstring("AB000001;|Folder01|Tag01|Tag02|;|Folder02|Tag01|Tag03|;")
Press return and the result will be:
Expand|Select|Wrap|Line Numbers
  1. --------------------
  2. AB000001;Folder01%Tag01;Folder01%Tag02;Folder02%Tag01;Folder02%Tag03;
  3. --------------------
Change to a function if you desire.

13 1466
zmbd
5,501 Expert Mod 4TB
Can you provide a sample input and output maybe two or three that covers the expected?
Sep 17 '13 #2
BikeToWork
124 100+
The sample input is: "AB000001;|Folder01|Tag01|Tag02|;|Folder02|Tag01|T ag03|;"

The expected results are:
"AB000001;Folder01[delim]Tag01;Folder01[delim]Tag02;Folder02[delim]Tag01;Folder02[delim]Tag03"

Another sample input is:
"AB000001;|Folder01|Tag01|Tag02|;|Folder02|Tag01|T ag03|;|Folder03|Tag04|Tag05|;"

For this sample the output would be:
"AB000001;|Folder01|[delim]|Tag01|;|Folder01|[delim]|Tag02|;|Folder02|[delim]|Tag01|;|Folder02|[delim]|Tag03|;|Folder03|[delim]|Tag04|;|Folder03|[delim]|Tag05|;"


I am going to substitute a real delimiter for [delim], so that is just a placeholder until we decide which delimiter to use. The input data for this function will be a delimited text file so I'll need to design the database to handle multiple rows.

Thanks for your response.
Sep 17 '13 #3
zmbd
5,501 Expert Mod 4TB
I'd use a couple of arrays and the split to handle this...
Sep 17 '13 #4
zmbd
5,501 Expert Mod 4TB
Just because you've done a lot of work on the other code... and it has been awhile since I've played with arrays:

In a standard module:
Expand|Select|Wrap|Line Numbers
  1. Option Compare Database
  2. Option Explicit
  3.  
  4. Sub splitstring(zstrIn As String)
  5.     Dim zstrArray_pass1() As String
  6.     Dim zstrArray_pass2() As String
  7.     Dim zstrtemp As String
  8.     Dim zstrLeader As String
  9.     Dim zstrParsed As String
  10.     Dim zv As Variant 'this is for the length check
  11.     Dim zi As Integer 'this is for outer loop
  12.     Dim zx As Integer 'this is for inner loop
  13.     Dim zr As Integer 'first pass upper bound
  14.     Dim zc As Integer 'second pass upper bound
  15.     '
  16.     'Parse the initial string based on the semicolon
  17.     zstrArray_pass1 = Split(zstrIn, ";")
  18.     zr = UBound(zstrArray_pass1)
  19.     '
  20.     For zi = 0 To zr
  21.         '
  22.         'Parse each cell in the initial array for the pipe character
  23.         zstrArray_pass2() = Split(zstrArray_pass1(zi), Chr$(124))
  24.         zc = UBound(zstrArray_pass2)
  25.         '
  26.         'Parse the string from the cell by adding it to the string based on length
  27.         For zx = 0 To zc
  28.             If zc > 0 Then
  29.                 'check for the value from the cell; setup for null value by zero length string
  30.                 zstrtemp = zstrArray_pass2(zx) & ""
  31.                 If Len(zstrtemp) Then
  32.                     zv = InStr(1, zstrtemp, "folder")
  33.                     If zv > 0 Then
  34.                         zstrLeader = zstrtemp
  35.                     Else
  36. '>change the deliminator here... I used the percent-sign
  37.                         zstrParsed = zstrParsed & zstrLeader & "%" & zstrtemp & ";"
  38.                     End If
  39.                 End If
  40.             Else
  41.                 'then this should be the the only value in the cell so append it to the string and clear
  42.                 zstrParsed = zstrArray_pass2(0) & ";"
  43.             End If
  44.         Next zx
  45.         '
  46.         'clear the second array for the next cell from the first pass
  47.         ReDim zstrArray_pass2(0)
  48.     Next zi
  49.     Debug.Print String(20, "-")
  50.     Debug.Print zstrParsed
  51.     Debug.Print String(20, "-")
  52. End Sub
In the immediate window (press <ctrl><g>) enter your example:
Expand|Select|Wrap|Line Numbers
  1. splitstring("AB000001;|Folder01|Tag01|Tag02|;|Folder02|Tag01|Tag03|;")
Press return and the result will be:
Expand|Select|Wrap|Line Numbers
  1. --------------------
  2. AB000001;Folder01%Tag01;Folder01%Tag02;Folder02%Tag01;Folder02%Tag03;
  3. --------------------
Change to a function if you desire.
Sep 17 '13 #5
BikeToWork
124 100+
Thanks, ZMBD. Your code is much less convoluted and more compact than mine and it works.
Sep 18 '13 #6
zmbd
5,501 Expert Mod 4TB
It's that Split() function that does the magic.
I used to have a zfncsplitstring() that I coded from back in the old days (prior to ACC2003) that would have added a few more lines! (O_O)
I may still have that around somewhere...
It still used an array, a couple of loops, and the Instr() and Mid() to parse the input string. If I remember correctly, maybe another 10 or 20 lines of code.
Sep 18 '13 #7
Rabbit
12,516 Expert Mod 8TB
You can also try regular expressions.
Expand|Select|Wrap|Line Numbers
  1. Function test(inputString As String, newDelim As String) As String
  2.     Dim myRegExp, Matches1, Matches2, myMatch2
  3.     Set myRegExp = CreateObject("vbscript.regexp")
  4.     myRegExp.IgnoreCase = True
  5.     myRegExp.Global = True
  6.  
  7.     myRegExp.Pattern = "^([a-z0-9]*);"
  8.     Set Matches1 = myRegExp.Execute(inputString)
  9.     test = Matches1(0)
  10.  
  11.     myRegExp.Pattern = ";\|(([a-z0-9]*)\|)((([a-z0-9]*)\|)*)"
  12.     Set Matches2 = myRegExp.Execute(inputString)
  13.     For Each myMatch2 In Matches2
  14.         myRegExp.Pattern = "([a-z0-9]*)\|"
  15.         test = test & myRegExp.Replace(myMatch2.subMatches(2), myMatch2.subMatches(1) & newDelim & "$1;")
  16.     Next
  17. End Function
Unfortunately the VB Script implementation of regex isn't the most robust so it doubles the code length. Had the implementation been better, it would only be ~8 lines of code or so.
Sep 18 '13 #8
zmbd
5,501 Expert Mod 4TB
If you use RegEx in VBA you may need to set the reference to the library.

(opps looks, like Rabbit has that taken careof in line3 with the set>createobject... that'll teach me to read code better)

You will also need to tweek this a tad if you want to remove all of the pipe charaters:
Expand|Select|Wrap|Line Numbers
  1. ?test("AB000001;|Folder01|Tag01|Tag02|;|Folder02|Tag01|Tag03|;")
  2. 'gives the following:
  3. AB000001;Folder01|Tag01;Folder01|Tag02;Folder02|Tag01;Folder02|Tag03;
  4. based in Rabbit's code
Sep 18 '13 #9
Rabbit
12,516 Expert Mod 8TB
I've modified my original code to allow for user input of a delimiter.
Sep 18 '13 #10
BikeToWork
124 100+
Rabbit, I really like your regular expression code but cannot get it to work on my computer. I changed the name of the function but not the code inside it.

Expand|Select|Wrap|Line Numbers
  1. Function Text1Parse(inputString As String, newDelim As String) As String
  2.     Dim myRegExp, Matches1, Matches2, myMatch2
  3.     Set myRegExp = CreateObject("vbscript.regexp")
  4.     myRegExp.IgnoreCase = True
  5.     myRegExp.Global = True
  6.  
  7.     myRegExp.Pattern = "^([a-z0-9]*);"
  8.     Set Matches1 = myRegExp.Execute(inputString)
  9.     TextParse = Matches1(0)
  10.  
  11.     myRegExp.Pattern = ";\|(([a-z0-9]*)\|)((([a-z0-9]*)\|)*)"
  12.     Set Matches2 = myRegExp.Execute(inputString)
  13.     For Each myMatch2 In Matches2
  14.         myRegExp.Pattern = "([a-z0-9]*)\|"
  15.         TextParse = TextParse & myRegExp.Replace(myMatch2.subMatches(2), myMatch2.subMatches(1) & newDelim & "$1;")
  16.     Next
  17. End Function
  18.  
It errors out on the line "TextParse = Matches1(0)" The runtime error number is 5 and error description is "Invalid Procedure or Argument." Any help with this is much appreciated.
Sep 20 '13 #11
BikeToWork
124 100+
Rabbit, sorry I forgot to include how I am calling the function from the debug window.

Expand|Select|Wrap|Line Numbers
  1. TextParse "|Folder01|Tag01|Tag02|;|Folder02|Tag01|Tag03|;|Folder03|Tag04|Tag05|;", "%"
Sep 20 '13 #12
Rabbit
12,516 Expert Mod 8TB
Isn't your input wrong? Shouldn't it be:
Expand|Select|Wrap|Line Numbers
  1. TextParse "AB000001;|Folder01|Tag01|Tag02|;|Folder02|Tag01|Tag03|;|Folder03|Tag04|Tag05|;", "%"
Also, your function name on line 1 Text1Parse doesn't match your call TextParse.
Sep 20 '13 #13
BikeToWork
124 100+
Sorry for the typos. The function is working properly now. Thanks again for your assistance and thanks to ZMBD.
Sep 20 '13 #14

Sign in to post your reply or Sign up for a free account.

Similar topics

5
by: Nige | last post by:
I've got a form which has the following code: <form action="/cgi-bin/FormMail.pl" method="POST" language="JavaScript" onsubmit="return FrontPage_Form1_Validator(this)" name="FrontPage_Form1"> ...
1
by: Surya | last post by:
I'm using Microsoft.Project.OLEDB.9.0 provider with a datareader to read data from a .mpp file (Microsoft Project plan) from ASP.NET The problem is All the STRING values, the datareader returns, do...
1
by: lily82 | last post by:
can sm 1 help me transform this code to C# code?? tq so much :wink: Goto : <% Dim counter Dim page Dim pages counter= 10 pages = 20 page = 1
5
by: Gianni Mariani | last post by:
The 3 compilers I tried all did different things. gcc 3.4.0 ICE's (Bug 15480) MS C++ 7.1 has meaningless diags Comeau accepts the code Obviously this is not somthing I'll be using until we...
12
by: Steven T. Hatton | last post by:
This is something I've been looking at because it is central to a currently broken part of the KDevelop new application wizard. I'm not complaining about it being broken, It's a CVS images. ...
2
by: Nobody | last post by:
Let me start off with a brief overview... This part is not really important, just saying what its for... I had been working on a Windows GUI library (DLL) when suddenly my boss told he wanted...
2
by: A. Saksena | last post by:
Hi all, Why this code does't work!! ======================== #include <iostream> template<typename T> class a { public: typedef T var; };
4
by: andersboth | last post by:
I want to run some code when my Windows Form Application is being shut down. I want to run this code when the Application is being shut down in the following situation: 1. The App is being...
12
by: Martin Ho | last post by:
This is the code I have so far, to generate 50.000 random numbers. This code will generate the same sequence every time I run it. But the problem being is that it's slow. It take on my p4 1.6gh...
4
by: RON | last post by:
Can anyone tell me how I would fix this code to do the following? I want the buttons to be 0 through 9 not 1 through 10 like they are now. == Inherits System.Windows.Forms.Form Public...
0
by: Charles Arthur | last post by:
How do i turn on java script on a villaon, callus and itel keypad mobile phone
0
by: ryjfgjl | last post by:
If we have dozens or hundreds of excel to import into the database, if we use the excel import function provided by database editors such as navicat, it will be extremely tedious and time-consuming...
0
BarryA
by: BarryA | last post by:
What are the essential steps and strategies outlined in the Data Structures and Algorithms (DSA) roadmap for aspiring data scientists? How can individuals effectively utilize this roadmap to progress...
1
by: nemocccc | last post by:
hello, everyone, I want to develop a software for my android phone for daily needs, any suggestions?
1
by: Sonnysonu | last post by:
This is the data of csv file 1 2 3 1 2 3 1 2 3 1 2 3 2 3 2 3 3 the lengths should be different i have to store the data by column-wise with in the specific length. suppose the i have to...
0
marktang
by: marktang | last post by:
ONU (Optical Network Unit) is one of the key components for providing high-speed Internet services. Its primary function is to act as an endpoint device located at the user's premises. However,...
0
by: Hystou | last post by:
Most computers default to English, but sometimes we require a different language, especially when relocating. Forgot to request a specific language before your computer shipped? No problem! You can...
0
Oralloy
by: Oralloy | last post by:
Hello folks, I am unable to find appropriate documentation on the type promotion of bit-fields when using the generalised comparison operator "<=>". The problem is that using the GNU compilers,...
0
tracyyun
by: tracyyun | last post by:
Dear forum friends, With the development of smart home technology, a variety of wireless communication protocols have appeared on the market, such as Zigbee, Z-Wave, Wi-Fi, Bluetooth, etc. Each...

By using Bytes.com and it's services, you agree to our Privacy Policy and Terms of Use.

To disable or enable advertisements and analytics tracking please visit the manage ads & tracking page.