15
I'll just start by looking at what you have, and then I'll get to your questions. (Edit: Seems I'm retreading a lot of what SirPython already covered. Didn't see that answer before I posted. Apologies for boring the reader, and no plagiarism of SirPython intended. Don't mess with a knighted snake!)
The HTML
The doctype for HTML5 is just <!DOCTYPE html&...
13
Strings are immutable. It means that when you allocate memory while creating a string, you are not able to reallocate it.
So, this code:
var a = 'a';
a = a + 'bc';
Will allocate a new block of memory for 'a' variable, instead of reusing already allocated.
When you do something like 'a' + 'b' + 'c' + 'd' each concatenation allocates a new block of memory ...
11
The only purpose of this script appears to be to illustrate the use the Array.prototype.slice.call "idiom" and the forEach function. So it's hard to make "improvements" when the purpose of the code isn't its actual result (which is trivial).
What the code actually does can be done in a number of ways, some of which won't (directly) use forEach and/or ...
10
HTML
In HTML, the property values should always be encased in ""s, even if the type is not necessarily intended to be a string. If needed, the JavaScript can parse the value into an actual number type.
In your HTML, you did not put ""s around the id. You should.
<button style="height:150px;width:150px" id="7" onclick="...
Inline styling
Inline ...
9
querySelectorAll
First off, if you're only dealing with relatively modern browsers (basically anything above IE7), you can use querySelectorAll, which is the fastest and easiest method to go about this:
document.querySelectorAll('[' + attrib + ']');
Here's the fiddle: http://jsfiddle.net/rc6Pq/
Sizzle
If you're stuck having to support IE7 and below, ...
9
Pure JS vs jQuery
Pure JavaScript will be significantly faster as you can see by this jsPerf which pits document.getElementByClassName vs the jQuery selector. Here are the results for Chrome 25:
$('.class') - 4355 operations per second
getElementsByClassName('class') - 94636 operations per second
As you can see, for this simple operation the jQuery option ...
9
I know you are doing this as an exercise, and personally I like the recursive function. But just as an alternative, there is also the much forgotten TreeWalker API.
Browser compatibility
Supported by IE9+, FF2+, Chrome 1+, Safari 3+, Opera 9+
Javascript
var treeWalker = document.createTreeWalker(document.getElementById("list"), NodeFilter.SHOW_ALL, ...
9
MVC thing is hard to be taken right. And yet it is the most popular (my guess) approach in the modern web development.
There are different ways to say what MVC means in particular application. I guess the cleanest part of the trio is the model. I'll call your model the DiceGame.
The DiceGame by itself is the most important part of your application. It ...
8
I don't really like this:
case ENTITY_REFERENCE_NODE:
if(node.getFirstChild() != null)
{
element.addEntity(nodeName, node.getFirstChild().getNodeValue());
break;
}
element.addEntity(nodeName, "");
break;
Idiomatically, more than one break in a case is confusing. You should either refactor to an if...else:
if(node....
8
Consider creating an object which can handle each case:
interface NodeHandler {
void handle(Node node, Element current, ...);
}
class ElementHandler implements NodeHandler {
public void handle(Node node, Element current, ...) {
readElement(node, current);
}
}
You can then populate a map, like this, to look up the appropriate handler ...
7
For a beginner, it isn't bad. There are some rough points to fix, but nothing serious!
Lets tackle by language:
HTML:
The first thing I've noticed is your doctype:
<!DOCTYPE html5>
That is an invalid doctype. Instead of html5, it should be just html, like this:
<!DOCTYPE html>
You have a closing <body>, but you forgot to open it ...
7
Some quick comments:
Const correctness
You really should make your code const correct.
For example at least these functions could be const declared:
bool hasChildren();
bool hasParent();
TreeNode* getParent();
TreeNode* getChild(int pos);
int childrenNumber();
int grandChildrenNum();
std::string getTextContent();
std::...
7
Descriptive names
As a global function name, generate is too vague: it should tell what it's generating, something like generateChart. As a method name it would be ok because the object itself would be what's being generated or doing the generation (e.g. chart.generate()).
(I'm aware that the function doesn't actually generate a chart as it currently is, ...
6
Considering this jsperf, I would stick to
var a = "asd" + "Foo" + "bAr";
6
Overall it looks pretty clean,
I'd like to see some comments on the functions so I know what they do without having to read the code, you could use JSDoc for this.
I'd recommend explicitly using Design By Contract or Defensive Programming so the users of the code know what to expect. Which one of these you choose will determine what you put in the comments.
...
6
Just two minor notes to add @Flambino's great answer:
Instead of x use intention-revealing names. What's the purpose of this variable? Name according to that. paragraphs would be a better name here, it explains the purpose of the variable and avoids mental mapping (readers/maintainers don't have to decode it every time). (It's just a minor thing here, ...
6
Personally instead of using .find I prefer passing context to the jQuery selector like this;
$('.remSpeed', y);
Which will find all instances of .remSpeed that are within the element y.
6
First of all, I would just like to say that this is a really good and useful function. From a Code Review standpoint, there are almost no errors in it that I know of. Here are a few things I found from examining it:
Keep spacing uniform
Line 8 doesn't have a space between parameters, where your other function calls do. This is most likely due to quick ...
6
Extending native prototypes is a little iffy. Not that this should cause any problems, but in principle it's best to avoid. The reason is mostly that if everyone extend native prototypes, then you're more likely to run into issues (i.e. if you include a library that tramples on your extensions or vice-versa). Hence the general consensus is to try to leave ...
6
Review of DOM game
Games coding is not easy and you have managed the fundamentals of a platformer, great work.
Unfortunately your game is technically flawed in a number of areas however first I will go over some design points.
Syntax
Your code syntax is not bad, but its also not great, specifically how you name variables and functions.
You have ...
6
Style and code
Numeric styles can be set using Number. eg modalParent.style.opacity = '1'; can be modalParent.style.opacity = 1;
To convert a string to a number use Number rather than parseInt, or coerce the value eg const foo = "1" * 1
Good code style's most important attribute is consistency. If you use a particular style use the same style throughout the ...
5
It sounds like you have your answer, but I wanted to contribute another point of view, maybe just for later reference or the good of the people :-)
Something that would most definitely speed things up would be to hold references to the objects instead of fetching them from the DOM every time. A simple example would be:
Instead of:
function ...
5
I suspect you're going to struggle to optimise that, as DOM manipulation is always slow. See this guy's answer.
You could try do something like he suggests, keeping your data and the DOM separate from each other.
Currently it loops through the entire form to count how many check boxes are selected, each time a checkbox is clicked
Why not increment or ...
5
Instead of storing them in an array, you can use $.each function for looping through them. Like this:
$("#Button").click(function() {
$(".class").each( function() {
alert( $(this).attr("name") );
}
});
5
You could do this:
$(function () {
var $carousel = $('#carousel');
var $switch = $('#switch');
var $headerSubmit = $('#header, #submit');
var $submit = $('#submit');
var text_strings = [ 'Sign In', 'Sign Up' ];
var form_strings = [ 'sign_in', 'sign_up' ];
$carousel.bind('slid', function() {
var index = $('#carousel .item'...
5
My 2 cents,
Well commented
Naming is never confusing
Quite readable
for my edification
* why [eval][0]('this') instead of eval('this'), to trick lint?
As far as I can tell, your library is fast because you optimized for the test case. You ought to generate a DOM structure with a thousand elements and query each one ( rendering your cache useless ), will ...
5
Here is a solution that is about as concise as your recursive solution. (8 lines of code.)
function walkDOM2(n) {
var stack = [n];
while (stack.length > 0) {
var node = stack.pop();
console.log(node);
stack = stack.concat(Array.prototype.slice.call(node.childNodes, 0).reverse());
}
}
Some notes on the above:
After you ...
5
MSDN documentation for this is a mess and for the most part, inadequate (it's better than MSDN's C++ ADO documentation though).
If you have access to MSXML, then you can use smart pointer wrappers that allow you to avoid worrying about releasing the objects. These smart pointers come with simplified function calls.
For instance, here is the normal ...
5
This code is good for tutorial use.
Some suggestion:
You are switching over 'x','y' and 'z' :
function rotateCube( axis ) {
switch ( axis ) {
case 'x':
mesh.rotation.x += 0.02;
break;
case 'y':
mesh.rotation.y += 0.02;
break;
case 'z':
mesh.rotation.z += 0.02;
...
5
Looks good to me. You could cache the ul:
var repoList = $("<ul>").appendTo('#results');
So you don´t have to query the dom again when appending the list items.
Also it is better to build the list items in memory and only append the complete list to the dom. This way you only trigger one reflow.
So in the $.each loop I would
repoList.append($("<...
Only top voted, non community-wiki answers of a minimum length are eligible
