# Tag Info

16

It's nicely done! One thing I'd pick on though is that your code is full of magic numbers: var camP = [random(500,1000), random(500,1000)]; // ... for (var y = -200; y < 200; y+=res) // ... for (var x = -200; x < 200; x+=res) // ... row.push(noise((x+camP[0])/200, (y+camP[1])/200)); // ... if (v < 0.49) // ... fill(random(30,40), random(100,128), ...

15

Welcome to Code Review! There are quite a few things I can talk about here. First off, your variable names need to be more descriptive. All of these variables should have more descriptive names: var res = 7; var area = []; var camP = [random(500,1000), random(500,1000)]; area = []; for (var y = -200; y < 200; y+=res) for (var x = -200; x < 200; x+=...

14

1. Review No docstring. What does this function do? What parameters does it take? What shape must the depth_image parameter be? What does it return? In Python, there is no need for a semi-colon at the end of a statement (unless another statement follows on the same line) and it is best to omit it. It seems that depth_image is required to have three ...

13

I'm just a beginner myself but I feel like I can point out a few things in this code. First, I would add some white space at the top of the file here: #import "ESFlashingErrorBar.h" #import "ESThemeManager.h" @implementation ESFlashingErrorBar #define kNumOfPoints 7 int lastFlash; Instead I would do this for increased readability: #import "...

13

Missing the specs You're missing a very important rule of the game: The pattern gets longer each time the player completes the pattern. If the player presses a wrong button, the game ends. You implemented only a one-shot round, which is not so interesting. Mutually exclusive conditions These conditions are mutually exclusive: if n == 1: # ... if ...

13

I ran through your code, and agree that the output is pretty. I like the idea, and it is a good use-case for a number of exercises. Your code suffers though from a few issues that are awkward. First up, you have just one real method. Why? You should isolate discrete logic elements in to their own methods. That's the most apparent issue, but we'll get there, ...

12

11

Just a few comments: std::vector<Vector2f> make_bezier(const std::vector<Vector2f>& anchors, double accuracy=10000.0) You're using float for all your other floating point variables. Unless there's some really good reason to use double here (for accuracy) I'd consider changing it to a float as well. std::vector<Vector2f> end; end....

11

Header file: It's usually preferred to have your own headers before library headers. This can help avoid possible dependencies and also keep the headers more organized. Do not use using namespace std in a header file. It can be okay to have it in an implementation file due to the locality (.cpp files do not get imported), but having it in a header file ...

11

The code is damn long, so just a few notes: Use initializer expressions like private JRadioButton setRed = new JRadioButton("Red", true); Use static factory methods to save even more like private JRadioButton setRed = makeColorButton("Red", true, Color.RED); private static JRadioButton makeColorButton( String name, boolean isSet, Color color) { ...

10

Just a few notes: You could improve readability by adding whitespace between operators. For instance, in the for loop statement: for (float i = 0.f; i < 1.f; i += 1.f/accuracy) {} For this as well, I would have a constant in place of the 1.f/accuracy to avoid performing the division operation each time (it's slow). Instead, you'll only be performing ...

10

Is it a good practice just to import everything so I don't have to go back and add stuff? In a modern IDE (e.g. NetBeans) you can just type in the class you intend to use and quick-fix the import. I usually do this so that I don't have spare imports laying around inadvertently. Also, you can use the source cleanup tool to remove unused imports when you're ...

10

Let your code breathe Your code is very hard to read - lines are long and crowded together, containing multiple method nesting (direction.subtract(Vector.multiply(2, Vector.multiply(Vector.dot(normal,...). Reading code is like reading a book. If you don't give the reader some breathing room (a period, coma, paragraph) - your reader will get lost, and ...

10

Disclaimer: This is the first time I have ever really looked into using OpenGL Keep in mind that my review may contain code that is not fully included in the question (such as the simplified main() function). Bugs I found that when I tried to resize the window, I would always crash the program. That wasn't very fun, so I set out to fix that first and ...

10

If this code is working. Then DON'T do any major refactoring without unit tests. You can add your standard modernizations that are easy to show work: Use Copy and Swap Idiom on the assignment operator: Image & Image::operator = (const Image & other) { freeImageStorage(); // This is not exception safe. initFromCopy(other); // If the ...

9

Make members final final member variables are easy to work with. As they can never be reassigned, they are less error prone, as accidental reuses are not possible. For example, in Dot, you can make all members final. In Controller, you can make blobs, dots, b, mf members final. Make members private It's a good policy to make member variables private by ...

8

Smart Pointers or Raw Pointers I was told by an industry professional that smart pointers (like shared_ptr) are frowned upon in game engines because of instances where reference counting adds an over head. Yes, shared_ptr has overhead over raw pointers. But shared_ptr also solves problems that raw pointers have. Namely, what happens if you do: Scene s; ...

8

You should always close any file you open. You can guarantee this by using a context manager. with open("some_file_name") as f: x = int(f.readline()) This is a safe (and clean) way to guarantee that a file is closed, no matter what happens inside the with block.

8

public static void flagOfLiberia(Graphics g) { Expo.setBackground(g,Expo.black); for (int d = 1; d <= numDots; d++) { int x = Expo.random(0,1000); int y = Expo.random(0,650); if (x < 300 && y < 300) Expo.setColor(g,Expo.darkBlue); else if (y % 100 < 50) Expo.setColor(g,Expo....

7

You're not using the C# naming convention: class ClassNamesShouldBePascalCase { const CONTANTS_ARE_IN_ALL_CAPS; private _membersShouldStartWithAUnderScoreAndBeCamelCase; //_camelCase public PropertiesShouldBePascalCase {get; set;} //PascalCase public void PublicMethodsShouldBePascalCase { object ...

7

Don't load the same resource repeatedly I don't have much time to give a full review but I can point out one big thing that I saw while looking through the code. public void paintComponent(Graphics g) { try { super.paintComponent(g); final Image backgroundImage = javax....

7

That seems good. I really like the callback mechanism to do whatever we want with the marked positions and not one specific action. That makes for a generic tool; good design. I don't have much to say to improve the algorithm itself, but here are my two cents anyway: Do you have a using std::swap; anywhere? Otherwise, I don't think that the unqualified ...

7

Some suggestions: In calcator, I would use a ternary expression: return 1. if n is None else n/100. calcator can be made even more efficient by having iterate_mandelbrot return 100. if the loop finished. Then you just divide the result of that by 100. This will result in 1. if the loop exits, avoiding the if test entirely. I would only do run self....

7

The first thing that strikes me as odd about your code is having the four single-character variables to represent color components. I would rather see a single drawColor variable with the UIColor type. Not only does this eliminate the need for the setColor method, but it also more easily lets us pull that color out for numerous reasons (make some other ...

7

I will focus on the C++ parts, not necesarily the algorithm. using namespace std; Don't! In header files this should never appear, and in C++ files (if you must use it) use it only in local namespaces. When you add it at the top of the file like you did, it imports every symbol in std:: locally (and prevents your code - or the code of any future mantainer'...

7

Seeing that you've come from Java, there are three main critiques that I have for you: Encapsulation It seems that you're exposing many of your class' data members unnecessarily. In particular, for your Renderer class, none of the marked public data members are used (or supposed to be used) by outside code. All of these should be marked private. /* LEEJAE ...

7

With the default values, the spiral does not look very smooth: it's easy to see that it's been drawn using a series of straight-line segments. That's because the step in $\theta$ is quite large — 0.5 radians is more than 28°. Setting step=0.1 and loops=10 is an improvement: But you can still see deviations from smoothness in various places. The problem ...

7

Not much to say about the last version; on the other hand, you could exploit PGF keys. \documentclass{article} \RequirePackage{xcolor} \RequirePackage{tikz} %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% % % combine ducks % %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% \makeatletter %1: skin colour \...

6

You want to avoid as much work inside drawRect: as possible. Instead of recalculating the attributed string for each line here do it whenever self.screen changes. That way it's done once instead of every time the some portion of the view is redrawn (which will happen a lot). Maybe keep a self.lines array with the attributed string for each line in it. Or ...

6

Coding style: This is a list of improvements you can apply to make your code nicer in a C++ context: Prefer enums or numerical constants over macros. VERTEX_FVF and VERTEX_FVF_TEX should either be members of an enum or const unsigned int objects. You did well by using std::uint8_t but you forgot to do the same for memcpy in the LoadTexture() function. ...

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