Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Text table #109

Merged
merged 7 commits into from Apr 15, 2014
Merged

Text table #109

merged 7 commits into from Apr 15, 2014

Conversation

JakeGinnivan
Copy link
Member

No description provided.

exampleTable.ElementAt(1).GetExampleValue(0, typeof(int?)).ShouldBe(null);
exampleTable.ElementAt(1).GetExampleValue(2, typeof(ExecutionOrder)).ShouldBe(ExecutionOrder.Transition);
var argException = Should.Throw<ArgumentException>(() => exampleTable.ElementAt(1).GetExampleValue(0, typeof(int)));
argException.Message.ShouldBe("Cannot convert <null> to Int32");
Copy link
Member Author

Choose a reason for hiding this comment

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

We maybe should put row number, column header name into this exception?

Copy link
Member

Choose a reason for hiding this comment

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

Great idea.

Copy link
Member

Choose a reason for hiding this comment

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

I don't have the context of this long discussion, but just wondering if it might be possible to have this:

exampleTable.GetExampleValue<int>(0,1)

rather than this:

exampleTable.ElementAt(0).GetExampleValue(1, typeof(int))

Copy link
Member Author

Choose a reason for hiding this comment

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

@mwhelan We can't use generics because we get the type from MethodInfo's and other reflection based types, also by the time the Scenario is involved it only has an Example not an ExampleTable (we create a scenario for each Example)

Only the tests will have code like this, the BDDfy codebase will not drill down like this

@JakeGinnivan
Copy link
Member Author

So, things to be done:

  • Example should not be IEnumerable and expose a Values property instead which is IEnumerable<ExampleValue>
  • Should refer to each value as a Cell, rather than a column when enumerating
  • Remove unneeded curlies
  • Move Parse onto ExampleTable as a static method

The ICollection on ExampleTable is not so much for reading rows, it is more about being able to do this:

new ExampleTable("Header1", "Header 2")
{
    { 1, 2 } // We are using collection initialisers here, can they work without implementing ICollection?
}

exampleTable.ElementAt(0).GetExampleValue(1, typeof(int)).ShouldBe(2);
exampleTable.ElementAt(0).GetExampleValue(2, typeof(decimal)).ShouldBe(3m);
exampleTable.ElementAt(1).GetExampleValue(0, typeof(string)).ShouldBe(null);
exampleTable.ElementAt(1).GetExampleValue(0, typeof(int?)).ShouldBe(null);
Copy link
Member

Choose a reason for hiding this comment

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

Need to verify the date at [1, 1]

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Haha, comments got in the way. Thanks :)

@MehdiK
Copy link
Member

MehdiK commented Apr 15, 2014

That should be it mate. Thanks :)

@MehdiK
Copy link
Member

MehdiK commented Apr 15, 2014

Also I can do it if you want.

@JakeGinnivan
Copy link
Member Author

I dont mind making the suggested changes. If you wanted to pick up one of the other tasks that would be awesome though :)

@MehdiK
Copy link
Member

MehdiK commented Apr 15, 2014

I love your style @JakeGinnivan. I really do :)

I will pick one up tomorrow

@JakeGinnivan
Copy link
Member Author

K, fixed all the issues raised

@MehdiK
Copy link
Member

MehdiK commented Apr 15, 2014

Cool. Thanks for the fixes mate. I'll do a final review and merge this soon.

MehdiK added a commit that referenced this pull request Apr 15, 2014
@MehdiK MehdiK merged commit 096e753 into TestStack:examples Apr 15, 2014
@JakeGinnivan JakeGinnivan deleted the TextTable branch April 15, 2014 17:54
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.

None yet

3 participants