-
Notifications
You must be signed in to change notification settings - Fork 1
Refactor storage.py #420
Description
File is a mess. Lots of duplicate code/similar logic. No structure in the file, all functions are at the same level, which makes it very hard to get a mental picture of the code. Function names are all very similar, so it's hard to find the responsibilities in the code. The tests don't mock much and use obscure monkeypatch mocks. Changing the code is a pain in the ***. At the moment it's just fixing the tests when you're done without knowing what you're actually fixing in the tests (or if you've broken something).
Refactor this file, adhere to some hierarchy with clear abstractions so that the program flow is clear. Use clear function names, write proper tests with the correct mocks. If I change the way a reference is created I don't want to have to fix 15 tests that all use that logic. Also, by not mocking function calls the tests for the functions themselves are often skipped. Without tests that specifically test those functions the intent of the functions is lost. For me, as a developer who didn't write the original code in the first place, it's hard to find out what exactly the code was supposed to do, because the return values are all mixed into other tests.