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

Quality of Life options for TomlTable and TomlValue #28

Merged
merged 2 commits into from
Jun 6, 2023

Conversation

johnkiddjr
Copy link
Contributor

Fixes #27

This adds the functionality of IEnumerator to TomlTable, allowing you to iterate on the table without needing to reference the Entries property.

foreach (var item in table.Entries)
{
    ...
}

Becomes

foreach (var item in table)
{
    ...
}

This also adds an override on the ToString method for TomlValue to return the StringValue property.

TomlString stringValue; //or any other TomlValue
Console.WriteLine(stringValue.StringValue);

Becomes

TomlString stringValue; //or any other TomlValue
Console.WriteLine(stringValue.ToString()); // outputs the value of StringValue now

@coveralls
Copy link

coveralls commented Jun 6, 2023

Coverage Status

coverage: 92.402% (-0.07%) from 92.47% when pulling d485db0 on johnkiddjr:master into acc83e1 on SamboyCoding:master.

@SamboyCoding
Copy link
Owner

I have no issues with the actual content of this pull request, so on that front, this is approved, but as per coveralls' comment above, the lack of unit tests for the enumeration and string values has resulted in coverage decreasing, which I'd rather avoid, if you wouldn't mind writing a couple tests to cover those cases -- though they really don't need it, it's easier to just enforce the rule of "test as high a percentage of the codebase as possible" rather than making case-by-case decisions.

@johnkiddjr
Copy link
Contributor Author

Agreed, added tests to ensure ToString and StringValue are equal (done for both TomlString and TomlLong) and tests to check that iterated values are the same as values pulled directly from the dictionary.

@SamboyCoding
Copy link
Owner

Appreciate it! Assuming this CI run passes fine, I'll merge this then.

@SamboyCoding
Copy link
Owner

Right well the explicit interface implementation wasn't covered but I can live with that, merging.

@SamboyCoding SamboyCoding merged commit fb2f2fd into SamboyCoding:master Jun 6, 2023
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.

Quality of Life Functions for TomlTable and TomlValue
3 participants