Inefficient code - macro takes too long

**dantray02** · 03-07-2014, 04:48 AM

Hi, I have the following code which is supposed to export the pages in a workbook to pdf and attach it to an email. This works exactly as it should but takes far too long. it is also my first venture into macros and I'm pretty sure it is not as efficient as it should be - most of it has come from googling and copying other peoples code.

Is there anything I can do to make the following quicker or more efficient:

Please Login or Register  to view this content.

[/CODE]

in particular this bit I'm sure can be done better, or quicker:

Please Login or Register  to view this content.

thanks in advance.

**TMS** · 03-07-2014, 05:16 AM

I can't test your code, no workbook, no worksheets, no data, but the second half could be better written.

Change:

Please Login or Register  to view this content.

To:

Please Login or Register  to view this content.

Not selecting sheets and ranges will make the code much quicker and means the final:

Please Login or Register  to view this content.

should be unnecessary.

I don't know what the routine Create_PDF does, but you could probably just pass an array of worksheets to that instead of selecting the sheets and then calling the routine.

It's not easy to provide a specific solution to a problem when most of the information required is "back at home".

Regards, TMS

**TMS** · 03-07-2014, 05:24 AM

May I ask what this construct does:

Please Login or Register  to view this content.

I've never seen anything like that and I don't understand how it works.

Because "F1:shareholderfunds" is in quotes, that must be some sort of Named Range, but a Named Range cannot have a colon (":") in it.

Confused.

Regards, TMS

**dantray02** · 03-07-2014, 05:40 AM

hi trevor, thanks.

the create_pdf is a function to send the selection to a pdf. There are a few macro's using this function and all do it within a second or two. This one that you have helped me with takes around 20 seconds to do.

For this:

Please Login or Register  to view this content.

The macro hides rows on each sheet before sending to pdf based on whether or not there is a 0 or 1 on each row in the K column on each sheet. The K column contains 1's or 0's based on IF functions for data on each row. Is this the best way to do this?

the main problem here is that I have no idea what i'm doing - I am very new to VBA.

I actually don't know what this even means:

Please Login or Register  to view this content.

I just know it works so I copied it for each page and changed the C to D, etc.

Thank you very much for your help.

**Olly** · 03-07-2014, 05:41 AM

Please Login or Register  to view this content.

That will refer to the range between cell F1 and the named range 'shareholderfunds', inclusive.

**apo** · 03-07-2014, 05:48 AM

Maybe this too.. not much saving in code space.. but i thought i would give it a try anyway..

Please Login or Register  to view this content.

**kvsrinivasamurthy** · 03-07-2014, 05:56 AM

Change this

Please Login or Register  to view this content.

as

Please Login or Register  to view this content.

similarly for others.

**apo** · 03-07-2014, 06:04 AM

I am wondering if it would be more efficient to use Autofilter using "0" as the criteria and then just Hide all the visible rows..?

**TMS** · 03-07-2014, 06:52 AM

I am wondering if it would be more efficient to use Autofilter using "0" as the criteria and then just Hide all the visible rows..?

And you would be left with what? Rows not selected in the filter would be hidden, and then youu hide the visible rows ...

**TMS** · 03-07-2014, 06:59 AM

@Olly:

Originally Posted by Olly

Please Login or Register  to view this content.

That will refer to the range between cell F1 and the named range 'shareholderfunds', inclusive.

Thank you. I did not know you could do that. I've always used either a range text string: Range("F1:F10"); a Named Range text string: Range("shareholderfunds"); or a string variable: Range(strShareholderfunds). And, looking at it, I wouldn't have thought that Excel/VBA would be prepared to evaluate that string into a static range and a Named Range part. I would, typically, define shareholderfunds as a Dynamic Named Range ... but then, I'm not sure how it is used here.

Thanks again.

Regards, TMS

**dantray02** · 03-07-2014, 07:16 AM

hi,thanks for all the suggestions:

what would this do, or what should it replace:

Please Login or Register  to view this content.

also, one other thing, which is the only thing stopping my document looking very professional, but the 6 page pdf that exports are different size pages, some are slightly wider than others and there appears to be no reason for this.

Has anyone seen this before?

Also, "shareholderfunds" is the name of a cell. It is the final line on a page of my document.

thanks,
dan.

**dantray02** · 03-07-2014, 07:28 AM

the auto filter would be a good idea I think - what code would I use for that - I have the filter on K1

thanks

**TMS** · 03-07-2014, 07:36 AM

the 6 page pdf that exports are different size pages, some are slightly wider than others and there appears to be no reason for this.

Pretty difficult to comment without seeing some typical data

**apo** · 03-07-2014, 08:41 AM

And you would be left with what? Rows not selected in the filter would be hidden, and then youu hide the visible rows ...

When i read my post again.. I see i didn't word it too well..

What i meant was AutoFilter the range using "0" as the criteria.. create a range with the visible cells.. turn off autofilter.. then hide the rows in the range that was just created..

I am just thinking it might be faster than a loop..

@dantray02.. its late here.. I will post what i mean with the autofilter tomorrow.. hopefully someone will chime in before that for you..

**TMS** · 03-07-2014, 09:29 AM

What i meant was AutoFilter the range using "0" as the criteria.. create a range with the visible cells.. turn off autofilter.. then hide the rows in the range that was just created..

I am just thinking it might be faster than a loop..

Sounds good

Inefficient code - macro takes too long

LinkBack

Thread Tools

Rate This Thread

Display

Inefficient code - macro takes too long

Re: Inefficient code - macro takes too long

Re: Inefficient code - macro takes too long

Re: Inefficient code - macro takes too long

Re: Inefficient code - macro takes too long

Re: Inefficient code - macro takes too long

Re: Inefficient code - macro takes too long

Re: Inefficient code - macro takes too long

Re: Inefficient code - macro takes too long

Re: Inefficient code - macro takes too long

Re: Inefficient code - macro takes too long

Re: Inefficient code - macro takes too long

Re: Inefficient code - macro takes too long

Re: Inefficient code - macro takes too long

Re: Inefficient code - macro takes too long

Thread Information

Users Browsing this Thread

Similar Threads

Clunky Slow Code, Hides Empty Rows, Takes too long...

Opening a network xll file in VBA takes so long it prompts for password instead of code pw

Long code takes an age to run

My Code takes too long to execute

[SOLVED] Counting how long a Macro takes to run

Bookmarks

Bookmarks

Posting Permissions