architv / soccer-cli

:soccer: Football scores for hackers. :computer: A command line interface for all the football scores.
MIT License
1.09k stars 222 forks source link

Clean up code for the Stdout writer. #47

Closed carlosvargas closed 8 years ago

carlosvargas commented 8 years ago
================== Spanish Primera Division ==================

Getafe                     -  vs  -                    Malaga   6:30 PM UTC
============================= CL =============================

PSV Eindhoven              2  vs  1      Manchester United FC
carlosvargas commented 8 years ago

Btw, I kept getting the ImportError you guys were getting before.

> python .\soccer\main.py --live
Traceback (most recent call last):
  File ".\soccer\main.py", line 11, in <module>
    from soccer.exceptions import IncorrectParametersException
ImportError: No module named soccer.exceptions

I had to remove the from exceptions import IncorrectParametersException and do import exceptions then just reference exceptions.IncorrectParametersException.

Saturn commented 8 years ago

The exception importing error is due to the name clash that exceptions will have against the standard exceptions module.

If you run it like: python -m soccer.main --league LLIGA --standings You should not get an import error. Running it in this way will allow it to find soccer.exceptions no problem.

I will have a proper study of your PR a little bit later :+1:

ueg1990 commented 8 years ago

looks great to me :smile:

though i still dont get why the need to make the league_header a method esp when the name of the method also doesnt really match the implementation. lol

carlosvargas commented 8 years ago

@Saturn can't believe I forgot about that. Thanks.

@ueg1990 I made it a function since it's being used in 2 places (live scores and league scores) and didn't want to have duplicate code. What else could we name it? That function is just printing the league name in a "pretty" header.

ueg1990 commented 8 years ago

oh i thought it was used in only one place...sorry....my bad

carlosvargas commented 8 years ago

haha np, there's a lot of code being moved, it would be easy to miss if you're not looking for it. I also considered adding that to the league standings, but wasn't sure if it's overkill since you're already specifying which league you want.