Needs cleanup

[bradenbest]

As mentioned in #6, I tried to step into the code and immediately noticed 3 things:

• The functions are absolutely huge and the code littered with comments.
• The functions do way too much.
• Multiple functions are repeating the same steps, making it impossible to change the download behavior without changing multiple functions.

The code needs some serious refactoring.

My suggestions are to:

1. Start breaking the functions down into smaller functions
2. Remove duplicate functionality (e.g. move all the code specific to downloading a page to download_page(xkcd_number), like the example referenced in the last comment in Suggestions #6)

[tasdikrahman]

Hey @bradenbest

Referring to #10 and #6

I appreciate it that you are taking your time to point out the issues in the code. And I whole hearted-ly agree that there is redundancy in the code(I mean a lot!). I mean I just hacked it up under one night and this was barely working. These issues make it hard to maintain the code and I agree to it.

However a lot of things like BeautifulSoup, pythonmagic are not necessary here. So a lot of rewrite is required here.

When I wrote the initial release for this, I wasn't aware of the fact that xkcd had an API to it. So if anybody were to get the latest issue of the xkcd and the relevant information would be provided by requesting the url : http://xkcd.com/info.0.json

It would return back something like this,

{
    "month": "1",
    "num": 1632,
    "link": "",
    "year": "2016",
    "news": "",
    "safe_title": "Palindrome",
    "transcript": "",
    "alt": "I hope that somewhere in the world, \"Panamax\" is the last option on a \"size\" drop-down menu on a sex toy site.",
    "img": "http:\/\/imgs.xkcd.com\/comics\/palindrome.png",
    "title": "Palindrome",
    "day": "20"
}

And similarly for comic number 1 we can request http://xkcd.com/1/info.0.json. And it will return back

{
    "month": "1",
    "num": 1,
    "link": "",
    "year": "2006",
    "news": "",
    "safe_title": "Barrel - Part 1",
    "transcript": "[[A boy sits in a barrel which is floating in an ocean.]]\nBoy: I wonder where I'll float next?\n[[The barrel drifts into the distance. Nothing else can be seen.]]\n{{Alt: Don't we all.}}",
    "alt": "Don't we all.",
    "img": "http:\/\/imgs.xkcd.com\/comics\/barrel_cropped_(1).jpg",
    "title": "Barrel - Part 1",
    "day": "1"
}

So simply using requests, we can do the whole thing.

So I can see a massive rewrite coming up.
If anybody is up and willing to take part in some of the action, I will divide the work parts with him.

Any other suggestions would be more than welcome 😄

@prodicus

[bradenbest]

@prodicus I can try, but gosh, I haven't programmed in python in a while. The last big one I did was an html scraper called ngdl, that worked by grabbing the first url that matched "alternate", setting two pointers at the index of the match, and expanding outward until it hit the surrounding quotes, and copying that substring.

start = match
end = match
while(string[start] != '"') start -= 1
while(string[end] != '"') end += 1
return string[start:end - 1]

You know, the kind of silly/clever things you do when hacking things together.

Couldn't get cookies to work though. I was hoping I could spoof it, but I need cookiejar, which looks irritating to use.

And before that was a personal bookkeeping project where I used JSON to save data. I like JSON.

[bradenbest]

Also, what's __init__.py supposed to be? I'm not familiar with python build systems (I usually use shell scripts or makefiles), so I'm not sure what it's purpose is, but it's 0 SLOC.

[bradenbest]

And if you're going to overhaul it, well, here's what I would do instead of simple refactoring:

1. Start a new branch called '2.x' while keeping 1.x on the master branch for feedback, production, etc.
2. Start developing the new version from scratch, start from version 2.0.0.
3. When 2.x is usable and you feel it's functioning at or above the level of 1.x, change 2.x to the default branch, optionally rename master to 1.x.

This will keep major (incompatible) versions separate and will allow you to gracefully deprecate the first version while still allowing people to fetch it if they want. That's what I did with one of my projects.

[tasdikrahman]

@bradenbest I am gonna see if I would be able to start it soon.

As for __init__.py, I would read all the links on the 1st page
https://www.google.co.in/?gfe_rd=cr&ei=glSgVumeLPKK8Qe88YrgDg#q=init.py+in+python

Now don't be so modest @bradenbest . Your projects look cool to me. 😄 . Meanwhile, you can start working on it if you wish.

[bradenbest]

@prodicus So that's what it's for. You should definitely take advantage of it for the next update. As in, split the source between files that handle a task, and then have main.py be a wrapper that gets installed to /usr/local/bin, imports the package and all its xkcd-dl-related modules, and calls them.

Whoa, word salad. Here, I made a test implementation:

tree
.
├── main.py
└── phrase
    ├── get.py
    ├── __init__.py (empty)
    └── say.py

1 directory, 7 files

main.py:

#!/usr/bin/env python

from phrase.say import *

sayphrase()

say.py:

from get import getphrase

def sayphrase():
    print getphrase()

get.py:

def getphrase():
    return "Hello World"

shell:

chmod +x main.py
./main.py
Hello World

[tasdikrahman]

looks like somebody is really excited eh! 💃

Good one though. 😄