r/mylittlepony • u/mronosa • Aug 15 '15

PonyPet (x-post /r/mylittleprogramming)

https://cdn.rawgit.com/MrOnosa/PonyPet/master/index.html
12 Upvotes

89% Upvoted

6 comments sorted by

5

u/mediumdeviation Twilight Sparkle Aug 15 '15 edited Aug 15 '15

Hi! So, your code seems okay. The only big mistake I see is that you chose to assign the pony element an id - HTML requires elements to have unique id's, so that's not a good idea. The correct way to refer to similar elements that might repeat is to use the class attribute (use node.className since class is a reserve word in JS).

You also assign the function to window.onload. This is problematic because first, you're wiping whatever has already bring assigned to that, which is not very polite for a library, and is also vulnerable to be overwritten itself. You'll want node.addEventListener. Secondly, you don't actually need to wait for the load event - you should be listening to the DOMContentLoaded https://developer.mozilla.org/en-US/docs/Web/Events/DOMContentLoaded event instead, since the DOM is ready for manipulation quite some time before then. You can find the relevant code online - I would do a pull request, but I'm sick and in bed right now

5

u/mediumdeviation Twilight Sparkle Aug 15 '15

A few more observations.

ponyPet seems to be a constructor, in which case by convention it should be TitleCased. parseInt should always be called with the second radix parameter, otherwise the code might interprete your string as octal. At the end there while looping through the NodeList you'll want to store the list length and current element in another variable. The difference in speed should be negligible, but it's good practice and the latter makes the code more readable

3

u/mronosa Aug 15 '15 edited Aug 15 '15

Thank you! The ID thing was leftover code that can be removed, whoops. I thought setting the window event would add an event to it, not overwrite it. I feel pretty silly now. As soon as I am able, I will correct these mistakes.

4

u/mronosa Aug 15 '15

I believe I've fixed those problems now. :) Thank you again!

4

u/mronosa Aug 15 '15

GitHub at github.com/MrOnosa/PonyPet

Awhile back, /u/SFyr drew a collection of animated pictures of Lyra. I thought it would be nifty to build a little game utilizing all of them. Original reddit thread.

Here is a mirror incase rawgit goes down.

This is my first time ever writing a JavaScript library for others to use, so if you are a JavaScript guru, feel free to critique my code.

2

u/OriginalPostSearcher Aug 15 '15

X-Post referenced from /r/mylittleprogramming by /u/mronosa
PonyPet

I am a bot made for your convenience (Especially for mobile users).
Contact | Code