ThreadSleep webservice



  • I think the code speaks for itself. The sheer wtfness of this simple method is mind-blowing. The useless comments. The fact that a webservice exists to to a Thread.Sleep. The fact that it returns a boolean (for what?). The catching of exception, in case a Thread.Sleep has crashed I guess. The empty Finally block. The fact that it's VB. Everything.

        ''' <summary>
        ''' Application service get delay of particular time based on the configuration settings "SleepTimeOut".
        ''' </summary>
        ''' <param name="userId">The user id.</param>
        ''' <param name="mandantId">The mandant id.</param>
        ''' <param name="userType">Type of the user.</param>
        ''' <returns><c>true</c> if success, <c>false</c> otherwise</returns>
        <WebMethod()> _
        Public Function ThreadSleep(ByVal userId As String, ByVal mandantId As String, ByVal userType As String) As Boolean
            Me.WriteLog("Control entered in ThreadSleep()", "userId", userId, "mandantId", mandantId, "userType", userType)
            Try
                Dim sleepTimeOut As Integer
                sleepTimeOut = IIf(Integer.TryParse(ConfigurationManager.AppSettings("SleepTimeOut"), sleepTimeOut), sleepTimeOut, 10000)
                Thread.Sleep(sleepTimeOut)
                Return True
            Catch ex As Exception
                Me.WriteLog(ex.ToString())
                Return False
            Finally
    
            End Try
        End Function

  • sockdevs

    The comments you can't have; those would be processed by a documentation generator.

    The rest are deffo :wtf:s though ;)



  • Is Mandant a DC Comics supervillain?



  • Mandant is an SAP term....



  • The comments: I mean, "userId : the user id"... yeah.

    And do you understand the summary ? I mean, if you didn't know the method name and body, would you guess what the method does?



  • @blakeyrat said:

    Is Mandant a DC Comics supervillain?

    That'd be awesome. I'll start naming my variables after superheroes and villains right away.


  • sockdevs

    Hmm… OK, you can have the comments too ;)



  • @cosmo0 said:

    Integer.TryParse(ConfigurationManager.AppSettings("SleepTimeOut")

    Integer.TryParse(ConfigurationManager.AppSettings("SleepTimeOut")
    

    I guess this changes so much you want to retrieve it each time?



  • Wait, why would you be using TryParse on an AppSetting? YOU KNOW THE TYPE!


  • sockdevs

    @blakeyrat said:

    Wait, why would you be using TryParse on an AppSetting? YOU KNOW THE TYPE!

    What if someone mistypes the value, and it no longer parses? True, the :wtf: factor of typing seventeen instead of 17 is extraordinary, but stupider things have happened…



  • @RaceProUK said:

    What if someone mistypes the value, and it no longer parses?

    Well if they're fucking sane Visual Studio will catch the error long before they even attempt to build it.


  • sockdevs

    I guess they could run a pre-build step that validates the App.config



  • Or they could do their development inside Visual Studio. I know, crazy crazy idea, I'm sorry.


  • sockdevs

    I'm assuming they are in VS; I've just never seen it type-check an App.config before, s'all.



  • So they're doing a sleep...and to do that they log the user, mandant, and user type. Which leads one to wonder why they did not also log the sleepTimeOut. Maybe that's a trade secret, hmmm?

    Or maybe, just maybe, it was idiotic to log this :wtf: event in the first place?



  • Defensive programming. More failsafes is always* good.

    *pedantic clarification: not actually always, but most of the time.



  • @cosmo0 said:

    Thread.Sleep(sleepTimeOut)
    Return True

    I may be being dumb as I don't muck with threading if I can avoid it but doesn't Thread.Sleep act on the currently active thread so that the webservice won't return till the sleep is over?


  • sockdevs

    @blakeyrat said:

    Wait, why would you be using TryParse on an AppSetting? YOU KNOW THE TYPE!

    wait. app.config appsettings can be annotated as different types?

    that's news to me and i'd love to see the documentation on how to use that, because i have to do tryparse all over the flipping place when reading from app.config (or web.config for webapps)



  • Crap. This looks like someone saw my OMGWTF2 submission on Github, ported my thread sleep "function" to VB, and used it.



  • @locallunatic said:

    I may be being dumb as I don't muck with threading if I can avoid it but doesn't Thread.Sleep act on the currently active thread so that the webservice won't return till the sleep is over?

    Speedup loop function!



  • @RaceProUK said:

    What if someone mistypes the value, and it no longer parses? True, the factor of typing seventeen instead of 17 is extraordinary, but stupider things have happened…

    It's not user input. I would just Integer.Parse it, if someone types "seventeen", they should get a parse error, not some built-in default that silently ignores what they typed.

    This also bothers me:

    sleepTimeOut = IIf(Integer.TryParse(ConfigurationManager.AppSettings("SleepTimeOut"), sleepTimeOut), sleepTimeOut, 10000)
    

    This is much clearer and how you are supposed to use TryParse if you want a default if it doesn't parse:

    sleepTimeout = 10000
    Integer.TryParse(ConfigurationManager.AppSettings("SleepTimeOut"), sleepTimeout)
    

    Even better:

    If Not Integer.TryParse(ConfigurationManager.AppSettings("SleepTimeOut"), sleepTimeout) Then
      Throw New ApplicationException("Setting SleepTimeout cannot be parsed as an integer.")
    End If
    


  • That's exactly what it will do, and is the most WTF thing of the whole snippet. Seeing as it's a web method the only reason I can see it exists is to simulate a delay, perhaps from an Ajax front end.


  • sockdevs

    @Jaime said:

    It's not user input.

    No, but someone has to type it into the App.config ;)



  • @accalia said:

    wait. app.config appsettings can be annotated as different types?

    that's news to me and i'd love to see the documentation on how to use that, because i have to do tryparse all over the flipping place when reading from app.config (or web.config for webapps)

    If you're working within Visual Studio, it autogenerates a Settings.cs or My.Settings.vb class that has strongly-typed variables and handles the parsing for you. The UI for this is a tab in the project's Properties page.


  • sockdevs

    @TwelveBaud said:

    If you're working within Visual Studio, it autogenerates a Settings.cs or My.Settings.vb class that has strongly-typed variables and handles the parsing for you.

    Is that really automatic? I don't recall ever having that option before… but then it's been a while since I've had to generate my own App.config/Web.config


  • sockdevs

    ok. that's a nice feature, but isn't the point about web.config/app.config to store environment specific changes so you can set what DB server to connect to or what port to listen to without having to recompile?


  • sockdevs

    IIUC, it doesn't need a recompile; the autogen class reads the *.config


  • sockdevs

    OOOH!

    well that is cool!

    /me wanders off to go implement that in several projects i'm working on for work....

    i can get rid of quite a few instances of this sort of code:

            private readonly Lazy<TimeSpan> _servicePollingDelay = new Lazy<TimeSpan>(() =>
            {
                double servicePollingDelaySeconds;
                return !double.TryParse(ConfigurationManager.AppSettings["ServicePollingDelaySeconds"], out servicePollingDelaySeconds) ? TimeSpan.FromSeconds(10) : TimeSpan.FromSeconds(servicePollingDelaySeconds);
            });
            private TimeSpan ServicePollingDelay { get { return _servicePollingDelay.Value; } }
    


  • @Jaime said:

    This is much clearer

    Also wrong. TryParse takes an out parameter, not ref, so it has to assign before exiting. And it assigns zero.

    Unless things are different in VBland, but I don't think so...


  • sockdevs

    Just double-checked how it works; basically you add a new Settings File, and it auto-populates your *.config with the stuff you put in it :smile:



  • @cosmo0 said:

    The fact that it returns a boolean (for what?).

    So you can tell if an exception was thrown inside the function?


  • sockdevs

    @tar said:

    So you can tell if an exception was thrown inside the function?

    TryParse does it, so why not?
    @Maciejasjmj said:
    Unless things are different in VBland, but I don't think so...

    It's the same API and the same runtime, so it'll be the same behaviour.



  • AFAIR the other way around - it's Parse that uses TryParse and throws an exception when it fails. Otherwise you'd be getting an entirely avoidable performance penalty of try/catch.



  • @TwelveBaud said:

    If you're working within Visual Studio, it autogenerates a Settings.cs or My.Settings.vb class that has strongly-typed variables and handles the parsing for you. The UI for this is a tab in the project's Properties page.

    But, the only way you get validation of the data in web.config, is if you change the settings with Visual Studio. That will only happen if the developers are in charge of the production config, and the workflow for changing the file goes through Visual Studio. In my experience the workflow for changing the production web.config is:

    • Open web.config in notepad
    • Make change
    • Save


  • @Maciejasjmj said:

    Also wrong. TryParse takes an out parameter, not ref, so it has to assign before exiting. And it assigns zero.

    Unless things are different in VBland, but I don't think so...

    Hmmm... you are correct. VB intellesense shows it as ByRef, but it does behave as an out even though VB has no out equivalent. I've never tried to use the variable when TryParse returns False, so I never noticed before. Either way, my "even better" example is still better and it does work.



  • @cosmo0 said:

    The catching of exception, in case a Thread.Sleep has crashed I guess.

    Java sleep throws a checked InterruptedException exception, maybe this was ported? (by a child?)


  • Discourse touched me in a no-no place

    @Jaime said:

    Even better:

    If Not Integer.TryParse(ConfigurationManager.AppSettings("SleepTimeOut"), sleepTimeout) Then
    Throw New ApplicationException("Setting SleepTimeout cannot be parsed as an integer.")
    End If

    No, better would be to use the method that does that for you.

    sleepTimeout = Integer.Parse(ConfigurationManager.AppSettings("SleepTimeOut"))
    

    Sure, it will throw a generic exception instead of one custom for your app. That's not a wrong thing to do in this case…



  • @dkf said:

    No, better would be to use the method that does that for you.

    That was my first suggestion.

    @Jaime said:

    It's not user input. I would just Integer.Parse it, if someone types "seventeen", they should get a parse error, not some built-in default that silently ignores what they typed.

    Throwing a custom exception is better because the exception is easier to understand. Therefore, using Integer.Parse is perfectly functional, but slightly worse.


  • mod

    Wrap and re-throw?



  • That give you the same result, but with two throws instead of one, so it will perform a bit worse. Of course, it shouldn't happen very often, so it's almost irrelevant in this case. I guess it's kind of weird suggesting that a method who's sole purpose is to do nothing could perform poorly.



  • @Jaime said:

    ```vb
    If Not Integer.TryParse(ConfigurationManager.AppSettings("SleepTimeOut"), sleepTimeout) Then
    Throw New ApplicationException("Setting SleepTimeout cannot be parsed as an integer.")
    End If

    <img src="/uploads/default/16884/a5d0e47d7e746d62.png" width="608" height="188"> 
    
    That warning has only been in the ApplicationException docs since .NET 4.5, but everywhere else similar guidance appeared starting with the first .NET 2.0 beta.

  • Discourse touched me in a no-no place

    @Jaime said:

    That was my first suggestion.

    And I missed seeing it because it was done in free text and not code.
    @Jaime said:
    Throwing a custom exception is better because the exception is easier to understand. Therefore, using Integer.Parse is perfectly functional, but slightly worse.

    Except then you're slightly obscuring the fact that it was a failure to parse an integer from a string; the message is only informative, whereas the exception type is much clearer. Potentially. :)

    The worse sin is that it makes the code longer and quite a lot less clear. It's virtually always best to write code to be as clear as possible so that the next time someone looks (e.g., you in 2 months' time!) they can see exactly what's going on.



  • @TwelveBaud said:

    That warning has only been in the ApplicationException docs since .NET 4.5, but everywhere else similar guidance appeared starting with the first .NET 2.0 beta.

    Actually, they've got back and forth, and there isn't agreement on what to use. Some people say that you want to use the most appropriate existing exception, like FormatException which is what you'll get if you simply use Parse. Others say you should never throw anything in the System namespace unless you are framework code. There are so many considerations on each side that this really come down to a style issue.

    The point of the warning text in MSDN is that ApplicationException means a bunch of things to a bunch of different code, so you can never be certain when catching it. The exception in the code I wrote will never be caught, so the warning doesn't really apply.



  • @cosmo0 said:

    The comments: I mean, "userId : the user id"... yeah.

    And do you understand the summary ? I mean, if you didn't know the method name and body, would you guess what the method does?


    I read the summary as implying that it would return the delay (i.e. how many milliseconds or whatever). "get delay"...

    Having read the code further, I discovered I was wrong...



  • @Jaime said:

    The exception in the code I wrote will never be caught, so the warning doesn't really apply.

    Never? Until the heat death of the universe you'll never need to implement a change that requires catching that exception?

    I don't believe you.


  • Discourse touched me in a no-no place

    @Jaime said:

    There are so many considerations on each side that this really come down to a style issue.

    I'm of the opinion (as a Java programmer) that there are expected exceptions and unexpected ones. Expected exceptions are the ones that you should deal with (whatever that means) and that should include all exceptions due to the service user giving you bad input; those aren't a service failure, they're a moron/asshat problem. Unexpected exceptions are there for things like misconfigurations and total failure cases.

    If you're dealing with a REST web service, expected exceptions should end up being a 4** error (or occasionally a 5** error, depending) but unexpected exceptions should always be a 5** error.



  • @dkf said:

    I'm of the opinion (as a Java programmer) that there are expected exceptions and unexpected ones.

    rumsfeld sings again – 01:34
    — rocsims



  • @dkf said:

    I'm of the opinion (as a Java programmer) that there are expected exceptions and unexpected ones. Expected exceptions are the ones that you should deal with (whatever that means) and that should include all exceptions due to the service user giving you bad input; those aren't a service failure, they're a moron/asshat problem. Unexpected exceptions are there for things like misconfigurations and total failure cases.

    If you're dealing with a REST web service, expected exceptions should end up being a 4** error (or occasionally a 5** error, depending) but unexpected exceptions should always be a 5** error.

    To put it in finer grained terms, you're saying that expected exceptions should be exogenous (and vexing, in your case, although vexing exceptions are a sign of a library design flaw), while unexpected exceptions are fatal and boneheaded?


  • sockdevs

    sounds about right to me. if the reason for the error is on the user you want a 4XX (Client error) response code. if the reason for the error is external to the user you want a 5xx (Server error) response code



  • @dkf said:

    as a Java programmer

    @dkf said:

    Expected exceptions are the ones that you should deal with (whatever that means) and that should include all exceptions due to the service user giving you bad input; those aren't a service failure, they're a moron/asshat problem.

    It sounds like you are talking about Java's "checked exceptions". The .Net design team isn't terribly impressed with the idea.


Log in to reply
 

Looks like your connection to What the Daily WTF? was lost, please wait while we try to reconnect.