• 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 all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Bear Bibeault
  • Devaka Cooray
  • Liutauras Vilda
  • Jeanne Boyarsky
Sheriffs:
  • Knute Snortum
  • Junilu Lacar
  • paul wheaton
Saloon Keepers:
  • Ganesh Patekar
  • Frits Walraven
  • Tim Moores
  • Ron McLeod
  • Carey Brown
Bartenders:
  • Stephan van Hulst
  • salvin francis
  • Tim Holloway

How can I make this code shorter?  RSS feed

 
Greenhorn
Posts: 9
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hello!

I'm doing JavaScript for about 1.5 month now. I'm trying to get better!

I wrote this code as a validator for inputs. I've found many existing code, though I didn't really understand what was going on, so did not use it (Expect for the mail validator var).

I read you shouldn't repeat yourself, so I'm trying to get better at this. My main question about this code is: What are some basic ways to make this code shorter?  

 
Marshal
Posts: 67164
169
IntelliJ IDE Java jQuery Mac Mac OS X
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Three things I see:
  • You are gathering and relying on a global variable with all the inputs gathered. That's fragile and not a good idea. Obtain a reference to each individual element as needed. And use good reference canes for the variables.
  • You are using magic numbers. Using indexes into the global array is a really really bad idea. Not only is it unreadable (what does "3" mean?), it's fragile and will break as soon as any order within the form changes.
  • All of your functions do the same thing. Create one function that does what you need to do. Any differences are passed as parameters.


  • Your turn.
     
    Bear Bibeault
    Marshal
    Posts: 67164
    169
    IntelliJ IDE Java jQuery Mac Mac OS X
    • Likes 1
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    And, you asked the wrong question: your goal should never be "make the code shorter". The goal should always be "make the the code clearer". Frequently, that does mean shorter, but "short" is not the goal. Clarity is.
     
    dario sanchez
    Greenhorn
    Posts: 9
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator

    Bear Bibeault wrote:Three things I see:

  • You are gathering and relying on a global variable with all the inputs gathered. That's fragile and not a good idea. Obtain a reference to each individual element as needed. And use good reference canes for the variables.

  • What do you mean by this, not using querySelectorAll to gather all inputs? Should I then gather the inputs individually, or use a loop?

  • You are using magic numbers. Using indexes into the global array is a really really bad idea. Not only is it unreadable (what does "3" mean?), it's fragile and will break as soon as any order within the form changes.

  • I understand indeed. I thought it was a good idea, since the index of the animations & inputs would be the same. So that's one number for three different uses. But yes, it's not thoughtful for others who will have to work with the code. I couldn't think if a better way to spread the correct node to the correct function. I did try using a for loop, but its downside was that I'd then still have to create something that divides them.


  • All of your functions do the same thing. Create one function that does what you need to do. Any differences are passed as parameters.

  • That's one thing I considered. But those functions have different rules right? While one form is only correct when it's not empty, the other will be correct if it's not an email.

    SWIM sent me this snippet https://jsfiddle.net/Slackwise/2chp34dL/ though I barely have an idea what's going on there.
     
    dario sanchez
    Greenhorn
    Posts: 9
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Sorry for the formatting, I don't know how to edit my reply.
     
    dario sanchez
    Greenhorn
    Posts: 9
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Well, rewrote it based on your feedback. Still, i have some magic numbers. How else would I call an element based on its index position though?

     
    Marshal
    Posts: 61766
    193
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Well, don't lines 62‑84 look better now (), but . . . .
  • 1: Your first if-else has a bang sign ! in its if, and the others all lack bang signs. That is risking confusion.
  • 2: You are writing the same thing 4×, only changing the arguments slightly. Can you think of a way to reduce the repetition?


  • . . . and I hope it isn't too late to say, “Welcome to the Ranch .”
     
    Campbell Ritchie
    Marshal
    Posts: 61766
    193
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    I invented the === operator here this morning; does it really exist in JavaScript (line 90)?
     
    Campbell Ritchie
    Marshal
    Posts: 61766
    193
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    You are still using magic numbers as Bear warned you against yesterday.
     
    dario sanchez
    Greenhorn
    Posts: 9
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Rewrote it again! The if/else part.

    And yeah, I'm going to take some time to see if I can find an alternative to the magic numbers.

     
    dario sanchez
    Greenhorn
    Posts: 9
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    And yes === exists in JavaScript I suppose. "1" == 1 (true) 1 === "1" false.
     
    dario sanchez
    Greenhorn
    Posts: 9
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    And thanks for the welcome !
     
    Consider Paul's rocket mass heater.
    • Post Reply Bookmark Topic Watch Topic
    • New Topic
    Boost this thread!