Podcast #128: We chat with Kent C Dodds about why he loves React and discuss what life was like in the dark days before Git. Listen now.

Hot answers tagged

18

Welcome to Code Review! Kudos to writing a fairly large program. Several things pop-out from your program. But, a few things first. If you are using any intelligent editor; please see if you can get a python linter (or a PEP-8 integration) in it. PEP-8 is the python's style guide, which makes the code consistent, and hence, readable/maintainable. I noticed ...


16

In your chatGUI's __init__ your setting up some constants, that should really just be defined as constants at the class level: class chatGUI: #Class That Handles GUI related tasks. BGCOLOUR = "#2a2a2a" # First Colour was: "#607D8B" # Contains hyperlink triggers. HYPER_LIST = ("http://", "www.", "https://", "ftp://") WINDOW_TITLE = "Oke's ...


14

Don't do wildcard imports Use import tkinter as tk and then prefix all tk classes and commands with tk. (eg: tk.Tk(), tk.Frame(...), etc). PEP8 discourages wildcard imports, and for good reasons. They pollute the global namespace, and they can overwrite variables and classes without you knowing. Tkinter is particularly susceptible to this since both tk ...


13

Don't use wildcard imports Instead of this: from tkinter import * Do this: import tkinter as tk With the latter, you then need to prefix all of your tk commands and classes with tk. (eg: tk.Tk()). This is a tiny bit of extra typing, but makes your code more readable. At some point you may choose to use ttk widgets which defines many classes with the ...


10

Class Scoping So your comment afore your global declaration of e1 mentions how you wish for e1 to be accessible in all methods. If you want a variable to be accessible in all methods of a class, the conventional way is to assign it as a property of self: class Foo(): def __init__(self, bar): self.boz = bar * 3 def baz(self): # ...


10

Playability For a game of 2048, I would expect that at least all of the tiles up to 2048 would have distinct colors. The animation for each move is extremely annoying and confusing: The sliding movement is usually too slow, but is sometimes fast. The result of two equal tiles colliding should result in a merged tile, but the merged tile appears ...


9

If you haven't read it already I'd highly recommend that you check out PEP8 as it is a great starting point for a lot of questions about Python code conventions. Some of what I write here will be repeated there. Tightly coupled functions One issue with your design is that your functions are designed such that you must call them in a particular sequence in ...


9

Coding style Please follow PEP8, the style guide of Python. The code will become so much easier to read. More tuple-assignments please Use tuple-assignments more aggressively. For example, instead of this: x1 = self.followers[-1].coords()[0] y1 = self.followers[-1].coords()[1] x2 = self.followers[-1].coords()[2] y2 = self.followers[-1]....


8

Now, I can't code in python or very well read it. But you have a bunch of comments, like these #the list of possible colour. colours = ['Red','Blue','Green','Pink','Black','Yellow','Orange','White','Purple','Brown'] #a function that will start the game. def startGame(event): #add an instructions label. instructions = tkinter.Label(root, text="Type in ...


8

Apart from the issues pointed out by other people, there are numerous problems with the way you use TCP sockets. There are many intricacies to using raw sockets which you might not be aware of. TCP is stream oriented One send call on one end does not equal one return value from recv on the other end. You might get a part of what you sent, or a combination ...


7

enter() def enter(): global x if x == 'add': answer = Entry(main) answer.grid(row = 0, column = 3, columnspan = 30) answer.insert(0, "%f"%(string[0] + string[1])) #Label(main, text = "%f"%(string[0] + string[1])).grid(row = 0, column = 3) #tkMessageBox.showinfo( "The Answer Is:", "%d"%(...


7

Don't repeat yourself This is really a bit excessive repetition: if val == 0: buttons[1].config(text = str(int(buttons[1].config('text')[-1]) ^ 1)) buttons[3].config(text = str(int(buttons[3].config('text')[-1]) ^ 1)) buttons[4].config(text = str(int(buttons[4].config('text')[-1]) ^ 1)) if val == 1: buttons[0].config(text = str(int(buttons[...


7

From PEP8: Wildcard imports (from <module> import *) should be avoided, as they make it unclear which names are present in the namespace, confusing both readers and many automated tools. You import Tkinter like that. Instead, use import Tkinter as tk and use tk. before each Tkinter variable. January = [ 27, 28, 29, 30, 31, 1, 2, ... That ...


6

1. Suitability for inspection Tom Gilb and Dorothy Graham (in their book Software Inspection) recommend that you inspect code in batches of about 250 lines. They observe that in practice, as the batch size increases, the quality of the inspection (in terms of number of defects discovered per line of code) falls, because people have limited powers of ...


6

Use more python, e.g. instead of: for i in hex_digits.keys(): if i == tot: tot = hex_digits[i] else: tot = tot Remove extraneous keys() and it becomes: for i in hex_digits: if i == tot: tot = hex_digits[i] else: tot = tot Then remove unnecessary else and it is: for i in hex_digits: if i == tot: ...


6

I ran your code and messed around a little with it, and it works pretty good. You also have a pretty cool idea here, that I might have to steal next time I do a TKinter GUI. As far as high-level design, I agree with your decision to have each entry line be its own object—it's much easier to think about than the other approach you described, with the ...


6

UI issues It's not obvious where the input text box is. It has no border or colour difference until it gains focus (which is difficult if you don't know where it is). It's not obvious where the output is. I would expect the output to be as prominent as the input — after all, it will contain just as much text. I would also want it to be a read-only text ...


6

First off, whitespace. You should have two blank lines between each top-level function/class/code block. Secondly, the standard for naming in Python is snake_case for functions and variables, and PascalCase for classes. I'm also noticing a lot of repetition with functions like addNumberOne, or addNumberTwo. I'd recommend extracting the repeated code, and ...


6

I love seeing functionality like this packed so concisely. Really nice job. UX When I ran your code, it ran fine, but a usability problem is that the text entry box does not have focus, you have to click on it to make it have the active focus/cursor. You should add this to your script: typing_ground.focus_set() I ran it with python and python3, both ...


6

Importing tkinter Pick a single way to import tkinter. You currently have this: import tkinter as tk from tkinter import * Remove the second line so that you only have this: import tkinter as tk That means that you have to prefix all tkinter classes and constants with tk., which is a Good Thing. The Zen of Python says explicit is better than implicit, ...


6

Bug You imported tkinter as a wildcard (*) so you should remove tkinter. from your program for proper operation. Minor imprecision The information text lacks spaces, some words stick to each other, giving a feeling of lack of professionality. Lack of focus on the important The most important part of the program is the entry when the user types its text, ...


6

I prefer import tkinter as tk and I can see that tk.Button, tk.INSERT, tk.FALSE are not my local variables/classes. You can use string.ascii_lowercase instead of letters list, and string.ascii_uppercase instead of capital. You don't have to convert string.ascii_lowercase and string.ascii_uppercase into list. https://docs.python.org/3/library/string....


6

Text widget indexes e.delete(0, tkinter.END) The canonical first character of a text widget is the index "1.0", not the integer zero. Your code still works, but only by accident. Layout out your widgets instructions = tkinter.Label(...) instructions.pack() ... scoreLabel = tkinter.Label(...) scoreLabel.pack() I personally think it would make your code ...


6

Don't use wildcard imports Change from tkinter import * to import tkinter as tk, and then prefix all tk classes and variables with tk. (eg: tk.Frame(...)). One reason is that PEP8 discourages wildcard imports. Also, by using the prefix your code becomes more self-documenting. One problem I see people make is that they import * from both tkinter and ttk. ...


6

Most of your inline comments are useless I see lots of comments like this: def login_button(): # Defines login_button That comment adds no information. It just adds visual "noise". I recommend removing all comments that simply state what the code does. Comments should explain why, and only if it's not obvious Group your layout code separately from ...


6

Nice. Here are some observations: consider using an IntEnum: This helps remove "magic" numbers from the source code. (A year from now, will you remember that if self.selected_type == 3 is a check to see if the mode is 'File'?) from enum import auto, IntEnum class SoundType(IntEnum): NOTE = auto() DESIGN = auto() FILE = auto() Then later ...


5

Style Python has a guide style called PEP 8. If you have no good reason not to follow it, just follow it. You'll find various tools to check your code (pep8, pep8online.com/, etc) and even to fix it more or less automatically (autopep8). Here's a list of problems detected : txt_original.py:13:80: E501 line too long (105 > 79 characters) txt_original.py:...


5

First off, the way you've named your functions is wrong, according to PEP8. Functions and variables should be in snake_case, and classes should be in CamelCase. Secondly, rather than creating a static attribute on your classes, NAME, you could just do the following: MyClass.__name__. Finally, there are no comments here. At the minimum, you should add some ...


5

In looking at your code, there are a few things I would do: Subclass TK.. class MainGUI(Tk): Attach all gui components to this class.... your frame for example. Add a queue and a polling method for GUI updates to the MainGUI class (this may help fix your freezing issue). You can use the TK.after method to ask the mainloop to do this periodically. Do the ...


5

It is mostly PEP8 conform, nice! One thing: Use just one empty line between functions. The imports datetime, glob and os are not used. I needed to change the tkinter-imports for python3 on linux: if sys.version_info.major == 2: import Tkinter as tk import tkMessageBox as pop_up import tkFileDialog else: import tkinter as tk import ...


Only top voted, non community-wiki answers of a minimum length are eligible