14
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&...
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 ...
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
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" ...
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
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::...
8
Don't implicitly create global variables for (i = 0; uses a global i. Always declare variables before using them - in modern times, declare them with let or const, not var (which has too many problems, such as an unintuitive function scope rather than block scope, to be worth using nowadays)
Iterators Or, even better, rather than iterating manually, invoke ...
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
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, ...
7
The first bug is that you are misusing setInterval(). You should be passing a function, not the void result of the function, when calling setInterval().
Even if you had used setInterval() correctly, you still have a second bug, which is that currentDate is set just once when the code loads. If you call openClose() every five seconds, you would get the ...
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 ...
6
Review
The biggest critique about the Javascript I have is that collapsedInput could be declared with const because it doesn't get re-assigned.
It is possible that the code in the event listener for the input event could be simplified using classList.toggle().
If there is only one element with class name collapsed then perhaps it would be better to use an ...
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($("<...
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
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
Just a couple of things that I really feel strongly about (didn't work line for line through your code though).
Seperation of logic and presentation
Yes, there is a .style object, however this does not mean you should use it as the primary way to style your objects. Instead just apply the corresponding class (either through the classList object or the ...
5
I would not hard code 'container', instead make it an attribute of gridOptions :
var options = {
....
containerClass = 'container',
....
}
I would consider extending the options to include an id or selector of a parent (an anchor) where to add the grid (if null then use document.body as you do now).You could also add to the options the ID of ...
5
Chris Thomson's answer makes some very good points about the overall quality, and Leopoldo Salvo's answer points out several hidden issues with the timer(s) and event handling. My comments will be more about alternative approaches to various parts. Not much code, more of a reinterpretation of the stated task.
The first thing I notice is that each update ...
5
Template
The first thing that comes into mind when building widgets is how the HTML is written. I'd avoid the HTML concatenated strings in JS. They're just so messy. I prefer templates instead, like Mustache.
Template in <script>
The parser will ignore since type is unknown to the browser:
<script type="text/template" id="myTemplate">
<...
5
Watch out!
jQuery is full of secrets. html method is not just a simple wrapper around innerHTML property.
Possible fail condition:
var src = `if ("<x/>" !== "<" + "x" + "/" + ">") {
console.log("Math gone wrong.");
}`;
$("<script>").html(src).appendTo(document.head)
<!-- latest jQuery -->
<script src="https://code.jquery....
5
Don't pollute the global namespace. You have 1 variable and 3 functions all going into the global namespace. This is especially a problem because of the generic names you have. So they could be overwritten without you knowing it and it will cause issues.
The easiest way to fix this is to wrap all your code in a self executing function.
(function() {
.....
5
First off, you have a bug. On the line if (tagElemCollection[i].id != "") {, you used the != operator instead of the !== operator. This line should be changed to if (tagElemCollection[i].id !== "") {.
Secondly, you have some spacing inconsistencies. For example, you wrote }else { in some places, and }else{ in others, when it should be } else {, and/or } ...
Only top voted, non community-wiki answers of a minimum length are eligible
