munge_filename isn't idempotent for filenames containing spaces #2041
Conversation
|
87fd74c just adds a failing test that demonstrates the problem. |
|
Is this a problem? The input is English, the output is munged, so why does it need to be idempotent? |
|
+1 for idempotent transformations. We should be able to call this multiple times and get the same result as a single call. |
|
A reason to fix bad code? I was calling it twice (once from uploader, once from an extension) and I wasted the morning trying to debug what amounted to a missing hyphen in an image filename. Whether I should have been calling it twice is another argument. |
|
idempotence is a great thing for validators because it's often not possible to tell if it will be called more than once. For example if the name transformation is done on a create or update validation step, but then passed back out to the user in a form, when they submit the form again the transformation will be called on the already-transformed data. I think it's better to use idempotent transformations than, say, implement some kind of tracking to prevent a transformation from being called more than once or have code that fails mysteriously. |
|
@wardi Yes good justification. That makes sense for munge_filename, munge_name and munge_tag. I guess we've just found an edge case for munge_filename that needs fixing up. However munge_title_to_name is a transformation that is not needed to be idempotent because it is from one field to another, and that's what I was thinking about when I flinched on this. @brew I'm afraid I take issue at you calling the code 'bad'. Better to be constructive. |
davidread
changed the title
brew
changed the title
| munge = munge_title_to_name(org) | ||
| nose_tools.assert_equal(munge, exp) | ||
|
|
||
| def test_munge_title_to_name_multiple_pass(self): |
There was a problem hiding this comment.
'Title to name' is the one identified as not designed to be idempotent. Could you remove the multiple pass test?
Applying
lib.munge.munge_filenamemultiple times to its own output doesn't always produce the same result for filenames that originally have spaces.In the first pass spaces are replaced with hyphens. In subsequent passes, hyphens are removed.