Macros becoming too big and unstable?

English
,
#1

I’ve been using a macro for the past year and it’s served me very well but I wanted to modify it and add more to it as I got more ideas.
I only added more functions that were not dependent on others, the original macro was around 4700 lines, a lot of that code was caught by a recorder and worked well, I added another 400 lines which was hand written and now I get runtime errors in all sorts of the code where it didn’t in the original. I’m also dealing with arrays that on an IDE would have lengths of 39,000.

The types of errors I have gotten have ranged from InStr() functions not being properly set despite that code being untouched, to the calling of the GlobalScope.BasicLibraries.loadLibrary failing.

Has holding too much in a module caused this to be unstable or have I inadvertently coded in conflicts? Should I split them up into different modules.

Thanks.

#2

Can you upload the macro code here? (At least the added lines.)

#3 
Sub ParTimeFill()

'Declare variables
	Dim oDoc As Object
    Dim oSheet As Object
    Dim pRange As Object
    Dim sInput As String
    Dim sInput2 As String
    Dim oRange As Object
    Dim oCell As Object
    Dim iRow As Integer
    Dim iCol As Integer
    Dim foundCells() As String
    Dim foundCount As Integer
    Dim arrData As Variant
    Dim i as Long
    
    oDoc = ThisComponent
    oSheet = oDoc.Sheets(0)
    
    'start find and replace
    
    'start 
    oRange = oSheet.getCellRangeByName("O1:O300")
    
    ReDim inputArray(2, 0, 0)
    
    For iRow = 0 To oRange.Rows.Count - 1
        For iCol = 0 To oRange.Columns.Count - 1
            oCell = oRange.getCellByPosition(iCol, iRow)
            If oCell.String = "PAR TIME HERE" Then
                ReDim Preserve foundCells(foundCount)
                foundCells(foundCount) = oCell.AbsoluteName
                
                Dim targetCellR As Object
                Dim targetCellU As Object
                targetCellR = oSheet.getCellByPosition(oCell.CellAddress.Column + 5, oCell.CellAddress.Row)
                targetCellU = oSheet.getCellByPosition(oCell.CellAddress.Column + 8, oCell.CellAddress.Row)
                
                DIm strR As String
                Dim strU As String
                Dim strCombined As String
                strR = targetCellR.String
                strU = targetCellU.String
                strCombined = strR & strU

			'Declare an array to store the data
			arrData = Array("flem2 OPEN LR,26.47436", "flem4U OPEN LR,49.33333", "flem3F OPEN G2,37.97581", "flem4UM OPEN G2,48.24603", "flem3 OPEN G2,41.4", "flemOPEN G3,50.875", "flem4U OPEN G1,54.20968", "flem3U OPEN G2,50.46154", "flem3 OPEN LR,37.93023", "flem3 OPEN G3,39.11905", "flemQlty G3,51.90789", "flemOPEN G2,52.81481", "flem3 OPEN G1,43.91875", "flem3UFM OPEN G1,52.03125", "flem4UM OPEN G3,47.86154", "flemOPEN LR,49.28017", "flem2 OPEN G3,25.82353", "flemBM96,46.58333", "flemBM90,42.71333", "flem3 OPEN,40.40076", "flem3U OPEN G1,55.43233", "flem34FM BM70,35.91071", "flem2F OPEN G3,27.15152", "flem3UFM OPEN,43.275", "flem3F OPEN LR,36.83333", "flem4U BM90,45.72222", "flem3U OPEN LR,48.22535", "flemBM80,41.93617", "flem3F OPEN G1,39.30435", "flem3F OPEN G3,39.84524", "flem3 Qlty LR,39.92", "flem3U BM80,42", "flem3 BM70,35.00758", "flem3UFM BM70,36.09524", "flem3U BM84,42.46622", "flem3U BM70,36.42333", "flem3U OPEN,47.18333", "flem2F OPEN,25.33333", "flem3U BM78,41.35", "flemBM70,37.35065", "flem4U BM70,37.79787", "flem2 OPEN,29.00658", "flem3U BM100,47.41667", "flem3UFM BM78,41.33333", "flem3U MDN,24.80952", "flem3U CL1,29.72917", "flem3U NMW,35.29762", "flem3UFM BM64,30.9", "flem3 BM64,31.86842", "flem4U BM64,30.60714", "flemBM84,43.52632", "flem3UFM OPEN G3,46.3", "flem3CG OPEN G3,43.28571", "flemOPEN G1,54.51923", "flemQlty,47.91837", "flemOPEN,42.8913", "flem3F OPEN,34.5", "flem3 Qlty,41.75943", "flem2 OPEN G2,32.80769", "flem4UM OPEN,46.33333", "flemFM BM78,38.82558", "flemBM78,39.95455", "flemBM100,45.4375", "flem3F BM78,38.95", "flem2U BM90,38.6875", "flem2UFM BM78,40.83333", "flem3U Qlty,46.66", "flemFM BM90,45.05", "flem2 Qlty,27.25", "flem3UFM Qlty,44.8", "flem2 Qlty LR,34", "flemQlty LR,50.14815", "flem4UM BM84,43.90909", "flem3CG OPEN LR,39.22727", "flem4U Qlty G2,52.65", "flem4UM BM70,30.77273", "flem3CG OPEN,34", "flem3U BM64,32", "flemNMW,38.1", "flem4UM BM78,41.8")
			'Check if the combined string exists in the array
			For i = LBound(arrData) To UBound(arrData)
					If InStr(arrData(i), strCombined) > 0 Then
					
			'If it does exist, post the value paired with the string in the array
					oCell.String = Replace(Mid(arrData(i), InStr(arrData(i), strCombined) + Len(strCombined)), ",", "")

		      		 End If
		      		 Next i
		      		 End If
        Next iCol
    Next iRow
End Sub

After experimenting with this I think my arrays are far too big and have to split them up, the code works now with smaller contents in the array. I think it must have something to do with memory limitations because an error message I received was it couldn’t find “alScope”. “alScope” is nothing but it is a part of the word “GlobalScope” which I use to call the library at the beginning of the main function. One array I estimate has around 7300 elements.

I have a laptop with 8GB of RAM which would be enough because it never maxxed out so I think I just pushed the software beyond its limits maybe?

EDIT: This code has the shortened array in it.

#4

It might. The compiled image used to be very limited; but in the recent versions, we have improved it. E.g., tdf#92620 (despite talking about password-protected macros) is related. That bug was fixed in v.24.2. It may (or may not) fix your issue (it’s impossible to be sure without seeing the file with the module, and exact steps to see the problem).

#5

Thanks for that reference. Should have mentioned I use Windows 11 and in fact I did upload to the latest version thinking it would fix it but it didn’t. notepad++ measures my module at 323,000 characters which doesn’t seem like a lot in 2024, perhaps I am hardware limited in some way I can’t tell.
Splitting up the modules seems like the best solution for now, a bit annoying but I only have to do it once as far as I can see.

#6

I haven’t looked at your code very carefully yet. Perhaps over time I will be able to read this more carefully, understand what exactly the procedure does, and suggest a different approach to solving the problem. Now I want to ask a question about Dim foundCount As Integer. I see that you are overriding the size of the foundCells() array through this variable. But I don’t see where in the code this variable itself is changed.

#7

It’s been some time since I wrote it, it really was just pieced together from things I could find online. From memory I had a lot of trouble passing a cell address as the “found” cell which would then be overwritten by something in the array. I tried to pass it as a string first and it didn’t but then somehow after trial and error passing a string with an integer parameter worked and it worked every time when I tested it so I just left it.
It’s absolutely not the greatest code, I’m certain it can be done more efficiently.

#8

Most likely, the second one. Do not rush to divide the code into several modules - perhaps the error is not in the size…
Let’s try to fix this particular procedure ParTimeFill()
First, let’s tell you in simple human words what the procedure should do.
In a given range (currently this is only 300 cells in the O column, “O1:O300”, but in real code this could be the entire O column, or some other column), you need to find cells containing the special phrase “PAR TIME HERE” . Oh no! Not “containing”, but exactly matching. In other words, the "PAR TIME HERE " cell will not be processed.
Now in the line with the found cell you need to take the texts from columns T and W and concatenate them into one string.
As far as I understand the idea, you should get strings like ""flem3F OPEN " or "“flemFM”, which should lead to a value from the table like “BM78,38.82558” or “BM70,30.77273” and this string should replace the value of the found cell, right?
Please tell me, but this whole table that you describe in arrData - is it somewhere in a spreadsheet just like a table? Perhaps taking the value from there would be easier than searching for the desired string in an array of strings without any hope of finding it there?

1 Like
#9

Yes, that’s what it does.

This particular arrData array is just placeholder data but they do write in decimal values as the new value.

The desired arrData can be put into a spreadsheet and I do have it in a csv file and that was my first intention.

Before I do all this the PAR TIME HERE is actually taken from an xlsx template which I apply to the sheet I’m working on, the sheet I’m working on is a generated sheet I get from a website.

For iRow = 0 To oRange.Rows.Count - 1
        For iCol = 0 To oRange.Columns.Count - 1
            oCell = oRange.getCellByPosition(iCol, iRow)
            If oCell.String = "PR" Then
                ReDim Preserve foundCells(foundCount)
                foundCells(foundCount) = oCell.AbsoluteName
                
                Dim targetCell As Object
                targetCell = oSheet.getCellByPosition(oCell.CellAddress.Column + 1, oCell.CellAddress.Row + 1)
                
                Dim messageTarget As String
                messageTarget = "F" & targetCell.CellAddress.Row
                
                sDocName = "third_template_aus.xlsx"
                'sPath = ThisComponent.getURL()
                sPath = "C:\Users\xxx\Documents\xxxx\"
                sPath = sPath & sDocName
                
                Set oSourceDoc = ui.OpenDocument(sPath, Hidden:=True, ReadOnly:=True)
                Set oDestDoc = CreateScriptService("Calc")
                oDestDoc.CopyToCell(oSourceDoc.Range("Sheet1.F5:AS6"), "Races."&messageTarget)

                Dim oSheet2 As Object
   				Dim oRange2 As Object
    
    			oSheet2 = ThisComponent.Sheets(0)
    			oRange2 = oSheet2.getCellRangeByPosition(5 , targetCell.CellAddress.Row, 44 , targetCell.CellAddress.Row)
    			ThisComponent.CurrentController.Select(oRange2)
    		
    			jCol = iCol
    			jRow = iRow+1
    			
				Do
    				pCell = oSheet.getCellByPosition(jCol, jRow)
    
   					 ' Check if cell has a border
  					  If pCell.TopBorder2.OuterLineWidth <> 0 Then
     					   borderCount = borderCount + 1
  							  Else
      						  Exit Do ' Stop loop when encountering a cell without a border
 					   End If
  					  jRow = jRow + 1
						Loop While True
    			
    			subNumber = borderCount

That code finds all the tables in the generated sheet and then I have another set of functions that drag down the template to the last row of that table because they aren’t all the same size, then it finds the next, this repeats until the 300th row is checked, I’ve never had a sheet that goes past it in two years and I think when I tried to do “O:O” it returned an error so I just set a high number.

So I tried to basically have a key pairing like in the array but instead in an external spreadsheet and if strCombined was found in /xxx/dictionary.Sheet1.A2 it would write the value from /xxx/dictionary.Sheet1.B2 in the generated sheet but I just couldn’t get it to work for whatever reason.

The array was an alternative solution that when I started to code it just worked easier for me, I got it done in a fraction of the time I spent trying for the external dictionary file and it served me very well for months until I want to add columns for more equations and then the errors in the OP started.

#10

Okay, let’s move step by step. It is not yet known where we will end up, but at least we will stop marking time in a place full of mistakes.

Let’s find all cells with a given text in a specified location.
Yes, this can be done by iterating through all the cells using .getCellByPosition() in a loop. But there are more effective ways. For example like this:

Function FindAllCells(oSearchRange As Variant, sSearchString As String) As Variant
Dim oSearchDescriptor As Variant
	oSearchDescriptor = oSearchRange.createSearchDescriptor()
	oSearchDescriptor.setSearchString(sSearchString)
	On Error Resume Next 
	FindAllCells = oSearchRange.findAll(oSearchDescriptor).getCells()
	On Error GoTo 0
End Function

Now, to find each cell on the first sheet of the current spreadsheet with the text “PAR TIME HERE” you can call this function and use the result that you got:

Sub testFindAllCells
Dim oSheets As Variant, oSheet As Variant
Dim allCells As Variant, oCell As Variant
	oSheets = ThisComponent.getSheets()
	oSheet = oSheets.getByIndex(0)

	allCells = FindAllCells(oSheet, "PAR TIME HERE")
	If isEmpty(allCells) Then 
		Print "Nothing found"
	Else 
		For Each oCell In allCells
			Print oCell.AbsoluteName, oCell.getString()
		Next oCell
	EndIf 
End Sub

Or if you want to limit the search to only one column O (the number of this column, it seems, is 14?), then like this:

	allCells = FindAllCells(oSheet.getColumns().getByIndex(14), "PAR TIME HERE")

You will make the further conversation much easier if you attach samples of the files you are working with to your next comment

#11

That’s great!

Thanks for doing this, you’re not obliged to optimise my code so thank you for your time.

That code worked well, it iterated through the column and MsxBox’d the cell address of each cell value matched.

test_template.xls (29.5 KB)

That’s the template I apply, everything from Column F and Column AS is applied and then the F column and then R though to AR columns dragged down to the end of each table and then copied onto the top of the next table and repeated until there’s no more.

Also I should say I’ve fixed my problem totally, everything is working fine. I split up the arrays into different modules and then I wrote this sub which is called in main sub

Sub CheckFirstLetter()

Dim oActiveSheet As Object
Dim oSelectedCell As Object
Dim CellValue As String

oActiveSheet = ThisComponent.getCurrentController().getActiveSheet()
oSelectedCell = oActiveSheet.getCellRangeByName("T1")
CellValue = oSelectedCell.String

Select Case Left(CellValue,1)

Case "A","B","C"
		Call AUS1AC.assignAll
Case "D","E","F","G"
		Call AUS1DG.assignAll
Case "H","I","J","K","L"
		Call AUS1HL.assignAll
Case "M","N","O","P"
		Call AUS1MP.assignAll
Case "Q","R","S","T"
		Call AUS1QT.assignAll
Case "W","X","Y","Z"
		Call AUS1WZ.assignAll

End Select


End Sub

The arrays have been sorted alphabetically, so it just needs to read the first letter of the target cell and then matches up whatever is has to match up.

#12

Don’t mention it, it wasn’t difficult for me at all. And even nice (I feel awkward when I see code that makes the computer do excessive work)

Are you talking about arrData = Array("")? Well, congratulations on this success. It’s a pity that I didn’t have time to propose an alternative solution - to return to the idea of ​​obtaining data for this array from a CSV file. This would really reduce the amount of code - the array description would turn into two not very long lines:

	GlobalScope.BasicLibraries.LoadLibrary("Tools")
	If not LoadDataFromFile(CSV_FILE_PATH_AND_NAME, arrData) Then Print "CSV-file Not found"

I don’t know how long your dictionary is and whether the file is sorted in ascending order - it won’t be too difficult to sort the array after reading from the file. And searching in an already sorted array can be accelerated many times using the binary search algorithm. Well, if the solution already exists, then forget about it.
I wish you success!

1 Like

Impressum (Legal Info) | Datenschutzerklärung (Privacy Policy)   Content on this site is licensed under a Creative Commons Attribution Share Alike 3.0 license.