The Weird and The Wonderful

Well... Actually I (the new developer) has to pay Frown | :(

:P

Hopefully the programmer was so called "Senior". That's a tough call for a junior programmer.

The root of the problem is that it's done in VB...

".45 ACP - because shooting twice is just silly" - JSOP, 2010
-----
You can never have too much ammo - unless you're swimming, or on fire. - JSOP, 2010
-----
"Why don't you tie a kerosene-soaked rag around your ankles so the ants won't climb up and eat your candy ass." - Dale Earnhardt, 1997

Found this in the daily WTF.

C#

private IAlertDocument CompareObjectAsIAlertDocumentOrNullIfNotCastable(object compareObject) {
    return compareObject as IAlertDocument;
}

What do you think? I can follow this pattern:

C#

string GetAStringThatRepresentsTheObjectPassedAsAnObjectParameterSoItWorksForAnyObject(object obj)
{
    return obj.ToString();
}

_Erik_ wrote:
string GetAStringThatRepresentsTheObjectPassedAsAnObjectParameterSoItWorksForAnyObject(object obj)
{
return obj.ToString();
}

You forgot part of the method name...
It should be... GetAStringThatRepresentsTheObjectPassedAsAnObjectParameterSoItWorksForAnyObjectAsLongAsTheObjectIsNotNull

I wasn't, now I am, then I won't be anymore.

Damn it!! I thought I could follow the pattern!! Thumbs Up | :thumbsup:

Throw new UserFellAsleepAtTheKeyboardException(me)

Dwayne J. Baldwin

Before I get started, some background:
gr1d[^] is a (very new) persistent multiplayer programming AI game: the idea is you code your agents and upload them, and let them do their thing. (Check it out, if you haven't!)

Now, with these agents, they have to inherit an interface. For brevity, I'll reduce it to the relevant details:

Public Class πRateSapient
Implements gr1d.API.Agent.IPirate1

...

Public Sub Tick(ByVal agentUpdate As Gr1d.Api.Agent.IAgentUpdateInfo) Implements ...
 ' Logic here
End Sub

(VB is NOT the code horror, thank you very much! :P )

Now, in my infinite wisdom, I started some abstractions. This included building it so all AIs under that schema (my search/claim/don't go picking fights) operated under the same logic.
How'd I do this? Well:

Public Module Logic

<Extension()>
Public Sub Tick(ByVal agent As πRateSapient, ByVal update As Agent.IAgentUpdateInfo)
 ' Logic here
End Sub

...
Can you see where this is going? I didn't.

Public Class πRateSapient
Implements gr1d.API.Agent.IPirate1

...

Public Sub Tick(ByVal agentUpdate As Gr1d.Api.Agent.IAgentUpdateInfo) Implements ...
 Me.Tick(agentUpdate)
End Sub

...
No debugging, no thought put into it, I just thought it'd call the extension method.
Whoops. And yeah, it was sloooow.

Moral of the story folks: think about your damn naming conventions, especially if you're using extension methods!!!
D'Oh! | :doh:

(And I was wondering why my agents did nothing!)

(Also, may I mention that running the game logic, and the site, on the same instance is also a bit of a horror?)

Hope you laughed. Laugh | :laugh:

[edit]
To be fair, I wasn't the only one with a recursive loop. But I sure as hell didn't help. xD
[/edit]

Don't forget to rate my post if it helped! Wink | ;)

"He has no enemies, but is intensely disliked by his friends."
"His mother should have thrown him away, and kept the stork."
"There's nothing wrong with you that reincarnation won't cure."
"He loves nature, in spite of what it did to him."

modified on Wednesday, December 22, 2010 6:05 AM

Note that extension methods with the same name and signature as an interface or class method will never be called.

http://msdn.microsoft.com/en-us/library/bb383977.aspx[^]

Dwayne J. Baldwin

Kinda learned that one, didn't I? :P

Don't forget to rate my post if it helped! Wink | ;)

"He has no enemies, but is intensely disliked by his friends."
"His mother should have thrown him away, and kept the stork."
"There's nothing wrong with you that reincarnation won't cure."
"He loves nature, in spite of what it did to him."

Last time I tried to even check the site out, it was at a crawl.

Not a new horror, nonetheless something tells me this If statement may be redundant...

fileName = ""

If Len(fileName) = 0 Then
    fileName = Left(DBEngine.Workspaces(0).Databases(0).Name, i) & "REF"
End If

Even the fileName = "" is redundant here.

Hmmm, tricky.

Give me a few hours and I'll get back to you on that one...

8)

Well, stating the obvious, it's quite likely that, either the coder didn't want to rely on code never being inserted between the initializer and guard, or, more likely, the coder added the initializer later, probably to test the guard.

Hey, you never know...

There may have been code between the initialiser and the if statement that has since been removed.

It might actually be a better piece of coding than you think. You always have the option of inserting some more code between the two and it might stand a chance of still working. Wink | ;)

True, I would agree with you... He is testing for null. Having it in there is better than not having it in there.

"Program testing can be used to show the presence of bugs, but never to show their absence."

<< please vote!! >>

So the if statement is redundent...
Maybe the dude has got a resentment towards some tester maybe?
fileName is obviously a string which is a referenced type. If it wasn't initialized could of contained some garbage and length would of therefor been > 0.
However this is not the case... It is initialised and therefor redundent.

"Program testing can be used to show the presence of bugs, but never to show their absence."

<< please vote!! >>

I posted an example like this in the lounge a few months ago...

also found ASP code like this...

for each element in Request.form
 if request.form("specificElement") <> "" then
  someVar = request.form("specificElement")
  ...
  [do some other stuff]
 end if
next

It hurts both my eyes and my brain Frown | :(

We do not see the scope of fileName. Maybe another thread can change it's value Smile | :)

then the code would be incorrect even with the test for zero length

incorrect - yes, I'm not talking about sync-ing threads. My point was in that case code would not be "Testing the obvious..."

.. ...

C#

try
{
     retry:
     while (filename = EmptyString)
     {
            if(Len(fileName) == 0)
            {
                 fileName = Left(DBEngine.Workspaces(0).Databases(0).Name, i) & "REF"
            }
    }
}
catch
{ 
    goto retry;
}
filename = "";

.... just trying to see how many people will complain about this.... Poke tongue | ;-P

I'd blame it on the Brain farts.. But let's be honest, it really is more like a Methane factory between my ears some days then it is anything else...

-----

"The conversations he was having with himself were becoming ominous."-.. On the radio...

ugh. if only we all understood how static works... and perhaps naming conventions... hmm, i guess understanding when to apply the word "static" to a class might also be helpful... i don't even care about the bad use of List<SqlParameter>.

Oh and don't worry, there were comments for the functions, the class, the property, and of course i didn't remove the one inside the constructor...

public class IDataAccesslayer
{
    private static SqlConnection conn = new SqlConnection(WebConfigurationManager.ConnectionStrings["connStr"].ToString());

    public IDataAccesslayer()
    {
        //
        // TODO: Add constructor logic here
        //
    }

    public static void ExecuteNoReturn(String spName, List<SqlParameter> parms)
    {
        try
        {
            conn.Open();

            SqlCommand cmd = new SqlCommand(spName, conn);
            cmd.CommandType = CommandType.StoredProcedure;

            foreach (SqlParameter parm in parms)
            {
                cmd.Parameters.Add(parm);
            }

            cmd.ExecuteNonQuery();
        }
        catch (Exception ex)
        {
            throw (ex);
        }
        finally
        {
            if (conn.State == ConnectionState.Open)
                conn.Close();
        }
    }
    public static DataSet ExecuteReturnDataSet(String spName, List<SqlParameter> parms)
    {
        //this was same as previous, except using a data-adapter
    }
}

Don't forget the abomination that is

throw (ex);