there are no rows that have blank info

How have you verified this?

Here is redraft of the code with some of what morgon was discussing and what I'd call better practices (declare vars as used, RaiseError, lowercase var names, etc). Note that qq is the same as ". So qq~fatalsToBrowser~ is just a confusing way to write "fatalsToBrowser" (or 'fatalsToBrowser' since there is no interpolation). A more common operator for export lists is qw which will apply quotes to a "list."

use warnings;
use strict;
use DBI;
use CGI::Carp qw( fatalsToBrowser );
use CGI qw( header );
use HTML::Entities qw( encode_entities ); # Updated and below.

my $attr = { RaiseError => 1 };

my $statement = <<"";
      SELECT top 1 Quote, CharacterName, GameTitle
        FROM dbo.GameQuotes
    ORDER BY +newid()

my $dbh = DBI->connect('dbi:ODBC:SQLServer', '', '', $attr)
    or die "$DBI::errstr";

my $sth = $dbh->prepare($statement);

$sth->execute;

my ( $quote, $character, $game ) = 
    map { encode_entities($_) }
    $sth->fetchrow_array;

print header();

print <<"";
<blockquote>
   <p>$quote</p>
   <cite>&ndash;$character <br />
      $game</cite>
<blockquote>

$dbh->disconnect;
[download]

My notes:

• I would move the connect attributes into DBI->connect() instead of using a constant variable.
• I would not use an empty marker in a here-doc: It is too easy to accidentally remove the empty line, and it is not obvious WHY you need that empty line. Use __END_OF_SQL__ if you don't find a marker with a better name.
• I would move the SQL statement into $dbh->prepare instead of using a constant variable. Using a variable only makes sense if you want to modify the initial statement before passing it to the database.
• You write data fetched from the database into an HTML document without properly escaping the data. The CGI module has the escapeHTML method for exactly this purpose.
• A CGI program is exposed to the network, maybe even the internet. But I don't see you enabling the taint mode. Start the script with #!/usr/bin/perl -T to reduce the risk of using input without proper validation.
• Enable taint checks in DBI for the same purpose: Add Taint => 1 to the connect attributes.
• fatalsToBrowser exposes information about the script that you don't want to give out to an attacker. It is fine for debugging, but evil for prodduction servers. Remove it or enable it only on debugging servers: use CGI::Carp; BEGIN { CGI::Carp->import(fatalsToBrowser) if $DEBUG; }

Alexander

--
Today I will gladly share my knowledge and experience, for there are no sweeter words than "I told you so". ;-)

Good advice. But not for me. :) This was a style/clarity update based on the original code, largely to just get RaiseError in there, not the way I would write it from scratch. HTML escaping is almost always a good idea (unless your data is stored that way) and I meant to put that in; I would use HTML::Entities instead so when code is taken out of CGI.pm, as it eventually would be in a perfect world, that's one less line to edit. I'll amend it. Empty (\n) HEREDOC markers became a little vogue in... CDBI? I think? And I prefer it. I've always found them less jarring than starting something by calling it the _END.

And I find the order by clause a bit strange: Why do you add backets to the newid - is a SQL-server speciality?

And by the way: Why do you not use the SQL in $Statment but rather repeat it again?

And is there anything wrong in using lower-case variable names and normal quotation marks just like everyone else does?

And I find the order by clause a bit strange: Why do you add backets to the newid - is a SQL-server speciality? That is just the way the SQL command is used.
And by the way: Why do you not use the SQL in $Statment but rather repeat it again? Old line of code form a test I was doing, thank for poiting it out I have removed it.
And is there anything wrong in using lower-case variable names and normal quotation marks just like everyone else does?My coding style is to make the first letter of all words in a varible uper-case, just an old habbit. Not sure what you mean by "normal quotation marks"
Probably not helpful but you could add a check to verify that the fetching does not fail (check @Quote for defined-ness).I could add a check to see if the array gets filled or not but what would I do if it did not? The point of this code is to grab a random quote from the DB and display it so I need to figure out why a blank quote is being grabed.

Just for curiosity:

Is there any difference (in SQL server) between

select top 1 Quote, CharacterName, GameTitle from dbo.GameQuotes order
+ by newid()
[download]

and

select top 1 Quote, CharacterName, GameTitle from dbo.GameQuotes order
+ by newid
[download]

i.e. without the parentheses at the end of newid?

select top 10 
newid() as randomrow, productid, productname 
from Products
order by randomrow
[download]

Try changing your SQL-code to:

select top 1 
Quote, CharacterName, GameTitle, newid() as randomrow  
from dbo.GameQuotes 
order by randomrow
[download]

Update: Of course the above "solution" scales very badly; so in general do not use it if your database contains many records. It "works" by giving all your records a random GUID (itself a rather expensive function), then doing a sort on this value which is NOT indexed and therefore very slow and finally throwing everything away but the first record.

Much better is to add an extra field in your database which has a sequence-number from 1 to whatever number of records you have. Most databases can do this automatically. Then just generate a random integer in the range 1 to number of records and SELECT the record with that sequence number. Much, much faster and less strain on your database server.

I think the randomrow column is not needed. Just order by random() (or order by rand() for MS SQL Server).

Alexander

--
Today I will gladly share my knowledge and experience, for there are no sweeter words than "I told you so". ;-)

I'm not sure for SQL Server 2008, but earlier versions of SQL-server would return the same value for rand() for all records within one select-statement. Other types of database, recalculate rand() for every record, as one would expect.

CountZero

A program should be light and agile, its subroutines connected like a string of pearls. The spirit and intent of the program should be retained throughout. There should be neither too little or too much, neither needless loops nor useless variables, neither lack of structure nor overwhelming rigidity." - The Tao of Programming, 4.1 - Geoffrey James

It occurred to me that if you have a quote which starts something like <The best games require... that maybe the HTML is burying it because of broken tags. I didn't think it was likely enough to mention though. The HTML escaping will expose/fix it if it's the case. I think it's more likely you do have empty/squirrelly data and you should make sure to do a totally braindead check like-

select Quote from dbo.GameQuotes

-and eyeball all of them.