some perl programs I wrote

U

Uri Guttman

R> check out some of my perl programs,
R> http://offlame.memebot.com

you have a lot of perl and programming to learn still.


#!/usr/bin/perl
use strict;
my $npflag = 0;
my $n;
my @z;

you don't use @z anywhere

print "Prime number generator starting at 3 and going up....\n";

foreach my $q (4 .. 10000) {
$npflag = 0;
foreach my $m (1 .. $q) {

you only need to test factors up to the square root of the goal. this is
a very well known optimization

next if $m == 1;
next if $m == $q;

your inner loop already is up to $q so why do you need to test for this
again? if you really wanted this make the inner loop go to $q -1

$_ = "$q" / "$m";

that is horrible in so many ways. why did you put "" around the
variables? especially since you then do a math operation on them. why
did you assign the result to $_ instead of a named variable?

if ($_ !~ /\./) {

did you know you could use named variables with the !~ operator? and
checking if a number has a fractional part with a regex is very
clunky. it is better to stick to pure math ops like int(), * and /.

$npflag = 1;
now, you know this number isn't a prime. why do you continue to test it
for being a prime?
}

}

if (! $npflag) {

ever heard of unless?

print "We have found prime : $q\n";
sleep 1;

so when you learn about all the corrections i said and understand what
they mean and don't do any of them again, post more code here. keep on
trying to get better.

uri
 
P

Peter J. Holzer

R> check out some of my perl programs,
R> http://offlame.memebot.com

you have a lot of perl and programming to learn still. [...]
$_ = "$q" / "$m";

that is horrible in so many ways. why did you put "" around the
variables? especially since you then do a math operation on them. why
did you assign the result to $_ instead of a named variable?

if ($_ !~ /\./) {

did you know you could use named variables with the !~ operator? and
checking if a number has a fractional part with a regex is very
clunky. it is better to stick to pure math ops like int(), * and /.

And especially %.

hp
 
U

Uri Guttman

R> check out some of my perl programs,
R> http://offlame.memebot.com
you have a lot of perl and programming to learn still. PJH> [...]
$_ = "$q" / "$m";

that is horrible in so many ways. why did you put "" around the
variables? especially since you then do a math operation on them. why
did you assign the result to $_ instead of a named variable?

if ($_ !~ /\./) {

did you know you could use named variables with the !~ operator? and
checking if a number has a fractional part with a regex is very
clunky. it is better to stick to pure math ops like int(), * and /.

PJH> And especially %.

somehow i forgot that one. it was very late and i was in shock from
reading that code. i seem to recall the OP posted this before as i seem
to have a link to one of its programs in my browser history. this does
not bode well as i bet it never took the previous feedback.

i should use it as a test to see what perl people know. a client of mine
does that, he sends out a sub about 20 lines long and asks the candidate
to give edits and feedback on making it better. it has many ways to be
improved. interested idea and much better than any multiple choice
tests. it gets into your true coding skills to see ways to write better
code.

uri
 
R

Ralph Malph

Uri said:
R> check out some of my perl programs,
R> http://offlame.memebot.com

you have a lot of perl and programming to learn still.
No, the OP needs to stop making some silly mistakes
but he is clearly a quite capable programmer.
You point out nothing but trivial things such as
unused variables
and act like a drama queen using words like
"horrible" to describe these things.

To the OP: there is a saying that bike sheds are
harder to make than nuclear power plants.
http://www.freebsd.org/doc/en_US.ISO8859-1/books/faq/misc.html#BIKESHED-PAINTING
Online you will find unemployed and jaded losers
willing to pick apart trivial details but not help
you with anything of significance.
Please, don't frustrate yourself looking for their
approval for it does not exist in their miserable
lives.
Trust me, it is quite telling that you only
received comments on one of your smaller works
listed on the (shorter) second page of your projects
list.
As with the bike shed you will receive a million
comments on the small prime number code but not
with your text editor!

Oh, and if you were bothered by Uri's comments
then please do not worry. He is a morbidly obese
shut in. One of the aforementioned jaded losers.
Nobody has seen him in months...his
doorways are now far to narrow to allow him out.
Even if he turns sideways.
I must hand it to him though, it is quite a novel
way to fight foreclosure!

Happy Days to You All!
Ralph Malph
 
J

Jürgen Exner

Ralph Malph said:
Uri Guttman wrote:
Oh, and if you were bothered by Uri's comments
then please do not worry. He is a morbidly obese
shut in. One of the aforementioned jaded losers.
Nobody has seen him in months...his
doorways are now far to narrow to allow him out.
Even if he turns sideways.
I must hand it to him though, it is quite a novel
way to fight foreclosure!

Considering that your posting history in this NG is very short, none of
your postings had any Perl-related content whatsoever, and almost all
are attacks on Uri, who on the otherhand is a long-standing well
respected member of CLPM I in your place would be very careful with such
libel and defamation. It not only puts you in a very poor light, it also
exposes you to legal actions.

jue
 
J

Jens Thoms Toerring

Robin said:
check out some of my perl programs,
http://offlame.memebot.com
let me know if you like them.... :)

I unfortunately have to agree with others here that didn't
like your code - but I only looked at the program for gene-
rating prime numbers and the "text editor" yet. And both are
not winners... Here's the one for generating prime numbers:

1 #!/usr/bin/perl
2 use strict;
3 my $npflag = 0;
4 my $n;
5 my @z;
6 print "Prime number generator starting at 3 and going up....\n";
7
8 foreach my $q (4 .. 10000) {
9 $npflag = 0;
10 foreach my $m (1 .. $q) {
11 next if $m == 1;
12 next if $m == $q;
13 $_ = "$q" / "$m";
14 if ($_ !~ /\./) {
15 $npflag = 1;
16 }
17 }
18
19 if (! $npflag) {
20 print "We have found prime : $q\n";
21 sleep 1;
22 }
23 }

Lets forget about the unused variables and that kind of stuff.

a) What you print out in line 6 isn't correct, you start with 4,
and not with 3 as you claim.
b) Including even numbers in checks for primes is a strange idea,
they are simple to avoid and are never prime (except 2;-)
c) Starting with $m set to 1 in line 10 and then checking in
line 11 that $m is not 1 is very strange, to say the least.
d) The same holds for going up to $q and then checking that $m
isn't $q.
e) Going up to $q instead of it's square root shows that you
should do a bit more research on prime numbers before you
atart writing programs to find primes.
f) The double quotes in line 13 indicate that you don't well
understand really what they are used for in Perl.
g) Using '$_' as a variable looks to me as bad as you can get.
h) Using a regex to check if a number can be divided by another
one is, to say the least, strange ("inefficent" doesn't cover
it appropriately).

And then, even if you remove the call to sleep(), it's slow as
molasses. Take the following program, that does logically more
or less the same as yours (using the same rather uneducated al-
gorithm):

#!/usr/bin/perl

use strict;
use warnings;

print "Prime number generator starting at 3 and going up....\n";

NOT_A_PRIME:
for ( my $q = 3; $q <= 10000; $q += 2 ) {
for ( my $m = 3; $m <= sqrt( $q ); $m += 2 ) {
next NOT_A_PRIME unless $q % $m;
}

print "We have found prime : $q\n";
}

It's about a factor of 2000 faster (sorry, that's only a very
rough guess - the updated version seems to spent most CPU time
on getting Perl to run and interpret it instead of doing any
real computations), and that with just a very few changes, using
a tiny bit of knowledge about prime numbers. What you may take
from this is: Look for a good algorithm (the one in my somewhat
faster version is definitely not a good one, already the ancient
Greeks, more than 2000 years ago, had a better one, look up the
"Sieve of Eratosthenes")... But even with a bad algorithm you
often can get much better performance if you think about it just
a tiny bit.

Concerning your "text editor": You didn't write a text editor.
You just use Perl TK widgets for all the hard work, so you're
using code that others have written that allows to input text,
select files etc. and there's nothing original you have done
there. Saying "I wrote a text editor" is, in this case, like
claiming "I build a car" because you just managed to start the
engine of a car someone gave you the keys to. And then it's not
even done very well, you could easily cut it down to nearly half
the code size (and thus make it easier to maintain) by not repea-
ting the nearly identical code in the functions savetop() and
savebottom(). Perhaps you should try to figure out how to pass
arguments to Perl Tk callback functions - it's actually not
that complicated...

I guess that you're rather young (from your code I would assume
that, while you're clever, you're not much more than about 14
year old) and, if that's the case, don't despair just because of
a bit of criticism. It simply takes time to learn things. Others
have already spent 10 or 20 years longer on it and you would have
to be an absolute genius to be better at such a very youg age.

Regards, Jens
 
R

Ralph Malph

Jens said:
I unfortunately have to agree with others here that didn't
like your code - but I only looked at the program for gene-
rating prime numbers and the "text editor" yet. And both are
not winners... Here's the one for generating prime numbers:

1 #!/usr/bin/perl
2 use strict;
3 my $npflag = 0;
4 my $n;
5 my @z;
6 print "Prime number generator starting at 3 and going up....\n";
7
8 foreach my $q (4 .. 10000) {
9 $npflag = 0;
10 foreach my $m (1 .. $q) {
11 next if $m == 1;
12 next if $m == $q;
13 $_ = "$q" / "$m";
14 if ($_ !~ /\./) {
15 $npflag = 1;
16 }
17 }
18
19 if (! $npflag) {
20 print "We have found prime : $q\n";
21 sleep 1;
22 }
23 }

Lets forget about the unused variables and that kind of stuff.

a) What you print out in line 6 isn't correct, you start with 4,
and not with 3 as you claim.
b) Including even numbers in checks for primes is a strange idea,
they are simple to avoid and are never prime (except 2;-)
c) Starting with $m set to 1 in line 10 and then checking in
line 11 that $m is not 1 is very strange, to say the least.
d) The same holds for going up to $q and then checking that $m
isn't $q.
e) Going up to $q instead of it's square root shows that you
should do a bit more research on prime numbers before you
atart writing programs to find primes.
f) The double quotes in line 13 indicate that you don't well
understand really what they are used for in Perl.
g) Using '$_' as a variable looks to me as bad as you can get.
h) Using a regex to check if a number can be divided by another
one is, to say the least, strange ("inefficent" doesn't cover
it appropriately).

And then, even if you remove the call to sleep(), it's slow as
molasses. Take the following program, that does logically more
or less the same as yours (using the same rather uneducated al-
gorithm):

#!/usr/bin/perl

use strict;
use warnings;

print "Prime number generator starting at 3 and going up....\n";

NOT_A_PRIME:
for ( my $q = 3; $q <= 10000; $q += 2 ) {
for ( my $m = 3; $m <= sqrt( $q ); $m += 2 ) {
next NOT_A_PRIME unless $q % $m;
}

print "We have found prime : $q\n";
}
Why didn't you do this

for ( my $q = 3; $q <= 10000; $q += 2 ) {
my $limit = sqrt($q);
for ( my $m = 3; $m <= $limit; $m += 2 ) {
next NOT_A_PRIME unless $q % $m;
}

and you save 52187 unnecessary calls to sqrt in
this example alone!
Why haven't the group regulars derided you
for "horrible" coding practices?

The truth is the writing code is hard.
Falsely claiming yourself as an expert
is easy.
 
S

sln

Jens said:
Robin said:
check out some of my perl programs,
http://offlame.memebot.com
let me know if you like them.... :)

I unfortunately have to agree with others here that didn't
like your code - but I only looked at the program for gene-
rating prime numbers and the "text editor" yet. And both are
not winners... Here's the one for generating prime numbers:
[snip]
Lets forget about the unused variables and that kind of stuff.
[snip]
And then, even if you remove the call to sleep(), it's slow as
molasses. Take the following program, that does logically more
or less the same as yours (using the same rather uneducated al-
gorithm):

#!/usr/bin/perl

use strict;
use warnings;

print "Prime number generator starting at 3 and going up....\n";

NOT_A_PRIME:
for ( my $q = 3; $q <= 10000; $q += 2 ) {
for ( my $m = 3; $m <= sqrt( $q ); $m += 2 ) {
next NOT_A_PRIME unless $q % $m;
}

print "We have found prime : $q\n";
}
Why didn't you do this

for ( my $q = 3; $q <= 10000; $q += 2 ) {
my $limit = sqrt($q);
for ( my $m = 3; $m <= $limit; $m += 2 ) {
next NOT_A_PRIME unless $q % $m;
}

and you save 52187 unnecessary calls to sqrt in
this example alone!
[snip]

my ($q, $m,$limit);
for ( $q = 3; $q <= 10000; $q += 2 ) {
$limit = sqrt($q);
for ( $m = 3; $m <= $limit; $m += 2 ) {
next NOT_A_PRIME unless $q % $m;
}

and 52187 + 5000 unnecessary initializations.

-sln
 
R

Ralph Malph

my ($q, $m,$limit);
for ( $q = 3; $q <= 10000; $q += 2 ) {
$limit = sqrt($q);
for ( $m = 3; $m <= $limit; $m += 2 ) {
next NOT_A_PRIME unless $q % $m;
}

and 52187 + 5000 unnecessary initializations.
ha! nice catch.
 
J

Jürgen Exner

Ralph Malph said:
No, the OP needs to stop making some silly mistakes
but he is clearly a quite capable programmer.

Sorry, but explicitly converting numbers into strings only to
immediately applying a numerical operation on those strings thus
converting them back into numbers is not a silly mistake but a very
major lack of understanding of basic fundamentals.
There would only one way to make it even worse: using this misguided
technique on floating point numbers.

jue
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,769
Messages
2,569,580
Members
45,054
Latest member
TrimKetoBoost

Latest Threads

Top