»
Tech Talk
»
Beyond the Basics
»
Statement Issues
| Beyond the Basics |
|
Statement Issues |
|
|
Christopher
Moderator
Member Rara Avis
since 1999-08-02
Posts 8296Purgatorial Incarceration 
|
Hello, I have a few issues with this part I'm working on right now. I've spent the past few days trying to figure it out and am positive it must be something small... but I can't find it. What is happening in this first part is that if I set the $list value as "1" it adds the email address and ID# (can I do this without it???). However, it will add it again and again, just like the last one I was working on. However, I don't see the same issues here? Lost. #!/usr/local/bin/perl require "toolbox.pl"; $list = 1; $email = "chris\@countlesshorizons.com"; #Check if wants on mailing list (0 = no, 1 = yes, 2 = remove). if ($list == 1) { open (MAILLIST, "mail_list.idx") || die "Can't open Mail List: $!"; @index = <MAILLIST>; close (MAILLIST); foreach $line (@index) { ($mEmail, $key) = split(/\|/, $line); if ($mEmail eq $email) { $temp = 0; last; } else { &listID; &getLock; open (MAILLIST, ">>mail_list.idx") || die "Can't open Mail List: $!"; print MAILLIST "$email|$listID\n"; close (MAILLIST); &dropLock; last; } } } ###This part here is relatively temporary - I am just not sure how to REMOVE a part of a record. I checked "delete" but that only works with hash elements and seems a long way to do it. elsif ($list == 2) { open (MAILLIST, "mail_list.idx") || die "Can't open Mail List: $!"; @index = <MAILLIST>; close (MAILLIST); foreach $line (@index) { ($mEmail, $key) = split(/\|/, $line); if ($mEmail ne $email) { last;} else { open (MAILLIST, ">mail_list.idx") || die "Can't open Mail List: $!"; foreach $line(@index) { print MAILLIST "$line\n"; } close (MAILLIST); &dropLock; last; } } } else {$temp = 0;} ### I think there MUST be a better way to do this??? sub listID { open (LISTID, "<list_id.db") || die "Can't open List ID file: $!\n\n"; $lastListID = <LISTID>; close (LISTID); &getLock; $nextListID = $lastListID + 1; $listID = $nextListID; $listID = sprintf("%6d", $listID); $listID =~tr/ /0/; open (LISTID, ">list_id.db"); print LISTID $listID; close (LISTID); &dropLock; } &header; print "Done."; &footer; |
||
| © Copyright 2002 C.G. Ward - All Rights Reserved | |||
|
Ron
Administrator
Member Rara Avis
since 1999-05-19
Posts 8669Michigan, US |
if ($list == 1) { open (MAILLIST, "mail_list.idx") || die "Can't open Mail List: $!"; @index = <MAILLIST>; close (MAILLIST); $found = 0; foreach $line (@index) { ($mEmail, $key) = split(/\|/, $line); if ($mEmail eq $email) { $temp = 0; $found = 1; last; } } # this MUST be outside the foreach loop or it'll always be run # the first time $mEmail doesn't match $email if (! $found) { &listID; &getLock; open (MAILLIST, ">>mail_list.idx") || die "Can't open Mail List: $!"; print MAILLIST "$email|$listID\n"; close (MAILLIST); &dropLock; } } elsif ($list == 2) { open (MAILLIST, "mail_list.idx") || die "Can't open Mail List: $!"; @index = <MAILLIST>; close (MAILLIST); my @newIndex; foreach $line (@index) { ($mEmail, $key) = split(/\|/, $line); if ($mEmail eq $email) { # do nothing cause we don't save it } else { chomp ($line); push (@newIndex, $line); # not the one, so save it in new array } } # must be moved outside the foreach loop! &getLock; # you forgot this in your code, I think open (MAILLIST, ">mail_list.idx") || die "Can't open Mail List: $!"; # write the newIndex, which no longer contains the removed email address foreach $line(@newIndex) { print MAILLIST "$line\n"; } close (MAILLIST); &dropLock; } else {$temp = 0;} # I don't understand what this does? |
||
|
Christopher
Moderator
Member Rara Avis
since 1999-08-02
Posts 8296Purgatorial Incarceration |
Thank you! I can't believe I missed that... OUT of the loop Chris, indeed.  The $temp=0 is just there because I need a final "else" statement and didn't know what to do with it. I couldn't use a single "if / else" because there will be three possible values passed. Ron, thank you so much for your time. [This message has been edited by Christopher (11-24-2002 03:52 AM).] |
||
|
Ron
Administrator
Member Rara Avis
since 1999-05-19
Posts 8669Michigan, US |
A few quick suggestions, if I may? The key to structured programming and to future maintenance is usually modularity. By structured, I mean that it's easier to "see" what's happening when it's broken into smaller chunks. By maintenance, I mean it's easier to change things later if they are repeated as little as possible. Both boil down to functions. For example, one way to visualize what's happening is to structure your "menu" into subroutines. &AddTheUser if ($list == 1); &RemoveUser if ($list ==2); Your add and remove routines then become separate subroutines and this will often make it easier to understand what is happening. Getting rid of the outer conditional simplifies, and you might have noticed you had a one-time operation embedded in a loop. Plus, naming the routines lends to self-documentation. And later, when you discover the need to update a changed email address, it'll be much easier to add a $list == 3. As for maintenace, any time you find yourself repeating the same code, think about making it a function. For example: sub ReadEmailList { open (MAILLIST, "mail_list.idx") || die "Can't open Mail List: $!"; my @index = <MAILLIST>; close (MAILLIST); return @index; } You could then call it from your other routines with @mailArray = ReadEmailList();. If you ever need to change the location of the file later, you'll only need to track down one read procedure. Note this can be done with Writes as well, but not nearly so easily or consistently. |
||
|
Ron
Administrator
Member Rara Avis
since 1999-05-19
Posts 8669Michigan, US |
Just saw your response, Chris. You don't need the final else clause. Just close the brackets and you're done. if ($condition) { … } elsif ($condition) { … } # end of block |
||
|
Christopher
Moderator
Member Rara Avis
since 1999-08-02
Posts 8296Purgatorial Incarceration |
Very cool - I was planning on making it modular, but your example helps me see that i can do it other places (when use same code more than once) to great benefit as well. If you don't mind, I want to ask a couple of questions in regard to this. The first: Can you give me a better understanding of the difference between "my" and "local"? I know that it keeps that variable to whatver block it's in, but my book also says: "Variables declared with "my" are 'lexically' scoped, whereas those declared with "local" are dynamically scoped." I'm not sure I get the difference? Next: Have broken this up since, but will use this to go through for understanding... elsif ($list == 2) { open (MAILLIST, "mail_list.idx") || die "Can't open Mail List: $!"; @index = <MAILLIST>; close (MAILLIST); my @newIndex; #Creating an array JUST for this block, right? foreach $line (@index) { ($mEmail, $key) = split(/\|/, $line); if ($mEmail eq $email) { } else { chomp ($line); push (@newIndex, $line); #this is the first part i'm getting lost on - I thought "push" added a value? Why would we be adding a value here? } } &getLock; open (MAILLIST, ">mail_list.idx") || die "Can't open Mail List: $!"; # "write the newIndex, which no longer contains the removed email address" - again, kind of confused here... i'm sure your answer to the above will explain it, but how does it no longer contain the addy? foreach $line(@newIndex) { print MAILLIST "$line\n"; } close (MAILLIST); &dropLock; } |
||
|
Ron
Administrator
Member Rara Avis
since 1999-05-19
Posts 8669Michigan, US |
The main difference between "my" and "local" are where and how Perl stores them in memory (the name space). I could probably spend three or four pages talking about "my" and "local" - or I can just give you a bit of history. When Perl first came out, "local" was the only alternative there was to global. It didn't work very well. So, the "my" scoping was added. And there's really little reason to use anything else. The main reason for limiting the scope of a variable is to make sure it doesn't affect other parts of the program. One of the most common bugs is to add a new routine to a 5,000 line application and accidentally use a variable name that's already been used somewhere else. When your new routine changes the value of that variable, it make other parts of the app do some very weird things. Rule of thumb - global variables, that can accessed anywhere in the program, should be avoided like HIV. When I started programming, computers had a lot less memory than is now typical, and we learned not to waste it unnecessarily. So, I tend to also use the "my" scoping to limit the lifetime of a variable. In the example you cite above, the @newIndex array can potentially get pretty big. But I know when the program execution leaves that elsif block, all the memory used by @newIndex will be released. One caveat, though, if only because I see it in a lot of programs. The scope of a "my" variable is determined by the block where it was declared. Loops are blocks, too, but that is NOT where you want to put your declarations, not even for throw-away variables. For example, in the example above, we could have declared my ($mEmail, $key) = … without any change in the logic. Those variables aren't used outside the loop, they're just throw-away variables. But every time the loop ends and starts anew, Perl is going to clean them out of memory, only to set aside memory again the next time it hits the "my" statement. If that loop runs through a few thousand iterations, that can amount to a LOT of unnecessary overhead. Variable declarations should very rarely be made within loops. You are correct, BTW, that push adds a value to an array. In the foreach loop, we check to see if the next $line in @index contains the email address you want to remove. If it doesn't, then we push it onto the @newIndex array. But if it IS the address we want to remove, we don't push it onto the new array. At the end of the loop, @newIndex should hold every $line that was in @index - EXCEPT for the email address you wanted to remove. We've essentially moved every element except one out of the original array into a new array. You wouldn't want to do something like this with a half-gigabyte file, but with arrays of just a few thousand small items, rebuilding it in memory is both the fastest and easiest to implement. |
||
|
Christopher
Moderator
Member Rara Avis
since 1999-08-02
Posts 8296Purgatorial Incarceration |
ooooooohhh! Now that is cool. I, for the moment, will take your answer between my and local at face value. If I can get away with the former then that works for me. Does that mean that local will eventually be phased out? Just curious. The rest - man you are so good. You answer one question and suddenly my understanding grows more than just that. I may actually do something right one of these days! Curious - do you have any suggestions for tracking subroutines? One thing I started to do was to embed them into other subroutines... but quickly realized that I would have to be very careful with that because it would be easy to get lost, or probably more importantly, make it very difficult for myself to make changes in the future. I've tried developing some way of tracking them, but other than simply listing them have been at a loss to figure one out. Again, thanks Ron. I really appreciate that you not only address my immediate issue, but help me to understand it so I can work other things out for myself! [This message has been edited by Christopher (11-27-2002 07:28 PM).] |
||
|
Ron
Administrator
Member Rara Avis
since 1999-05-19
Posts 8669Michigan, US |
Tracking subroutines? You mean as opposed to remembering them? Don't know what you're using, but you might try downloading TextPad. It has some very powerful features, including very easy to use bookmarks. When you can't remember a subroutine call, it makes it easy to find or get to it. |
||
|
|
⇧ top of page ⇧ |
|
| All times are ET (US). All dates are in Year-Month-Day format. | ||
