• (cs) in reply to Syarzhuk
    Syarzhuk:

    RayS:
    What I'm sure we can all agree on though is that whoever mandated this 'security' feature needs to have the 2nd half of his brain removed to put in the jar with the 1st half.

    This is impossible, because .jar is a Java format and this is VB coder we're talking about. VBX would do just fine, though.



    Technically, it's zip format.  The only thing that associates it with Java is the extension.  (I get the joke though)
  • (cs) in reply to dance2die
    dance2die:
    I really hate it when people do something like

    "confirm <> True"

    instead of writing simpler and  clearer code like

    "confirm = False"

    Gosh...


    Someone may have already said this but, you used the assignment operator "=" when you wanted to use the comparison operator "=="
  • Anonymous Coward (unregistered)

    I can think of two reasons why you might want to do something like this.

    1. You want to avoid if statements in the higher level code (simply write the condition as the last parameter)
    2. You want to flesh out your code from the top down and don't want to have to worry about the underlying methods doing something stupid because the developer that was writing them hadn't tested them out fully yet.
    Course, it would be easier just to comment out the function call if you really didn't want it to run...but then again you could have some global static variable that enables all function calls simply by making it true...wouldn't have to go through line by line uncomment that pesky function call.  I think it'd be really funny to have a boolean for each programmer on the project and you enable the functions that they call by setting thier corresponding boolean to true.

    Dim paulaCode As Boolean = True
    Dim daveCode As Boolean = False

    Public Sub xxx()
        ...
        DeleteAccount("000000", paulaCode)
        ...
        CreateAccount("000000",daveCode)
        ...
    End Sub

  • bob (unregistered) in reply to Murph
    Murph:
    Sorry... Method should look like this:

    public void deleteAccount(String accountNum, paulaBean confirm)
    {
       if (
    confirm.getPaula() != "Brillant")
       {
          return;
       }
       ...
    }



    That would not work.
    You need getPaula().intern(), or .equals.
    + http://www.javaranch.com/campfire/StoryCups.jsp
    + http://www.javaranch.com/campfire/StoryPassBy.jsp
  • (cs) in reply to bugsRus
    bugsRus:
    Anonymous:
    Anonymous:
    dubwai:

    OneFactor:
    I'm just waiting for people to post about how today's WTF is so bad because of its performance impact rather than the sheer idiocy behind thinking up such a concoction. Then there will be the "premature optimization is the root of all evil" acolytes putting in their 0010 cents with interest.

    It's more like: some anonymous coward will post about how the WTF isn't a WTF because it's faster.  Then someone will post about how it's not faster and it is in fact slower.  Then you'll have the people saying that not using the WTF for performance reasons is a micro-optimization.

     

    This method of structuring is proven to be faster because if confirm is false it does nothing... hence instance performance boost!

    But in fact it's slower since confirm value has to be evaluated.



    Come on!  We all know if you use today's example, you are really doing optimization before it's really necessary, therefore it's a micro-optimization.

    You have it wrong.   Not using this example in all your functions is a micro optimization.    Unless the function really is a bottle neck you should always add this confirmation.

    Function isTrue(test, confirm)
      if confirm <> isTrue(True, True)
         return False
      return test
    end function

    There, see how much better that is?   Course you might want to run it through a profiler, if isTrue really is a bottle neck you can be justified for removing the check from this particular function.  However that still doesn't excuse you from optimizing ALL your functions like that without using a profiler first.
  • Raymond Lewallen (unregistered)

    Coolio.. You can also "attempt" to run the method with a value of Nothing/String.Empty as an account number.  It won't tell you until you've already create the overhead of the Ado objects and it pukes on you.

  • (cs) in reply to hank miller
    hank miller:


    Function isTrue(test, confirm)
      if confirm <> isTrue(True, True)
         return False
      return test
    end function



    On further thought, this isn't quite good enough yet - I forgot a couple of critical parts... Try:

    Function isTrue(test, confirm)
    if isTrue((isTrue(confirm, True) <> isTrue(True, True)))
    # randomize the bias
    if random(0,1) == 1 # return either 1 or 0 - I forget the syntax of random
    return True
    else
    return False
    return test
    end function

    Brillant if I do say so myself.

  • (cs) in reply to Timbo
    Anonymous:
    Oliver Klozoff:
    ....this one stumps me. I cannot fathom any situation where this code would ever be useful for anything other than Confirm == True.


    Unit Tests.


    I bet your customers would be really happy if you were to ship this POS software after is passes all of the unit tests.  And, I bet you run those tests often because they run so fast.

    PHB: Why are all of our customers complaining about bugs in our latest release of PaulaRAD?
    You: Don't know, it passed all of the unit tests!
    PHB: Hum.  Maybe we need to sell more training. Stupid users, you know.
    You: Yeah!
  • anon (unregistered)
    Alex Papadimoulis:

    For example, they felt the need to have one extra layer of confirmation before doing "serious" things. Such "serious" methods would only do their thing when True was passed as the value of the Confirm parameter ...



    I'm not a VB programmer, so this explanation might not make sense, but ...

    Assuming that DeleteAccount() is a method in your business logic, that will be called from the GUI, this style of coding ensures that someone doesn't leave out the confirmation step. I would see it being called like so:
    DeleteAccount(accountNum, showConfirmDialog())
    Mind you, that doesn't explain passing the confirm value to the database ...

  • Ben Ryves (unregistered) in reply to Mung Kee
    Mung Kee:
    dance2die:
    I really hate it when people do something like

    "confirm <> True"

    instead of writing simpler and  clearer code like

    "confirm = False"

    Gosh...


    Someone may have already said this but, you used the assignment operator "=" when you wanted to use the comparison operator "=="
    As it's BASIC we're talking about, == wouldn't work. = is the comparison as well as the assignment operator.
  • mame (unregistered) in reply to Mung Kee
    Mung Kee:
    dance2die:
    I really hate it when people do something like

    "confirm <> True"

    instead of writing simpler and  clearer code like

    "confirm = False"

    Gosh...


    Someone may have already said this but, you used the assignment operator "=" when you wanted to use the comparison operator "=="
    No he didn't, in vb comparison is made with single equality sign. Thats why braindamaged vb coders that move to C# write if (null == something) ...
  • (cs) in reply to anon
    Anonymous:
    Alex Papadimoulis:

    For example, they felt the need to have one extra layer of confirmation before doing "serious" things. Such "serious" methods would only do their thing when True was passed as the value of the Confirm parameter ...



    I'm not a VB programmer, so this explanation might not make sense, but ...

    Assuming that DeleteAccount() is a method in your business logic, that will be called from the GUI, this style of coding ensures that someone doesn't leave out the confirmation step. I would see it being called like so:
    DeleteAccount(accountNum, showConfirmDialog())
    Mind you, that doesn't explain passing the confirm value to the database ...


    <font size="2">but isn't

    if(showConfirmDialog())
        DeleteAccount(accountNum);

    </font>
    <font size="2">
    clearer and more efficient?


    I got your point, though.
    </font>

  • AnonymousCoder (unregistered) in reply to John Smallberries
    John Smallberries:
    Anonymous:
    Alex Papadimoulis:

    For example, they felt the need to have one extra layer of confirmation before doing "serious" things. Such "serious" methods would only do their thing when True was passed as the value of the Confirm parameter ...



    I'm not a VB programmer, so this explanation might not make sense, but ...

    Assuming that DeleteAccount() is a method in your business logic, that will be called from the GUI, this style of coding ensures that someone doesn't leave out the confirmation step. I would see it being called like so:
    DeleteAccount(accountNum, showConfirmDialog())
    Mind you, that doesn't explain passing the confirm value to the database ...


    <FONT size=2>but isn't

    if(showConfirmDialog())
        DeleteAccount(accountNum);

    </FONT>
    <FONT size=2>
    clearer and more efficient?


    I got your point, though.
    </FONT>


    No!

    <FONT size=2>if(isTrue(showConfirmDialog(), True))
        DeleteAccount(accountNum);</FONT>

    <FONT size=2>is clearer and more efficient, not to mention performance advantagous.</FONT>

  • (cs) in reply to anon
    Anonymous:
    Mind you, that doesn't explain passing the confirm value to the database ...


    I think that's the real WTF here... since they're so keen on having layers of confirmation, why should they automatically pass the confirmation value from one layer down to the next? Clearly, since they added a CONFIRM paramater to the database command, not to use it would be a security breach. Not sure how to fix it entirely, but something like this could work as a countermeasure:

    Dim bDoubleCheck as Boolean

    Randomize
    If CInt(Rnd * 100) <= 60 Then
         ' Let's give a 60% bias towards True, since they passed it as Confirm (they probably mean it)
         bDoubleCheck = True
    Else
         '40% chance of passing False; you can't be too careful
         bDoubleCheck = False
    End If

    Then replace the offending line with:

         .CreateParamater("CONFIRM", adInteger, adParamInput, 4, CInt(bDoubleCheck))

    There. NOW let's see someone try to delete a player account accidentally!
  • AnonymousCoder (unregistered) in reply to DeusEx

    DeusEx:
    Anonymous:
    Mind you, that doesn't explain passing the confirm value to the database ...


    I think that's the real WTF here... since they're so keen on having layers of confirmation, why should they automatically pass the confirmation value from one layer down to the next? Clearly, since they added a CONFIRM paramater to the database command, not to use it would be a security breach. Not sure how to fix it entirely, but something like this could work as a countermeasure:

    Dim bDoubleCheck as Boolean

    Randomize
    If CInt(Rnd * 100) <= 60 Then
         ' Let's give a 60% bias towards True, since they passed it as Confirm (they probably mean it)
         bDoubleCheck = True
    Else
         '40% chance of passing False; you can't be too careful
         bDoubleCheck = False
    End If

    Then replace the offending line with:

         .CreateParamater("CONFIRM", adInteger, adParamInput, 4, CInt(bDoubleCheck))

    There. NOW let's see someone try to delete a player account accidentally!

    Ahhh? There's 60% chance someone will...

  • Philip Stephens (unregistered)

    What gets me is not just that the person who instigated this practice was a moron, but that none of the programmers working there were willing to question the practice.  "Some shops just do things differently" is not a satisfactory alternative to having a brain.

  • Anonymous (unregistered) in reply to Philip Stephens

    Anonymous:
    What gets me is not just that the person who instigated this practice was a moron, but that none of the programmers working there were willing to question the practice.  "Some shops just do things differently" is not a satisfactory alternative to having a brain.

    I am sorry to admit this but I worked in a place like that. Where I saw lots of odd things like this, maybe not to this degree, but still. I wish I could've done something about, but it was so entranched into their systems that it would require massive and costly code rewrite. A few people (originally inventors) thought it's the most ingenious thing ever developed, the rest thought it was plain stupid, but nothing could be done at that point to fix it. This WTF is a good example; if confirm variable is passed into every 'serious' function on many different layers then imagine how much effort has to go into removing what seems to be "harmless" extra parameter. But, I bet that original inventor of this "extra layer of confirmation" feature thinks it's the most ingenious thing, he's probably applying for a patent right now!

     

  • (cs) in reply to Philip Stephens
    What gets me is not just that the person who instigated this practice was a moron, but that none of the programmers working there were willing to question the practice.  "Some shops just do things differently" is not a satisfactory alternative to having a brain.


    One might guess that none of 'foreign' developers had neither formal education in CS nor real experience at all.

  • Anonymous (unregistered) in reply to travisowens
    travisowens:

    I guess I would be stating the obvious when I would say confirmation is a concept of a UI and not functionality.

    Should the user (or in the case of an account delete, the admin) be asked to confirm a delete operation, definetly!

    Should the code, hell no.  And obviously no confirmation is actually going on here, we're just requiring the coder to send an extra variable for no good reason.

    So technically their own methods aren't confirming if they should execute because they don't exit/pause and request a seperate confirmation.  Instead they bundle the request and confirmation together, which technically isn't a confirmation.


    I'd bet you dollars to donuts that the value of Confirm isn't passed as a known value.  It's a variable or a return from another function.  Probably something like this:

    bool Confirm = CallSomeFunctionThatChecksIfWeShouldDeleteTheAccount()
    DeleteAccount(AcctNmbr, Confirm)

    It's still braindead, of course, but not quite as silly as passing a value that is known at compile time.  If it were done something like this, probably no one would raise an eyebrow:

    if (CallSomeFunctionThatChecksIfWeShouldDeleteTheAccount()) then
        DeleteAccount(AcctNmbr)
    endif

    (VB isn't my language, so forgive any silly syntax errors I may have introduced.)


  • (cs) in reply to travisowens
    travisowens:

    I guess I would be stating the obvious when I would say confirmation is a concept of a UI and not functionality.

    Should the user (or in the case of an account delete, the admin) be asked to confirm a delete operation, definetly!

    Should the code, hell no.  And obviously no confirmation is actually going on here, we're just requiring the coder to send an extra variable for no good reason.

    So technically their own methods aren't confirming if they should execute because they don't exit/pause and request a seperate confirmation.  Instead they bundle the request and confirmation together, which technically isn't a confirmation.

    I think the previous post said it best about sending a user & pass with the method, now you can really prevent people from executing methods unless they look up the user/pass in the code.  And imagine the coding hell it will create when you need to change the user/pass oneday!  My evil mind is in bliss as we speak!


    Maybe Confirm could be a Class that emails the developer as the implementation of the default property, to ask if they're really absolutely sure every time. Using a db lookup and xp_sendmail of course, with the sa password in the code. (password: "whydoIneedone?")
  • (cs) in reply to Timbo
    Anonymous:
    Oliver Klozoff:
    ....this one stumps me. I cannot fathom any situation where this code would ever be useful for anything other than Confirm == True.


    Unit Tests.


    A function that is deliberately designed to work differently during testing as opposed to real use - that doesn't sound too clever either.
  • (cs) in reply to Syarzhuk
    Syarzhuk:

    travisowens:
    I think the previous post said it best about sending a user & pass with the method, now you can really prevent people from executing methods unless they look up the user/pass in the code.  And imagine the coding hell it will create when you need to change the user/pass oneday!  My evil mind is in bliss as we speak!

    No coding hell if you use proper development techniques, specifically in this case - named constants. Refactor the previous poster's code to:

    Global PROPER_USER_NAME = "programmer"

    Global PROPER_PASSWORD = "initech"

    Function Add( a, b, user, password )
    if user=PROPER_USER_NAME  and password=PROPER_PASSWORD then

       Add = a + b
    else 
       Add = 0 ' unauthorized programmer, will get bad result
    End Function

    That was, if the password ever changes, you only need to change it in one place and no coding hell ensures



    You give them way too much credit. The code would probably look like this:

    Public Sub DeleteAccount(AccountNum As String, Confirm As Boolean, User As String, Pass As String)

    If Confirm <> True Then Exit Sub
    If IsTrue(checkUser(User, Pass, decideConfirm(Confirm, User, Pass), User, Pass),
    decideConfirm(Confirm, User, Pass), User, Pass) <> True Then Exit Sub


    Dim cmd As New ADODB.Command
    With cmd
    .ActiveConnection = getConn()
    .CommandText = "DELETE_PLAYER_ACCOUNT"
    .Parameters.Append _
    .CreateParameter("ACCOUNT_NUM", adChar, adParamInput, 8, AccountNum)
    .Parameters.Append _
    .CreateParameter("CONFIRM", adInteger, adParamInput, 4, CInt(Confirm))
    .Execute
    End With

    End Sub

  • LLXX (unregistered)

       Actually, it would be a good idea for VERY CRITICAL functions to have a second layer of confirmation to them, e.g. those that may destroy large amounts of data. A buffer overflow or some other bug (a hardware RAM glitch?) might randomly call such a function. It reminds me of a time long ago when a buggy DOS program (with nothing to do with disks) somehow called the BIOS Format Track function, wiping out a large portion of the disk. I keep wondering why the developers of the BIOS didn't implement a second level of confirmation, as that was definitely a critical function. However, doing so to non-critical functions is definitely a WTF.

  • Anonymous (unregistered)

    Is it possible to use lazy evaluation in VB?

    Maybe evaluation of the boolean will result in a dialog box being created.

    That would still be very poor design, but at least it would serve some
    useful purpose.

  • (cs) in reply to Anonymous
    Anonymous:
    I wish I could've done something about, but it was so entranched into their systems that it would require massive and costly code rewrite.

    Two words: design debt.

    Works a lot, the only thing is assigning monetary value to it.  Somehow that always gets attention.
  • noel (unregistered) in reply to Grant
    Grant:
    This is clearly the We-use-our-production-database-for-development-and-QA-so-we-can't-actually-run-some-sql-statements durring-development-and-QA design pattern.

    Saves time on post-production maintenance by directly debugging on the production server with the production database! Best design pattern ever.

Leave a comment on “Foreign Methodologies”

Log In or post as a guest

Replying to comment #:

« Return to Article