3
\$\begingroup\$

I'm working with product codes, so a user can only scan these type of product codes into a textarea.

I don't know if my code is written the best way possible, or has good performance.

I'm using a whitelist solution for preventing bad entries.

It saves into file all product codes after 3 seconds of last keyup.

Is it good to use javascript and jQuery mixed?

And is it the correct way to use a php query to fetch product code and product code ending?

Why is "use strict" mode useless in my code?

"use strict";
toFile.focus();

var timeoutId;
toFile.addEventListener("keyup", event => {
    clearTimeout(timeoutId);
    timeoutId = setTimeout(function() {
        const data = toFile.value.split("\n");
        const result = data.unique();
        info.textContent = result.length !== data.length ? "Duplicate removed" : "";
        toFile.value = result.join('\n');
    }, 100);

    timeoutId = setTimeout(function() {
        onInput(document.getElementById('toFile'));
    }, 3000);
});

if (!Array.prototype.unique) {
    Object.defineProperty(Array.prototype, "unique", {
        configurable: false,  // these 3 properties default to false so not needed
        enumerable: false,    // But have added them just to show their availability.
        writable: false,         
        value: function() { 
            const existing = {};
            return this.filter(a => existing[a] = !existing[a] ? true : false);
        }
    });
} else {
    throw new Error("Array.prototype.unique already defined.");
}

const regexNounFilters = [<?php
    $data = $pdo->query("SELECT PRODUCT_CODE AS code, ENDING AS ec FROM test")->fetchAll(PDO::FETCH_OBJ);

    foreach ($data as $key) {
        $separator = ($key != end($data)) ? ", " : '';
        $std = "/^(" . $key->code . ")([a-zA-Z0-9]{" . $key->ec . "})$/";
        echo $std.$separator;
    }
?>];

// example: const regexNounFilters = [/^(AAAB)([a-zA-Z0-9]{7})$/, /^(BBBBBC)([a-zA-Z0-9]{9})$/];

const extractNouns = string =>
  string
  .split('\n')
  .filter(line =>
    regexNounFilters.some(re =>
      line.trim().toUpperCase().match(re)
    )
);

function onInput(target) {
    target.value = extractNouns(target.value).join('\n');
}

function saveToFile() {
    console.log('Saving to the db');
    toFile = $('#toFile').val().replace(/\n\r?/g, '<br />');
    $.ajax({
        url: "test2.php",
        type: "POST",
        data: {toFile:toFile}, // serializes the form's elements.
        beforeSend: function(xhr) {
            // Let them know we are saving
            $('#status').html('Saving...');
        },
        success: function(toFile) {
            // You can get data returned from your ajax call here. ex. jqObj.find('.returned-data').html()
            // Now show them we saved and when we did
            var d = new Date();
            $('#status').html('Saved! Last: ' + d.toLocaleTimeString());
        },
    });
}

$('.form').submit(function(e) {
    saveToFile();
    e.preventDefault();
});
\$\endgroup\$
1
\$\begingroup\$

Responses to your questions

Is it good to use javascript and jQuery mix?

If you are going to include the jQuery library, you might as well use it whenever possible. That may mean converting expressions like

document.getElementById('toFile')

to

$('#toFile')

and

target.value = extractNouns(target.value).join('\n');

to

target.val(extractNouns(target.val()).join('\n');

However, there are some people who believe jQuery isn't as relevant in today's web. For more information, check out youmightnotneedjquery.com/, which offers alternatives to many of the utilities jQuery offers, like the Fetch API for AJAX requests.

And is it correct way use php query to fetch product code and product code ending?

It is difficult to tell what "correct way" means. Do you mean is it acceptable to use PHP to generate Javascript code inline? If so, there are many factors, like who would be reading your code (including your future self), etc. Refer to answers to Is it considered bad practice to have PHP in your JavaScript.

The product codes and endings timeoutId used to generate regexNounFilters could be output using implode() and the values stored in $std inside the foreach could be returned in a callback passed to array_map() -

So instead of:

foreach ($data as $key) {
    $separator = ($key != end($data)) ? ", " : '';
    $std = "/^(" . $key->code . ")([a-zA-Z0-9]{" . $key->ec . "})$/";
    echo $std.$separator;
}

Something like this could be used:

echo implode(", ", array_map(function($key) {
    return "/^(" . $key->code . ")([a-zA-Z0-9]{" . $key->ec . "})$/";
}, $data);

Other review feedback

At the start of the JavaScript code I see this:

toFile.focus();

where is toFile defined? is it just utilizing that element using the id attribute? If so, this is basically a global variable reference, which can lead to trouble/confusion in the future as the code grows. Refer to answers to Do DOM tree elements with ids become global variables?. It would be wise to utilize document.getElementById('toFile'), as is done in the second callback in the keyup event handler.


In the keyup event listener on toFile, timeoutId is assigned to two separate timers,

timeoutId = setTimeout(function() {
    const data = toFile.value.split("\n");
    const result = data.unique();
    info.textContent = result.length !== data.length ? "Duplicate removed" : "";
    toFile.value = result.join('\n');
}, 100);

timeoutId = setTimeout(function() {
    onInput(document.getElementById('toFile'));
}, 3000);

Should the first assignment be preserved? Because the second one overwrites it, there is not any way the first can be cleared via clearTimeout(). If the first should be clearable, use a different name for those two timer ids.


The DOM queries could be optimized by saving references in variables - for example, $('#status') appears in the beforeSend and success handlers of the AJAX call.

beforeSend: function(xhr) {
    // Let them know we are saving
    $('#status').html('Saving...');
},

A reference could be stored to the DOM element before the call to $.ajax() - e.g.

const successContainer = $('#status') 

Then the beforeSend callback could be simplified to a function partial:

beforeSend: successContainer.html.bind(successContainer, "Saving..."),

And the success callback can also utilize that variable:

success: function(toFile) {
    // You can get data returned from your ajax call here. ex. jqObj.find('.returned-data').html()
    // Now show them we saved and when we did
    var d = new Date();
    statusContainer.html('Saved! Last: ' + d.toLocaleTimeString());
},

A function partial could also be used to simplify the second timer:

timeoutId = setTimeout(onInput.bind(null, document.getElementById('toFile')), 3000);

You didn't include the HTML so I am not sure which element(s) this targets:

$('.form').submit(function(e) {

But I would guess it is a <form> tag with attribute class="form". If that is the case, the class attribute isn't necessary, even for CSS. The form element(s) could be selected with the tag name:

$('form').submit(function(e) {
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.