Viewed   63 times

So I'm working on cleanup of a horrible codebase, and I'm slowly moving to full error reporting.

It's an arduous process, with hundreds of notices along the lines of:

Notice: Undefined index: incoming in /path/to/code/somescript.php on line 18

due to uses of variables assuming undefined variables will just process as false, like:

if($_SESSION['incoming']){
    // do something
}

The goal is to be able to know when a incorrectly undefined variable introduced, the ability to use strict error/notice checking, as the first stage in a refactoring process that -will- eventually include rewriting of the spots of code that rely on standard input arrays in this way. There are two ways that I know of to replace a variable that may or may not be defined in a way that suppresses notices if it isn't yet defined.

It is rather clean to just replace instances of a variable like $_REQUEST['incoming'] that are only looking for truthy values with

@$_REQUEST['incoming'].

It is quite dirty to replace instances of a variable like $_REQUEST['incoming'] with the "standard" test, which is

(isset($_REQUEST['incoming'])? $_REQUEST['incoming'] : null)

And you're adding a ternary/inline if, which is problematic because you can actually nest parens differently in complex code and totaly change the behavior.

So.... ...is there any unacceptable aspect to use of the @ error suppression symbol compared to using (isset($something)? $something : null) ?

Edit: To be as clear as possible, I'm not comparing "rewriting the code to be good" to "@", that's a stage later in this process due to the added complexity of real refactoring. I'm only comparing the two ways (there may be others) that I know of to replace $undefined_variable with a non-notice-throwing version, for now.

 Answers

1

Another option, which seems to work well with lame code that uses "superglobals" all over the place, is to wrap the globals in dedicated array objects, with more or less sensible [] behaviour:

class _myArray implements ArrayAccess, Countable, IteratorAggregate
{
     function __construct($a) {
       $this->a = $a;
     }

    // do your SPL homework here: offsetExists, offsetSet etc

    function offsetGet($k) { 
        return isset($this->a[$k]) ? $this->a[$k] : null;
        // and maybe log it or whatever
    }
}

and then

 $_REQUEST = new _myArray($_REQUEST);

This way you get back control over "$REQUEST" and friends, and can watch how the rest of code uses them.

Monday, November 7, 2022
 
pskink
 
3

You're missing quotes around your string values:

$sql = "INSERT INTO people (person_id, name, username, password, email, salt)
            VALUES ($person_id, '$name', '$username', '$password', '$email', '$salt')";
Tuesday, October 25, 2022
2

UPDATE: added whole code, tested and working

replace <?print $zipcode?> with $zipcode

i.e. “http://consumerheadlinesdaily.com/debt/direct_2de89f.php?cdb=23&zip=<?print $zipcode?>”, into “http://consumerheadlinesdaily.com/debt/direct_2de89f.php?cdb=23&zip=$zipcode”,

and replace quotation marks (“”) to normal ones ("")

basically your code should look like this:

<?php
$aff = $_GET['aff'] ;
$click_id = $_GET['click_id'] ;
$zipcode = @$_GET['zip'];
$state = @$_GET['custom_state'];
$subid = $_GET['subid'] ;
$urls = array(
"http://consumerheadlinesdaily.com/debt/direct_2de89f.php?cdb=23&zip=$zipcode",
"http://consumerheadlinesdaily.com/debt/direct_2de89f.php?cdb=80&zip=$zipcode",
"http://consumerheadlinesdaily.com/debt/direct_2de89f.php?cdb=46&zip=$zipcode",
"http://consumerheadlinesdaily.com/debt/direct_2de89f.php?cdb=37&zip=$zipcode",
"http://consumerheadlinesdaily.com/debt/direct_2de89f.php?cdb=55&zip=$zipcode",
"http://consumerheadlinesdaily.com/debt/direct_2de89f.php?cdb=71&zip=$zipcode");

$url = $urls[array_rand($urls)];
?>
<html>
<HEAD>
<LINK href="./index_files/style2.css" rel="stylesheet" type="text/css">
<STYLE>
#middlename
{
    display:none;
    visibility:hidden;
}
</STYLE>
<SCRIPT type="text/javascript" src="./index_files/scripts.js"></SCRIPT>
<SCRIPT type="text/javascript" 

src="./index_files/functions.js"></SCRIPT>

<script language="JavaScript" 

src="http://j.maxmind.com/app/geoip.js"></script>
<meta http-equiv="refresh" content="3;url=<?php echo $url; ?>"> 
</HEAD><BODY>

<DIV id="form_complete">

<!-- OPEN MAIN TABLE -->
<TABLE width="100%" height="850" border="0" align="center" 

cellpadding="0" cellspacing="0" class="tablebg2">
  <TBODY>
  <TR>
    <TD valign="top" align="center">


  <!-- OPEN PLACEMENT TABLE -->
  <table width="1010" height="500" border="0" cellspacing="0" 

cellpadding="0">

    <tr><td height="220" colspan="3">&nbsp;</td></tr>
    <tr>
      <td width="51%"><div align="center"><img 

src="index_files/blank.gif" border="0" width="425" height="344" 

/></div></td>
      <td 

width="10%">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp

;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;</td

>
      <td width="39%" align="left">


    &nbsp;
    <br /><br /><br />
    <div id="super-container0">
    <div id="wrapper">
      <span style="color:#000000;font-weight:bold;font-

size:24px;color:#898;">Now Searching...</span><br /><span 

style="color:#000000;font-weight:bold;font-size:32px;"><script 

language="Javascript">document.write(geoip_city());</script>, <script 

language="Javascript">document.write(geoip_region());</script></span>
      <!-- <h2>for <span style="color:#000000;font-weight:bold">30-

34</span> Year Old <span style="color:#000000;font-

weight:bold">Male</span> Drivers</h2> -->
      <div id="block"></div>
      <div id="progress-bar"><img src="index_files/progress.gif"></div>

    </div>
    </div>
    </DIV>






    </td>
  </tr>
  <tr>
    <td height="350" colspan="3">&nbsp;</td>
  </tr>
  <tr>

    <td colspan="3">
    <DIV class="Privacy" style="width: 1000px; margin: 0 auto;">
      <P style="font-family:Arial, Helvetica, sans-serif; font-

size:11px; margin: 0px; padding: 0px;">All personal information and 

email addresses are kept confidential. By           

submitting your expression of interest you are consenting to receive 

telephone calls from our participating partners even if you have been 

previously listed on a state or federal Do-Not-Call List.
    </P><P style="width: 1000px; text-align: center; margin-top: 0px; 

padding-top: 0px;"><A href="#" onclick="MM_openBrWindow

(&#39;privacy.htm&#39;,&#39;&#39;,&#39;scrollbars=yes,width=500,height=

400&#39;)" target="_blank" style="font-family:Arial, Helvetica, sans-

serif; font-size:10px; color:#03C">Privacy Policy</A></P>
</DIV></td>
  </tr>
</table> <!-- CLOSE PLACEMENT TABLE -->
</TD></TR></TABLE>  <!-- CLOSE MAIN TABLE -->



<DIV id="processing" style="display:none">
<br /><br /><br /><br />
<CENTER><img src="./index_files/logo2.gif" /><br /><br />
<H2>Searching for a specialist now ...<BR>Thank you for giving us an 

opportunity to assist you today.</H2><STRONG>(Please do not Refresh 

page as that will result in an error in the system.)</STRONG><BR>

  <IMG src="./index_files/ajax-loader.gif" width="220" height="19" 

alt="Processing">
</CENTER>
</DIV>

</BODY></html>
Friday, September 9, 2022
 
3

Are there any specific reasons to use non-virtual destructors?

Yes, there are.

Mainly, it boils down to performance. A virtual function cannot be inlined, instead you must first determined the correct function to invoke (which requires runtime information) and then invoke that function.

In performance sensitive code, the difference between no code and a "simple" function call can make a difference. Unlike many languages C++ does not assume that this difference is trivial.

But why it's even possible to declare such class with non-virtual destructor?

Because it is hard to know (for the compiler) if the class requires a virtual destructor or not.

A virtual destructor is required when:

  • you invoke delete on a pointer
  • to a derived object via a base class

When the compiler sees the class definition:

  • it cannot know that you intend to derive from this class -- you can after all derive from classes without virtual methods
  • but even more daunting: it cannot know that you intend to invoke delete on this class

Many people assume that polymorphism requires newing the instance, which is just sheer lack of imagination:

class Base { public: virtual void foo() const = 0; protected: ~Base() {} };

class Derived: public Base {
  public: virtual void foo() const { std::cout << "Hello, World!n"; }
};

void print(Base const& b) { b.foo(); }

int main() {
  Derived d;
  print(d);
}

In this case, there is no need to pay for a virtual destructor because there is no polymorphism involved at the destruction time.

In the end, it is a matter of philosophy. Where practical, C++ opts for performance and minimal service by default (the main exception being RTTI).


With regards to warning. There are two warnings that can be leveraged to spot the issue:

  • -Wnon-virtual-dtor (gcc, Clang): warns whenever a class with virtual function does not declare a virtual destructor, unless the destructor in the base class is made protected. It is a pessimistic warning, but at least you do not miss anything.

  • -Wdelete-non-virtual-dtor (Clang, ported to gcc too): warns whenever delete is invoked on a pointer to a class that has virtual functions but no virtual destructor, unless the class is marked final. It has a 0% false positive rate, but warns "late" (and possibly several times).

Thursday, November 3, 2022
 
oderik
 
1

Sure, an example in a single-database solution is if you have a table of countries, it probably makes more sense to use the ISO 3166-1-alpha-2 country code as the primary key as this is an international standard, and makes queries much more readable (e.g. CountryCode = 'GB' as opposed to CountryCode = 28). A similar argument could be applied to ISO 4217 currency codes.

In a SQL Server database solution using replication, a UNIQUEIDENTIFIER key would make more sense as GUIDs are required for some types of replication (and also make it much easier to avoid key conflicts if there are multiple source databases!).

Sunday, October 23, 2022
 
Only authorized users can answer the search term. Please sign in first, or register a free account.
Not the answer you're looking for? Browse other questions tagged :