Page 2 of 2

Re: Search for webmon

Posted: Sat Jul 20, 2024 5:17 am
by Lantis
I’ll read it properly and let you know. I can tell you right off the start that I loathe “arrow functions” and think they’re an abomination. Maybe I’m just getting old :roll: but if ChatGPT thinks those are an improvement it has rocks in its head :P

I think ChatGPT can give you a good scaffold but it often misses nuances in code and makes weird assumptions. I think it’s fantastic for quickly saying “what’s the syntax for inserting an element into an array at a given position?” and then you can write your code without having to go look it up. Again, maybe I’m just getting old and forgetful.

Re: Search for webmon

Posted: Sat Jul 20, 2024 5:28 am
by ispyisail
Had a bit of a laugh ......

Tricky to explain the humour to my wife though :)
.......and more modern JavaScript code

Re: Search for webmon

Posted: Sat Jul 20, 2024 5:30 am
by ispyisail
What is Arrow Function?
An Arrow Function in JavaScript, introduced in ES6, offers a concise syntax for defining function expressions using =>. It maintains lexical `this` binding and provides shorter, more readable code compared to traditional functions. Arrow functions enhance code structure by simplifying function syntax without sacrificing functionality or clarity.

Arrow functions are anonymous functions i.e. functions without a name but they are often assigned to any variable. They are also called Lambda Functions.

Re: Search for webmon

Posted: Sat Jul 20, 2024 6:49 am
by fifonik
I've added comment that debounce should be added. I have not added it as I was not sure if the code will be used at all or not.


filterTableRows is worse then my implementation. It is always do some filtering even when searchTerm is empty.


Caching in dataset is sub-optimal as it is writing data into dataset on every cycle of the loop:

Code: Select all

row.dataset.host = host;
row.dataset.value = value;

Modern syntax -- well, it is not always available (I do not know what gargoyle targeting to so I have not used it by porpose) and it is not fastest quite often.
array.forEach was never be the fastest.
Examples:
https://dev.to/maafaishal/benchmarking- ... cenow-1jjg
https://leanylabs.com/blog/js-forEach-m ... or-for_of/
Many others on stack overflow etc.

Code: Select all

const rows = Array.from(tbody.rows);
on every filtering call -- no comments here.

Code: Select all

const input = document.getElementById(inputId);
if(!input) return;

const container = document.getElementById(containerId);
if(!container){
	// No container for the table exists -- hide filter input
	input.style.display='none';
	return;
}
=>

Code: Select all

const input = document.getElementById(inputId);
const container = document.getElementById(containerId);

if (!input || !container) {
       if (input) input.style.display = 'none';
      return;
}
Less optimal.
There is no reason to find container element if there is no input element.
And for my opinion, new bit of code is harder to read because of the nested IFs.

Code: Select all

if(...condition...){
    row.style.display = ''; // 'table-row' can also be used
    row.className = visibleIndex++ % 2 ? 'even' : 'odd';
} else {
    row.style.display = 'none';
}
=>

Code: Select all

row.style.display = isVisible ? '' : 'none';
row.className = isVisible ? (index % 2 ? 'even' : 'odd') : '';
Shorter, but less optimal. Second 'if' added with no reason.

Code: Select all

rows.forEach((row, index) => {
    ...
    row.className = isVisible ? (index % 2 ? 'even' : 'odd') : '';
});
Bug here.
odd/even class should be calculated based on visible rows only.
Imagine you have 3 rows and with entering something in input field row 2 is filtered out. With the updated code row 1 and row 3 will have 'odd' class so the table will not be striped here.

Code: Select all

const value = row.dataset.value || (row.cells[2]?.querySelector('a')?.getAttribute('href') || '').split('://')[1]?.toUpperCase() || '';
Sub-optimal: querySelector is expensive and should not be used here as we know the structure of the cell.
Potential bug here: split(...)[1] -- technically it might be more then one '://' here (in query string if we decide to display it one day).


I would not approve the code :-P

Re: Search for webmon

Posted: Sat Jul 20, 2024 8:39 am
by ispyisail
I would not approve the code :-P
Me neither :)

ChatGPT is not as good as Pro's yet :)

Re: Search for webmon

Posted: Fri Jul 26, 2024 6:55 am
by Lantis
I've implemented this under https://github.com/ericpaulbishop/gargo ... 444698bd10
It is your code with minor modifications.

I added a 1 second debounce to the interface. It's (in my opinion) the best length tradeoff for responsive UI vs not firing the function on every keypress.
It does fire every ~10 seconds when the underlying table refreshes. That behaviour is only noticeable if you're watching for it and I don't think that is worth doing anything to.

Please let me know if you see any issues. It worked fine in my brief testing.
Thanks again for your contribution!

Re: Search for webmon

Posted: Fri Jul 26, 2024 7:17 am
by fifonik
Thanks a lot.

I've noticed that you did not remove 'row' class from preceding select dropdown (I mentioned that in html comment after the select field).
It does not look right in this case as the select and input are overlapping (I'm checking with default theme).

As for the debounce interval, I'm usually going for lower value (400 or so), but it does not matter, 1000 will do.