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

Writing a perl quiz. Need advice.

by DigitalKitty (Parson)
on Mar 08, 2003 at 05:34 UTC ( [id://241340]=perlquestion: print w/replies, xml ) Need Help??

DigitalKitty has asked for the wisdom of the Perl Monks concerning the following question:

Hi all.

This is a homework project. I have already finished it but I was wondering if any of you had an opinion regarding where I could improve something? The amount of work I have put into this is trvial by programming standards. Never-the-less, I intensely dislike submitting assignments I know could have been improved upon.

Thanks in advance,

-Katie.
#!/usr/bin/perl -wT use strict; use CGI qw( :standard ); use CGI::Carp qw( fatalsToBrowser ); print "Content-type: text/html\n\n"; my ( $question_one, $question_two, $question_three, $question_four, $question_five, $question_six, $question_seven, $question_eight, $question_nine, $question_ten ); my $total = 10; my $score; my $final_score; $question_one = param( "Q1" ); $question_two = param( "Q2" ); $question_three = param( "Q3" ); $question_four = param( "Q4" ); $question_five = param( "Q5" ); $question_six = param( "Q6" ); $question_seven = param( "Q7" ); $question_eight = param( "Q8" ); $question_nine = param( "Q9" ); $question_ten = param( "Q10" ); if( $question_one eq "Larry Wall" ) { print "1). You answered: $question_one <FONT COLOR=\"Green\">Correct +!</FONT><BR>"; $score++; } else { print "1). You answered: $question_one <FONT COLOR=\"RED\">Incorrect +</FONT><BR>"; } if( $question_two eq "Unix administrators" ) { print "2). You answered: $question_two <FONT COLOR=\"Green\">Correct +!</FONT><BR>"; $score++; } else { print "2). You answered: $question_two <FONT COLOR=\"RED\">Incorrect +</FONT><BR>"; } if( $question_three eq "Text processing" ) { print "3). You answered: $question_three <FONT COLOR=\"Green\">Corre +ct!</FONT><BR>"; $score++; } else { print "3). You answered: $question_three <FONT COLOR=\"RED\">Incorre +ct</FONT><BR>"; } if( $question_four eq "Glue" ) { print "4). You answered: $question_four <FONT COLOR=\"Green\">Correc +t!</FONT><BR>"; $score++; } else { print "4). You answered: $question_four <FONT COLOR=\"RED\">Incorrec +t</FONT><BR>"; } if( $question_five eq "Operating system utilities and services" ) { print "5). You answered: $question_five <FONT COLOR=\"Green\">Correc +t!</FONT><BR>"; $score++; } else { print "5). You answered: $question_five <FONT COLOR=\"RED\">Incorrec +t</FONT><BR>"; } if( $question_six eq "True" ) { print "6). You answered: $question_six <FONT COLOR=\"Green\">Correct +!</FONT><BR>"; $score++; } else { print "6). You answered: $question_six <FONT COLOR=\"RED\">Incorrect +</FONT><BR>"; } if( $question_seven eq "Regular expressions" ) { print "7). You answered: $question_seven <FONT COLOR=\"Green\">Corre +ct!</FONT><BR>"; $score++; } else { print "7). You answered: $question_seven <FONT COLOR=\"RED\">Incorre +ct</FONT><BR>"; } if( $question_eight eq "Open source" ) { print "8). You answered: $question_eight <FONT COLOR=\"Green\">Corre +ct!</FONT><BR>"; $score++; } else { print "8). You answered: $question_eight <FONT COLOR=\"RED\">Incorre +ct</FONT><BR>"; } if( $question_nine eq "TCL" ) { print "9). You answered: $question_nine <FONT COLOR=\"Green\">Correc +t!</FONT><BR>"; $score++; } else { print "9). You answered: $question_nine <FONT COLOR=\"RED\">Incorrec +t</FONT><BR>"; } if( $question_ten eq "Marc Andreessen" ) { print "10). You answered: $question_ten <FONT COLOR=\"Green\">Correc +t!</FONT><BR>"; $score++; } else { print "10). You answered: $question_ten <FONT COLOR=\"RED\">Incorrec +t</FONT><BR>"; } $final_score = ( $score * 10 ); print <<EOF; <TABLE> <TR><TD>Total questions: $total</TD><TD>Total answered correctly: $sco +re</TD><TD>Your final score: $final_score\%</TD></TR> </TABLE> EOF

edited: Sun Mar 9 15:00:05 2003 by jeffa - added readmore tags

Replies are listed 'Best First'.
Re: Writing a perl quiz. Need advice.
by zengargoyle (Deacon) on Mar 08, 2003 at 05:55 UTC

    one thing, you're not lazy enough.

    # untested use strict; my %questions = ( 1 => { question => "Who's Perl's top dog?", answer => "Larry Wall" }, 2 => { question => "What's the answer?", answer => "42" }, ); foreach my $question (sort {$a <=> $b} keys %questions) { my $guess = param("$Q$question"); print "$question). You answered: $questions{$question}{question} wit +h $guess. "; if ($guess eq $questions{$question}{answer}) { print q|<font color="green">Correct</font><br>|; } else { print q|<font color="red">Incorrect</font><br>|; } } # keeping totals left as homework =)
•Re: Writing a perl quiz. Need advice.
by merlyn (Sage) on Mar 08, 2003 at 15:37 UTC
    Immediate reactions (which probably echo that of others):
    • Nearly any time you have many variables that are similarly named, you should put those into a structure. In this case, an array would work nicely.
    • Nearly any time you have code that repeats, you should factor it out to be a subroutine. Although, in this case making the data an array would probably have sufficed to reduce the code to one copy.
    • Remove fatalsToBrowser in production code. It reveals far too much to precisely the person who can do the least about it. I have an example of "fatals to email" in one of my columns.

    -- Randal L. Schwartz, Perl hacker
    Be sure to read my standard disclaimer if this is a reply.

Re: Writing a perl quiz. Need advice.
by dws (Chancellor) on Mar 08, 2003 at 05:52 UTC
    I was wondering if any of you had an opinion regarding where I could improve something?

    If you're gathering input from text fields (and not menus), don't do case-dependent comparisons. Rejecting "Tcl" because the quiz is hard-coded to expect "TCL" is cruel.

Re: Writing a perl quiz. Need advice.
by Zaxo (Archbishop) on Mar 08, 2003 at 06:10 UTC

    Gee, makes me want to see the questions :-)

    I tend to write something like this with the form generation and the cgi receipt in the same file. That makes for closer coupling between the questions and the answers. If you go for a template solution, that changes the script organization, but the coupling between questions and answers should be preserved.

    I'd avoid the intermediate variables if I could. Direct evaluation of the cgi params will not cause any problems in comparison to a literal string.

    The similarity of your if () {} else {} clauses suggests that sub, printf, and ?: operator can simplify things.

    After Compline,
    Zaxo

Re: Writing a perl quiz. Need advice.
by simon.proctor (Vicar) on Mar 08, 2003 at 12:19 UTC
    I notice that you load the CGI module and then print the content header by hand. I would have thought it would have been best to use the module code?

    You've also got fatalsToBrowser in there - however once your program is done and dusted I would have thought it best to remove it?

      Update: Don't keep fatalsToBrowser in your production script. See postings below for reasons.

      You've also got fatalsToBrowser in there - however once your program is done and dusted I would have thought it best to remove it?

      I disagree. In the same line of thought, you could propose to remove use strict/use warnings after your program is thoroughly tested. This is somewhat like asking your children to forget their good education once they are grown up :-).

      The only reason I can come up with to remove any of the three is to to improve speed, and even that one is not sufficient: I challenge you to prove that the speed gain is more than marginal.

      My experience shows that for many scripts I "finished" at a certain time, there is another time I have to go back and make some changes. Software is not like a statue that you once create and never change. But if you are going to change it, you better leave all the security mechanisms in place, or else you'll shoot yourself in the foot.

      Even software that you are "sure" you will never modify might fail mysteriously due to not-yet uncovered bugs, a section of code you never really tested, an error condition that never ocurred before etc. For this, it's good to have FatalsToBrowser.

      Apart from that, I like to treat every piece of software I write seriously. It helps me to acquire good coding habits, and it tends to save me a lot of work later.

        I disagree with your disagreement :-)

        Fatals to browser can reveal information that you do not want the users to see. The error message can potentially reveal information about your code that would allow a malicious user to exploit the bug that caused the error in nasty ways.

        In production code you log the errors, but don't show them to use user.

        The reason to remove fatalsToBrowser is that you know something about security and you are not an idiot. Let me be clear. I am not saying that you are an idiot. I am saying that you have something basic to learn about security. Please learn it from my post and then stop giving dangerously bad advice.

        The purpose of fatalsToBrowser is to provide useful debugging information in the browser. This speeds up development. However it means that if someone finds "unexpected behaviour" in your application, they can use your debugging information to get insight into how your application works, and then use that to fine-tune an attack.

        For a common example, suppose that you interface with a database. And, as happens depressingly often, you don't quote a field that shows up in an SQL query. You now suffer from the possibility of an SQL injection attack - someone can enter in anything they want and have it become part of your SQL. If they can figure out the right thing, they can get "interesting results". (Such as downloading your credit card data. Or taking over your database server.)

        Now obviously this is much easier if they can find out something about the structure of the query that they are breaking. And something about your database. Which is far, far easier if you provide useful debugging information in the browser. See, for instance, http://www.sensepost.com/misc/SQLinsertion.htm (or many other pages on SQL injection at Google) for how an attacker can use debugging information to keep on escalating an attack until they can get anything they want from your database.

        Not publically providing debugging information doesn't fix the bug, but it does make it a lot harder to exploit. It often makes the difference between a minor intrusion and a serious problem.

        So how do you do this with minimal impact on your ability to debug? The answer is simple. You make fatalsToBrowser be something that is readily flipped in your code. Preferably either in a configuration variable that differs between production and development, or as something that your standard build process does. That way you get all of the benefits while developing or whenever you need it in production, but without making an attacker's life any easier than it has to be.

Re: Writing a perl quiz. Need advice.
by Anonymous Monk on Mar 08, 2003 at 14:37 UTC
Re: Writing a perl quiz. Need advice.
by Anonymous Monk on Mar 08, 2003 at 15:11 UTC
    Here's a some comments/opinions I had which might help you
    #!/usr/bin/perl -wT use strict; use CGI qw( :standard ); use CGI::Carp qw( fatalsToBrowser ); # use CGI # print "Content-type: text/html\n\n"; print header,start_html(); # When I read this I assumed these variables would be # the text of the question but they are the text # of the answers, but as you see later you don't need them. =head1 my ( $question_one, $question_two, $question_three, $question_four, $question_five, $question_six, $question_seven, $question_eight, $question_nine, $question_ten ); =cut # These you don't need yet; #my $total = 10; #my $score; #my $final_score; # If you use leading zeros in the names they sort # and loop naturally ie Q01, Q02, Q03 # You have the answers in the parameter hash so no # need to copy them =head1 $question_one = param( "Q1" ); $question_two = param( "Q2" ); $question_three = param( "Q3" ); $question_four = param( "Q4" ); $question_five = param( "Q5" ); $question_six = param( "Q6" ); $question_seven = param( "Q7" ); $question_eight = param( "Q8" ); $question_nine = param( "Q9" ); $question_ten = param( "Q10" ); =cut # The correct answers I would put in a DATA block at the end # and read them in here but there are lots of formats # you could use my %correct = (); while (<DATA>){ chomp; my $id = substr($_,0,3); my $value = substr($_,4); $correct{$id} = $value; } # It's easy now to loop around the # questions checking the answers # and updating the score my $total = 0; my $score = 0; foreach (sort keys %correct){ my $answer = param($_) || "" ; # The || gives it defaults, and + my $correct = $correct{$_} ; # using temp variables make it ea +sier to read # marking my $result; if ($answer eq $correct){ $result = qq(<font color="green">Correct</font>); ++$score; } else { # It's nice to know what the correct answer was $result = qq(<font color="red">Incorrect The Correct answer was + $correct</font>); } # use qq to avoid all those \'s print qq[$_ ). You answered : $answer $result <br>\n ]; # use [ be +cause of ) in text ++$total; } # work out the % marking to 1 decimal place my $mark = 100 * $score / $total unless $total == 0; # avoid div by z +ero error $mark = sprintf "%.1f",$mark; # use the cgi module and # since you have a table you can # use 2 columns to make it look neat print table ( Tr( td ("Total questions"), td ($total) ), Tr( td ("Correct"), td ($score) ), Tr( td ("% Score"), td ($mark) ) ); print end_html(); __DATA__ Q01 Larry Wall Q02 Unix administrators Q03 Text processing Q04 Glue Q05 Operating system utilities and services Q06 True Q07 Regular expressions Q08 Open source Q09 TCL Q10 Marc Andreessen
    poj
Re: Writing a perl quiz. Need advice.
by BUU (Prior) on Mar 08, 2003 at 05:56 UTC
    Step 1) Array of Submitted answers.
    Step 2) Array of real answers
    @ra=qw/foo baz one two/; @sub=qw/foo qux one three/; print join'<br>',map@ra[$_]eq$sub[$_]?"answered $_right":"answersed $_ + wrong!",0..@sub;
    Tracking the number wrong is left as an exercise for the reader...

      Sorry, but this is wrong.

      Ignoring the fact that you've used an array slice to get a single value from an array (@ra[$_]), which I'll assume is just a typographical error, your use of 0..@sub to iterate over the indices1 of the array won't work. @array returns the number of values in the array, so your code iterates over the arrays from indicies 0 to 4, or five elements. The arrays only have four elements each.

      To fix it, you could try something like 0 .. $#sub.

      Two other points:

      • You're missing a space in $_right.
      • You telling the user they've got question 0 correct, when most people would expect that to be question 1.

      Obfu'd code is ok, but obfu'd code that doesn't work seems a bit pointless to me

      cheers
      davis
      Is this going out live?
      No, Homer, very few cartoons are broadcast live - it's a terrible strain on the animator's wrist

      1: Is it indices or indexes? - I can never remember

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: perlquestion [id://241340]
Approved by valdez
help
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others sharing their wisdom with the Monastery: (3)
As of 2024-03-29 06:18 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found