Seite 1 von 1
Kleines Web-Such-Skript
Verfasst: Freitag 25. Juli 2008, 12:53
von ne0h
Hallo Pythonier,
ich habe gestern aus etwas Langeweile mal das Archiv zum Python Kochbuch durchsucht (welches sich hier findet:
http://aspn.activestate.com/ASPN/Python/Cookbook/) und dabei ist mir ein kleines, interessantes Script in die Hände gefallen ->
http://code.activestate.com/recipes/573447/
Das Skript startet den Webbrowser mit der spezifischen URL/URI, welche sich aus dem Suchstring zusammensetzt. DIe Suchbegriffe werden einfach in der Kommandozeile eingegeben und abgeschickt. Das Original-Skript ist recht kurz und knapp und anhand dieser Idee habe ich das Ganze einfach mal aus Spass erweitert.
Also, hier der Code dazu und meine Bitte bzw. Frage:
Was daran ist gut, was schlecht, was falsch, wie würdet Ihr sowas angehen? Ich plane, das noch weiter auszubauen und etwas besser zu verpacken, sprich, ich will daraus ein etwas umfangreicheres Prog bauen mit kompletter OO usw. Mir gehts erstmal nur um den Ansatz und die Ausführung, z.B. meine zwei "while"-Schleifen unten, mit denen ich das Ganze durchlaufe.
Code: Alles auswählen
#!c:\Python25\python\python.exe
# -*- coding: utf-8 -*-
# @Author: ne0h
import webbrowser
import sys
print "\nWebsearch from command line. Type '!quit!' to exit the program.\n"
print "Which searchengine do you want to use?\n"
print "[1] Google German"
print "[2] Google English"
print "[3] Yahoo Search"
print "[4] Microsoft Live Search\n"
# variables
sEngine = ""
chosen_sEngine = ""
def chooseSE(sEngine):
# URLs to the searchengines
url_googleDE = "http://www.google.de/search?hl=de&q="
url_googleEN = "http://www.google.com/search?hl=en&q="
url_yahooSearch = "http://de.search.yahoo.com/search?p="
url_msLiveSearch = "http://search.live.com/results.aspx?q="
if eval(sEngine) == 1:
print "Your searchengine is: Google German.\n"
return url_googleDE
elif eval(sEngine) == 2:
print "Your searchengine is: Google English.\n"
return url_googleEN
elif eval(sEngine) == 3:
print "Your searchengine is: Yahoo Search.\n"
return url_yahooSearch
else:
print "Your searchengine is: Microsoft Live Search.\n"
return url_msLiveSearch
while True:
if sEngine == "!quit!":
sys.exit("\n...program closed.")
if sEngine.isdigit():
if eval(sEngine) >= 1 and eval(sEngine) <= 4:
break
else:
sEngine = raw_input("Your choice: ")
else:
sEngine = raw_input("Your choice: ")
chosen_sEngine = chooseSE(sEngine)
while True:
searchTerm = raw_input("\nSearch for: ")
if searchTerm == "!quit!":
sys.exit("\n...program closed.")
url_add1 = ''
searchTerm = '+'.join((searchTerm.split(' ')))
print "Your searchterm(s) were: \"%s\"" % (searchTerm, )
webbrowser.open(chosen_sEngine + searchTerm)
if __name__ == "__main__":
main()
Über Kritik und Anregungen würde ich mich freuen.
Gruss
ne0h
Verfasst: Freitag 25. Juli 2008, 13:34
von rayo
Hi
Mach alle eval() Aufrufe weg, das sind grosse Sicherheitslöcher. Arbeite mit den Datentypumwandlungsfunktionen.
Wenn du den eingegeben String als Integer/Zahl möchtest, dann verwende int(eingabe_string) und nicht eval.
Gruss
Verfasst: Freitag 25. Juli 2008, 14:14
von Leonidas
Ich würde alles in Funktionen packen statt modulglobalen Code, die URLs in ein Dict packen und statt der vielen ``prints`` ein einzelnes ``print`` mit einem Multi-Line-String nutzen.
Verfasst: Samstag 26. Juli 2008, 00:25
von dor_neue
kann ja sein, dass ich blind bin, aber Du hast Zeile 66 den Aufruf der Main-Funktion - aber keine Main-Funktion...
Außerdem gefallen mir die global Variablen in Zeile 18 und 19 nicht, weil:
1.) Du keine Variablen deklarieren musst in Python
2.) Weil die dort global deklariert sind
Außerdem würde ich die 1. Zeile löschen - eine leere Zeile am Anfang irritiert nur...
Und zu allem Überfluss, bitte mehr auskommentieren - ich weiß, is nen einfaches Programm, aber kommentiere mehr aus und Du hast in 6 Monaten weniger Probleme Dich wieder reinzuarbeiten...
Sollte man von Anfang an machen, sonst vergisst man es auch bei großen Problemen...
Verfasst: Samstag 26. Juli 2008, 08:51
von lunar
dor_neue hat geschrieben:Und zu allem Überfluss, bitte mehr auskommentieren - ich weiß, is nen einfaches Programm, aber kommentiere mehr aus und Du hast in 6 Monaten weniger Probleme Dich wieder reinzuarbeiten...
Naja, wenn er mehr
auskommentiert, dann läuft das Programm nicht mehr
SCNR
Verfasst: Sonntag 27. Juli 2008, 00:29
von dor_neue
lunar hat geschrieben:dor_neue hat geschrieben:Und zu allem Überfluss, bitte mehr auskommentieren - ich weiß, is nen einfaches Programm, aber kommentiere mehr aus und Du hast in 6 Monaten weniger Probleme Dich wieder reinzuarbeiten...
Naja, wenn er mehr
auskommentiert, dann läuft das Programm nicht mehr
SCNR
ich kenn es eben nur unter dem Begriff "auskommentieren"...
heißt also - bitte mehr Kommentare deinem Quellcode hinzufügen um Schlüsselstellen hervorzuheben und Variablen-Übergaben zu erklären...
besser so

Verfasst: Sonntag 27. Juli 2008, 00:34
von Leonidas
Nett zu lesen ist auch eine Diskussion zum Thema richtige Variablennamen vs. Kommentare:
AA, BB, CC, and DD.
Verfasst: Dienstag 29. Juli 2008, 11:10
von ne0h
Hi,
ich konnte mich meines Themas erst heute wieder annehmen. Ich habe mein Skript mal umgearbeitet, entsprechend Euren Vorschlägen. Dabei habe ich auch die meissten Variablennamen sprechender benannt und somit so gut es geht auf Kommentare verzichtet.
Auch wenn der Tip gut gemeint war, ich bin der Meinung, dass Kommentare erst dann sinnvoll sind, wenn sich entsprechende Bezeichnungen der Variablen und allgemein die Funktionalität nicht mehr eindeutig darstellen lässt.
Das mit der ersten Zeile war nur ein Versehen, ich hab da wohl zufällig beim Posten auf Enter gedrückt.
Und ich habe jetzt natürlich auch die main()-Funktion eingebaut, die ja zuvor gefehlt hat, obwohl ich Sie aufgerufen habe.
Code: Alles auswählen
#!c:\Python25\python.exe
# -*- coding: utf-8 -*-
# @Author: ne0h
import webbrowser
import sys
def chooseSearchengine(myChoice):
searchEngineURLs = {
"URLgoogleDE" : "http://www.google.de/search?hl=de&q=",
"URLgoogleEN" : "http://www.google.com/search?hl=en&q=",
"URLyahooSearch" : "http://de.search.yahoo.com/search?p=",
"URLmsLiveSearch" : "http://search.live.com/results.aspx?q="}
if int(myChoice) == 1:
print "You selected: Google German.\n"
return searchEngineURLs["URLgoogleDE"]
elif int(myChoice) == 2:
print "You selected: Google English.\n"
return searchEngineURLs["URLgoogleEN"]
elif int(myChoice) == 3:
print "You selected: Yahoo Search.\n"
return searchEngineURLs["URLyahooSearch"]
else:
print "You selected: Microsoft Live Search.\n"
return searchEngineURLs["URLmsLiveSearch"]
def main():
print """\nWebsearch from command line. Type '!quit!' to exit the program.
Which searchengine do you want to use?
[1] Google German
[2] Google English
[3] Yahoo Search
[4] Microsoft Live Search\n"""
myChoice = raw_input("Your choice: ")
while True:
if myChoice == "!quit!":
sys.exit("\n...program closed.")
if myChoice.isdigit():
if int(myChoice) >= 1 and int(myChoice) <= 4:
break
else:
myChoice = raw_input("Your choice: ")
else:
myChoice = raw_input("Your choice: ")
searchengine = chooseSearchengine(myChoice)
while True:
searchTerm = raw_input("\nSearch for: ")
if searchTerm == "!quit!":
sys.exit("\n...program closed.")
url_add1 = ''
searchTerm = '+'.join((searchTerm.split(' ')))
print "Your searchterm(s) were: \"%s\"" % (searchTerm, )
webbrowser.open(searchengine + searchTerm)
if __name__ == "__main__":
main()
So, noch einige weitere Ideen oder Tips zum jetzigen Stand?
Gruss
ne0h
Verfasst: Mittwoch 30. Juli 2008, 09:28
von sma
Da ist IMHO noch zu viel Redundanz. Wie wäre so was?
Code: Alles auswählen
class SE:
def __init__(self, name, url): self.name, self.url = name, url
def search(self):
print "You selected %s" % self.name
...
webbrowser.open(self.url + ...)
search_engines = [
SE("Google German", "http://www.google.de/search?hl=de&q="),
SE("Google English", "..."),
...
]
def main():
print "Which searchengine do you want to use?"
for i, se in enumerate(search_engines):
print " [%d] %s" % (i + 1, se.name)
choice = int(raw_input("You choice:")) - 1
if 0 <= choice < len(search_engines): search_engines[choice].search()
Stefan
Verfasst: Mittwoch 30. Juli 2008, 13:32
von Hyperion
sma hat geschrieben:Da ist IMHO noch zu viel Redundanz. Wie wäre so was?
Wobei man sich in Deiner Lösung über den Sinn der Klasse streiten könnte

Verfasst: Mittwoch 30. Juli 2008, 21:22
von ne0h
@sma
Hm, interessanter Ansatz. Danke, ich werde mir das mal genauer anschauen und versuchen, dahingehend zu optimieren und vielleicht auch direkt jetzt schon als OO-Ansatz zu schreiben.
Gruss
ne0h
Verfasst: Mittwoch 13. August 2008, 11:42
von Y0Gi
Schau dir diese Funktion mal in Ruhe an:
ne0h hat geschrieben:Code: Alles auswählen
def chooseSearchengine(myChoice):
searchEngineURLs = {
"URLgoogleDE" : "http://www.google.de/search?hl=de&q=",
"URLgoogleEN" : "http://www.google.com/search?hl=en&q=",
"URLyahooSearch" : "http://de.search.yahoo.com/search?p=",
"URLmsLiveSearch" : "http://search.live.com/results.aspx?q="}
if int(myChoice) == 1:
print "You selected: Google German.\n"
return searchEngineURLs["URLgoogleDE"]
elif int(myChoice) == 2:
print "You selected: Google English.\n"
return searchEngineURLs["URLgoogleEN"]
elif int(myChoice) == 3:
print "You selected: Yahoo Search.\n"
return searchEngineURLs["URLyahooSearch"]
else:
print "You selected: Microsoft Live Search.\n"
return searchEngineURLs["URLmsLiveSearch"]
Es gibt ein Prinzip namens DRY (Don't Repeat Yourself). Hier ist so einiges redundant:
- `int(myChoice)`
- `print "You selected: ..."`
- `return searchEngineURLs["..."]`
Ergo: Alles auslagern!
Dann sieht es schon deutlich einfacher aus (zudem habe ich PEP 8-konforme Namen benutzt):
Code: Alles auswählen
def choose_search_engine(my_choice):
search_engines = {
"googleDE": ("Google German", "http://www.google.de/search?hl=de&q="),
"googleEN": ("Google English", "http://www.google.com/search?hl=en&q="),
"yahooSearch": ("Yahoo Search", "http://de.search.yahoo.com/search?p="),
"msLiveSearch": ("Microsoft Live Search", "http://search.live.com/results.aspx?q="),
}
choice = int(my_choice)
if choice == 1:
key ="googleDE"
elif choice == 2:
key = "googleEN"
elif choice == 3:
key = "yahooSearch"
else:
key = "msLiveSearch"
name, url = search_engines[key]
print "You selected: %s.\n" % name
return url
Wie du siehst, habe ich zusammen gehörende Daten (Name und URL einer Suchmaschine) in der selben Struktur (jetzt: ein Dictionary mit 2-Tupeln als Werte) untergebracht. Auf den redundanten "URL"-Präfix der Schlüssel habe ich ebenfalls verzichtet.
Nun zeigt sich weiterhin, dass `choice` ziemlich direkt auf die Keys gemappt wird. Ich halte ein dict hier für sehr angebracht und entsprechend sollte man den Schlüssel als Parameter übergeben. Wenn du jedoch *wirklich* C-like Integer verwenden möchtest (wie hier durch dein Menü erforderlich), *könntest* du auch gleich ein Tupel statt eines dict verwenden:
Code: Alles auswählen
def choose_search_engine(my_choice):
search_engines = )
"msLiveSearch": ("Microsoft Live Search", "http://search.live.com/results.aspx?q="),
"googleDE": ("Google German", "http://www.google.de/search?hl=de&q="),
"googleEN": ("Google English", "http://www.google.com/search?hl=en&q="),
"yahooSearch": ("Yahoo Search", "http://de.search.yahoo.com/search?p="),
)
choice = int(my_choice)
if choice not in (1, 2, 3):
# dynamische Alternative:
#if choice not in xrange(1, len(search_engines)): # -> [1, 2, 3]
choice = 0
name, url = search_engines[choice]
print "You selected: %s.\n" % name
return url
Dabei habe ich die Default-Suchmaschine an den Anfang verschoben, so dass sie als Element 0 angesprochen wird und dadurch die bisherigen Nummern weiter verwendet werden können, ohne dass die choice-Zahl selbst noch verschoben werden muss.