Skip to main content
added 57 characters in body
Source Link
  • Do not use capitalization for function or variable names. This violates idiomatic python naming conventions (PEP8 link). Try out pylint to catch these kinds of issues
  • Prefer str.format() over concatenation. str.format() is not only a lot more flexible, but easier to use than concatenation and more efficient for combining more than 2 strings. With str.format() you do not need to explicitly convert objects to the string type (less TypeError exceptions). Instead of str(name) + ".jpg" do "{}.jpg".format(name)
  • Prefer iteration over indexingiteration over indexing. A list or any container can be iterated over to yield its contents. For example do for link in links_list: instead of while k <= (len(links_list) - 1):
  • Use sub functions to break up your code to improve readability and decrease complexity. Splitting up your code into logical pieces also improves testability and reusability. This function is on the larger side.
  • Do not use capitalization for function or variable names. This violates idiomatic python naming conventions (PEP8 link). Try out pylint to catch these kinds of issues
  • Prefer str.format() over concatenation. str.format() is not only a lot more flexible, but easier to use than concatenation and more efficient for combining more than 2 strings. With str.format() you do not need to explicitly convert objects to the string type (less TypeError exceptions). Instead of str(name) + ".jpg" do "{}.jpg".format(name)
  • Prefer iteration over indexing. A list or any container can be iterated over to yield its contents. For example do for link in links_list: instead of while k <= (len(links_list) - 1):
  • Use sub functions to break up your code to improve readability and decrease complexity. Splitting up your code into logical pieces also improves testability and reusability. This function is on the larger side.
  • Do not use capitalization for function or variable names. This violates idiomatic python naming conventions (PEP8 link). Try out pylint to catch these kinds of issues
  • Prefer str.format() over concatenation. str.format() is not only a lot more flexible, but easier to use than concatenation and more efficient for combining more than 2 strings. With str.format() you do not need to explicitly convert objects to the string type (less TypeError exceptions). Instead of str(name) + ".jpg" do "{}.jpg".format(name)
  • Prefer iteration over indexing. A list or any container can be iterated over to yield its contents. For example do for link in links_list: instead of while k <= (len(links_list) - 1):
  • Use sub functions to break up your code to improve readability and decrease complexity. Splitting up your code into logical pieces also improves testability and reusability. This function is on the larger side.
edited body
Source Link
  • Do not use capitalization for function or variable names. This violates idiomatic python naming conventions (PEP8 link). Try out pylint to catch these kinds of issues
  • Prefer str.format() over concatenation. str.format() is not only a lot more flexible, but easier to use than concatenation and more efficient for combining more than 2 strings. With str.format() you do not need to explicitly convert objects to the string type (less TypeError exceptions). Instead of str(name) + ".jpg" do "{}.jpg".format(name)
  • Prefer iteration over indexing. A list or any container can be iterated over to yield its contents. For example do for link in links_list: instead of while k <= (len(links_list) - 1):
  • Use sub functions to break up your code to improve readability and decrease complexity. Splitting up your code into logical pieces also improves testability and reusability. This function is on the larger side.
 

You could still randomly pick a filename, however I would still check if that filename already exists to stop overwriting. When you pick random numbers there is some chance that a number may appear in the same program run, which. A program is not very robust programmingif it steps on its own toes by potentially overwriting files it just downloaded.

then youYou could do something likeuse this generator in the main function:

As a side note, links_list is a very redundant name. A list implies multiple but so does the pluralization links. I (and Brandon Rhodes) would rather name it link_list. If this was ruby, you would want to just name it links.

 

I do have a suspicion that some of these splits are not needed. Also the indexing the first element of each list in links_list (from your code) belongs here, since it's an implementation detail of finding the links from an html source tree.

 
 

Note: I did not do error checking and exception handling, since the other answer covers that appropriately. I think the best solution would be some combination of mine and @MrGrj's answer.

  • Do not use capitalization for function names. This violates idiomatic python naming conventions (PEP8 link). Try out pylint to catch these kinds of issues
  • Prefer str.format() over concatenation. str.format() is not only a lot more flexible, but easier to use than concatenation and more efficient for combining more than 2 strings. With str.format() you do not need to explicitly convert objects to the string type (less TypeError exceptions). Instead of str(name) + ".jpg" do "{}.jpg".format(name)
  • Prefer iteration over indexing. A list or any container can be iterated over to yield its contents. For example do for link in links_list: instead of while k <= (len(links_list) - 1):
  • Use sub functions to break up your code to improve readability and decrease complexity. Splitting up your code into logical pieces also improves testability and reusability. This function is on the larger side.

You could still randomly pick a filename, however I would still check if that filename already exists to stop overwriting. When you pick random numbers there is some chance that a number may appear in the same program run, which is not very robust programming.

then you could do something like:

As a side note, links_list is a very redundant name. A list implies multiple. I (and Brandon Rhodes) would rather name it link_list. If this was ruby, you would want to just name it links.

I do have a suspicion that some of these splits are not needed. Also the indexing of links_list (from your code) belongs here, since it's an implementation detail of finding the links from an html source tree.

Note: I did not do error checking and exception handling since the other answer covers that appropriately. I think the best solution would be some combination of mine and @MrGrj's answer.

  • Do not use capitalization for function or variable names. This violates idiomatic python naming conventions (PEP8 link). Try out pylint to catch these kinds of issues
  • Prefer str.format() over concatenation. str.format() is not only a lot more flexible, but easier to use than concatenation and more efficient for combining more than 2 strings. With str.format() you do not need to explicitly convert objects to the string type (less TypeError exceptions). Instead of str(name) + ".jpg" do "{}.jpg".format(name)
  • Prefer iteration over indexing. A list or any container can be iterated over to yield its contents. For example do for link in links_list: instead of while k <= (len(links_list) - 1):
  • Use sub functions to break up your code to improve readability and decrease complexity. Splitting up your code into logical pieces also improves testability and reusability. This function is on the larger side.
 

You could still randomly pick a filename, however I would still check if that filename already exists to stop overwriting. When you pick random numbers there is some chance that a number may appear in the same program run. A program is not robust if it steps on its own toes by potentially overwriting files it just downloaded.

You could use this generator in the main function:

As a side note, links_list is a very redundant name. A list implies multiple but so does the pluralization links. I (and Brandon Rhodes) would rather name it link_list. If this was ruby, you would want to just name it links.

 

I do have a suspicion that some of these splits are not needed. Also indexing the first element of each list in links_list (from your code) belongs here, since it's an implementation detail of finding the links from an html source tree.

 
 

Note: I did not do error checking and exception handling, since the other answer covers that appropriately. I think the best solution would be some combination of mine and @MrGrj's answer.

edited body
Source Link
  • Do not use capitalization for function names. This violates idiomatic python naming conventions (PEP8 link). Try out pylint to catch these kinds of issues
  • Prefer str.format() over concatenation. str.format() is not only a lot more flexible, but easier to use than concatenation and more efficient for combining more than 2 strings. With str.format() you do not need to explicitly convert objects to the string type (less TypeError exceptions). Instead of str(name) + ".jpg" do "{}.jpg".format(name)
  • Use sub functions to break up your code to improve readability and decrease complexity. Splitting up your code into logical pieces also improves testability and reusability. This function is on the larger side.
  • Prefer iteration over indexing. A list or any container can be iterated over to yield its contents. For example do for link in links_list: instead of while k <= (len(links_list) - 1):
  • Use sub functions to break up your code to improve readability and decrease complexity. Splitting up your code into logical pieces also improves testability and reusability. This function is on the larger side.

Here's would be an examplea function for retrieving a unique filename.:

  • Do not use capitalization for function names. This violates idiomatic python naming conventions (PEP8 link). Try out pylint to catch these kinds of issues
  • Prefer str.format() over concatenation. str.format() is not only a lot more flexible, but easier to use than concatenation and more efficient for combining more than 2 strings. With str.format() you do not need to explicitly convert objects to the string type (less TypeError exceptions). Instead of str(name) + ".jpg" do "{}.jpg".format(name)
  • Use sub functions to break up your code to improve readability and decrease complexity. Splitting up your code into logical pieces also improves testability and reusability. This function is on the larger side.
  • Prefer iteration over indexing. A list or any container can be iterated over to yield its contents. For example do for link in links_list: instead of while k <= (len(links_list) - 1):

Here's would be an example function for retrieving a unique filename.

  • Do not use capitalization for function names. This violates idiomatic python naming conventions (PEP8 link). Try out pylint to catch these kinds of issues
  • Prefer str.format() over concatenation. str.format() is not only a lot more flexible, but easier to use than concatenation and more efficient for combining more than 2 strings. With str.format() you do not need to explicitly convert objects to the string type (less TypeError exceptions). Instead of str(name) + ".jpg" do "{}.jpg".format(name)
  • Prefer iteration over indexing. A list or any container can be iterated over to yield its contents. For example do for link in links_list: instead of while k <= (len(links_list) - 1):
  • Use sub functions to break up your code to improve readability and decrease complexity. Splitting up your code into logical pieces also improves testability and reusability. This function is on the larger side.

Here's a function for retrieving a unique filename:

added 823 characters in body
Source Link
Loading
added 259 characters in body
Source Link
Loading
added 314 characters in body
Source Link
Loading
added 15 characters in body
Source Link
Loading
added 2 characters in body
Source Link
Loading
added 157 characters in body
Source Link
Loading
added 416 characters in body
Source Link
Loading
added 558 characters in body
Source Link
Loading
added 558 characters in body
Source Link
Loading
Source Link
Loading