views:

622

answers:

4

I am trying to make a bookmarklet that when clicked will check the URL of the current tab/window to see if it contains 'char1' and/or 'char2' (a given character). If both chars are present it redirects to another URL, for the other two it will append the current URL respectively.

I believe there must be a more elegant way of stating this than the following (which has so far worked perfectly for me) but I don't have great knowledge of Javascript. My (unwieldy & repetitive) working code (apologies):

if (window.location.href.indexOf('char1') != -1 &&
    window.location.href.indexOf('char2') != -1)
{
    window.location="https://website.com/";
}
else if (window.location.href.indexOf('char1') != -1)
{
    window.location.assign(window.location.href += 'append1');
}
else if (window.location.href.indexOf('char2') != -1)
{
    window.location.assign(window.location.href += 'append2');
}

Does exactly what I need it to but, well... not very graceful to say the least.

Is there a simpler way to do this, perhaps with vars or a pseudo-object? Or better code?

+1  A: 

The only reduction I can see is to pull out the redundant indexof calls into vars and then test the vars. It's not going to make any appreciable difference in performance though.

var hasChar1 = window.location.href.indexOf('char1') != -1;
var hasChar2 = window.location.href.indexOf('char2') != -1;
if (hasChar1)
{
   if (hasChar2)
   {
      window.location="https://website.com/";
   }
   else
   {
      window.location.assign(window.location.href+='append1');
   }
} 
else if (hasChar2)
{
    window.location.assign(window.location.href+='append2');
}
dthorpe
Thanks for responding, I'm grateful for your input. This is a much better structured 'if' and resolves a little more logically than mine. I'm curious to know if there was a reason why you used the same assign(loc.href+=) as me... it's redundant, but the only way I could force the result onto the location.href as opposed to returning a string to the main document window.
Wintermute
This is working very well for me. I have gone further and, as Andy E suggested, added a new var: loc = window.location. It's now just loc.property or loc.function.
Wintermute
I didn't look closely at the window.location assignments. Say hello to Neuromancer for me. ;>
dthorpe
He runs with loa now, but I'll try to get his Hash
Wintermute
A: 

var location =window.location.href

if (location.indexOf('char1')!=-1 &&  location.indexOf('char2')!=-1)
{window.location="https://website.com/";} 
else if (location.href.indexOf('char1')!=-1) {window.location.assign(location+='append1');}
else if (location.indexOf('char2')!=-1) {window.location.assign(location+='append2');}
Salil
Wintermute
+1  A: 

A (sort-of) refactoring of dthorpe's suggestion:

var hasC1  = window.location.href.indexOf('char1')!=-1
var hasC2  = window.location.href.indexOf('char2')!=-1
var newLoc = hasC1 
               ? hasC2 ? "https://website.com/" : window.location.href+'append1'
               : hasC2 ? window.location.href+'append1' : '';

if (newLoc)
    window.location = newLoc;

Calling assign is the same as assigning a value to window.location, you were doing both with the addition assignment += operator in the method anyway:

window.location.assign(window.location.href+='append2')

This would actually assign "append2" to the end of window.location.href before calling the assign method, making it redundant.

You could also reduce DOM lookups by setting window.location to a var.

Andy E
"Calling assign is the same as assigning a value to window.location..." That's what I thought, but the script kept returning the appended URL to the document window instead of the address bar, and the address bar contained the javascript. History-1 returned to the former page with the appended URL in the address bar. So I wrapped it in the assign function to try and force it to use the result. It worked, but was redundant. Incidentally, your code is doing the same thing: returning the appended URL to the doc win, not the address bar.
Wintermute
Thank you for taking time to help out. I tried to make some vars myself but it was resolving; I'm not sure what I did wrong but I was highly distracted by my friends' kids who were taking my house to pieces at the time haha... I appreciate your input.
Wintermute
@Wintermute: np. That does seem like an odd issue you're having. You could change `window.location = newLoc` for `window.location.assign(newLoc)` but as I mentioned before, they are near enough the same thing.
Andy E
Wintermute
@Wintermute: lol no problem :-)
Andy E
+1  A: 

Kind of extendable code. Am i crazy?

var loc = window.location.href;
var arr = [{
  url: "https://website.com/",
  chars: ["char1", "char2"]
}, {
  url: loc + "append1",
  chars: ["char1"]
}, {
  url: loc + "append2",
  chars: ["char2"]
}];

function containsChars(str, chars)
{
  var contains = true;
  for(index in chars) {
    if(str.indexOf(chars[index]) == -1) {
      contains = false;
      break;
    }
  }
  return contains;
}

for(index in arr) {
 var item = arr[index];
 if(containsChars(loc, item.chars)) {
    window.location.href = item.url;
    break;
 }
}
vooD
Wintermute