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
more falafel please
Feb 26, 2005

forums poster

It's better than
code:
if(something){
  DoWork();
^^ TWO loving SPACES EVEN THOUGH THE REST OF THE PROJECT USES TABS
}else{
  DoOtherWork();
}

Adbot
ADBOT LOVES YOU

That Turkey Story
Mar 30, 2003

more falafel please posted:

It's better than
code:
if(something){
  DoWork();
^^ TWO loving SPACES EVEN THOUGH THE REST OF THE PROJECT USES TABS
}else{
  DoOtherWork();
}

Holy moly fucckkkk!!!!!!!!!! Uggg I just want to kill myself! Yuck! Two loving spaces? God dammit!

poo poo...

such a nice boy
Mar 22, 2002

That Turkey Story posted:

Holy moly fucckkkk!!!!!!!!!! Uggg I just want to kill myself! Yuck! Two loving spaces? God dammit!

poo poo...

Dude, whitespace changes can be incredibly annoying, and sometimes autocorrect functions don't work well.

king_kilr
May 25, 2007

Vanadium posted:

There is at least one C-inspired language that requires that return is used like a function with parentheses and all, as far as I can tell it is mostly because the compiler guy could not be bothered to specialcase return's syntax. :shobon:

PHP?

Steampunk Mario
Aug 12, 2004

DIAGNOSIS ACQUIRED

such a nice boy posted:

Dude, whitespace changes can be incredibly annoying, and sometimes autocorrect functions don't work well.

The horror... the horror. - Col. Kurtz, ~1970

Zombywuf
Mar 29, 2008

more falafel please posted:

It's better than
code:
if(something){
  DoWork();
^^ TWO loving SPACES EVEN THOUGH THE REST OF THE PROJECT USES TABS
}else{
  DoOtherWork();
}

Sheesh, M-x indent-region and get on with your life.

pokeyman
Nov 26, 2006

That elephant ate my entire platoon.

king_kilr posted:

PHP?

code:
<?php
function foo() {
    return 2;
}

echo foo();
?>
code:
2
Keep guessing.

ashgromnies
Jun 19, 2004

Triple Tech posted:

These things, as Perl programmer, piss me off:
code:
@stack = ();
for ($i = 0; $i < scalar @elements; $i++) {
  $temporary = do_something_to $elements[$i];

  push @stack, $temporary;
}

See, I wrote

code:

map( do_something_to $_, @elements );

and someone bitched at me and called it unreadable and obfuscated and "an improper use of map".

Triple Tech
Jul 28, 2006

So what, are you quitting to join Homo Explosion?
As written, that is an improper use of map, philosophically.
code:
# reassignment
munge_and_set($_) for @elements;

# side-effect free
my @new_elements = map { munge($_) } @elements;

# side-effect free in void context
map { munge($_) } @elements;

# works but is bad form, equivalent to first example
map { munge_and_set($_) } @elements;
Edit: The philosophical part here is... When you use for, you're addressing a copy of each element and doing something with it in an iterative context. When you use map, you're multiplying each element functionally. The return value could be more than one element, the same element, a different element, or an empty list, for each element. And that gets returned. Traditionally inside of a map or a for, you don't want to be reassigning values. If you want, you can assign the list the value of the same list mapped over.
code:
# be reborn, list!
@list = map { transform($_) } @list;

Triple Tech fucked around with this message at 14:58 on Apr 4, 2008

ashgromnies
Jun 19, 2004

Triple Tech posted:

As written, that is an improper use of map, philosophically.
code:
# reassignment
munge_and_set($_) for @elements;

# side-effect free
my @new_elements = map { munge($_) } @elements;

# side-effect free in void context
map { munge($_) } @elements;

# works but is bad form, equivalent to first example
map { munge_and_set($_) } @elements;
Edit: The philosophical part here is... When you use for, you're addressing a copy of each element and doing something with it in an iterative context. When you use map, you're multiplying each element functionally. The return value could be more than one element, the same element, a different element, or an empty list, for each element. And that gets returned. Traditionally inside of a map or a for, you don't want to be reassigning values. If you want, you can assign the list the value of the same list mapped over.
code:
# be reborn, list!
@list = map { transform($_) } @list;

Hmm, maybe I'm misreading this.

What about

code:
this_returns_an_array($_) for @elements;

map { this_returns_an_array($_) } @elements;
Will the first not work?

Triple Tech
Jul 28, 2006

So what, are you quitting to join Homo Explosion?
If this_returns_an_array contains no side effects to the variables submitted, then both of those do nothing. They're void context. In the first one, the fact that it returns something doesn't even matter. You can't capture the output of a loop when it's written like that. In the second scenario, you're just missing the left side of the assignment. my @array = that expression. It's returning an array for each element in the elements list, and then jamming all of that into one long array, and then that array isn't going anywhere. Again, void context.

For loops never get assigned to something. Maps should always be either assigned to something or chained to another process, like a function's input, or the list input to a sort or grep.

more falafel please
Feb 26, 2005

forums poster

Zombywuf posted:

Sheesh, M-x indent-region and get on with your life.

That's the problem -- I'm stuck doing that (well, gg=G, but same difference). This code is five years old, and full of crap like this -- defining log macros badly so they need double parentheses, bizarro mixes of Systems Hungarian, CamelCase, camelCase, and under_score_words, depending on this guy's mood that month, static members called m_ivpFoo (instance variable). And of course design problems -- the initializer list for one class's ctor is 124 lines long.

I've asked him if he's going to fix any of it, but he says it'd be too much effort when he could just rewrite it.

The problem of course is that there's never time -- it's always crunch.

Zombywuf
Mar 29, 2008

more falafel please posted:

That's the problem -- I'm stuck doing that (well, gg=G, but same difference). This code is five years old, and full of crap like this -- defining log macros badly so they need double parentheses, bizarro mixes of Systems Hungarian, CamelCase, camelCase, and under_score_words, depending on this guy's mood that month, static members called m_ivpFoo (instance variable). And of course design problems -- the initializer list for one class's ctor is 124 lines long.

Do you not have check-in access? Or is this one of those case where source control would be too large a change?

quote:

The problem of course is that there's never time -- it's always crunch.

Always the way.

Randomosity
Sep 21, 2003
My stalker WAS watching me...
Holy poo poo.

I've seen some awful code on my job... but THIS has to be the worst thing I've ever seen.

The function is used to create a series of dropdown boxes to select month, day, year, hour, and minute.

php:
<?
function displayHTMLMenu( $type, $name, $class, $value_digits, $value_selected, $value_top_name, $value_top_value, $on_change_function_name, $return_or_echo_content = "echo" )
{
    //global $CM_app_system_path;

    //    get menu data START
    if ($type == "month_names")
    {
        if( $_SESSION["sessionStat"]["lowresolution"] )
        {
            $nxtFile = LIST_DATA_FILES_PATH . "list_date_month_names_abbrv.txt";
        }
        else
        {
            $nxtFile = LIST_DATA_FILES_PATH . "list_date_month_names.txt";
        }
    }
    else if ($type == "day_numbers")
    {
        $nxtFile = LIST_DATA_FILES_PATH . "list_date_day_numbers.txt";
    }
    else if ($type == "year_numbers")
    {
        $nxtFile = LIST_DATA_FILES_PATH . "list_date_year_numbers.txt";
    }
    else if ($type == "year_numbers_current_and_previous_in_january")
    {
        // no file for year_numbers_current_and_previous_in_january, generate dynamically
        $all_data_array[] = array (date("Y"), date("Y"), date("Y"));
        
        if (date("M") == "Jan")
        {
            $all_data_array[] = array (date("Y")-1, date("Y")-1, date("Y")-1);
        }
    }
    else if ($type == "hour_numbers")
    {
        $nxtFile = LIST_DATA_FILES_PATH . "list_date_hour_numbers.txt";
    }
    else if ($type == "minute_numbers")
    {
        $nxtFile = LIST_DATA_FILES_PATH . "list_date_minute_numbers.txt";
    }
    else    // default
    {
        $nxtFile = LIST_DATA_FILES_PATH . "list_date_minute_numbers.txt";
    }

    if( isset( $nxtFile ) && file_exists( $nxtFile ) )
    {
        $fd = fopen ($nxtFile, "r") or die ("Could not open file");
        flock ($fd, 2);
        while (!feof($fd))
        {
            $buffer = fgets ($fd, 4096);
            $next_line_array = split("\|", $buffer);
            $all_data_array[] = $next_line_array;
            //echo $all_data_array[0];
        }
        flock ($fd, 3);
        fclose($fd);
    }
    else
    {
        // file not found
        //echo "<h1>file not found</h1>";
    }
    //    get menu data END

    $content_to_return = "<select name=\"$name\" class=\"$class\" onchange=\"$on_change_function_name(this);\">";
    
    if (!empty ($value_top_name) || !empty ($value_top_value))
    {
        $content_to_return .= "<option value=\"$value_top_value\">$value_top_name";
    }

    foreach ($all_data_array as $next_data_array)
    {
        //echo $next_data_array[0];
        if ($next_data_array[0] != "")
        {
            //    get next value START
            if ($value_digits == 1)    // no leading zeros
            {
                $next_value = $next_data_array[1];
            }
            else if ($value_digits == 2)    // leading zeros
            {
                $next_value = $next_data_array[2];
            }
            else
            {
                $next_value = $next_data_array[2];    // leading zeros, default
            }
            //    get next value END

            if ($next_value == $value_selected)
            {
                $content_to_return .= "<option value=\"$next_value\" selected>$next_data_array[0]";
            }
            else
            {
                $content_to_return .= "<option value=\"$next_value\">$next_data_array[0]";
            }
        }
    }

    $content_to_return .= "</select>";
    
    if( $return_or_echo_content == "return" )
    {
        return $content_to_return;
    }
    else
    {
        echo $content_to_return;
    }
?>
:froggonk:

ashgromnies
Jun 19, 2004

Triple Tech posted:

If this_returns_an_array contains no side effects to the variables submitted, then both of those do nothing. They're void context. In the first one, the fact that it returns something doesn't even matter. You can't capture the output of a loop when it's written like that. In the second scenario, you're just missing the left side of the assignment. my @array = that expression. It's returning an array for each element in the elements list, and then jamming all of that into one long array, and then that array isn't going anywhere. Again, void context.

For loops never get assigned to something. Maps should always be either assigned to something or chained to another process, like a function's input, or the list input to a sort or grep.

Yea, I wrote it wrong :( I meant that they did have side effects on the variable being passed in.

chocojosh
Jun 9, 2007

D00D.
Usercontrol DateLookupWindow. It contains a textbox with a hyperlink to a calendar window so that we can pick a date and have it auto populate the textbox with the date (the user can also manually type in a date into the textbox if they want).

DateLookupWindow has a property called Date, whose type is a string. WHY? Either call it Text (to indicate the text of the checkbox) or let it be a Date and in the getter/setter handle the appropriate behaviour.

dwazegek
Feb 11, 2005

WE CAN USE THIS :byodood:
(C#) I'd just like to know, am I an idiot for considering adding extension methods for stuff like string.IsNullOrEmpty() and Object.ReferenceEquals because I prefer this syntax:
code:
string s = ...
if(s.IsNullOrEmpty())
{
   //
}
over this syntax:
code:
string s = ...
if(string.IsNullOrEmpty(s))
{
   //
}

_aaron
Jul 24, 2007
The underscore is silent.
Yeah, you're probably an idiot, because won't s.IsNullOrEmpty() throw an exception is s is null?

edit: Well gently caress me, this is totally incorrect. I just tested it out quick, and this does not throw an exception at all. I'll leave it up in case anyone wants to laugh at me :smith:.

_aaron fucked around with this message at 18:54 on Apr 7, 2008

dwazegek
Feb 11, 2005

WE CAN USE THIS :byodood:

_aaron posted:

Yeah, you're probably an idiot, because won't s.IsNullOrEmpty() throw an exception is s is null?
No, extension methods are pretty much just syntactic sugar for a call to a method in a referenced static class.
code:
public static class SomeClass
{
  public static bool IsNullOrEmpty(this string s)
  {
    return string.IsNullOrEmpty(s);
  }
}

//these two calls are the same:
s.IsStringNullOrEmpty();
SomeClass.IsNullOrEmpty(s);
Since you're not actually calling an instance method of "s", but actually passing it as the first parameter to an extension method, it doesn't matter if it's null or not.

SLOSifl
Aug 10, 2002


It certainly goes against expected behavior if you think about it, but I find myself typing s.IsNullOrEmpty() all the time anyway.

nebby
Dec 21, 2000
resident mog
For what it's worth, the "IsNullOrEmpty" phrase is pretty cumbersome, I much prefer Ruby's "blank?" which implies it is either null or empty (this applies to both arrays and strings.)

Victor
Jun 18, 2004
I general, I'd keep away from stuff like s.IsNullOrEmpty(), but I think I would make an except for that specific case. Especially since the name itself acknowledges that null is a valid "argument".

Zombywuf
Mar 29, 2008

dwazegek posted:

No, extension methods are pretty much just syntactic sugar for a call to a method in a referenced static class.
code:
public static class SomeClass
{
  public static bool IsNullOrEmpty(this string s)
  {
    return string.IsNullOrEmpty(s);
  }
}

//these two calls are the same:
s.IsStringNullOrEmpty();
SomeClass.IsNullOrEmpty(s);
Since you're not actually calling an instance method of "s", but actually passing it as the first parameter to an extension method, it doesn't matter if it's null or not.

Yuck. I'd like to plead on behalf of all maintenance programmers everywhere that you don't do that.

Also, why should it not be possible to call a member of a null object?
code:
#include <iostream>

class foo {
public:
  bool is_null() {
    return this == 0;
  }
};

int main() {
  foo *a = 0;
  std::cout << a->is_null() << std::endl;
}
Works fine in C++. This works for exactly the same reason as the above D flat code, but you at least know where to find is_null in this case.

That Turkey Story
Mar 30, 2003

Zombywuf posted:

Yuck. I'd like to plead on behalf of all maintenance programmers everywhere that you don't do that.

Also, why should it not be possible to call a member of a null object?
code:
#include <iostream>

class foo {
public:
  bool is_null() {
    return this == 0;
  }
};

int main() {
  foo *a = 0;
  std::cout << a->is_null() << std::endl;
}
Works fine in C++. This works for exactly the same reason as the above D flat code, but you at least know where to find is_null in this case.

No it doesn't. That code has undefined behavior. It is never safe to invoke a non-static member function through a null pointer.

Zombywuf
Mar 29, 2008

Really, even POD?

That Turkey Story
Mar 30, 2003

Yeah, even PODS.

Zombywuf
Mar 29, 2008

Ah well, good job this is the coding horrors thread. I'd be interested to know if any compilers actually choke on it. Especially is foo::is_null is defined in an external compilation unit.

more falafel please
Feb 26, 2005

forums poster

That Turkey Story posted:

No it doesn't. That code has undefined behavior. It is never safe to invoke a non-static member function through a null pointer.

It may be undefined behavior because the underlying implementation is unspecified, but what the compiler's going to generate is this (using C as portable assembly):
code:
struct foo;

bool foo_is_null(foo* this) {
    return this == 0;
}

int main() {
  foo *a = 0;
  printf("%d\n", foo_is_null(a));
}
I understand why "undefined behavior" generally means no-no, but in cases like this it doesn't matter what the standard says to some degree, because it works with the only way you would implement POD types.

skeevy achievements
Feb 25, 2008

by merry exmarx
The Java time/date/calendar and file IO libraries.

That Turkey Story
Mar 30, 2003

more falafel please posted:

I understand why "undefined behavior" generally means no-no, but in cases like this it doesn't matter what the standard says to some degree, because it works with the only way you would implement POD types.

Undefined behavior isn't "generally a no-no" it's always a no-no. It does matter what the standard says. We all know what the probable implementation will do, but that doesn't mean you should ever rely on it, especially here since there's no reason to.

Mustach
Mar 2, 2003

In this long line, there's been some real strange genes. You've got 'em all, with some extras thrown in.

Zombywuf posted:

Also, why should it not be possible to call a member of a null object?
Because 99.9999999999999999999% of legitimate member functions will try to do something with what this points to.

SuperGoon
Nov 13, 2003
Here is a gem I uncovered today in a code review. This was written by an H-1B contractor. He has a masters degree and makes more than me. :[

code:
String s = httpServletRequest.getParameter("something");

if(s.trim().length() == 0 || s.equals(null)) {
    //do something
}
sigh...

noonches
Dec 5, 2005

It represents my lifestyle and stratus as a street savy irreverent youth, who lives large, and yet hungers for the next level in life.
code:
<select ="prefix">
    <option value="<?=$_SESSION['prefix']?>"><?=$_SESSION['prefix']?></option>
    <option value="mr">mr</option>
    <option value="mrs">mrs</option>
    <option value="ms">ms</option>
    <option value="dr">dr</option>
</select>
So I guess rather than just adding "selected" with like a tiny piece of code, it's trying to add the option you had previously selected to the top so it defaults to the selected one.

It's not that bad, it's just irritatingly lazy.

pokeyman
Nov 26, 2006

That elephant ate my entire platoon.

noonches posted:

code:
<select ="prefix">
    <option value="<?=$_SESSION['prefix']?>"><?=$_SESSION['prefix']?></option>
    <option value="mr">mr</option>
    <option value="mrs">mrs</option>
    <option value="ms">ms</option>
    <option value="dr">dr</option>
</select>
So I guess rather than just adding "selected" with like a tiny piece of code, it's trying to add the option you had previously selected to the top so it defaults to the selected one.

It's not that bad, it's just irritatingly lazy.

That is pretty sad, especially since it'll have duplicates of whatever value's thrown in. However, what's this "tiny piece of code" you'd use to add "selected"? It's as long as that if I recall correctly.

noonches
Dec 5, 2005

It represents my lifestyle and stratus as a street savy irreverent youth, who lives large, and yet hungers for the next level in life.
<?=($_SESSION['prefix']=="")?"selected":""?>

One of those per line is not that much, esp since it's a cut and paste job and avoids a nasty dupliacte.

cowboy beepboop
Feb 24, 2001

SuperGoon posted:

Here is a gem I uncovered today in a code review. This was written by an H-1B contractor. He has a masters degree and makes more than me. :[

code:
String s = httpServletRequest.getParameter("something");

if(s.trim().length() == 0 || s.equals(null)) {
    //do something
}
sigh...

Can you explain why it's stupid? Looks okay to me :haw:

floWenoL
Oct 23, 2002

my stepdads beer posted:

Can you explain why it's stupid? Looks okay to me :haw:

What happens when s is null?

cowboy beepboop
Feb 24, 2001

floWenoL posted:

What happens when s is null?

Haha oh I see.

Victor
Jun 18, 2004

floWenoL posted:

What happens when s is null?
The earth implodes.
code:
catch (NullReferenceException)
{
    Planets.Earth.FormBlackHole();
}

Adbot
ADBOT LOVES YOU

Mikey-San
Nov 3, 2005

I'm Edith Head!
meh

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