Register a SA Forums Account here!
JOINING THE SA FORUMS WILL REMOVE THIS BIG AD, THE ANNOYING UNDERLINED ADS, AND STUPID INTERSTITIAL ADS!!!

You can: log in, read the tech support FAQ, or request your lost password. This dumb message (and those ads) will appear on every screen until you register! Get rid of this crap by registering your own SA Forums Account and joining roughly 150,000 Goons, for the one-time price of $9.95! We charge money because it costs us money per month for bills, and since we don't believe in showing ads to our users, we try to make the money back through forum registrations.
 
  • Post
  • Reply
IT BEGINS
Jan 15, 2009

I don't know how to make analogies
php:
<?
extract($vals);
$ref = $referencenotes;
if (!strstr ("XDXD", "$CUSTCODE$")) {
    $refnum      = $vals["referencenotes2"];
    $brk = "; ";
    if (strlen ($refnum))
        $ref .= (strlen ($ref) ? "$brk" :"").$refnum;

    $refnum      = $vals["referencenotes3"];
    if (strlen ($refnum))
        $ref .= (strlen ($ref) ? "$brk" :"").$refnum;

    $refnum      = $vals["custponum"];
    if (strlen ($refnum))
        $ref .= (strlen ($ref) ? "$brk" :"").$refnum;

    $refnum      = $vals["custinvnum"];
    if (strlen ($refnum))
        $ref .= (strlen ($ref) ? "$brk" :"").$refnum;

    $refnum      = $vals["custdeptnum"];
    if (strlen ($refnum))
        $ref .= (strlen ($ref) ? "$brk" :"").$refnum;

    $refnum      = $vals["department"];
    if (strlen ($refnum))
        $ref .= (strlen ($ref) ? "$brk" :"").$refnum;
}
?>
Somehow, I don't think think this was the best way to handle this. If you're wondering what $CUSTCODE$ is, seems they decided to implement a pre-processor for php. :what:

Adbot
ADBOT LOVES YOU

IT BEGINS
Jan 15, 2009

I don't know how to make analogies

nielsm posted:

What I do every day.

PHP, uniting us through horrible code and worse coders.

I just don't understand how people who have been using a language for upwards of a decade don't know about its most basic features or best practices. I end up saying things like "there are array functions now, and you probably shouldn't use globals and extract() everywhere" or "pg_result was deprecated in 2001 and removed from all available documentation in 2003, why is it still being added to our codebase in 2013?" way too often.

IT BEGINS fucked around with this message at 15:22 on Aug 27, 2013

IT BEGINS
Jan 15, 2009

I don't know how to make analogies
In a file called reportTemplateDynamicLibrary.php, formatted exactly as you see here:

What the hell does this even do?

php:
<?
function clean_driving($driving){
if(empty($driving)) return"";
$driving = trim($driving);
$driving = substr($driving,strlen($driving)-1) =="," ? substr($driving,0,-1): $driving;
$driving =str_replace(",",",\n\t",$driving);
return "$driving,\n";
}   
?>
Do you like globals? Like extract()? We've got plenty:

php:
<?
function output_line(){
global $schema, $last_invoice_row,$comma_pipe,$table_storage,$report_type,$library;
foreach($schema as $fields => $attributes){
    $lookup = strtolower($fields);
    extract($attributes);
    $value =
    (isset($last_invoice_row[$lookup]) ? $last_invoice_row[$lookup] :
            ($table ==''? $field :
                ((strstr($format,"varchar") || strstr($format,"date")) ? "" : "0.00"
                )
            )
        );
    $value = is_array($value) ? $value[$report_type] : $value;
    $output .=$value.$comma_pipe;
    }
insert_storage($output);
echo substr($output,0,-1)."\n";

    return;
}   
?>
What is a commapipe?

php:
<?
    if($comma_pipe =="|")    print_footer();
?>
php:
<?
function round1 ($string) {
return round($string,1);
}
function round2 ($string) {
return round($string,2);
}
function round3 ($string) {
setlocale(LC_MONETARY, 'en_US');

return money_format('%.3n', $string);
}
function round4 ($string) {
    setlocale(LC_MONETARY, 'en_US');

return money_format('%.4n', $string);
}
function round5 ($string) {
    setlocale(LC_MONETARY, 'en_US');
return money_format('%.5n', $string);
}
?>
:wtc:

IT BEGINS
Jan 15, 2009

I don't know how to make analogies
Just one more piece that I have the work with. Showcases everything that is wrong with my coworkers. :wtc:

php:
<?

    function checkPgStatus($db, $r, $msg) {
        global $DEBUG;

        if ($r) return 1;

        if (ini_get("display_errors")) echo "<br><B><I><font color=red>\n";
        if (strstr($msg, "NOERRORS:")) return 0;

        if (strstr($msg, "DONTEXIT:")) {
            if (ini_get("display_errors")) {
                echo str_replace("DONTEXIT:", "", $msg);
                //echo "</B></I><br></font>\n";
                echo(pg_ErrorMessage($db));
            }
            return 0;
        }

        if (1 || ini_get("display_errors")) {
            print_backtrace(debug_backtrace());
            echo "<br>$msg";
            echo(pg_ErrorMessage($db));
        }

        $footer=getenv("FT_FOOTER");
        if (strlen(@$footer)) include $footer;
        echo "SQL error occured, exitting.\n";
        exit(1);
    }
?>

IT BEGINS
Jan 15, 2009

I don't know how to make analogies

drasticactions posted:

code:
~~~~~~
Additional Code goes here.
~~~~~~

Always a sign that good code follows.

php:
<?
function timeline_charts($attribute,$secondary,$element,$secondary_element,$minimumdisplay,$unittype="",$filters,$graphtype,$min,$date_grouping){
global $database, $user_table, $commondb, $width, $cdate, $library,$division_display;
    extract($library);
    $attribute_field = str_replace("UNITTYPE",$unittype,$attribute_field);
    if($secondary)  $secondary_field = str_replace("UNITTYPE",$unittype_secondary,$secondary_field);
    $unittype = !empty($unittype) ? $unittype : $attribute_unittype;
    if($secondary)  $unittype_secondary = !empty($unittype_secondary) ? $unittype_secondary : $secondary_unittype;
    extract($unittype_map[$unittype],EXTR_PREFIX_ALL,"attribute");
    if($secondary)  extract($unittype_map[$unittype_secondary],EXTR_PREFIX_ALL,"secondary");
    $attribute_header_suffix = !empty($unittype) ? " $attribute_header{$attribute_unit_type}": "";
    if($secondary)  $secondary_header_suffix = !empty($unittype_secondary) ? " $secondary_header{$secondary_unit_type}": "";
    if(is_array($filters)){
        foreach($filters as $maps => $values){
            $maps_array = str_replace("list","_map",$maps);
            extract(${$maps_array},EXTR_PREFIX_ALL,"filter");
            $filter_string .= $filter_where;
            foreach($values as $array_node => $filter_field){
                $filter_string .= "'$filter_field',";
                }
            $filter_string = substr($filter_string,0,-1).") ";
        }
    }
?>
Yup, not loving confusing at all.

IT BEGINS
Jan 15, 2009

I don't know how to make analogies
pre:
var syntax ={
    ...
    grammer : function (input,value){
       ...
    },
    ...
}
Yup, you definitely know better.

Also, maybe you shouldn't have 15 thousand[!] uncommitted files in your repository with a last committed date of April 5th. But then again, what do I know? Like you said, I obviously don't know the first thing about programming.

IT BEGINS
Jan 15, 2009

I don't know how to make analogies
php:
<?
if (pg_num_rows($pg_res) == 1) {
    $fromcity = pg_result($pg_res, 0, "city");
    $fromstate = pg_result($pg_res, 0, "state");
    $fromcompany = pg_result($pg_res, 0, "name");
    $fromaddress = pg_result($pg_res, 0, "address");
    $fromaddress2 = pg_result($pg_res, 0, "address2");
    $fromphone = pg_result($pg_res, 0, "phone");
}
?>
Please look up the function pg_result. Notice how it's not in the php.net documentation. Want to know why? It was deprecated 10 years ago.

I don't know what's worse: the fact that we are calling the old version of pg_fetch_result() 6 times instead of doing something sane; the fact that we are using functions that have been deprecated for 10 years; the fact that this function still works.

IT BEGINS
Jan 15, 2009

I don't know how to make analogies
php:
<?
while(strstr($field_value,"TABLE.")){
    $new_field_value = $new_field ="";
    $field_explode = explode(" ",$field_value);
    foreach($field_explode as $words){
        $new_field = trim($words);
        if(strstr($words,"TABLE.")){
            $new_field  = str_replace("TABLE.","",$new_field);
            $buffer_field = $new_field;
            $new_field = create_field($new_field,$variations,$library[$new_field],$level,$overwrite_table);
            //echo "field: $buffer_field-->$new_field<br>\n";
            if(strstr($new_field,"global_dynamic_vars") && isset($global_dynamic_vars[$buffer_field])) {
                $new_field  = "'".$global_dynamic_vars[$buffer_field]."'";
            }
            if(empty($new_field)) {
                $new_field = "DO NOT DISPLAY";
            }
        }
        if($testing++ > 100000) break;

        $new_field_value .= "$new_field ";
    }
    $field_value =$new_field_value;
}
?>

IT BEGINS
Jan 15, 2009

I don't know how to make analogies

ARACHNOTRON posted:

Not code, however, a UCD (so it is related to code):


a grad student did this

This is the use case diagram for the professor cutting off his own head.

IT BEGINS
Jan 15, 2009

I don't know how to make analogies

SupSuper posted:

Did someone say use cases?


Fear me, for I am Lord Use Case

Seriously though, what the hell. I think at a certain point you have to go 'these guys just look like they are firing lasers at each other, maybe this isn't a useful diagram any more.'

IT BEGINS
Jan 15, 2009

I don't know how to make analogies
I just think it's a bit flagrant to post an absurd job with poo poo benefits and probably poo poo pay, knowing that because you're a pretty large organization that can afford to give people the appropriate compensation. They also know that because of the community, they'll get a bunch of suckers who apply and will work out their rear end just because this is PA, and not complain about it.

Not to mention that, while being lied to is lovely, being told up front that your job is going to be poo poo can be worse because you have no recourse. It's like 'hey, you knew what you signed up for.'

IT BEGINS
Jan 15, 2009

I don't know how to make analogies
God reading the comments there just makes me mad. A ton of ad hominem and the usual 'working more than 40 hours = dedication' poo poo. Look, if you want to do your company the favor of working more hours than you really have to, that's fine. But your company should then do you the favor of paying you how much that is worth to them.

Maybe I'm wrong, but I feel like here in the US, people have the weird notion that working absurd hours is a sign of dedication, but that getting paid for those hours is somehow a sign of greed.

IT BEGINS
Jan 15, 2009

I don't know how to make analogies

Dren posted:

a reasonable post.

You're probably right. Still, my experience makes me think the position might be filled by a fan for 80k because he doesn't know better.

IT BEGINS
Jan 15, 2009

I don't know how to make analogies

mjau posted:

So he's not a native english speaker, yet he thought he was qualified to reject changes to the english documentation by someone who is?

Or maybe he felt that taking an entire commit to change three words wasn't something worth adding, docs or not.

IT BEGINS
Jan 15, 2009

I don't know how to make analogies

Opinion Haver posted:

Good point, we might run out of commits.

You're a core contributor with a fairly decent understanding of English. You get a commit that changes three words, and you don't really understand why. You know it screws with git blame and is just one more freaking CLA you have to read that day. You reject it.

A few minutes later, some guy pushes that commit through without consulting you. You get a little miffed, since only you and one other person have rights to push these commits, so you revert the change and tell him to stop being a dick.

Is this really an unreasonable line of logic?

IT BEGINS
Jan 15, 2009

I don't know how to make analogies

Pilsner posted:

Men and women pursue and enjoy different things.

Never thought I'd see :biotruths: in the Coding Horrors thread, but then I'm pretty naive about these things.

Maybe this thread should be like gamers.txt - post codehorrorgrog or get banned.

---

No nice sample code to post, but my coworkers insist on using php's extract. Everywhere. For those not familiar with this fantastic function, it lets you dump the keys of an associative array into the symbol table as variables. In practice, this means you have no clue where the hell a particular variable came from, even in an IDE, since lovely code quality means variables are referenced and re-assigned multiple times. Hours spent looking through functions with signatures and code like this:

php:
<?
function shit_function()
{
    global $database, $library;
    extract($library)
    
    ...
}
?>

IT BEGINS
Jan 15, 2009

I don't know how to make analogies
pre:
WITH hist AS (
	SELECT
		invoicenum,
		min(modified || '|' || source) as modified
	FROM invoices
	LEFT JOIN invoicehistory on lookup = invoicenum
	GROUP BY invoicenum
)
SELECT
	...,
	hist.modified
FROM invoices
JOIN hist USING (invoicenum)
WHERE ...
whyyyyyy?

Our system prints out columns with pipe delimiters so a post-processor can convert it to .csv or .xls. Instead of just selecting both fields, our resident asshat decided it would be better to pipe delimit them straight from sql. Thus making sure that we have to un-delimit them to make links to the 'source' field, and that we can't run validation on this field before sending it out. Fantastic.

IT BEGINS
Jan 15, 2009

I don't know how to make analogies

Ithaqua posted:

I was thinking I could do a binary search to find the right index to insert at and skip the sorting entirely.

This would definitely be the fastest (and maybe best) way, especially if you are dealing with tens of thousands of entries.

IT BEGINS
Jan 15, 2009

I don't know how to make analogies
I thought I had seen everything. $comma_pipe was in my nightmares. No, that was only the beginning:

php:
<?
function MapValsToBasedefs($fnames, $basedefs)
{
        //$fnames = array_map('strtolower', $fnames);
        foreach($basedefs as $arname=>$basedef)
        {
            global $$arname;

            $$arname = SetToNull( $$arname);
            $$arname = FindAndMapFieldsToTableNames($basedef, $fnames, $$arname);
            foreach ($$arname as $fld=>$idx) {
                if ($idx !== null) {
                    $offsetArr[$idx] = $fld;
                }
            }
            // $$offsetArr = array_flip($$arname);  cant use array_flip when some values are null
            if (count($offsetArr))
                ksort($offsetArr);
            $arname_Offset = "{$arname}_Offset";
            global $$arname_Offset;
            $$arname_Offset = $offsetArr;
        }
        return true;
}
?>
What the actual gently caress.

IT BEGINS
Jan 15, 2009

I don't know how to make analogies
php:
<?
function output_line(){
global $schema, $charges,$last_invoice_row,$last_shipment_row, $total_charges, $carrier,$comma_pipe,$table_storage;
?>
Like ... why. loving whyyyyyy?

The extra-extra-extra fun part? The poo poo that calls this uses an extract() on a gigantic array that may or may not contain some of these values. Fan. loving. Tastic.

Bonus:

php:
<?
    //var_dump($last_invoice_row);
    //var_dump($last_shipment_row);
//    print_r($charges);
//     print_r($total_charges); exit(1);
?>
What even is debugging. Or version control.

IT BEGINS fucked around with this message at 14:30 on Mar 14, 2014

IT BEGINS
Jan 15, 2009

I don't know how to make analogies
Just in case you ever wanted a new way to write the number 0:

code:
sum(coalesce(b.charge,b.approved)-coalesce(b.charge,b.approved)) as Disputed_Amount,
There you loving go.

IT BEGINS
Jan 15, 2009

I don't know how to make analogies

QuarkJets posted:

It'd be like calling someone stupid for being a bad dancer

I see it more as calling someone stupid for refusing to take the time to learn to dance ... in a world where half the planet communicates via dancing.

-----

php:
<?
function initClaimInfo($trno, $myuserID=""){
    global $userid;
    global $s_carriercode, $s_custcode, $s_shipperid, $s_credate, $s_checknum;
    global $s_trackingno, $s_phone, $s_weight, $s_declaredvalue, $s_freightcharge;
    global $s_reference, $s_ref_desc, $_REQUEST;
    global $statuslist, $s_status, $s_id, $s_expdeldate;
    global $s_ackdate, $s_paiddate, $s_remark, $s_carrierclaimid, $s_reason;
    global $s_userid, $s_paidamount, $s_barcode, $s_delstatus;
    global $s_c_description, $s_c_sku, $s_c_units, $s_c_weightperunit, $s_c_totalweight;
    global $s_c_costperunit, $s_c_totalcost, $s_c_createdby, $s_c_dcr, $cntlines, $liveCount;

    $s_trackingno = $trno;
    $s_userid = $myuserID ? : $userid;
    $s_carriercode=""; $s_custcode=""; $s_shipperid=""; $s_credate=""; $s_checknum="";
    $s_phone=""; $s_weight=""; $s_declaredvalue=""; $s_freightcharge="";
    $s_ref_desc="";
    $statuslist=""; $s_status=""; $s_id=""; $s_expdeldate="";
    $s_ackdate=""; $s_paiddate=""; $s_remark=""; $s_carrierclaimid=""; $s_reason="";
    $s_paidamount=""; $s_barcode=""; $s_delstatus="";
    for ($rc=1; $rc <= 5; $rc++) {
        $s_reference[$rc]="";
        $s_ref_desc[$rc]="";
    }   
}
?>
I don't even know anymore.

IT BEGINS
Jan 15, 2009

I don't know how to make analogies

Mogomra posted:

Of course, you should probably use extract() instead, but :shrug:

Or just be sane and have one drat signature for the functions instead of some crazy variable poo poo.

Also gently caress extract into the next universe.

IT BEGINS
Jan 15, 2009

I don't know how to make analogies
php:
<?
// more horrors above

insertdata();
retrievedata();
print_tables($content, $periods, $total);

echo "</pre>";
exit(1);


?>
</BODY>
</excel-tab>


?>
Fan. Tastic.

IT BEGINS
Jan 15, 2009

I don't know how to make analogies

down with slavery posted:

</excel-tab> wtf is that

Some internal HTML-esque poo poo we use to convert reports into excel spreadsheets. I dare not look into that abyss.

Also, following the above garbage:

php:
<?
function insertdata()
{
    global $database;
    // ...
}

function retrievedata()
{
    global $content, $periods, $database, $total, $filters_array, $section_list;
    // ...
}

function print_tables($content, $periods, $total)
{
    global $section_list;
    // ...
}
?>
Edit: there's nothing like realizing in the middle of pulling out these globals that half of those are actually being modified and then used afterwards in another function. Good times. Not mad at all. :bang:

IT BEGINS fucked around with this message at 18:37 on Dec 3, 2014

IT BEGINS
Jan 15, 2009

I don't know how to make analogies
I know I've posted like five of these already, but there's more and I can't handle it.

php:
<?
function print_section ($tblname,$direction, &$nlines, $showd, $invFilter) {
global $database, $bydate, $groupby, $totals, $grandtotals, $filter, $more_hdr, $more_fields, $inboundCarriers;

...

extract($fields[$tblname]);

...

extract ($row);

...

}
?>

IT BEGINS
Jan 15, 2009

I don't know how to make analogies
I think it's pretty reasonable to go on a rant for a page or two about parts of programming culture that you don't like/disagree with. On the other hand, if you have such a big problem with a part of programming that you are compelled to write a 50+ page rant about it, you may need to reconsider your career (or go into academia and get paid for it instead).

IT BEGINS
Jan 15, 2009

I don't know how to make analogies

MrMoo posted:

This has been commented on many times, the strong language has proved productive in the LKML in order to make progress.

I think there's a reasonable middle ground between overly courteous and 'you should be retroactively aborted', but that's just me I guess? :shrug:

IT BEGINS fucked around with this message at 16:23 on Jan 29, 2015

IT BEGINS
Jan 15, 2009

I don't know how to make analogies
php:
<?
$nchanged = pg_cmdtuples($result);
?>
This doesn't look so bad, right? What could be so horrible about a single line of PHP code?

Here's whats wrong with it: the last time you could find this function documented on php.net was ... 2004. When it had been deprecated for the previous 2+ years.

I don't know what's worse - that this function still exists in the code base I work on, or that a function that's been deprecated for 12+ years still works.

IT BEGINS
Jan 15, 2009

I don't know how to make analogies

Skuto posted:

Yeah, can't have old working code, let's deprecate and replace by some SwiftNodeGoDartAngular#.js equivalent with new bugs.

Only kids and hipsters think code is a thing that rusts. You don't seem very hip, so...

I don't think it's reasonable for this code to work without even throwing warnings. Like, why are we removing the docs if this is still part of the language? I suppose we can have the backwards compatibility debate here, but I think it's a little silly to assume that code that was deprecated 15 years ago should just silently/magically work on the newest version of the language.

IT BEGINS
Jan 15, 2009

I don't know how to make analogies

ExcessBLarg! posted:

How does someone find out pg_cmdtuples is deprecated though? Here's the ways:

1. Chance read of documentation and active review of stable code.

It's literally only this. I have a poo poo-ton of code to wade through that uses functions like these.

Also lol @ deprecated in PHP 5 - it was deprecated in 4.2.0. That's April 2002. Fun times.

IT BEGINS
Jan 15, 2009

I don't know how to make analogies

My Boss posted:

its old Ed's code
pretty straightforward

php:
<?
    if ($qrytype == "u" || $qrytype == 'f')
        $tblsvarnamelist=array ($flisth, $flistd, $flistc);
    else
        $tblsvarnamelist=array ($flistp, $flista);

...

    GetConditionRowsTable($tbllist, $varlist, $var_op, $values,
            $upcaseflag,$varlist2, $scope1, $var_join_op, $scope2,
            $tblsvarnamelist, $varnamelist, $varnamelist_empty);

...

    if ($qrytype == "p")
    {
        //for the time being
        include_once "../upsaudit/viewdetailtbl.php";
        class upsinvoicereptable extends UPSInvoiceDetailTable
        {
            function GetSQL ()
            {
                global $ressql;
                return $ressql;
            }
        }
        $tbl = new UPSInvoiceRepTable ();
    }
?>
gently caress you.

IT BEGINS
Jan 15, 2009

I don't know how to make analogies
Can we just vent about dump coding-related horrors in here? Examples (PHP):

  • A co-worker refuses to stop putting spaces between function names parenthesis, despite being the only developer who does so. When asked why, his response is "that's how they do it in C".
  • A co-worker regularly pollutes class files with random functions when adding functionality. When asked why he didn't at least add them to the class in the file, he says "I didn't really know where to put them".
  • A co-worker refuses to stop using extract() everywhere. drat near everything that hits the DB has something like
    php:
    <?
    while ($row = pg_fetch_assoc($res)) {
        extract($row);
        ...
    }
    ?>
    After explaining that this makes it impossible for the developers to know which variables are safe or where they come from, his response is "Yeah but it's easier this way. I don't have to write '$row' everywhere."
  • Several co-workers refuse to stop using globals inside function calls. Despite lengthy discussions, and being shown several articles and google tech talks about how globals are an issue, there is no change. "It's easier this way, I don't have to pass so many parameters".
  • A co-worker keeps rolling giant, unusable frameworks every three to six months. Initially, they featured a central 3000-line file that contained a single array that described every field in the database, organized alphabetically by field name. Now, after we recently began using KendoUI for our front-end, he rolled a front-end framework on top of it.
  • Several co-workers describe OOP as "just a question of style." They refuse to organize related functionality into classes because "it takes too much time, and then I have to always instantiate these classes everywhere."

Am I crazy? Is this crazy?

IT BEGINS
Jan 15, 2009

I don't know how to make analogies

Mogomra posted:

I don't call functions with a space between the name and the parameters, but define them that way. Am I just perpetuating the horror? What's so terrible about an extra space there?

No, this is fine. It's the space between the function call and the parameters that fucks with me. The following:

php:
<?
function poo ($a) { ... } // great

foreach ($butts as $butt) {
    poo ($butt); // not great
}
?>

IT BEGINS
Jan 15, 2009

I don't know how to make analogies
I probably should quit, but the pay is really good for my experience and the perks are awesome. Then again, the stupidity is overwhelming. My friend in QA recently overhead one of the guys arguing with the boss about some code - apparently he used the phrase it's not a bug, it's a feature completely unironically. Also, more dumb stuff like this:

php:
<?
    foreach($checknum_storage as $carriercode => $custcode_array){
        foreach($custcode_array as $custcode => $currency_array){
            foreach($currency_array as $currency => $checks_array){
                foreach($checks_array as $checknum_span_value => $contents){
                extract($contents);
                ...
                }
            }
        }
    }
?>
They'll regularly add a global variable call instead of adding it as a parameter because "Then I'd have to change it every place where that function is called." I still regularly come across bits of code that use boolean switches to execute stuff:

php:
<?
$x = ($somevalue == true);
$x && executeSomething ();
?>
After five years of using Mercurial, several devs still don't know how to resolve merge conflicts or how to use branches/bookmarks. The rabbit hole goes pretty deep.

IT BEGINS
Jan 15, 2009

I don't know how to make analogies

Snapchat A Titty posted:

It sounds like there's no management where IT BEGINS works, so coding style is obviously a free for all.

Everything here is a free-for-all. There's no care for code quality and project management is a crap-shoot. We've got seven developers and what is essentially seven teams. I'm currently working on three different major projects for which I am the sole developer, with extremely occasional input from my boss. I don't know how, but miraculously this ship is still afloat (and growing, somehow).

After repeated attempts to explain why OOP is useful for us, why refactoring code that we have to regularly maintain is good, why separation of concerns is critical, I've mostly given up. Hell, I just barely convinced my higher ups to move our reports and templates into the same repository as the rest of our software. I was told 'they are unrelated' (they use a huge amount of shared code and are part of the same overall piece of software). Never mind that I contribute more than 50% of the code to this reporting engine and I'm regularly forced to take part in some sort of 'branch disco' because my changes can't be kept in sync.

Edit: Also sorry if this isn't the thread to vent, but I figured you guys could enjoy some Schadenfreude that wasn't strictly code.

IT BEGINS
Jan 15, 2009

I don't know how to make analogies

Athas posted:

Apart from the extract in the inner body and dubious whitespace, what else is wrong? I am not a PHP programmer.

Mainly the extract, but also having a four-layer array is probably not a good thing. It's not strictly bad but it comes from other code like

php:
<?
$checknum_storage[$carriercode][$mastercustcode_custcode][$currency][$checknum_span_value]['controlnum']= $controlnum;
$checknum_storage[$carriercode][$mastercustcode_custcode][$currency][$checknum_span_value]['invoicenum']= $invoicenum;
$checknum_storage[$carriercode][$mastercustcode_custcode][$currency][$checknum_span_value]['checknum_span']= $checknum_span;
?>
so at the least it's quite confusing.

Edit: from the same file:

php:
<?
function maintain_checknums($row){
    global $database, $checknum_storage,$current_date_time,$current_date;
    extract($row);
    if(!isset($mastercustcode_custcode)) $mastercustcode_custcode = $custcode;
    if(!isset($current_date_time)) $current_date_time = date('H:i:s',strtotime(date('Y/m/d H:i:s')));
    if(!isset($current_date)) $current_date = date('m/d/Y',strtotime(date('Y/m/d H:i:s')));
?>

IT BEGINS
Jan 15, 2009

I don't know how to make analogies

John Big Booty posted:

What exactly is it supposed to be doing in the innermost loop?

Everything?

For a specific carrier it runs a report, for a particular customer it updates an invoice, and for all customers it updates a check number. It also sometimes prints out formatted lines containing some of the stuff in $contents. It also has an extremely helpful name.

php:
<?
function updateFinalResults ($database, $userid, $output, $checknum_storage) { ... }
?>

IT BEGINS
Jan 15, 2009

I don't know how to make analogies
Welcome to Taco Hell

php:
<?
function maintain_checknums($row){
    global $database, $checknum_storage,$current_date_time,$current_date;
    extract($row);
    if(!isset($mastercustcode_custcode)) $mastercustcode_custcode = $custcode;
    if(!isset($current_date_time)) $current_date_time = date('H:i:s',strtotime(date('Y/m/d H:i:s')));
    if(!isset($current_date)) $current_date = date('m/d/Y',strtotime(date('Y/m/d H:i:s')));
    $check_not_maxed = true;
    $counter = 0;
    $altered_check = $checknum_span_value;
    if($max_per_check){
        while($check_not_maxed){
            $altered_check = $checknum_span_value.$suffix;
            $amount_to_max = $carriercode !="FDX" ? $totalcharges : $approvedamount;
            if($amount_to_max > $max_per_check && !isset($checknum_storage[$carriercode][$mastercustcode_custcode][$currency][$altered_check])){
                if($total_lines_read ==1 || $checknum_type_case =="CHECKNUM")
                    $check_not_maxed = false;
                else{
                    $sql = "
                    select
                        seq,
                        dtl.approvedamount,
                        netcharges as totalcharges
                    from
                        {$table_type}invoice hdr join
                        {$table_type}invoicedetail dtl using (invoicenum)
                    where
                        hdr.invoicenum ='$invoicenum'
                        $custom_where
                        and $checknum_subdivision = '$checknum_span_value'
                    order by seq";
                    $res = pg_query($database,$sql);
                    $row['approvedamount']= $row['totalcharges'] =0;
                    while ($sub_checknum_row = pg_fetch_assoc($res)){
                        extract($sub_checknum_row);
                        $amount_to_max = $carriercode !="FDX" ? $totalcharges : $approvedamount;

                        if($amount_to_max > $max_per_check){
                            $too_big = $row;
                            $too_big['approvedamount'] = $approvedamount;
                            $too_big['totalcharges'] = $totalcharges;
                            $too_big['custom_where'] = " and seq = $seq";
                            $too_big['total_lines_read'] =1;
                            $controlnum = maintain_checknums($too_big);
                            continue;
                        }
                        $amount_to_field = $carriercode !="FDX" ? 'totalcharges' : 'approvedamount';
                        $amount_to_max = $carriercode !="FDX" ? $totalcharges : $approvedamount;
                            if(($row[$amount_to_field] + $amount_to_max) > $max_per_check){
                            $row['custom_where'] .=$row['custom_where2'];
                            $controlnum = maintain_checknums($row);
                            $row['approvedamount']= $row['totalcharges'] =0;
                            $row['custom_where'] = str_replace($row['custom_where2'],"",$row['custom_where']);
                            }
                        $row['total_lines_read']++;
                        $row['approvedamount'] +=$approvedamount;
                        $row['totalcharges'] +=$totalcharges;
                        $row['custom_where2'] =" and seq <='$seq'";
                        }
                    $row['custom_where'] .=$row['custom_where2'];
                    $controlnum = maintain_checknums($row);

                    return $controlnum;
                }
            }
            $amount_to_field = $carriercode !="FDX" ? 'totalcharges' : 'approvedamount';
            $amount_to_max = $carriercode !="FDX" ? $totalcharges : $approvedamount;
            if(($checknum_storage[$carriercode][$mastercustcode_custcode][$currency][$altered_check][$amount_to_field]+$amount_to_max) < $max_per_check)
                $check_not_maxed = false;

            $suffix = "-".++$counter;
        }
    }
    $checknum_span_insert = $checknum_span_value =$altered_check;
    if(strstr($checknum_span_value,"-"))
        $checknum_span_insert = implode("-",explode("-",$checknum_span_value,-1));

    if(!isset($checknum_storage[$carriercode][$mastercustcode_custcode][$currency][$checknum_span_value])) {

        $controlnum =get_next_checknum_status($row,$status);
        SqlUpdate($database,"insert into mastercheckdetails values ( '$mastercustcode_custcode','$controlnum','$checknum_type_case','$checknum_span_insert','$checknum_span','$table_type','$carriercode',null,null,null,'$current_date','$current_date_time', '{$row["currency"]}')");
        $checknum_storage[$carriercode][$mastercustcode_custcode][$currency][$checknum_span_value]['controlnum']= $controlnum;
        $checknum_storage[$carriercode][$mastercustcode_custcode][$currency][$checknum_span_value]['invoicenum']= $invoicenum;
        $checknum_storage[$carriercode][$mastercustcode_custcode][$currency][$checknum_span_value]['checknum_span']= $checknum_span;
    }
    $controlnum = $checknum_storage[$carriercode][$mastercustcode_custcode][$currency][$checknum_span_value]['controlnum'];
    $checknum_storage[$carriercode][$mastercustcode_custcode][$currency][$checknum_span_value]['approvedamount'] += $approvedamount;
    $checknum_storage[$carriercode][$mastercustcode_custcode][$currency][$checknum_span_value]['totalcharges'] += $totalcharges;
    $row['controlnum'] = $controlnum;
    if($checknum_type_case =="SUBCHECKNUM")
        update_invoice_detail_checknum($row);


    return $controlnum;
}
?>
Edit: I just noticed this function calls itself recursively, and I have no idea why. Also it occasionally just returns from mid-function. Also not sure why it does that.

IT BEGINS fucked around with this message at 21:03 on Apr 24, 2015

Adbot
ADBOT LOVES YOU

IT BEGINS
Jan 15, 2009

I don't know how to make analogies

canis minor posted:

You had me with this already :psyboom: later on it was just more of wtfs

I guess you can show them all the warnings in PHP doc (http://php.net/manual/en/function.extract.php), if that can amount for something (looking however at the rest of the code I'll remain doubtful if anything can really be done). Please, get away from these people, at least for your mental sanity.

I've tried. Their response was that the warnings are about handling user input and files, so it's fine to do this. Fun.

Hell, I've had something like the following conversation:

quote:

Me: Hey can you please group this new functionality into a class, at least? I don't mind having a bunch of methods I'd just like to know where they come from and what they are related to.
Him: Nah, this makes it easier. I can just include the file without having to create the class, and then use my function. Also it would be slower if I did a class, since the class would have all these methods.

Me: Well, it wouldn't be slower. And yeah, that's exactly the problem - often times you're including a bunch of files and I don't know what comes from where.
Him: No, this is stupid. I'd have to create an object every time I wanted to use one of these. And I'd have to use $this a lot. That will take too much time.

Me: But, like, I can't understand this code.
Him: I don't see what the problem is, it's fairly straightforward.

  • 1
  • 2
  • 3
  • 4
  • 5
  • Post
  • Reply