Check if three arrays contains the same elementChecking if two byte arrays are the sameTesting if numbers in the array can be added up to equal the largest number in the arrayCheck if 2 arrays have (exactly) the same elements recursivelySum Of Two Arrays element by element and arrays can be of unequal lengthIterating over an object that contains arraysThe difference of two arraysSumming same index values in 2D arrays in javascript
Radar Altimeter in Space Shuttle
Multiple devices with one IPv6 to the Internet?
How to initiate a conversation with a person who recently had transition but you were not in touch with them?
How did the T-850 still function after it removed its second battery?
Meaning of "in arms"
Alternatives to boxes
Using parent's property and will as evidence of assets
Dangers of being sympathetic to the killer
Yarok and Animate Dead
Echo bracket symbol to terminal
Why did Grima shed a tear?
18-month-old kicked out of church nursery
I can be found near gentle green hills and stony mountains
How does the Gameboy Link Cable work?
Where do overtones in a 555 generated square wave come from?
How to help my son improve without being discouraging?
Features of a Coda section
indent and noindent: details from Knuth's The TeXbook
When is the Lie algebra of automorphisms of a geometrical structure finite-dimensional?
Is velocity a valid measure of team and process improvement?
Why is there no “real analytic continuation”
What is the "more" means here?
What does "speed checked" mean?
Can a stolen Android phone with USB debugging enabled have screen lock bypassed?
Check if three arrays contains the same element
Checking if two byte arrays are the sameTesting if numbers in the array can be added up to equal the largest number in the arrayCheck if 2 arrays have (exactly) the same elements recursivelySum Of Two Arrays element by element and arrays can be of unequal lengthIterating over an object that contains arraysThe difference of two arraysSumming same index values in 2D arrays in javascript
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty,.everyoneloves__bot-mid-leaderboard:empty
margin-bottom:0;
$begingroup$
Do 3 arrays contain a common element, if they contain output it.
For example: [1,2,3], [2,3], [2,4] -> answer = 2
let arr1 = [1, 3, 11, 32, 44, 99];
let arr2 = [4, 12, 15, 99];
let arr3 = [4, 11, 13, 15, 23, 43];
function searchThreeSameNum(arr1, arr2, arr3)
let i = 0;
let j = 0;
while (1) j == arr2.length)
return 'No equal numbers';
if (arr1[i] < arr2[j])
i++;
continue;
else if (arr1[i] > arr2[j])
j++;
continue;
else if (arr1[i] == arr2[j])
for (let k = 0; k < arr3.length; k++)
if (arr1[i] == arr3[k])
return arr1[i];
return 'No equal numbers';
I will be glad if you give me any tips to improve my code. Thanks in advance.
Sorry, I'm not an English speaker.
javascript array
$endgroup$
|
show 1 more comment
$begingroup$
Do 3 arrays contain a common element, if they contain output it.
For example: [1,2,3], [2,3], [2,4] -> answer = 2
let arr1 = [1, 3, 11, 32, 44, 99];
let arr2 = [4, 12, 15, 99];
let arr3 = [4, 11, 13, 15, 23, 43];
function searchThreeSameNum(arr1, arr2, arr3)
let i = 0;
let j = 0;
while (1) j == arr2.length)
return 'No equal numbers';
if (arr1[i] < arr2[j])
i++;
continue;
else if (arr1[i] > arr2[j])
j++;
continue;
else if (arr1[i] == arr2[j])
for (let k = 0; k < arr3.length; k++)
if (arr1[i] == arr3[k])
return arr1[i];
return 'No equal numbers';
I will be glad if you give me any tips to improve my code. Thanks in advance.
Sorry, I'm not an English speaker.
javascript array
$endgroup$
1
$begingroup$
Need more clarification. You could start by giving several cases, or at least the answer to this one. And when you say "common element", are you expecting just one answer? What happens if there's more than one? Can there be more than one?
$endgroup$
– Joseph
Jun 5 at 15:12
$begingroup$
What do you mean by "first" common element? The lowest value? Or some metric involving the three indices?
$endgroup$
– Toby Speight
Jun 5 at 15:32
1
$begingroup$
The lowest value
$endgroup$
– Gervenel
Jun 5 at 15:40
$begingroup$
I would rather compare two arrays, then compare that result to the third. I believe such code would be more reusable and easy to adapt to find the same value in 4 arrays. But it would be MUCH less efficient if you generally expect to find the first match of all 3 arrays fairly early.
$endgroup$
– Zizy Archer
Jun 6 at 12:38
1
$begingroup$
I just noticed how freakishly anemic theSet
API in ECMAScript is.
$endgroup$
– Jörg W Mittag
Jun 6 at 19:46
|
show 1 more comment
$begingroup$
Do 3 arrays contain a common element, if they contain output it.
For example: [1,2,3], [2,3], [2,4] -> answer = 2
let arr1 = [1, 3, 11, 32, 44, 99];
let arr2 = [4, 12, 15, 99];
let arr3 = [4, 11, 13, 15, 23, 43];
function searchThreeSameNum(arr1, arr2, arr3)
let i = 0;
let j = 0;
while (1) j == arr2.length)
return 'No equal numbers';
if (arr1[i] < arr2[j])
i++;
continue;
else if (arr1[i] > arr2[j])
j++;
continue;
else if (arr1[i] == arr2[j])
for (let k = 0; k < arr3.length; k++)
if (arr1[i] == arr3[k])
return arr1[i];
return 'No equal numbers';
I will be glad if you give me any tips to improve my code. Thanks in advance.
Sorry, I'm not an English speaker.
javascript array
$endgroup$
Do 3 arrays contain a common element, if they contain output it.
For example: [1,2,3], [2,3], [2,4] -> answer = 2
let arr1 = [1, 3, 11, 32, 44, 99];
let arr2 = [4, 12, 15, 99];
let arr3 = [4, 11, 13, 15, 23, 43];
function searchThreeSameNum(arr1, arr2, arr3)
let i = 0;
let j = 0;
while (1) j == arr2.length)
return 'No equal numbers';
if (arr1[i] < arr2[j])
i++;
continue;
else if (arr1[i] > arr2[j])
j++;
continue;
else if (arr1[i] == arr2[j])
for (let k = 0; k < arr3.length; k++)
if (arr1[i] == arr3[k])
return arr1[i];
return 'No equal numbers';
I will be glad if you give me any tips to improve my code. Thanks in advance.
Sorry, I'm not an English speaker.
javascript array
javascript array
edited Jun 5 at 15:39
Gervenel
asked Jun 5 at 14:51
GervenelGervenel
586 bronze badges
586 bronze badges
1
$begingroup$
Need more clarification. You could start by giving several cases, or at least the answer to this one. And when you say "common element", are you expecting just one answer? What happens if there's more than one? Can there be more than one?
$endgroup$
– Joseph
Jun 5 at 15:12
$begingroup$
What do you mean by "first" common element? The lowest value? Or some metric involving the three indices?
$endgroup$
– Toby Speight
Jun 5 at 15:32
1
$begingroup$
The lowest value
$endgroup$
– Gervenel
Jun 5 at 15:40
$begingroup$
I would rather compare two arrays, then compare that result to the third. I believe such code would be more reusable and easy to adapt to find the same value in 4 arrays. But it would be MUCH less efficient if you generally expect to find the first match of all 3 arrays fairly early.
$endgroup$
– Zizy Archer
Jun 6 at 12:38
1
$begingroup$
I just noticed how freakishly anemic theSet
API in ECMAScript is.
$endgroup$
– Jörg W Mittag
Jun 6 at 19:46
|
show 1 more comment
1
$begingroup$
Need more clarification. You could start by giving several cases, or at least the answer to this one. And when you say "common element", are you expecting just one answer? What happens if there's more than one? Can there be more than one?
$endgroup$
– Joseph
Jun 5 at 15:12
$begingroup$
What do you mean by "first" common element? The lowest value? Or some metric involving the three indices?
$endgroup$
– Toby Speight
Jun 5 at 15:32
1
$begingroup$
The lowest value
$endgroup$
– Gervenel
Jun 5 at 15:40
$begingroup$
I would rather compare two arrays, then compare that result to the third. I believe such code would be more reusable and easy to adapt to find the same value in 4 arrays. But it would be MUCH less efficient if you generally expect to find the first match of all 3 arrays fairly early.
$endgroup$
– Zizy Archer
Jun 6 at 12:38
1
$begingroup$
I just noticed how freakishly anemic theSet
API in ECMAScript is.
$endgroup$
– Jörg W Mittag
Jun 6 at 19:46
1
1
$begingroup$
Need more clarification. You could start by giving several cases, or at least the answer to this one. And when you say "common element", are you expecting just one answer? What happens if there's more than one? Can there be more than one?
$endgroup$
– Joseph
Jun 5 at 15:12
$begingroup$
Need more clarification. You could start by giving several cases, or at least the answer to this one. And when you say "common element", are you expecting just one answer? What happens if there's more than one? Can there be more than one?
$endgroup$
– Joseph
Jun 5 at 15:12
$begingroup$
What do you mean by "first" common element? The lowest value? Or some metric involving the three indices?
$endgroup$
– Toby Speight
Jun 5 at 15:32
$begingroup$
What do you mean by "first" common element? The lowest value? Or some metric involving the three indices?
$endgroup$
– Toby Speight
Jun 5 at 15:32
1
1
$begingroup$
The lowest value
$endgroup$
– Gervenel
Jun 5 at 15:40
$begingroup$
The lowest value
$endgroup$
– Gervenel
Jun 5 at 15:40
$begingroup$
I would rather compare two arrays, then compare that result to the third. I believe such code would be more reusable and easy to adapt to find the same value in 4 arrays. But it would be MUCH less efficient if you generally expect to find the first match of all 3 arrays fairly early.
$endgroup$
– Zizy Archer
Jun 6 at 12:38
$begingroup$
I would rather compare two arrays, then compare that result to the third. I believe such code would be more reusable and easy to adapt to find the same value in 4 arrays. But it would be MUCH less efficient if you generally expect to find the first match of all 3 arrays fairly early.
$endgroup$
– Zizy Archer
Jun 6 at 12:38
1
1
$begingroup$
I just noticed how freakishly anemic the
Set
API in ECMAScript is.$endgroup$
– Jörg W Mittag
Jun 6 at 19:46
$begingroup$
I just noticed how freakishly anemic the
Set
API in ECMAScript is.$endgroup$
– Jörg W Mittag
Jun 6 at 19:46
|
show 1 more comment
5 Answers
5
active
oldest
votes
$begingroup$
Your code assumes that each of the 3 arrays is sorted. Otherwise the <
operator would not work. It's ok to assume this. You should have mentioned this in your question.
You use the ==
operator for comparing the numbers and the lengths. You should better use the ===
since the ==
operator considers 0 and "0"
equal, which is not good in most cases.
It does not matter which of the 3 arrays comes first. The result will always be the same. Therefore it would be nice if the code looked the same for each of the 3 arrays. Your current code looks different for arr3
.
I would write the code differently:
function smallestCommonElement(a, b, c)
let i = 0, j = 0, k = 0;
while (i < a.length && j < b.length && k < c.length)
const max = Math.max(a[i], b[j], c[k]);
let same = true;
while (i < a.length && a[i] < max) i++, same = false;
while (j < b.length && b[j] < max) j++, same = false;
while (k < c.length && c[k] < max) k++, same = false;
if (same)
return a[i];
return null;
The idea is to start at the beginning of the arrays. In each step, look at the current values and find the maximum number. Advance each array to this maximum number. If none of the 3 arrays has been advanced, this means that the current values from all the arrays must be the same. In that case, return this value. Otherwise the values must be different, so try again. Do all this until one of the arrays is at the end, in which case there is no common element.
Looking again at your code, there is a bug. Given the arrays [2, 3], [2, 3], [3]
, your code will return 'No equal number'
even though the 3 appears in each array. Using a debugger (or pen and paper), you should step through your code to see where the bug is.
It's an edge case, and it happens in the part of the code that differs from the other parts. That's why I suggested that the code for all 3 arrays should look the same. It's one less chance of introducing bugs.
$endgroup$
2
$begingroup$
I would changelet n = 0;
tolet same=1;
and instead ofn++;
dosame=0;
. Thenif (n === 0)
becomesif (same)
. This clearly tells what the variable does.
$endgroup$
– Zizy Archer
Jun 6 at 12:56
$begingroup$
@Zizy thanks for the remark, I improved the code.
$endgroup$
– Roland Illig
Jun 6 at 16:09
2
$begingroup$
Math.max
can take more than 2 numbers :)
$endgroup$
– hjpotter92
Jun 6 at 19:06
$begingroup$
@hjpotter92 Thanks for the remark, the code is getting better with each revision :)
$endgroup$
– Roland Illig
Jun 6 at 23:16
add a comment
|
$begingroup$
Although your code seems to work, it is difficult to read. Loops inside loops and many if
, else if
blocks and continue
or return
. Let's start with some big issues:
You use an endless while loop when it is clear you don't have to. The code below would perform the same function:
function searchThreeSameNum(arr1, arr2, arr3)
let i = 0, j = 0;
while (i < arr1.length && j < arr2.length)
if (arr1[i] < arr2[j])
i++;
continue;
else if (arr1[i] > arr2[j])
j++;
continue;
else if (arr1[i] == arr2[j])
for (let k = 0; k < arr3.length; k++)
if (arr1[i] == arr3[k]) return arr1[i];
return 'No equal numbers';
This has one less return 'No equal numbers';
(code repetition) and only one return
from within the while loop.
Now let's look at what the loop is actually doing. It runs through array 1 and 2 and tries to find equal pairs of values in them. If a pair is found it searches in the third array for the same value and returns it when it is found.
There is a handy array method called includes(). It could replace the whole looping of the third array, like this:
function searchThreeSameNum(arr1, arr2, arr3)
let i = 0, j = 0;
while (i < arr1.length && j < arr2.length)
if (arr1[i] < arr2[j])
i++;
continue;
else if (arr1[i] > arr2[j])
j++;
continue;
else if (arr1[i] == arr2[j])
if (arr3.includes(arr1[i])) return arr1[i];
return 'No equal numbers';
I could even go a step further and get rid of the while loop altogether by looping through array 1 and see if its values are contained in array 2 and 3, like this:
function searchThreeSameNum2(arr1, arr2, arr3)
for (number of arr1)
if (arr2.includes(number) && arr3.includes(number)) return number;
return 'No equal numbers';
Note that this simplification makes this function easy to read, but also less efficient. It has to checks array 2 and 3 completely, for every element of array 1, until a match is found.
The assumption, in the function above, is that array 1 is properly sorted. If it isn't you can sort it.
function searchThreeSameNum2(arr1, arr2, arr3)
let sorted = arr1.sort((a, b) => a - b);
for (number of sorted)
if (arr2.includes(number) && arr3.includes(number)) return number;
return 'No equal numbers';
Arrays 2 and 3 don't need to be sorted.
In summary
- If you use a while loop, always break the loop with a proper condition in the right place. Do not use endless loops.
- Try not to use complex execution flow constructions, with lot of
else
,continue
andreturn
. They are difficult to read and to debug. - Use the build in array methods, they are very handy, but don't overdo it.
$endgroup$
2
$begingroup$
Searching the whole ofarr2
andarr3
for each number inarr1
gives youO(N^2)
complexity if they all have equal length. OrO(A * B)
complexity if they're different. (Assuming worst-case where no hit is found). Reducing this toO(A + B +C)
or so is the point of writing an algorithm instead of just using brute force library functions. The OP's code only searchesarr3
for matches inarr1[i] == arr2[j]
. It still has a bad N^2 worst-case, unlike Roland's answer, but only when arr1 and arr2 have a lot of matching elements vs. your always.
$endgroup$
– Peter Cordes
Jun 6 at 1:49
1
$begingroup$
For small problem sizes simplifying to a brute-force algorithm can be appropriate for simplicity readability, and I agree with your comments about it being hard to follow the logic. But your claim "we don't need the complex while loop at all" is an oversimplification that ignores the point of the algorithm the OP was attempting. And BTW, your final version only needsarr1
sorted if you care about finding the smallest same element, rather than a same element.
$endgroup$
– Peter Cordes
Jun 6 at 1:54
$begingroup$
@PeterCordes You're right that, at larger array sizes, the proposed algorithm is less efficient than an algorithm that walks all three arrays. I might indeed have gone a step too far. In this case there is clearly a trade-off between readability and efficiency. I stressed the former because it was lacking in the question, but if the latter is important, a slightly more complex algorithm is probably the beter choice.
$endgroup$
– KIKO Software
Jun 6 at 8:54
$begingroup$
Sure, it's useful to present the last most-readable option. My suggestion is just that you should introduce it accurately, as a less efficient algorithm that's simpler to write and read. For very small arrays it might be more efficient in practice (smaller / simpler code with less overhead). But Roland's algorithm looks very good for even small-ish arrays.
$endgroup$
– Peter Cordes
Jun 6 at 9:32
$begingroup$
I agree, I'll add that into my answer.
$endgroup$
– KIKO Software
Jun 6 at 9:52
|
show 1 more comment
$begingroup$
An easier way is to use JavaScript's Array.includes, Array.some and/or Array.find methods
let arr1 = [1, 3, 11, 32, 44, 99]
let arr2 = [4, 12, 15, 99]
let arr3 = [4, 11, 13, 15, 23, 43]
function searchThreeSameNum (arr1, arr2, arr3)
return arr1.find(number =>
return arr2.includes(number) && arr3.includes(number)
)
const result = searchThreeSameNum(arr1, arr2, arr3)
console.log(result)
$endgroup$
$begingroup$
This is probably the simplest possible answer. I'd write it asconst searchThreeSameNum = (arr1, arr2, arr3) => arr1.find(number => arr2.includes(number) && arr3.includes(number))
though
$endgroup$
– JollyJoker
Jun 7 at 7:51
add a comment
|
$begingroup$
Well, you have one huge bug in the code. You have return …
as the last line of the while(1)
loop. This return
will be hit the moment first arr1
and arr2
elements match but arr3
does not contain that element. I suggest you to use Roland's approach as it works and keeps your general algorithm the same.
$endgroup$
add a comment
|
$begingroup$
Sometimes it is worth generalizing an algorithm to handle an arbitrary number of inputs. Not always, of course, but at least the exercise of thinking about how you would generalize an algorithm can force you to consider whether there is any unnecessary code duplication---this might be the algorithmic analogue of looking for magic constants.
It turns out that, with the exact same number of lines of code, you can make a smallestCommonElement
function that takes any number of arrays as arguments.
This has a time complexity at least as good as the original. Without having done a careful analysis, if there are $k$ arrays of lengths $n_1,dots,n_k$, then it seems to run in $O(n_1 + dots + n_k)$ time. (The index into a given array is guaranteed to increase at least once every two iterations of the main loop.) The original seems to have time complexity $O((n_1 + n_2)n_3)$.
This code uses Roland's idea of using while
loops to step indices forward for each array and Kiko Software's suggestion to avoid the while(1)
.
/* Takes sorted numerical arrays as arguments, returns the smallest
common element between them or null. */
function smallestCommonElement(/*arguments*/)
// Indices into the given arrays
let indices = Array(arguments.length).fill(0);
// The current possible smallest common element
let cur_val = -Infinity;
do
var same = true;
for (let i = 0; i < arguments.length; i++)
// Step an array forward to cur_val (or beyond if the array
// doesn't have cur_val)
while (indices[i] < arguments[i].length && arguments[i][indices[i]] < cur_val)
indices[i]++;
if (indices[i] < arguments[i].length)
if (arguments[i][indices[i]] > cur_val)
// We went past cur_val, so record in 'same' that cur_val does
// not represent the smallest common element this time through.
same = false;
cur_val = arguments[i][indices[i]];
else
// We got to the end of this array, so there is no smallest common element.
return null;
while (!same);
return cur_val;
$endgroup$
add a comment
|
Your Answer
StackExchange.ifUsing("editor", function ()
StackExchange.using("externalEditor", function ()
StackExchange.using("snippets", function ()
StackExchange.snippets.init();
);
);
, "code-snippets");
StackExchange.ready(function()
var channelOptions =
tags: "".split(" "),
id: "196"
;
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function()
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled)
StackExchange.using("snippets", function()
createEditor();
);
else
createEditor();
);
function createEditor()
StackExchange.prepareEditor(
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader:
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/4.0/"u003ecc by-sa 4.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
,
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
);
);
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f221724%2fcheck-if-three-arrays-contains-the-same-element%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
5 Answers
5
active
oldest
votes
5 Answers
5
active
oldest
votes
active
oldest
votes
active
oldest
votes
$begingroup$
Your code assumes that each of the 3 arrays is sorted. Otherwise the <
operator would not work. It's ok to assume this. You should have mentioned this in your question.
You use the ==
operator for comparing the numbers and the lengths. You should better use the ===
since the ==
operator considers 0 and "0"
equal, which is not good in most cases.
It does not matter which of the 3 arrays comes first. The result will always be the same. Therefore it would be nice if the code looked the same for each of the 3 arrays. Your current code looks different for arr3
.
I would write the code differently:
function smallestCommonElement(a, b, c)
let i = 0, j = 0, k = 0;
while (i < a.length && j < b.length && k < c.length)
const max = Math.max(a[i], b[j], c[k]);
let same = true;
while (i < a.length && a[i] < max) i++, same = false;
while (j < b.length && b[j] < max) j++, same = false;
while (k < c.length && c[k] < max) k++, same = false;
if (same)
return a[i];
return null;
The idea is to start at the beginning of the arrays. In each step, look at the current values and find the maximum number. Advance each array to this maximum number. If none of the 3 arrays has been advanced, this means that the current values from all the arrays must be the same. In that case, return this value. Otherwise the values must be different, so try again. Do all this until one of the arrays is at the end, in which case there is no common element.
Looking again at your code, there is a bug. Given the arrays [2, 3], [2, 3], [3]
, your code will return 'No equal number'
even though the 3 appears in each array. Using a debugger (or pen and paper), you should step through your code to see where the bug is.
It's an edge case, and it happens in the part of the code that differs from the other parts. That's why I suggested that the code for all 3 arrays should look the same. It's one less chance of introducing bugs.
$endgroup$
2
$begingroup$
I would changelet n = 0;
tolet same=1;
and instead ofn++;
dosame=0;
. Thenif (n === 0)
becomesif (same)
. This clearly tells what the variable does.
$endgroup$
– Zizy Archer
Jun 6 at 12:56
$begingroup$
@Zizy thanks for the remark, I improved the code.
$endgroup$
– Roland Illig
Jun 6 at 16:09
2
$begingroup$
Math.max
can take more than 2 numbers :)
$endgroup$
– hjpotter92
Jun 6 at 19:06
$begingroup$
@hjpotter92 Thanks for the remark, the code is getting better with each revision :)
$endgroup$
– Roland Illig
Jun 6 at 23:16
add a comment
|
$begingroup$
Your code assumes that each of the 3 arrays is sorted. Otherwise the <
operator would not work. It's ok to assume this. You should have mentioned this in your question.
You use the ==
operator for comparing the numbers and the lengths. You should better use the ===
since the ==
operator considers 0 and "0"
equal, which is not good in most cases.
It does not matter which of the 3 arrays comes first. The result will always be the same. Therefore it would be nice if the code looked the same for each of the 3 arrays. Your current code looks different for arr3
.
I would write the code differently:
function smallestCommonElement(a, b, c)
let i = 0, j = 0, k = 0;
while (i < a.length && j < b.length && k < c.length)
const max = Math.max(a[i], b[j], c[k]);
let same = true;
while (i < a.length && a[i] < max) i++, same = false;
while (j < b.length && b[j] < max) j++, same = false;
while (k < c.length && c[k] < max) k++, same = false;
if (same)
return a[i];
return null;
The idea is to start at the beginning of the arrays. In each step, look at the current values and find the maximum number. Advance each array to this maximum number. If none of the 3 arrays has been advanced, this means that the current values from all the arrays must be the same. In that case, return this value. Otherwise the values must be different, so try again. Do all this until one of the arrays is at the end, in which case there is no common element.
Looking again at your code, there is a bug. Given the arrays [2, 3], [2, 3], [3]
, your code will return 'No equal number'
even though the 3 appears in each array. Using a debugger (or pen and paper), you should step through your code to see where the bug is.
It's an edge case, and it happens in the part of the code that differs from the other parts. That's why I suggested that the code for all 3 arrays should look the same. It's one less chance of introducing bugs.
$endgroup$
2
$begingroup$
I would changelet n = 0;
tolet same=1;
and instead ofn++;
dosame=0;
. Thenif (n === 0)
becomesif (same)
. This clearly tells what the variable does.
$endgroup$
– Zizy Archer
Jun 6 at 12:56
$begingroup$
@Zizy thanks for the remark, I improved the code.
$endgroup$
– Roland Illig
Jun 6 at 16:09
2
$begingroup$
Math.max
can take more than 2 numbers :)
$endgroup$
– hjpotter92
Jun 6 at 19:06
$begingroup$
@hjpotter92 Thanks for the remark, the code is getting better with each revision :)
$endgroup$
– Roland Illig
Jun 6 at 23:16
add a comment
|
$begingroup$
Your code assumes that each of the 3 arrays is sorted. Otherwise the <
operator would not work. It's ok to assume this. You should have mentioned this in your question.
You use the ==
operator for comparing the numbers and the lengths. You should better use the ===
since the ==
operator considers 0 and "0"
equal, which is not good in most cases.
It does not matter which of the 3 arrays comes first. The result will always be the same. Therefore it would be nice if the code looked the same for each of the 3 arrays. Your current code looks different for arr3
.
I would write the code differently:
function smallestCommonElement(a, b, c)
let i = 0, j = 0, k = 0;
while (i < a.length && j < b.length && k < c.length)
const max = Math.max(a[i], b[j], c[k]);
let same = true;
while (i < a.length && a[i] < max) i++, same = false;
while (j < b.length && b[j] < max) j++, same = false;
while (k < c.length && c[k] < max) k++, same = false;
if (same)
return a[i];
return null;
The idea is to start at the beginning of the arrays. In each step, look at the current values and find the maximum number. Advance each array to this maximum number. If none of the 3 arrays has been advanced, this means that the current values from all the arrays must be the same. In that case, return this value. Otherwise the values must be different, so try again. Do all this until one of the arrays is at the end, in which case there is no common element.
Looking again at your code, there is a bug. Given the arrays [2, 3], [2, 3], [3]
, your code will return 'No equal number'
even though the 3 appears in each array. Using a debugger (or pen and paper), you should step through your code to see where the bug is.
It's an edge case, and it happens in the part of the code that differs from the other parts. That's why I suggested that the code for all 3 arrays should look the same. It's one less chance of introducing bugs.
$endgroup$
Your code assumes that each of the 3 arrays is sorted. Otherwise the <
operator would not work. It's ok to assume this. You should have mentioned this in your question.
You use the ==
operator for comparing the numbers and the lengths. You should better use the ===
since the ==
operator considers 0 and "0"
equal, which is not good in most cases.
It does not matter which of the 3 arrays comes first. The result will always be the same. Therefore it would be nice if the code looked the same for each of the 3 arrays. Your current code looks different for arr3
.
I would write the code differently:
function smallestCommonElement(a, b, c)
let i = 0, j = 0, k = 0;
while (i < a.length && j < b.length && k < c.length)
const max = Math.max(a[i], b[j], c[k]);
let same = true;
while (i < a.length && a[i] < max) i++, same = false;
while (j < b.length && b[j] < max) j++, same = false;
while (k < c.length && c[k] < max) k++, same = false;
if (same)
return a[i];
return null;
The idea is to start at the beginning of the arrays. In each step, look at the current values and find the maximum number. Advance each array to this maximum number. If none of the 3 arrays has been advanced, this means that the current values from all the arrays must be the same. In that case, return this value. Otherwise the values must be different, so try again. Do all this until one of the arrays is at the end, in which case there is no common element.
Looking again at your code, there is a bug. Given the arrays [2, 3], [2, 3], [3]
, your code will return 'No equal number'
even though the 3 appears in each array. Using a debugger (or pen and paper), you should step through your code to see where the bug is.
It's an edge case, and it happens in the part of the code that differs from the other parts. That's why I suggested that the code for all 3 arrays should look the same. It's one less chance of introducing bugs.
edited Jun 6 at 23:16
answered Jun 5 at 16:47
Roland IlligRoland Illig
15.4k2 gold badges24 silver badges58 bronze badges
15.4k2 gold badges24 silver badges58 bronze badges
2
$begingroup$
I would changelet n = 0;
tolet same=1;
and instead ofn++;
dosame=0;
. Thenif (n === 0)
becomesif (same)
. This clearly tells what the variable does.
$endgroup$
– Zizy Archer
Jun 6 at 12:56
$begingroup$
@Zizy thanks for the remark, I improved the code.
$endgroup$
– Roland Illig
Jun 6 at 16:09
2
$begingroup$
Math.max
can take more than 2 numbers :)
$endgroup$
– hjpotter92
Jun 6 at 19:06
$begingroup$
@hjpotter92 Thanks for the remark, the code is getting better with each revision :)
$endgroup$
– Roland Illig
Jun 6 at 23:16
add a comment
|
2
$begingroup$
I would changelet n = 0;
tolet same=1;
and instead ofn++;
dosame=0;
. Thenif (n === 0)
becomesif (same)
. This clearly tells what the variable does.
$endgroup$
– Zizy Archer
Jun 6 at 12:56
$begingroup$
@Zizy thanks for the remark, I improved the code.
$endgroup$
– Roland Illig
Jun 6 at 16:09
2
$begingroup$
Math.max
can take more than 2 numbers :)
$endgroup$
– hjpotter92
Jun 6 at 19:06
$begingroup$
@hjpotter92 Thanks for the remark, the code is getting better with each revision :)
$endgroup$
– Roland Illig
Jun 6 at 23:16
2
2
$begingroup$
I would change
let n = 0;
to let same=1;
and instead of n++;
do same=0;
. Then if (n === 0)
becomes if (same)
. This clearly tells what the variable does.$endgroup$
– Zizy Archer
Jun 6 at 12:56
$begingroup$
I would change
let n = 0;
to let same=1;
and instead of n++;
do same=0;
. Then if (n === 0)
becomes if (same)
. This clearly tells what the variable does.$endgroup$
– Zizy Archer
Jun 6 at 12:56
$begingroup$
@Zizy thanks for the remark, I improved the code.
$endgroup$
– Roland Illig
Jun 6 at 16:09
$begingroup$
@Zizy thanks for the remark, I improved the code.
$endgroup$
– Roland Illig
Jun 6 at 16:09
2
2
$begingroup$
Math.max
can take more than 2 numbers :)$endgroup$
– hjpotter92
Jun 6 at 19:06
$begingroup$
Math.max
can take more than 2 numbers :)$endgroup$
– hjpotter92
Jun 6 at 19:06
$begingroup$
@hjpotter92 Thanks for the remark, the code is getting better with each revision :)
$endgroup$
– Roland Illig
Jun 6 at 23:16
$begingroup$
@hjpotter92 Thanks for the remark, the code is getting better with each revision :)
$endgroup$
– Roland Illig
Jun 6 at 23:16
add a comment
|
$begingroup$
Although your code seems to work, it is difficult to read. Loops inside loops and many if
, else if
blocks and continue
or return
. Let's start with some big issues:
You use an endless while loop when it is clear you don't have to. The code below would perform the same function:
function searchThreeSameNum(arr1, arr2, arr3)
let i = 0, j = 0;
while (i < arr1.length && j < arr2.length)
if (arr1[i] < arr2[j])
i++;
continue;
else if (arr1[i] > arr2[j])
j++;
continue;
else if (arr1[i] == arr2[j])
for (let k = 0; k < arr3.length; k++)
if (arr1[i] == arr3[k]) return arr1[i];
return 'No equal numbers';
This has one less return 'No equal numbers';
(code repetition) and only one return
from within the while loop.
Now let's look at what the loop is actually doing. It runs through array 1 and 2 and tries to find equal pairs of values in them. If a pair is found it searches in the third array for the same value and returns it when it is found.
There is a handy array method called includes(). It could replace the whole looping of the third array, like this:
function searchThreeSameNum(arr1, arr2, arr3)
let i = 0, j = 0;
while (i < arr1.length && j < arr2.length)
if (arr1[i] < arr2[j])
i++;
continue;
else if (arr1[i] > arr2[j])
j++;
continue;
else if (arr1[i] == arr2[j])
if (arr3.includes(arr1[i])) return arr1[i];
return 'No equal numbers';
I could even go a step further and get rid of the while loop altogether by looping through array 1 and see if its values are contained in array 2 and 3, like this:
function searchThreeSameNum2(arr1, arr2, arr3)
for (number of arr1)
if (arr2.includes(number) && arr3.includes(number)) return number;
return 'No equal numbers';
Note that this simplification makes this function easy to read, but also less efficient. It has to checks array 2 and 3 completely, for every element of array 1, until a match is found.
The assumption, in the function above, is that array 1 is properly sorted. If it isn't you can sort it.
function searchThreeSameNum2(arr1, arr2, arr3)
let sorted = arr1.sort((a, b) => a - b);
for (number of sorted)
if (arr2.includes(number) && arr3.includes(number)) return number;
return 'No equal numbers';
Arrays 2 and 3 don't need to be sorted.
In summary
- If you use a while loop, always break the loop with a proper condition in the right place. Do not use endless loops.
- Try not to use complex execution flow constructions, with lot of
else
,continue
andreturn
. They are difficult to read and to debug. - Use the build in array methods, they are very handy, but don't overdo it.
$endgroup$
2
$begingroup$
Searching the whole ofarr2
andarr3
for each number inarr1
gives youO(N^2)
complexity if they all have equal length. OrO(A * B)
complexity if they're different. (Assuming worst-case where no hit is found). Reducing this toO(A + B +C)
or so is the point of writing an algorithm instead of just using brute force library functions. The OP's code only searchesarr3
for matches inarr1[i] == arr2[j]
. It still has a bad N^2 worst-case, unlike Roland's answer, but only when arr1 and arr2 have a lot of matching elements vs. your always.
$endgroup$
– Peter Cordes
Jun 6 at 1:49
1
$begingroup$
For small problem sizes simplifying to a brute-force algorithm can be appropriate for simplicity readability, and I agree with your comments about it being hard to follow the logic. But your claim "we don't need the complex while loop at all" is an oversimplification that ignores the point of the algorithm the OP was attempting. And BTW, your final version only needsarr1
sorted if you care about finding the smallest same element, rather than a same element.
$endgroup$
– Peter Cordes
Jun 6 at 1:54
$begingroup$
@PeterCordes You're right that, at larger array sizes, the proposed algorithm is less efficient than an algorithm that walks all three arrays. I might indeed have gone a step too far. In this case there is clearly a trade-off between readability and efficiency. I stressed the former because it was lacking in the question, but if the latter is important, a slightly more complex algorithm is probably the beter choice.
$endgroup$
– KIKO Software
Jun 6 at 8:54
$begingroup$
Sure, it's useful to present the last most-readable option. My suggestion is just that you should introduce it accurately, as a less efficient algorithm that's simpler to write and read. For very small arrays it might be more efficient in practice (smaller / simpler code with less overhead). But Roland's algorithm looks very good for even small-ish arrays.
$endgroup$
– Peter Cordes
Jun 6 at 9:32
$begingroup$
I agree, I'll add that into my answer.
$endgroup$
– KIKO Software
Jun 6 at 9:52
|
show 1 more comment
$begingroup$
Although your code seems to work, it is difficult to read. Loops inside loops and many if
, else if
blocks and continue
or return
. Let's start with some big issues:
You use an endless while loop when it is clear you don't have to. The code below would perform the same function:
function searchThreeSameNum(arr1, arr2, arr3)
let i = 0, j = 0;
while (i < arr1.length && j < arr2.length)
if (arr1[i] < arr2[j])
i++;
continue;
else if (arr1[i] > arr2[j])
j++;
continue;
else if (arr1[i] == arr2[j])
for (let k = 0; k < arr3.length; k++)
if (arr1[i] == arr3[k]) return arr1[i];
return 'No equal numbers';
This has one less return 'No equal numbers';
(code repetition) and only one return
from within the while loop.
Now let's look at what the loop is actually doing. It runs through array 1 and 2 and tries to find equal pairs of values in them. If a pair is found it searches in the third array for the same value and returns it when it is found.
There is a handy array method called includes(). It could replace the whole looping of the third array, like this:
function searchThreeSameNum(arr1, arr2, arr3)
let i = 0, j = 0;
while (i < arr1.length && j < arr2.length)
if (arr1[i] < arr2[j])
i++;
continue;
else if (arr1[i] > arr2[j])
j++;
continue;
else if (arr1[i] == arr2[j])
if (arr3.includes(arr1[i])) return arr1[i];
return 'No equal numbers';
I could even go a step further and get rid of the while loop altogether by looping through array 1 and see if its values are contained in array 2 and 3, like this:
function searchThreeSameNum2(arr1, arr2, arr3)
for (number of arr1)
if (arr2.includes(number) && arr3.includes(number)) return number;
return 'No equal numbers';
Note that this simplification makes this function easy to read, but also less efficient. It has to checks array 2 and 3 completely, for every element of array 1, until a match is found.
The assumption, in the function above, is that array 1 is properly sorted. If it isn't you can sort it.
function searchThreeSameNum2(arr1, arr2, arr3)
let sorted = arr1.sort((a, b) => a - b);
for (number of sorted)
if (arr2.includes(number) && arr3.includes(number)) return number;
return 'No equal numbers';
Arrays 2 and 3 don't need to be sorted.
In summary
- If you use a while loop, always break the loop with a proper condition in the right place. Do not use endless loops.
- Try not to use complex execution flow constructions, with lot of
else
,continue
andreturn
. They are difficult to read and to debug. - Use the build in array methods, they are very handy, but don't overdo it.
$endgroup$
2
$begingroup$
Searching the whole ofarr2
andarr3
for each number inarr1
gives youO(N^2)
complexity if they all have equal length. OrO(A * B)
complexity if they're different. (Assuming worst-case where no hit is found). Reducing this toO(A + B +C)
or so is the point of writing an algorithm instead of just using brute force library functions. The OP's code only searchesarr3
for matches inarr1[i] == arr2[j]
. It still has a bad N^2 worst-case, unlike Roland's answer, but only when arr1 and arr2 have a lot of matching elements vs. your always.
$endgroup$
– Peter Cordes
Jun 6 at 1:49
1
$begingroup$
For small problem sizes simplifying to a brute-force algorithm can be appropriate for simplicity readability, and I agree with your comments about it being hard to follow the logic. But your claim "we don't need the complex while loop at all" is an oversimplification that ignores the point of the algorithm the OP was attempting. And BTW, your final version only needsarr1
sorted if you care about finding the smallest same element, rather than a same element.
$endgroup$
– Peter Cordes
Jun 6 at 1:54
$begingroup$
@PeterCordes You're right that, at larger array sizes, the proposed algorithm is less efficient than an algorithm that walks all three arrays. I might indeed have gone a step too far. In this case there is clearly a trade-off between readability and efficiency. I stressed the former because it was lacking in the question, but if the latter is important, a slightly more complex algorithm is probably the beter choice.
$endgroup$
– KIKO Software
Jun 6 at 8:54
$begingroup$
Sure, it's useful to present the last most-readable option. My suggestion is just that you should introduce it accurately, as a less efficient algorithm that's simpler to write and read. For very small arrays it might be more efficient in practice (smaller / simpler code with less overhead). But Roland's algorithm looks very good for even small-ish arrays.
$endgroup$
– Peter Cordes
Jun 6 at 9:32
$begingroup$
I agree, I'll add that into my answer.
$endgroup$
– KIKO Software
Jun 6 at 9:52
|
show 1 more comment
$begingroup$
Although your code seems to work, it is difficult to read. Loops inside loops and many if
, else if
blocks and continue
or return
. Let's start with some big issues:
You use an endless while loop when it is clear you don't have to. The code below would perform the same function:
function searchThreeSameNum(arr1, arr2, arr3)
let i = 0, j = 0;
while (i < arr1.length && j < arr2.length)
if (arr1[i] < arr2[j])
i++;
continue;
else if (arr1[i] > arr2[j])
j++;
continue;
else if (arr1[i] == arr2[j])
for (let k = 0; k < arr3.length; k++)
if (arr1[i] == arr3[k]) return arr1[i];
return 'No equal numbers';
This has one less return 'No equal numbers';
(code repetition) and only one return
from within the while loop.
Now let's look at what the loop is actually doing. It runs through array 1 and 2 and tries to find equal pairs of values in them. If a pair is found it searches in the third array for the same value and returns it when it is found.
There is a handy array method called includes(). It could replace the whole looping of the third array, like this:
function searchThreeSameNum(arr1, arr2, arr3)
let i = 0, j = 0;
while (i < arr1.length && j < arr2.length)
if (arr1[i] < arr2[j])
i++;
continue;
else if (arr1[i] > arr2[j])
j++;
continue;
else if (arr1[i] == arr2[j])
if (arr3.includes(arr1[i])) return arr1[i];
return 'No equal numbers';
I could even go a step further and get rid of the while loop altogether by looping through array 1 and see if its values are contained in array 2 and 3, like this:
function searchThreeSameNum2(arr1, arr2, arr3)
for (number of arr1)
if (arr2.includes(number) && arr3.includes(number)) return number;
return 'No equal numbers';
Note that this simplification makes this function easy to read, but also less efficient. It has to checks array 2 and 3 completely, for every element of array 1, until a match is found.
The assumption, in the function above, is that array 1 is properly sorted. If it isn't you can sort it.
function searchThreeSameNum2(arr1, arr2, arr3)
let sorted = arr1.sort((a, b) => a - b);
for (number of sorted)
if (arr2.includes(number) && arr3.includes(number)) return number;
return 'No equal numbers';
Arrays 2 and 3 don't need to be sorted.
In summary
- If you use a while loop, always break the loop with a proper condition in the right place. Do not use endless loops.
- Try not to use complex execution flow constructions, with lot of
else
,continue
andreturn
. They are difficult to read and to debug. - Use the build in array methods, they are very handy, but don't overdo it.
$endgroup$
Although your code seems to work, it is difficult to read. Loops inside loops and many if
, else if
blocks and continue
or return
. Let's start with some big issues:
You use an endless while loop when it is clear you don't have to. The code below would perform the same function:
function searchThreeSameNum(arr1, arr2, arr3)
let i = 0, j = 0;
while (i < arr1.length && j < arr2.length)
if (arr1[i] < arr2[j])
i++;
continue;
else if (arr1[i] > arr2[j])
j++;
continue;
else if (arr1[i] == arr2[j])
for (let k = 0; k < arr3.length; k++)
if (arr1[i] == arr3[k]) return arr1[i];
return 'No equal numbers';
This has one less return 'No equal numbers';
(code repetition) and only one return
from within the while loop.
Now let's look at what the loop is actually doing. It runs through array 1 and 2 and tries to find equal pairs of values in them. If a pair is found it searches in the third array for the same value and returns it when it is found.
There is a handy array method called includes(). It could replace the whole looping of the third array, like this:
function searchThreeSameNum(arr1, arr2, arr3)
let i = 0, j = 0;
while (i < arr1.length && j < arr2.length)
if (arr1[i] < arr2[j])
i++;
continue;
else if (arr1[i] > arr2[j])
j++;
continue;
else if (arr1[i] == arr2[j])
if (arr3.includes(arr1[i])) return arr1[i];
return 'No equal numbers';
I could even go a step further and get rid of the while loop altogether by looping through array 1 and see if its values are contained in array 2 and 3, like this:
function searchThreeSameNum2(arr1, arr2, arr3)
for (number of arr1)
if (arr2.includes(number) && arr3.includes(number)) return number;
return 'No equal numbers';
Note that this simplification makes this function easy to read, but also less efficient. It has to checks array 2 and 3 completely, for every element of array 1, until a match is found.
The assumption, in the function above, is that array 1 is properly sorted. If it isn't you can sort it.
function searchThreeSameNum2(arr1, arr2, arr3)
let sorted = arr1.sort((a, b) => a - b);
for (number of sorted)
if (arr2.includes(number) && arr3.includes(number)) return number;
return 'No equal numbers';
Arrays 2 and 3 don't need to be sorted.
In summary
- If you use a while loop, always break the loop with a proper condition in the right place. Do not use endless loops.
- Try not to use complex execution flow constructions, with lot of
else
,continue
andreturn
. They are difficult to read and to debug. - Use the build in array methods, they are very handy, but don't overdo it.
edited Jun 6 at 9:59
answered Jun 5 at 16:44
KIKO SoftwareKIKO Software
3,9937 silver badges17 bronze badges
3,9937 silver badges17 bronze badges
2
$begingroup$
Searching the whole ofarr2
andarr3
for each number inarr1
gives youO(N^2)
complexity if they all have equal length. OrO(A * B)
complexity if they're different. (Assuming worst-case where no hit is found). Reducing this toO(A + B +C)
or so is the point of writing an algorithm instead of just using brute force library functions. The OP's code only searchesarr3
for matches inarr1[i] == arr2[j]
. It still has a bad N^2 worst-case, unlike Roland's answer, but only when arr1 and arr2 have a lot of matching elements vs. your always.
$endgroup$
– Peter Cordes
Jun 6 at 1:49
1
$begingroup$
For small problem sizes simplifying to a brute-force algorithm can be appropriate for simplicity readability, and I agree with your comments about it being hard to follow the logic. But your claim "we don't need the complex while loop at all" is an oversimplification that ignores the point of the algorithm the OP was attempting. And BTW, your final version only needsarr1
sorted if you care about finding the smallest same element, rather than a same element.
$endgroup$
– Peter Cordes
Jun 6 at 1:54
$begingroup$
@PeterCordes You're right that, at larger array sizes, the proposed algorithm is less efficient than an algorithm that walks all three arrays. I might indeed have gone a step too far. In this case there is clearly a trade-off between readability and efficiency. I stressed the former because it was lacking in the question, but if the latter is important, a slightly more complex algorithm is probably the beter choice.
$endgroup$
– KIKO Software
Jun 6 at 8:54
$begingroup$
Sure, it's useful to present the last most-readable option. My suggestion is just that you should introduce it accurately, as a less efficient algorithm that's simpler to write and read. For very small arrays it might be more efficient in practice (smaller / simpler code with less overhead). But Roland's algorithm looks very good for even small-ish arrays.
$endgroup$
– Peter Cordes
Jun 6 at 9:32
$begingroup$
I agree, I'll add that into my answer.
$endgroup$
– KIKO Software
Jun 6 at 9:52
|
show 1 more comment
2
$begingroup$
Searching the whole ofarr2
andarr3
for each number inarr1
gives youO(N^2)
complexity if they all have equal length. OrO(A * B)
complexity if they're different. (Assuming worst-case where no hit is found). Reducing this toO(A + B +C)
or so is the point of writing an algorithm instead of just using brute force library functions. The OP's code only searchesarr3
for matches inarr1[i] == arr2[j]
. It still has a bad N^2 worst-case, unlike Roland's answer, but only when arr1 and arr2 have a lot of matching elements vs. your always.
$endgroup$
– Peter Cordes
Jun 6 at 1:49
1
$begingroup$
For small problem sizes simplifying to a brute-force algorithm can be appropriate for simplicity readability, and I agree with your comments about it being hard to follow the logic. But your claim "we don't need the complex while loop at all" is an oversimplification that ignores the point of the algorithm the OP was attempting. And BTW, your final version only needsarr1
sorted if you care about finding the smallest same element, rather than a same element.
$endgroup$
– Peter Cordes
Jun 6 at 1:54
$begingroup$
@PeterCordes You're right that, at larger array sizes, the proposed algorithm is less efficient than an algorithm that walks all three arrays. I might indeed have gone a step too far. In this case there is clearly a trade-off between readability and efficiency. I stressed the former because it was lacking in the question, but if the latter is important, a slightly more complex algorithm is probably the beter choice.
$endgroup$
– KIKO Software
Jun 6 at 8:54
$begingroup$
Sure, it's useful to present the last most-readable option. My suggestion is just that you should introduce it accurately, as a less efficient algorithm that's simpler to write and read. For very small arrays it might be more efficient in practice (smaller / simpler code with less overhead). But Roland's algorithm looks very good for even small-ish arrays.
$endgroup$
– Peter Cordes
Jun 6 at 9:32
$begingroup$
I agree, I'll add that into my answer.
$endgroup$
– KIKO Software
Jun 6 at 9:52
2
2
$begingroup$
Searching the whole of
arr2
and arr3
for each number in arr1
gives you O(N^2)
complexity if they all have equal length. Or O(A * B)
complexity if they're different. (Assuming worst-case where no hit is found). Reducing this to O(A + B +C)
or so is the point of writing an algorithm instead of just using brute force library functions. The OP's code only searches arr3
for matches in arr1[i] == arr2[j]
. It still has a bad N^2 worst-case, unlike Roland's answer, but only when arr1 and arr2 have a lot of matching elements vs. your always.$endgroup$
– Peter Cordes
Jun 6 at 1:49
$begingroup$
Searching the whole of
arr2
and arr3
for each number in arr1
gives you O(N^2)
complexity if they all have equal length. Or O(A * B)
complexity if they're different. (Assuming worst-case where no hit is found). Reducing this to O(A + B +C)
or so is the point of writing an algorithm instead of just using brute force library functions. The OP's code only searches arr3
for matches in arr1[i] == arr2[j]
. It still has a bad N^2 worst-case, unlike Roland's answer, but only when arr1 and arr2 have a lot of matching elements vs. your always.$endgroup$
– Peter Cordes
Jun 6 at 1:49
1
1
$begingroup$
For small problem sizes simplifying to a brute-force algorithm can be appropriate for simplicity readability, and I agree with your comments about it being hard to follow the logic. But your claim "we don't need the complex while loop at all" is an oversimplification that ignores the point of the algorithm the OP was attempting. And BTW, your final version only needs
arr1
sorted if you care about finding the smallest same element, rather than a same element.$endgroup$
– Peter Cordes
Jun 6 at 1:54
$begingroup$
For small problem sizes simplifying to a brute-force algorithm can be appropriate for simplicity readability, and I agree with your comments about it being hard to follow the logic. But your claim "we don't need the complex while loop at all" is an oversimplification that ignores the point of the algorithm the OP was attempting. And BTW, your final version only needs
arr1
sorted if you care about finding the smallest same element, rather than a same element.$endgroup$
– Peter Cordes
Jun 6 at 1:54
$begingroup$
@PeterCordes You're right that, at larger array sizes, the proposed algorithm is less efficient than an algorithm that walks all three arrays. I might indeed have gone a step too far. In this case there is clearly a trade-off between readability and efficiency. I stressed the former because it was lacking in the question, but if the latter is important, a slightly more complex algorithm is probably the beter choice.
$endgroup$
– KIKO Software
Jun 6 at 8:54
$begingroup$
@PeterCordes You're right that, at larger array sizes, the proposed algorithm is less efficient than an algorithm that walks all three arrays. I might indeed have gone a step too far. In this case there is clearly a trade-off between readability and efficiency. I stressed the former because it was lacking in the question, but if the latter is important, a slightly more complex algorithm is probably the beter choice.
$endgroup$
– KIKO Software
Jun 6 at 8:54
$begingroup$
Sure, it's useful to present the last most-readable option. My suggestion is just that you should introduce it accurately, as a less efficient algorithm that's simpler to write and read. For very small arrays it might be more efficient in practice (smaller / simpler code with less overhead). But Roland's algorithm looks very good for even small-ish arrays.
$endgroup$
– Peter Cordes
Jun 6 at 9:32
$begingroup$
Sure, it's useful to present the last most-readable option. My suggestion is just that you should introduce it accurately, as a less efficient algorithm that's simpler to write and read. For very small arrays it might be more efficient in practice (smaller / simpler code with less overhead). But Roland's algorithm looks very good for even small-ish arrays.
$endgroup$
– Peter Cordes
Jun 6 at 9:32
$begingroup$
I agree, I'll add that into my answer.
$endgroup$
– KIKO Software
Jun 6 at 9:52
$begingroup$
I agree, I'll add that into my answer.
$endgroup$
– KIKO Software
Jun 6 at 9:52
|
show 1 more comment
$begingroup$
An easier way is to use JavaScript's Array.includes, Array.some and/or Array.find methods
let arr1 = [1, 3, 11, 32, 44, 99]
let arr2 = [4, 12, 15, 99]
let arr3 = [4, 11, 13, 15, 23, 43]
function searchThreeSameNum (arr1, arr2, arr3)
return arr1.find(number =>
return arr2.includes(number) && arr3.includes(number)
)
const result = searchThreeSameNum(arr1, arr2, arr3)
console.log(result)
$endgroup$
$begingroup$
This is probably the simplest possible answer. I'd write it asconst searchThreeSameNum = (arr1, arr2, arr3) => arr1.find(number => arr2.includes(number) && arr3.includes(number))
though
$endgroup$
– JollyJoker
Jun 7 at 7:51
add a comment
|
$begingroup$
An easier way is to use JavaScript's Array.includes, Array.some and/or Array.find methods
let arr1 = [1, 3, 11, 32, 44, 99]
let arr2 = [4, 12, 15, 99]
let arr3 = [4, 11, 13, 15, 23, 43]
function searchThreeSameNum (arr1, arr2, arr3)
return arr1.find(number =>
return arr2.includes(number) && arr3.includes(number)
)
const result = searchThreeSameNum(arr1, arr2, arr3)
console.log(result)
$endgroup$
$begingroup$
This is probably the simplest possible answer. I'd write it asconst searchThreeSameNum = (arr1, arr2, arr3) => arr1.find(number => arr2.includes(number) && arr3.includes(number))
though
$endgroup$
– JollyJoker
Jun 7 at 7:51
add a comment
|
$begingroup$
An easier way is to use JavaScript's Array.includes, Array.some and/or Array.find methods
let arr1 = [1, 3, 11, 32, 44, 99]
let arr2 = [4, 12, 15, 99]
let arr3 = [4, 11, 13, 15, 23, 43]
function searchThreeSameNum (arr1, arr2, arr3)
return arr1.find(number =>
return arr2.includes(number) && arr3.includes(number)
)
const result = searchThreeSameNum(arr1, arr2, arr3)
console.log(result)
$endgroup$
An easier way is to use JavaScript's Array.includes, Array.some and/or Array.find methods
let arr1 = [1, 3, 11, 32, 44, 99]
let arr2 = [4, 12, 15, 99]
let arr3 = [4, 11, 13, 15, 23, 43]
function searchThreeSameNum (arr1, arr2, arr3)
return arr1.find(number =>
return arr2.includes(number) && arr3.includes(number)
)
const result = searchThreeSameNum(arr1, arr2, arr3)
console.log(result)
answered Jun 6 at 14:10
Khaled OsmanKhaled Osman
1512 bronze badges
1512 bronze badges
$begingroup$
This is probably the simplest possible answer. I'd write it asconst searchThreeSameNum = (arr1, arr2, arr3) => arr1.find(number => arr2.includes(number) && arr3.includes(number))
though
$endgroup$
– JollyJoker
Jun 7 at 7:51
add a comment
|
$begingroup$
This is probably the simplest possible answer. I'd write it asconst searchThreeSameNum = (arr1, arr2, arr3) => arr1.find(number => arr2.includes(number) && arr3.includes(number))
though
$endgroup$
– JollyJoker
Jun 7 at 7:51
$begingroup$
This is probably the simplest possible answer. I'd write it as
const searchThreeSameNum = (arr1, arr2, arr3) => arr1.find(number => arr2.includes(number) && arr3.includes(number))
though$endgroup$
– JollyJoker
Jun 7 at 7:51
$begingroup$
This is probably the simplest possible answer. I'd write it as
const searchThreeSameNum = (arr1, arr2, arr3) => arr1.find(number => arr2.includes(number) && arr3.includes(number))
though$endgroup$
– JollyJoker
Jun 7 at 7:51
add a comment
|
$begingroup$
Well, you have one huge bug in the code. You have return …
as the last line of the while(1)
loop. This return
will be hit the moment first arr1
and arr2
elements match but arr3
does not contain that element. I suggest you to use Roland's approach as it works and keeps your general algorithm the same.
$endgroup$
add a comment
|
$begingroup$
Well, you have one huge bug in the code. You have return …
as the last line of the while(1)
loop. This return
will be hit the moment first arr1
and arr2
elements match but arr3
does not contain that element. I suggest you to use Roland's approach as it works and keeps your general algorithm the same.
$endgroup$
add a comment
|
$begingroup$
Well, you have one huge bug in the code. You have return …
as the last line of the while(1)
loop. This return
will be hit the moment first arr1
and arr2
elements match but arr3
does not contain that element. I suggest you to use Roland's approach as it works and keeps your general algorithm the same.
$endgroup$
Well, you have one huge bug in the code. You have return …
as the last line of the while(1)
loop. This return
will be hit the moment first arr1
and arr2
elements match but arr3
does not contain that element. I suggest you to use Roland's approach as it works and keeps your general algorithm the same.
answered Jun 6 at 12:51
Zizy ArcherZizy Archer
1411 bronze badge
1411 bronze badge
add a comment
|
add a comment
|
$begingroup$
Sometimes it is worth generalizing an algorithm to handle an arbitrary number of inputs. Not always, of course, but at least the exercise of thinking about how you would generalize an algorithm can force you to consider whether there is any unnecessary code duplication---this might be the algorithmic analogue of looking for magic constants.
It turns out that, with the exact same number of lines of code, you can make a smallestCommonElement
function that takes any number of arrays as arguments.
This has a time complexity at least as good as the original. Without having done a careful analysis, if there are $k$ arrays of lengths $n_1,dots,n_k$, then it seems to run in $O(n_1 + dots + n_k)$ time. (The index into a given array is guaranteed to increase at least once every two iterations of the main loop.) The original seems to have time complexity $O((n_1 + n_2)n_3)$.
This code uses Roland's idea of using while
loops to step indices forward for each array and Kiko Software's suggestion to avoid the while(1)
.
/* Takes sorted numerical arrays as arguments, returns the smallest
common element between them or null. */
function smallestCommonElement(/*arguments*/)
// Indices into the given arrays
let indices = Array(arguments.length).fill(0);
// The current possible smallest common element
let cur_val = -Infinity;
do
var same = true;
for (let i = 0; i < arguments.length; i++)
// Step an array forward to cur_val (or beyond if the array
// doesn't have cur_val)
while (indices[i] < arguments[i].length && arguments[i][indices[i]] < cur_val)
indices[i]++;
if (indices[i] < arguments[i].length)
if (arguments[i][indices[i]] > cur_val)
// We went past cur_val, so record in 'same' that cur_val does
// not represent the smallest common element this time through.
same = false;
cur_val = arguments[i][indices[i]];
else
// We got to the end of this array, so there is no smallest common element.
return null;
while (!same);
return cur_val;
$endgroup$
add a comment
|
$begingroup$
Sometimes it is worth generalizing an algorithm to handle an arbitrary number of inputs. Not always, of course, but at least the exercise of thinking about how you would generalize an algorithm can force you to consider whether there is any unnecessary code duplication---this might be the algorithmic analogue of looking for magic constants.
It turns out that, with the exact same number of lines of code, you can make a smallestCommonElement
function that takes any number of arrays as arguments.
This has a time complexity at least as good as the original. Without having done a careful analysis, if there are $k$ arrays of lengths $n_1,dots,n_k$, then it seems to run in $O(n_1 + dots + n_k)$ time. (The index into a given array is guaranteed to increase at least once every two iterations of the main loop.) The original seems to have time complexity $O((n_1 + n_2)n_3)$.
This code uses Roland's idea of using while
loops to step indices forward for each array and Kiko Software's suggestion to avoid the while(1)
.
/* Takes sorted numerical arrays as arguments, returns the smallest
common element between them or null. */
function smallestCommonElement(/*arguments*/)
// Indices into the given arrays
let indices = Array(arguments.length).fill(0);
// The current possible smallest common element
let cur_val = -Infinity;
do
var same = true;
for (let i = 0; i < arguments.length; i++)
// Step an array forward to cur_val (or beyond if the array
// doesn't have cur_val)
while (indices[i] < arguments[i].length && arguments[i][indices[i]] < cur_val)
indices[i]++;
if (indices[i] < arguments[i].length)
if (arguments[i][indices[i]] > cur_val)
// We went past cur_val, so record in 'same' that cur_val does
// not represent the smallest common element this time through.
same = false;
cur_val = arguments[i][indices[i]];
else
// We got to the end of this array, so there is no smallest common element.
return null;
while (!same);
return cur_val;
$endgroup$
add a comment
|
$begingroup$
Sometimes it is worth generalizing an algorithm to handle an arbitrary number of inputs. Not always, of course, but at least the exercise of thinking about how you would generalize an algorithm can force you to consider whether there is any unnecessary code duplication---this might be the algorithmic analogue of looking for magic constants.
It turns out that, with the exact same number of lines of code, you can make a smallestCommonElement
function that takes any number of arrays as arguments.
This has a time complexity at least as good as the original. Without having done a careful analysis, if there are $k$ arrays of lengths $n_1,dots,n_k$, then it seems to run in $O(n_1 + dots + n_k)$ time. (The index into a given array is guaranteed to increase at least once every two iterations of the main loop.) The original seems to have time complexity $O((n_1 + n_2)n_3)$.
This code uses Roland's idea of using while
loops to step indices forward for each array and Kiko Software's suggestion to avoid the while(1)
.
/* Takes sorted numerical arrays as arguments, returns the smallest
common element between them or null. */
function smallestCommonElement(/*arguments*/)
// Indices into the given arrays
let indices = Array(arguments.length).fill(0);
// The current possible smallest common element
let cur_val = -Infinity;
do
var same = true;
for (let i = 0; i < arguments.length; i++)
// Step an array forward to cur_val (or beyond if the array
// doesn't have cur_val)
while (indices[i] < arguments[i].length && arguments[i][indices[i]] < cur_val)
indices[i]++;
if (indices[i] < arguments[i].length)
if (arguments[i][indices[i]] > cur_val)
// We went past cur_val, so record in 'same' that cur_val does
// not represent the smallest common element this time through.
same = false;
cur_val = arguments[i][indices[i]];
else
// We got to the end of this array, so there is no smallest common element.
return null;
while (!same);
return cur_val;
$endgroup$
Sometimes it is worth generalizing an algorithm to handle an arbitrary number of inputs. Not always, of course, but at least the exercise of thinking about how you would generalize an algorithm can force you to consider whether there is any unnecessary code duplication---this might be the algorithmic analogue of looking for magic constants.
It turns out that, with the exact same number of lines of code, you can make a smallestCommonElement
function that takes any number of arrays as arguments.
This has a time complexity at least as good as the original. Without having done a careful analysis, if there are $k$ arrays of lengths $n_1,dots,n_k$, then it seems to run in $O(n_1 + dots + n_k)$ time. (The index into a given array is guaranteed to increase at least once every two iterations of the main loop.) The original seems to have time complexity $O((n_1 + n_2)n_3)$.
This code uses Roland's idea of using while
loops to step indices forward for each array and Kiko Software's suggestion to avoid the while(1)
.
/* Takes sorted numerical arrays as arguments, returns the smallest
common element between them or null. */
function smallestCommonElement(/*arguments*/)
// Indices into the given arrays
let indices = Array(arguments.length).fill(0);
// The current possible smallest common element
let cur_val = -Infinity;
do
var same = true;
for (let i = 0; i < arguments.length; i++)
// Step an array forward to cur_val (or beyond if the array
// doesn't have cur_val)
while (indices[i] < arguments[i].length && arguments[i][indices[i]] < cur_val)
indices[i]++;
if (indices[i] < arguments[i].length)
if (arguments[i][indices[i]] > cur_val)
// We went past cur_val, so record in 'same' that cur_val does
// not represent the smallest common element this time through.
same = false;
cur_val = arguments[i][indices[i]];
else
// We got to the end of this array, so there is no smallest common element.
return null;
while (!same);
return cur_val;
/* Takes sorted numerical arrays as arguments, returns the smallest
common element between them or null. */
function smallestCommonElement(/*arguments*/)
// Indices into the given arrays
let indices = Array(arguments.length).fill(0);
// The current possible smallest common element
let cur_val = -Infinity;
do
var same = true;
for (let i = 0; i < arguments.length; i++)
// Step an array forward to cur_val (or beyond if the array
// doesn't have cur_val)
while (indices[i] < arguments[i].length && arguments[i][indices[i]] < cur_val)
indices[i]++;
if (indices[i] < arguments[i].length)
if (arguments[i][indices[i]] > cur_val)
// We went past cur_val, so record in 'same' that cur_val does
// not represent the smallest common element this time through.
same = false;
cur_val = arguments[i][indices[i]];
else
// We got to the end of this array, so there is no smallest common element.
return null;
while (!same);
return cur_val;
/* Takes sorted numerical arrays as arguments, returns the smallest
common element between them or null. */
function smallestCommonElement(/*arguments*/)
// Indices into the given arrays
let indices = Array(arguments.length).fill(0);
// The current possible smallest common element
let cur_val = -Infinity;
do
var same = true;
for (let i = 0; i < arguments.length; i++)
// Step an array forward to cur_val (or beyond if the array
// doesn't have cur_val)
while (indices[i] < arguments[i].length && arguments[i][indices[i]] < cur_val)
indices[i]++;
if (indices[i] < arguments[i].length)
if (arguments[i][indices[i]] > cur_val)
// We went past cur_val, so record in 'same' that cur_val does
// not represent the smallest common element this time through.
same = false;
cur_val = arguments[i][indices[i]];
else
// We got to the end of this array, so there is no smallest common element.
return null;
while (!same);
return cur_val;
edited Jun 6 at 18:11
answered Jun 6 at 17:48
Kyle MillerKyle Miller
1214 bronze badges
1214 bronze badges
add a comment
|
add a comment
|
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f221724%2fcheck-if-three-arrays-contains-the-same-element%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
1
$begingroup$
Need more clarification. You could start by giving several cases, or at least the answer to this one. And when you say "common element", are you expecting just one answer? What happens if there's more than one? Can there be more than one?
$endgroup$
– Joseph
Jun 5 at 15:12
$begingroup$
What do you mean by "first" common element? The lowest value? Or some metric involving the three indices?
$endgroup$
– Toby Speight
Jun 5 at 15:32
1
$begingroup$
The lowest value
$endgroup$
– Gervenel
Jun 5 at 15:40
$begingroup$
I would rather compare two arrays, then compare that result to the third. I believe such code would be more reusable and easy to adapt to find the same value in 4 arrays. But it would be MUCH less efficient if you generally expect to find the first match of all 3 arrays fairly early.
$endgroup$
– Zizy Archer
Jun 6 at 12:38
1
$begingroup$
I just noticed how freakishly anemic the
Set
API in ECMAScript is.$endgroup$
– Jörg W Mittag
Jun 6 at 19:46