New Exercise book-store #465

rpottsoh · 2016-12-06T00:38:41Z

Attempting to create slug for new xpascal exercise.

Create harry-potter

Create description.md

petertseng · 2016-12-06T02:37:26Z

Oh, this exercise looks interesting. As the README makes clear, a few choices for the optimisation. I am interested to see how it will go.

It would be nice if we can have the test cases in a file in this repo too, as https://github.com/exercism/x-common/blob/master/CONTRIBUTING.md#improving-consistency-by-extracting-shared-test-data has it. In this case, the task should only require using the same tests as those in exercism/delphi#14

rpottsoh · 2016-12-06T15:50:27Z

@petertseng thanks for the tip. The json file seems pretty straight forward.

Insti · 2016-12-06T17:19:21Z

exercises/harry-potter/description.md

@@ -0,0 +1,57 @@
+This exercise is based on the exercise of the same name found at cyber-dojo.org


This information is provided in the source section of the metadata and does not need to be repeated here.

Insti · 2016-12-06T17:20:58Z

I'd like to see this exercise called something more generic like 'book-discounts'. The books being Harry Potter books is irrelevant to the problem.

Insti · 2016-12-06T18:26:17Z

exercises/harry-potter/description.md

+
+If you buy 4 different books, you get a 20% discount.
+
+If you go the whole hog, and buy all 5, you get a huge 25%


There are 7 books in the Harry Potter series.

Could apply to Game of Thrones, a 5 book series I think.....

Trying to make this more generic.....

was harry-potter is now book-store

Insti · 2016-12-08T17:26:34Z

exercises/book-store/description.md

+Note that if you buy, say, four books, of which 3 are different titles, you get a 10% discount on the 3 that 
+form part of a set, but the fourth book still costs 8 EUR. 
+
+Your mission is to write a piece of code to calculate the price of any conceivable shopping basket (containing only books from the same series), giving as big a discount as possible.


This is a bit confusing, it sounds like it wants me to calculate every possible price/combination. But further down it seems to indicate it just wants a method that given a list of books will calculate the cheapest total price for those books.

I see your point, however I don't really see a better way to word this. At this point its is just copy/pasted from cyber-dojo. I suppose, as it is, it adds to the challenge of the problem. I know when I originally worked this problem I ended up disregarding this piece, but I think it is needed initially to help the reader understand the requirements.

if were were to change it, I would suggest just changing "any conceivable" to "a given", which should make it clear that the basket is the input.

At this point its is just copy/pasted from cyber-dojo.

What is the licensing on that text? We may need to rewrite the text (or at least heavily edit it), even though we're referring to cyber-dojo as the source.

@kytrinyx I've been scouring the cyber-dojo site and I haven't found anything at this point that makes the statement one way or the other that the exercise is protected. In general the only licensing that I see is regarding the comercial use of the site as a whole. Some of the other exercises there site other sites as sources for complete instructions. Some of the exercises have NO instructions and only reference the source site.

Should we ask for permission to use the text?

I've been scouring the cyber-dojo site and I haven't found anything at this point that makes the statement one way or the other that the exercise is protected.

Anything that doesn't have a public license is copyrighted by default, and we can't use it.

The cyber-dojo website is released under a couple of variants of the BSD license. Does it cite another source for the book sale exercise?

@kytrinyx I am still looking. Here is a link to a license, looks like boiler plate. https://github.com/cyber-dojo/start-points-exercises/blob/master/LICENSE.md

I am still looking around. I don't know if this license really grants permission for use of exercises or just relating to the site as a whole.

That license does grant use of the exercise, but only under the same license, which we don't have. The exercises here are under an MIT license.

Insti

Thanks for taking on board the suggestions, this is looking much better.

rpottsoh · 2016-12-08T17:43:20Z

@Insti thanks, I'm trying. It's the first "original" exercise that I have added to exercism. Had I only known.... Kidding, wouldn't be doing this if I didn't want to learn too.

Attempt at producing a canonical-data.json file book-store exercise.

rpottsoh · 2016-12-08T18:55:50Z

I think I have completed what needs to be done. Let me know where I've errored and I'll take care of it.

Insti · 2016-12-08T19:05:55Z

It thinks your json is invalid.

Insti · 2016-12-08T19:06:58Z

exercises/book-store/canonical-data.json

+      },
+      {
+         "description": "basket 112233445512",
+         "basket": "112233445512",


It would be nice if the basket was an array rather than a string.

String type comes with the added benefit of TStringHelper which makes parsing the basket a little easier. For the sake of my example solution I could possibly convert the input to an array, I would just convert it to a string with in the class.

Also added a description for the exercise and added exercise name.

Insti · 2016-12-08T19:11:15Z

exercises/book-store/canonical-data.json

+         },
+         {
+            "description": "TestBasketContainsMyBooks",
+		    "mybasket": "3"


missing comma here

Insti · 2016-12-08T19:12:14Z

Read this comment I wrote on another PR, about test descriptions: #451 (comment)

Insti · 2016-12-08T19:14:24Z

Having a good canonical-data.json file can be harder than writing the original specification.
It might be good to split this off into a separate PR so we can get the initial exercise merged in and once a few tracks have implemented it we can work out what the canonical test data should be.

I will add this back via a seperate PR.

petertseng

I know you are planning to add canonical-data.json back later, though I'll still leave some notes for it, to prepare for the time when it gets added back.

petertseng · 2016-12-08T19:36:34Z

exercises/book-store/canonical-data.json

@@ -0,0 +1,76 @@
+{
+   "#": [
+     "Not sure what to type here, come back to this later."


if there is nothing in particular to add (any notes for anyone who wishes to port this exercise to a track), the entire thing can be deleted.

petertseng · 2016-12-08T19:37:46Z

exercises/book-store/canonical-data.json

+   ],
+   "cases": [
+      {
+         "description": "TestSingleBookPrice",


what is the difference between this and "TestTotalBasketPriceSingleBook"?

If the only function under test in this exercise is "Given this basket, what is its price?" then it seems there is no need to have this one.

In retrospect this probably isn't needed. I have a property that just returns the price of a single book. The test is just confirming that it equals 8.

petertseng · 2016-12-08T19:38:36Z

exercises/book-store/canonical-data.json

+         "expected": 8.0
+      },
+      {
+         "description": "TestBasketContainsMyBooks",


What does this case do?

If the only function under test in this exercise is "Given this basket, what is its price?" then it seems there is no need to have this one.

This statement also seems true for this case.

May be too simple of a test. It is simply echoing back the contents of its basket. It should equal what was placed in the basket.

rpottsoh · 2016-12-08T19:49:17Z

@petertseng I've gone ahead and moved the canonical-data.json to another PR. I am not really clear on how the inner workings of the machine (exercism.io) operate so don't have a solid grasp on what the exact role the three files play (canonical-data.json, description.md, and metadata.yml). At this point I am trying to format the files as best I can to examples from other exercises. In the case of the json file I am not sure what tests should be listed and which shouldn't. I've assumed that they should all be listed. I guess further discussion related to the json file should take place here #470 .

kytrinyx · 2016-12-08T23:54:10Z

I think all the files for the exercise should go into the same PR, (and in a single commit) since they all belong to one, atomic change.

rpottsoh · 2016-12-08T23:59:19Z

That is the way I had it but was encouraged to pull the canonical data out into a separate PR. Sent from my android device.

Insti · 2016-12-09T09:26:53Z

@kytrinyx I asked @rpottsoh to move the canonical data to another PR.

Reasoning:
There are many pre-existing exercises that do not have canonoical-data files.
The description.md and metadata.yml files for this exercise were almost complete and ready to be merged.

By getting the exercise in place it gives multiple tracks an opportunity to implement the problem and help find/explore the required test cases, and find/resolve any issues with the description. Before getting to a place where track implementers can just assume that the canonical-data is complete and everything is ready to go.

petertseng · 2016-12-09T23:17:08Z

I think things are OK. A few minor things but I am not sure if they are blocking.

petertseng · 2016-12-09T23:18:17Z

exercises/book-store/description.md

+
+For example, how much does this basket of books cost?
+
+2 copies of the first book


Note that at https://github.com/rpottsoh/x-common/blob/rpottsoh-patch-1/exercises/book-store/description.md the lines get mashed together. Bullet points?

Or do we expect people to always be reading the raw md file in a text editor rather than rendering? (I always read raw, but I don't know about others)

The MD file gets rendered on the website at e.g. http://exercism.io/exercises/ruby/hello-world/readme

So it seems important to get the formatting to look good rendered too, not just in plain text.

petertseng · 2016-12-09T23:18:45Z

exercises/book-store/description.md

+1 copy of the fifth book
+
+One way of group these 8 books is:
+ 1 group of 5 --> 25% discount (1st,2nd,3rd,4th,5th)


These also get mashed together. Not sure of the best way to deal with it - use backticks?

kytrinyx · 2016-12-10T01:19:51Z

I asked @rpottsoh to move the canonical data to another PR.

@Insti fair enough. I saw the other PR first, and it didn't have a [WIP] to indicate that it would have to be merged after a different PR, so I was unaware of the fact that there was a different PR in progress until I got further down my notifications list.

rpottsoh · 2016-12-12T17:36:43Z

Good question. I will look again with this in mind as well as check other sources that might have the same/similar exercise. Sent from my android device.

rpottsoh · 2016-12-12T19:08:35Z

I see. So is the exercise dead? Sent from my android device.

Made minor grammatical updates. The description is no longer an exact copy of the description found at cyber-dojo.

petertseng · 2016-12-13T19:55:35Z

exercises/book-store/description.md

+
+Resulting in:
+
+- 5 x (8 2.00) == 5 x 6.00 == $30.00


missing minus signs, the (8 2.00) doesn't look right

petertseng · 2016-12-13T19:55:50Z

exercises/book-store/description.md

+Resulting in:
+
+- 5 x (8 2.00) == 5 x 6.00 == $30.00
+- +3 x (8 0.80) == 3 x 7.20 == $21.60


also missing minus sign, should be (8 - 0.80) right?

petertseng · 2016-12-13T19:58:25Z

exercises/book-store/description.md

+- 4 books at a 20% discount
+- +4 books at a 20% discount
+
+Resulting:


should this be "Resulting in" instead of just "Resulting"?

@PeterSeng

@PeterSeng suggestions.

petertseng · 2016-12-14T09:57:46Z

I do not think there are other changes that I would suggest.

I am not competent to discuss copyright, so I make no comment on that.

Before I forget to mention it:

Before merging this PR, please squash commits.

kytrinyx · 2017-01-02T17:16:34Z

In terms of copyright, as long as we don't exactly copy their text word for word, and link to them as a source, I think it is fine.

rpottsoh · 2017-01-03T18:11:15Z

Text was changed a little. Looks like C# and F# have already implemented the exercise. 👍

robkeim · 2017-01-03T19:16:53Z

@rpottsoh thanks to @ErikSchierboom we now have two people that have submitted implementations on the site for the this exercise in both C# and F# 👍

ErikSchierboom · 2017-01-03T19:36:28Z

And that's thanks to @robkeim who implemented the problem!

rpottsoh · 2017-01-03T20:24:54Z

@ErikSchierboom I noticed! Thanks to you and @robkeim for taking the time to get it out there.

Replace hard-coded Bowling tests with test generator (#456) Also updated the example solution to address new test case

rpottsoh added 10 commits December 5, 2016 19:19

Create harry-potter

fc2f3c2

Merge pull request #1 from rpottsoh/rpottsoh-patch-1

cd400f1

Create harry-potter

Create description.md

eac9f63

Merge pull request #2 from rpottsoh/rpottsoh-patch-1

4421deb

Create description.md

Delete description.md

b7a263d

Delete harry-potter

5b53197

Create description.md

f5ecac4

Create metadata.yml

ab3c6f1

Incorparted description information from cyber-dojo

34e908a

updated meta data

3ea936a

Insti reviewed Dec 6, 2016

View reviewed changes

rpottsoh and others added 3 commits December 8, 2016 10:55

Made blurb generic

b05216f

Could apply to Game of Thrones, a 5 book series I think.....

Removed Harry Potter References

c3e2244

Trying to make this more generic.....

Renamed exercise

9f18d96

was harry-potter is now book-store

rpottsoh changed the title ~~New Exercise harry-potter~~ New Exercise book-store Dec 8, 2016

Insti reviewed Dec 8, 2016

View reviewed changes

Added canonical data

9a7e7fe

Attempt at producing a canonical-data.json file book-store exercise.

Insti reviewed Dec 8, 2016

View reviewed changes

Corrected formatting error

611ea3a

Also added a description for the exercise and added exercise name.

Insti reviewed Dec 8, 2016

View reviewed changes

exercises/book-store/canonical-data.json

},

{

"description": "TestBasketContainsMyBooks",

"mybasket": "3"

Copy link

Contributor

Insti Dec 8, 2016

Choose a reason for hiding this comment

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

missing comma here

Added missing comma

d58666e

Removed canonical-data.json

3d70586

I will add this back via a seperate PR.

petertseng reviewed Dec 8, 2016

View reviewed changes

Insti mentioned this pull request Dec 9, 2016

exercise: book-store, canonical data #470

Merged

petertseng reviewed Dec 9, 2016

View reviewed changes

rpottsoh added 2 commits December 12, 2016 14:47

Added some formatting

71d61f1

Rewording Description

37b0fe8

Made minor grammatical updates. The description is no longer an exact copy of the description found at cyber-dojo.

petertseng reviewed Dec 13, 2016

View reviewed changes

Update description.md

2858c29

@PeterSeng suggestions.

rpottsoh self-assigned this Dec 14, 2016

rpottsoh added the documentation label Dec 14, 2016

rpottsoh merged commit 371e55e into exercism:master Dec 14, 2016

rpottsoh deleted the rpottsoh-patch-1 branch December 14, 2016 19:56

emcoding pushed a commit that referenced this pull request Nov 19, 2018

Replace hard-coded Bowling tests with test generator (Fixes #456) (#465)

142093e

Replace hard-coded Bowling tests with test generator (#456) Also updated the example solution to address new test case

		@@ -0,0 +1,57 @@
		This exercise is based on the exercise of the same name found at cyber-dojo.org


		If you buy 4 different books, you get a 20% discount.

		If you go the whole hog, and buy all 5, you get a huge 25%


		For example, how much does this basket of books cost?

		2 copies of the first book

New Exercise book-store #465

New Exercise book-store #465

Conversation

rpottsoh commented Dec 6, 2016

petertseng commented Dec 6, 2016

rpottsoh commented Dec 6, 2016

Choose a reason for hiding this comment

Insti commented Dec 6, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Insti left a comment

Choose a reason for hiding this comment

rpottsoh commented Dec 8, 2016

rpottsoh commented Dec 8, 2016

Insti commented Dec 8, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Insti commented Dec 8, 2016

Insti commented Dec 8, 2016

petertseng left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rpottsoh commented Dec 8, 2016 • edited Loading

kytrinyx commented Dec 8, 2016

rpottsoh commented Dec 8, 2016 via email

Insti commented Dec 9, 2016 • edited Loading

petertseng commented Dec 9, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kytrinyx commented Dec 10, 2016

rpottsoh commented Dec 12, 2016 via email

rpottsoh commented Dec 12, 2016 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petertseng commented Dec 14, 2016

kytrinyx commented Jan 2, 2017

rpottsoh commented Jan 3, 2017

robkeim commented Jan 3, 2017

ErikSchierboom commented Jan 3, 2017

rpottsoh commented Jan 3, 2017

rpottsoh commented Dec 8, 2016 •

edited

Loading

Insti commented Dec 9, 2016 •

edited

Loading