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 aThread.Sleep
has crashed I guess. The emptyFinally
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
-
The comments you can't have; those would be processed by a documentation generator.
The rest are deffo 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?
-
Is Mandant a DC Comics supervillain?
That'd be awesome. I'll start naming my variables after superheroes and villains right away.
-
Hmm… OK, you can have the comments too ;)
-
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!
-
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 factor of typingseventeen
instead of17
is extraordinary, but stupider things have happened…
-
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.
-
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.
-
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 event in the first place?
-
Defensive programming. More failsafes is always* good.
*pedantic clarification: not actually always, but most of the time.
-
Thread.Sleep(sleepTimeOut)
Return TrueI 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?
-
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.
-
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
loopfunction!
-
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.
-
-
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
orMy.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.
-
If you're working within Visual Studio, it autogenerates a
Settings.cs
orMy.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 ownApp.config
/Web.config
…
-
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?
-
IIUC, it doesn't need a recompile; the autogen class reads the
*.config
-
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; } }
-
This is much clearer
Also wrong.
TryParse
takes anout
parameter, notref
, so it has to assign before exiting. And it assigns zero.Unless things are different in VBland, but I don't think so...
-
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
-
The fact that it returns a boolean (for what?).
So you can tell if an exception was thrown inside the function?
-
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 usesTryParse
and throws an exception when it fails. Otherwise you'd be getting an entirely avoidable performance penalty oftry/catch
.
-
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
-
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.
-
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?)
-
Even better:
If Not Integer.TryParse(ConfigurationManager.AppSettings("SleepTimeOut"), sleepTimeout) Then
Throw New ApplicationException("Setting SleepTimeout cannot be parsed as an integer.")
End IfNo, 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…
-
No, better would be to use the method that does that for you.
That was my first suggestion.
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.
-
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.
-
```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.
-
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.
-
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.
-
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...
-
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.
-
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.
-
I'm of the opinion (as a Java programmer) that there are expected exceptions and unexpected ones.
-
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?
-
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
-
as a Java programmer
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.