Chubertdev's snippet thread



  • I've probably posted a hundred snippets over in the status thread, so now I'm just going to create my own thread, and put them here.

    First up (VB .NET):

    Dim bIsPostBack As Boolean = False
    InsertRecords(iSomethingID, iSomethingElseID, bIsPostBack)
    


  • @chubertdev said:

    InsertRecords

    bIsPostBack is a ByRef? Your shitty prototype lacks so many details!



  • Ok, here's more:

    Private Sub Page_Load(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles MyBase.Load
       If Not Page.IsPostBack Then
          If SomeOtherConditional Then
             Dim bIsPostBack As Boolean = False
             InsertRecords(iSomethingID, iSomethingElseID, bIsPostBack)
          End If
       End If
    End Sub
    


  • But the third argument could still be some sort of ByRef arg where you can't just pass true/false! And what if you want to make your page.ispostback variable doesn't get modified?



  • @delfinom said:

    But the third argument could still be some sort of ByRef arg where you can't just pass true/false! And what if you want to make your page.ispostback variable doesn't get modified?

    Yeah, it's ByVal.

    And Page.IsPostBack has definitely changed, we observed it!



  • But what if it has multiple quantum states?



  • @delfinom said:

    But what if it has multiple quantum states?

    It becomes a cat.



  • This is going to end well. This is what happens when C# developers work in VB .NET

    Try
       If iClientID > 0 Then
          sClientNumber = IIf(Not IsDBNull(.Rows(0).Item("ClientNumber")), .Rows(0).Item("ClientNumber"), "")
          ...
          Return True
       Else
          Return False
       End If
    Catch ex As Exception
       Return False
    End Try
    


  • Dear C# .NET developers: don't assume that you understand how VB .NET works:

    If sDate <> Nothing Then

    (yes, sDate is a String)



  • Uggh, I hate this pattern:

    Dim bVar As Boolean = IIf(dr.Item("FieldName").ToString <> "", True, False)



  • owies.
    So they're the kind of people who write dr.Item("FieldName").ToString() != "" ? true : false?



  • @aliceif said:

    owies.
    So they're the kind of people who write dr.Item("FieldName").ToString() != "" ? true : false?

    Makes me wonder.



  • Crap.

    Searching for: true : false
    Found 572 occurrence(s) in 87 file(s)
    
    Searching for: false : true
    Found 21 occurrence(s) in 21 file(s)
    


  • I should set up rules to make this a compiler error:

    Dim iField As Integer = dr.Item("FieldName").ToString()

    This is a VB .NET project created in 2002, worked on by many people that are not familiar with .NET, and those that are, only worked in C#.



  • Bwahahahahhaha

            Public Property SystemID() As Integer
                Get
                    Return iSystemID
                End Get
                Set(ByVal Value As Integer)
                    iSystemID = Value
                    iSystem = Value
                    Select Case Value
                        Case 2
                            OtherVar &= "some unrelated stuff here"
                    End Select
                End Set
            End Property
    


  • @chubertdev said:

    This is a VB .NET project created in 2002

    Ouch. I'm assuming a lot of what you're finding are tidbits that are either:

    • C# developers not understanding VB or knowing the syntax in VB for what they'd do in C#
    • iffy designs due to being written in VB.NET in the 1.0/1.1 days, that haven't been modified since (hopefully) moving to newer .NET Framework versions

    Or some healthy mix of both.


  • Discourse touched me in a no-no place

    @ChaosTheEternal said:

    Or some unhealthy mix of both.

    FTFY



  • @ChaosTheEternal said:

    Ouch. I'm assuming a lot of what you're finding are tidbits that are either:

    • C# developers not understanding VB or knowing the syntax in VB for what they'd do in C#
    • iffy designs due to being written in VB.NET in the 1.0/1.1 days, that haven't been modified since (hopefully) moving to newer .NET Framework versions

    Or some healthy mix of both.

    That, plus a whole lot more. This app has been 2.0 for the longest while. It was recently converted to 4.0.

    So I see (current) co-workers that added code while it was 2.0 like this:

    Dim var As String = IIf(IsDBNull(dr.Item("FieldName")), "", dr.Item("FieldName").ToString)

    Which fails, but can easily be fixed in 4.0:

    Dim var As String = If(IsDBNull(dr.Item("FieldName")), "", dr.Item("FieldName").ToString)



  • sigh

                        Public Property Billed() As DateTime
                            Get
                                Return dBilled
                            End Get
                            Set(ByVal Value As DateTime)
                            End Set
                        End Property
    


  • Yeah, I was considering asking about the framework version it started in because you kept posting code with IIf in it.

    Was a bit of a hint when you posted the String <> Nothing one that it wasn't 2.0 or up, but I've seen that from people who never touched 1.1.



  • @ChaosTheEternal said:

    Yeah, I was considering asking about the framework version it started in because you kept posting code with IIf in it.

    Was a bit of a hint when you posted the String &lt;&gt; Nothing one that it wasn't 2.0 or up, but I've seen that from people who never touched 1.1.

    "Convert to 4.0" at my company:

    1. change .NET version in IIS
    2. try to compile
    3. fix what breaks
    4. converted!


  • Oh I've been having some fun of porting something that was written in .NET 1.1 (and is still running) to .NET 4.0.

    I'm, of course, doing more improvements as I upgrade besides making sure it just compiles, but it's on standby for now so I'm not working on it.

    At least in my case with upgrading this, it wasn't built by people who preferred C# and couldn't grok VB.NET who wrote it, so what I'm really upgrading is just things that didn't exist in .NET 1.1 (like If something Then variable = this Else variable = that End If to variable = If(something, this, that).



  • Love it:

    If bLastRecord = True Then
       objInstance.LastRecord = "True"
    End If
    

    Does this annoy anyone else?

    Dim strVal As String = If(Not bConditional, "true string", "false string")



  • @chubertdev said:

    Does this annoy anyone else?

    If by 'annoy' you mean 'amuse' then yes, keep them coming.



  • Shouldn't that be

    objInstance.LastRecord = IIf(bLastRecord <> True, objInstance.LastRecord, "True")
    

    ?



  • gaaaaaah

    If iRecordTypeID <> 1 And iRecordTypeID <> 3 And iRecordTypeID <> 6 And iRecordTypeID <> 9 Then
       ...
    Else
       ...
    End If
    


  • Nice.

    dtClientList = New DataTable
    dtClientList = dsClients.Tables(0)
    


  • Ignoring what that code says about the technical debt in the database and separation of concerns, etc., I wouldn't say it's all that bad. At least the writers can be trusted to understand it, which might not be true of

    If Array.IndexOf({1, 3, 6, 9}, iRecordTypeID) = -1 Then
    

    or

    Select Case iRecordTypeID
    	Case 1, 3, 6, 9
    	Case Else


  • Yeah, those IDs truly are magic. It's an ID for a lookup table that has identity insert off, so we manually add new record types, but it's not any better. I'd be tempted to make an enum for it, then write a select case for that (similar to your second example).


  • Discourse touched me in a no-no place

    • It's a spare
    • He's...what's the GC equivalent of pre-rolling dice?


  • @chubertdev said:

    It's an ID for a lookup table that has identity insert off, so we manually add new record types, but it's not any better. I'd be tempted to make an enum for it

    I've got to say creating a lookup table of IDs that need to be included in one branch and excluded in the other would be my preferred approach (choosing a sensible name for it might be difficult). Hard wiring the IDs into the code at all is not great. Perhaps you're getting desensitized to the WTFs.


  • Discourse touched me in a no-no place

    @LurkerAbove said:

    Perhaps you're getting desensitized to the WTFs.

    Oooh, fightin' words!



  • @FrostCat said:

    Oooh, fightin' words!

    Is there a badge for that?


  • FoxDev

    @chubertdev said:

    If bLastRecord = True Then
    objInstance.LastRecord = "True"
    End If

    well if it's legal for that LastRecored value to previously be True and i just has to be set if not already ser whe bLastRecord is true....

    i mean that seems unlikely but i've heard of crazier business rules.



  • No, I just have limited power to tear this application apart, which is what's needed.



  • Backfilling from the status thread


    Dealing with this type of code.

    ALTER PROCEDURE [dbo].[ProcName]
       @UserID INT,
       @RecordID INT,
       @ViewType INT,
       @Skip INT,
       @IsUserType1 BIT,
       @IsUserType2 BIT
    AS
       ...
    

    Current status: dealing with crap like this:

    Error Message: Redirect URI cannot contain newline characters.

    Seriously:

    httpResponse.Redirect("Error.aspx?Msg=" + ex.Message);


    Current status: wrapping my head around this:

    Convert.ToDateTime(System.DateTime.Now).ToString("yyyyMMdd")


    Current status: files moved, looking at more code:

            if(ht["ReportDate"].ToString() != string.Empty)
                ht["ReportDate"] = Convert.ToDateTime(ht["ReportDate"]).ToString("yyyyMMdd");
    

    Wow, it's entertaining.

            StreamWriter sw = null;
            StreamReader sr = null;
    
            try
            {
                using (new Impersonator(username, domain, password))
                {
                    // ...
                }
            }
            catch (Exception ex)
            {
                Logger.LogErrorToText(ex.ToString());
                httpResponse.Redirect("Error.aspx?Msg=" + ex.Message);
            }
            finally
            {
                if (sw != null) sw.Close();
                if (sr != null) sr.Close();
            }
    

    This is special.

        //Should probably be combined with Escape
        static private string RemoveSpecialChars(string content)
        {
            //content = Regex.Replace(content, @"\r\n"," "); // Replaces \r\n with space [redacted]
            content = Regex.Replace(content, @"[^\u0020-\u007E]", " "); //Removes all special characters (leaves only 32-126 decimal)
            content = content.Replace(@"\", @"\E\"); //Escape characters according to [redacted]. ORDER IS SIGNIFICANT!!! This line must go first
            content = content.Replace(@"|", @"\F\");
            content = content.Replace(@"^", @"\S\");
            content = content.Replace(@"&", @"\T\");
            content = content.Replace(@"~", @"\R\");
            return content;
        }
    

    Very special:

        protected void Page_Error(object sender, EventArgs e)
        {
            if (!Response.IsRequestBeingRedirected)
            {
                Logger.LogErrorToText(e.ToString());
                Response.Redirect("Error.aspx?Msg=" + Server.GetLastError().Message);
            }
        }
    

            foreach (DataColumn dc in dr.Table.Columns)
            {
                ht[dc.ColumnName] = dr[dc.ColumnName];
            }
    

    Brillant.


    rdr is a SqlDataReader:

    hasVal = rdr.GetString(rdr.GetOrdinal("val")).Trim() != string.Empty;
    

    Current status: investigating a potential WTF:

    @DateCriteria='&gt; CAST(FLOOR(CAST(GETDATE()-7 AS FLOAT)) AS DATETIME)'


    Even worse, the text comes from a select control:

         &lt;select name="DropDownListTimeInterval" onchange="javascript:setTimeout(&#39;__doPostBack(\&#39;DropDownListTimeInterval\&#39;,\&#39;
    
    \&#39;)&#39;, 0)" id="DropDownListTimeInterval"&gt;
    	&lt;option value="BETWEEN CAST(FLOOR(CAST(GETDATE()+0 AS FLOAT)) AS DATETIME) AND CAST(FLOOR(CAST(GETDATE()+1 AS FLOAT)) AS 
    
    DATETIME)"&gt;Today&lt;/option&gt;
    	&lt;option value="BETWEEN CAST(FLOOR(CAST(GETDATE()-1 AS FLOAT)) AS DATETIME) AND CAST(FLOOR(CAST(GETDATE()+0 AS FLOAT)) AS 
    
    DATETIME)"&gt;Yesterday&lt;/option&gt;
    	&lt;option selected="selected" value="&gt; CAST(FLOOR(CAST(GETDATE()-7 AS FLOAT)) AS DATETIME)"&gt;Past Week&lt;/option&gt;
    	&lt;option value="&gt; CAST(FLOOR(CAST(GETDATE()-30 AS FLOAT)) AS DATETIME)"&gt;Past Month&lt;/option&gt;
    
    &lt;/select&gt;
    

    Trying to find where the data gets inserted.

    p_TableGet
    p_TableInsert
    p_TableInsertGet
    

    I think that I can rule out the first one...


    Don't worry, p_TableInsertGet makes more sense when you see this parameter:

    @returnPK_ID INT OUTPUT


    WTF

    no records exist for [ID] with ForeignKeyID = 3.
    some records exists for [ID] with ForeignKeyID = 3.
    

    YOUR LOGS ARE AWFUL FUGGIN' HELPFUL


                catch (Exception ex)
                {
                    Logger.Log(ex.ToString());
                    Logger.LogErrorToText(ex.ToString());
                    return false;
                }
                finally
                {
                    if (cfg.VerboseLogging)
                        Logger.Log("exiting from [AppName]...");
                }
    

    SO. EFFING. USELESS.


    Spot the WTFs

    Response.Write(BuildJSMessage(ex.Message))


    Dealing more with the VB project.

    UserControl1.ascx.vb
    UserControl2.ascx.vb
    UserControl3.ascx.vb
    UserControl4.ascx.vb
    UserControl5.ascx.vb
    

    Wow.

            public string GetTimeStamp(string type)
            {
                string sStamp = "";
    
                try
                {
                    string sDate = DateTime.Now.ToString("yyyyMMdd");
                    string sTime = DateTime.Now.ToString("HHmmssffff");
    
                    switch (type)
                    {
                        case "date":
                            sStamp = sDate;
                            break;
    
                        case "time":
                            sStamp = sTime;
                            break;
    
                        case "dateandtime":
                            sStamp = sDate + "_" + sTime;
                            break;
                    }
                }
                catch (Exception ex)
                {
                    Logger.Log(ex.ToString());
                    Logger.LogErrorToText(ex.ToString());
                }
                finally
                {
                    if (sc.bVerbose)
                        Logger.Log("Exiting from GetTimeStamp...");
                }
    
                return sStamp;
            }
    

                catch (Exception e)
                {
                    // ignore this error, it's a result of Response.End and unavoidable
                    if (!e.Message.Equals("Thread was being aborted."))
                    {
                        Logger.LogErrorToText("MethodName()", e.Message, WindowsIdentity.GetCurrent().Name.ToString(), null);
                    }
                }
    

                              try
                                {
                                    fi.Delete();
                                }
                                catch
                                {
                                    //continue on
                                }
    

    On Error Resume Next is alive and well in .NET


    Meah.

                catch (System.Threading.ThreadAbortException)
                {
                    // ignore this error, it's a result of Response.End and unavoidable
                }
                catch (Exception e)
                {
                    Logger.LogErrorToText("MethodName()", e.Message, WindowsIdentity.GetCurrent().Name.ToString(), null);
                }
    

    Broken pencil code:

    return status = "Failed";


    protected void btnHiddenBuildForm_Click(object sender, EventArgs e)
    {
       (25 lines of code)
    }
    
    protected void btnHiddenRebuildForm_Click(object sender, EventArgs e)
    {
       (25 lines of code, only difference is one boolean value)
    }
    

    public string[] SaveRecords(int RecordID)
    {
       // CRUD stuff
    
       return null;
    }
    

    Always returns null. Method name is plural, only saves one at a time.

    sigh


    Another from the broken pencil department of code:

            error: function (xhr, status, error) {
                alert("Error starting app.");
            }
    

    SELECT DISTINCT FieldName,
    	NULL, NULL, NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL
    	--Null, NULL, NULL, 
    
    NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL
    FROM DatabaseName.dbo.Blah
    

    using System;
    using System.IO;
    using System.Data;
    using System.Collections.Generic;
    using System.Data.SqlClient;
    using System.Security.Principal;
    using System.Configuration;
    using System.Web.UI;
    using System.Web.UI.WebControls;
    using System.Diagnostics;
    using System.Threading;
    using System.Web.UI;
    using System.Security.AccessControl;
    using CustomCommonLib;
    

    Guess which one(s) are actually used.


    With Ctrl1
       .Property = Value
    End With
    Ctrl2.Property1 = Value1
    Ctrl2.Property2 = Value2
    Ctrl2.Property3 = Value3
    Ctrl2.Property4 = Value4
    

    ISNULL(RecordID, '')

    Yup, RecordID is an int


    Grep'ing for //

    //Login div
    //Table
    //Table Body
    //Table Row
    //Table cell
    //User name label
    //Table Row
    //Table cell
    //Username input
    //Table Row
    //Table Cell
    //User name label
    //Table Row
    //Table Cell
    //password input
    //Table Row
    //Table Cell
    //Change password link
    //Table Row
    //Table Cell
    //Error Label
    //Error Div
    //Login Button   
    //Methods      
    //Properties
    

    Holy hell.

                else if (res is SqlException)
                {
                    SqlException exp = (SqlException)res;
                    Logger.Log(exp.Message);
                }
    

    ApplicationAccess
    ----------------------
    1
    1
    1
    NULL
    NULL
    1
    NULL
    NULL
    NULL
    NULL
    NULL
    1
    NULL
    NULL
    NULL
    NULL
    NULL
    NULL
    NULL
    NULL
    NULL
    NULL
    0
    0
    

    WTF

    Dim bReportable As Boolean = IIf(bBoardReportable, False, True)

    Yes, bBoardReportable is a boolean.


    Yeah, I understand that. The original developer didn't.

    $chk = $multiselect.find("input:checkbox[value='" + assignedarry[i] + "']");
    $chk[0].click();
    

    Trying to remember the name for this anti-pattern:

    DataSet ds = new DataSet();
    ds = classInstance.getData();
    

    Yes. That's how things are done at my company:

    Dim DC As New ClassName
    Dim DS As New DataSet
    DS = DC.GetResults()
    DC.Dispose()
    DC = Nothing
    

    As opposed to my code:

    Using DS As DataSet = (New ClassName).GetResults()


    If selComment.SelectedItem.Text &lt;&gt; "Select a Section Type" Then LoadComments()
    

    The best part is that the default option says -- Select Comment --


    Current status: it's Tuesday. Way better than Monday, but still Tuesday:

    bool isTestingMode = false; // MAKE SURE THIS IS FALSE WHEN NOT TESTING!!!!!!!!


    I just swore very loudly:

    (not exact)

    Public Class ClassName
       Public Sub StoreError(...)
          ...
       End Sub
    End Class
    
    Namespace NameOfNameSpace
       Public Class ClassName
          Public Sub StoreError(...)
             ...
          End Sub
       End Class
    End Namespace
    

    Really???????

    Public Sub Method1()
       Try
          Method2()
       Catch ex As Exception
          ' log it
       End Try
    End Sub
    

    What the fing f.

                    If CheckDataSet(DS) Then
                        With DS.Tables(0)
                            DT = DS.Tables(0)
                            Return True
                        End With
                    Else
                        Return False
                    End If
    

            Catch ev As Exception
                Dim s As String = ev.Message
                s = s
            End Try
    

    Different styles on consecutive lines😄

                        Dim iGroupID As Integer = lstGroupID.SelectedItem.Value
                        LoadObjects(lstStyles.SelectedItem.Value, iGroupID)
    

    I give up.

    iDoctorID = If(Not IsDBNull(.Rows(0).Item("DoctorID")), .Rows(0).Item("DoctorID").ToString, 0)


    I should create my own CodeSOD thread.

                    Dim csp As SqlParameterCollection = DataLayer.NonQuery_WReturn(sProcName, spInput)
                    If Not IsNothing(csp) Then
                        For Each sp As SqlParameter In csp
                            If sp.Direction = ParameterDirection.Output OrElse sp.Direction = ParameterDirection.InputOutput Then
                                If Not IsDBNull(sp.Value) Then
                                    iRecordID = sp.Value
                                End If
                            End If
                        Next
                    End If
    

  • FoxDev

    hmm... one like is not enough. could you possibly split these out into separate posts so that likes may be applied against individual snippits please?



  • Yeah, I've done that over in the status thread.


  • FoxDev

    +&

    Well played kind sir, well played.



  • I actually filtered the thread to my posts, went through all of them (over 400), quoted the CodeSOD ones, and formatted them in Notepad before posting here.



  • Private Function Logon(ByVal bLogon As Boolean) As Boolean

    It's not hard to figure out what happens if you pass False to this function.



  • Logout?



  • Please say that you changed the names here:

    @chubertdev said:

    Namespace NameOfNameSpace
    Public Class ClassName
    Public Sub StoreError(...)
    ...
    End Sub
    End Class
    End Namespace

    Or is it some ever-so-useful code template?

    This looks like somebody practicing how to write and call Subs or maybe another useful template for wrapping a sub in try:

    @chubertdev said:

    Public Sub Method1()
    Try
    Method2()
    Catch ex As Exception
    ' log it
    End Try
    End Sub



  • Yes, all the code here is anonymized.



  • @chubertdev said:

    The best part is that the default option says -- Select Comment --

    If the database is storing "-- Select Comment --" if the user doesn't change it then that is fucked up.
    But at least you can tell it's not a viable piece of data. Viable values as defaults are a bugbear of mine in databases. You can't tell if the user chose to leave it because it's right or left it because they were lazy and is, in fact, wrong. Nulls carry the information of "user did not enter any data". Defaults destroy that information. It can be displayed with defaults replacing the nulls any time we like. There's no excuse.



  • @LurkerAbove said:

    If the database is storing "-- Select Comment --" if the user doesn't change it then that is fucked up.
    But at least you can tell it's not a viable piece of data. Viable values as defaults are a bugbear of mine in databases. You can't tell if the user chose to leave it because it's right or left it because they were lazy and is, in fact, wrong. Nulls carry the information of "user did not enter any data". Defaults destroy that information. It can be displayed with defaults replacing the nulls any time we like. There's no excuse.

    It looks more like this:

    OptionTitle	Value
    -------------------------------
    X		Something
    Y		Something else
    Z		Third thing
    

    The drop down will have the default item, plus X, Y, and Z.

    If you select X, a textbox below will have "Something" in it

    So the actual value of what you select in the drop down isn't stored.



  • I see. I misread. So this is like in the SelectedIndexChanged event of a SelectedItem combobox?

    So now you see what happens when they don't use the magic IDs in the table.



  • @LurkerAbove said:

    I see. I misread. So this is like in the SelectedIndexChanged event of a SelectedItem combobox?

    So now you see what happens when they don't use the magic IDs in the table.

    Method called in Page_Load, actually.


  • Discourse touched me in a no-no place

    @chubertdev said:

    foreach (DataColumn dc in dr.Table.Columns)
    {
    ht[dc.ColumnName] = dr[dc.ColumnName];
    }

    Brillant.

    At the risk of being TRWTF, what's wrong with this? I don't work with DataSets enough to know. This is...duplicating the schema of a table into another table?


  • Discourse touched me in a no-no place

    @chubertdev said:

    Dealing more with the VB project.

    UserControl1.ascx.vb
    UserControl2.ascx.vb
    UserControl3.ascx.vb
    UserControl4.ascx.vb
    UserControl5.ascx.vb

    Ironically, these are 5 controls whose UI presents as a large number. TRWTF is that UserControl5 displays a 6.


Log in to reply