0

I'm writing a code to gather the last certain number of products viewed on an e-commerce site. I'm able to write it just fine, but it seems like there should be an easier way to do it with a for or a while loop. If I ever decide I need more products to be stored in the future, then I have to write more code, and it get's n+1 longer with each item. Here's the code right now (up to 3 items)

    var url = $.cookie('mycookie');
    var myArray = url.split('»');
    var myArrayLen=myArray.length;

    if (myArrayLen == 1){
          if (url==currentUrl)
            {
                var recentProducts='';
                $.cookie('mycookie', '»'+currentUrl, { path: '/', expires: cookieExp });
            }
            else
            {
                var recentProducts='<div class="mycookie"><h4>Recently Viewed Products</h4>';
                recentProducts += '<a href="' + myArray[1] + '">Link1</a>';
                $.cookie('mycookie', url+'»'+currentUrl, { path: '/', expires: cookieExp });
            }
        }


        else if (myArrayLen == 2){
            if (currentUrl==myArray[1])
            {
                var recentProducts='<div class="mycookie"><h4>Recently Viewed Products</h4>';
                recentProducts += '<a href="' + myArray[2] + '">Link2</a>';
                $.cookie('mycookie', '»'+myArray[2]+'»'+currentUrl, { path: '/', expires: cookieExp });
            }
            else if (currentUrl==myArray[2])
            {
                var recentProducts='<div class="mycookie"><h4>Recently Viewed Products</h4>';
                recentProducts += '<a href="' + myArray[1] + '">Link1</a>';
                $.cookie('mycookie', '»'+myArray[1]+'»'+currentUrl, { path: '/', expires: cookieExp });
            }
            else
            {
                var recentProducts='<div class="mycookie"><h4>Recently Viewed Products</h4>';
                recentProducts += '<a href="' + myArray[1] + '">Link1</a>';
                recentProducts += '<a href="' + myArray[2] + '">Link2</a>';
                $.cookie('mycookie', '»'+myArray[1]+'»'+myArray[2]+'»'+currentUrl, { path: '/', expires: cookieExp });
            }
        } 



        else if (myArrayLen == 3){
            if (currentUrl==myArray[1])
            {
                var recentProducts='<div class="mycookie"><h4>Recently Viewed Products</h4>';
                recentProducts += '<a href="' + myArray[2] + '">Link1</a>';
                recentProducts += '<a href="' + myArray[3] + '">Link2</a>';
                $.cookie('mycookie', '»'+myArray[2]+'»'+myArray[3]+'»'+currentUrl, { path: '/', expires: cookieExp });
            }
            else if (currentUrl==myArray[2])
            {
                var recentProducts='<div class="mycookie"><h4>Recently Viewed Products</h4>';
                recentProducts += '<a href="' + myArray[1] + '">Link1</a>';
                recentProducts += '<a href="' + myArray[3] + '">Link2</a>';
                $.cookie('mycookie', '»'+myArray[1]+'»'+myArray[3]+'»'+currentUrl, { path: '/', expires: cookieExp });
            }
            else if (currentUrl==myArray[3])
            {
                var recentProducts='<div class="mycookie"><h4>Recently Viewed Products</h4>';
                recentProducts += '<a href="' + myArray[1] + '">Link1</a>';
                recentProducts += '<a href="' + myArray[2] + '">Link2</a>';
                $.cookie('mycookie', '»'+myArray[1]+'»'+myArray[2]+'»'+currentUrl, { path: '/', expires: cookieExp });
            }
            else
            {
                var recentProducts='<div class="mycookie"><h4>Recently Viewed Products</h4>';
                recentProducts += '<a href="' + myArray[1] + '">Link1</a>';
                recentProducts += '<a href="' + myArray[2] + '">Link2</a>';
                recentProducts += '<a href="' + myArray[3] + '">Link3</a>';
                $.cookie('mycookie', '»'+myArray[1]+'»'+myArray[2]+'»'+myArray[3]+'»'+currentUrl, { path: '/', expires: cookieExp });
            }
        }
        recentProducts='</div>';

As you can see, 1 items requires one if/else, 2 items requires an if, else if, and else; 3 items requires an if, else if, else if, and else. And so on.

This script is looking at the current URL and comparing it to the array, if it's in the array, then it won't display the current URL, but it'll append it to the end of the array so when you go to the next item it will display. Let me know if you have any questions. Help is greatly appreciated!

  • 2
    `for (var idx=0; idx < length; ++idx) { statements };` http://stackoverflow.com/a/3010848/36866 – some Nov 13 '13 at 21:29
  • 1
    SO is not a code writing service, give the for loop a shot let us know how it goes – Andy Jones Nov 13 '13 at 21:34
  • use some templating library like underscore – sublime Nov 13 '13 at 21:38
  • Andy - My apologies. This was my first question asked. I didn't exactly want someone to write the whole code, more give me insight. If I post again I'll phrase my question better. I got an answer below that worked tho. – Paul Kragthorpe Nov 13 '13 at 22:24

1 Answers1

-1

I would suggest you do the following

if (myArrayLen >= 1){
    var index = myArray.indexOf(currentUrl);
    if(index > -1)
        myArray.splice(index,1); //removes currentUrl from myArray
    var recentProducts='<div class="mycookie"><h4>Recently Viewed Products</h4>';
    var cook = '';
    //create links for the rest of the items in the loop.
    for(var i=1;i<myArray.length;i++) {
        recentProducts += '<a href="' + myArray[i] + '">Link'+ i +'</a>';
        cook += '»'+myArray[i];
    }
    recentProducts += '</div>';
    $.cookie('mycookie', cook+'»'+currentUrl, { path: '/', expires: cookieExp });
}
Danny
  • 7,368
  • 8
  • 46
  • 70
  • I would do two changes: Add `myArray = myArray.slice(-2);` (or how many items you want) after you removed the current url, to keep the list from growing too large. The second change would be to use `myArray.join('»')` instead of manually building a string. – some Nov 13 '13 at 21:58
  • And a third thing: In javascript arrays are zero based, so your for-loop is off by one. – some Nov 13 '13 at 22:00
  • I thought it would be off as well, but I think when it does the split('»') it stores a blank value in 0. – Paul Kragthorpe Nov 13 '13 at 22:15
  • Danny - Thank you! While I didn't use the exact code, your part about removing currentUrl from the array before the for loop was what I was looking for! I figured it was something simple like that. Been wracking my head all day on it. Thanks again! – Paul Kragthorpe Nov 13 '13 at 22:22
  • @PaulKragthorpe In your code you have a '»' as the first character in the cookie, in Dannys code there isn't. Therefore a cookie set with your code will have a blank item at index 0, but not if the cookie was set with Dannys code. – some Nov 13 '13 at 23:07
  • @some splice and slice are different, the array is only shrinking here. 2nd, he starts everything at the 1st index, it looks like the 0 index is empty when he does the split, like Paul said (I never tested the code so cant assume). Also » is always added first as you can see in the for loop, so it will always be the first character. – Danny Nov 14 '13 at 00:48
  • @Danny I do know the difference between splice and slice. I suggested that you add a slice on the row **AFTER** splice (**not replace it**), since you have no idea what the content of the cookie is: It could have 100+ urls in it, because it could have been set by another function, or they could have changed how many urls they want, or the user could have changed the cookie. A simple slice after the splice will take care of the number of items and make sure you have a maximum number of items in the array. – some Nov 14 '13 at 01:24
  • I see that you do `cook += '»'+myArray[i];` so you do get the » at the beginning. I missed that because that isn't how it normally is done. So you do get an empty element at the first element when the split is done. This is however a code smell that stinks so much that people living on the other side of the globe complains. In javascript arrays start at index 0 and by ignoring that you open up yourself to plenty of bugs in your programs. It is also an anti-pattern to concatenate strings they way you do it. – some Nov 14 '13 at 01:25
  • @some how else would you concatenate the strings such that it starts with »? Also I he was already ignoring the zero index, I was only going to fix so much. – Danny Nov 14 '13 at 01:42
  • @Danny At the cookie row, if you really want to start with » : `'»'+myArray.join('»')`, but I do not see any reason to start with ». It is better to only have it between the urls. Then `myArray=url.split('»')` and `url=myArray.join('»')` will give the expected result. – some Nov 14 '13 at 01:51
  • @some I don't see any reason to start it with » either, but he did, so I left it that way for him. Also wouldn't `join` have to loop through the array again, I don't see any reason that would be more efficient when I am already looping through the array to create the markup. Not that it matters in this example, but in general I don't see why it would be a good idea when I already have a for loop. – Danny Nov 14 '13 at 13:07
  • 1
    @Danny In javascript strings are immutable (can't be changed). So when you do your loop and add a substring, your are actually creating a new string in every loop. That can be quite expensive depending on the javascript engine: A couple of years ago I did a test with string concatenation vs array.join. I had to cancel the test on MSIE because it didn't finish in 20 minutes, while it only took a couple of seconds on the other browsers. With array.join MSIE was as fast as everyone else. With array.join the engine can optimize the memory usage, and only copy the content once. – some Nov 14 '13 at 18:15