Skip to content

trinary: Make exercise schema-compliant #645

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

Merged
merged 2 commits into from
Mar 7, 2017
Merged

trinary: Make exercise schema-compliant #645

merged 2 commits into from
Mar 7, 2017

Conversation

rbasso
Copy link
Contributor

@rbasso rbasso commented Mar 7, 2017

Related to #625.

"description": "returns the decimal representation of the input trinary value",
"cases": [
{
"description": "trinary 1 is decimal 1",
"property": "toDecimal",
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't the property simply be "Decimal" or even "Base10"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the original file, the test group was named to_decimal, so I just adapted it to make it a valid property according to the JSON Schema: toDecimal.

IMHO, decimal, toDecimal and base10 do not capture what it means. The output is a number here, not a string, so there is no sense talking about what is its base. It is represented in JSON as a decimal, but that is just a convenience for visualization.

if I where to choose I name, it would probably be fromTrinary.

Summarizing, I just tried to minimize the number of changes. But I don't think the name is good by any criteria.

@kotp kotp merged commit de10ad7 into exercism:master Mar 7, 2017
@kotp
Copy link
Member

kotp commented Mar 7, 2017

As an exercise that will soon be deprecated, this is definitely good enough.

@rbasso rbasso deleted the trinary-schema branch March 7, 2017 11:32
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.

2 participants