Home |
Search |
Today's Posts |
#1
Posted to microsoft.public.excel.programming
|
|||
|
|||
The For Loop Worked, but need critique
It took me a day and a half, but I wrote my first "For
loop", and it does what I want it to. I am posting this to invite critique to see how it could be more efficiently written or cleaned up some if any. The purpose of this loop was to work with Columns D and E, which are page and line number columns respectively. They are text entries when I get through with them even though I am using a numeric value pagenumcounter and linenumcounter to keep up with the values as they change. But page 1 is shown in the field as 01, and line 1 is shown as 01, so that they sort properly with page 10 and line 10 values etc when they are in a database. There are several constraints placed on the pagenum. Each time, the range linenum gets to 12, then the next time the loop processes the line, the pagenum is to go up by 1, and the linenum is to go back to a 1, and start climbing again. The other constraint is that if the value in column B changes which is prmo, then we want to treat the page and line number the same as if the line number gets to 12. We want to start the line number back at 1 and add 1 to the page number regardless of what the line number had been. So, I compare two spreadsheet range values "Currprmo" and "prevprmo" in the spreadsheet to accomplish that part. I used spreadsheet range "currprmo" to manage the current line being processed, and I used the spreadsheet range "prevprmo" to look back at what the value of prmo was in the previous line. With all that said, here is my procedure for your critique: Sub add_page_and_line() Dim cRows As Long cRows = Cells(Rows.Count, "A").End(xlUp).Row Range("pagenumcounter").Value = 1 Range("linenumcounter").Value = 0 Range("rownow").Value = 1 Range("d1").Select For i = 1 To cRows Range("Currprmo").Value = Range("B1").Offset(Range ("Rownow") - 1, 0).Value If i = 1 Then Range("Prevprmo").Value = Range("B1").Value Else Range("Prevprmo").Value = Range("B1").Offset(Range ("Rownow") - 2, 0).Value End If Range("rownow").Value = Range("rownow").Value + 1 If Range("Linenumcounter").Value + 1 = 13 Or Range ("Currprmo").Value _ < Range("Prevprmo") Then Range("pagenumcounter").Value = Range ("pagenumcounter").Value + 1 Range("linenumcounter").Value = 1 Else Range("linenumcounter").Value = Range ("linenumcounter").Value + 1 End If ActiveCell.Value = "=pagenum" Selection.Copy Selection.PasteSpecial Paste:=xlValues, Operation:=xlNone, SkipBlanks:= _ False, Transpose:=False Application.CutCopyMode = False ActiveCell.Offset(0, 1).Select ActiveCell.Value = "=linenum" Selection.Copy Selection.PasteSpecial Paste:=xlValues, Operation:=xlNone, SkipBlanks:= _ False, Transpose:=False Application.CutCopyMode = False ActiveCell.Offset(1, -1).Select Next Range("d1").Select End Sub |
#2
Posted to microsoft.public.excel.programming
|
|||
|
|||
The For Loop Worked, but need critique
Hi
it's a long post but one thing that is always preferable is to address your objects directly rather than by selecting them e.g. untested, mind Range("B2").copy rather than Range("B2").select selection.copy it is shorter to code and MUCH quicker to run once your used to it, it is possibly easier to debug. Regards Tim "Bruce Roberson" wrote in message ... It took me a day and a half, but I wrote my first "For loop", and it does what I want it to. I am posting this to invite critique to see how it could be more efficiently written or cleaned up some if any. The purpose of this loop was to work with Columns D and E, which are page and line number columns respectively. They are text entries when I get through with them even though I am using a numeric value pagenumcounter and linenumcounter to keep up with the values as they change. But page 1 is shown in the field as 01, and line 1 is shown as 01, so that they sort properly with page 10 and line 10 values etc when they are in a database. There are several constraints placed on the pagenum. Each time, the range linenum gets to 12, then the next time the loop processes the line, the pagenum is to go up by 1, and the linenum is to go back to a 1, and start climbing again. The other constraint is that if the value in column B changes which is prmo, then we want to treat the page and line number the same as if the line number gets to 12. We want to start the line number back at 1 and add 1 to the page number regardless of what the line number had been. So, I compare two spreadsheet range values "Currprmo" and "prevprmo" in the spreadsheet to accomplish that part. I used spreadsheet range "currprmo" to manage the current line being processed, and I used the spreadsheet range "prevprmo" to look back at what the value of prmo was in the previous line. With all that said, here is my procedure for your critique: Sub add_page_and_line() Dim cRows As Long cRows = Cells(Rows.Count, "A").End(xlUp).Row Range("pagenumcounter").Value = 1 Range("linenumcounter").Value = 0 Range("rownow").Value = 1 Range("d1").Select For i = 1 To cRows Range("Currprmo").Value = Range("B1").Offset(Range ("Rownow") - 1, 0).Value If i = 1 Then Range("Prevprmo").Value = Range("B1").Value Else Range("Prevprmo").Value = Range("B1").Offset(Range ("Rownow") - 2, 0).Value End If Range("rownow").Value = Range("rownow").Value + 1 If Range("Linenumcounter").Value + 1 = 13 Or Range ("Currprmo").Value _ < Range("Prevprmo") Then Range("pagenumcounter").Value = Range ("pagenumcounter").Value + 1 Range("linenumcounter").Value = 1 Else Range("linenumcounter").Value = Range ("linenumcounter").Value + 1 End If ActiveCell.Value = "=pagenum" Selection.Copy Selection.PasteSpecial Paste:=xlValues, Operation:=xlNone, SkipBlanks:= _ False, Transpose:=False Application.CutCopyMode = False ActiveCell.Offset(0, 1).Select ActiveCell.Value = "=linenum" Selection.Copy Selection.PasteSpecial Paste:=xlValues, Operation:=xlNone, SkipBlanks:= _ False, Transpose:=False Application.CutCopyMode = False ActiveCell.Offset(1, -1).Select Next Range("d1").Select End Sub |
#3
Posted to microsoft.public.excel.programming
|
|||
|
|||
The For Loop Worked, but need critique
Bruce,
A few immediate thoughts. I know you use CDO, but I hope the lack of indentation is caused by that, and that you use it. A few comments in the code might be useful for later use. Sub add_page_and_line() Dim cRows As Long cRows = Cells(Rows.Count, "A").End(xlUp).Row Range("pagenumcounter").Value = 1 Range("linenumcounter").Value = 0 Range("rownow").Value = 1 Range("d1").Select This last line seems superfluous, other cells are selected in the loop, and I cannot see any action upon the selected cell For i = 1 To cRows Range("Currprmo").Value = Range("B1").Offset(Range ("Rownow") - 1, 0).Value This is a personal thing, but I don't like defaulting properties. I would always explicitly state what property I am using, in this case Range("Currprmo").Value = Range("B1").Offset(Range("Rownow").Value - 1, 0).Value If i = 1 Then Range("Prevprmo").Value = Range("B1").Value Else Range("Prevprmo").Value = Range("B1").Offset(Range("Rownow") - 2, 0).Value End If Range("rownow").Value = Range("rownow").Value + 1 If Range("Linenumcounter").Value + 1 = 13 Or Range("Currprmo").Value _ < Range("Prevprmo") Then Again, a personal thing, but I break each test to its own line, If Range("Linenumcounter").Value + 1 = 13 Or _ Range("Currprmo").Value < Range("Prevprmo") Then I find it more readable Range("pagenumcounter").Value = Range("pagenumcounter").Value + 1 Range("linenumcounter").Value = 1 Else Range("linenumcounter").Value = Range("linenumcounter").Value + 1 End If ActiveCell.Value = "=pagenum" Selection.Copy Selection.PasteSpecial Paste:=xlValues, Operation:=xlNone, SkipBlanks:= _ False, Transpose:=False Application.CutCopyMode = False All of these arguments are not necessary, the only non-default you are using is Paste, so keep it simple Selection.PasteSpecial Paste:=xlValues and the same below ActiveCell.Offset(0, 1).Select ActiveCell.Value = "=linenum" Selection.Copy Selection.PasteSpecial Paste:=xlValues, Operation:=xlNone, SkipBlanks:= _ False, Transpose:=False Application.CutCopyMode = False ActiveCell.Offset(1, -1).Select This can all be simplified to, without doing the select. Select is slow and costly (in processing terms), but much worse can be problemmatical, because it means that the worksheet you are working on has to be visible. If it isn't, you get the dreaded 1004 error (you've seen that haven't you), so it should be avoided wherever possible With ActiveCell.Offset(0, 1) .Value = "=linenum" .Copy .PasteSpecial Paste:=xlValues .Offset(1, -1).Select End With Application.CutCopyMode = False Next Range("d1").Select End Sub HTH Bob |
#4
Posted to microsoft.public.excel.programming
|
|||
|
|||
The For Loop Worked, but need critique
Bob:
What is the "CDO" that I am using that you are referring to? Otherwise, I'm printing out your message so I can study it more carefully. Thanks, Bruce *** Sent via Developersdex http://www.developersdex.com *** Don't just participate in USENET...get rewarded for it! |
#5
Posted to microsoft.public.excel.programming
|
|||
|
|||
The For Loop Worked, but need critique
That's the horrible manky web based interface that you used to post your question with in the
first place. :-) Seriously though, the best way to interact with these groups is through a newsreader such as Outlook Express (And some will giv eyou just as much grief about that one as well, but it works for me most of the time). Simply add a news service to it using the microsoft news server msnews.microsoft.com and it is much easier to read/write/follow your messages than what you are using. That having beed said, when I log on from work, I don't have acces to news servers, they are restricted, so the CDO interface is the only way I can access from work. At home though, it is a much nicer experience all round. Chip Pearson has some more info on his site at the following link:- http://cpearson.com/excel/links.htm#Newsgroups -- Regards Ken....................... Microsoft MVP - Excel Sys Spec - Win XP Pro / XL2K & XLXP ---------------------------------------------------------------------------- Attitude - A little thing that makes a BIG difference ---------------------------------------------------------------------------- "Bruce Roberson" wrote in message ... Bob: What is the "CDO" that I am using that you are referring to? Otherwise, I'm printing out your message so I can study it more carefully. Thanks, Bruce *** Sent via Developersdex http://www.developersdex.com *** Don't just participate in USENET...get rewarded for it! |
#6
Posted to microsoft.public.excel.programming
|
|||
|
|||
The For Loop Worked, but need critique
That's the horrible manky web based interface that you used to post your question with in the first place. :-) Hey Ken, That was my question to be rude and superior about <vbg. Bob |
#7
Posted to microsoft.public.excel.programming
|
|||
|
|||
The For Loop Worked, but need critique
LOL - I didn't say half as many nasty things as I could have done, so I figured there was plenty
left for you. <vbg -- Regards Ken....................... Microsoft MVP - Excel Sys Spec - Win XP Pro / XL2K & XLXP ---------------------------------------------------------------------------- Attitude - A little thing that makes a BIG difference ---------------------------------------------------------------------------- "Bob Phillips" wrote in message ... That's the horrible manky web based interface that you used to post your question with in the first place. :-) Hey Ken, That was my question to be rude and superior about <vbg. Bob |
#8
Posted to microsoft.public.excel.programming
|
|||
|
|||
The For Loop Worked, but need critique
Blame the need on having to use the web based interface on the IT people at
my office. They get a power trip tieing accountants hands (especially mine for some reason) <vbg They have my Lotus Notes so tied down that I do well to send an email. And they have the gall to lock me out of my own PC's access to the Outlook Express directory. So, I guess I'm stuck with this "manky web based approach" when at work. But at home, its definitely Outlook Express for me. Have a good weekend. Bruce "Bob Phillips" wrote in message ... That's the horrible manky web based interface that you used to post your question with in the first place. :-) Hey Ken, That was my question to be rude and superior about <vbg. Bob |
#9
Posted to microsoft.public.excel.programming
|
|||
|
|||
The For Loop Worked, but need critique
LOL - I know EXACTLY how you feel on that one.
-- Regards Ken....................... Microsoft MVP - Excel Sys Spec - Win XP Pro / XL2K & XLXP ---------------------------------------------------------------------------- Attitude - A little thing that makes a BIG difference ---------------------------------------------------------------------------- "Bruce Roberson" wrote in message ... Blame the need on having to use the web based interface on the IT people at my office. They get a power trip tieing accountants hands (especially mine for some reason) <vbg They have my Lotus Notes so tied down that I do well to send an email. And they have the gall to lock me out of my own PC's access to the Outlook Express directory. So, I guess I'm stuck with this "manky web based approach" when at work. But at home, its definitely Outlook Express for me. Have a good weekend. Bruce "Bob Phillips" wrote in message ... That's the horrible manky web based interface that you used to post your question with in the first place. :-) Hey Ken, That was my question to be rude and superior about <vbg. Bob |
Reply |
Thread Tools | Search this Thread |
Display Modes | |
|
|
Similar Threads | ||||
Thread | Forum | |||
Find loop doesn't loop | Excel Discussion (Misc queries) | |||
The reply worked, but I still need help.... | Excel Worksheet Functions | |||
Most hours worked | New Users to Excel | |||
It Worked!!! :-) | Excel Discussion (Misc queries) | |||
Hours worked | Excel Worksheet Functions |