421453710d51d7daaea8069af0aa4126

is there a better, shorter, cleaner way of doing this? The below function takes url names found on the page that are longer then 30 characters and shortens them, adding ... after. For example <a href="http://google.com">this is googles web site please visit it</a> into this <a href="http://google.com">this is googles web site plea...</a>

1
2
3
4
5
6
7
8
9
10
11
12
13
<script type="text/javascript">

window.onload = function() {

       var anchors = document.getElementsByTagName('a');

       for (var i = 0; i < anchors.length; i++) {
               anchors[i].innerHTML = anchors[i].innerHTML.length >= 30?anchors[i].innerHTML.substring(0, 30) + ' ...':anchors[i].innerHTML;
       }

}

</script>

Refactorings

No refactoring yet !

90ec841f866ac35c4ce90ee3cfffbdc3

Vime

November 9, 2007, November 09, 2007 14:44, permalink

4 ratings. Login to rate!

Probably the ternary operator is not useful here. You reassign also the short link.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
<script type="text/javascript">

window.onload = function() {

       var anchors = document.getElementsByTagName('a');

       for (var i = 0; i < anchors.length; i++) {
               if ( anchors[i].innerHTML.length >= 30 ) 
                 anchors[i].innerHTML = anchors[i].innerHTML.substring(0, 30) +'...';
       }

}

</script>
Avatar

V

November 10, 2007, November 10, 2007 22:24, permalink

1 rating. Login to rate!

The line is a bit shorter, making the ternary operator easier to parse visually.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
<script type="text/javascript">

window.onload = function() {

       var anchors = document.getElementsByTagName('a');

       for (var i = 0; i < anchors.length; i++) {
               var text = anchors[i].innerHTML;
               anchors[i].innerHTML = text.length >= 30?text.substring(0, 30)+' ...':text;
       }

}

</script>
421453710d51d7daaea8069af0aa4126

DeathfireD

November 11, 2007, November 11, 2007 13:10, permalink

No rating. Login to rate!

thanks guys. Looks like there both shorter then mine.

5170ca260dbd2cdfd5a887a4dba7636f

Jeremy Weiskotten

January 8, 2008, January 08, 2008 14:31, permalink

1 rating. Login to rate!

This isn't a refactoring per se because it changes the behavior of the code, but I think there's a bug. A string with length of exactly 30 should not be trimmed to 30 and appended with "...". Likewise, a string with length 31 or 32, trimmed to 30 and appended with "..." will now be longer than it was to begin with (although probably not as wide if you're using a variable). I suggest trimming to 27 if the length is more than 30 characters, and then appending "...".

1
2
3
4
5
6
7
8
9
10
11
<script type="text/javascript">
window.onload = function() {
       var anchors = document.getElementsByTagName('a');

       for (var i=0; i<anchors.length; i++) {
              if (anchors[i].innerHTML.length > 30) {
                     anchors[i].innerHTML = ?anchors[i].innerHTML.substring(0, 27) + ' ...':anchors[i].innerHTML;
              }
       }
}
</script>

Your refactoring





Format Copy from initial code

or Cancel