If you have a new account but are having problems posting or verifying your account, please email us on hello@boards.ie for help. Thanks :)

Hello all! Please ensure that you are posting a new thread or question in the appropriate forum. The Feedback forum is overwhelmed with questions that are having to be moved elsewhere. If you need help to verify your account contact hello@boards.ie

Coding Horror

Evil Phil · 2009-09-18 10:18:29

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…

Options

Comments

Options
#452 12-01-2015 10:55am
oscarBravo

Technology & Internet Moderators Posts: 28,792 Mod ✭✭✭✭

Join Date: May 2001

Posts: 26912

0
Options
#453 12-01-2015 10:21pm
Colonel Panic

Registered Users Posts: 2,018 ✭✭✭

Join Date: August 2011

Posts: 1985

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

0
Options
#454 12-01-2015 10:38pm
Anesthetize

Registered Users Posts: 1,461 ✭✭✭

Join Date: December 2014

Posts: 1447
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'

0
Options
#455 13-01-2015 12:06am
IRLConor

Subscribers Posts: 4,075 ✭✭✭

Join Date: April 2007

Posts: 3923

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.

0
Options
#456 13-01-2015 12:09am
Maximus Alexander

Registered Users Posts: 13,080 ✭✭✭✭

Join Date: July 2011

Posts: 13042

Anesthetize wrote: »

That's a paddlin'

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

0
Advertisement
Options
#457 13-01-2015 10:27am
Aswerty

Registered Users Posts: 586 ✭✭✭

Join Date: September 2010

Posts: 568

Colonel Panic wrote: »

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

Subjectively; the former. Objectively; the latter.

0
Options
#458 13-01-2015 10:30am
Calina

Registered Users Posts: 8,219 ✭✭✭

Join Date: December 2004

Posts: 7725

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.

0

Options

#459 13-01-2015 2:48pm

Buford T Justice

Registered Users Posts: 6,250 ✭✭✭

Join Date: December 2009

Posts: 5961

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

Options

#460 13-01-2015 2:52pm

irishgeo

Registered Users Posts: 9,386 ✭✭✭

Join Date: March 2001

Posts: 8777

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

Options
#461 13-01-2015 9:44pm
Maximus Alexander

Registered Users Posts: 13,080 ✭✭✭✭

Join Date: July 2011

Posts: 13042

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.

0
Advertisement
Options
#462 15-01-2015 10:29am
L.Jenkins

Moderators, Computer Games Moderators, Technology & Internet Moderators Posts: 19,240 Mod ✭✭✭✭

Join Date: May 2009

Posts: 18486
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

0
Options
#463 15-01-2015 11:38pm
Buford T Justice

Registered Users Posts: 6,250 ✭✭✭

Join Date: December 2009

Posts: 5961

Itzy wrote: »

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

0

Options

#464 23-01-2015 5:13am

punk_one82

Registered Users Posts: 1,148 ✭✭✭

Join Date: January 2006

Posts: 1102

/**
     * 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.

Options
#465 25-01-2015 6:49pm
L.Jenkins

Moderators, Computer Games Moderators, Technology & Internet Moderators Posts: 19,240 Mod ✭✭✭✭

Join Date: May 2009

Posts: 18486
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.

0
Options
#466 27-01-2015 12:49am
punk_one82

Registered Users Posts: 1,148 ✭✭✭

Join Date: January 2006

Posts: 1102

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.

0
Options
#467 30-01-2015 7:01pm
Capt'n Midnight

Moderators, Recreation & Hobbies Moderators, Science, Health & Environment Moderators, Technology & Internet Moderators Posts: 90,802 Mod ✭✭✭✭

Join Date: March 2003

Posts: 89042

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

0
Options
#468 30-01-2015 7:54pm
oscarBravo

Technology & Internet Moderators Posts: 28,792 Mod ✭✭✭✭

Join Date: May 2001

Posts: 26912

Oh.

My.

Teapot.

:eek:

0
Options
#469 30-01-2015 9:37pm
BrokenArrows

Registered Users Posts: 7,500 ✭✭✭

Join Date: February 2005

Posts: 6935

Surely noone can be that stupid.

0
Options
#470 30-01-2015 10:02pm
[Rasta]

Registered Users Posts: 428 ✭✭

Join Date: February 2008

Posts: 426

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.

0
Options
#471 06-02-2015 10:15am
L.Jenkins

Moderators, Computer Games Moderators, Technology & Internet Moderators Posts: 19,240 Mod ✭✭✭✭

Join Date: May 2009

Posts: 18486

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.

0
Advertisement
Options
#472 16-03-2015 9:30pm
Capt'n Midnight

Moderators, Recreation & Hobbies Moderators, Science, Health & Environment Moderators, Technology & Internet Moderators Posts: 90,802 Mod ✭✭✭✭

Join Date: March 2003

Posts: 89042

Panda antivirus software labels itself as malware

0
Options
#473 17-03-2015 9:19pm
L.Jenkins

Moderators, Computer Games Moderators, Technology & Internet Moderators Posts: 19,240 Mod ✭✭✭✭

Join Date: May 2009

Posts: 18486

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.

0
Options
#474 05-04-2015 7:24pm
FobleAsNuck

Registered Users Posts: 793 ✭✭✭

Join Date: June 2014

Posts: 717

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

0
Options
#475 07-04-2015 9:49pm
Pov06

Closed Accounts Posts: 1,487 ✭✭✭

Join Date: July 2009

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

Kill me now :eek:

0
Options
#476 07-04-2015 9:58pm
Boskowski

Closed Accounts Posts: 5,361 ✭✭✭

Join Date: November 2007

Posts: 5269
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.

0
Options
#477 07-04-2015 10:48pm
brevity

Registered Users Posts: 8,496 ✭✭✭

Join Date: October 2013

Posts: 8446

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

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

0
Options
#478 09-04-2015 2:02pm
Pov06

Closed Accounts Posts: 1,487 ✭✭✭

Join Date: July 2009

Posts: 1424

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.

0
Options
#479 09-04-2015 2:22pm
Boskowski

Closed Accounts Posts: 5,361 ✭✭✭

Join Date: November 2007

Posts: 5269

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.

0
Options
#480 10-04-2015 11:20am
not1but4

Registered Users Posts: 262 ✭✭

Join Date: August 2010

Posts: 213
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?

0
Advertisement
Options
#481 10-04-2015 11:25am
Maximus Alexander

Registered Users Posts: 13,080 ✭✭✭✭

Join Date: July 2011

Posts: 13042
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);

0