• olly (unregistered) in reply to cdog

    Anonymous:

    I hope (maybe not) there is a complementary function that "unescapes" the quotes


    I hope not, because then all the actual poor defenseless grave accents would be senselessly converted to single quotes.

    Not *all* of them. Only the first one [;)]

  • Evan (unregistered) 

    ' __ quotCheck2 __
' Replaces single quote chars (') with apostrophes (`)
'  to get SQL-Safe Strings
Function sqlChkName2(sName As String) As String
  Dim sNewName As String
  Dim iPos As Integer
  Dim iLen As Integer

    iPos = InStr(1,sName,"'")
      iLen = Len(sName)

      sNewName = sName
      If iPos > 0 Then
sNewName = Mid(sName, 1, iPos - 1) + "`" + Mid(sName, intPos + 1)
      End If
    

    sqlChkName2 = sNewName
End Function



    While all the WTFs in this have been commented exccessively, no-one has mentioned the redundant assignment of sNewName. Really, it should be an If .. Then... Else rather than a pre-assignment followed be a change, purely based on a assembly code view. Single hits to memory are always better than two.
  • (cs) in reply to Bellinghman

    Bellinghman:
    As for the double calling of Instr - that happens only if there is an apostrophe in the name anyway.

    But since you need to call Instr regardless to determine whether there's an apostrophe in the name, why not store that value in a variable and not call Instr again?

    Sure, it's not going to bring your program to its knees, but calling any function twice in a row to get the exact same result when you can store that result is bad design.

  • Random Joe (unregistered)

    I think the WTF here is the variable naming conventions. Prefixing s to strings, and i to integers. While such an idea might be a nice thought, it's mostly useless in practise. Not to mention that it makes the code visually unappealing, which is one of the most important aspects of good code, in my opinion.

  • stretch (unregistered) in reply to Jeff S
    Jeff S:

    Oh, and by the way, for the hard core VB programmers out there: 

    This is a perfect example of  why you should ALWAYS use Option Explicit.



    Man I had to look all the way to the bottom for someone to say Option Explicit.  Not too many VB programmers here?
  • (cs) in reply to Random Joe
    Anonymous:
    I think the WTF here is the variable naming conventions. Prefixing s to strings, and i to integers. While such an idea might be a nice thought, it's mostly useless in practise. Not to mention that it makes the code visually unappealing, which is one of the most important aspects of good code, in my opinion.


    Haha!  You must be a troll.  Welcome to the party, sit down, and have a beer.
  • ERG (unregistered) in reply to Random Joe

    Anonymous:
    I think the WTF here is the variable naming conventions. Prefixing s to strings, and i to integers. While such an idea might be a nice thought, it's mostly useless in practise. Not to mention that it makes the code visually unappealing, which is one of the most important aspects of good code, in my opinion.

     

    Go grab a VB book that is pre .Net and one of the first two chapters will tell you to put sString, iInteger, oObject, dDoble, frmForm, cmdCommandButton..... it's a VB(very basic) thing.....

     

    Of course now M$ has retracted that in lue of 

    Dim ThisISAnIntegerForCountingThingsWhileILoopInAForLoop01 As Integer

    Dim ThisISAnIntegerForCountingThingsWhileILoopInAForLoop02 As Integer

    Dim ThisISAnIntegerForCountingThingsWhileILoopInAForLoop03 As Integer

  • Z (unregistered) in reply to rogthefrog
    rogthefrog:

    Bellinghman:
    As for the double calling of Instr - that happens only if there is an apostrophe in the name anyway.

    But since you need to call Instr regardless to determine whether there's an apostrophe in the name, why not store that value in a variable and not call Instr again?

    Sure, it's not going to bring your program to its knees, but calling any function twice in a row to get the exact same result when you can store that result is bad design.



    It's surely bad design to introduce an extra variable just to store a simple computation.
  • (cs) in reply to Z

    Anonymous:


    It's surely bad design to introduce an extra variable just to store a simple computation.

    Yeah! You should just do the computation over and over.  Why use a *variable* to store info ! yuck! What kind of idiot programmer would do that?  Why did they ever even invent those stupid things, who needs them?

    (I can only imagine if this guy was my boss ... "Uh, Jeff, nice code but what's up with the variables?  Can we lose those? I heard somewhere that storing simple computations in variables is an indicator of bad software design")

  • One-Year Experienced Programmer (unregistered)

    First, it does not "replace" the single quote chars found in the orginal string. It does return a string supposedly with all the ' replaced by `.

    And then the logic replaces only the first ' found in the string, and immediately returns. Either you need a for loop in this method, or you can call this method recursively on the string until all ' are replaced.

    And there's no need to introduce the local variable sNewName!

    I found only three :-(

     

     

  • Bob (unregistered) in reply to Aristotle
    Aristotle:
    Mangling data? The whole thing is a WTF in and of itself: written by someone who never heard of escaping.

    I once did something similar because that was the way management wanted it. The GUI the salespeople used was perfectly capable of dealing with names like O'Hara and storing them safely in that form, but for some reason they had been told to enter that as OHARA instead. My procedure, which worked on data from a web form, was required to do the same thing for consistency: uppercase it and strip out any ' characters.

    As an added bonus, the president of the company was of Irish descent and ... yes, indeed ... his own name, mangled, was one of the first rows in the customer table.

    I'm not claiming this was a good idea, just saying -that particular decision- may not have been the programmer's fault.

  • midas (unregistered)

    To be fair, VB 5.0 and older didn't have a Replace function.

    Doesn't forgive this broken crap though.

  • Avyn (unregistered)

    I see the biggest WTF of them all-and the one everyone missed. It's the very first thing there: `is not an apostrophe. Even worse, it's actually an opening single quote. This thing is doing the inverse of what it purports to be doing, and, appearantly, not even doing it right. Unprononcable scream.

  • Matt W (unregistered)

    The real WTF is the fact that this person has re-invented the square wheel. If you were to use a parameterized query (whether it's inline or a stored proc), then as long as you're not using dynamic SQL then your problem will just go away. Any hacker will sit there until doomsday and they won't be able to inject nuffink, geddit ?

Leave a comment on “Replace("bad programmer", "good programmer") ”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article