Kleines Web-Such-Skript

Wenn du dir nicht sicher bist, in welchem der anderen Foren du die Frage stellen sollst, dann bist du hier im Forum für allgemeine Fragen sicher richtig.
Antworten
ne0h
User
Beiträge: 115
Registriert: Samstag 16. Februar 2008, 11:35

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
rayo
User
Beiträge: 773
Registriert: Mittwoch 5. November 2003, 18:06
Wohnort: Schweiz
Kontaktdaten:

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
Leonidas
Python-Forum Veteran
Beiträge: 16025
Registriert: Freitag 20. Juni 2003, 16:30
Kontaktdaten:

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.
My god, it's full of CARs! | Leonidasvoice vs (former) Modvoice
dor_neue
User
Beiträge: 74
Registriert: Montag 16. Juni 2008, 18:51

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...
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
dor_neue
User
Beiträge: 74
Registriert: Montag 16. Juni 2008, 18:51

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 :P
Leonidas
Python-Forum Veteran
Beiträge: 16025
Registriert: Freitag 20. Juni 2003, 16:30
Kontaktdaten:

Nett zu lesen ist auch eine Diskussion zum Thema richtige Variablennamen vs. Kommentare: AA, BB, CC, and DD.
My god, it's full of CARs! | Leonidasvoice vs (former) Modvoice
ne0h
User
Beiträge: 115
Registriert: Samstag 16. Februar 2008, 11:35

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
sma
User
Beiträge: 3018
Registriert: Montag 19. November 2007, 19:57
Wohnort: Kiel

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
Benutzeravatar
Hyperion
Moderator
Beiträge: 7478
Registriert: Freitag 4. August 2006, 14:56
Wohnort: Hamburg
Kontaktdaten:

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 ;-)
ne0h
User
Beiträge: 115
Registriert: Samstag 16. Februar 2008, 11:35

@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
Y0Gi
User
Beiträge: 1454
Registriert: Freitag 22. September 2006, 23:05
Wohnort: ja

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.
Antworten