/    Sign up×
Community /Pin to ProfileBookmark

[RESOLVED] Code works but it can probably be written more efficiently

Hi, as per the title, my code works and provides the specified outcome for this assignment. However, I believe there is probably a more efficient way of writing it – instead of having 3 separate functions I would think there could be 1 function that would provide the necessary output (colour and shape) each time an image is clicked on. Just curious to know and I also want to learn what isn’t in our text book. Thank you in advance.

[B][COLOR=”#0000CD”]This part of the assignment:[/COLOR][/B]

[COLOR=”#0000CD”]Task4: Write your Own Code
Write a program with the following specs
• The HTML document contains 3 images: a black circle, a red triangle, and a blue square.
• Preload these images into a JS array, before the HTML page is loaded.
• Attach an onclink event to each image such that the color and the shape of the image will be displayed once you click on the image.[/COLOR]

[B]My code:[/B]

[CODE]<!DOCTYPE html>

<html>

<head>
<title>JavaScript A6 Task 4 Stringle</title>
<meta charset=”UTF-8″/>

<style type=”text/css”>
img
{
cursor:pointer;
}
</style>

<script type=”text/javascript”>

var myImages = Array(“circleblack.jpg”,”trianglered.png”,”squareblue.png”);

function myCircleFunction()
{
document.getElementById(“circle”).innerHTML = “This is a black circle”;
}

function myTriangleFunction()
{
document.getElementById(“triangle”).innerHTML = “This is a red triangle”;
}

function mySquareFunction()
{
document.getElementById(“square”).innerHTML = “This is a blue square”;
}

</script>

</head>

<body>

<p><em><strong>Click the shape to trigger a function.</strong></em></p>

<img name=”img0″ src=”circleblack.jpg” onclick=”myCircleFunction()” alt=”Black circle” />

<img name=”img1″ src=”trianglered.png” onclick=”myTriangleFunction()” alt=”Red triangle” />

<img name=”img2″ src=”squareblue.png” onclick=”mySquareFunction()” alt=”Blue square” />

<p id=”circle”></p>
<p id=”triangle”></p>
<p id=”square”></p>

</body>
</html>[/CODE]

to post a comment
JavaScript

26 Comments(s)

Copy linkTweet thisAlerts:
@mikewgdNov 19.2014 — Well this is an assignment. So I dont want to just give you the answers.

  • 1. I would used event handling on elements, onclick bad practice

  • 2. Use if/else or switch statements in one function.


  • Does that make sense?
    Copy linkTweet thisAlerts:
    @BlondieCauthorNov 19.2014 — Hi and thank you and yes it makes sense. It says to attach an onclick event so I have to use that. We've gone through the if/else so I'll try it with that.
    Copy linkTweet thisAlerts:
    @JeffTheCobbNov 19.2014 — If you used associative arrays you'd only need one function and call it with the name of the object.
    Copy linkTweet thisAlerts:
    @JeffTheCobbNov 19.2014 — Also, from what I see you're not pre-loading the images per the assignment. But I'm guessing that's what the "myImages" array is for.
    Copy linkTweet thisAlerts:
    @BlondieCauthorNov 19.2014 — We haven't covered associative arrays - I'll read about this one on my own as it doesn't show anywhere in our material. Thanks for the suggestion.
    Copy linkTweet thisAlerts:
    @BlondieCauthorNov 19.2014 — Also, from what I see you're not pre-loading the images per the assignment. But I'm guessing that's what the "myImages" array is for.[/QUOTE]

    Yes that's why I was using the array - it's following the sample in our text. Normally wouldn't you use an Onload?
    Copy linkTweet thisAlerts:
    @JeffTheCobbNov 19.2014 — Yeah, you would. But since you're using three images you'd need to check that all three are loaded before dropping them onto the page.
    Copy linkTweet thisAlerts:
    @iBeZiNov 19.2014 — I don't know if this is going out of the scope of your assignment, but generally when you preload an image using JavaScript you need to create an image element and set it's src to the image, which will send a request to start downloading the image, if I were to do your assignment I would do it like this, but again, I don't know if this is going out of the level of your assignment.

    Basically;

    [LIST=1]
  • [*]make an image element for each image in the array, set it's source attribute to the image url.

  • [*]loop through all the images on the page adding an event listener

  • [*]get the clicked image from the event passed by the event listener

  • [*]use the images alt tag to display the shape and color

  • [/LIST]



    [code=html]
    <!doctype html>
    <html lang="en">
    <head>
    <meta charset="UTF-8">
    <title>Document</title>
    </head>
    <body>
    <script>
    var images = ["circleblack.jpg","trianglered.png","squareblue.png"];

    //loop though images array, create an image element for each one and set the src to the image url
    for(var i = 0, l = images.length; i < l; i++) {
    var img = document.createElement('img');
    img.src = images[i];
    }

    </script>
    <img name="img0" src="circleblack.jpg" alt="Black circle" />

    <img name="img1" src="trianglered.png" alt="Red triangle" />

    <img name="img2" src="squareblue.png" alt="Blue square" />
    <div id="info"></div>
    <script>
    function showInfo(e) {

    //get the clicked image from the event object and the shape and color from the alt attribute
    var image = e.target,
    desc = image.getAttribute('alt');

    //display color and shape in info div
    info.innerHTML = 'This is a '+desc;
    }

    //get all the image elements on the page + the info div
    var images = document.getElementsByTagName('img'),
    info = document.getElementById('info');

    //loop through all the image elements on the page adding a click event which calls the showInfo function
    for(var i = 0, l = images.length; i < l; i++) {
    images[i].addEventListener('click',showInfo);
    }
    </script>
    </body>
    </html>
    [/code]


    Also, just a heads up on associative arrays, in JavaScript there's no such thing, you're probably thinking of objects which are key - value pair stores, for example this is an object containing 2 arrays.

    [code=php]
    var myObj = {
    array1: [
    1,
    2,
    3
    ],
    array2: [
    4,
    5,
    6
    ]
    };
    [/code]
    Copy linkTweet thisAlerts:
    @BlondieCauthorNov 19.2014 — Ok - if I break down your code - some we've covered and some we haven't. And looking at your array sample code - we've covered that. I think what threw me there was the term associative array - in the course material multiple array was used. I'll eventually pick up on the lingo and become more familiar with it the more I read and practice and with all the tips and suggestions people have given me in the forum.

    I've realized with this assignment that I'm getting stuck on how to pull together all the pieces I've learned separately into one block of code. As per the earlier suggestion of using if/else in my function instead of using 3 different functions like in my original code, I'm working on that to see if I can make it work since although my code returns what it's supposed to, it isn't written as efficiently as it could be.

    Thanks for the info about the preload as we have not covered that and the example the instructor wants us to refer to doesn't have anything like what you coded in it. The sample in the book doesn't even have the alt description for the images which I think it should to show proper HTML - but that's just me.

    In your code function [COLOR="#B22222"]showInfo(e)[/COLOR], where does the [COLOR="#B22222"]e[/COLOR] come from or is that a standard character used?
    Copy linkTweet thisAlerts:
    @iBeZiNov 19.2014 — Multiple arrays (or multi-dimensional arrays) are pretty much just arrays that contain arrays, it's similar to what I coded above but not quite the same, this would be an example of a multi-dimensional array.

    [code=php]
    var multiArray = [
    [
    1,
    2,
    3,
    ],
    [
    4,
    5,
    6
    ]
    ];
    [/code]


    Note that it's entirely made using arrays [square brackets]. What I did above was create an object {curly braces} and set 2 properties of the object (array1 and array2) which were arrays.

    The 'e' in showInfo could be anything, it will contain information about the event passed by the event listener. Think of showInfo as a regular function which you can pass an argument to, the event listener uses that function and passes in the event object as the argument which we capture in the variable 'e', you could rename it to 'event' if that makes things easier to understand.
    Copy linkTweet thisAlerts:
    @BlondieCauthorNov 19.2014 — I learn more here than I do in my course. Someday I hope I know enough to be able to help people out. I think forums are great and that people will help others out is really great. I know that when I'm stuck I can always ask here. I print of the dialogue and use it as reference material because what's mentioned here isn't in my text.
    Copy linkTweet thisAlerts:
    @iBeZiNov 19.2014 — I find that some courses just provide the bare minimum and then expect the student to go off and figure out the rest for themselves. It can be a bit frustrating but I personally found that I learnt more about JavaScript by just playing around with it, learning things by myself using online resources. At least the course is giving you challenges to complete, by doing those your searching around for the best way to accomplish the task which has led you here.
    Copy linkTweet thisAlerts:
    @JeffTheCobbNov 19.2014 — Blondie... Are you doing this course as a requisite for something else like web design?
    Copy linkTweet thisAlerts:
    @BlondieCauthorNov 20.2014 — I find that some courses just provide the bare minimum and then expect the student to go off and figure out the rest for themselves. It can be a bit frustrating but I personally found that I learnt more about JavaScript by just playing around with it, learning things by myself using online resources. At least the course is giving you challenges to complete, by doing those your searching around for the best way to accomplish the task which has led you here.[/QUOTE]

    After I finish the remaining four courses I'll come back to JavaScript - there is so much more I want to learn and develop a better understanding. I'm struggling with understanding it and want to overcome that - it will take time and trial and error - kind of how I learn.
    Copy linkTweet thisAlerts:
    @BlondieCauthorNov 20.2014 — Blondie... Are you doing this course as a requisite for something else like web design?[/QUOTE]

    I am doing a Web Design & Creation certificate and this is my fourth course - JavaScript is one of the optional ones I chose - for me I really felt it was important to take this one. I'll need a lot more than just this JavaScript course - I'll tackle it again after completing the remaining four courses.
    Copy linkTweet thisAlerts:
    @BlondieCauthorNov 20.2014 — Blondie... Are you doing this course as a requisite for something else like web design?[/QUOTE]

    I am taking this as part of a Web Design & Creation certificate - this is my fourth course and one of the optional ones but for me it was an important one to take. I'll tackle it again after I complete the remaining four courses.
    Copy linkTweet thisAlerts:
    @rootNov 20.2014 — IMHO some methods are overly complicated, nothing wrong at all in using the onclick tag in HTML, that is what it is for.

    Whilst everyone is on the I would do it this way band wagon, this is what I would do [code=html]<!DOCTYPE html>
    <html>
    <head>
    <title>JavaScript A6 Task 4 Stringle</title>
    <meta charset="UTF-8"/>
    <style type="text/css">
    img { cursor:pointer; }
    </style>

    <script>
    var d = document,
    imgLoad = new Array();

    function myClickFunction(o){
    d.getElementById("outputMessage").innerHTML = "This is a " + o.title;
    }

    window.onload = function(){
    if( l=d.images.length ){
    for(i=0; i<l; i++){
    imgLoad[i] = new Image();
    imgLoad[i].src = d.images[i].src;
    console.log(">>> "+imgLoad[i].src); // output to the console log the SRC url
    }
    }
    }
    </script>
    </head>
    <body>
    <p><em><strong>Click the shape to trigger a function.</strong></em></p>
    <img name="circle" src="blacktriangle.png" onclick="myClickFunction(this);" title="Black triangle" />
    <img name="triangle" src="redcircle.png" onclick="myClickFunction(this);" title="Red circle" />
    <img name="square" src="bluesquare.png" onclick="myClickFunction(this);" title="Blue square" />
    <p id="outputMessage"></p>
    </body>
    </html>[/code]


    If I wanted to be more resourceful then I would consider having just the <img> tags and during the image preload process, I then would apply onclick, title and other attributes like height, width, etc if needed.

    When using <img tags, the spec calls for a minimum of three tag elements, the name, src and title tags to be completed, using the title tag causes the content of the title to be viewed when hovered over with the mouse pointer.

    the myClickFunction(this) function call passes a reference to the object that calls the function, this in turn can be used to find and display the appropriate text from the image "Title" text and to a single output display element.

    To view the console log in your browser, hit the F12 button, if it doesn't open up a console then you need to delve in to the browser menu to find it.

    You should have plenty to go on and give you ideas on how to go about completing your assignment.
    Copy linkTweet thisAlerts:
    @iBeZiNov 20.2014 — Sorry but quite a few things you just said are plain wrong, for starters the 'name' and 'title' attributes are not required on an img element, the 'title' attribute is a generic attribute that can be used on pretty much any element and the 'name' attribute does nothing. The only attribute that is required on an img element is src, it's generally good practise to include alt as well.

    Nothing wrong with the onclick attribute? Yes there is, it's awful practise, your mixing JavaScript with your html in a terrible way, read up on Unobtrusive JavaScript.
    Copy linkTweet thisAlerts:
    @rootNov 20.2014 — The "name" tag does nothing eh? Name is needed to access that element via its DOM by name otherwise you would need to access it via the Tag element and iterate any arrays resulting from that.

    The <img> only attribute only acts as a place holder. Title is needed to assist people with screen readers, its an accessibility requirement, especially in the EU. ALT tags are used for the display of a name of an image before the images have loaded, Lynx type browsers will read these tags, especially the ALT tag but being a texted based only output, you will find that if you only use <img> as a place holder in the hope of attaching something to it at a later date via JavaScript, Ooop, sorry, you won't be able to do that in a Lynx browser, they don't support JavaScript.

    No, nothing wrong with using onclick elements in HTML as "THAT" is what they were implemented for in HTML to allow the user to directly apply a function to that item.

    I don't need to read up about JavaScript TYVM, why do I need to comply with some stuffed shirts demands to code in a certain way when their is absolutely nothing wrong with the way in which the code is written.

    I figure that you are one of the newer coders on the block, been taught that certain things are bad practice, you have to ask why before accepting them and "because" is not an answer.

    Yes I know about various methods of coding, seems that people have forgotten that JavaScript is not a compiled language and applying standards used in other languages like C, C++, Cobol, Pascal and so on.
    Copy linkTweet thisAlerts:
    @iBeZiNov 20.2014 — You're wrong, the name tag on images has been [B]deprecated since html4[/B], if you want to give an image a unique name you should use ID.

    The [B]ALT attribute[/B] is used for screen readers, not title.

    I never said just use <img> by itself to later add attributes with JavaScript, for starters you need to provide a src, that is required.

    If you refuse to accept that separation of JavaScript and HTML is a very good thing then you sound like a terrible web developer, just because JS isn't a compiled language doesn't excuse you for writing sloppy code. There are a lot of things left over from when web development was more archaic, and the inline JavaScript attributes are one of them, they shouldn't be used any more, and yes, I know why, I don't just accept things as being bad practise without explanation. [B]Unobtrusive JavaScript[/B]
    Copy linkTweet thisAlerts:
    @JeffTheCobbNov 20.2014 — I am doing a Web Design & Creation certificate and this is my fourth course - JavaScript is one of the optional ones I chose - for me I really felt it was important to take this one. I'll need a lot more than just this JavaScript course - I'll tackle it again after completing the remaining four courses.[/QUOTE]

    Well, once you start to get it down pretty good you'll start having more fun with it. Because then you can just "create" and not have to think or do as much searching.

    Just keep writing little things with it and you'll pick it up. That's one of the reasons I like looking around these forums. I find little projects that people have trouble with and see if I can write them myself. It's also a way for me to take a break from the bigger projects. I'm working on a fairly intensive database web-app that uses HTML, Javascript, PHP and MySQL. Sometimes I get a little burnt and like to mess around with other things for a bit ?

    Anyway, best of luck!

    Jeff
    Copy linkTweet thisAlerts:
    @rootNov 22.2014 — Title is used for the roll over, just because something is depreciated, doesn't mean you can't use it. Alt only provides a text replacement while the image loads.

    the name tag is an array meaning you can have a group of objects and not have to track a group of unique ID's
    Copy linkTweet thisAlerts:
    @iBeZiNov 22.2014 — OK fine, I agree you [i]could[/i] use name, but there are better and more reliable ways of accessing elements in the DOM. Using the name attribute can cause some problems, for example take this:

    [code=html]
    <a href='#' name='myName'>link</a>
    <img src='image1.png' name='myName'>
    <div name='myName'></div>
    <div id='myName'></div>

    <script>
    console.log(document.getElementsByName('myName'));
    </script>
    [/code]


    I would expect that to bring back all the elements using the name attribute, BUT in IE9 it ignores the DIV with the name attribute and instead brings back the DIV with the ID attribute. In that case you'd probably be better off doing "document.querySelectorAll('[name="myName"]')".

    The only time I'd use the name attribute is on form elements.

    And yes, the title attribute is used for roll over text, but that's all it's used for, the ALT attribute is used for accessibility, that 'text replacement' is what screen readers see.
    Copy linkTweet thisAlerts:
    @BlondieCauthorNov 23.2014 — Well, once you start to get it down pretty good you'll start having more fun with it. Because then you can just "create" and not have to think or do as much searching.

    Just keep writing little things with it and you'll pick it up. That's one of the reasons I like looking around these forums. I find little projects that people have trouble with and see if I can write them myself. It's also a way for me to take a break from the bigger projects. I'm working on a fairly intensive database web-app that uses HTML, Javascript, PHP and MySQL. Sometimes I get a little burnt and like to mess around with other things for a bit ?

    Anyway, best of luck!

    Jeff[/QUOTE]


    Thanks Jeff. Thank goodness for those who have the experience and knowledge to come on here and help. One of the other courses includes PHP and MySQL so I'll be taking that one eventually. I do find the more I read and re-read the code and add this or take away that and see the outcome, the more I understand it.
    Copy linkTweet thisAlerts:
    @rootNov 23.2014 — OK fine, I agree you [i]could[/i] use name, but there are better and more reliable ways of accessing elements in the DOM. Using the name attribute can cause some problems, for example take this:

    [code=html]
    <a href='#' name='myName'>link</a>
    <img src='image1.png' name='myName'>
    <div name='myName'></div>
    <div id='myName'></div>

    <script>
    console.log(document.getElementsByName('myName'));
    </script>
    [/code]


    I would expect that to bring back all the elements using the name attribute, BUT in IE9 it ignores the DIV with the name attribute and instead brings back the DIV with the ID attribute. In that case you'd probably be better off doing "document.querySelectorAll('[name="myName"]')".

    The only time I'd use the name attribute is on form elements.

    And yes, the title attribute is used for roll over text, but that's all it's used for, the ALT attribute is used for accessibility, that 'text replacement' is what screen readers see.[/QUOTE]


    name is not a valid tag for a DIV but...

    document.getElementsByName('myName') returns an array of 3 elements, an anchor, image, div.


    console.log(document.getElementsByName('myName')[0]);

    console.log(document.getElementsByName('myName')[1]);

    console.log(document.getElementsByName('myName')[2]);

    returns:

    <a href=&#8203;"#" name=&#8203;"myName">&#8203;link&#8203;</a>&#8203;

    <img src=&#8203;"image1.png" name=&#8203;"myName">&#8203;

    <div name=&#8203;"myName">&#8203;</div>&#8203;


    However, you are missing the point of having the name tag, using the name tag to group elements is more a case of being able to group "like" elements such as form elements like a radio set or check box group, you wouldn't group unlike items together like your demo.

    Having to "query" the DOM Tree for something that is going to be use repetitively is better to access directly, when you add things dynamically to the DOM Tree, it is beneficial to add id's and name tags.
    Copy linkTweet thisAlerts:
    @iBeZiNov 23.2014 — The point of my demo was to show browser inconsistency with document.getElementsByName, in IE9 it won't select certain elements by name but it will select elements using that name as an ID, try running that code in IE9 and see which elements you get back.
    ×

    Success!

    Help @BlondieC spread the word by sharing this article on Twitter...

    Tweet This
    Sign in
    Forgot password?
    Sign in with TwitchSign in with GithubCreate Account
    about: ({
    version: 0.1.9 BETA 5.18,
    whats_new: community page,
    up_next: more Davinci•003 tasks,
    coming_soon: events calendar,
    social: @webDeveloperHQ
    });

    legal: ({
    terms: of use,
    privacy: policy
    });
    changelog: (
    version: 0.1.9,
    notes: added community page

    version: 0.1.8,
    notes: added Davinci•003

    version: 0.1.7,
    notes: upvote answers to bounties

    version: 0.1.6,
    notes: article editor refresh
    )...
    recent_tips: (
    tipper: @AriseFacilitySolutions09,
    tipped: article
    amount: 1000 SATS,

    tipper: @Yussuf4331,
    tipped: article
    amount: 1000 SATS,

    tipper: @darkwebsites540,
    tipped: article
    amount: 10 SATS,
    )...