Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1038)

Issue 351750043: init

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 weeks, 2 days ago by yanhao.charles
Modified:
2 weeks, 2 days ago
Reviewers:
jhuig
CC:
jhuig_163.com
Visibility:
Public.

Description

init

Patch Set 1 #

Total comments: 12
Unified diffs Side-by-side diffs Delta from patch set Stats (+128 lines, -0 lines) Patch
M a1.py View 1 chunk +128 lines, -0 lines 12 comments Download

Messages

Total messages: 1
yanhao.charles
2 weeks, 2 days ago (2018-08-02 04:03:35 UTC) #1
一般如果函数超过200行,就应该考虑拆成多个更小的函数了
这样有利于代码复用,另外拆分出来的函数名称也相当于一种代码注释,说明了代码的功能,提高可读性

https://codereview.appspot.com/351750043/diff/1/a1.py
File a1.py (right):

https://codereview.appspot.com/351750043/diff/1/a1.py#newcode7
a1.py:7: button = 1
不必要的变量应该删掉,保持代码简洁

https://codereview.appspot.com/351750043/diff/1/a1.py#newcode9
a1.py:9: sta = [] # use to calculate the input between parentthese or
sta -> stack
用完整的单词,代码首先是给人看的,要注意可读性(readability)、可维护性(maintainable)

https://codereview.appspot.com/351750043/diff/1/a1.py#newcode11
a1.py:11: c = 0 # count for "("
最好不要用single letter variable,可读性不好,要用有意义的单词

https://codereview.appspot.com/351750043/diff/1/a1.py#newcode18
a1.py:18: # 4: *
flag没必要是整数,可以就存放一个字符,反而更容易理解,不是吗?

https://codereview.appspot.com/351750043/diff/1/a1.py#newcode20
a1.py:20: a = input("writer down the input: ")
这里也是,不要用`a`作为变量名
一般只有少数情况可以用单个字母的变量,比如`n`一般用来表示数量

https://codereview.appspot.com/351750043/diff/1/a1.py#newcode26
a1.py:26: while i < length:
为什么不用一个for循环呢
for i in range(length):

https://codereview.appspot.com/351750043/diff/1/a1.py#newcode48
a1.py:48: summ = sta.pop() * summ
上面这一串if ope == 2|3|4,很不好
这种叫做magic number,意思就是别人读的你代码,完全不知道这些数字表示什么,就是一个神秘的值放在那里
一般建议的做法是,事先定义一些常量,比如:
OP_PLUS = 1
OP_MINUS = 2
OP_MULT = 3
OP_DIV = 4
然后再写if语句,就很好懂了:
if ope == OP_PLUS:
...

https://codereview.appspot.com/351750043/diff/1/a1.py#newcode54
a1.py:54: print("wrong ), please check and re-input")
代码里面夹杂着这些错误处理的逻辑,会显得代码比较乱,更高级也更好的做法是用exception

https://codereview.appspot.com/351750043/diff/1/a1.py#newcode71
a1.py:71: elif 48 <= ord(a[i]) <= 57 or a[i] == ".":
magic number又出现了

https://codereview.appspot.com/351750043/diff/1/a1.py#newcode98
a1.py:98: elif a[i] == "/":# divid
elif a[i] == "/":# divid   ->
elif a[i] == "/":  # divid
注释和代码之间最好加2个空格,这是一般默认的约定

https://codereview.appspot.com/351750043/diff/1/a1.py#newcode103
a1.py:103: elif a[i] == "*":#multiply
elif a[i] == "*":#multiply   ->
elif a[i] == "*":  # multiply

https://codereview.appspot.com/351750043/diff/1/a1.py#newcode122
a1.py:122: print(res)
函数结束,加一个空行
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b