• q (unregistered)

    Frist feelings: not filling it.

  • Darkenon (unregistered)

    Technically notFilled is a good name for the method, but they should've still used isNullOrEmpty

  • Helten (unregistered) in reply to Darkenon

    Technically, "notFilled" is a very bad name, because it does the exact opposite of what one would expect.

  • (nodebb)

    I like how the word filled is used here. It's as if a string object is this empty container (vessel) like a bottle which needs to be filled with a liquid or something. Does filled mean fully filled? Or partially filled?

  • (nodebb)

    no, notFilled is a bad name, always go with positives in this case Filled, it gets rid of ambiguity in interpretation and prevents double negatives. notFilled is just double plus un-good.

  • IA (unregistered)

    That's why the naming of variables and methods should be positive. We have operator ! to negate.

  • Foo AKA Fooo (unregistered)

    "This code, as written, would fail with an ArgumentException if you did supply an email, password, or user key."

    So how is it not impossible that they didn't fail to not notice this during any and every single test? Did they never enter nothing into none of those fields? Or didn't they release this wrapper library without testing nothing at all?

  • (>'-')> (unregistered)

    Don't extension methods seem like a /really/ bad idea? In terms of breaking various abstractions I mean. It's as though MS is appealing to programmers' worst instincts, just to attract developers to their language (and IDE) and force other languages to choose between following their lead and doing the Right Thing.

    Python monkey patching doesn't count because Python doesn't count as a proper language.

  • Erwin (unregistered)

    All tests returned notFailed so they went ahead and released the wrapper library.

  • LCrawford (unregistered)

    | Don't extension methods seem like a /really/ bad idea?

    No, they're a great idea if applied properly - for example porting a VBA application with extensive string manipulation. Very readable even without tracking down the extension method.

    (as long as they're not the opposite behavior of the function name as in this example)

  • (nodebb) in reply to (>'-')>

    Extension methods don't break encapsulation. They don't have access to the private/protected/internal members of the class. An extension method is just syntactic sugar on top of a static method.

    The only difference between

    class MyExtensions 
{
    public static bool notEmpty(this string str) { /* Do stuff */ }
}

    and

    class MyExtensions 
{
    public static bool notEmpty(string str) { /* Do stuff */ }
}

    is that the first can be called like string s = "foo"; bool b = s.notEmpty();, where the second needs string s = "foo"; bool b = MyExtensions.notEmpty(s);

  • (nodebb) in reply to Erwin

    All tests or BOTH tests? I think a little more testing may have been required.

  • fa (unregistered)

    Can you call extension method on null values in C#? I'm not a C#er, but would have expected a NullReferenceException...

    IsNullOrEmpty is defined to take a string argument, seemingly for this reason.

  • Brad (unregistered)

    As Homer said, "I'm not not licking toads."

  • Alipha (unregistered) in reply to fa

    Yes, you can call an extension method on a null object, which would be confusing.

  • jd (unregistered) in reply to fa

    afair from technical high school you totally can. This way, you can even "avoid" such exceptions/errors by checking nulls in the extension method and drive people mad! :D

    Reason it is allowed is that it is still allowed to call it like a normal static method, even if it has the "this" modifier on the first argument.

    That's just memory for the final exams though, it may be wrong.

  • Little Bobby Tables (unregistered) in reply to Foo AKA Fooo

    "So how is it not impossible that they didn't fail to not notice this during any and every single test? Did they never enter nothing into none of those fields? Or didn't they release this wrapper library without testing nothing at all?"

    I would understand it as that someone had written the wrapper, but never actually used it, let alone tested it.

  • -to- (unregistered) in reply to Foo AKA Fooo

    The check fails if (and only if) email, password, AND userkey are filled. It succeeds with email/password, email/userkey, or... Nothing. which I guess nobody bothered to test for.

  • -to- (unregistered) in reply to -to-

    ...or not. Double negations do things to your brain. I need coffee.

  • (nodebb)

    TRWTF is people that think IsNullOrEmpty is enough. What do you do when some clown just sends spaces?

  • my name is missing (unregistered)

    It really should have returned FileNotFound.

  • Thanatos (unregistered)

    You can call an extension method on a null argument. However, it's considered bad practice and most extension methods will throw an ArgumentNullException if you do.

  • Sole Purpose of VIsit (unregistered) in reply to Alipha

    I think Eric Ray has the final word on this.

    An extension method is just syntactic sugar around a static method that takes advantage of the public (and maybe internal, but not protected and not private) members of the argument supplied, conventionally, as "this." So, obviously, you can pass a null reference, so long as "this" is a reference type. It's no more confusing than calling the equivalent static method with a null reference.

    Now, calling an extension method on "object" would be a tad questionable, I agree. I'd expect the C# compiler to complain if it found a clash between an extension method and a bog-standard class method ... but it's such a horrible idea, I've never tried it, so I don't know.

    Similarly, defining generic extension methods is something probably best left to library writers who are very, very, careful about what they are doing.

    Not that any of this really applies to today's WTF, which is entirely horrible in its own simple and unextended right. Or wrong. Or something.

  • (nodebb)

    The main problem with extension methods is potential name conflicts, if two libraries both try to define string.notFilled.

  • LazyBoot (unregistered) in reply to Zenith

    You use IsNullOrWhiteSpace.

  • Paul Neumann (unregistered) in reply to Sole Purpose of VIsit

    I'd expect the C# compiler to complain if it found a clash between an extension method and a bog-standard class method

    Instance methods take priority and there will be no warning, let alone error.

    library writers who are very, very, careful about what they are doing

    Did you happen to notice today's article?

  • Paul Neumann (unregistered) in reply to Barry Margolin 0

    if two libraries both try to define string.notFilled

    Then the first namespace import gets the invocation, again see above no warning or error.

  • Paul Neumann (unregistered)

    TRWTF: A comment with a link to thedailywtf[dot]com is flagged for moderation.

  • Perri Nelson (unregistered) in reply to Zenith

    Use IsNullOrWhiteSpace instead. That checks for Null, Empty, and all white space.

  • Perri Nelson (unregistered) in reply to Sole Purpose of VIsit

    TRWTF is assuming that library writers are very, very careful about what they are doing.

  • ooOOooGa (unregistered) in reply to Paul Neumann

    Oh, I have had comments flagged for moderation without any links at all.

    I suspect that the moderation system is just taking a random sampling and using statistical analysis to determine whether all comments are valid or not. Within the standard +/- 5% error margins.

  • James (unregistered) in reply to Helten

    It does the opposite of what you would expect; their use case is to fill the string variable with data. Although, I would agree with other commenters that "not" !should be a name prefix.

  • Foo AKA Fooo (unregistered) in reply to Paul Neumann

    Seems quite dangerous to me. So you use an extension method, and some later version of the class adds a method of the same name that behaves slightly differently -- not necessarily as egregious as today's example, doing the exact opposite, but maybe handling edge cases differently. Then your code starts behaving differently, maybe so slightly that you don't realize it until it fails spectacularly. Well, I'd want at least a warning there!

  • Neveralull (unregistered)

    You should always use isNullOrBlank if blank strings are not desirable. This would return true if the string was null, empty, or blank. isNullOrEmpty should only be used if blank strings are OK. You don't need to define your own extension method that does exactly the same thing as the built-in function, obviously.

  • James (unregistered) in reply to James

    Clearly, I need to read more carefully before commenting.

  • Counterexample (unregistered) in reply to Perri Nelson

    ‍

  • (nodebb) in reply to KattMan

    Yes, if there is one piece of good advice that a previous lead dev pushed was to not use not in method names (or a negative) for that reason

    Do not use not. ;-)

  • Mr. Double Negative (unregistered) in reply to Foo AKA Fooo

    I didn't not see what you didn't not do there.

  • Wizofaus (unregistered) in reply to Thanatos

    Most extension methods?? Actually I'd be willing to be most throw a NullReferenceException because they access files/methods of 'this' without checking. But "most" extension methods are written by who-knows-who and does who-knows-what. I will say though being able to use null propagation syntax when calling extension methods is one of the main reasons I use them. "optionalObject?.IsReady() == true" is a good deal nicer than having your code littered with null checks (hello Java).

  • AI (unregistered) in reply to IA

    That's why the naming of variables and methods should be negative. We have operator ! to validate.

  • (nodebb) in reply to Developer_Dude

    "Do not use not" has broader application: in signage and documentation, people have a tendency to remember only the more salient terms, but handle "not"/"no" more as stopwords. "Do not stack untested items on front shelves" becomes "blah stack blah untested blah front shelves". Next thing, untested items are unwittingly being stacked on the front shelves because the rule isn't being triggered. When this is health-and-safety stuff, negative wording is potentially hazardous.

    The requirements are easier to remember and in application need less conscious deliberation when they are phrased in positive language. If you have a "dos" and "donts" list, replace the "donts" with the "dos" that should be done instead.

  • Little Bobby Tables (unregistered) in reply to Watson

    "No smoking!"

    Yes indeed, I'm gasping. (spark, puff)

  • zybex (unregistered)

    This is NOT an extension method in c#: bool notFilled(string str)

    The proper extension method needs to have the "this" keyword, and also needs to be static (and usually public, though that's not mandatory): public static bool notFilled(this string str)

    If it's compiling, then there must be a proper extension method somewhere which is being called instead of the posted one (which is buggy as described). Anyway, trying to call an extension method on a null string will still result in an exception. That's why IsNullOrEmpty is not an extension method but a static class method: if (string.IsNullOrEmpty(str)) { ... }

  • zybex (unregistered) in reply to zybex

    [repost with better formatting]

    This is NOT an extension method in c#:

    bool notFilled(string str)

    The proper extension method needs to have the "this" keyword, and also needs to be static (and usually public, though that's not mandatory):

    public static bool notFilled(this string str)

    If it's compiling, then there must be a proper extension method somewhere which is being called instead of the posted one (which is buggy as described). Anyway, trying to call an extension method on a null string will still result in an exception. That's why IsNullOrEmpty is not an extension method but a static class method:

    if (string.IsNullOrEmpty(str)) { ... }

  • I can be a robot if you want me to be (unregistered) in reply to KattMan

    Perhaps there's an in-house style which says all variable names must be camelCase, so "filled" wouldn't be allowed but "notFilled" would.

  • Paul Neumann (unregistered) in reply to zybex

    trying to call an extension method on a null string will still result in an exception.

    false

    class Program {
	void Main() {
		string myString = null;	
		Console.WriteLine($"Is `myString` filled in?: {myString.IsFilledIn()}");
	}
}

static class StringExtensions {
	internal static bool IsFilledIn(this string candidate) =>
		!string.IsNullOrWhiteSpace(candidate);
}

    Is myString filled in?: False

  • guest (unregistered)

    Nobody mentioned another problem with this code sample: the name "instantiate" for the first method. The method doesn't instantiate anything except an ArgumentException and it only does that if one or more of the parameters are not empty or null. If the name were "checkInstantiation" it would a little better in that it would indicate what the method was apparently trying to do.

  • Pedro (unregistered) in reply to Paul Neumann

    Interesting, I thought it would throw a null exception. Anyway, the other point stands: the "this" keyword is missing, so that was not the method being called.

  • markm (unregistered) in reply to I can be a robot if you want me to be

    "Perhaps there's an in-house style which says all variable names must be camelCase, so "filled" wouldn't be allowed but "notFilled" would."

    So call it "isFilled", which also correctly describes what the function does (2nd best). Or swap "return true" and "return false" so it's not doing the opposite of what the name says and probably generating a bug everytime the function is used (3rd best, because we're all more likely to misuse a negative-name). Or read the string library documentation and use one of the existing, thoroughly validated functions rather than re-inventing it (best).

  • Hieronymus Cow-Herd (unregistered)

    As indicated by the ellipsis before the method end, this is not the full content of the "instantiate" method - just part of the argument validation preamble.

Leave a comment on “This Null Leaves Me Feeling Empty”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article