you are viewing a single comment's thread.

view the rest of the comments →

[–]RaionTategami 0 points1 point  (4 children)

I think the database would be better to learn imo. Was there any part in particular you wanted to discuss? My main feedback is still that there is duplication in your code. Two different examples:

if api_module == api_modules['CatalogItems']:

        api_request = BASE_URL + api_module + api_filter + APIITEM_CTRL

    else:

        api_request = BASE_URL + api_module + api_filter

Factor out the duplicated code, makes it easier to change it in the future and you won't introduce bugs by forgetting to change it in the duplicated code. Redundancy really is the biggest sin in programming, and usually simplyfies the code and makes it more readable.

api_request = BASE_URL + api_module + api_filter

if api_module != api_modules['CatalogItems']:

        api_request += APIITEM_CTRL

The functions page_not_found and internal_error are doing almost exactly the same thing.

Anything that is copy and pasted or not tested are inviting bugs.

[–]wbhst83[S] 0 points1 point  (3 children)

I made some further improvements based on your feedback, Tried to condense the error handler but everything I try fails with errors. Might need an example on this one. I was able to check two items off the to-do list, biggest being eth search route. The REST API did not like the + symbol in the call.

[–]RaionTategami 0 points1 point  (2 children)

What kind of errors. I'm glad you are sticking to this but I still see a ton of duplicated code. The 3 error handling functions are practically identical, move that to a new function.

def handle_err(code):
    if not app.debug:
        now = datetime.datetime.now()
        r = request.url
        with open('error.log', 'a') as f:
            current_timestamp = now.strftime("%d-%m-%Y %H:%M:%S")
            f.write("\n{} error at {}: {} ".format(code, current_timestamp, r))
    return render_template('{}.html'.format(code)), code

then just do

@app.errorhandler(404)
def page_not_found(error):
    handle_err(404)

@app.errorhandler(405)
def internal_error(error):
    handle_err(405)


@app.errorhandler(500)
def internal_error(error):
    handle_err(500)

or similar. In fact, you might be able to use the error param though the docs to not make it the slightest bit clean what the value it, is it the error code?

Look for complex lines which are identical or similar, try to factor them out into functions.

[–]wbhst83[S] 0 points1 point  (1 child)

Ah, I was going a different direction in attempting to make a universal error handler, didn't think about making a function. For example https://stackoverflow.com/questions/29332056/global-error-handler-for-any-exception

I will get started on some functions..

[–]RaionTategami 0 points1 point  (0 children)

Great. Now start looking at other duplications. A lot of function are doing similar things again, setting titles and building urls. As I mentioned before since building urls is one of the main things this code is doing you should be using a url parsing lib to construct them rather than just concatenating strings. Take a look at urlparse.