Comment On When an Enum's an Enum

Bahahahaha!

I'm gonna write them an email in Rich Text so I can break their puny code.

Do they ever look at the documentation ?
They would have found Enum.IsDefined() in less than 1 minute...

public static EmailFormat FromEmailsFormat( EmailsFormat emailsFormat )

My head, it hurts

Isn't it possible the function is meant to guard against some other developer adding new values to the enum and thereby breaking whatever function is using FromEmailsFormat() as a check?

Not that I am saying this is the optimal way to accomplish this...

What is the WTF here?

In most computers the enum will be stored in a 1,2,4, or 8 byte area of memory. And of those 256, 65536, 4+ billion or 16+ -billion-billion-some possible values, only TWO of them are defined values.

So any good program should definitely inspect the incoming parameter for validity.

Real careful languages, like Pascal and Ada do automatic range-checking at the point of call on all enum-like values.

I don't know if this is due to anonymisation, but it looks like this is actually a method to convert between two enums - EmailFormat and EmailsFormat.

As has already been pointed out, it would have been much more sensible to use Enum.IsDefined() (assuming of course that this is C# code we're looking at here). However, many people don't realise that an Enum won't necessarily contain one of the values defined:

using System;



namespace Test

{

	public enum TestEnum

	{

		This,

		That,

		TheOther

	}

	

	public class TestClass

	{

		public static void Main() {

		

			TestEnum te;

			

			te = TestEnum.That;

			Console.WriteLine("{0}: {1}", te.ToString(), Enum.IsDefined(typeof(TestEnum), te));

		

			te = (TestEnum)100;

			Console.WriteLine("{0}: {1}", te.ToString(), Enum.IsDefined(typeof(TestEnum), te));

		}

	}

}

The code converts from Email<b>s</b>Format to EmailFormat. It's glue code, most likely there as a result of integrating a slightly incompatible module into the program. There's a million situations in which you'd want to do something like this.

Ancient_Hacker:

What is the WTF here?

In most computers the enum will be stored in a 1,2,4, or 8 byte area of memory. And of those 256, 65536, 4+ billion or 16+ -billion-billion-some possible values, only TWO of them are defined values.

So any good program should definitely inspect the incoming parameter for validity.

Real careful languages, like Pascal and Ada do automatic range-checking at the point of call on all enum-like values.

Look at the function more closely. It doesn't do ANYTHING except check that you only use the two currently defined values. That is, it only gets in the way when you want to maintain.

It DOES make sense to do strict checks like this before you use the enum, but it's not the sort of thing you can abstract out!!

Strilanc:

Look at the function more closely. It doesn't do ANYTHING except check that you only use the two currently defined values.

Actually, as has been pointed out more than once already, it does do something. It accepts an EmailsFormat (plural on Emails) and returns an EmailFormat (singular on Email).

It's a conversion routine. Granted, it could have been written more eloquently, but still, it is doing something.

I suppose the WTF is that nobody added a comment to document what the function was doing. They may have thought it was obvious, but clearly the submitter missed that it was converting between different enums.

I wonder if they'll read these comments and go back and document the code...

Ben:

The code converts from Email<b>s</b>Format to EmailFormat. It's glue code, most likely there as a result of integrating a slightly incompatible module into the program. There's a million situations in which you'd want to do something like this.

You're absolutely right; it's converting an value from one Enum to another (EmailsFormat -> EmailFormat). Didn't see that first time.

In which case, the function could be reduced to

return (EmailFormat)Enum.Parse(typeof(EmailFormat), Enum.GetName(typeof(EmailsFormat), emailsFormat);

assuming of course that there was 1-to-1 parity with the defined names in the enumerations.

Of course, it might make more sense to use an explicit type conversion definition on EmailFormat to convert from EmailsFormat, but that's debateable.

The real WTF is that they use HTML as an E-Mail format.

the real wtf is everyone above me making this way too complex. Unless you're trying to "impress" the person who relaces you someday with impossible code, and maybe they are so terrible, they think its awesome complex code. Whereas I would be like, wow, I need to submit this guys code to WTF, he made it way too complex.

wtf:

the real wtf is everyone above me making this way too complex. Unless you're trying to "impress" the person who relaces you someday with impossible code, and maybe they are so terrible, they think its awesome complex code. Whereas I would be like, wow, I need to submit this guys code to WTF, he made it way too complex.

I want some of what you've been smoking.

benryves:

I don't know if this is due to anonymisation, but it looks like this is actually a method to convert between two enums - EmailFormat and EmailsFormat.

Yes it does. So some ppl here need some reading skills ;)

Nowhere in the Codesnippet is the value of EmailFormat.HTML defined...

Just imagine EmailFormat to be defined like this:

public enum EmailFormat
{
Plain,
ExtraFunky,
HTML,
Text
}

And this makes sense... Well except for the funky emails ;)

FredSaw:

wtf:

the real wtf is everyone above me making this way too complex. Unless you're trying to "impress" the person who relaces you someday with impossible code, and maybe they are so terrible, they think its awesome complex code. Whereas I would be like, wow, I need to submit this guys code to WTF, he made it way too complex.

I want some of what you've been smoking.

george bush

all my professors in college stressed readability (at least the ones I respected, anyway)...

I remember once a prof showing the class an example of some bad coding practices. All the variables, classes and objects were named after autobots. Seeing the OptimusPrime class (derived from the parent class FortressMaximus) being invoked and calling it's 'siren' and 'bumbleBee' methods definitely got some laughs out of the class. She said that it was no joke, this was actually submitted, in a 300-level class, and that student dropped out of the class after failing that project. I think OptimusPrime was a implementation of the Sieve of Eratosthenes, which I found to be rather clever.

IceFreak2000:

In which case, the function could be reduced to

return (EmailFormat)Enum.Parse(typeof(EmailFormat), Enum.GetName(typeof(EmailsFormat), emailsFormat);

Right. Although it's Java, isn't it?

Maybe we need to make a game of guessing the language of the example code.

all my professors in college stressed readability (at least the ones I respected, anyway)...

I remember once a prof showing the class an example of some bad coding practices. All the variables, classes and objects were named after autobots. Seeing the OptimusPrime class (derived from the parent class FortressMaximus) being invoked and calling it's 'siren' and 'bumbleBee' methods definitely got some laughs out of the class. She said that it was no joke, this was actually submitted, in a 300-level class, and that student dropped out of the class after failing that project. I think OptimusPrime was a implementation of the Sieve of Eratosthenes, which I found to be rather clever.

Enum.IsDefined() was introduced in .NET 2.0. This code could have been written against the 1.1 framework.

The real WTF should be that they have two different but nearly identical enums, even down to the name.

rdrunner:

public enum EmailFormat
{
Plain,
ExtraFunky,
HTML,
Text
}

My extra-funky emails usually start with something like, "Hi rember me we were chating the other day i finaly got my pictures up if you wanna see go to www.hotasianbabes.com see you their.

Nope, not Java. C#.

Brandon:

Nope, not Java. C#.

You got me. I should have known by the uppercase method name.

Could have been written as the following assuming it was Java, which it certainly could be despite the function name...

public static EmailFormat FromEmailsFormat( EmailsFormat format ) {

    try {

      return EmailFormat.valueOf( format.name() );

    // generify the exception? :(

    } catch( IllegalArgumentException e ) { 

      throw new Exception( "Unknown EmailsFormat enum value!" );

    }

}

Of course their code is probably marginally faster since it's a simple int comparison instead of a hash lookup. And if EmailFormat has more values which also exist in EmailFormats and we want to exclude all but two values, what they have is a reasonable alternative (and arguably prettier) than adding an if( format.name().equals("Text") || format.name().equals("HTML") ) check to the damn thing.

Would be nice if the JDK had an isDefined() like C# so you didn't need the try/catch everywhere too. :/

Err, I dont' know C#, but I know Java, which also has enums. Why would a IsDefined() method be useful. If you have an instance of an enum, it's an instance of the enum, and it is obviously defined, or do I miss something?
Does C# allow creating instances of an enum which aren't actually valid instances of the enum? That looks pretty strange to me (unless C# enums are in fact integers disguised in enums, as in C, which would be stupid for an OO language as C#)

Great idea, since

return (EmailFormat)Enum.Parse(typeof(EmailFormat), Enum.GetName(typeof(EmailsFormat), emailsFormat);

is much better than

return FromEmailFormat(emailsFormat);

Come on, this is not a WTF at all.

But it is a WTF that C# doesn't guarantee that enums only hold valid values. What's the point? That's like having a boolean that sometimes be a string.

Dan:

all my professors in college stressed readability (at least the ones I respected, anyway)...

I remember once a prof showing the class an example of some bad coding practices. All the variables, classes and objects were named after autobots. Seeing the OptimusPrime class (derived from the parent class FortressMaximus) being invoked and calling it's 'siren' and 'bumbleBee' methods definitely got some laughs out of the class. She said that it was no joke, this was actually submitted, in a 300-level class, and that student dropped out of the class after failing that project. I think OptimusPrime was a implementation of the Sieve of Eratosthenes, which I found to be rather clever.

Dude, that's fvcking awesome! Being a oldschool Transformers geek myself, I would have called the professor a Decepticon-lover and make it a point to include a Megatron and/or Galvatron class/function in a future project.

ahnfelt:

Great idea, since

return (EmailFormat)Enum.Parse(typeof(EmailFormat), Enum.GetName(typeof(EmailsFormat), emailsFormat);

is much better than

return FromEmailFormat(emailsFormat);

Come on, this is not a WTF at all.

But it is a WTF that C# doesn't guarantee that enums only hold valid values. What's the point? That's like having a boolean that sometimes be a string.

You mean like: enum Bool { TRUE,FALSE,FILE_NOT_FOUND };

IceFreak2000:

As has already been pointed out, it would have been much more sensible to use Enum.IsDefined() (assuming of course that this is C# code we're looking at here). However, many people don't realise that an Enum won't necessarily contain one of the values defined:

It turns out that IsDefined is a bad thing for enum range checks. See http://blogs.msdn.com/brada/archive/2003/11/29/50903.aspx or
the Framework Design Guidelines http://msdn2.microsoft.com/en-us/library/czefa0ke(vs.71).aspx.

I would have probably created a method that does the check and returns a bool and let the calling method throw the exception. e.g.

public void SomeMethod(EmailsFormat emailsFormat)
{

if (CheckEmailFormatRange==false)
{
throw new ArgumentOutOfRangeException("emailsFormat")
}
}


public static bool CheckEmailFormatRange(EmailsFormat emailsFormat )
{
switch ( emailsFormat )
{
case EmailsFormat.HTML:
return true;
case EmailsFormat.Text:
return true;
default:
return false;
}
}

The enum could have been defined in a seperate file or jar. Heck, it could even becoming from some 3rd party library!!! Someone could make a change to the enum (for example adding RTF) and then that could would fail. Unless your value is boolean (not even Boolean) assume that not A implies B. Check for B if you want B. This is a case of fail fast coding and I like it.

The enum could have been defined in a seperate file or jar. Heck, it could even becoming from some 3rd party library!!! Someone could make a change to the enum (for example adding RTF) and then that could would fail. Unless your value is boolean (not even Boolean) assume that not A implies B. Check for B if you want B. This is a case of fail fast coding and I like it.

ahnfelt:

But it is a WTF that C# doesn't guarantee that enums only hold valid values. What's the point?

This way you can have

[Flags]

enum SomeFlags

{

  None,

  Flag1 = 0x100,

  Flag2 = 0x200,

  Flag3 = 0x400,

  FlagValueMask = 0xFF;

}



...



SomeFlags zzz = SomeFlags.Flag1 | SomeFlags.Flag2 | (SomeFlags)42;

which is kinda useful sometimes and would be impossible or incur terrible performance penalty if range checks are compulsory.

Well, simply imagine a large system handled by different teams. One uses an enum, and the other group uses another enum type, strings, or I-don't-know which WTF they may have used. They make a function so they're partially shielded from the incoming data, and the Great Day comes when they decide to share a common assembly.

But, hey, if tomorrow they add a new enum type which they don't support? If their beautiful alliance ends in some management fight? They'll be safe with this no-use function, and they won't have to change all calls to it.

are there better ways to do the same thing? Yes. Is this a bad way to do this? I'm not sure...

As an aside, the code in the snippet actually is valid in Java. Nothing there, except maybe the uppercase method name (which is more style than anything), points to C# explicitly.

FredSaw:

wtf:

the real wtf is everyone above me making this way too complex. Unless you're trying to "impress" the person who relaces you someday with impossible code, and maybe they are so terrible, they think its awesome complex code. Whereas I would be like, wow, I need to submit this guys code to WTF, he made it way too complex.

I want some of what you've been smoking.

hey, mee too! Nice joke! I had a big laugh :)

CAPTCHA: kungfu, WTF?

SomeFlags zzz = SomeFlags.Flag1 | SomeFlags.Flag2 | (SomeFlags)42;

This is useful, indeed, but why call it an enum? Call it a BitSet, but not an enum.

LAME!!!

JB:

SomeFlags zzz = SomeFlags.Flag1 | SomeFlags.Flag2 | (SomeFlags)42;

This is useful, indeed, but why call it an enum? Call it a BitSet, but not an enum.

Precisely. I mean, why not go the whole hog and declare every variable as Object, casting where necessary? Oh, I see the JVM already does that...

I see some of the comments are sinking, as usual, to a level of WTFery somewhat below the OP (which is at least a praiseworthy, if doomed, attempt to enforce correctness).

There's a lot of assumptions we have to make here, but I would have thought that, anonymised or not, and C#/Java/C++/whatever, the sensible pattern for this code is either

(a) use the damn language support if available or
(b) explicitly tie the data and code together in a class.

struct OurEnum

{

    typedef enum {/* values */} TheirEnum;

    OurEnum (TheirEnum& enum) : enum_ (enum) {}

    // Copy, assignment, etc elided

    TheirEnum operator()() {/* Do checks and throw if reqd */}

private:

    TheirEnum   enum_;

};

Yes, it changes the syntax and potentially requires refactoring. That's the price you pay for correctness. Don't like it? Use the low-level enum without checks.

And it doesn't matter if TheirEnum is defined in another jar, another file, or even another universe. Where you want to use it, you instantiate OurEnum and use that instead. You can't control third-party use.

Actually, I'm more curious about the guy who failed CompSci 300 while using silly (but entertaining) function/class names. Was that actually the reason that he failed? If so, the "professor" has an almighty hair up her ass and should be introduced face-first into the maw of one of those bots.

In ADA, you don't muck around with enums. Instead, you declare new limited-range types based on integers. Sure, you can do that in Pascal, too, but in Ada, you can also make them *not* be assignment-compatible with other int-derived types, so you can actually enforce that invalid values are not allowed. (I think Java and .Net will prevent you from using invalid enum values but I'm not positive, I'll admit.)

benryves:

I don't know if this is due to anonymisation, but it looks like this is actually a method to convert between two enums - EmailFormat and EmailsFormat.

Not seen in this example are the functions that convert an EmailFormat enum to an EmailFormats enum, an EmailsFormat enum to an EmailFormats enum and both EmailsFormat and EmailFormats enums to EmailsFormats enums and vice versa, and those that convert arrays of one to arrays of another.

THIS IS GOOD PRACTICE!!!:

The enum could have been defined in a seperate file or jar. Heck, it could even becoming from some 3rd party library!!! Someone could make a change to the enum (for example adding RTF) and then that could would fail. Unless your value is boolean (not even Boolean) assume that not A implies B. Check for B if you want B. This is a case of fail fast coding and I like it.

Spot on. There might only be two enum values now, but you want it to fail sanely (as opposed to mysteriously later on) if somebody adds another without updating the code properly. Defensive programming.

As an aside, the code in the snippet actually is valid in Java

Seeming as I'm in an incredibly pedantic mood today - no it's not.

If it were java it would have to have a "throws Exception" clause in the method signature...

WTF?

Why are the valid values for the Enum hard-coded. They should be queried from a database and loaded into an array. Then, if your valid values change, you don't have to change your code.

Amateur

Biffa:

As an aside, the code in the snippet actually is valid in Java

Seeming as I'm in an incredibly pedantic mood today - no it's not.

If it were java it would have to have a "throws Exception" clause in the method signature...

Actually, you're wrong. Having a "throws Exception" in the method header only applies if some code other than a "throw" command throws an Exception. It's used mainly in place of try-catch blocks. So yes, that's valid Java.

Coding for future expansion is okay, if not taken to extremes.

I've seen programs that look up the number of bits per byte and the the number of days per week, by a SQL query of a database. Just in case they've changed since the last page refresh.

Mo:

Spot on. There might only be two enum values now, but you want it to fail sanely (as opposed to mysteriously later on) if somebody adds another without updating the code properly. Defensive programming.

OK, how about instead of this, in your code you just checked if what you're passed equals HTML or equals text? I don't see that the enum adds any value here whatsoever.

jgr4:

I suppose the WTF is that nobody added a comment to document what the function was doing. They may have thought it was obvious, but clearly the submitter missed that it was converting between different enums.

I wonder if they'll read these comments and go back and document the code...

Do what to the code?

Ancient_Hacker:

Coding for future expansion is okay, if not taken to extremes.

I've seen programs that look up the number of bits per byte and the the number of days per week, by a SQL query of a database. Just in case they've changed since the last page refresh.

Worked in a place like that once. The ERP had number of days in a week, months in a year, days in each month, names of the days of the week, and the current year were all stored in a DB.

You have *no* idea how hard it was to resist adding an 8th day to the week and calling it Mattday.