I takt med at wallnot.dk har fået flere og flere funktionaliteter, og dermed også flere databasefelter, er koden bag blevet rodet.
Her er et eksempel på, hvordan min kode blev svær for mig selv at forstå, og hvad jeg gjorde for at gøre den lidt bedre.
For at hente artikler til Wallnot, besøger en robot avisers hjemmesider og finder gratis-artikler. Logikken er lidt forskellig fra medie til medie, for det er forskelligt, hvordan medierne afslører, om en artikel er bag en betalingsmur eller ej.
Det er også meget forskelligt, om aviserne benytter sig af “gratis artikler, der kræver login” (Politiken, Ingeniøren, Jyllands-Posten), og det er forskelligt, hvad risikoen er for dubletter (Ritzau-artikler går igen mange steder, men ikke alle. Jyllands-Posten og Finans kopierer artikler til og fra hinanden.)
Her er for eksempel den gamle logik for Danmarks Radio:
def dr():
# Define medium
medium = "dr"
# Request site
data = requests.get("https://www.dr.dk/nyheder/service/feeds/allenyheder/")
result = data.text
# Soup site and create a list of links and their titles
soup = BeautifulSoup(result, "xml")
# List of unique urls
urllist = {link.text for link in soup.find_all('link') if "/nyheder/" in link.text and not any(term in link.text for term in excluded) and not any(term == link.text for term in mustnotbe)}
# Loop that requests all article links, soups them and checks whether they have a paywall and generates a list with current free articles. Also gets titles of links.
for url in urllist:
if not 'http' in url:
url = 'https://dr.dk' + url
if url not in lastbatch and url not in newbatch:
try:
data = requests.get(url)
result = data.text
soup = BeautifulSoup(result, "lxml")
node_id = soup.find('meta', attrs={'name':'ensighten:urn'})
if node_id:
id = node_id['content'][node_id['content'].find("article:")+8:]
urlid = "dr_" + id
if urlid not in newbatch and urlid not in lastbatch:
title = soup.find('meta', attrs={'property':'og:title'})
title = title['content']
title = title.strip(" ")
#api_url = "https://www.dr.dk/tjenester/urd/tms/urn:dr:drupal:article:" + id
#api_request = requests.get(api_url)
timestamp = soup.find('meta', attrs={'property':'article:published_time'})['content']
dateofarticle = datetime.strptime(timestamp, '%Y-%m-%dT%H:%M:%S%z')
if '<p class="dre-article-body__paragraph dre-variables">/ritzau/</p>' in result:
ritzau = True
body = soup.find('div', attrs={'class': 'dre-article-body'})
firstparagraph = body.find('p').get_text()
firstparagraph = firstparagraph.strip(" ")
else:
ritzau = False
if ritzau == True:
article = (title, urlid, dateofarticle, medium, url, datetime.now(), ritzau, firstparagraph)
else:
article = (title, urlid, dateofarticle, medium, url, datetime.now(), ritzau)
insert_unique_id_article(connection, article)
newbatch.append(url)
newbatch.append(urlid)
except Exception as e:
print(url)
print(e)
else:
newbatch.append(url)
Hvert medie har altså sin egen funktion til at hente artikler, og som du måske kan læse af koden, kalder hver funktionen en anden funktion, som indsætter artikler i databasen:
insert_unique_id_article(connection, article)
newbatch.append(url)
Problemet
Funktionen insert_unique_id_article var mildest talt blevet rodet.
Først blev indsat nogle ekstra værdier til article for at understøtte, at jeg for et tidspunkt valgte at artikler, der senere får en paywall, ikke slettes fra databasen, men blot markeres som med paywall.
Så havde den en overordnet logik, der skilte artikler, der skal tjekkes for dubletter fra artikler, der ikke skal.
Og under denne logik nogle forskellige variationer for, hvor mange ekstra variable og hvilke værdier, disse variable har, der tilføjes til hver article. Og mange forskellige sql-sætninger i forgreningen til rent faktisk at tilføje hver artikel til databasen.
Her er den uoverskuelige kode:
def insert_unique_id_article(connection, article):
with connection:
with connection.cursor() as cur:
try:
article += (False, "") # Not currently behind a paywall, no current archive url,
if not article[6] and not article[3] == "jyllandsposten" and not article[3] == "finansdk": # Ritzau is false and not jyllandsposten and not finansdk
if len(article) == 10: # article has loginwall variable inserted already
article += (False, ) # Not a duplicate
sql = ''' INSERT INTO wall_article(title,unique_id,date,medium,url,created_at,ritzau,loginwall,paywall_detected,archive_url,duplicate)
VALUES(%s,%s,%s,%s,%s,%s,%s,%s,%s,%s,%s) '''
cur.execute(sql, article)
else:
article += (False, False) # Not behind loginwall, not a duplicate
sql = ''' INSERT INTO wall_article(title,unique_id,date,medium,url,created_at,ritzau,paywall_detected,archive_url,loginwall,duplicate)
VALUES(%s,%s,%s,%s,%s,%s,%s,%s,%s,%s,%s) '''
cur.execute(sql, article)
else: # Ritzau is true or jyllandsposten or finans
sql = ''' SELECT * from wall_article WHERE title ILIKE %s OR excerpt ILIKE %s'''
article_title = "%"+article[0]+"%"
article_excerpt = "%"+article[7]+"%"
cur.execute(sql, (article_title, article_excerpt))
results = cur.fetchall()
if results:
duplicate = False
for result in results:
timestamp = result[3].astimezone(pytz.utc)
# Time in minutes between suspected duplicates is calculated
if article[2] < timestamp:
difference_in_minutes = (timestamp-article[2]).seconds/60
elif article[2] > timestamp:
difference_in_minutes = (article[2]-timestamp).seconds/60
elif article[2] == timestamp:
difference_in_minutes = 0
# Less than or 5 hours between, mark duplicate and insert as duplicate
if difference_in_minutes <= 300:
duplicate = True # important to avoid duplicate insert
article += (True, False) # a duplicate, no loginwall
sql = ''' INSERT INTO wall_article(title,unique_id,date,medium,url,created_at,ritzau,excerpt,paywall_detected,archive_url,duplicate,loginwall)
VALUES(%s,%s,%s,%s,%s,%s,%s,%s,%s,%s,%s,%s) '''
cur.execute(sql, article)
break
# ONLY FOR NON-duplicates
if not duplicate:
article += (False, False) # Not a duplicate, no loginwall
sql = ''' INSERT INTO wall_article(title,unique_id,date,medium,url,created_at,ritzau,excerpt,paywall_detected,archive_url,duplicate,loginwall)
VALUES(%s,%s,%s,%s,%s,%s,%s,%s,%s,%s,%s,%s) '''
cur.execute(sql, article)
else:
article += (False, False) # Not a duplicate, no loginwall
sql = ''' INSERT INTO wall_article(title,unique_id,date,medium,url,created_at,ritzau,excerpt,paywall_detected,archive_url,duplicate,loginwall)
VALUES(%s,%s,%s,%s,%s,%s,%s,%s,%s,%s,%s,%s) '''
cur.execute(sql, article)
except Error as e:
# If unique_id is already in database and the article is a politiken share link,
# the original URL is updated with the share url
if e.pgcode == "23505" and "politiken.dk/del" in article[4]:
connection.rollback()
sql = ''' Update wall_article set url = %s, loginwall = %s where unique_id = %s '''
cur.execute(sql, (article[4], True, article[1]))
elif not e.pgcode == "23505":
print(e, article)
Løsning
Jeg løste – eller måske formindskede – problemet ved at ensarte, hvor mange variable hvert medies funktion sender til funktionen insert_unique_id_article, sådan at medierne uanset om de fx bruger “login-mure” eller ej, sender en article af samme længde med samme variabelrækkefølge af sted til insert_unique_id_article.
Så nu går disse kodelinjer igen og er identiske, uanset hvilket medie, der er tale om:
article = (title, urlid, dateofarticle, medium, url, datetime.now(), ritzau, excerpt, loginwall)
insert_unique_id_article(connection, article)
Det har gjort insert_unique_id_article en del kortere og mere læsbar. Der er nu kun én mulig sql-sætning, færre mulige forgreninger og det er ensartet, hvor mange variable der tilføjes til article i hver forgrening:
def insert_unique_id_article(connection, article):
with connection:
with connection.cursor() as cur:
try:
sql = ''' INSERT INTO wall_article(title,unique_id,date,medium,url,created_at,ritzau,excerpt,loginwall,paywall_detected,archive_url,duplicate)
VALUES(%s,%s,%s,%s,%s,%s,%s,%s,%s,%s,%s,%s) '''
article += (False, "") # Add defaults: Not currently behind a paywall, no current archive url,
if not article[6] and not article[3] == "jyllandsposten" and not article[3] == "finansdk": # Ritzau is false and not jyllandsposten and not finansdk so no duplicate risk based on current newspaper practices
article += (False, ) # Not a duplicate
cur.execute(sql, article)
else: # Ritzau is true or jyllandsposten or finans, so run duplicate check
search_sql = ''' SELECT * from wall_article WHERE title ILIKE %s OR excerpt ILIKE %s'''
article_title = "%"+article[0]+"%"
article_excerpt = "%"+article[7]+"%"
cur.execute(search_sql, (article_title, article_excerpt))
results = cur.fetchall()
if results:
duplicate = False
for result in results:
timestamp = result[3].astimezone(pytz.utc)
# Time in minutes between suspected duplicates is calculated
if article[2] < timestamp:
difference_in_minutes = (timestamp-article[2]).seconds/60
elif article[2] > timestamp:
difference_in_minutes = (article[2]-timestamp).seconds/60
elif article[2] == timestamp:
difference_in_minutes = 0
# Less than or 5 hours between, mark duplicate and insert as duplicate
if difference_in_minutes <= 300:
duplicate = True # important to avoid duplicate insert
break
article += (duplicate, ) # Add duplicate status
cur.execute(sql, article)
else:
article += (False, ) # Not a duplicate
cur.execute(sql, article)
except Error as e:
# If unique_id is already in database and the article is a politiken share link,
# the original URL is updated with the share url
if e.pgcode == "23505" and "politiken.dk/del" in article[4]:
connection.rollback()
sql = ''' Update wall_article set url = %s, loginwall = %s where unique_id = %s '''
cur.execute(sql, (article[4], True, article[1]))
elif not e.pgcode == "23505":
print(e, article)
En bedre løsning?
Min problematik handler rigtigt meget om vægtningen mellem graden af identisk kode, der går igen flere steder, og graden af abstraktion.
Det er rart og logisk for mig, at kunne arbejde med og rette fejl i hvert enkelt medie hver for sig. Men der er meget kode, der går igen for hvert medie, og også forskellig kode, der gør det samme, afhængig af hvornår jeg lige har haft fat i koden sidst.
Det er besværligt, at jeg, hvis jeg på et tidspunkt indfører en ny funktionalitet, der kræver et nyt databasefelt, hvis værdi kan være forskellig fra medie til medie, er nødt til at opdatere hvert enkelt medies funktion, for at sikre mig at længden på den artikel, der sendes til insert_unique_id_article er den samme for alle medier.
En løsning kunne være et lidt højere abstraktionsniveau, hvor jeg:
- Laver article om til en ordbog (dictionary) i hvert medies funktion.
- Sørger for at hvert medie sender article til en hjælpefunktion, der gør artiklen klar til at indsætte i databasen, ved at gennemgå ordbogens nøgler og tilføje de nøgler, der evt. mangler, for at artiklen har de nødvendige variable til at kunne indsættes. Funktionen kunne hedder prepare_article.
- Sætter prepare_article til at sende artiklen til en endnu mere forenklet insert_unique_id_article.
En anden mulig løsning kunne være at kigge nærmere på modulet psycopg2, som er det modul, der lader mit Python-program tale med min database. Lige nu bruger jeg datatypen tuple når jeg indsætter data i databasen, og hvis der mangler værdier i min tuple i forhold til min datamodel for artikler, fungerer mit program ikke. Måske kan psycopg2 forstå dictionaries i stedet og fodres med standardværdier, der kan indsættes, hvis en artikel mangler felter fra datamodellen?
Det må jeg finde ud af, når jeg har tid, eller et problem der kræver, at jeg finder en bedre løsning.