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 ...


12

First off, it's fantastic that you're grouping your widgets together when you create them, and then grouping your calls to grid together. Most people who are starting to learn tkinter don't do that. The way you've done it makes the code much easier to understand than if you mixed it all together. Don't use wildcard imports Wildcard imports are discouraged ...


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

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 ...


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 ...


10

1. Instead of: def __init__(self, root): """Constructor""" self.root = root # root is a passed Tk object do: def __init__(self, root: tkinter.Tk) -> None: self.root = root Python 3 has built in support for type annotations, so there's no reason to use comments to say what the type of a variable is. If you use mypy (and if you're writing ...


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]....


9

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 ...


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( "...


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

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....


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 ...


7

Separation of business from presentation It's mostly good; Calculator is effectively your business layer. The one thing that creeps me out a little is using a tk.StringVar in it. One way to have a "pure" Calculator that has zero requirements on tk is to accept bound function references to answer.set and equation.set as arguments to your constructor....


7

To start, it's crucial that you stop creating a brand new label ten times a second. Just modify the existing one. Also, this is so simple that a class is not called for. Move as much as possible away from your thread, into your setup routine. Finally, your use of %H is likely incorrect given that you also include %p; you probably want %I for a 12-hour clock. ...


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

Imports There is a more idiomatic way of doing import tkinter tk = tkinter it is import tkinter as tk. You’re also importing StringVar, Label and Button from tkinter but still calling them using the tk namespace. Either remove the import line or remove the tk. part when creating such objects. Same for font, you import it from tkinter and use it only ...


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

This is not a good way to read in data. You have a .txt file purely just to read in a name. You're also reliant on data being in the right form. If someone tried to put in multiple names A really slow typer A really fast typer I don't type Then you mangle them all together in name: with open("NAME_FILE.txt") as f: name = "".join(f.readlines()) print(...


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

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 ...


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