3
\$\begingroup\$

I am trying to figure out whether the below is better than just having a raw script or just a script with functions.

class WebsiteImages(object):
    
    def __init__(self,photographer,Website_url):
        self.photographer = photographer
        self.Website_url = Website_url
        
    def GetNumberOfResultPages(self):
        #Get the exact number of pages in the results
        from selenium import webdriver
        browser = webdriver.Firefox()
        browser.get(self.Website_url) 
        last_number_page = browser.find_elements_by_xpath("//span[@class='search-pagination__last-page']") 
        for i in last_number_page:
            number_of_pages = i.text
        return number_of_pages

    def GetImageIds(self):
        number_of_pages = self.GetNumberOfResultPages()
        Website_ids = []
        self.number_of_pages = number_of_pages
        #For each page get the image IDs
        import urllib
        from bs4 import BeautifulSoup
        import sys
        from time import sleep
        for page in range(1,int(number_of_pages)+1):
            #Extract the image id only and save it in file
            url = urllib.request.urlopen(self.Website_url+'&page='+str(page)+'&sort=best')
            sleep(1)
            content = url.read()
            soup = BeautifulSoup(content, 'lxml')
            #search on page for div class and extract the id between the gi-asset attribute
            images_found = soup.find_all('gi-asset') #gi-asset is the HTML object that contains the image and the id in the search results
            for i in range(len(images_found)):  #range(len(images_found))
                Website_id = images_found[i].get('data-asset-id') #data-asset-id' is the HTML object that contains the ID
                if Website_id not in 'placeholder':
                    Website_ids.append(Website_id)
        return Website_ids

# Define some photographers
john_smith = WebsiteImages('John%20Smith', 'https://www.WebsiteImages.co.uk/search/photographer?assettype=image&photographer=John%20smith')

# Now we can get to the image IDs easily
#print(john_smith.GetNumberOfResultPages())
print(john_smith.GetImageIds())

The idea of using the class was to make the script more organised and the outputs accessible for different search results. Example below:

one_guy = WebsiteImages('One%20Guy', 'https://www.WebsiteImages.co.uk/search/photographer?photographer=John%20smith')

two_guy = WebsiteImages('Two%20Guy', 'https://www.WebsiteImages.co.uk/search/photographer?photographer=John%20smith')
\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

Class inheritance

Classes no longer have to be subclasses from object

class WebsiteImages:

Naming Conventions

Methods and variable names should be snake_case. Classes should be PascalCase, so you got that correct :-).

Imports

Any and all imports should go at the top of the file.

f"" strings

You should use f"" strings to directly implement variables into your strings.

url = urllib.request.urlopen(f"{self.website_url}&page={page}&sort=best")

User Interaction

The way a user has to interact with this program is pretty challenging to understand. If I was a normal user, I would do something like this:

person = WebsiteImages("John Smith", "https://www.WebsiteImages.co.uk")

But I would get an error, or no images returned because I didn't know the name had to be encoded with %20 and I didn't know I needed to specify a very particular url. I would blame it on the program and look for something else that's more user friendly.


I would solve this by only requiring a name. Since you're going to one website, you can save the url as an instance variable and sort things out there, instead of relying on a user to provide perfect input. Never trust user input.

john = WebsiteImages("John Smith")
print(john.get_image_ids())

And within the constructor you can do something like this:

def __init__(self, photographer: str):
    self.photographer = '%20'.join(photographer.split())
    self.website_url = f"https://www.WebsiteImages.co.uk/search/photographer?photographer={self.photographer}"
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Just one comment. No need to join the '%20' bit as it is an HTML unicode and it is automatically generated in the link if it is blank :) \$\endgroup\$ Jun 30, 2020 at 18:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.