Page 1 of 2 12 LastLast
Results 1 to 10 of 19

Thread: Making code more efficient?

  1. #1
    Join Date
    Sep 2008
    Location
    Bristol - UK
    Posts
    842
    Thanks
    32
    Thanked 132 Times in 131 Posts

    Default Making code more efficient?

    Hi there, I recently uploaded an image map with centres on it that the user can click and text appears next to the map describing the centre. At the moment it all works fine, but it doesn't seem to be very efficient to me as there is lots of code which I think could be condensed down, although I'm not sure how.

    You can see what I mean here:

    http://www.bombthehills.com/ridingspots-uk.html

    The code I am referring to is this:

    Code:
    <area shape="circle" coords="283,656,7" href="javascript:ReverseDisplay('ivyleaf'); javascript:HideContent('swmbc'); javascript:HideContent('quantock'); javascript:HideContent('haredown'); javascript:HideContent('knockhill'); javascript:HideContent('airfusion'); javascript:HideContent('xbp'); javascript:HideContent('oakwood'); javascript:HideContent('waterside'); javascript:HideContent('newmilns'); javascript:HideContent('halesb'); javascript:HideContent('edge'); javascript:HideContent('courtfarm'); javascript:HideContent('redhill'); javascript:HideContent('outtograss'); javascript:HideContent('bugsboarding');" alt="ivyleaf">
      <area shape="circle" coords="299,647,7" href="javascript:ReverseDisplay('swmbc'); javascript:HideContent('ivyleaf'); javascript:HideContent('quantock'); javascript:HideContent('haredown'); javascript:HideContent('knockhill'); javascript:HideContent('airfusion'); javascript:HideContent('xbp'); javascript:HideContent('oakwood'); javascript:HideContent('waterside'); javascript:HideContent('newmilns'); javascript:HideContent('halesb'); javascript:HideContent('edge'); javascript:HideContent('courtfarm'); javascript:HideContent('redhill'); javascript:HideContent('outtograss'); javascript:HideContent('bugsboarding');" alt="MBC">
      <area shape="circle" coords="351,636,8" href="javascript:ReverseDisplay('quantock'); javascript:HideContent('ivyleaf'); javascript:HideContent('swmbc'); javascript:HideContent('haredown'); javascript:HideContent('knockhill'); javascript:HideContent('airfusion'); javascript:HideContent('xbp'); javascript:HideContent('oakwood'); javascript:HideContent('waterside'); javascript:HideContent('newmilns'); javascript:HideContent('halesb'); javascript:HideContent('edge'); javascript:HideContent('courtfarm'); javascript:HideContent('redhill'); javascript:HideContent('outtograss'); javascript:HideContent('bugsboarding');" alt="quantock">
      <area shape="circle" coords="459,643,8" href="javascript:ReverseDisplay('haredown'); javascript:HideContent('ivyleaf'); javascript:HideContent('swmbc'); javascript:HideContent('quantock'); javascript:HideContent('knockhill'); javascript:HideContent('airfusion'); javascript:HideContent('xbp'); javascript:HideContent('oakwood'); javascript:HideContent('waterside'); javascript:HideContent('newmilns'); javascript:HideContent('halesb'); javascript:HideContent('edge'); javascript:HideContent('courtfarm'); javascript:HideContent('redhill'); javascript:HideContent('outtograss'); javascript:HideContent('bugsboarding');" alt="haredown">
      <area shape="circle" coords="501,641,8" href="javascript:ReverseDisplay('knockhill'); javascript:HideContent('ivyleaf'); javascript:HideContent('swmbc'); javascript:HideContent('quantock'); javascript:HideContent('haredown'); javascript:HideContent('airfusion'); javascript:HideContent('xbp'); javascript:HideContent('oakwood'); javascript:HideContent('waterside'); javascript:HideContent('newmilns'); javascript:HideContent('halesb'); javascript:HideContent('edge'); javascript:HideContent('courtfarm'); javascript:HideContent('redhill'); javascript:HideContent('outtograss'); javascript:HideContent('bugsboarding');" alt="knockhill">
      <area shape="circle" coords="285,253,9" href="javascript:ReverseDisplay('airfusion'); javascript:HideContent('ivyleaf'); javascript:HideContent('swmbc'); javascript:HideContent('quantock'); javascript:HideContent('haredown'); javascript:HideContent('knockhill'); javascript:HideContent('xbp'); javascript:HideContent('oakwood'); javascript:HideContent('waterside'); javascript:HideContent('newmilns'); javascript:HideContent('halesb'); javascript:HideContent('edge'); javascript:HideContent('courtfarm'); javascript:HideContent('redhill'); javascript:HideContent('outtograss'); javascript:HideContent('bugsboarding');" alt="airfusion">
      <area shape="circle" coords="488,616,8" href="javascript:ReverseDisplay('xbp'); javascript:HideContent('ivyleaf'); javascript:HideContent('swmbc'); javascript:HideContent('quantock'); javascript:HideContent('haredown'); javascript:HideContent('knockhill'); javascript:HideContent('airfusion'); javascript:HideContent('oakwood'); javascript:HideContent('waterside'); javascript:HideContent('newmilns'); javascript:HideContent('halesb'); javascript:HideContent('edge'); javascript:HideContent('courtfarm'); javascript:HideContent('redhill'); javascript:HideContent('outtograss'); javascript:HideContent('bugsboarding');" alt="xbp">
      <area shape="circle" coords="464,606,8" href="javascript:ReverseDisplay('oakwood'); javascript:HideContent('ivyleaf'); javascript:HideContent('swmbc'); javascript:HideContent('quantock'); javascript:HideContent('haredown'); javascript:HideContent('knockhill'); javascript:HideContent('airfusion'); javascript:HideContent('xbp'); javascript:HideContent('waterside'); javascript:HideContent('newmilns'); javascript:HideContent('halesb'); javascript:HideContent('edge'); javascript:HideContent('courtfarm'); javascript:HideContent('redhill'); javascript:HideContent('outtograss'); javascript:HideContent('bugsboarding');" alt="oakwood">
      <area shape="circle" coords="447,595,8" href="javascript:ReverseDisplay('waterside'); javascript:HideContent('ivyleaf'); javascript:HideContent('swmbc'); javascript:HideContent('quantock'); javascript:HideContent('haredown'); javascript:HideContent('knockhill'); javascript:HideContent('airfusion'); javascript:HideContent('xbp'); javascript:HideContent('oakwood'); javascript:HideContent('newmilns'); javascript:HideContent('halesb'); javascript:HideContent('edge'); javascript:HideContent('courtfarm'); javascript:HideContent('redhill'); javascript:HideContent('outtograss'); javascript:HideContent('bugsboarding');" alt="waterside">
      <area shape="circle" coords="291,291,8" href="javascript:ReverseDisplay('newmilns'); javascript:HideContent('ivyleaf'); javascript:HideContent('swmbc'); javascript:HideContent('quantock'); javascript:HideContent('haredown'); javascript:HideContent('knockhill'); javascript:HideContent('airfusion'); javascript:HideContent('xbp'); javascript:HideContent('oakwood'); javascript:HideContent('waterside'); javascript:HideContent('halesb'); javascript:HideContent('edge'); javascript:HideContent('courtfarm'); javascript:HideContent('redhill'); javascript:HideContent('outtograss'); javascript:HideContent('bugsboarding');" alt="newmilns">
      <area shape="circle" coords="364,476,8" href="javascript:ReverseDisplay('halesb'); javascript:HideContent('ivyleaf'); javascript:HideContent('swmbc'); javascript:HideContent('quantock'); javascript:HideContent('haredown'); javascript:HideContent('knockhill'); javascript:HideContent('airfusion'); javascript:HideContent('xbp'); javascript:HideContent('oakwood'); javascript:HideContent('waterside'); javascript:HideContent('newmilns'); javascript:HideContent('edge'); javascript:HideContent('courtfarm'); javascript:HideContent('redhill'); javascript:HideContent('outtograss'); javascript:HideContent('bugsboarding');" alt="halesb">
      <area shape="circle" coords="361,514,10" href="javascript:ReverseDisplay('edge'); javascript:HideContent('ivyleaf'); javascript:HideContent('swmbc'); javascript:HideContent('quantock'); javascript:HideContent('haredown'); javascript:HideContent('knockhill'); javascript:HideContent('airfusion'); javascript:HideContent('xbp'); javascript:HideContent('oakwood'); javascript:HideContent('waterside'); javascript:HideContent('newmilns'); javascript:HideContent('halesb'); javascript:HideContent('courtfarm'); javascript:HideContent('redhill'); javascript:HideContent('outtograss'); javascript:HideContent('bugsboarding');" alt="edge">
      <area shape="circle" coords="359,545,8" href="javascript:ReverseDisplay('courtfarm'); javascript:HideContent('ivyleaf'); javascript:HideContent('swmbc'); javascript:HideContent('quantock'); javascript:HideContent('haredown'); javascript:HideContent('knockhill'); javascript:HideContent('airfusion'); javascript:HideContent('xbp'); javascript:HideContent('oakwood'); javascript:HideContent('waterside'); javascript:HideContent('newmilns'); javascript:HideContent('halesb'); javascript:HideContent('edge'); javascript:HideContent('redhill'); javascript:HideContent('outtograss'); javascript:HideContent('bugsboarding');" alt="courtfarm">
    etc... (Too long for post)

    What I want to happen is that when you click on a centre, it hides any centre that is currently showing and replaces it with the one the user clicks, so instead of writing out HideContent for every single possibility is there not a way where i can use (*) or something to hide all? This would make it much more readable and save on time and file size.

    Hope you can help with this,

    Thanks,

    Jack.

  2. #2
    Join Date
    Feb 2008
    Location
    Cebu City Philippines
    Posts
    1,160
    Thanks
    17
    Thanked 277 Times in 275 Posts

    Default

    First, this part:
    Code:
    function ReverseDisplay(d) {
    if(document.getElementById(d).style.display == "none") { document.getElementById(d).style.display = "block"; }
    else { document.getElementById(d).style.display = "none"; }
    }
    ...could be trimmed to:
    Code:
    function ReverseDisplay(d) {
    var el=document.getElementById(d);
    	el.style.display=el.style.display!='none'?'none':'';
    }
    You could modify your HideContent function to:
    Code:
    var hideEl=['ivyleaf','swmbc','quantock','haredown','knockhill',
    'airfusion','xbp','oakwood','waterside','newmilns','halesb','edge','courtfarm',
    'redhill','outtograss','bugsboarding'
    ]; // Place in this array the ID of the elements involved
    
    function HideContent() {
    for(var i=0;i<hideEl.length;i++)
    	document.getElementById(hideEl[i]).style.display = "none";
    }
    That way, you can change (for example this line in) your markup:
    Code:
    <area shape="circle" coords="283,656,7" href="javascript:ReverseDisplay('ivyleaf'); javascript:HideContent('swmbc'); javascript:HideContent('quantock'); javascript:HideContent('haredown'); javascript:HideContent('knockhill'); javascript:HideContent('airfusion'); javascript:HideContent('xbp'); javascript:HideContent('oakwood'); javascript:HideContent('waterside'); javascript:HideContent('newmilns'); javascript:HideContent('halesb'); javascript:HideContent('edge'); javascript:HideContent('courtfarm'); javascript:HideContent('redhill'); javascript:HideContent('outtograss'); javascript:HideContent('bugsboarding');" alt="ivyleaf">
    ...to:
    Code:
    <area shape="circle" coords="283,656,7" href="#" onclick="HideContent();ReverseDisplay('ivyleaf');return false;" alt="ivyleaf">
    Avoid using javascript pseudo url as it will result into your page's invalidity.

    Hope that makes sense.
    Learn how to code at 02geek

    The more you learn, the more you'll realize there's much more to learn
    Ray.ph!

  3. The Following User Says Thank You to rangana For This Useful Post:

    Schmoopy (11-04-2008)

  4. #3
    Join Date
    Sep 2008
    Location
    Bristol - UK
    Posts
    842
    Thanks
    32
    Thanked 132 Times in 131 Posts

    Default

    Just implemented the code, thanks a lot for that, it's going to save me a lot of time when adding new centres

    Now just to sort out the weird blank space at the bottom that occurs when you click on a centre =/

    I guess that's to do with the CSS though,

    Still, thanks for this great code

    Jack.

  5. #4
    Join Date
    Sep 2008
    Location
    Bristol - UK
    Posts
    842
    Thanks
    32
    Thanked 132 Times in 131 Posts

    Default

    Only thing that's not quite working yet is the show content part:

    function ReverseDisplay(d) {
    var centres=document.getElementById(d);
    centres.style.display=centres.style.display!='none'?'none':'';
    }

    It does work when I keep it like it was before, but the centres can't be hidden again, as in, once you've clicked on a centre you can't hide it again, it will stay there until a new centre is clicked, which isn't too much of a problem but it's still just a little thing.

    There's something wrong with the last part of the code I think :

    !='none'?'none':'';

    But I'm just that bad at javascript I can't fix it myself *ashamed*

    Never seen a ? used in javascript so it's sort of new for me,

    Hope this is just a quick typo,

    Jack

  6. #5
    Join Date
    Feb 2008
    Location
    Cebu City Philippines
    Posts
    1,160
    Thanks
    17
    Thanked 277 Times in 275 Posts

    Default

    Update your page.

    Also, ensure that HideContent(); comes early before ReverseDisplay('element_id');.

    As in:
    Code:
    <area shape="circle" coords="283,656,7" href="#" onclick="HideContent();ReverseDisplay('ivyleaf');return false;" alt="ivyleaf">
    Learn how to code at 02geek

    The more you learn, the more you'll realize there's much more to learn
    Ray.ph!

  7. #6
    Join Date
    Sep 2008
    Location
    Bristol - UK
    Posts
    842
    Thanks
    32
    Thanked 132 Times in 131 Posts

    Default

    I haven't updated the page yet because it doesn't fully work, nothing's happening if I use the code you submitted for the "ReverseDisplay" function =/

    JS :

    Code:
    function HideContent() {
    for(var i=0;i<hideCentres.length;i++)
    	document.getElementById(hideCentres[i]).style.display = "none";
    }
    
    function ShowContent(d) {
    document.getElementById(d).style.display = "block";
    }
    
    function ReverseDisplay(d) {
    var centres=document.getElementById(d);
    	centres.style.display=centres.style.display!='none'?'none':'';
    }
    
    var hideCentres=['ivyleaf','swmbc','quantock','haredown','knockhill',
    'airfusion','xbp','oakwood','waterside','newmilns','halesb','edge','courtfarm',
    'redhill','outtograss','bugsboarding'];
    HTML:

    Code:
    <area shape="circle" coords="283,656,7" href="#" onclick="HideContent(); ReverseDisplay('ivyleaf'); return false;" alt="ivyleaf">
      <area shape="circle" coords="299,647,7" href="#" onclick="HideContent(); ReverseDisplay('swmbc'); return false;" alt="SWMBC">
      <area shape="circle" coords="351,636,8" href="#" onclick="HideContent(); ReverseDisplay('quantock'); return false;" alt="quantock">
      <area shape="circle" coords="459,643,8" href="#" onclick="HideContent(); ReverseDisplay('haredown'); return false;" alt="haredown">
      <area shape="circle" coords="501,641,8" href="#" onclick="HideContent(); ReverseDisplay('knockhill'); return false;" alt="knockhill">
      <area shape="circle" coords="285,253,9" href="#" onclick="HideContent(); ReverseDisplay('airfusion'); return false;" alt="airfusion">
      <area shape="circle" coords="488,616,8" href="#" onclick="HideContent(); ReverseDisplay('xbp'); return false;" alt="xbp">
      <area shape="circle" coords="464,606,8" href="#" onclick="HideContent(); ReverseDisplay('oakwood'); return false;" alt="oakwood">
      <area shape="circle" coords="447,595,8" href="#" onclick="HideContent(); ReverseDisplay('waterside'); return false;" alt="waterside">
      <area shape="circle" coords="291,291,8" href="#" onclick="HideContent(); ReverseDisplay('newmilns'); return false;" alt="newmilns">
      <area shape="circle" coords="364,476,8" href="#" onclick="HideContent(); ReverseDisplay('halesb'); return false;" alt="halesb">
      <area shape="circle" coords="361,514,10" href="#" onclick="HideContent(); ReverseDisplay('edge'); return false;" alt="edge">
      <area shape="circle" coords="359,545,8" href="#" onclick="HideContent(); ReverseDisplay('courtfarm'); return false;" alt="courtfarm">
      <area shape="circle" coords="368,572,8" href="#" onclick="HideContent(); ReverseDisplay('redhill'); return false;" alt="redhill">
      <area shape="circle" coords="378,552,7" href="#" onclick="HideContent(); ReverseDisplay('outtograss'); return false;" alt="outtograss">
      <area shape="circle" coords="399,560,8" href="#" onclick="HideContent(); ReverseDisplay('bugsboarding'); return false;" alt="bugsboarding">
    Nothing happens when you click on the centres, only if I revert back to the old ReverseDisplay function does anything happen.

    Are you sure it's right?

    Jack.

  8. #7
    Join Date
    Sep 2008
    Location
    Bristol - UK
    Posts
    842
    Thanks
    32
    Thanked 132 Times in 131 Posts

    Default

    Oh wait, I added "block" after the 'none'?'none':'block';

    And that's worked, it's still not hiding it though if the centre is already visible, only changing when a new centre is clicked, updated version at:

    http://www.bombthehills.com/ridingspots-uk.html

    Jack.

  9. #8
    Join Date
    Feb 2008
    Location
    Cebu City Philippines
    Posts
    1,160
    Thanks
    17
    Thanked 277 Times in 275 Posts

    Default

    It seemed that you haven't updated your ukmap.js.
    Learn how to code at 02geek

    The more you learn, the more you'll realize there's much more to learn
    Ray.ph!

  10. #9
    Join Date
    Sep 2008
    Location
    Bristol - UK
    Posts
    842
    Thanks
    32
    Thanked 132 Times in 131 Posts

    Default

    Ok updated it again, but since it's still not working, and i'm using the exact code you said it's here:

    http://www.bombthehills.com/ridingspots-ukbeta.html

    and the JS file at:

    http://www.bombthehills.com/ukmap2.js

    But still nothing happening

  11. #10
    Join Date
    Feb 2008
    Location
    Cebu City Philippines
    Posts
    1,160
    Thanks
    17
    Thanked 277 Times in 275 Posts

    Default

    This file: http://www.bombthehills.com/ridingspots-ukbeta.html

    ...is still pointing on ukmap.js
    Learn how to code at 02geek

    The more you learn, the more you'll realize there's much more to learn
    Ray.ph!

Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •