# Tag Info

10

How can I not mess up the global scope? You can use an IIFE ( Immediately Invoked Function Expression ) to surround your code and then assign all your variables with var <- Very important. (function(){ var quicksearch = document.getElementById('quicksearch_top'); var comment = document.getElementById('comment'); var severity = document....

7

What about implementing your signal handling code inside a class? This could look something like the following: class GracefulExit: def __enter__(self): # set up signals here # store old signal handlers as instance variables def __exit__(self, type, value, traceback): # restore old signal handlers You can then use this in ...

7

I think it is mostly fine as it is. I would declare variables at the level they are actually needed, not before. Also, xAxis = TILE_WIDTH_AND_HEIGHT + xAxis; can be written as xAxis += TILE_WIDTH_AND_HEIGHT. i and v are not very good variable names (too short, and they don't fit together). What does v even stand for? vertical? In that case, I would use ...

6

is there a better way to achieve this functionality? You can use Function.bind() to create a function with the this context bound to Wyg.Editor: const node = document.getElementById('myFavoriteNode'); node.addEventListener('click', Wyg.Editor.nodeClickedEvent.bind(Wyg.Editor)); See this demonstrated in the snippet below. Notice that const was used ...

6

Write it this way instead: for (int v = 0; v < 3; v++) { for (int h = 0; h < 3; h++) { float x = SPACE_BETWEEN_TILES + h * TILE_WIDTH_AND_HEIGHT; float y = SPACE_BETWEEN_FILES + v * TILE_WIDTH_AND_HEIGHT; shapeRenderer.rect(x, y, 10, 10); } } … because: With your repeated addition, floating-point inaccuracies would ...

6

In addition to the comments in Heslacher answer, I have few more notes on your code: remove lines instead of commenting them. What's the use of lines such as //using System.Linq;? You're not going to use it, so you don't even need to mention it. try to avoid using static variables and replace them with instance variables. Having an instance-local state is ...

6

My current task is to find a score from an array where the highest/lowest scores have been taken away, and if the highest/lowest occur more than once (ONLY if they occur more than once), one of them can be added So the programflow can be written as: Find the highest item of the array Find the lowest item of the array Find the number of occurance of ...

5

Looks quite good overall; not much to criticise. (I'm not looking at the overall structure or logic - just the style; the game of life can be written in so many ways) Anyway, what I have is very trivial: Missing indentation in the bodies of tick and getLiveNeighbourCount Don't use a for...in loop for arrays (in isCellAlive); better and clearer to use a ...

4

myapp = {}; I think the general convention is to use camelCase, so myApp. But, why are you creating a namespace with only one property? And then later, with e.g. isBorn, you create functions in the global namespace? In any case, I would never use namepsaces named "myX" in any production code. myapp.World.prototype.tick = function() { Tick is a function!? ...

4

I'm not sure why you'd want to do this, however your code is really hard to understand. You merge your functions together better. name_of finds the parent frame, and then also goes on to handle creating and caching scope, along with finding the next name. This is defiantly breaking single responsibility. I'd also merge all your one/two line functions into ...

4

Move semantics: The makeScopeGuard() function accepts the lambda by r-value ref; but passes a normal ref to the constructor of ScopeGuard. template <typename Lambda> ScopeGuard<Lambda> makeScopeGuard(Lambda&& func) { return ScopeGuard<Lambda>(func); } You should either update ScopeGuard to take the function by r-value ref or ...

4

Accessing statics via this Defining the object reference outside the object just to gain access via a miss used accessor (this) is a hack and not how to use static objects to handle events. Binding objects to a function First what you did could have been a little less complicated You had something like class Editor { static nodeClick(event){ ...

3

NO This is not pythonic... and this is ugly... aaah, I can't get that code out of my head... There is this thing called __all__ 1. How does it work Make a list, put everything you want to make public in it and boom, you're done. 2. What it doesn't do You will still be able to do my_module.datetime if datetime is imported in my_module 3. What does it ...

3

Some extra points that Sam Onᴇᴌᴀ's good review skipped. Capitalizing function names Don't capitalize functions if they are not used to create an object. In JavaScript we only capitalize functions or objects that can be instantiated with the new token. Thus MakeStats should be makeStats. However if you were to return the object statJSON then you could use ...

3

To add a couple of points to konijns answer; You can remove the need for verbose switch statements with a dict: var kn = { 49: 'P1', 50: 'P2', 51: 'P3', 52: 'P4', 53: 'P5', }; ... var keyStr = kn[keyCode]; You can call a function when the page loads using the onload event: window.onload = focusOnLoad; Since it's only executed ...

3

In my opinion, the real problem you have with your algorithm is the number of time you go through your array. Going through it only once would bring better performance. I'd have a hard time explaining what I changed without showing you how I rewrote your algo. I decided to add all the values, since you can't know what is the highest/lowest score until you ...

2

I figured out how I would do this and did it step by step, much like I assume that you have done it. I like to build things rather than destroy them, so I started at the beginning of the problem First, I created methods to get the highest value and the lowest value. I used a foreach instead of trying to use indexes which allowed me to make it clearer what ...

2

You can avoid the global by passing the original handler as function parameter and binding it with a lambda in set_signals: def exit_gracefully(signum, frame, original_sigint): #... def set_signals(): original_sigint = signal.getsignal(signal.SIGINT) bound_exit_gracefully = lambda signum, frame: exit_gracefully(signum, frame, original_sigint) ...

2

Interesting question, your code is fairly easy to follow, JsHint cannot find anything serious to complain about. Since you have a validate function, I would also create a callback function this way your call to openModal could be this.parchment.openModal(template, this.createLinkCallback, this.validate ); Because you provide the functions themselves as ...

2

Ah, the age old pragmatism vs. purism. While it sounds like you have a healthy amount of skepticism, I think this is actually one of the few instances where it's OK to use a global variable. The core of your problem is you need to share data (state) between two routes. A global variable is the simplest way of doing this if all you're doing is appending the ...

2

Don't use a private attribute for the database, use a protected one. That way both classes can share the same $_db, and you only need one setDatabase. As for your code being OO or not, there simply isn't enough code here to make the claim one way or the other. 2 No. The visibility has little to do with security with that respect. (PHP Document on Visibility) In short: Public: Accessible by anything, within the object or outside. Private: Accessible only by that object. Protected: Accessible only by that object and objects that extend that object. Give a method (or property) only the scope it needs. A DB class ... 2 Feedback Simplifying block of if statements The set of if statements is a bit repetitive, especially since the values match the property names of the nested JSON object that is stored in localStorage. A simpler way to implement this would be to declare tempJSON first, with the nested location values at 0, then iterate through locationLog and increment the ... 2 I would suggest constants be defined for the values of the Manager::phase - e.g. const PHASE_FROM_DEPLOY_TO_MOVE = -1; const PHASE_SHIP_MOVES = 0; const PHASE_FROM_FIREORDERS_TO_RESOLVE_FIRE = 2; const PHASE_FROM_DAMAGE_CONTROL_TO_NEW_TURN_AND_DEPLOYMENT = 3 And use those in constant names in place of the values. That way, somebody looking at the logic can ... 2 Using$parent is an anti-pattern and sign of bad architecture. What if you code changes together with your parent chain - it will instantly break your code. Also, have you thought about how you are going to test it? To have a clean readable code, you want someone looking at it immediately see the meaning. What is the meaning of this object? - ng-model = "...

2

is there a better way to achieve this functionality? You might use a wrapping function: let node = document.getElementById('myFavoriteNode'); node.addEventListener('click', event => Wyg.Editor.nodeClickedEvent(event)); // or, using regular function expression node.addEventListener('click', function(event) { return Wyg.Editor.nodeClickedEvent(event); }...

1

There is a benefit to using local variables besides memory savings, namely avoiding polluting the global namespace. You can use closures to adhere to the best practice of using only local variables without having to re-query the DOM for elements you've already selected. Using closures to maintain references to objects Although it'll still consume as much ...

1

TL;DR: MSVC does some extra stuff and really doesn't like the call to makeScopeGuard. To elaborate: The main functions in the original examples only takes 16 instruction (GCC) or 23 (+8 inside makeScopeGuard) instructions (MSVC) respectively. GCC inlined everything it could, proved that valid_ and committed_ don't change (so they are basically constant), ...

1

Making a copy of the lambda there is going to hurt you a little bit, at least in practice. Anyway, I strongly recommend using what I call "the Auto macro": http://www.club.cc.cmu.edu/~ajo/disseminate/auto.h int main() { int f = foo("/dev/random"); if (!f) { return EXIT_FAILURE; } Auto(baz(f)); int b = 1; bar(b); return ...

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