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

error-handling: implement exercise #798

Merged
merged 19 commits into from
Oct 24, 2017
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
f9765ef
(WIP) error-handling: implement exercise
cmccandless Oct 13, 2017
04f385b
error-handling: update config.json
cmccandless Oct 13, 2017
b1c2967
error-handling: add README
cmccandless Oct 13, 2017
135cdb7
error-handling: add solution template
cmccandless Oct 13, 2017
e8e0ae1
error-handling: write test cases
cmccandless Oct 13, 2017
2a75c79
write example solution
cmccandless Oct 13, 2017
2c9575a
error-handling: fixes for flake8 compliance
cmccandless Oct 13, 2017
6543fcd
error-handling: change topics to snake_case
cmccandless Oct 21, 2017
6adccde
error-handling: remove incorrect Source section
cmccandless Oct 21, 2017
68b8f22
error-handling: improve use of context manager
cmccandless Oct 21, 2017
fbaefd6
Merge branch 'master' into implement-error-handling
cmccandless Oct 21, 2017
24c7f79
error-handling: further improve context manager implementation
cmccandless Oct 23, 2017
995a836
Merge branch 'master' into implement-error-handling
cmccandless Oct 23, 2017
bc7eb88
error-handling: remove redundant error handling inside "with"
cmccandless Oct 23, 2017
6414219
Merge branch 'master' into implement-error-handling
cmccandless Oct 23, 2017
d01e63e
error-handling: replace assertTrue and assertFalse with assertIs
cmccandless Oct 24, 2017
1cb2a6d
Merge branch 'implement-error-handling' of github-personal:cmccandles…
cmccandless Oct 24, 2017
55b13a2
error-handling: conform to parameter order convention
cmccandless Oct 24, 2017
b147fcd
Merge branch 'master' into implement-error-handling
Oct 24, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions config.json
Original file line number Diff line number Diff line change
Expand Up @@ -854,6 +854,16 @@
"conditionals"
]
},
{
"uuid": "3a2a947a-01b3-1e80-e32b-de1756fd88365adf12e",
"slug": "error-handling",
"core": false,
"unlocked_by": null,
"difficulty": 3,
"topics": [
"exception handling"
Copy link
Contributor

Choose a reason for hiding this comment

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

We've switched to snake_case naming convention for topics to increase consistency among tracks, so could you please choose them from the following list - https://github.com/exercism/problem-specifications/blob/master/TOPICS.txt

]
},
{
"uuid": "e7351e8e-d3ff-4621-b818-cd55cf05bffd",
"slug": "accumulate",
Expand Down
27 changes: 27 additions & 0 deletions exercises/error-handling/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Error Handling

Implement various kinds of error handling and resource management.

An important point of programming is how to handle errors and close
resources even if errors occur.

This exercise requires you to handle various errors. Because error handling
is rather programming language specific you'll have to refer to the tests
for your track to see what's exactly required.

### Submitting Exercises

Note that, when trying to submit an exercise, make sure the solution is in the `exercism/python/<exerciseName>` directory.

For example, if you're submitting `bob.py` for the Bob exercise, the submit command would be something like `exercism submit <path_to_exercism_dir>/python/bob/bob.py`.


For more detailed information about running tests, code style and linting,
please see the [help page](http://exercism.io/languages/python).

## Source

Problem 6 at Project Euler [http://projecteuler.net/problem=6](http://projecteuler.net/problem=6)
Copy link
Contributor

Choose a reason for hiding this comment

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

This source isn't right - I presume that there's actually no source needed for this exercise?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct; I shall remove it. The included source section was in the README from another exercise from which I created this one.

Copy link
Contributor

Choose a reason for hiding this comment

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

@cmccandless if it was based on canonical-data, it will be good to open PR there to fix it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was not. I copied the exercise-specific sections from the C# README and the track-specific sections (submitting, incomplete, etc) from another Python exercise. The source section was accidentally copied with the info from the other Python exercise.


## Submitting Incomplete Solutions
It's possible to submit an incomplete solution so you can see how others have completed the exercise.
14 changes: 14 additions & 0 deletions exercises/error-handling/error_handling.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
def handle_error_by_throwing_exception():
pass


def handle_error_by_returning_none(input_data):
pass


def handle_error_by_returning_tuple(input_data):
pass


def filelike_objects_are_closed_on_exception(filelike_object):
pass
52 changes: 52 additions & 0 deletions exercises/error-handling/error_handling_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import unittest

import error_handling as er

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please also leave a comment stating what version of canonical-data.json the tests were adopted as discussed in #784 if aplicable?
The format is:

# Tests adapted from `problem-specifications//canonical-data.json` @ v1.0.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At this time, there is no canonical data for this exercise; these tests were adapted from the C# implemetation of this exercise. Is there a standard way of noting this?

Copy link
Contributor

Choose a reason for hiding this comment

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

If there is no tests in problem-specifications repo for this exercise, then it's Ok to leave it as is.

We agreed with @N-Parsons to use comment above to denote the version of canonical-data.json from which tests were adopted


class FileLike(object):
def __init__(self):
self.is_open = False

def open(self):
self.is_open = True

def __enter__(self):
self.is_open = True

def close(self):
self.is_open = False

Copy link
Contributor

@N-Parsons N-Parsons Oct 22, 2017

Choose a reason for hiding this comment

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

If you add return self here (plus make the change to __exit__), then this class will work with a context manager with an assignment.

def __exit__(self):
Copy link
Contributor

@N-Parsons N-Parsons Oct 22, 2017

Choose a reason for hiding this comment

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

__exit__ needs to accept 4 arguments, but we don't need to use them for anything here, so we can just use def __exit__(self, *args):

self.is_open = False


class ErrorHandlingTest(unittest.TestCase):
def test_throw_exception(self):
with self.assertRaises(Exception):
er.handle_error_by_throwing_exception()

def test_return_none(self):
self.assertEqual(1, er.handle_error_by_returning_none('1'),
'Result of valid input should not be None')
self.assertIsNone(er.handle_error_by_returning_none('a'),
'Result of invalid input should be None')

def test_return_tuple(self):
successful_result, result = er.handle_error_by_returning_tuple('1')
self.assertTrue(successful_result, 'Valid input should be successful')
self.assertEqual(1, result, 'Result of valid input should not be None')

failure_result, result = er.handle_error_by_returning_tuple('a')
self.assertFalse(failure_result,
'Invalid input should not be successful')

def test_filelike_objects_are_closed_on_exception(self):
filelike_object = FileLike()
filelike_object.open()
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think that we should be passing an open object to the user's function. It seems strange to me that we'd pass something open to a function that (if we weren't just throwing an exception) may or may not close it - I would expect closing to be handled in the place that it's opened. I think it should be up to the user to open and close the filelike object however they see fit.

with self.assertRaises(Exception):
er.filelike_objects_are_closed_on_exception(filelike_object)
self.assertFalse(filelike_object.is_open)


if __name__ == '__main__':
unittest.main()
22 changes: 22 additions & 0 deletions exercises/error-handling/example.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
def handle_error_by_throwing_exception():
raise Exception()


def handle_error_by_returning_none(input_data):
try:
return int(input_data)
except ValueError:
return None


def handle_error_by_returning_tuple(input_data):
try:
return (True, int(input_data))
except ValueError:
return (False, None)


def filelike_objects_are_closed_on_exception(filelike_object):
with filelike_object:
filelike_object.close()
Copy link
Contributor

@N-Parsons N-Parsons Oct 21, 2017

Choose a reason for hiding this comment

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

This isn't closing the filelike object on an exception, it's closing the filelike object and then raising an exception afterwards.

A better solution might be to have a was_open property and do_something method in the FileLike class, with the latter just raising some form of exception. Then it should be up to the user to call open, do_something, and close in that order. The tests can check that an exception was raised, that is_open is False, and that was_open is True.

It'd probably then be worth using the exercise's .meta/hints.md file to include something in the README about what methods exist in the filelike_object. The .meta/hints.md file is then included into the README when it's generated using the configlet - it's fine to just use copy-and-paste to put together the README, but also putting it in the hints file makes sure that it won't accidentally get lost in future.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, as a learner I would expect that a context manager could be used here (eg. with open(filelike_object) as f:) - it's the Pythonic way to handle files. I'm not sure what would need to be implemented in the FileLike class to make this work though. If we can't get a context manager working, then I think that the README needs to tell learners that context managers would be the typical way to do it with files, but that they won't work with the custom class here, because otherwise there is a massive amount of room for confusion and frustration.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The built-in open() method only takes a file name; you cannot implement a class to support open(filelike_object). However, I like the idea of do_something() raising an exception. See my latest changes.

raise Exception()