Help coderanch get a
new server
by contributing to the fundraiser
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Jeanne Boyarsky
  • Ron McLeod
  • Paul Clapham
  • Liutauras Vilda
Sheriffs:
  • paul wheaton
  • Rob Spoor
  • Devaka Cooray
Saloon Keepers:
  • Stephan van Hulst
  • Tim Holloway
  • Carey Brown
  • Frits Walraven
  • Tim Moores
Bartenders:
  • Mikalai Zaikin

jquery - code review

 
author & internet detective
Posts: 41945
911
Eclipse IDE VI Editor Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
This seems poorly symmetrical. The code to take a string and turn into zero or more selected options in a select list is one line. The code to do the inverse is a story. Am I missing some pattern for doing the later? I originally had it as one line without the null check and got a JavaScript null reference.

 
Sheriff
Posts: 14691
16
Eclipse IDE VI Editor Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
What about that ?
 
author
Posts: 15385
6
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Christophe Verré wrote:What about that ?



.val() returns a string so the join is useless.
 
Eric Pascarello
author
Posts: 15385
6
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
$(...) is expensive, you want to try to minimize the amount of times you look things up.

 
Christophe Verré
Sheriff
Posts: 14691
16
Eclipse IDE VI Editor Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Eric Pascarello wrote:.val() returns a string so the join is useless.



http://api.jquery.com/val/

In the case of <select multiple="multiple"> elements, the .val() method returns an array containing each selected option.

 
Sheriff
Posts: 67750
173
Mac Mac OS X IntelliJ IDE jQuery TypeScript Java iOS
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I'd use chaining and the this pointer in the handler body to minimize calls to $().
 
Eric Pascarello
author
Posts: 15385
6
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

// the one liner gives an error
// $('#a').val($('#b').val().join(',');



That line is missing a )




Running example here: http://jsbin.com/ivubi4/
 
Eric Pascarello
author
Posts: 15385
6
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Christophe Verré wrote:

Eric Pascarello wrote:.val() returns a string so the join is useless.



http://api.jquery.com/val/

In the case of <select multiple="multiple"> elements, the .val() method returns an array containing each selected option.



Totally missed select in her question...
 
Jeanne Boyarsky
author & internet detective
Posts: 41945
911
Eclipse IDE VI Editor Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I learned two things in this thread. The idiom of merging things with or (which is very Perl like but it didn't occur to me for JavaScript) and that you can chain an event handler with something that executes now. Both of which make perfect sense now that they came up.
 
He got surgery to replace his foot with a pig. He said it was because of this tiny ad:
We need your help - Coderanch server fundraiser
https://coderanch.com/t/782867/Coderanch-server-fundraiser
reply
    Bookmark Topic Watch Topic
  • New Topic