Feedback #2

New Issue

cyclane · 2024-01-04T18:05:42Z

cyclane commented

2024-01-04 18:05:42 +00:00

Your non-recursive solution is nice. The tests are not that comprehensive, but cover some success and failure cases.

Judging by the tests I see you have recognised a possibility of cycles in the input data, but such possibility doesn't seem to be addressed in the code of the library.
Would it be difficult for you to address it gracefully (e.g. by not accepting such input data and throwing an exception)? If you do please make it a separate commit in the repo, so I can clearly see what has been changed.

So:

Disallow recursive inputs, instead of relying on the filesystem.
- I'm thinking a basic hashset is fine. We will need to do this as a separate loop anyway so that files aren't created in this case.
Improve tests.

> Your non-recursive solution is nice. The tests are not that comprehensive, but cover some success and failure cases. > > Judging by the tests I see you have recognised a possibility of cycles in the input data, but such possibility doesn't seem to be addressed in the code of the library. > Would it be difficult for you to address it gracefully (e.g. by not accepting such input data and throwing an exception)? If you do please make it a separate commit in the repo, so I can clearly see what has been changed. So: - [x] Disallow recursive inputs, instead of relying on the filesystem. - I'm thinking a basic hashset is fine. We will need to do this as a separate loop anyway so that files aren't created in this case. - [x] Improve tests.

cyclane added the due date 2024-01-11

2024-01-05 23:07:49 +00:00

cyclane referenced this issue

2024-01-07 14:48:21 +00:00

Deny cyclic FSFolders #3

cyclane added a new dependency 2024-01-07 14:48:52 +00:00

#3 Deny cyclic FSFolders

cyclane referenced this issue from a commit

2024-01-07 14:56:18 +00:00

Deny cyclic FSFolders (#3)

cyclane referenced this issue

2024-01-07 15:05:33 +00:00