Error Checking in Excel
Thanks Thanks:  0
Likes Likes:  0
Page 1 of 2 12 LastLast
Results 1 to 10 of 12

Thread: Tedious definitions

  1. #1
    Board Regular
    Join Date
    Mar 2002
    Location
    England, UK.
    Posts
    526
    Post Thanks / Like
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    Default

    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 ]

  2. #2
    MrExcel MVP Mark O'Brien's Avatar
    Join Date
    Feb 2002
    Location
    Columbus, OH, USA
    Posts
    3,530
    Post Thanks / Like
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    Default

    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

  3. #3
    Board Regular
    Join Date
    Mar 2002
    Location
    England, UK.
    Posts
    526
    Post Thanks / Like
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    Default

    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

  4. #4
    MrExcel MVP Mark O'Brien's Avatar
    Join Date
    Feb 2002
    Location
    Columbus, OH, USA
    Posts
    3,530
    Post Thanks / Like
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    Default

    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.




  5. #5
    Board Regular
    Join Date
    Mar 2002
    Location
    England, UK.
    Posts
    526
    Post Thanks / Like
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    Default

    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 ]

  6. #6
    Board Regular
    Join Date
    Mar 2002
    Location
    England, UK.
    Posts
    526
    Post Thanks / Like
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    Default

    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

  7. #7
    MrExcel MVP Mark O'Brien's Avatar
    Join Date
    Feb 2002
    Location
    Columbus, OH, USA
    Posts
    3,530
    Post Thanks / Like
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    Default

    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

  8. #8
    Board Regular
    Join Date
    Feb 2002
    Posts
    75
    Post Thanks / Like
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    Default

    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.


    "Interfere? Of course we should interfere! Always do what you're best at, that's what I say!" -- The Doctor, Nightmare of Eden

  9. #9
    MrExcel MVP Mark O'Brien's Avatar
    Join Date
    Feb 2002
    Location
    Columbus, OH, USA
    Posts
    3,530
    Post Thanks / Like
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    Default

    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 ]

  10. #10
    Board Regular
    Join Date
    Mar 2002
    Location
    England, UK.
    Posts
    526
    Post Thanks / Like
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    Default


    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 ]

Some videos you may like

User Tag List

Like this thread? Share it with others

Like this thread? Share it with others

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •