3
\$\begingroup\$

I am trying to write a util that converts an SO URL to revised version, for example, given this url:

'https://stackoverflow.com/q/57449318/10731613'

this util returns:

'https://stackoverflow.com/posts/57449318/revisions'

here is the code

import re
url = input()
if len(url)<10:
    url = 'https://stackoverflow.com/q/57449318/10731613'
url.replace(re.findall('\/\d{3,9}', url)[-1], '', 1).replace('/q', '/posts', 1)+'/revisions'

It works, and I would like some code review.

\$\endgroup\$
  • 1
    \$\begingroup\$ There's a separate site for that: Code Review \$\endgroup\$ – ForceBru Aug 11 at 11:18
  • \$\begingroup\$ @ForceBru Thank you! How can I move this post there? \$\endgroup\$ – yaojp Aug 11 at 11:27
  • 1
    \$\begingroup\$ I think you can just delete your question here and ask a separate one there. I don't see an option to migrate it to Code Review \$\endgroup\$ – ForceBru Aug 11 at 11:29
  • \$\begingroup\$ @yaojp please can you post a link to the Code Review question once it's up? I'd be interested to help there. \$\endgroup\$ – Geza Kerecsenyi Aug 11 at 11:46
  • \$\begingroup\$ I don't agree that this code is working. The return value of the last line is ignored. Also I'm wondering about the meaning of if len(url)<10:. \$\endgroup\$ – somega Aug 11 at 15:17
2
\$\begingroup\$

I'd suggest making this a function - while as a oneliner, it's good as is, when we start scaling it, adding in more cases, etc., it becomes a mess that requires rearranging for even the tiniest changes.

But first, let's start off with the changes I'd suggest.

First off, I'd like to slightly adjust your regex to handle URL parameters:

\/\d{3,9}(\/?\?([\-\w%*.~]+=[\-\w%*.~]+(&*))*)?$

The explanation is below. Also, to make it a bit easier for us later, we'll wrap this all in a capture group (simply surround with brackets).

(  # Start main set
  \/\d{3,9}  # Find any '/' character followed by 3-9 numbers
  (  # Start 'Parameter' set, which matches any query parameters
    \/  # Match '/' character...
      ?  # ... optionally
    \?  # Match escaped '?' character
    (  # Start 'Parameter List' set
      [\-\w%*.~]  # Character set, according to HTML5 spec (parameter name)
                +  # Match as many characters as possible
      =  # Followed by equals
      [\-\w%*.~]  # Character set, according to HTML5 spec (parameter value)
                +  # Match as many characters as possible
      (&*)  # Optionally match any '&' characters on the end.
    )  # End 'Parameter List' set
      *  # 'Parameter List' set is optional, but can also be repeated infinite times.
  )  # End 'Parameter' set
    ?  # Parameter set is optional
)
  $  # The match should be at the end of the string

It's fine that we're matching the query parameters with the last set of numbers (which we're going to delete), i.e it's not a problem that we delete the query parameters, because using the same parameters on a different page would be fairly useless.

Next, I'd put all of your code in a function block:

def convertURLToRevisionsFromQ(url):

Note how we're taking url as an argument, not as a user input.

The next step is to implement some basic error-checking, to throw some ValueErrors. Error handling manually is always superior to either catching built-in errors, or, even worse, not doing anything about them. So, we'll write an error handling helper function inside our main function, like so:

def throwInvalidInputError(err):
  raise ValueError("Invalid URL type! Error: " + err)

We can now do some error checking right off the bat, before we go on with processing the URL:

question_path_search = re.search('(\/q(uestions)?\/)', url)

if "stackoverflow.com/" not in url:
  throw_invalid_input_error("URL not from Stack Overflow.")
if len(url) < 23 or not question_path_search:
  throw_invalid_input_error("URL not from a Stack Overflow question path.")

You can see that we are also accepting /questions/ as valid, since we'll add support for those paths too in a moment.

The next change I'd suggest is to add support for URLs which don't necessarily have both number sequences, e.g https://stackoverflow.com/q/57449318 as opposed to https://stackoverflow.com/q/57449318/10731613. We're also going to bring in our Regex here, since we're finally going to start matching the ending. The only difference is, the 'ending' bit we need to remove will count as either only the query parameters, in the case of just one number sequence, or the last number sequence and the query parameters, in the case of two number sequences. So, we'll do the following:

matched = re.search('((\/?\?([\-\w%*.~]+=[\-\w%*.~]+(&*))*)?$)', url)
ending = matched[0]

if not matched:
  throw_invalid_input_error("Could not find question IDs in valid format!")

if len(re.findall('\/\d{3,9}', url)) > 1:
  matched = re.findall('(\/\d{3,9}(\/?\?([\-\w%*.~]+=[\-\w%*.~]+(&*))*)?$)', url)
  if not matched:
    throw_invalid_input_error("Could not find question IDs in valid format!")

  ending = matched[-1][0]

if len(matched) > 1:
  throw_invalid_input_error("Malformed URL")

Now, it's mostly just your code, except we'll add in one more flexibility: allowing both /q and /questions. To do this, we'll just use the question_path_search variable, which we defined earlier. Grab the first capture group from it, and that will be the version of /q vs /questions we'll use:

question_path = question_path_search[0]

url = url.replace(ending, '', 1)\
  .replace(question_path, '/posts/', 1)\
  + 'revisions'

Finally, we'll just do return url.

So, here's our final code, with a bit of testing code at the bottom:

import re

def convertURLToRevisionsFromQ(url):
  def throw_invalid_input_error(err):
    raise ValueError("Invalid URL type! Error: " + err)


  question_path_search = re.search('(\/q(uestions)?\/)', url)

  if "stackoverflow.com/" not in url:
    throw_invalid_input_error("URL not from Stack Overflow.")
  if len(url) < 23 or not question_path_search:
    throw_invalid_input_error("URL not from a Stack Overflow /q question path.")


  matched = re.search('((\/?\?([\-\w%*.~]+=[\-\w%*.~]+(&*))*)?$)', url)
  ending = matched[0]

  if not matched:
    throw_invalid_input_error("Could not find question IDs in valid format!")

  if len(re.findall('\/\d{3,9}', url)) > 1:
    matched = re.findall('(\/\d{3,9}(\/?\?([\-\w%*.~]+=[\-\w%*.~]+(&*))*)?$)', url)
    if not matched:
      throw_invalid_input_error("Could not find question IDs in valid format!")

    ending = matched[-1][0]

  if len(matched) > 1:
    throw_invalid_input_error("Malformed URL")


  question_path = question_path_search[0]

  url = url.replace(ending, '', 1)\
    .replace(question_path, '/posts/', 1)\
    + 'revisions'

  return url


res = convertURLToRevisionsFromQ(input("Enter Stack Overflow URL: "))
print(res)

Not so 'tiny' any more, but it's definitely more adaptable, and will probably work with few adjustments to whatever similar problem you throw at it. Now, it's more generally a URL path redirector, than just some ultra-specific code to solve an ultra-specific problem.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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