Skip to content

Commit

Permalink
Merge pull request #102 from fau-fablab/refactor-code-smells-and-unne…
Browse files Browse the repository at this point in the history
…cessary-complexity

Refactor code smells and unnecessary complexity
  • Loading branch information
patkan committed Nov 19, 2015
2 parents 3f9ce38 + a6c1039 commit 4a4d884
Show file tree
Hide file tree
Showing 19 changed files with 69 additions and 72 deletions.
8 changes: 5 additions & 3 deletions FabLabKasse/UI/ClientDialogCode.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,12 @@


class SelectClientDialog(QtGui.QDialog, Ui_SelectClientDialog):

""" GUI code for the dialog for selecting clients """

def __init__(self, parent, shopping_backend):
""" constructor for the ClientDialog
:param parent: parent GUI dialog
:type parent: QtGui.QDialog
:param shopping_backend: current instance of ShoppingBackend
Expand Down Expand Up @@ -76,12 +78,12 @@ def insertIntoLineEdit(self, char):
def backspaceLineEdit(self):
if self.lineEdit_pin.hasFocus():
oldtext = self.lineEdit_pin.text()
if len(oldtext) > 0:
if oldtext:
self.lineEdit_pin.setText(oldtext[:-1])
self.lineEditPINUpdate()
else:
oldtext = self.lineEdit_client.text()
if len(oldtext) > 0:
if oldtext:
self.lineEdit_client.setText(oldtext[:-1])
self.lineEditClientUpdate()

Expand All @@ -99,7 +101,7 @@ def lineEditClientUpdate(self):

# Check if client number existst and switch to client in comboBox_client
client = self.getClient()
if client == None:
if client is None:
self.comboBox_client.setCurrentIndex(0)
return

Expand Down
2 changes: 1 addition & 1 deletion FabLabKasse/UI/LoadFromMobileAppDialogCode.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def __init__(self, parent, app_url):
QtCore.QTimer.singleShot(0, lambda: self.setWindowState(QtCore.Qt.WindowMaximized))
set_layout_items_visible(self.verticalLayout_app_download, False)
self.pushButton_app.clicked.connect(self._show_app_download)
if app_url == None:
if app_url is None:
self.pushButton_app.setVisible(False)
else:
LoadFromMobileAppDialog.set_qr_label(self.label_qr_app, app_url)
Expand Down
4 changes: 2 additions & 2 deletions FabLabKasse/UI/PayupCashDialogCode.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ def showSuggestedDonations(self):
pass
# .disconnect() returns TypeError if currently nothing is connected to the signal
# http://stackoverflow.com/questions/21586643/pyqt-widget-connect-and-disconnect/21587045#21587045
if len(suggestedDonations) > 0:
if suggestedDonations:
donationValue = suggestedDonations.pop()
# functools.partial is used here instead of lambda because "donationValue" needs to be evaluated here.
# when using a lambda, it would be wrongly used by-reference and get the value of the last iteration of this loop
Expand Down Expand Up @@ -182,7 +182,7 @@ def update(self):
self.state = "initCanPayout"
elif self.state == "initCanPayout":
pay = p.canPayout()
if pay != None:
if pay is not None:
[self.payoutMaximumAmount, self.payoutRemainingAmount] = pay

# all amounts under 50€ may be paid with <= n+50€
Expand Down
2 changes: 1 addition & 1 deletion FabLabKasse/UI/PayupManualDialogCode.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def insertIntoLineEdit(self, char):

def backspaceLineEdit(self):
oldtext = self.lineEdit.text()
if len(oldtext) > 0:
if oldtext:
self.lineEdit.setText(oldtext[:-3])
self.lineEditUpdated()

Expand Down
6 changes: 3 additions & 3 deletions FabLabKasse/cashPayment/client/PaymentDeviceClient.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ def poll(self):
cmd = None
else:
raise Exception("unknown status")
if cmd == None:
if cmd is None:
return
print "SEND CMD: " + cmd # +"\n"
self.process.write(cmd + "\n")
Expand All @@ -139,10 +139,10 @@ def poll(self):
self.lastResponseTime = monotonic_time.monotonic() # get monotonic time. until python 3.3 we have to use this extra module because time.monotonic() is not available in older versions.

response = self.process.readline()
if response == None and self.waitingForResponse \
if response is None and self.waitingForResponse \
and monotonic_time.monotonic() - self.lastResponseTime > 50:
raise Exception("device {} server timeout (>50sec)".format(self))
if response != None:
if response is not None:
print "got response: '" + response + "'"
assert self.waitingForResponse
self.waitingForResponse = False
Expand Down
10 changes: 5 additions & 5 deletions FabLabKasse/cashPayment/client/PaymentDevicesManager.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ def _canPayout_total(self, canPayoutAmounts):
:return: [totalMaximumRequest, totalRemaining]
"""

if len(canPayoutAmounts) == 0:
if not canPayoutAmounts:
return [0, 0]
[totalMaximumRequest, totalRemaining] = canPayoutAmounts[0]
for [maximumRequest, remainingAmount] in canPayoutAmounts[1:]:
Expand Down Expand Up @@ -298,7 +298,7 @@ def formatCent(x):
requested = self.requestedPayin
elif self.mode.startswith("payout"):
requested = self.requestedPayout
if requested != None:
if requested is not None:
text += u":\n{} von {} ".format(formatCent(totalSum), formatCent(requested))
if self.mode.startswith("payin"):
text += "bezahlt (maximal " + formatCent(self.maximumPayin) + ")"
Expand Down Expand Up @@ -418,23 +418,23 @@ def wait():
while p.startingUp():
wait()
pay = None
while pay == None:
while pay is None:
wait()
pay = p.canPayout()
print pay
print "Es dürfen maximal {} Cent gezahlt werden. Ein Rest-Rückgeld von unter {} Cent wird nicht zurückgegeben!".format(pay[0], pay[1])
shouldPay = 4213
p.payin(shouldPay, shouldPay + pay[0])
received = None
while received == None:
while received is None:
received = p.getFinalAmount()
wait()
print "Geld erhalten: {}".format(received)
p.poll()
if received > shouldPay:
p.payout(received - shouldPay)
paidOut = None
while paidOut == None:
while paidOut is None:
paidOut = p.getFinalAmount()
wait()
paidOut = -paidOut
Expand Down
10 changes: 5 additions & 5 deletions FabLabKasse/cashPayment/server/NV11/NV11Device.py
Original file line number Diff line number Diff line change
Expand Up @@ -233,10 +233,10 @@ def flushRead(self):
while self.ser.inWaiting() > 0:
self.warn("flushing serial read buffer")
self.ser.read(self.ser.inWaiting())
if len(self.rawBuffer) > 0:
if self.rawBuffer:
self.warn("flushing raw input buffer")
self.rawBuffer = []
if len(self.buffer) > 0:
if self.buffer:
self.warn("flushing input buffer")
self.buffer = []

Expand All @@ -254,7 +254,7 @@ def read(self):

# read bytes for de-stuffing

while len(self.rawBuffer) > 0:
while self.rawBuffer:
if self.rawBuffer[0] == 0x7F:
# the STX byte needs special care (byte-stuffing needs to be reversed)
if len(self.rawBuffer) == 1:
Expand Down Expand Up @@ -283,7 +283,7 @@ def read(self):

# attention, python subranging is a bit strange: array[a:b] returns array[a] ... array[b-1] (not including b!)

while len(self.buffer) > 0:
while self.buffer:
self.debug2("buffer: " + hex(self.buffer))

# check validity of packet
Expand Down Expand Up @@ -350,7 +350,7 @@ class Response(object):
}

def __init__(self, data):
if len(data) == 0:
if not data:
# empty response AFTER decoding
self.status = -1
self.data = []
Expand Down
2 changes: 1 addition & 1 deletion FabLabKasse/cashPayment/server/exampleServer.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
# The text of the license conditions can be read at
# <http://www.gnu.org/licenses/>.

from .cashServer import *
from .cashServer import CashServer
import random


Expand Down
10 changes: 5 additions & 5 deletions FabLabKasse/cashPayment/server/helpers/banknote_stack_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def _would_stack_from_payout(self, payout_stack, requested_payout):
True if it might be helpful. The driver should do further checks if it is really necessary.
:rtype: bool
"""
if len(payout_stack) == 0:
if not payout_stack:
# no notes to payout
return False
if requested_payout < min(payout_stack):
Expand All @@ -82,7 +82,7 @@ def _simulate_simple_payout(self, payout_stack, requested_payout):
"""
simulated_payout = 0
payout_stack = copy.copy(payout_stack)
while len(payout_stack) > 0:
while payout_stack:
if simulated_payout + payout_stack[-1] <= requested_payout:
# would pay out note, if it is not too large
simulated_payout += payout_stack[-1]
Expand Down Expand Up @@ -130,7 +130,7 @@ def get_next_payout_action(self, payout_stack, requested_payout):
"""which action should be taken next?
(see the documentation for BanknoteStackHelper for more context information)
"""
if len(payout_stack) == 0:
if not payout_stack:
return "stop"
if self._forced_stacking_is_helpful(payout_stack, requested_payout) and self._would_stack_from_payout(payout_stack, requested_payout):
return "stack"
Expand All @@ -145,7 +145,7 @@ def get_next_payout_action(self, payout_stack, requested_payout):

def can_payout(self, payout_stack):
""" implementation for CashServer.getCanPayout()"""
if len(payout_stack) == 0:
if not payout_stack:
# no notes available at all
return self.accepted_rest

Expand Down Expand Up @@ -210,7 +210,7 @@ def unittest_payout_forced_stacking(self, random_generator):
origpayout_stack = copy.deepcopy(payout_stack)

simulated_payout = 0
while len(payout_stack) > 0:
while payout_stack:
if self._forced_stacking_is_helpful(payout_stack, requested_payout):
pass
# print("extra stack away useful")
Expand Down
22 changes: 11 additions & 11 deletions FabLabKasse/cashPayment/server/mdbCash/mdb.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ def flushRead(self):
while self.ser.inWaiting() > 0:
self.warn("flushing serial read buffer")
self.ser.read(self.ser.inWaiting())
if len(self.buffer) > 0:
if self.buffer:
self.warn("flushing input buffer")
self.buffer = ""

Expand All @@ -144,7 +144,7 @@ def read(self):
else:
pass
# logging.debug("not read:" + self.ser.read())
if len(self.buffer) > 0:
if self.buffer:
logging.debug("buffer: {}".format(self.buffer.__repr__()))
if not ("\n" in self.buffer):
# we have not yet received a full response
Expand Down Expand Up @@ -187,7 +187,7 @@ def checksum(self, data):
# raises BusError or InterfaceHardwareError, see send()
# returns valid data
def cmd(self, command, data=None):
if data == None:
if data is None:
data = []
assert 0 <= command < 8
bytes = [self.addr << 3 | command] + data
Expand All @@ -203,11 +203,11 @@ def cmd(self, command, data=None):
# timeout for interface board: 1sec
time.sleep(0.1)
resp = self.read() # possibly raises BusError (will be caught below) or InterfaceHardwareError (will not be caught)
if resp != None:
if resp is not None:
resp = resp.strip()
logging.debug("resp: " + resp.__repr__())
break
if resp == None:
if resp is None:
raise InterfaceHardwareError("interface timeout")
else:
break
Expand All @@ -216,7 +216,7 @@ def cmd(self, command, data=None):
logging.debug("bus error, resending")
time.sleep(1)
continue
if resp == None:
if resp is None:
raise BusError("no successful response after 3 retries")
responseData = []
# convert response (hex-string) to a byte array
Expand All @@ -242,7 +242,7 @@ def extensionCmd(self, data):
# timeout for interface board: 1sec
time.sleep(0.1)
resp = self.read() # possibly raises BusError or InterfaceHardwareError (both will not be caught)
if resp != None:
if resp is not None:
resp = resp.strip()
logging.debug("resp: " + resp.__repr__())
return resp
Expand Down Expand Up @@ -313,7 +313,7 @@ def getBits(byte, lowest, highest): # cut out bits lowest...highest (including
status = {"manuallyDispensed": [], "accepted": [], "busy": False}
if data == [MdbCashDevice.ACK]:
return status
while len(data) > 0:
while data:
# parse status response
if data[0] & 1 << 7:
# coin dispensed because of MANUAL! REQUEST (by pressing the button at the changer device itself)
Expand Down Expand Up @@ -510,15 +510,15 @@ def _getPossiblePayout(v, t):
# go through the coins from large to small
for [index, value] in v:
n = t[index]["count"]
if previousCoinValue != None and n * value < previousCoinValue:
if previousCoinValue is not None and n * value < previousCoinValue:
# we dont have enough of this coin to "split" one previous coin
continue
if previousCoinValue == None:
if previousCoinValue is None:
previousCoinValue = 0
totalAmount += n * value - previousCoinValue
previousCoinValue = value

if previousCoinValue == None:
if previousCoinValue is None:
return [0, 0]

return [totalAmount, previousCoinValue]
Expand Down
16 changes: 7 additions & 9 deletions FabLabKasse/gui.py
Original file line number Diff line number Diff line change
Expand Up @@ -302,13 +302,11 @@ def restart(self):
# trigger reboot ('sudo reboot' will be executed by run.py)
result = dialog.clickedButton()
if result == rebootButton:
f = open("./reboot-now", "w")
f.write("42")
f.close()
with open("./reboot-now", "w") as f:
f.write("42")
elif result == shutdownButton:
f = open("./shutdown-now", "w")
f.write("42")
f.close()
with open("./shutdown-now", "w") as f:
f.write("42")
logging.info("exiting because of restart request")
self.close()

Expand Down Expand Up @@ -511,7 +509,7 @@ def updateProductsAndCategories(self, categories=None, products=None, category_p
for c in category_path[:-1]:
self._add_to_category_path(c.name, c.categ_id, bold=False)
# make last button with bold text
if len(category_path) > 0:
if category_path:
self._add_to_category_path(category_path[-1].name, category_path[-1].categ_id, bold=True)

# set "all products" button to bold if the root category is selected
Expand Down Expand Up @@ -678,7 +676,7 @@ def insertIntoLineEdit(self, char):

def backspaceLineEdit(self):
oldtext = self.lineEdit.text()
if len(oldtext) > 0:
if oldtext:
self.lineEdit.setText(oldtext[:-1])
self.on_lineEdit_changed()

Expand Down Expand Up @@ -816,7 +814,7 @@ def insertIntoLineEdit_Suche(self, char):

def backspaceLineEdit_Suche(self):
oldtext = self.lineEdit_Suche.text()
if len(oldtext) > 0:
if oldtext:
self.lineEdit_Suche.setText(oldtext[:-1])
self.lineEdit_Suche.setFocus()
self.searchItems(preview=True)
Expand Down
2 changes: 1 addition & 1 deletion FabLabKasse/scripts/logWatchAndCleanup.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ def isWarningLine(line):
if isOldUnzippedLog:
assert subprocess.call(["gzip", f]) == 0, "calling gzip failed"

if len(errorLines) == 0:
if not errorLines:
sys.exit(0)

print("Hi, this is FabLabKasse/scripts/logWatch.sh.\nThere were warnings or errors in the recent logfile.\nPrinting the recent {} ones per file:\n".format(MAX_ERRORS_PER_LOG))
Expand Down
2 changes: 1 addition & 1 deletion FabLabKasse/shopping/backend/abstract.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ class OrderLine(object):
def __init__(self, order_line_id, qty, unit, name, price_per_unit, price_subtotal, delete_if_zero_qty=True):

self.order_line_id = order_line_id
if order_line_id == None:
if order_line_id is None:
self.order_line_id = next(_id_counter) # may cause problems after ca. 2**30 calls because QVariant in gui somewhere converts values to int32. but who cares...
self.qty = qty
self.unit = unit
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ def printFiltered(consumption, search=None, regexp=None, scaleFactor=1, ignoreC
"""
sumFiltered = 0

if search == None:
if search is None:
# filter by regexp
search = ""
filterStr = regexp
Expand Down
Loading

0 comments on commit 4a4d884

Please sign in to comment.