10

You're doing some weird things with the scripts. I consider the first to be better, since especially in the second one, you're doing a lot of things with variables which we have better ways to do. I'll point those out separately - lets first have a look at your first script. Your first script Code Duplication If you find yourself typing the same thing a ...


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


7

If you let me to be frank, this is not a class but rather a ratatouille - that is a random collection of functions. I do understand your idea and a chain of thoughts and in a way you are trying to do the right thing - to achieve the main goal of every programmer - to reduce the amount of code written. And it's for the good you started with OOP. However, ...


6

Performance The recursive function is as fast as I can think of. The iterative function should enqueue root.left and root.right instead of root and root to gain a cycle (micro-optimisation). Review I find it weird that the null node is considered symmetric IsSymmetric(null). I would throw an error for invalid input. Both algorithms are not able to deal ...


4

They both look like they are \$O(n)\$. A quick check with timeit show they both take essentially the same amount of time, and the time grows linearly with the length of the input string. It would be better to iterate over the characters in the string rather use an explicit index over the length of the string (while curr < len(s) is often a smell). def ...


4

Or since the check functions build on previous functions should I structure it the following way This is not a case that makes you go for inheritance. You should go for inheritance to solve the problem of "repeating same properties in more than 1 class", for example, you have 2 classes Lion and Cheetah class Lion{ protected $speed; protected $age; ...


3

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


3

First of all I would like to say that, in my opinion, testing API in a such way is wrong, because how does such unit test behave when: developer machine does not have Internet connection? developer machine does not have proper certificates? machine that run test does not have acces to such domain? external API URL changed? some new requirements (headers, ...


3

Your global variable naming violates PEP 8. Names in all upper-case are constants; variables that never change in value. Your globals aren't constant though. Lines like LIVES -= 1 change the value that LIVES holds. Yes, global names should be in uppercase, but only because globals should also ideally only be constants. Global mutable state is a pain to ...


3

It's clear that you're starting to pick up some good habits, but this code still has a way to go. This is a comment Module Docstring: No need to write this - it's obvious from context. Type hints def __init__(self, sf, cho, noa): Those parameters have no documentation, and it isn't clear what type they are. Do a Google for type hinting in Python - that ...


1

Your first solution's time complexity is non-trivial to determine, and it won't work for long lists because it'll run out of stack, while your second solution obviously needs linear time (optimal) and is unlikely to run out of memory, so I would scrap the first one right away. The first solution is also inferior because of the _mem member. There is no ...


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