patternpythonMinor
Search every char from a string for every char in another
Viewed 0 times
searchchareveryforanotherfromstring
Problem
I am adding a feature to a program that will let the user add new dictionaries as a file to the file folder to be used as a note taking repository. I just got the code finished for creating the file name from user input, however it feels as if there should be a smarter way to perform the task than what I have done.
The following is what I have come up with that does work but I don't know if I should be doing this in a shorter method with either a built in function or a smarter one-liner.
Can you explain any ways that you think this could be improved? I have searched for other solutions but they were not well explained enough that I could understand them very clearly.
EDIT: Added my imports to code. Note: the print statement at the bottom was just to test the result of the code and see it in console. It will be changed to return when I finish up the rest of the program.
EDIT: 04/21/2017 2:30 PM
Thanks for all the great feedback everyone and thanks to
```
va
The following is what I have come up with that does work but I don't know if I should be doing this in a shorter method with either a built in function or a smarter one-liner.
Can you explain any ways that you think this could be improved? I have searched for other solutions but they were not well explained enough that I could understand them very clearly.
EDIT: Added my imports to code. Note: the print statement at the bottom was just to test the result of the code and see it in console. It will be changed to return when I finish up the rest of the program.
from tkinter import *
import sys
import time
import tkinter.messagebox
import tkinter.simpledialog
import json
finalFilename =''
def new_lib_prompt(event=None):
finalFilename =''
name_of_new_library = tkinter.simpledialog.askstring('Create New Note Library', 'Alphanumeric lowercase and "_" only', initialvalue = "Name_Here")
from string import ascii_letters,digits
validFilenameChars = "-_.() %s%s" %(ascii_letters, digits)
stringofnewname = str(name_of_new_library)
stringofvalid = str(validFilenameChars)
for i in stringofnewname:
if i in stringofvalid:
if i == ' ':
new_i = '_'
finalFilename += ''.join(new_i)
else:
lower_i = i.lower()
finalFilename += ''.join(lower_i)
else:
pass
print (finalFilename)EDIT: 04/21/2017 2:30 PM
Thanks for all the great feedback everyone and thanks to
holroy for his very useful and thought out response. If anyone is interested here is the solution I was looking for after taking advice from a few answers.```
va
Solution
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.
Algorithm comments
When summarized your code does the following things:
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
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
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:
And this possibly requires a little explanation:
In other words, the previous statements joins all characters from
A longer version to write the same is something like (with a lot of edge cases removed):
I'm also accustomed to separating how you get stuff (aka the
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):
Note that one could join the
- 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
forloops orifblock 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,validFilenameCharsandstringofnewname. 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 usedtkIntera lot, so you might need it for some reason, but it kind of looks superfluous to me as you don't use theeventin the function at all.
- Why the
str(some_variable)? – Why do you do this? Is this related totkIntersomehow as well? Do you really need to "stringify" the return fromtkInter?
- 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 ofjoin.
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 theforloops is valid, then return thec
for c in suggested_name ...– Split thesuggested_nameinto single characters
if c in VALID_CHARS– Only do the action (that is returnc) from theforloop if theifexpression is true. And the expression verifies that the charactercis within the stringVALID_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 += cI'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 traCode Snippets
''.join(c for c in suggested_name if c in VALID_CHARS)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 += cfrom 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"Context
StackExchange Code Review Q#161421, answer score: 6
Revisions (0)
No revisions yet.