First of all let's address some of the more prominent style issues of your code, before diving into an alternate approach to solving it.
- Have imports at top of code – At first I didn't see the
from string import ...
line as it was hidden in the code, but this is not good coding practice. Put this at the top of the file, so it is easy to see.
- Have more vertical space – Please add an extra line here and there, to increase readability in your code. Typically I would add spaces in front of
for
loops or if
block to accentuate these blocks.
- Be consistent in variable naming – Naming variables is hard but choose one style for variable naming, and don't mix it like
name_of_new_library
, validFilenameChars
and stringofnewname
. This make the code a whole lot harder to read, which in turns makes it harder to maintain in the long run.
- Functions should return, not print their result – To have a function print its output is usually a code smell, and it would be better to have it return the result. If by some reasoning it's required to print it out, I would maybe indicate that in the function name.
- Using the
%
operator is depreceated in favor of str.format – See format examples for the newer and better method of adding arguments into strings.
- Why the
event=None
– I haven't used tkInter
a lot, so you might need it for some reason, but it kind of looks superfluous to me as you don't use the event
in the function at all.
- Why the
str(some_variable)
? – Why do you do this? Is this related to tkInter
somehow as well? Do you really need to "stringify" the return from tkInter
?
- Only join when needed! – The join command is mainly used to join lists, so joining a list of one character is extra work, and not needed. To append a single character you could do
txt += char
. See below for a better use of join
.
Algorithm comments
When summarized your code does the following things:
- Reads a suggestion using
tkInter
- Builds a constant of
validFilenameChars
- Converts any spaces into underscores, and
- lowercases all other characters
As covered before the import should be done at top, and so should also declaration of constants. When you start validifying the name you split the string into single characters, and use a somewhat strange += ''.join(new_i)
to add onto the new filename.
There does however exist functions which does both of your required operations. See str.lower() and str.replace(). Do however note that, the latter replace substrings. In your case str.replace(' ', '_')
would work, but if replacing multiple chars see how to replace multiple characters in a string?
The only part not covered then is the valid characters part, and this is the slightly ugly part of the new alternative. My proposed solution is to do:
''.join(c for c in suggested_name if c in VALID_CHARS)
And this possibly requires a little explanation:
''.join(_some list_)
– This expects a list of something, which it will join together by the separator in front, that is the empty string.
c ...
– If the for
loops is valid, then return the c
for c in suggested_name ...
– Split the suggested_name
into single characters
if c in VALID_CHARS
– Only do the action (that is return c
) from the for
loop if the if
expression is true. And the expression verifies that the character c
is within the string VALID_CHARS
.
In other words, the previous statements joins all characters from suggested_filename
using the empty string separator, for those characters who are present in VALID_CHARS
. That is quite a mouthful, but it is a very useful expression!
A longer version to write the same is something like (with a lot of edge cases removed):
my_list = []
for c in suggested_name:
if c in VALID_CHARS:
my_list.append(c)
my_separator = ''
my_txt = my_list[0]
for c in my_list[1:]:
my_txt += my_separator
my_txt += c
I'm also accustomed to separating how you get stuff (aka the tkInter
dialog) from validation/transformation of the result, so I would make a function to handle this.
Code refactored
So what would the code look like if applying all of this? Maybe something like this (untested as I don't have tkinter available):
from string import ascii_letters, digits
VALID_CHARS = '-_.() {}{}'.format(ascii_letters, digits)
def get_valid_library_name(suggestion):
valid_filename = ''.join(c for c in suggestion if c in VALID_CHARS)
valid_filename = valid_filename.replace(' ', '_').lower()
return valid_filename
a_name = tkinter.simpledialog.askstring(
'Create New Note Library',
'Alphanumeric lowercase and "_" only',
initialvalue = "Name_Here")
print('{} into {}'.format(a_name, get_valid_library_name(a_name))
filename = get_valid_library_name("MY someWHAT ugly suggestion#$!&&")
print(filename) # Would output "my_somewhat_ugly_suggestion"
Note that one could join the valid_filename
transformation into fewer lines, but keeping it readable is also a good thing. For real code I would most likely have the valid_filename = ''.join(..)
on one line, and then do a return valid_filename.replace().lower()
instead of the two next lines.
As a final note, also see Turn a string into a valid filename which I found when researching for this answer. It uses some alternate stuff from unicode, and has some other related ideas on how to handle validation of filenames.