# Tag Info

39

Portability / Compatibility: Does it work correctly on all modern browsers (excluding old versions of Internet Explorer)? In Safari, there is a cute tingly-shiny (don't know how to call it) effect as the fully spread pieces fade out. In Chrome, either there is no such effect, or it's so faint it's not visible. (I'm on a Mac.) (Sorry, I don't know enough ...

38

Performance It works fine for me in Opera and Chrome, but it is buggy in Firefox. Profiling reveals that redraw is responsible (big surprise :) ), and there is not much to optimize without changing the whole concept. Two minor optimizations might be: save 2 * Math.PI in a constant. assign context.fillStyle = particle.color; outside the particles loop (...

36

Some of the numbers might seem plucked out of thin air but they aren't Still better have them explained. I can understand 3.6 and even 19.6 (it is $2g$, isn't it?), but I have no idea what 98/490 stands for. Declare them as symbolic constants with meaningful names. I don't see a need for abs in reaction_time = round(abs(diff.total_seconds()), 2) ...

26

The Great! You're not making the basic beginner error of using using namespace std;! The main() function is only 3 lines of code. The function declarations in the nbody class that don't change things include const which will help optimization later. The code utilizes the C++ random number generation rather than the C srand() and rand() functions. Because ...

22

You should always have an Alt attribute in your img tags <img src="http://i.stack.imgur.com/bGZ1m.png" width="80" height="43"> <img src="http://i.stack.imgur.com/bGZ1m.png" width="80" height="43" alt="Toy Airplane - Based on public domain image http://pixabay.com/en/aeroplane-aircraft-airplane-flight-161999/" > new lines added ...

20

Immutable objects are awesome. They are robust, predictable, and inherently thread-safe. Make the x, and y fields final, and change the operations to return the resulting Vector, for example: class Vector { private final int x; private final int y; public Vector(int x, int y) { this.x = x; this.y = y; } public Vector ...

18

I suggest breaking the code into functions, so you can easily import your code into a Python shell and test that your computation is exact (and not just thinking it is). To expand on other answers, you may: retry asking the speed if it is not an integer (detected when the int function raises a ValueError); in the same vein, since you don't really need to ...

16

The most important points have already been mentioned: Use double instead of int for the coordinates Make the fields private Concerning the recommendation to make the class immutable, I have to say that one has to really consider the possible application cases here. The Escape Analysis has significantly been improved in the recent Java versions. But for ...

15

There are many, many things wrong with your code; I'll list some, and then I advise you to fix as many of them as you can (and more) and then repost a new question with the revised code. First of all, you should compile your code with -W -Wall -Wextra (or -W4 if you use MSVC). Read the first warning diagnostic. Fix it. Recompile. Read the first diagnostic. ...

14

Here are some observations that may help you improve your code. Make sure to #include all required headers This program calls printf and srand but does not include the corresponding headers. Fix that by adding these lines: #include <cstdio> #include <cstdlib> Use objects You have a Particle structure and then separate functions that operate ...

13

Good start, but here are a few comments: The name for this class should be Vector2D rather than Vector, because you cannot represent a 3D or nD vectors using this class. int is not right type for representing points; doubles fit more double x; double y; By default, fields in Java have package access and it's usually a horrible idea. You should hide your ...

13

Your code is performing integer division for the vector, which is bound to fail: public void div(Vector vector) { this.x /= vector.x; this.y /= vector.y; } In this case, if the input vector is say x = 10 and y = 10, and we are x = 19 and y=19, the result will be: this.x = 1; this.y = 1; which is counterintuitive. I would recommend either not ...

13

(I know I'm quite late to celebrate...anyway) I wanted the firework to sparkle, so I created a Color object. When the firework is "old enough", the particles simply begin to sparkle using a random opacity :-) I also addressed @janos remark regarding the end of the animation: the last fireworks now have a couple of seconds to disappear. function ...

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

13

It crashes if user input for speed_ms is not integer If user presses any key before t = input("GO!! ") his reaction time is saved as 0. This is a major flaw in a code, and should be fixed.

12

After reading the program, I think it is pretty good for start, but there is some room for further improvement. Small improvements Imports Consider importing specific classes, instead of using the wildcard import, so that your namespace is not cluttered up. (Although there are also benefits in importing the whole package, see this SO question). Double ...

12

If it's wet, the friction_coefficient will be lower. Which means that Bd will be lower (which is probably wrong). It's difficult to see that, because of all the unnecessary parentheses. You missed the necessary ones. You probably want Bd = speed_ms**2 / (19.6 * friction_coefficient) Rt, Rd, and Bd should get better names, or need a comment. In particular ...

12

player_1 is read and assigned but never used. If the user misspells wet (e.g. as Wet or as wet), the program assumes that dry was meant - it should ask again until it gets an answer it understands or the player gives up (and consider adding icy for more fun). There's a lot of unnecessary parentheses - they could be trimmed to make the code more readable.

12

First, this is an awesome video! Upvoted for that reason alone. :) If n=2, the program finishes within milliseconds, whereas for n=3, it takes a whopping 115 seconds. Do you know about big-O notation? Just off the top of my head after watching that video: for n=2 you're computing the number of collisions for a 1kg block and a 100**2 = 10,000kg block, ...

11

if (this.velocity > dampening) { this.velocity -= dampening; } else { this.velocity = 0; } this.velocity = -this.velocity; Seems like you could use Math functions for this: this.velocity = Math.min(dampening - this.velocity, 0); That will simplify it somewhat. Logic behind it: if (this.velocity > dampening) { this.velocity -= dampening;...

11

This is pretty good work for a self-taught programmer. I'd like to make a few suggestions to improve readability. First, I would define a 2D vector class. That would allow you to simplify expressions where you are doing the same operation to both the x and the y coordinate, such as r = (r[x] + v[x]*dt + a[x]/2*dt**2 , r[y] + v[y]*dt + a[y]/2*dt**2), into ...

10

When using HTML local storage, you should always check for the possibility of failure. Some possible failure modes are: Local storage is not supported by the browser Local storage is prohibited for that domain by policy The storage quota has been exceeded Therefore, instead of checking if(typeof(Storage) !== "undefined"), you should wrap the operations in ...

10

All points by @Edvard are good. Here are a couple more. Encapsulation All your functions mutate a Particle structure. So why not just make this methods on your class? struct Particle{ double rx, ry;//position components double vx, vy;//velocity components double fx, fy;//force components double mass;//mass of the particle }; Particle ...

10

Just a note on your commenting: Should you be breaking XML comments up across lines like that? /// <summary> /// Class used for calculating /// how an object orbits another /// object. /// </summary> I feel that should be three lines: /// <summary> /// Class used for calculating how an object orbits another object. /// </summary> ...

10

I spent quite some time looking at this: High Level I love what you did here, a great showcase of using JavaScript in a powerful way. But from a readability and maintainability perspective, this code is not very good. Globals, globals everywhere This is good reading. Your code is suffering from (1) and (2) Naming Your constants are not consistently ...

10

I would consider the practice ok in this case. Your variable try appear to be roughly the "ASCII" equivalent of the ones used in the equation. You cite the paper where you got the equations from. (This is probably the most important thing to do) This allows the programmer to understand the context. Remember readability is subjective, look at something like ...

10

Refactoring code structure As of right now, you have: A struct named Object. A global std::vector<Object> of Objects. A function named attraction, which (I assume) calculates the attraction between two separate Objects. A function named render which (I assume) renders all of the objects in bodies. While this structure works fine, it's somewhat rigid ...

10

I'll build off of Quuxplusone's answer. You have issues with tab. int tab[N]; tab[0] = N-1; tab[N+1] = 0; // out of bounds Only the elements 0 through N-1 are available, so this assignment is wrong. Your initialization loop also attempts to initialize tab[N]. And in flip, since a and b are randomly generated between 0 and N-1, getting tab[a+2] can be N+1 ...

10

Your code is written in a hybrid C/C++ style. For instance your destructor has a delete (I can't find where the corresponding new is) and that is basically never needed. Use a std::vector to store array-like data. Also you do a lot of parameter passing like void Nbody::update_position(Particle *p). Use references instead, and use const Particle &p if ...

10

In addition to the other answers: Use unsigned integer type for DIM, Nbody.step, and Nbody.n since none of this can be negative; Use constexprsince C++11 instead just const for both DIM and EPS; Get rid of the unused argc and argv arguments in main; Consider more usage of const. For example f in Nbody::force() can be const, and a in Nbody::update_position ...

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