nav[aria-label="Primary Navigation"] { padding: 0; & ul { list-style: none; width: 100%; display: flex; flex-direction: row; justify-content: start; align-items: start; gap: 30px; padding: 0; & li { margin: 0; } & ul li { list-style: none; } } }

Child Item

Coding Horror

Evil Phil

This is a place to vent your frustration however there is one rule:

1. Do not name a company, organisation or individual as an attempt to undermine or discredit them in any way. This will really be a question of moderator discretion but use your common sense here. Boards.ie Ltd does not want to have fight what is rightfully somebody else's slander case.

Other than that feel free to discuss programming gotcha's, frustration with badly managed projects (just don't name anyone), frustration with technology or just plain idiotic things you've encountered.

Find more posts tagged with

Quick Links

Comments

oscarBravo

Colonel Panic

No coding standards or coding standards you don't agree with. Which is worse?

Anesthetize

Maximus Alexander wrote: »
You'd hate me then, because I'd probably have written:
if (.....) <some statement>;
else <some statement>;

<some statement>;

That's a paddlin'

IRLConor

Colonel Panic wrote: »

No coding standards or coding standards you don't agree with. Which is worse?

Coding standards you don't agree with. At least when there are no coding standards, you're not angry all the time.

Maximus Alexander

Anesthetize wrote: »

That's a paddlin'

Meh. Java's unnecessarily verbose at times. I value consistency and clarity, but also brevity.

Aswerty

Colonel Panic wrote: »

No coding standards or coding standards you don't agree with. Which is worse?

Subjectively; the former. Objectively; the latter.

Calina

Colonel Panic wrote: »

No coding standards or coding standards you don't agree with. Which is worse?

Well, if there are no coding standards you stand a chance of getting some you approve of implemented at some point.

Coding standards you don't agree with are just a recipe for daily frustration.

Buford T Justice

One of the many many loops in one method. The old, lets just bring everything back in an SQL query and work it all out in a controller afterwards.

foreach ($aryLineItems as $lineItemKey => $lineItemVal) {
                    $arySelectedSupplierProducts = array();
                    if (!empty($lineItemVal['LineItem']['line_item_product_id'])) {
                        if (strpos($lineItemVal['LineItem']['line_item_product_id'], "|") !== false) {
                            $aryLineItemProductIds = explode("|", $lineItemVal['LineItem']['line_item_product_id']);
                            foreach ($aryLineItemProductIds as $lineItemProductQtyId) {
                                if (strpos($lineItemProductQtyId, "::") !== false) {
                                    $lineItemProductId = substr($lineItemProductQtyId, 0, strpos($lineItemProductQtyId, '::'));
                                    $lineItemProductQty = substr($lineItemProductQtyId, (strpos($lineItemProductQtyId, '::') + 2));
                                }
                                else {
                                    $lineItemProductId = substr($lineItemProductQtyId, 0, strpos($lineItemProductQtyId, '::'));
                                    $lineItemProductQty = "1";
                                }
                                $arySelectedSupplierProducts[$lineItemProductId] = $lineItemProductQty;
                            }
                        }
                        else {
                            if (strpos($lineItemVal['LineItem']['line_item_product_id'], "::") !== false) {
                                $lineItemProductQtyId = $lineItemVal['LineItem']['line_item_product_id'];
                                $lineItemProductId = substr($lineItemProductQtyId, 0, strpos($lineItemProductQtyId, '::'));
                                $lineItemProductQty = substr($lineItemProductQtyId, (strpos($lineItemProductQtyId, '::') + 2));
                                $arySelectedSupplierProducts[$lineItemProductId] = $lineItemProductQty;
                            }
                            else {
                                $lineItemProductId = $lineItemVal['LineItem']['line_item_product_id'];
                                $lineItemProductQty = "1";
                                $arySelectedSupplierProducts[$lineItemProductId] = $lineItemProductQty;
                            }
                        }
                    }

                    if (count($arySelectedSupplierProducts) > 0) {
                        $aryLineItemProducts = $this->Arrangement->LineItem->LineItemProduct->find('all', array(
                                'recursive' => 0,
                                'conditions' => array('LineItemProduct.id' => array_keys($arySelectedSupplierProducts)),
                            )
                        );
                        $aryLineItems[$lineItemKey]['LineItemSupplierProducts'] = $aryLineItemProducts;
                        $aryLineItems[$lineItemKey]['LineItemSupplierProductsQtys'] = $arySelectedSupplierProducts;
                    }
                    else {
                        if (!empty($lineItemVal['LineItem']['cheque_name'])) {
                            $aryLineItems[$lineItemKey]['LineItemSupplierProducts'][0]['Contact']['company_name'] = $lineItemVal['LineItem']['cheque_name'];
                        }
                    }

                }
                $aryData['LineItem'][$sLineItemType] = $aryLineItems;
            }

And lets not forget

foreach ($aryLineItems as $lineItemKey => $lineItemVal) {
                    $arySelectedSupplierProducts = array();
                    if (!empty($lineItemVal['LineItem']['line_item_product_id'])) {
                        if (strpos($lineItemVal['LineItem']['line_item_product_id'], "|") !== false) {
                            $aryLineItemProductIds = explode("|", $lineItemVal['LineItem']['line_item_product_id']);
                            foreach ($aryLineItemProductIds as $lineItemProductQtyId) {
                                if (strpos($lineItemProductQtyId, "::") !== false) {
                                    $lineItemProductId = substr($lineItemProductQtyId, 0, strpos($lineItemProductQtyId, '::'));
                                    $lineItemProductQty = substr($lineItemProductQtyId, (strpos($lineItemProductQtyId, '::') + 2));
                                }
                                else {
                                    $lineItemProductId = substr($lineItemProductQtyId, 0, strpos($lineItemProductQtyId, '::'));
                                    $lineItemProductQty = "1";
                                }
                                $arySelectedSupplierProducts[$lineItemProductId] = $lineItemProductQty;
                            }
                        }
                        else {
                            if (strpos($lineItemVal['LineItem']['line_item_product_id'], "::") !== false) {
                                $lineItemProductQtyId = $lineItemVal['LineItem']['line_item_product_id'];
                                $lineItemProductId = substr($lineItemProductQtyId, 0, strpos($lineItemProductQtyId, '::'));
                                $lineItemProductQty = substr($lineItemProductQtyId, (strpos($lineItemProductQtyId, '::') + 2));
                                $arySelectedSupplierProducts[$lineItemProductId] = $lineItemProductQty;
                            }
                            else {
                                $lineItemProductId = $lineItemVal['LineItem']['line_item_product_id'];
                                $lineItemProductQty = "1";
                                $arySelectedSupplierProducts[$lineItemProductId] = $lineItemProductQty;
                            }
                        }
                    }

                    if (count($arySelectedSupplierProducts) > 0) {
                        $aryLineItemProducts = $this->Arrangement->LineItem->LineItemProduct->find('all', array(
                                'recursive' => 0,
                                'conditions' => array('LineItemProduct.id' => array_keys($arySelectedSupplierProducts)),
                            )
                        );
                        $aryLineItems[$lineItemKey]['LineItemSupplierProducts'] = $aryLineItemProducts;
                        $aryLineItems[$lineItemKey]['LineItemSupplierProductsQtys'] = $arySelectedSupplierProducts;
                    }
                    else {
                        if (!empty($lineItemVal['LineItem']['cheque_name'])) {
                            $aryLineItems[$lineItemKey]['LineItemSupplierProducts'][0]['Contact']['company_name'] = $lineItemVal['LineItem']['cheque_name'];
                        }
                    }

                }
                $aryData['LineItem'][$sLineItemType] = $aryLineItems;
            }

Only about 3K lines of code like that in one controller

irishgeo

Buford T Justice wrote: »

One of the many many loops in one method. The old, lets just bring everything back in an SQL query and work it all out in a controller afterwards.

foreach ($aryLineItems as $lineItemKey => $lineItemVal) {
                    $arySelectedSupplierProducts = array();
                    if (!empty($lineItemVal['LineItem']['line_item_product_id'])) {
                        if (strpos($lineItemVal['LineItem']['line_item_product_id'], "|") !== false) {
                            $aryLineItemProductIds = [B]explode[/B]("|", $lineItemVal['LineItem']['line_item_product_id']);
                            foreach ($aryLineItemProductIds as $lineItemProductQtyId) {
                                if (strpos($lineItemProductQtyId, "::") !== false) {
                                    $lineItemProductId = substr($lineItemProductQtyId, 0, strpos($lineItemProductQtyId, '::'));
                                    $lineItemProductQty = substr($lineItemProductQtyId, (strpos($lineItemProductQtyId, '::') + 2));
                                }
                                else {
                                    $lineItemProductId = substr($lineItemProductQtyId, 0, strpos($lineItemProductQtyId, '::'));
                                    $lineItemProductQty = "1";
                                }
                                $arySelectedSupplierProducts[$lineItemProductId] = $lineItemProductQty;
                            }
                        }
                        else {
                            if (strpos($lineItemVal['LineItem']['line_item_product_id'], "::") !== false) {
                                $lineItemProductQtyId = $lineItemVal['LineItem']['line_item_product_id'];
                                $lineItemProductId = substr($lineItemProductQtyId, 0, strpos($lineItemProductQtyId, '::'));
                                $lineItemProductQty = substr($lineItemProductQtyId, (strpos($lineItemProductQtyId, '::') + 2));
                                $arySelectedSupplierProducts[$lineItemProductId] = $lineItemProductQty;
                            }
                            else {
                                $lineItemProductId = $lineItemVal['LineItem']['line_item_product_id'];
                                $lineItemProductQty = "1";
                                $arySelectedSupplierProducts[$lineItemProductId] = $lineItemProductQty;
                            }
                        }
                    }

                    if (count($arySelectedSupplierProducts) > 0) {
                        $aryLineItemProducts = $this->Arrangement->LineItem->LineItemProduct->find('all', array(
                                'recursive' => 0,
                                'conditions' => array('LineItemProduct.id' => array_keys($arySelectedSupplierProducts)),
                            )
                        );
                        $aryLineItems[$lineItemKey]['LineItemSupplierProducts'] = $aryLineItemProducts;
                        $aryLineItems[$lineItemKey]['LineItemSupplierProductsQtys'] = $arySelectedSupplierProducts;
                    }
                    else {
                        if (!empty($lineItemVal['LineItem']['cheque_name'])) {
                            $aryLineItems[$lineItemKey]['LineItemSupplierProducts'][0]['Contact']['company_name'] = $lineItemVal['LineItem']['cheque_name'];
                        }
                    }

                }
                $aryData['LineItem'][$sLineItemType] = $aryLineItems;
            }

And lets not forget

foreach ($aryLineItems as $lineItemKey => $lineItemVal) {
                    $arySelectedSupplierProducts = array();
                    if (!empty($lineItemVal['LineItem']['line_item_product_id'])) {
                        if (strpos($lineItemVal['LineItem']['line_item_product_id'], "|") !== false) {
                            $aryLineItemProductIds = explode("|", $lineItemVal['LineItem']['line_item_product_id']);
                            foreach ($aryLineItemProductIds as $lineItemProductQtyId) {
                                if (strpos($lineItemProductQtyId, "::") !== false) {
                                    $lineItemProductId = substr($lineItemProductQtyId, 0, strpos($lineItemProductQtyId, '::'));
                                    $lineItemProductQty = substr($lineItemProductQtyId, (strpos($lineItemProductQtyId, '::') + 2));
                                }
                                else {
                                    $lineItemProductId = substr($lineItemProductQtyId, 0, strpos($lineItemProductQtyId, '::'));
                                    $lineItemProductQty = "1";
                                }
                                $arySelectedSupplierProducts[$lineItemProductId] = $lineItemProductQty;
                            }
                        }
                        else {
                            if (strpos($lineItemVal['LineItem']['line_item_product_id'], "::") !== false) {
                                $lineItemProductQtyId = $lineItemVal['LineItem']['line_item_product_id'];
                                $lineItemProductId = substr($lineItemProductQtyId, 0, strpos($lineItemProductQtyId, '::'));
                                $lineItemProductQty = substr($lineItemProductQtyId, (strpos($lineItemProductQtyId, '::') + 2));
                                $arySelectedSupplierProducts[$lineItemProductId] = $lineItemProductQty;
                            }
                            else {
                                $lineItemProductId = $lineItemVal['LineItem']['line_item_product_id'];
                                $lineItemProductQty = "1";
                                $arySelectedSupplierProducts[$lineItemProductId] = $lineItemProductQty;
                            }
                        }
                    }

                    if (count($arySelectedSupplierProducts) > 0) {
                        $aryLineItemProducts = $this->Arrangement->LineItem->LineItemProduct->find('all', array(
                                'recursive' => 0,
                                'conditions' => array('LineItemProduct.id' => array_keys($arySelectedSupplierProducts)),
                            )
                        );
                        $aryLineItems[$lineItemKey]['LineItemSupplierProducts'] = $aryLineItemProducts;
                        $aryLineItems[$lineItemKey]['LineItemSupplierProductsQtys'] = $arySelectedSupplierProducts;
                    }
                    else {
                        if (!empty($lineItemVal['LineItem']['cheque_name'])) {
                            $aryLineItems[$lineItemKey]['LineItemSupplierProducts'][0]['Contact']['company_name'] = $lineItemVal['LineItem']['cheque_name'];
                        }
                    }

                }
                $aryData['LineItem'][$sLineItemType] = $aryLineItems;
            }

Only about 3K lines of code like that in one controller

what does explode do?:D

Maximus Alexander

Buford T Justice wrote: »

One of the many many loops in one method. The old, lets just bring everything back in an SQL query and work it all out in a controller afterwards.

...

Only about 3K lines of code like that in one controller

My stomach actually knotted up thinking about what that work day must have been like.

L.Jenkins

Buford T Justice wrote: »
One of the many many loops in one method. The old, lets just bring everything back in an SQL query and work it all out in a controller afterwards.
Some Code
And lets not forget
More Code
Only about 3K lines of code like that in one controller

Buford T Justice

Itzy wrote: »

That kinda sums it up. I'm the one that's left with servicing & trying to fix this steaming pile of scutter

punk_one82

/**
     * this method is not used.
     * 
     * @param busEntity  Business Entity. 
     * @return null 
     */
	@Override
	public BenefitEntity methodName(final BusinessEntity busEntity) {
		// Not used
		return null;
	}

Stuff like this drives me mad, but probably not uncommon at all.

L.Jenkins

punk_one82 wrote: »

/**
     * this method is not used.
     * 
     * @param busEntity  Business Entity. 
     * @return null 
     */
	@Override
	public BenefitEntity methodName(final BusinessEntity busEntity) {
		// Not used
		return null;
	}

Stuff like this drives me mad, but probably not uncommon at all.

You mean stuffing methods in or leaving a bunch of shít hanging around when it's not needed? Working on Application migration and fault finding and have found way too much unnecessary crap in nearly every file I went through.

punk_one82

Itzy wrote: »

You mean stuffing methods in or leaving a bunch of shít hanging around when it's not needed? Working on Application migration and fault finding and have found way too much unnecessary crap in nearly every file I went through.

Leaving a load of crap in the code. If a method has no use and somebody has the time to comment it saying so why not just remove the method.

Capt'n Midnight

From the Most stupid requests you've ever had at work? thread

Wossack wrote: »

Database admin decides he needs to save some space on their sql servers, so goes through C:\windows and deletes all files whose filenames are blue (opposed to normal uncompressed black), thinking that meant they were backed up and redundant.

Happy with his work, he scripts it, and runs it across another 23 servers before he cops the first one has keeled over.. and, like stars winking out in the night sky, the rest gradually follow suit

oscarBravo

Oh.

My.

Teapot.

:eek:

BrokenArrows

Surely noone can be that stupid.

[Rasta]

Currently we have numerous VB scripts (inside BPM application workflows) that have hardcoded strings for database access...

Not to mention the numerous 'temporary' fixes to these workflows which still grab information from dev servers (why in the sweet jesus would you do that anyway).

There is no documentation for these workflows, because 'we make a lot of changes and documentation would just take too much time'.
I looked through the publish history of these workflows, most hadn't had any change in the past 3 months. Adding people to CCs on emails that are sent by the workflow was a common reason given for a publish.

Changes are usually published to live without testing (only a peer review is done to see if both people agree that the changes shouldn't cause an issue).

A lot of the code either lacks comments or has comments which are out of date.
Quite often method/function/member names have no correlation with their functionality/purpose or are even entirely misleading.

I saw an if statement:
if(archived == false && archivedInverse == true)
..some code

and at some point earlier on:
if(archived == true)
archivedInverse = false
else
archivedInverse = true

not a coding issue but:
We backup data onto the same machine the data is on. After 3 days it is automatically picked up by an external company who copy it onto tapes.
Someone requested a backup 2 weeks ago. Yet to see any progress.

L.Jenkins

BrokenArrows wrote: »

Surely noone can be that stupid.

You'd be surprised and I wouldn't be if the first thing they did on a Linux box was to run rm -rf / as an admin to save space also.

Capt'n Midnight

Panda antivirus software labels itself as malware

L.Jenkins

Capt'n Midnight wrote: »

Panda antivirus software labels itself as malware

That was one of the best reads in a long time, apparently bricking the machine it was installed on.

FobleAsNuck

https://www.youtube.com/watch?v=BKorP55Aqvg

Pov06

if (some_var < 0)
    another_var -= abs(some_var);
else
    another_var -= some_var;

Kill me now :eek:

Boskowski

Pov06 wrote: »

if (some_var < 0)
    another_var -= abs(some_var);
else
    another_var -= some_var;

Kill me now :eek:

At least the compiler will sort that out.

brevity

Not quite a coding horror as such but a great article nonetheless

http://www.stilldrinking.org/programming-sucks

Pov06

Boskowski wrote: »

At least the compiler will sort that out.

The more worrying part is the fact that a developer with such concepts is working on big projects.

Boskowski

Pov06 wrote: »

The more worrying part is the fact that a developer with such concepts is working on big projects.

It is - although technically it's not wrong. Just a bit .... convoluted?

The worrying bit is the obvious lack of understanding.

not1but4

Pov06 wrote: »

if (some_var < 0)
    another_var -= abs(some_var);
else
    another_var -= some_var;

Kill me now :eek:

At the risk of asking a very stupid question what is the issue with this?

Is it the case if you where to use a 16 bit integer (range being -32768 to + 32767)

You'd get the following;
abs(-32767) = 32767
abs(-32768) = -32768

EDIT:
What would be the correct implementation of this then?

Maximus Alexander

not1but4 wrote: »

At the risk of asking a very stupid question what is the issue with this?

Is it the case if you where to use a 16 bit integer (range being -32768 to + 32767)

You'd get the following;
abs(-32767) = 32767
abs(-32768) = -32768

No, it's that it's totally redundant.

It could be replaced with

another_var -= abs(some_var);