Are there any ways I could simplify this JavaScript function?

Question

I'm writing JavaScript scripts that interact with GUI windows. In the most basic terms, I define objects and write functions to interact with these objects. My standard practice is to make an object for each field in a window and then a function to interact and a function to verify.

So for example, if I have a window like this:

I'd have one object for Field #1's drop down and another for the text box. Same for field #2. I'd also have an object for the Done Button. Then I'd have a function for selecting from field #1's drop down, entering text into the text object, clicking the done button, etc. This is a simplification, but the main idea. We use a class for the general window layout and then child classes for each individual window but they all basically work this way.

Okay, so I've got a window where I'm trying to make a shortcut function where I want to select an option from the drop down depending on whether or not the text in the textbox is not already set for that. This is easy to check for since there are only two options "Yes" and "No".

So I've got this bit of lengthy code for a simple operation:

function toggleOption(option)
{
    //Simplistic check to see that users are only using Yes or No
    if (option.length > 3)
    {
        test.fatal("FATAL: \"" + option + "\" is not a valid input. Please use a variant of \"Yes\" or \"No\"");
    }
    
    //Allows for multiple variants of yes/no to be used in the option parameter
    option = option.substring(0,1).toUpperCase();
    var text;
    

    //Defines a text variable depending on what option is used 
    if (option === "Y")
    {
        text = option + " - Yes";
    }
    else
    {
        text = option + " - No";
    }
    

    // Opens the window if not already open
    if (!object.exists(names.Window))
    {
        openWindow();
    }
    

    // There are two fields that will be selected
    // Only selects the option if the text box is set to a different option.
    if (waitForObjectExists(names.TextBox1).text !== text)
    {
        selectField1(option);
        clickDoneButton();
    }
    
    // Only selects the option if the text box is set to a different option.
    if (waitForObjectExists(names.TextBox2).text !== text)
    {
        selectField2(option);
        clickDoneButton();
    }
    

    //The assumption is that this is going to take care of closing the window (which automatically closes when done is clicked) if none of the options are selected.
    if (object.exists(names.Window))
    {
        closeWindow();
    }
}

So this code all works, but it seems ridiculously convoluted and bloated for something that should be pretty simple. I was wondering if anyone had any insights on methods I could use to simplify this and make it more efficient.

NOTE: I replaced actual object names with generic names to hopefully make the code more legible. Sorry if I missed any text changes.

Since there's nothing wrong here, perhaps this question belongs on https://codereview.stackexchange.com/ — Wyck, Jul 08 '20 at 18:02
@Wyck when suggesting users post on CR it would be great if there was also a suggestion like "_Please read the relevant help center pages like '[What topics can I ask about here?](https://codereview.stackexchange.com/help/on-topic)' and '[How do I ask a good question?](https://codereview.stackexchange.com/help/how-to-ask)_". In the current form the code above would likely be closed as off-topic because it is missing context (e.g. `names` is undefined). — Sᴀᴍ Onᴇᴌᴀ, Jul 08 '20 at 18:09
@Wyck I apologize. I didn't even realize that existed. I've always associated Stack overflow with anything coding related. — JohnN, Jul 09 '20 at 13:21

Natanhel Poliszuk · Answer 1 · 2020-07-08T19:14:02.177

Well, you can use a ternary computing :

    if (option === "Y")
    {
        text = option + " - Yes";
    }
    else
    {
        text = option + " - No";
    }

turns to:

    text = option  + ((option === "Y") ? " - Yes" : " - No" );

You can also make a generic arrow function in ES6 to encapsulate option and text:

    // There are two fields that will be selected
    // Only selects the option if the text box is set to a different option.
    if (waitForObjectExists(names.TextBox1).text !== text)
    {
        selectField1(option);
        clickDoneButton();
    }
    
    // Only selects the option if the text box is set to a different option.
    if (waitForObjectExists(names.TextBox2).text !== text)
    {
        selectField2(option);
        clickDoneButton();
    }

Something like this:

const clickButton = (selectFunc, nameTextBox) => {
    // I use closure here to encapsulate 'option' and 'text'
    if (waitForObjectExists(nameTextBox).text !== text){
        selectFunc(option);
        clickDoneButton();
    }
}

and the usage:

clickButton(selectField1, names.TextBox1)
clickButton(selectField2, names.TextBox2)

Cheers! :-)

Are there any ways I could simplify this JavaScript function?

1 Answers1