Combine multiple click function into one function

Need help to reduce function code. I have similar functions with unique button id the variables changes. I am trying if possible to put all in one function. Here is just snippet there is more than 48

$("#b").click(function(){
var to_add = $("#t1").val();
$("#master").append("<option value='" + to_add +"'>"+to_add+"</option>");
$("#t1").val('');
$("#purchases").val(to_add); 
});

$("#b1").click(function(){
$("#master option:selected").each(function() {
$("#purchases").append("<option>"+$(this).text()+"</option>");
$("#master option[value= '"+ $(this).val() + "' ]").remove();
});
$('#purchases option').prop('selected', true);
});

$("#b2").click(function(){
$("#purchases option:selected").each(function() {
$("#master").append("<option>"+$(this).text()+"</option>");
$("#purchases option[value= '"+ $(this).val() + "' ]").remove();
});
$('#purchases option').prop('selected', true);
});

$("#b3").click(function(){
$("#master option:selected").each(function() {
$("#salaries").append("<option>"+$(this).text()+"</option>");
$("#master option[value= '"+ $(this).val() + "' ]").remove();
});
$('#salaries option').prop('selected', true);
});

$("#b4").click(function(){
$("#salaries option:selected").each(function() {
$("#master").append("<option>"+$(this).text()+"</option>");
$("#salaries option[value= '"+ $(this).val() + "' ]").remove();
});
$('#salaries option').prop('selected', true);
});

You’l see button 1 and 2 works together as 3 and 4 works together

First of all we’ll get it properly indented, to make it easier to understand what we’re looking at.

$("#b").click(function() {
    var to_add = $("#t1").val();
    $("#master").append("<option value='" + to_add + "'>" + to_add + "</option>");
    $("#t1").val('');
    $("#purchases").val(to_add);
});

$("#b1").click(function() {
    $("#master option:selected").each(function() {
        $("#purchases").append("<option>" + $(this).text() + "</option>");
        $("#master option[value= '" + $(this).val() + "' ]").remove();
    });
    $('#purchases option').prop('selected', true);
});

$("#b2").click(function() {
    $("#purchases option:selected").each(function() {
        $("#master").append("<option>" + $(this).text() + "</option>");
        $("#purchases option[value= '" + $(this).val() + "' ]").remove();
    });
    $('#purchases option').prop('selected', true);
});

$("#b3").click(function() {
    $("#master option:selected").each(function() {
        $("#salaries").append("<option>" + $(this).text() + "</option>");
        $("#master option[value= '" + $(this).val() + "' ]").remove();
    });
    $('#salaries option').prop('selected', true);
});

$("#b4").click(function() {
    $("#salaries option:selected").each(function() {
        $("#master").append("<option>" + $(this).text() + "</option>");
        $("#salaries option[value= '" + $(this).val() + "' ]").remove();
    });
    $('#salaries option').prop('selected', true);
});

We can now move out different values to strings, and attempt to come up with identical lines:

$("#b").click(function() {
    var select = $("#master");
    var field = $("#purchases");
    var valueEl = $("#t1");
    var to_add = valueEl.val();
    select.append("<option value='" + to_add + "'>" + to_add + "</option>");
    valueEl.val('');
    field.val(to_add);
});

$("#b1").click(function() {
    var source = $("#master");
    var target = $("#purchases");
    $("option:selected", source).each(function() {
        target.append("<option>" + $(this).text() + "</option>");
        $("option[value= '" + $(this).val() + "' ]", source).remove();
    });
    $('option', target).prop('selected', true);
});

$("#b2").click(function() {
    var source = $("#purchases");
    var target = $("#master");
    $("option:selected", source).each(function() {
        target.append("<option>" + $(this).text() + "</option>");
        $("option[value= '" + $(this).val() + "' ]", source).remove();
    });
    $('option', source).prop('selected', true);
});

$("#b3").click(function() {
    var source = $("#master");
    var target = $("#salaries");
    $("option:selected", source).each(function() {
        target.append("<option>" + $(this).text() + "</option>");
        $("option[value= '" + $(this).val() + "' ]", source).remove();
    });
    $('option', target).prop('selected', true);
});

$("#b4").click(function(){
    var source = $("#salaries");
    var target = "#master";
    $("option:selected", source).each(function() {
        master.append("<option>"+$(this).text()+"</option>");
        $("option[value= '"+ $(this).val() + "' ]", source).remove();
    });
    $('option', source).prop('selected', true);
});

And we can now move some lines out to helper functions.

$("#b").click(function() {
    var select = $("#master");
    var field = $("#purchases");
    var valueEl = $("#t1");
    var to_add = valueEl.val();
    select.append("<option value='" + to_add + "'>" + to_add + "</option>");
    valueEl.val('');
    field.val(to_add);
});

function moveOption(source, target) {
    $("option:selected", source).each(function(option) {
        target.append("<option>" + $(option).text() + "</option>");
        $("option[value= '" + $(option).val() + "' ]", source).remove();
    });
}

$("#b1").click(function() {
    var source = $("#master");
    var target = $("#purchases");
    moveOption(source, target);
    $('option', target).prop('selected', true);
});

$("#b2").click(function() {
    var source = $("#purchases");
    var target = $("#master");
    moveOptions(source, target);
    $('option', source).prop('selected', true);
});

$("#b3").click(function() {
    var source = $("#master");
    var target = $("#salaries");
    moveOption(source, target);
    $('option', target).prop('selected', true);
});

$("#b4").click(function() {
    var source = $("#salaries");
    var target = $("#master");
    moveOption(source, target);
    $('option', source).prop('selected', true);
});

The master switching back and forth between source and target, make it difficult to also refactor the line below moveOption that selects the option. A separate master variable is needed, so help us make better sense of things:

$("#b").click(function() {
    var master = $("#master");
    var field = $("#purchases");
    var valueEl = $("#t1");
    var to_add = valueEl.val();
    master.append("<option value='" + to_add + "'>" + to_add + "</option>");
    valueEl.val('');
    field.val(to_add);
});

function moveOption(source, target) {
    $("option:selected", source).each(function(option) {
        target.append("<option>" + $(option).text() + "</option>");
        $("option[value= '" + $(option).val() + "' ]", source).remove();
    });
}

$("#b1").click(function() {
    var master = $("#master");
    var target = $("#purchases");
    moveOption(master, target);
    $('option', target).prop('selected', true);
});

$("#b2").click(function() {
    var master = $("#master");
    var source = $("#purchases");
    moveOptions(source, master);
    $('option', source).prop('selected', true);
});

$("#b3").click(function() {
    var master = $("#master");
    var target = $("#salaries");
    moveOption(master, target);
    $('option', target).prop('selected', true);
});

$("#b4").click(function() {
    var source = $("#salaries");
    var master = $("#master");
    moveOption(source, master);
    $('option', source).prop('selected', true);
});

We can now rename source/target to a more generic name of select.

$("#b").click(function() {
    var master = $("#master");
    var field = $("#purchases");
    var valueEl = $("#t1");
    var to_add = valueEl.val();
    master.append("<option value='" + to_add + "'>" + to_add + "</option>");
    valueEl.val('');
    field.val(to_add);
});

function moveOption(source, target) {
    $("option:selected", source).each(function(option) {
        target.append("<option>" + $(option).text() + "</option>");
        $("option[value= '" + $(option).val() + "' ]", source).remove();
    });
}

$("#b1").click(function() {
    var master = $("#master");
    var select = $("#purchases");
    moveOption(master, select);
    $('option', select).prop('selected', true);
});

$("#b2").click(function() {
    var master = $("#master");
    var select = $("#purchases");
    moveOptions(select, master);
    $('option', select).prop('selected', true);
});

$("#b3").click(function() {
    var master = $("#master");
    var select = $("#salaries");
    moveOption(master, select);
    $('option', select).prop('selected', true);
});

$("#b4").click(function() {
    var master = $("#master");
    var select = $("#salaries");
    moveOption(select, master);
    $('option', select).prop('selected', true);
});

We can’t yet move the option select line out to the moveOption function, because we have two different types of move options. One moves to master, the other moves away from master.

$("#b").click(function() {
    var master = $("#master");
    var field = $("#purchases");
    var valueEl = $("#t1");
    var to_add = valueEl.val();
    master.append("<option value='" + to_add + "'>" + to_add + "</option>");
    valueEl.val('');
    field.val(to_add);
});

function moveOption(source, target) {
    $("option:selected", source).each(function(option) {
        target.append("<option>" + $(option).text() + "</option>");
        $("option[value= '" + $(option).val() + "' ]", source).remove();
    });
}

function moveOptionToMaster(master, select) {
    moveOption(select, master);
}

function removeOptionFromMaster(master, select) {
    moveOption(master, select);
}

$("#b1").click(function() {
    var master = $("#master");
    var select = $("#purchases");
    moveOptionToMaster(master, select);
    $('option', select).prop('selected', true);
});

$("#b2").click(function() {
    var master = $("#master");
    var select = $("#purchases");
    removeOptionFromMaster(master, select);
    $('option', select).prop('selected', true);
});

$("#b3").click(function() {
    var master = $("#master");
    var select = $("#salaries");
    moveOptionToMaster(master, select);
    $('option', select).prop('selected', true);
});

$("#b4").click(function() {
    var master = $("#master");
    var select = $("#salaries");
    removeOptionFromMaster(master, select);
    $('option', select).prop('selected', true);
});

And now that the function properties are consistent, we can move the option select code in to the function too.

function moveOption(source, target) {
    $("option:selected", source).each(function(option) {
        target.append("<option>" + $(option).text() + "</option>");
        $("option[value= '" + $(option).val() + "' ]", source).remove();
    });
}

function moveOptionToMaster(master, select) {
    moveOption(select, master);
    $('option', select).prop('selected', true);
}

function removeOptionFromMaster(master, select) {
    moveOption(master, select);
    $('option', select).prop('selected', true);
}

$("#b").click(function() {
    var master = $("#master");
    var field = $("#purchases");
    var valueEl = $("#t1");
    var to_add = valueEl.val();
    master.append("<option value='" + to_add + "'>" + to_add + "</option>");
    valueEl.val('');
    field.val(to_add);
});

$("#b1").click(function() {
    var master = $("#master");
    var select = $("#purchases");
    moveOptionToMaster(master, select);
});

$("#b2").click(function() {
    var master = $("#master");
    var select = $("#purchases");
    removeOptionFromMaster(master, select);
});

$("#b3").click(function() {
    var master = $("#master");
    var select = $("#salaries");
    moveOptionToMaster(master, select);
});

$("#b4").click(function() {
    var master = $("#master");
    var select = $("#salaries");
    removeOptionFromMaster(master, select);
});

To help clean up the first section of code, we can create an addOption function too.

function addOption(select, value) {
    select.append("<option>" + value + "</option>");
}

function moveOption(source, target) {
    $("option:selected", source).each(function(option) {
        target.append("<option>" + $(option).text() + "</option>");
        $("option[value= '" + $(option).val() + "' ]", source).remove();
    });
}

function moveOptionToMaster(master, select) {
    moveOption(select, master);
    $('option', select).prop('selected', true);
}

function removeOptionFromMaster(master, select) {
    moveOption(master, select);
    $('option', select).prop('selected', true);
}

$("#b").click(function() {
    var master = $("#master");
    var field = $("#purchases");
    var valueEl = $("#t1");
    addOption(master, valueEl.val());
    valueEl.val('');
    field.val(to_add);
});

$("#b1").click(function() {
    var master = $("#master");
    var select = $("#purchases");
    moveOptionToMaster(master, select);
});

$("#b2").click(function() {
    var master = $("#master");
    var select = $("#purchases");
    removeOptionFromMaster(master, select);
});

$("#b3").click(function() {
    var master = $("#master");
    var select = $("#salaries");
    moveOptionToMaster(master, select);
});

$("#b4").click(function() {
    var master = $("#master");
    var select = $("#salaries");
    removeOptionFromMaster(master, select);
});

Those click functions can be made more sensible too, because there’s a clear connection between them.

function addOption(select, value) {
    select.append("<option>" + value + "</option>");
}

function moveOption(source, target) {
    $("option:selected", source).each(function(option) {
        target.append("<option>" + $(option).text() + "</option>");
        $("option[value= '" + $(option).val() + "' ]", source).remove();
    });
}

function moveOptionToMaster(master, select) {
    moveOption(select, master);
    $("option", select).prop("selected", true);
}

function removeOptionFromMaster(master, select) {
    moveOption(master, select);
    $("option", select).prop("selected", true);
}

function initButtons({add, remove, target}) {
    $(add).click(function() {
        moveOptionToMaster(master, $(target));
    });
    $(remove).click(function() {
        removeOptionFromMaster(master, $(target));
    });
}

var master = $("#master");

$("#b").click(function() {
    var valueField = $("#t1");
    addOption(master, valueField.val());
    $("#purchases").val(valueField.val());
    valueField.val("");
});

initButtons({add: "#b1", remove: "#b2", target: "purchases"});
initButtons({add: "#b3", remove: "#b4", target: "#salaries"});```

And we’re now starting to get to something interesting.

As you can see it’s a process of gradual improvement. That’s the nice thing about refactoring. It about making small improvements over and over, that end up resulting in a large improvements.

Thanx for your detail instruction, To understand properly how to analyze before implement I started with STEP 1 (move out different values to strings) to understand exactly in which direction you moved. I implemented in my code to see the result it worked 100%. In STEP 2 (move some lines out to helper functions) however when I move item from master to salaries it move a blank line and not the item I have selected.

I was on shakey ground with no html code to test that things continue to properly work. I’ll make another run through it shortly using an actual test page to ensure that things keep working as they should.

Can I please get from you some HTML code that can be used to let me suitably exercise the JavaScript code? I can fake some up, but it helps if the intended usage helps to guide things too.

<!DOCTYPE html>
    <html lang="en">
    <head>
        <meta charset="UTF-8">
        <meta name="viewport" content="width=device-width, initial-scale=1.0">
        <meta http-equiv="X-UA-Compatible" content="ie=edge">
        <title>Learn Javascript</title>
        <script src="https://ajax.googleapis.com/ajax/libs/jquery/3.1.1/jquery.min.js">
    </script>
 
</head>

<select name=master[] id=master class="master" multiple="multiple" size='23'>
<?php   
    $file = fopen("temp.csv", "r");
    while (($row = fgetcsv($file, 0, ",")) !== FALSE) {
        $master = $row[0];
?>
    <option><?php echo $master; ?></option>
<?php
    }
?>
</select> 
<form action="update.php" method="post">
    <input type=button class="master" name=b1 id=b1 value='Move >'>
    <input type=button class="master" name=b1 id=b1 value='< Remove'>&nbsp;&nbsp;&nbsp;&nbsp;
<select name=purchases[] id=purchases multiple="multiple" class=master>
<?php   
    $file = fopen("purchases.csv", "r");
    while (($row = fgetcsv($file, 0, ",")) !== FALSE) {
        $purchases = $row[0];
?>
    <option value="<?php echo $purchases;?>"><?php echo $purchases;?></option>
<?php
    }

<form action="update.php" method="post">
    <input type=button class="master" name=b3 id=b3 value='Move >'>
    <input type=button class="master" name=b4 id=b4 value='< Remove'>&nbsp;&nbsp;&nbsp;&nbsp;
<select name=salaries[] id=salaries multiple="multiple" class=master>
<?php   
    $file = fopen("salaries.csv", "r");
    while (($row = fgetcsv($file, 0, ",")) !== FALSE) {
        $salaries = $row[0];
?>
    <option value="<?php echo $salaries;?>"><?php echo $salaries;?></option>
<?php
    }
?>
</select>
&nbsp;&nbsp;&nbsp;&nbsp;<input type="submit" value="Save File" name="submit">
</form>

The csv files you can populate with any data the upload.php file to save data you can help me here as well to get it into a simplify (there is more than 24 but here I just used 2 as an example) and thank you for still trying

<?php
header("Location:".$URL.'index.php');

if ($_POST['purchases']) {
$purchases = $_POST['purchases'];
$purchasesunique = array_unique($purchases);
sort($purchasesunique);
foreach ($purchasesunique as $key => $value) {
    $result.=$value."\n";
}
file_put_contents('purchases.csv',$result);
}

if ($_POST['salaries']) {
$salaries = $_POST['salaries'];
$salariesunique = array_unique($salaries);
sort($salariesunique);
foreach ($salariesunique as $key => $value) {
    $result.=$value."\n";
}
file_put_contents('salaries.csv',$result);
}


?>

There doesn’t seem to be any section in that HTML code for purchases which is also used quite a lot in the JavaScript code, or the #b1 and #b2 buttons. Does that look similar to the salaries or is it different?

Edit: Oh, and what HTML code is that #b and #t1 code relate to as well?

Hi Paul you can discard that part as it is used only once

will edit above code

Should I remove it completely from the code being worked on?

If the #t1, #b, #b1, #b2 should be included, I want some suitable HTML code so that I can test that my updates work properly.

the html code has been updated

Thank you for that.

I don’t see any HTML code relating to #b or #t1. Is that to be removed from the scripting code being worked on?

Sorry forget to say, the #t1, #b you can omit from code as it is only used once and will run that separately

Okay, good one. We’ll go with that.

The HTML code in post #6 isn’t valid. There’s missing closing tags for the select and form element for example.

Can you please check that the HTML code is correct so that we can start from a known good starting point, before the JavaScript work occurs?

Will check I have fiddling with code let me just get it back in working order then will post

Thanks. I didn’t mean to make this a big issue, but it helps to have a good foundation on which to build on.

Ok, herewith my index.php file

<!DOCTYPE html>
    <html lang="en">
    <head>
        <meta charset="UTF-8">
        <meta name="viewport" content="width=device-width, initial-scale=1.0">
        <meta http-equiv="X-UA-Compatible" content="ie=edge">
        <title>Learn Javascript</title>
        <script src="https://ajax.googleapis.com/ajax/libs/jquery/3.1.1/jquery.min.js">
    </script>

</head>

<select name=master[] id=master class="master" multiple="multiple" size='23'>
<?php   
    $file = fopen("temp.csv", "r");
    while (($row = fgetcsv($file, 0, ",")) !== FALSE) {
        $master = $row[0];
?>
    <option><?php echo $master; ?></option>
<?php
    }
?>
</select> 

<form action="update.php" method="post">
    <input type=button class="master" name=b1 id=b1 value='Move >'>
    <input type=button class="master" name=b2 id=b2 value='< Remove'>&nbsp;&nbsp;&nbsp;&nbsp;
<select name=purchases[] id=purchases multiple="multiple" class=master>
<?php   
    $file = fopen("purchases.csv", "r");
    while (($row = fgetcsv($file, 0, ",")) !== FALSE) {
        $purchases = $row[0];
?>
    <option value="<?php echo $purchases;?>"><?php echo $purchases;?></option>
<?php
    }
?>
</select>
&nbsp;&nbsp;&nbsp;&nbsp;<input type="submit" value="Save File" name="submit">
</form><br /><br />

<form action="update.php" method="post">
    <input type=button class="master" name=b3 id=b3 value='Move >'>
    <input type=button class="master" name=b4 id=b4 value='< Remove'>&nbsp;&nbsp;&nbsp;&nbsp;
<select name=salaries[] id=salaries multiple="multiple" class=master>
<?php   
    $file = fopen("salaries.csv", "r");
    while (($row = fgetcsv($file, 0, ",")) !== FALSE) {
        $salaries = $row[0];
?>
    <option value="<?php echo $salaries;?>"><?php echo $salaries;?></option>
<?php
    }
?>
</select>
&nbsp;&nbsp;&nbsp;&nbsp;<input type="submit" value="Save File" name="submit">
</form>

 <script>
 $("#b1").click(function() {
    var source = $("#master");
    var target = $("#purchases");
    $("option:selected", source).each(function() {
        target.append("<option>" + $(this).text() + "</option>");
        $("option[value= '" + $(this).val() + "' ]", source).remove();
    });
    $('option', target).prop('selected', true);
});

$("#b2").click(function(){
    var source = $("#purchases");
    var target = "#master";
    $("option:selected", source).each(function() {
        master.append("<option>"+$(this).text()+"</option>");
        $("option[value= '"+ $(this).val() + "' ]", source).remove();
    });
    $('option', source).prop('selected', true);
});

$("#b3").click(function() {
    var source = $("#master");
    var target = $("#salaries");
    $("option:selected", source).each(function() {
        target.append("<option>" + $(this).text() + "</option>");
        $("option[value= '" + $(this).val() + "' ]", source).remove();
    });
    $('option', target).prop('selected', true);
});

$("#b4").click(function(){
    var source = $("#salaries");
    var target = "#master";
    $("option:selected", source).each(function() {
        master.append("<option>"+$(this).text()+"</option>");
        $("option[value= '"+ $(this).val() + "' ]", source).remove();
    });
    $('option', source).prop('selected', true);
});
    </script>

Herewith update.php

<?php
header("Location:".$URL.'index.php');

if ($_POST['purchases']) {
$purchases = $_POST['purchases'];
$purchasesunique = array_unique($purchases);
sort($purchasesunique);
foreach ($purchasesunique as $key => $value) {
    $result.=$value."\n";
}
file_put_contents('purchases.csv',$result);
}

if ($_POST['salaries']) {
$salaries = $_POST['salaries'];
$salariesunique = array_unique($salaries);
sort($salariesunique);
foreach ($salariesunique as $key => $value) {
    $result.=$value."\n";
}
file_put_contents('salaries.csv',$result);
}

?>

Thank you for that. For the purpose of reworking the JavaScript code, I have removed all complexities with php and file access, and replaced the php-generated options with some test options of foo bar, baz qux, quux and quuz.

I’ve run the scripting code through a beautifier and JSLint to give me a good starting point, and some initial test structure has been added to the test page.

mocha.setup("bdd");
const expect = chai.expect;
describe("select tests", function () {

});
mocha.run();

Next up is to create some tests that exercise the existing JavaScript code, before making any further changes to it.

2 nifty tools just linked to them busy reading & testing

When putting together some tests, I’m noticing that the after using move or remove to move an option, that trying to move or remove that new option doesn’t remove it from the select box.

Is that the correct and intended behaviour to start with? That options that get added to a select box can’t have some options removed from them?

If not, and they are supposed to be removed, I can fix that up by setting the value attribute of added options in your original JavaScript code, before going ahead with refactoring and reworking the code.