Skip to content

Conversation

@AlexEndris
Copy link
Contributor

I put the two methods into the ObjectAssetions, since Enum.HasFlag should be supported on all platforms that fluentassertions supports.

Hope the tests are extensive enough, unfortunately I couldn't run them for all platforms.

Thanks for creating and maintaining this project! It's always a pleasure to use it!

PS: fixed a tiny typo in one comment. :)

@AlexEndris
Copy link
Contributor Author

I've also written up a snippet for the documentation here on github, in case you accept this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better use the new internal Given and Then methods on the AssertionScope. If you don't, and this is used within the equivalency API, the first Should().BeOfType<> would not throw immediatelly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So essentially something like so:

Execute.Assertion
    .BecauseOf(because, reasonArgs)
    .ForCondition(Subject.GetType() == expectedFlag.GetType())
    .FailWith("some text")
    .Then
    .Given(() => Subject as Enum)
    .ForCondition(enum => enum.HasFlag(expectedFlag))
    .FailWith("some other text");

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes!

dennisdoomen added a commit that referenced this pull request Mar 25, 2015
Added support for HaveFlag and NotHaveFlag
@dennisdoomen dennisdoomen merged commit 461674b into fluentassertions:develop Mar 25, 2015
@dennisdoomen dennisdoomen mentioned this pull request Mar 25, 2015
@Meir017
Copy link
Member

Meir017 commented Mar 25, 2015

Shouldn't there be a separate class for Enum assertions? it will improve the performance by not having to test if the expected value is an enum.

something like this:

public class EnumAssertions
    {
        public readonly Enum Subject;

        public EnumAssertions(Enum value)
        {
            Subject = value;
        }

        public AndConstraint<EnumAssertions> HaveFlag(Enum expectedFlag, string because = "",
            params object[] reasonArgs)
        {
            Execute.Assertion
                .ForCondition(Subject.GetType() == expectedFlag.GetType())
                .FailWith("Expected the enum to be of type {0} type but found {1}{reason}.", expectedFlag.GetType(), Subject.GetType())
                .Then
                .ForCondition(Subject.HasFlag(expectedFlag))
                .FailWith("The enum was expected to have flag {0} but found {1}{reason}.", Subject, expectedFlag);

            return new AndConstraint<EnumAssertions>(this);
        }
        public AndConstraint<EnumAssertions> NotHaveFlag(Enum expectedFlag, string because = "",
            params object[] reasonArgs)
        {
            Execute.Assertion
                .ForCondition(Subject.GetType() == expectedFlag.GetType())
                .FailWith("Expected the enum to be of type {0} type but found {1}{reason}.", expectedFlag.GetType(), Subject.GetType())
                .Then
                .ForCondition(!Subject.HasFlag(expectedFlag))
                .FailWith("Did not expect the enum to have flag {0}{reason}.", Subject, expectedFlag);

            return new AndConstraint<EnumAssertions>(this);
        }
    }

@dennisdoomen
Copy link
Member

Hmm, not sure C# allows you to create extension methods on enumerations.

@Meir017
Copy link
Member

Meir017 commented Mar 25, 2015

It does 👍 the code I wrote above compiles

@dennisdoomen
Copy link
Member

Those are not extension methods. What does your 'Should' look like?

@Meir017
Copy link
Member

Meir017 commented Mar 25, 2015

public static class EnumAssertionsExtentions
    {
        public static EnumAssertions Should(this Enum actualValue)
        {
            return new EnumAssertions(actualValue);
        }
    }

@AlexEndris
Copy link
Contributor Author

I tried that. The problem there was, that I had to replicate all the stuff that is already in ObjectAssertions, like Be and the type check.
Then I thought it might be very convenient if you can have the actual enum in the "Be"... But that resulted into having EnumAssertions and since there is no support in C# for the generics constraint on Enums (or ValueType for that matter) I would only be able to do this partially by putting the constraints to what Enum implements (IFormattable, IComparable). This would then lead to the compiler taking the wrong "Should" in some cases.
That's at least why I chose to not implement it in EnumAssertions. (I already had the whole implementation)

Apart from that. What is the actual performance improvement? Would be interesting.

@Meir017
Copy link
Member

Meir017 commented Mar 25, 2015

you don't need to check every time that the expected is an Enum object and this way you have only two steps instead of three steps

  1. assert that the expected is the same enum as the subject
  2. assert that the condition is true (HaveFlag / NotHaveFlag)
    you don't need to check that the expected is an Enum

I personally prefer creating an extension for any enum I create to make it easier (skips the step that asserts that the expected is the same enum as the subject)

@AlexEndris
Copy link
Contributor Author

You said what you did, yes. But my question was, in terms of performance, what is the improvement?

Also another thing I'd like to add. There is also the question of how to else enums can be compared. So would there be an overload for int etc. if, for what ever reason, you'd like to check the value, or the string that is being produced. I just felt that there is no significant benefit to creating EnumAssertions, but some functionality that is already there had to be duplicated. Enums are a tricky case in this, I think. But I do agree that generally, thinking about responsibility, it should be in an own class!

@Meir017
Copy link
Member

Meir017 commented Mar 25, 2015

maybe some future version of .Net will have Enum constraints 😦

@AlexEndris
Copy link
Contributor Author

As a response to your edit:

I personally prefer creating an extension for any enum I create to make it easier (skips the step that asserts that the expected is the same enum as the subject)

I would love that in code, I tried to generalise that in having "EnumAssertions", like I said above. But this unfortunately isn't possible. Then, in my book, it would be significant enough to introduce the class.

And about enum constraints, check this: http://stackoverflow.com/a/1331811/978594
I fear, this won't happen that quickly.

@adamvoss
Copy link
Contributor

@AlexEndris
Copy link
Contributor Author

Huh, interesting. Can you give me some insight to this? Mono related? AOP? Or are these constraints in .Net but just not added to the compiler? (which I kind of doubt)

Edit: AOP it is, apparently. I really have to give this a try! Thanks for that gorgeous link!

@adamvoss
Copy link
Contributor

ILWeaving (which Fody does) is used for AOP. However, in this case the constraints are supported by the CLR, but not part of C# so Fody just writes out the IL code for the constraint.

@AlexEndris
Copy link
Contributor Author

I've just gave it a try, both in fluentassertions and in a normal pcl. For whatever reason it won't output the changed assembly for FluentAssertions.Core. I created a pcl project with the very same profile as FA.Core, where it worked flawlessly. Then I thought might be because of the StyleCop target in the project, so I tried it without it. Still no suuccess. Any idea why this might be? Maybe I've missed something?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants