Consulting

Results 1 to 7 of 7

Thread: Solved: Variable type question

  1. #1
    VBAX Contributor
    Joined
    Apr 2006
    Posts
    144
    Location

    Solved: Variable type question

    Hi everyone,

    The following code is to merge all records from range A3 downwards from all sheets into sheet1. The data to be merged needs to be unique - ie if I have multiple records with string value of "SSSS" in column B in any sheet, only 1 entry is copied into sheet1. The problem I'm having is the code merges everything including duplicates.

    The problem, I think, is the variable declaration. Local variable cl is defined a range and I'm trying to do a countif based on a range rather than a value of a range in the for loop.

    Sub Combine()
        Dim cl             As Range
        Dim wks             As Worksheet
     
        For Each wks In ThisWorkbook.Worksheets
            If wks.Name <> "Sheet1" And wks.Range("A3") <> "" Then
                For Each cell In wks.Range(wks.Range("A3"), wks.Range("A5").End(xlDown))
                If WorksheetFunction.CountIf(Worksheets("Master").Range("B:B"), cl) = 0 Then
                  cl.EntireRow.Copy Destination:=Worksheets("Sheet1").Range("A65536").End(xlUp).Offset(1, 0)
                End If
                Next cl
            End If
        Next wks
     
    End Sub
    Any suggestion please. Thanks for your help.

    Regards


    KP

  2. #2
    Administrator
    Chat VP
    VBAX Guru johnske's Avatar
    Joined
    Jul 2004
    Location
    Townsville, Australia
    Posts
    2,872
    Location
    It will error because you haven't set the range, e.g.

    [VBA]
    Set cl = Range("A1:A10")
    [/VBA]
    You know you're really in trouble when the light at the end of the tunnel turns out to be the headlight of a train hurtling towards you

    The major part of getting the right answer lies in asking the right question...


    Made your code more readable, use VBA tags (this automatically inserts [vba] at the start of your code, and [/vba ] at the end of your code) | Help those helping you by marking your thread solved when it is.

  3. #3
    Distinguished Lord of VBAX VBAX Grand Master Bob Phillips's Avatar
    Joined
    Apr 2005
    Posts
    25,443
    Location
    I think cl should be cell. No Option Explicit!
    ____________________________________________
    Nihil simul inventum est et perfectum

    Abusus non tollit usum

    Last night I dreamed of a small consolation enjoyed only by the blind: Nobody knows the trouble I've not seen!
    James Thurber

  4. #4
    VBAX Master Norie's Avatar
    Joined
    Jan 2005
    Location
    Stirling, Scotland
    Posts
    1,831
    Location
    The problem has nothing to do with variable declaration as far as I can see.

    If you only want unique values why are you testing if the count is 0?

    Surely you should be checking for a count of 1?

  5. #5
    VBAX Contributor
    Joined
    Apr 2006
    Posts
    144
    Location

    Smile

    Hi everyone,

    XLD has quickly identified a typo which prevented the code from running. I declared Cl as range but I wrongly refer this variable as Cell in the For loop. The codes works perfectly without expression Option Explicit.

    Johnske, your suggestion is noted & thanks. However, the expression wks.Range(wks.Range("A3"), wks.Range("A3").End(xlDown)) is more dynamic in terms of variable ranges in sheets.

    Norie, you're right - nothing wrong with the variable object being declared as range. I use the countif is to copy only unique entries to sheet1. In other words, when an entry has not been copied to sheet1 before, the countif function returns 0 and the code proceeds to copying.

    The working code is as follows:

    Sub Combine() Dim cl As Range Dim wks As Worksheet
    For Each wks In ThisWorkbook.Worksheets If wks.Name <> "Sheet1" And wks.Range("A3") <> "" Then For Each cell In wks.Range(wks.Range("A3"), wks.Range("A3").End(xlDown)) If WorksheetFunction.CountIf(Worksheets("Master").Range("B:B"), cl) = 0 Then cl.EntireRow.Copy Destination:=Worksheets("Sheet1").Range("A65536").End(xlUp).Offset(1, 0) End If Next cl End If Next wks
    End Sub
    Your inputs are greatly appreciated. Have a nice weekend.

    Best regards


    KP

  6. #6
    Administrator
    Chat VP VBAX Guru johnske's Avatar
    Joined
    Jul 2004
    Location
    Townsville, Australia
    Posts
    2,872
    Location
    Quote Originally Posted by Digita
    ... The codes works perfectly without expression Option Explicit...
    you miss the point entirely - if you had used Option Explicit, Visual Basic would have told you that you had used an undeclared variable. In fact, if you type Option Explicit at the top of a code module and paste your code (above) beneath that, then click 'Debug' > 'Compile VBAProject', it will still tell you that...

    You need Option Explicit to force you to make your code logically consistent, otherwise Visual Basic is more or less free to interpret your code in its own way. This may possibly - or may not - give you the intended result for now, but this is just by chance, it can give you entirely different results when used in another workbook with a different layout
    You know you're really in trouble when the light at the end of the tunnel turns out to be the headlight of a train hurtling towards you

    The major part of getting the right answer lies in asking the right question...


    Made your code more readable, use VBA tags (this automatically inserts [vba] at the start of your code, and [/vba ] at the end of your code) | Help those helping you by marking your thread solved when it is.

  7. #7
    VBAX Contributor
    Joined
    Apr 2006
    Posts
    144
    Location
    I have added the Option explicit on top of the code.

    Thanks again Johnske for your explanation.

    Regards


    KP

Posting Permissions

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