10

Positive Changes getReader() is only called once, while called multiple times before. there are more early exits as before, which results in less nested and lesser deepened nested statements method getPdfStreamUf hides the fallback method away from the main method, this functionality deserves its own method Negative Changes I agree with the developer that ...


10

Welcome to code review, and good job for your first program I suppose. Style Docstrings: Python documentation strings (or docstrings) provide a convenient way of associating documentation with Python modules, functions, classes, and methods. As you can see, even for a relatively simple function, documenting using comments quickly makes it unpleasant and ...


9

Your app function is just the same as the print function. So there is no need for it to exist at the moment. When working with files you should always use the with keyword to ensure the file is properly closed (even in the case of exceptions). The pathlib module has a Path object which makes handling file paths a lot easier. You are doing a lot of ...


6

This is a very interesting task. Good work on doing it. Here are some criticism. print('If your integer is a negative, the script will ask.') print("DO NOT GIVE -X integers for input!!!") print('Script will output multiple - symbols for a negative integer transformation') Do not use both double quote ("") and single quote ('') strings. Pick one. I ...


5

Welcome to Code Review and welcome to C++! Let's go through the code and see what can be improved. In C++, the headers of the form <xxx.h> are deprecated, which means they should not be used. You are recommended to use #include <ctime> instead of #include <time.h>. using namespace std; is considered bad practice because it causes name ...


5

Unfortunately, there is at least one blatant issue with the program in its current state: unused arguments. Let's have a look at it and find out why that's important. Before that, a short disclaimer, though: I'm not a F# developer. However, I know functional programming (e.g. Haskell). I don't know the .NET lands by heart. Take this review with a grain of ...


5

Your first code snippet has the problem that (in Python 3) input always returns a string, so type(user_in) will always be str, which will never compare equal to int. Your second code snippet solves the problem in the correct way: try to convert the result to an integer, and catch the exception in case that fails. Fixing that code is as simple as adding a ...


4

As a preface: Your code works good, is readable and does its job efficiently. The following review, though long, doesn't mean there is anything substantially wrong with it :-) General comments In the constructor, you're binding the event handlers to this. However, since you declare these event handlers as arrow functions, they are already bound to the ...


4

Welcome to the wonderful land of computer programming! There are a few things you could do that could help clean up that code. However, if this is for a class, I would recommend not to add functions, make sure the player is giving a valid safe input, and avoiding namespace issues for future development. But if it is for a class, here are yet a few Where ...


3

Pythonic. Zen of Python. Idioms of Python. These are phrases that will come up in python code. Basically they refer to the readability. Other programmers have to be able to read the code you write. Here are a few things that can be done to improve your knowledge. In the console, help() can be used in conjunction of other methods to get the docstring and ...


3

Fix your imports of tkinter You are importing tkinter twice, once with a wildcard and once "as tk". You should not be using the global import at all (see PEP9). Stick with a single import: import tkinter as tk There will be places in your code that need to be modified to account for this. For example, change all instance of Checkbutton to tk.Checkbutton. ...


3

We can simplify this quite a bit. At a high level, note that all you are doing is: Converting each digit of the input to an array of possible neighbors. Now you have an array of neighbor arrays. The answer is simply the cartesian cross product of those neighbor arrays. Sort them. Turn the into back into strings. The cartesian cross-product is a simple ...


3

The "terminating" allocators work well for small programs like this; in larger projects or libraries, we want to do something better than terminate the program when allocation fails. A common naming scheme (perhaps taken from Perl) is malloc_or_die() - that's slightly clearer about the behaviour. It's usual to end your error message (and indeed program ...


3

A couple of things: Any time you have a function which could fail, it should return an Option<T>. Ask yourself, if someone calls convert_to_hex_from_binary("foobar") and gets back "", is that reasonable? They will need to manually check that their input makes sense, or that the output makes sense every time. Static checking of these errors is part ...


3

Using @classmethod for constructors is perfectly fine. It is basically the only reason you ever need to use them. Classes create a namespace (and Namespaces are one honking great idea -- let's do more of those!). You don't need to preface all your methods and classes with my_. Your code becomes much more readable if you just name them according to what they ...


3

I see two race conditions. When you write the file, two processes may write it at the same time and one entry will be lost. Between reading the user list and adding a new user, i.e. in the time when the user chooses a password, another process could add the same user, so the user is added twice even when you checked that the user is not in the list when ...


2

I disagree on you with the need for a wrapper around the stdlib argparse, but that's a matter of taste. And I don't see anything wrong with your implementation of the rather thin wrapper. Classmethod as Constructor This is NOT what you're doing. A constructor creates an instance in a way somehow different from the standard and returns it. An example: ...


2

Early returns are OK. if (args < 4) { printf(....); return; } .... emphasizes where the business logic is. The condition (c = **++argv) != '\n' looks sort of strange. It is indeed possible to embed a newline in an argument, but it doesn't warrant a special case. It is just one way to malform an argument, and there are plenty of them. c = d; does ...


2

PDF specification ISO 32000-1:2008. Funny, that's a roughly 800 page document if I see that right :) You might want to link to relevant parts if you want a reader to read it, otherwise that doesn't really help with the review here. The refactored version has less nesting, yes, so that's somewhat easier to read. However I've the feeling that the extracted ...


2

Instead of creating an empty list and appending the values to it, try using list comprehension. so instead of y_location = [] x_location = [] for each in range(-size, size+1): y_location.append(parabola(each, size)) x_location.append(each) try y_location = [parabola(each,size) for each in range(-size,size+1)] x_location = list(range(-size,size+1))...


1

The stuff with counter_one and counter_two is rather oblique, and it seems that it would break if someone were to choose the same password as someone else (or username, but you do have safeguards against that). If I understand what you're trying to do correctly, you could just do account_match = any([(f"'{user_name}'" in line) and (f"'{password}'" in line) ...


1

Firstly, thanks for providing the test program. That always makes code easier to review. Unfortunately, when I tried it, I found a null pointer dereference here: std::string s = *std::get_if<std::string>(&t); I had to replace with auto p = std::get_if<std::string>(&t); if (!p) { continue; } std::string s = *p; The ...


1

Avoid trying to combine assignment and compare in the same statement. The cast within the statement makes it even hard to read. Also, handle EOF with getchar. You can get EOF with redirected input. Instead of this: while((ch = (char) getchar()) != '\n') { . . . } This: int value = getchar(); ch = (char)value; while (value != EOF and ch != '\n') { ...


1

This code looks good. There are only a couple improvements I would suggest. Loop over operations The code is somewhat repetitive for the operations, which goes against the Don't Repeat Yourself principle. Those operations could be added to an array and iterated over. You might have to get creative with calling functions but the operation functions could be ...


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