• 3rik (unregistered)

    This is the kind of code where it is not clear if the guy either deserve a raise or to be fired.

  • Harrow (unregistered)

    It was hard to write. It should be hard to read.

  • Swamp Thang (unregistered) in reply to 3rik

    Probably both, or at least promoted to a position where they can't do anymore damage.

  • A Nonny Moose (unregistered)

    We could call this kind of code “dysfunctional programming”.

  • (nodebb)

    It's not "Functional Programming", it's "Job Security Programming".

    And just like every other style of programming it never works quite as well as you would hope.

  • Barry Margolin (github) in reply to Harrow

    But it shouldn't be hard to write.

    if ((int StatusCode) >= 200 && (int StatusCode) <= 299) {
  ResultMessage rm = (ResultMessage) Response;
  if (rm.resultMessageCode == ResultMessageCode.Done) {
    return true;
  }
  throw new NotImplementedException;
} else {
  return false;
}

    Addendum 2023-08-10 10:28: Yes, I realize I got the (int) cast syntax wrong. How about the WTF that the "Edit" link doesn't actually allow you to edit?

  • dpm (unregistered)

    The day has finally come when I completely agree with Remy on a WTF. Hallelujah!

  • Peter Smith (github)

    There's actually a problem with the compare to 299, and arguably two problems: we can all agree that a 200 status here is 100% OK. But if the server returns, says, 207 WEBDAV MULTI STATUS, is that really a correct status? I'm more likely to get this from an incorrectly configured server than a functioning one. The Windows docs themselves simply use a comparison to 200 in their example of a good error code.

    The "arguable" problem is the readability of the compare: I nearly wrote a paragraph on why the compare to 299 was incorrectly excluding 299 as an OK status code. But that was because I read the code wrong several times until I did a triple check on the syntax. Thank goodness I deleted my complaint, and nobody will know I misread a few lines of code :-)

  • (nodebb)

    Couldn't agree more on >= vs <=. I just was not seeing the distinction. It's lkie raeding thsoe senetnces wtih most wrods misppelled execpt for frist and lsat leetter.

  • LCrawford (unregistered)

    I'm surprised no one has noted the

    fist => frist

  • dpm (unregistered) in reply to Peter Smith

    RFC 1945 states "The first digit of the Status-Code defines the class of response" and "2xx: Success - The action was successfully received, understood, and accepted." so I can't rely blame the coder for that.

    RFC 4918 states that in certain cases of errors, 207 can be returned, which confuses me (but not enough that I'm going to research it further).

  • (nodebb)

    I wished C# would let you write a single condition as 200 <= (int)StatusCode <= 299, and the compiler would break that up for you.

    But my brain is breaking on the fact that they wrote some ass-backwards version of 299 >= (int)StatusCode >= 200. Who does that? The only way this makes sense to me is if they write right-to-left in their preferred (written) language, including how they do maths by hand.

  • (nodebb)

    This looks very boorish and pueril. Clearly one of these many programmers with no regards for coworkers who will need to maintain this code later.

    Also it seems like this place doesn't have any code reviews and coding standards to curb this behavior upfront.

  • löchlein deluxe (unregistered)

    Remy, write down the truth tables for =＞ and ＜= sometimes… the impleqations are endless!

  • (nodebb)

    It initializes the member property IsSuccessCode using a lambda

    Actually, it's a getter, not an initialization at all, every time the property is accessed it evaluates the expression.

    An initializer would look like this:

    public bool IsSuccessCode { get; } = 299 >= (int)StatusCode && (int)StatusCode >= 200 &&
	Response switch
	{
		ResultMessage rm => rm.ResultMessageCode == ResultMessageCode.Done,
		_ => throw new NotImplementedException()
	};

    Also, I wouldn't so much say this is "functional" programming so much as using lambda syntax.

    Anyway, for readability (while maintaining the original behaviour), I would have written the posted code as:

    public bool IsSuccessCode
{
    get
    {
        if (Response is not ResultMessage rm)
        {
            throw new NotImplementedException();
        }

        return (int)StatusCode >= 200 &&
               (int)StatusCode <= 299 &&
               rm.ResultMessageCode == ResultMessageCode.Done;
    }
}
  • TrayKnots (unregistered)

    Got confused on the => <= as well. But...

    That's a clear case of it is bad because you're not used to it. If you were used to it, you wouldn't do a double take. Bad because new, not bad because bad.

  • Pattern (unregistered) in reply to prueg

    We could do StatusCode is >= 200 and <= 299 if that makes you happy

  • (nodebb) in reply to Peter Smith

    HTTP defines all 2xx error codes as success codes; this code is there absolute correct to not only handle 200.

    • Informational responses (100 – 199)
    • Successful responses (200 – 299)
    • Redirection messages (300 – 399)
    • Client error responses (400 – 499)
    • Server error responses (500 – 599)

    https://developer.mozilla.org/en-US/docs/Web/HTTP/Status

  • (nodebb) in reply to prueg

    Your are not really aware how operators work in C#, are you? :D

    Besides, a between operator is a very bad idea because we already have on available in the non-programming language SQL. And what is so bad about it, well because it's double inclusive and most operations especially when it comes to floating point numbers would actually require an non-inclusive part to avoid rounding issues. So in other words, for a between you would need at least four operators, which is so silly, no major programming language has bothered to do so to my knowledge.

  • (nodebb) in reply to DCRoss

    You are obviously not a C# developer.

    Pattern matching is the recommended way to do this for numerous reasons I won't get into right now; there's tons of tech talk videos from the languages designers themselves online that give a nice overview why and dozens of links to read for the details.

    The only thing wrong with this code is actually that the two conditions are part of the pattern and shouldn't be outside the match.

  • Duke of New York (unregistered)
    Comment held for moderation.
  • (nodebb)

    This is the problem with the increasing sophistication of parsing technology since the 1980s, it's now possible to create languages in which you can write code that requires the consumption of at least two Camberwell carrots to understand.

  • (nodebb) in reply to MaxiTB

    I'm only talking about writing e.g. 0 <= x < 10 as syntactic sugar, and the compiler splits that into 0 <= x && x < 10. You could use any combination of comparison operators (<, <=, >=, >), although you'd want either both less-thans or both greater-thans, not a mix of both (0 > x < -2 is just stupid). It just looks that little bit more obvious what the conditional is testing.

    I would hate to use if (x between 0 and 10), as that isn't immediately clear whether or not it's inclusive on both ends, which often you don't want. Even if it is clearly specified in the documentation what "between" meant.

  • (nodebb) in reply to MaxiTB

    no major programming language has bothered

    Swift half does it. It defines two kinds of range: a closed range and a half open range. In Swift, the condition can be written as (200 ... 299).contains(status) or (200 ..< 300).contains(status).

  • (nodebb) in reply to Peter Smith

    if the server returns, says, 207 WEBDAV MULTI STATUS, is that really a correct status?

    Yes, that is to say, it is a success. Of course, if the server isn't expected to be a WebDAV server, the body of the response may be incomprehensible to the client. But that could happen even with a 200.

  • Apophis (unregistered)
    Comment held for moderation.
  • Gnasher729 (unregistered)

    Http success status codes have different meanings. If I expect status 200 and get status 206, as a developer I want to know what’s going on and then decide if the outcome is something that I can use or not.

  • James (unregistered) in reply to jeremypnet
    Comment held for moderation.

Leave a comment on “A Piece of Switch”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article