Skip to content
This repository has been archived by the owner on Jul 10, 2023. It is now read-only.

[zdict.py] code refactoring and test cases #47

Open
4 of 5 tasks
iblislin opened this issue Oct 7, 2015 · 5 comments
Open
4 of 5 tasks

[zdict.py] code refactoring and test cases #47

iblislin opened this issue Oct 7, 2015 · 5 comments

Comments

@iblislin
Copy link
Member

iblislin commented Oct 7, 2015

It seams that there is some global vars. Could we avoid this? or is it a necessary evil?

And, i want to introduce this coding style:
https://en.wikibooks.org/wiki/Computer_Programming/Coding_Style/Minimize_nesting
Please comment

I will keep updating the working list here:

  • reduce nesting
    • main()
    • MetaInteractivePrompt
  • test case
    • main()
  • Remove global vars
    • zdict.py
  • Remove dependency of args (avoid just pass args from zdict.py)
    • DictBase.lookup
@fkztw
Copy link
Member

fkztw commented Oct 7, 2015

I'm ok with this coding style.

I used global vars for args and dictionary_map because almost every function in zdict.py need these two vars.
If u don't want to use global, just pass these two vars as arguments to those functions which needed them.
I'm ok with that, I was too lazy to add these two arguments. lol

@iblislin
Copy link
Member Author

iblislin commented Oct 7, 2015

Thanks for comment.

I will work on removing the global vars.

iblislin added a commit that referenced this issue Oct 7, 2015
@fkztw fkztw added this to the v0.0.7 milestone Oct 19, 2015
@iblislin
Copy link
Member Author

@M157q global vars are really hard to remove.... The appear every where.. @wdv4758h suggests that we need to refactor the flow control of zdict.py

iblislin added a commit that referenced this issue Nov 12, 2015
Avoid just pass args from ``zdict.py``,
we list those flags explicitly.
@fkztw
Copy link
Member

fkztw commented Mar 27, 2016

20160327_11 36 21

  • zdict/__main__.py
  • zdict/dictionaries/moe.py
  • zdict/dictionaries/spanish.py
  • zdict/dictionaries/template.py
    • as a template for dictionaries unit testing
  • zdict/dictionaries/urban.py
  • zdict/dictionaries/yahoo.py
  • zdict/dictionary.py
  • zdict/easter_eggs.py
  • zdict/exceptions.py
  • zdict/utils.py
  • zdict/zdict.py

@fkztw
Copy link
Member

fkztw commented Apr 17, 2016

@iblis17 I'm thinking about just leaving this issue for refactoring only.
I'm going to create another issue for the unit-testings part.
So, you can make this issue turn into a PR base on issue-48 branch for code review.
What do you think?

@fkztw fkztw removed this from the Next milestone Mar 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants