Page Index Toggle Pages: 1
Topic Tools
Hot Topic (More than 10 Replies) revised setup.pl for consideration (Read 5,384 times)
Carsten
Ex Member


Re: revised setup.pl for consideration
Reply #10 - Nov 4th, 2005 at 12:42pm
Post Tools
hmm- ok, having a look at a way of reusing code wouldn't harm i guess - and of course the extra IF statements are not that big a problem - all i'm saying is that the functionallity of the 'illegal' sub routine is not in use by most boards.
« Last Edit: Nov 4th, 2005 at 12:43pm by »  
Back to top
 
IP Logged
 
old goat
YaBB Legends (Inactive)
*
Offline



Posts: 2,488
Location: York, UK
Re: revised setup.pl for consideration
Reply #9 - Nov 4th, 2005 at 11:36am
Post Tools
would an alternative be to split the sub into a series of subs that can then be called by either 'illegal' or 'my', thus without any conditional testing inside the parent sub?

  

Please don't PM me for support - I'll only go and ignore it!
Back to top
AIM  
IP Logged
 
Carsten
Ex Member


Re: revised setup.pl for consideration
Reply #8 - Nov 4th, 2005 at 10:58am
Post Tools
If this was about a "normal" sub routine only running once or twice, i would agree. But the problem is that it's called one time per forum member - this is one of the most "time sensitive" sub routines in the converter. On a board like yabbforum.com it will run 34,000+ times during conversion.

The "illegal" sub routine was only added to take care of very old boards prior to YaBB 1 Gold SP1 (December 2001), where testing for illegal characters in account names was implemented. Meaning only very few boards will have any illegal account names at all - and if they have - only a few.

Now, adding 5 extra conditional tests for something that will only occur in maybe 5 pct. of the boards is not my idea of optimising the process.
« Last Edit: Nov 4th, 2005 at 10:59am by »  
Back to top
 
IP Logged
 
Tea-Master
YaBB Legends (Active)
*
Offline



Posts: 3,623
Location: Germany
Re: revised setup.pl for consideration
Reply #7 - Nov 4th, 2005 at 8:56am
Post Tools
imho i would go with s_m_b's suggestion to increase the code reuse. he's right if he says that having two seperate functions doing nearly the same is not desireable because it makes code maintenance much harder and increases memory usage (yes unnoticeable but since you, carsten, are talking about an IF statement taking too much time...). if s_m_b would change his $illegalUserFlag to a boolean variable (or in perl an integer) it would be the right way to go imho. remark: string comparisons are really slow at least a lot slower then comparing too integers.
you could of course also do some very fancy things with eval if you want to but...
« Last Edit: Nov 4th, 2005 at 8:59am by Tea-Master »  

--> also known as Agent Zed Wink
Back to top
 
IP Logged
 
old goat
YaBB Legends (Inactive)
*
Offline



Posts: 2,488
Location: York, UK
Re: revised setup.pl for consideration
Reply #6 - Nov 3rd, 2005 at 8:26pm
Post Tools
Put that way, yes, but as you are only running the process once, and 30000 members is going to take a fair while, I don't suppose you'd notice too bad.

agree to differ, perhaps? I'm still learning PERL, after all! 
  

Please don't PM me for support - I'll only go and ignore it!
Back to top
AIM  
IP Logged
 
Carsten
Ex Member


Re: revised setup.pl for consideration
Reply #5 - Nov 3rd, 2005 at 8:19pm
Post Tools
Perl is quite capable in doing if-else tests, but doing it 5 x 30,000 = 150,000 times does have an effect.
  
Back to top
 
IP Logged
 
old goat
YaBB Legends (Inactive)
*
Offline



Posts: 2,488
Location: York, UK
Re: revised setup.pl for consideration
Reply #4 - Nov 3rd, 2005 at 8:09pm
Post Tools
You mean the
Code
Select All
if ($illegalUserFlag eq "true")	{
			open(LLFILE, "$convmemberdir/$user.ll");
				($lastonline, $lastpost, $lastim) = <LLFILE>;
				close(LLFILE);
		    }
		    else    {
				fopen(LLFILE, "$convmemberdir/$user.ll");
				($lastonline, $lastpost, $lastim) = <LLFILE>;
				fclose(LLFILE);
		    } 


sections?
Unless PERL is lousy at processing if-else these are hardly going to have any effect .

Like I said, its only my opinion!  Cheesy
  

Please don't PM me for support - I'll only go and ignore it!
Back to top
AIM  
IP Logged
 
Carsten
Ex Member


Re: revised setup.pl for consideration
Reply #3 - Nov 3rd, 2005 at 7:50pm
Post Tools
old goat wrote on Nov 3rd, 2005 at 6:55pm:
Edited:
no! The sub call is inside the conditional test
Code
Select All
if ($uname !~ /\A[0-9A-Za-z#+-\.@^_]+\Z/) {
		    &IllegalUser($uname);
		    $illegalUserFlag = qq~true~;
		    }
 


- all I did was change the if - else to if so only illegal names had the extra sub to go through. All then go through the one sub to process all the user records.


Yes! You do run all the accounts through the MyUpdateUser sub routine with "with several "illegal account" conditional test" - which i think is bad practice.
« Last Edit: Nov 3rd, 2005 at 7:52pm by »  
Back to top
 
IP Logged
 
old goat
YaBB Legends (Inactive)
*
Offline



Posts: 2,488
Location: York, UK
Re: revised setup.pl for consideration
Reply #2 - Nov 3rd, 2005 at 6:55pm
Post Tools
Given that on the last pass through it a couple of errors were found, and the sequence of processes between the two original subs is different...  that is the only reason.

IMHO repeating whole blocks of code to allow for slightly different situations is bad practice - far too easy to forget a small change needed in one version, or accidentally cause problems with changing the order of program flow. The very fact that you need to have the IllegalUser version in, means the problem has been hit, and makes it work doing best as possible.
More exactly required for converting from third-party boards, where different rules have been applied!

Edited:
Quote:
What you do is run all accounts through a sub routine with several "illegal account" conditional test. Don't think that is a very good idea on a board with 30,000 "legal" members.

no! The sub call is inside the conditional test
Code
Select All
if ($uname !~ /\A[0-9A-Za-z#+-\.@^_]+\Z/) {
		    &IllegalUser($uname);
		    $illegalUserFlag = qq~true~;
		    }
 


- all I did was change the if - else to if so only illegal names had the extra sub to go through. All then go through the one sub to process all the user records.



It is of course all relative  Smiley

As I say, just my observation, and as you point out probably not of great importance.
« Last Edit: Nov 3rd, 2005 at 7:03pm by old goat »  

Please don't PM me for support - I'll only go and ignore it!
Back to top
AIM  
IP Logged
 
Carsten
Ex Member


Re: revised setup.pl for consideration
Reply #1 - Nov 3rd, 2005 at 6:27pm
Post Tools
Seems to remember we already had this discussion. I don't really see any gain here. The reason for making the conditional test for illegal user account names split the conversion into two separate sub routines is that by far the most boards will not have any illegal accounts at all and if they do it's normally a very few.

What you do is run all accounts through a sub routine with several "illegal account" conditional test. Don't think that is a very good idea on a board with 30,000 "legal" members.
  
Back to top
 
IP Logged
 
old goat
YaBB Legends (Inactive)
*
Offline



Posts: 2,488
Location: York, UK
revised setup.pl for consideration
Nov 3rd, 2005 at 11:33am
Post Tools
Being as we no longer have yabbtest, I figured this is as good place as any to offer up code alterations...

Working through setup.pl as the template for my converter, I happened on a load of dupped code (this is from the latest cvs copy btw), and have rewritten and tested this.

in a nutshell, the IllegalUser and MyUpdateUser subs are 95% the same, so I turned the IllegalUser into just the lines to correct the name etc - up to
Code
Select All
 $fixeduser =~ s/(\S+?)(\.\S+$)/$1/i; 


everthing else is then handled by the MyUpdateUser sub with a flag to switch the file open/close segments between the two versions.

Also, I've tweaked the 'date at time' corrector to make it more flexible to '1/1/05' type dates - i.e. single-digit months/days.

You can ignore these if you want, but I needed to make the changes to accomodate my converter better, and figured they might be of some use in 'setup.pl' too...   Smiley

  

Setup_002.zip (Attachment deleted)

Please don't PM me for support - I'll only go and ignore it!
Back to top
AIM  
IP Logged
 
Page Index Toggle Pages: 1
Topic Tools
 
  « Board Index ‹ Board  ^Top