Tedious definitions

[RET79]

Hi. I have a code which works great, but, I would like to make it more readable. It starts off by defining many constants and dynamic ranges. However, I was wondering if someone could help me condense these sort of declarations at the beginning of the code..... take a look !

Dim arrmonth1, arrmonth2, arrmonth3, arrmonth4, _
arrmonth5, arrmonth6, arrmonth7, arrmonth8, _
arrmonth9, arrmonth10, arrmonth11, arrmonth12 As Variant

Dim rngmonth1, rngmonth2, rngmonth3, rngmonth4, _
rngmonth5, rngmonth6, rngmonth7, rngmonth8, _
rngmonth9, rngmonth10, rngmonth11, rngmonth12 As Range

Dim zm1, zm2, zm3, zm4, zm5, zm6, zm7, zm8, zm9, zm10, zm11, zm12 As Long

Set rngmonth1 = Range([D8], [D8].End(xlDown))
Set rngmonth2 = Range([I8], [I8].End(xlDown))
Set rngmonth3 = Range([N8], [N8].End(xlDown))
Set rngmonth4 = Range([S8], [S8].End(xlDown))
Set rngmonth5 = Range([X8], [X8].End(xlDown))
Set rngmonth6 = Range([AC8], [AC8].End(xlDown))
Set rngmonth7 = Range([AH8], [AH8].End(xlDown))
Set rngmonth8 = Range([AM8], [AM8].End(xlDown))
Set rngmonth9 = Range([AR8], [AR8].End(xlDown))
Set rngmonth10 = Range([AW8], [AW8].End(xlDown))
Set rngmonth11 = Range([BB8], [BB8].End(xlDown))
Set rngmonth12 = Range([BG8], [BG8].End(xlDown))

zm1 = rngmonth1.Rows.Count
zm2 = rngmonth2.Rows.Count
zm3 = rngmonth3.Rows.Count
zm4 = rngmonth4.Rows.Count
zm5 = rngmonth5.Rows.Count
zm6 = rngmonth6.Rows.Count
zm7 = rngmonth7.Rows.Count
zm8 = rngmonth8.Rows.Count
zm9 = rngmonth9.Rows.Count
zm10 = rngmonth10.Rows.Count
zm11 = rngmonth11.Rows.Count
zm12 = rngmonth12.Rows.Count

arrmonth1 = rngmonth1
arrmonth2 = rngmonth2
arrmonth3 = rngmonth3
arrmonth4 = rngmonth4
arrmonth5 = rngmonth5
arrmonth6 = rngmonth6
arrmonth7 = rngmonth7
arrmonth8 = rngmonth8
arrmonth9 = rngmonth9
arrmonth10 = rngmonth10
arrmonth11 = rngmonth11
arrmonth12 = rngmonth12


Thanks,
RET79

[ This Message was edited by: RET79 on 2002-04-01 06:52 ]

[Mark O'Brien]

arrmonth1 = rngmonth1
arrmonth2 = rngmonth2
arrmonth3 = rngmonth3
arrmonth4 = rngmonth4
arrmonth5 = rngmonth5
arrmonth6 = rngmonth6
arrmonth7 = rngmonth7
arrmonth8 = rngmonth8
arrmonth9 = rngmonth9
arrmonth10 = rngmonth10
arrmonth11 = rngmonth11
arrmonth12 = rngmonth12

I don't understand why you set arrmonth at when you could just use rngmonth or rngmonth.value, if that's what you're after.

There's not much you can do to tidy up the code except use an array, something like:

Dim rngMonth(1 To 12) As Range

Dim zm(1 To 12) As Long



For i = 1 To 12

  Set rngMonth(i) = Range([D8].Offset(, (5 * (i - 1))), [D8].Offset(, (5 * (i - 1))).End(xlDown))

  zm(i) = rngMonth(i).Rows.Count

Next



HTH


		

[RET79]

Thanks Mark, that's exactly what I was after.
Now, regarding the arrays, would it be ok if I emailed you the rest of the code? It may look long but that's because it's very repetitive but it runs quick - bad to read that's all. If you could see a way I could use the range there and avoid using the array then I would be very grateful.

Thanks,

RET79

[Mark O'Brien]

I'm not too keen on getting stuff emailed to me.

How about you post a bit of your code here?

I think I get the gist of what your asking for. Basically everything that your code is going to do, it's going to do it 12 times. There should be a way to loop through the array and perform those operations.

I would suggest that you post one line from each operation or as many as you feel are necessary to get a representation of what you're trying to accomplish.

For example, your first post would have looked something like this:

First operation:
Set rngmonth1 = Range([D8], [D8].End(xlDown))
...
Set rngmonth12 = Range([BG8], [BG8].End(xlDown))

Second operation:
zm1 = rngmonth1.Rows.Count
...
zm12 = rngmonth12.Rows.Count

Third operation:

arrmonth1 = rngmonth1
...
arrmonth12 = rngmonth12


The reason it's easier to post here is that I may not have time to look at your workbook today. (or this week since I'll be out of the office in a couple of hours) You'll have a better chance of a quick answer on this board.

[RET79]

Mark, don't worry, I understand about the email thing. Once reason I suggested the email thing was that maybe it would not make too much sense without the accompanying sheet of which the macro extracts from. But you can probably guess what goes on from the code itself I guess, here goes:


Sub ExtractDataGeneralMacro()


With Application
.Calculation = xlCalculationManual
.ScreenUpdating = False


Dim dest As Object
Dim inputsheet As Object
Set inputsheet = Application.ThisWorkbook.Worksheets("InputSheet")
Set dest = Application.ThisWorkbook.Worksheets.Add

inputsheet.Activate
Set rngcolhead = Range([a10], [a10].End(xlDown))
z = rngcolhead.Rows.Count

dest.Activate
Range([A1], [A1].Offset(0, z - 1)) = WorksheetFunction.Transpose(rngcolhead)

'Correct up to here

Dim arrmonth1, arrmonth2, arrmonth3, arrmonth4, _
arrmonth5, arrmonth6, arrmonth7, arrmonth8, _
arrmonth9, arrmonth10, arrmonth11, arrmonth12 As Variant
Dim rngmonth1, rngmonth2, rngmonth3, rngmonth4, _
rngmonth5, rngmonth6, rngmonth7, rngmonth8, _
rngmonth9, rngmonth10, rngmonth11, rngmonth12 As Range
Dim zm1, zm2, zm3, zm4, zm5, zm6, zm7, zm8, zm9, zm10, zm11, zm12 As Long
inputsheet.Activate
i = 1


Set rngmonth1 = Range([D8], [D8].End(xlDown))
Set rngmonth2 = Range([I8], [I8].End(xlDown))
Set rngmonth3 = Range([N8], [N8].End(xlDown))
Set rngmonth4 = Range([S8], [S8].End(xlDown))
Set rngmonth5 = Range([X8], [X8].End(xlDown))
Set rngmonth6 = Range([AC8], [AC8].End(xlDown))
Set rngmonth7 = Range([AH8], [AH8].End(xlDown))
Set rngmonth8 = Range([AM8], [AM8].End(xlDown))
Set rngmonth9 = Range([AR8], [AR8].End(xlDown))
Set rngmonth10 = Range([AW8], [AW8].End(xlDown))
Set rngmonth11 = Range([BB8], [BB8].End(xlDown))
Set rngmonth12 = Range([BG8], [BG8].End(xlDown))

zm1 = rngmonth1.Rows.Count
zm2 = rngmonth2.Rows.Count
zm3 = rngmonth3.Rows.Count
zm4 = rngmonth4.Rows.Count
zm5 = rngmonth5.Rows.Count
zm6 = rngmonth6.Rows.Count
zm7 = rngmonth7.Rows.Count
zm8 = rngmonth8.Rows.Count
zm9 = rngmonth9.Rows.Count
zm10 = rngmonth10.Rows.Count
zm11 = rngmonth11.Rows.Count
zm12 = rngmonth12.Rows.Count

arrmonth1 = rngmonth1
arrmonth2 = rngmonth2
arrmonth3 = rngmonth3
arrmonth4 = rngmonth4
arrmonth5 = rngmonth5
arrmonth6 = rngmonth6
arrmonth7 = rngmonth7
arrmonth8 = rngmonth8
arrmonth9 = rngmonth9
arrmonth10 = rngmonth10
arrmonth11 = rngmonth11
arrmonth12 = rngmonth12

For k = 3 To zm1 Step 1
Debug.Print arrmonth1(k, 1)
Next k

For k = 3 To zm2 Step 1
Debug.Print arrmonth2(k, 1)
Next k

For k = 3 To zm3 Step 1
Debug.Print arrmonth3(k, 1)
Next k

For k = 3 To zm4 Step 1
Debug.Print arrmonth4(k, 1)
Next k

For k = 3 To zm5 Step 1
Debug.Print arrmonth5(k, 1)
Next k

For k = 3 To zm6 Step 1
Debug.Print arrmonth6(k, 1)
Next k

For k = 3 To zm7 Step 1
Debug.Print arrmonth7(k, 1)
Next k

For k = 3 To zm8 Step 1
Debug.Print arrmonth8(k, 1)
Next k

For k = 3 To zm9 Step 1
Debug.Print arrmonth9(k, 1)
Next k

For k = 3 To zm10 Step 1
Debug.Print arrmonth10(k, 1)
Next k

For k = 3 To zm11 Step 1
Debug.Print arrmonth11(k, 1)
Next k

For k = 3 To zm12 Step 1
Debug.Print arrmonth12(k, 1)
Next k


Dim arrcells1, arrcells2, arrcells3, arrcells4, _
arrcells5, arrcells6, arrcells7, arrcells8, _
arrcells9, arrcells10, arrcells11, arrcells12 As Variant
Dim rngcells1, rngcells2, rngcells3, rngcells4, _
rngcells5, rngcells6, rngcells7, rngcells8, _
rngcells9, rngcells10, rngcells11, rngcells12 As Range
Dim zc1, zc2, zc3, zc4, zc5, zc6, zc7, zc8, zc9, zc10, zc11, zc12 As Long



Range([E9], [E9].End(xlToRight)).Select
Range(Selection, Selection.End(xlDown)).Select
Set rngcells1 = Selection

Range([J9], [J9].End(xlToRight)).Select
Range(Selection, Selection.End(xlDown)).Select
Set rngcells2 = Selection

Range([O9], [O9].End(xlToRight)).Select
Range(Selection, Selection.End(xlDown)).Select
Set rngcells3 = Selection

Range([T9], [T9].End(xlToRight)).Select
Range(Selection, Selection.End(xlDown)).Select
Set rngcells4 = Selection

Range([Y9], [Y9].End(xlToRight)).Select
Range(Selection, Selection.End(xlDown)).Select
Set rngcells5 = Selection

Range([AD9], [AD9].End(xlToRight)).Select
Range(Selection, Selection.End(xlDown)).Select
Set rngcells6 = Selection

Range([AI9], [AI9].End(xlToRight)).Select
Range(Selection, Selection.End(xlDown)).Select
Set rngcells7 = Selection

Range([AN9], [AN9].End(xlToRight)).Select
Range(Selection, Selection.End(xlDown)).Select
Set rngcells8 = Selection

Range([AS9], [AS9].End(xlToRight)).Select
Range(Selection, Selection.End(xlDown)).Select
Set rngcells9 = Selection

Range([AX9], [AX9].End(xlToRight)).Select
Range(Selection, Selection.End(xlDown)).Select
Set rngcells10 = Selection

Range([BC9], [BC9].End(xlToRight)).Select
Range(Selection, Selection.End(xlDown)).Select
Set rngcells11 = Selection

Range([BH9], [BH9].End(xlToRight)).Select
Range(Selection, Selection.End(xlDown)).Select
Set rngcells12 = Selection

zc1 = rngcells1.Rows.Count
zc2 = rngcells2.Rows.Count
zc3 = rngcells3.Rows.Count
zc4 = rngcells4.Rows.Count
zc5 = rngcells5.Rows.Count
zc6 = rngcells6.Rows.Count
zc7 = rngcells7.Rows.Count
zc8 = rngcells8.Rows.Count
zc9 = rngcells9.Rows.Count
zc10 = rngcells10.Rows.Count
zc11 = rngcells11.Rows.Count
zc12 = rngcells12.Rows.Count

tl8 = Range("C6")
If tl8 = 0 Then
i = 1
Else
i = Int(210 / tl8)
End If

filepath = Range("A4")
Debug.Print filepath

Filesheet = Range("A7")
Debug.Print Filesheet



'Most declarations finished, now let's loop through the files and extract data, one type at a time.
'loop through type 1 months here,


Dim Month As Integer
For Month = 3 To zm1 Step 1
Workbooks.Open Filename:=filepath & arrmonth1(Month, 1) & ".xls"
Worksheets(Filesheet).Select

ReDim arrcells1(0 To i, 0 To rngcells1.Rows.Count - 1)
For k = 1 To i
For cellx = 1 To rngcells1.Rows.Count - 1

arrcells1(k - 1, cellx - 1) = Cells(rngcells1(cellx + 1, 2), rngcells1(cellx + 1, 3)).Offset(0, tl8 * (k - 1)).Value
Debug.Print arrcells1(k - 1, cellx - 1)

Next cellx
Next k
ActiveWorkbook.Close False
dest.Activate
[B65336].End(xlUp).Offset(1, 0).Select
Range(Selection, Selection.Offset(i - 1, rngcells1.Rows.Count - 2)) = arrcells1

Next Month

'TYPE 2 extraction


For Month = 3 To zm2 Step 1
Workbooks.Open Filename:=filepath & arrmonth2(Month, 1) & ".xls"
Worksheets(Filesheet).Select

ReDim arrcells2(0 To i, 0 To rngcells2.Rows.Count - 1)
For k = 1 To i
For cellx = 1 To rngcells2.Rows.Count - 1

arrcells2(k - 1, cellx - 1) = Cells(rngcells2(cellx + 1, 2), rngcells2(cellx + 1, 3)).Offset(0, tl8 * (k - 1)).Value
Debug.Print arrcells2(k - 1, cellx - 1)

Next cellx
Next k
ActiveWorkbook.Close False
dest.Activate
[B65336].End(xlUp).Offset(1, 0).Select
Range(Selection, Selection.Offset(i - 1, rngcells2.Rows.Count - 2)) = arrcells2

Next Month


'TYPE 3 extraction , same as above and repeat the whole code for type4,5,6...12









End With

With Application
.Calculation = xlCalculationAutomatic
End With
End Sub





[ This Message was edited by: RET79 on 2002-04-01 08:28 ]

[RET79]

As you can see above, it is pretty tedious and repetitive, although the code runs really quick. If you can help me condense it I would be most grateful. Mark, I think the array declarations are certianly the way forward.

RET79

[Mark O'Brien]

You're right it is quite a hideous macro.

It should be quite straightforward for you to modify it. (emphasis on "you" here)

Here's an example built on my original answer. I've included some additional lines of code that have been commented out, but they should work:

Dim rngMonth(1 To 12) As Range

Dim zm(1 To 12) As Long

Dim rngRows As Range

Dim i as Integer



For i = 1 To 12

'This line is used to make the code more readable.

  Set rngRows = [D8].Offset(, (5 * (i - 1)))

  

  Set rngMonth(i) = Range(rngRows, rngRows.End(xlDown))

  zm(i) = rngMonth(i).Rows.Count

  

  'This would do all of the Debug.Print statements

  For j = 3 To zm(i)

    Debug.Print rngMonth(i)(k, 1)

  Next

  

'TODO:: Use this type of statement to set up the rngCells array

'TODO:: Modify and put the rest of your code in here



'+++++++++++++++++++++++++++++++++++++++++++++++++

'EXAMPLE

'+++++++++++++++++++++++++++++++++++++++++++++++++

'Dim rngCells(1 To 12) As Range

'Dim rngGroup as range



'Set rngGroup = [E9].Offset(, (5 * (i - 1)))

'Set rngCells(i) = Range(Range(rngGroup, rngGroup.End(xlToRight)), Range(rngGroup, rngGroup.End(xlToRight)).End(xlDown))



'TODO::  Once your code is working and everything is set, move all of the Dim

'        statements to the top of the module.  This makes it easier to read.

Next





End Sub

Basically, it looks like you can do all of your commands within that For i = 1 to 12 loop. You'll need to think and see how you can apply it. I hope my example gives you something to get started with. It would probably take me an hour to get it done as the example itself took 10 minutes.

Simple rule, anything you have that is declared like rngCells2, rngCells2...etc can be reduced to a few lines of code by declaring arrays and using a loop, e.g.:

Dim rngCells(1 to 12) as Range

Dim i as Integer



For i = 1 to 12

  Set rngCells = whatever....etc etc

  ...rest of your code...

Next

If you have any problems with specific parts then feel free to repost here.

Cheers

HTH

[Tim Francis-Wright]

Just so you know, when you write

Dim rngmonth1, rngmonth2, rngmonth3, rngmonth4, _
rngmonth5, rngmonth6, rngmonth7, rngmonth8, _
rngmonth9, rngmonth10, rngmonth11, rngmonth12 As Range

Dim zm1, zm2, zm3, zm4, zm5, zm6, zm7, zm8, zm9, zm10, zm11, zm12 As Long

only the last variable on each line is DIM'd as a Range or a Long. The others are defined
as Variants.

[Mark O'Brien]

Good point.

I was trying to avoid saying that. That's a pet peeve of mine when things get declared like that, but since I was getting rid of those statements anyway I didn't raise it.

I'm also not mentioning that the zm(i) array is redundant in:

  zm(i) = rngMonth(i).Rows.Count

  

  'This would do all of the Debug.Print statements

  For j = 3 To zm(i)

and that:

  For j = 3 to rngMonth(i).Rows.Count

would be better (faster). However, I wanted to keep the code as similar as possible.

Also, I forgot to slap in a "Dim j as Integer" in the declarations. That's a bad Mark, bad Mark I say.

All in all, the original code is actually quite a reasonalbe attempt. The only thing it's short of is knowledge of loops and arrays.

_________________
[b] Mark O'Brien

[ This Message was edited by: Mark O'Brien on 2002-04-01 10:16 ]

[RET79]

Also, will this code work when using arrcells(1) instead of arrcells1 ( and rngcells(1) instead of rngcells1):

arrcells1(k - 1, cellx - 1) = Cells(rngcells1(cellx + 1, 2), rngcells1(cellx + 1, 3)).Offset(0, tl8 * (k - 1)).Value
Debug.Print arrcells1(k - 1, cellx - 1)


That's all for now, thanks again guys,

RET79




[ This Message was edited by: RET79 on 2002-04-01 14:18 ]