Composing URLs for fetching passenger fares from multiple providers

Question

Can someone give me advice, how to make this code look nice?

def getFare(self, orig, dest, odate, idate, vid, pax, lang, epi, country_from, country_to):
 def create_link(link_str, API_url, dep_date, ret_date, magic_code, lang):
 '''
 :param link_str: str on which you will use format method
 :param API_url: on which url you should send request
 :param dep_date: departure date as strftime('%d.%m.%Y')
 :param ret_date: arrival date as strftime('%d.%m.%Y')
 :param magic_code: some numbers that identifies the request
 :param lang:
 :return: completed url for request
 '''
 kid_age = ''
 if len(pax['ages']) > 0:
 for age in pax['ages']:
 kid_age += str(age) + ','
 kid_age = kid_age[:-1]
 ages = 0 if kid_age == '' else kid_age
 trip_type = 'false' if idate else 'true'
 created_link = link_str.format(
 url=API_url,
 orig=orig,
 dest=dest,
 dep_date=dep_date,
 ret_date=ret_date,
 adults=pax['a'],
 children=pax['c'] + pax['i'],
 age=ages,
 trip_type=trip_type,
 magic_code=magic_code,
 epi=epi,
 curr=vendor_info[2],
 lang=lang
 )
 return created_link
 if vid == 1:
 user_id = 'some_id'
 new_url = 'http://www.www.www/search/results'
 idate = odate.strftime('%d.%m.%Y') if not idate else idate.strftime('%d.%m.%Y')
 format_url = '{url}/dc/{orig}/rc/{dest}/dd/{dep_date}/rd/{ret_date}/noa/{adults}/noc/{children}/ca/{age}/' \
 'ow/{trip_type}/mid/{magic_code}/curr/{curr}/lang/{lang}'
 url = create_link(format_url, new_url, odate.strftime('%d.%m.%Y'), idate, user_id, lang)
 else:
 two_way = '/rd/%s' % idate.strftime('%d.%m.%Y') if idate else ''
 format_url = 'http://{url}/iws/search/dc/{orig}/rc/{dest}/dd/{dep_date}{ret_date}/' \
 'noa/{adults}/noc/{children}/ca/{age}/ow/{trip_type}/mr/50/mid/{magic_code}/msparams/{epi}/' \
 'curr/{curr}/lang/{lang}'
 url = create_link(format_url, vendor_info[0], odate.strftime('%d.%m.%Y'), two_way, vendor_info[1], lang)
 res = requests.get(url, timeout=(3.1, 30))

Is this a good idea to put a string and parameters for format method in different function and format the string in it?

• 200_success: What does vid == 1 mean, and why would the URL not start with http in that case?
• Headmaster: @200_success this is just some identification number. Depend on this number the right url will be chosen. format_url = '{url}/dc...'.format(new_url) it will start with http

Answer 1

You can definitely simplify some things. For example, determining the ages variable:

kid_age = ''
if len(pax['ages']) > 0:
 for age in pax['ages']:
 kid_age += str(age) + ','
 kid_age = kid_age[:-1]
ages = 0 if kid_age == '' else kid_age

can be done in a single line and str.join():

ages = ','.join(pax['ages']) if pax['ages'] else 0

Some other notes:

• docstrings should be enclosed in triple double quotes, not single quotes
• I would move create_link from out of the function to "helpers" or "libs"
• define your format_url format strings as constants and move to the module level
• follow lower_case_with_underscores variable and method naming convention
• define your date format as a constant as well - this way, if it changes, you would make a change in a single place only

Answer 2

Some information about my decision.

Put a string and parameters for format method in different function and format the string in it is a BAD idea.

It is better to split my if/else statement into two functions and make everything there.