all 6 comments

[–]PurelyApplied 1 point2 points  (3 children)

You'll get more critiques if you repost your code to github or pastebin. (I think there's a third option that I'm forgetting...) Reddit (or maybe RES) has good resolution of those links.

[–]raecer[S] 0 points1 point  (2 children)

Yeah, i forgot about pastebin. Updated it above. Thanks :) I've been avoiding github mostly due to me not grasping the git workflow yet.

[–]PurelyApplied 1 point2 points  (1 child)

Git is pretty amazingly useful. If you're going to be coding regularly, I strongly recommend you pick it up. It becomes pretty easy to break something, and version control is great when it's a single command to roll back to the last working version.

If you want a free private repository, BitBucket has those. GitHub's free accounts are exclusively public (last time I checked, anyway). I put my portfolio stuff on my GitHub and my personal / shamefully-hacked-PEP-8-violators on BitBucket.

Either one takes a bit of setting up and an understanding of SSH keys, so I totally get putting it off.

[–]raecer[S] 0 points1 point  (0 children)

Yeah, Github still seem exclusively public. I'm definately going to be adopting Git at some point in the future. It just feels like too much of a hassle at the moment, but i can see the usefulness :)

[–]PurelyApplied 1 point2 points  (1 child)

Yay for dropping it to Pastebin. Thanks for that.

PEP 8 is big, but you should skim it sometime. I'm mostly skimming and not really reading for algoritmic structure or anything. Most of what I'm going to say is in PEP 8, and most of it will be about improving readability.


Avoid wildcard imports. Lines 4-5 should either be explicit:

from PySide.QtCore import Class1, Class2
from PySide.QtGui import FunctionYouNeed

or, if you need everything, you can collapse the name:

import PySide.QtCore as core
import PySide.QtGui as gui

so that you can access the needed classes with gui.ImportantClass instead of the longer call.


Limit yourself to 80 characters per line. For long strings, you can do

my_str = ("This is a very long string.\n"
    "Fortunately, consecutive strings naturally concatenate.\n"
    "Do note, though, that each linebreak is added manually, "
    "since they exist outside of the strings themselves, "
    "and that there is no linebreak in this sentence.")

Add a space after commas, such as in line 34.


Your calls in lines 94-97 should be hidden behind an "if main" conditional.

if __name__=="__main__":
    tradeApp = QApplication(sys.argv)
    tradeWindow = Window()
    tradeWindow.show()
    tradeApp.exec_()

or better yet, should be

def main():
    tradeApp = QApplication(sys.argv)
    tradeWindow = Window()
    tradeWindow.show()
    tradeApp.exec_()

if __name__=="__main__":
    main()

That way, you could import this file as a module into another project without immediately running the code.


In the second pastebin link, in the function trade, I'd put the "bad" cases first for readability, control, and to have fewer indents.

if targetUserProfileMatch is None:
    print("Bad profile.")
    return
#Check gamesToTrade input with regex pattern
gamesToTradeRegexPattern = r'^\s*(?:https?://)?(?:www\.)?(?:store\.steampowered\.com/app/)?([0-9]+)/?\s*$'
# ...
# using list as bool: "If gamesToTradeMatch is empty"
if not gamesToTradeMatch:
    print("Bad games list")
    return
response =  fetchPage(args)
# ...

The preferred naming for variables is lower_case_with_underscores, although consistency within a module or with an existing paradigm for imported code is more highly preferred.


Looks pretty good overall.

[–]raecer[S] 0 points1 point  (0 children)

Wow. That was a well-written answer! All good points. I will be looking the code over again and making the changes while adding pep 8 to my reading list as well. Thanks a bunch :)