Beefy Boxes and Bandwidth Generously Provided by pair Networks
We don't bite newbies here... much

Re: Critique

by pg (Canon)
on Dec 06, 2003 at 02:45 UTC ( #312715=note: print w/replies, xml ) Need Help??

in reply to Critique

"it just seems long and not very neat"

I feel the opposite. Your code is neat and I like it. The logic is clear, easy to understand and easy to maintain. You have a good coding style.

Is it too long? No. You don't predetermine the size of your code. As long as it does the right thing in the right way, whatever it is long or short, that is the right size.

Although it is just a piece of demo code, I somehow would like it to be able to handle leap year:

sub get_days { if($curr_mon =~ /$three_one/) { return 31; } elsif($curr_mon =~ /$three_zero/) { return 30; } elsif($curr_mon == 2) { if (is_leap_year()) { return 29; } else { return 28; } } else { die "Bad month entered!\n"; } } sub is_leap_year { my (undef, undef, undef, undef,undef, $year) = localtime(time); $year += 1900; return 1 if ($year % 400 == 0); return 0 if ($year % 100 == 0); return 1 if ($year % 4 == 0); return 0; }

Replies are listed 'Best First'.
Re: Re: Critique
by phenom (Chaplain) on Dec 06, 2003 at 03:24 UTC
    Thank you very much, pg. There were two extra things I was trying to use here for learning purposes: hashes and the lines pertaining to the regex under get_days. Hashes, since I understand the basics about them, but not more advanced stuff (like the array of hashes, hashes of hashes, blah blah blah); and also, I had just seen someone here recently use the following:
    my $blah = join('|', @array);
    to produce the automatic "or" effect. Thanks again!!
      Is the code in your note a copy of the actual code you ran? The reason I ask is that your month hash is misnumbered. The hash has two month 8's.



      Perl has one Great Advantage and one Great Disadvantage:

      It is very easy to write a complex and powerful program in three lines of code.


      The Needs of the World and my Talents run parallel to infinity.
        No, it wasn't. I printed it from the original system and retyped it... Should have proof read it.
Re: Re: Critique
by ihb (Deacon) on Dec 06, 2003 at 10:59 UTC

    You hit the head of the nail when you pointed out that it doesn't handle leap years. This is my only real problem with the code: it having its own date logic.

      Since it was only supposed to deal with 2004, I never thought to include leap year functionality. But since some of the responses have given me some ways of doing it, I'm going to re-write it now. That's a good thing - I'm learning a lot from all these responses.
Re: Re: Critique
by ysth (Canon) on Dec 08, 2003 at 00:19 UTC
    If we get rid of the clunky way too long variable names and use simple $m and $y, we can have:
    sub get_days{$m-2?30+($m*3%7<4):28+!($y%4||$y%400*!($y%100))}
    And if we ask really nicely, bart may write it in 68k assembler for us :)
Re: Re: Critique
by Paulster2 (Priest) on Dec 07, 2003 at 14:09 UTC

    While this works out very well, Date::Manip can handle it without putting in extra code. The perldoc on this goes to great lengths to explain everything about itself, also. It is a very well written and documented module, highly worth the time to check it out. I recommend it for all of your date manipulation needs!

    UPDATE: I am refering to the Re: Critique written by pg above. Sorry about that!


Log In?

What's my password?
Create A New User
Node Status?
node history
Node Type: note [id://312715]
[Tux]: but I <3 the way people test here
[Tanktalus]: Tux: use sort -V to sort your perls. Looks much nicer that way ;)
[Tux]: Hmmm, pm-cb-g does not seem to accept dead-key diacriticals as all my other (perl) applications do. choroba?
[Tux]: Tanktalus this was the output of autocompletion in my tcsh
[erix]: ah, but I have different compilation- versions of Pg too, so double that: 26 * 2 = more than 50 :P
[Tanktalus]: When I showed one of my coworkers my perl list, he told me that my sort routine was broken when 5.8.8 showed up after 5.26.0 ... which made me investigate sort, which is when I found this option :)
erix atm running billion row table replication (pg-)tests...
[Tanktalus]: Tux: ah, still... :)
Tux writes mail to tcsh developers ...
Tanktalus snickers :)

How do I use this? | Other CB clients
Other Users?
Others taking refuge in the Monastery: (8)
As of 2017-09-25 16:47 GMT
Find Nodes?
    Voting Booth?
    During the recent solar eclipse, I:

    Results (284 votes). Check out past polls.