Longest common prefix
$begingroup$
I have made a function for finding the longest common prefix for the challenge on the leetcode site. This is the code:
var longestCommonPrefix = function(strs) {
let longestPrefix = '';
if (strs.length > 0) {
longestPrefix = strs[0];
for (let i = 1; i < strs.length; i++) {
for (let j = 0; j < longestPrefix.length; j++) {
if (strs[i][j] != longestPrefix[j]) {
longestPrefix = longestPrefix.slice(0, j);
break;
}
}
}
}
return longestPrefix;
};
I am sure there is a way to make this code better, but not sure how to do that. Would appreciate any help.
javascript algorithm strings programming-challenge
$endgroup$
add a comment |
$begingroup$
I have made a function for finding the longest common prefix for the challenge on the leetcode site. This is the code:
var longestCommonPrefix = function(strs) {
let longestPrefix = '';
if (strs.length > 0) {
longestPrefix = strs[0];
for (let i = 1; i < strs.length; i++) {
for (let j = 0; j < longestPrefix.length; j++) {
if (strs[i][j] != longestPrefix[j]) {
longestPrefix = longestPrefix.slice(0, j);
break;
}
}
}
}
return longestPrefix;
};
I am sure there is a way to make this code better, but not sure how to do that. Would appreciate any help.
javascript algorithm strings programming-challenge
$endgroup$
add a comment |
$begingroup$
I have made a function for finding the longest common prefix for the challenge on the leetcode site. This is the code:
var longestCommonPrefix = function(strs) {
let longestPrefix = '';
if (strs.length > 0) {
longestPrefix = strs[0];
for (let i = 1; i < strs.length; i++) {
for (let j = 0; j < longestPrefix.length; j++) {
if (strs[i][j] != longestPrefix[j]) {
longestPrefix = longestPrefix.slice(0, j);
break;
}
}
}
}
return longestPrefix;
};
I am sure there is a way to make this code better, but not sure how to do that. Would appreciate any help.
javascript algorithm strings programming-challenge
$endgroup$
I have made a function for finding the longest common prefix for the challenge on the leetcode site. This is the code:
var longestCommonPrefix = function(strs) {
let longestPrefix = '';
if (strs.length > 0) {
longestPrefix = strs[0];
for (let i = 1; i < strs.length; i++) {
for (let j = 0; j < longestPrefix.length; j++) {
if (strs[i][j] != longestPrefix[j]) {
longestPrefix = longestPrefix.slice(0, j);
break;
}
}
}
}
return longestPrefix;
};
I am sure there is a way to make this code better, but not sure how to do that. Would appreciate any help.
javascript algorithm strings programming-challenge
javascript algorithm strings programming-challenge
edited Dec 17 '18 at 8:42
Leff
asked Dec 7 '17 at 13:59
LeffLeff
1727
1727
add a comment |
add a comment |
3 Answers
3
active
oldest
votes
$begingroup$
From a short review;
- You should sort the strings by length ascending if you start by assigning
longestPrefix = strs[0];
the prefix cannot be longer than the shortest string. I would assign
longestPrefix[j]
to a variable, avoiding an array access in a nested loop
I would
return
the found value instead of callingbreak
- Break only exits one iteration in the loop anyway
- It seems okay that if no string list is provided, that
undefined
is returned
Personal preference, I prefer
list
overstrs
function(strs)
creates an anonymous function, which is terrible in stack traces, just use the good oldfunction longestCommonPrefix(strs) {
*
$endgroup$
add a comment |
$begingroup$
I would find the alphabetically smallest and largest string and just run your algorithm on these two. That would avoid the embedded loop.
var longestCommonPrefix = function(strs) {
if (!strs)
return '';
let smallest = strs.reduce( (min, str) => min < str ? min : str, strs[0] );
let largest = strs.reduce( (min, str) => min > str ? min : str, strs[0] );
for (let i=0; i<smallest.length; i++) {
if (smallest[i] != largest[i])
return smallest.substr(0,i);
}
return '';
};
In answer to konijn
it would be minimally faster to get the smallest/largest by doing:
let smallest = strs[0];
let largest = strs[0];
for (let i=1; i<strs.length; i++) {
let s= strs[i];
if (s > largest) largest = s;
if (s < smallest) smallest = s;
}
$endgroup$
$begingroup$
Would asort
with apop
and anunshift
not be faster?
$endgroup$
– konijn
Dec 7 '17 at 18:40
1
$begingroup$
fwiw, I wouldn't useshift
since it requires moving all the elements in the array,[0]
would be quicker. Evenpop
requires modifying the array, but to answer your comment most sorts are quite expensive O(n.log n) or O(n^2) and require a lot of moving and copying. The two calculations could be combined in a loop if performance was really critical (see my addition)
$endgroup$
– Marc Rohloff
Dec 7 '17 at 22:34
$begingroup$
How you you return an answer by inspecting only the shortest and longest? Consider {'ab', 'acd', 'abcd'} -> 'a'
$endgroup$
– Pierre Menard
Dec 17 '18 at 19:57
$begingroup$
Because if the smallest and largest string share a common prefix every other string would share it too. For example 'abaaa' and 'adeee' share the prefix 'a' everything inbetween ('abbbb', 'ac', 'ada', 'adaa') will all share the same prefix.
$endgroup$
– Marc Rohloff
Dec 19 '18 at 15:17
add a comment |
$begingroup$
var longestCommonPrefix = function(strs) {
if (!strs || strs.length === 0) {
return '';
}
var smallest = strs.reduce((min, max) => {
return min < max ? min : max
}, strs[0]);
var longest = strs.reduce((min, max) => {
return min > max ? min : max
}, strs[0]);
var res = '';
for (var i = 0; i < smallest.length; i++) {
if (smallest[i] !== longest[i]) {
return smallest.substr(0, i);
}
else {
res += smallest[i];
}
}
return res;
};
New contributor
$endgroup$
add a comment |
Your Answer
StackExchange.ifUsing("editor", function () {
return StackExchange.using("mathjaxEditing", function () {
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
});
});
}, "mathjax-editing");
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/3.0/"u003ecc by-sa 3.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%2f182217%2flongest-common-prefix%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
3 Answers
3
active
oldest
votes
3 Answers
3
active
oldest
votes
active
oldest
votes
active
oldest
votes
$begingroup$
From a short review;
- You should sort the strings by length ascending if you start by assigning
longestPrefix = strs[0];
the prefix cannot be longer than the shortest string. I would assign
longestPrefix[j]
to a variable, avoiding an array access in a nested loop
I would
return
the found value instead of callingbreak
- Break only exits one iteration in the loop anyway
- It seems okay that if no string list is provided, that
undefined
is returned
Personal preference, I prefer
list
overstrs
function(strs)
creates an anonymous function, which is terrible in stack traces, just use the good oldfunction longestCommonPrefix(strs) {
*
$endgroup$
add a comment |
$begingroup$
From a short review;
- You should sort the strings by length ascending if you start by assigning
longestPrefix = strs[0];
the prefix cannot be longer than the shortest string. I would assign
longestPrefix[j]
to a variable, avoiding an array access in a nested loop
I would
return
the found value instead of callingbreak
- Break only exits one iteration in the loop anyway
- It seems okay that if no string list is provided, that
undefined
is returned
Personal preference, I prefer
list
overstrs
function(strs)
creates an anonymous function, which is terrible in stack traces, just use the good oldfunction longestCommonPrefix(strs) {
*
$endgroup$
add a comment |
$begingroup$
From a short review;
- You should sort the strings by length ascending if you start by assigning
longestPrefix = strs[0];
the prefix cannot be longer than the shortest string. I would assign
longestPrefix[j]
to a variable, avoiding an array access in a nested loop
I would
return
the found value instead of callingbreak
- Break only exits one iteration in the loop anyway
- It seems okay that if no string list is provided, that
undefined
is returned
Personal preference, I prefer
list
overstrs
function(strs)
creates an anonymous function, which is terrible in stack traces, just use the good oldfunction longestCommonPrefix(strs) {
*
$endgroup$
From a short review;
- You should sort the strings by length ascending if you start by assigning
longestPrefix = strs[0];
the prefix cannot be longer than the shortest string. I would assign
longestPrefix[j]
to a variable, avoiding an array access in a nested loop
I would
return
the found value instead of callingbreak
- Break only exits one iteration in the loop anyway
- It seems okay that if no string list is provided, that
undefined
is returned
Personal preference, I prefer
list
overstrs
function(strs)
creates an anonymous function, which is terrible in stack traces, just use the good oldfunction longestCommonPrefix(strs) {
*
edited Dec 7 '17 at 17:36
answered Dec 7 '17 at 15:59
konijnkonijn
27.1k455236
27.1k455236
add a comment |
add a comment |
$begingroup$
I would find the alphabetically smallest and largest string and just run your algorithm on these two. That would avoid the embedded loop.
var longestCommonPrefix = function(strs) {
if (!strs)
return '';
let smallest = strs.reduce( (min, str) => min < str ? min : str, strs[0] );
let largest = strs.reduce( (min, str) => min > str ? min : str, strs[0] );
for (let i=0; i<smallest.length; i++) {
if (smallest[i] != largest[i])
return smallest.substr(0,i);
}
return '';
};
In answer to konijn
it would be minimally faster to get the smallest/largest by doing:
let smallest = strs[0];
let largest = strs[0];
for (let i=1; i<strs.length; i++) {
let s= strs[i];
if (s > largest) largest = s;
if (s < smallest) smallest = s;
}
$endgroup$
$begingroup$
Would asort
with apop
and anunshift
not be faster?
$endgroup$
– konijn
Dec 7 '17 at 18:40
1
$begingroup$
fwiw, I wouldn't useshift
since it requires moving all the elements in the array,[0]
would be quicker. Evenpop
requires modifying the array, but to answer your comment most sorts are quite expensive O(n.log n) or O(n^2) and require a lot of moving and copying. The two calculations could be combined in a loop if performance was really critical (see my addition)
$endgroup$
– Marc Rohloff
Dec 7 '17 at 22:34
$begingroup$
How you you return an answer by inspecting only the shortest and longest? Consider {'ab', 'acd', 'abcd'} -> 'a'
$endgroup$
– Pierre Menard
Dec 17 '18 at 19:57
$begingroup$
Because if the smallest and largest string share a common prefix every other string would share it too. For example 'abaaa' and 'adeee' share the prefix 'a' everything inbetween ('abbbb', 'ac', 'ada', 'adaa') will all share the same prefix.
$endgroup$
– Marc Rohloff
Dec 19 '18 at 15:17
add a comment |
$begingroup$
I would find the alphabetically smallest and largest string and just run your algorithm on these two. That would avoid the embedded loop.
var longestCommonPrefix = function(strs) {
if (!strs)
return '';
let smallest = strs.reduce( (min, str) => min < str ? min : str, strs[0] );
let largest = strs.reduce( (min, str) => min > str ? min : str, strs[0] );
for (let i=0; i<smallest.length; i++) {
if (smallest[i] != largest[i])
return smallest.substr(0,i);
}
return '';
};
In answer to konijn
it would be minimally faster to get the smallest/largest by doing:
let smallest = strs[0];
let largest = strs[0];
for (let i=1; i<strs.length; i++) {
let s= strs[i];
if (s > largest) largest = s;
if (s < smallest) smallest = s;
}
$endgroup$
$begingroup$
Would asort
with apop
and anunshift
not be faster?
$endgroup$
– konijn
Dec 7 '17 at 18:40
1
$begingroup$
fwiw, I wouldn't useshift
since it requires moving all the elements in the array,[0]
would be quicker. Evenpop
requires modifying the array, but to answer your comment most sorts are quite expensive O(n.log n) or O(n^2) and require a lot of moving and copying. The two calculations could be combined in a loop if performance was really critical (see my addition)
$endgroup$
– Marc Rohloff
Dec 7 '17 at 22:34
$begingroup$
How you you return an answer by inspecting only the shortest and longest? Consider {'ab', 'acd', 'abcd'} -> 'a'
$endgroup$
– Pierre Menard
Dec 17 '18 at 19:57
$begingroup$
Because if the smallest and largest string share a common prefix every other string would share it too. For example 'abaaa' and 'adeee' share the prefix 'a' everything inbetween ('abbbb', 'ac', 'ada', 'adaa') will all share the same prefix.
$endgroup$
– Marc Rohloff
Dec 19 '18 at 15:17
add a comment |
$begingroup$
I would find the alphabetically smallest and largest string and just run your algorithm on these two. That would avoid the embedded loop.
var longestCommonPrefix = function(strs) {
if (!strs)
return '';
let smallest = strs.reduce( (min, str) => min < str ? min : str, strs[0] );
let largest = strs.reduce( (min, str) => min > str ? min : str, strs[0] );
for (let i=0; i<smallest.length; i++) {
if (smallest[i] != largest[i])
return smallest.substr(0,i);
}
return '';
};
In answer to konijn
it would be minimally faster to get the smallest/largest by doing:
let smallest = strs[0];
let largest = strs[0];
for (let i=1; i<strs.length; i++) {
let s= strs[i];
if (s > largest) largest = s;
if (s < smallest) smallest = s;
}
$endgroup$
I would find the alphabetically smallest and largest string and just run your algorithm on these two. That would avoid the embedded loop.
var longestCommonPrefix = function(strs) {
if (!strs)
return '';
let smallest = strs.reduce( (min, str) => min < str ? min : str, strs[0] );
let largest = strs.reduce( (min, str) => min > str ? min : str, strs[0] );
for (let i=0; i<smallest.length; i++) {
if (smallest[i] != largest[i])
return smallest.substr(0,i);
}
return '';
};
In answer to konijn
it would be minimally faster to get the smallest/largest by doing:
let smallest = strs[0];
let largest = strs[0];
for (let i=1; i<strs.length; i++) {
let s= strs[i];
if (s > largest) largest = s;
if (s < smallest) smallest = s;
}
edited Dec 7 '17 at 22:37
answered Dec 7 '17 at 17:42
Marc RohloffMarc Rohloff
3,02946
3,02946
$begingroup$
Would asort
with apop
and anunshift
not be faster?
$endgroup$
– konijn
Dec 7 '17 at 18:40
1
$begingroup$
fwiw, I wouldn't useshift
since it requires moving all the elements in the array,[0]
would be quicker. Evenpop
requires modifying the array, but to answer your comment most sorts are quite expensive O(n.log n) or O(n^2) and require a lot of moving and copying. The two calculations could be combined in a loop if performance was really critical (see my addition)
$endgroup$
– Marc Rohloff
Dec 7 '17 at 22:34
$begingroup$
How you you return an answer by inspecting only the shortest and longest? Consider {'ab', 'acd', 'abcd'} -> 'a'
$endgroup$
– Pierre Menard
Dec 17 '18 at 19:57
$begingroup$
Because if the smallest and largest string share a common prefix every other string would share it too. For example 'abaaa' and 'adeee' share the prefix 'a' everything inbetween ('abbbb', 'ac', 'ada', 'adaa') will all share the same prefix.
$endgroup$
– Marc Rohloff
Dec 19 '18 at 15:17
add a comment |
$begingroup$
Would asort
with apop
and anunshift
not be faster?
$endgroup$
– konijn
Dec 7 '17 at 18:40
1
$begingroup$
fwiw, I wouldn't useshift
since it requires moving all the elements in the array,[0]
would be quicker. Evenpop
requires modifying the array, but to answer your comment most sorts are quite expensive O(n.log n) or O(n^2) and require a lot of moving and copying. The two calculations could be combined in a loop if performance was really critical (see my addition)
$endgroup$
– Marc Rohloff
Dec 7 '17 at 22:34
$begingroup$
How you you return an answer by inspecting only the shortest and longest? Consider {'ab', 'acd', 'abcd'} -> 'a'
$endgroup$
– Pierre Menard
Dec 17 '18 at 19:57
$begingroup$
Because if the smallest and largest string share a common prefix every other string would share it too. For example 'abaaa' and 'adeee' share the prefix 'a' everything inbetween ('abbbb', 'ac', 'ada', 'adaa') will all share the same prefix.
$endgroup$
– Marc Rohloff
Dec 19 '18 at 15:17
$begingroup$
Would a
sort
with a pop
and an unshift
not be faster?$endgroup$
– konijn
Dec 7 '17 at 18:40
$begingroup$
Would a
sort
with a pop
and an unshift
not be faster?$endgroup$
– konijn
Dec 7 '17 at 18:40
1
1
$begingroup$
fwiw, I wouldn't use
shift
since it requires moving all the elements in the array, [0]
would be quicker. Even pop
requires modifying the array, but to answer your comment most sorts are quite expensive O(n.log n) or O(n^2) and require a lot of moving and copying. The two calculations could be combined in a loop if performance was really critical (see my addition)$endgroup$
– Marc Rohloff
Dec 7 '17 at 22:34
$begingroup$
fwiw, I wouldn't use
shift
since it requires moving all the elements in the array, [0]
would be quicker. Even pop
requires modifying the array, but to answer your comment most sorts are quite expensive O(n.log n) or O(n^2) and require a lot of moving and copying. The two calculations could be combined in a loop if performance was really critical (see my addition)$endgroup$
– Marc Rohloff
Dec 7 '17 at 22:34
$begingroup$
How you you return an answer by inspecting only the shortest and longest? Consider {'ab', 'acd', 'abcd'} -> 'a'
$endgroup$
– Pierre Menard
Dec 17 '18 at 19:57
$begingroup$
How you you return an answer by inspecting only the shortest and longest? Consider {'ab', 'acd', 'abcd'} -> 'a'
$endgroup$
– Pierre Menard
Dec 17 '18 at 19:57
$begingroup$
Because if the smallest and largest string share a common prefix every other string would share it too. For example 'abaaa' and 'adeee' share the prefix 'a' everything inbetween ('abbbb', 'ac', 'ada', 'adaa') will all share the same prefix.
$endgroup$
– Marc Rohloff
Dec 19 '18 at 15:17
$begingroup$
Because if the smallest and largest string share a common prefix every other string would share it too. For example 'abaaa' and 'adeee' share the prefix 'a' everything inbetween ('abbbb', 'ac', 'ada', 'adaa') will all share the same prefix.
$endgroup$
– Marc Rohloff
Dec 19 '18 at 15:17
add a comment |
$begingroup$
var longestCommonPrefix = function(strs) {
if (!strs || strs.length === 0) {
return '';
}
var smallest = strs.reduce((min, max) => {
return min < max ? min : max
}, strs[0]);
var longest = strs.reduce((min, max) => {
return min > max ? min : max
}, strs[0]);
var res = '';
for (var i = 0; i < smallest.length; i++) {
if (smallest[i] !== longest[i]) {
return smallest.substr(0, i);
}
else {
res += smallest[i];
}
}
return res;
};
New contributor
$endgroup$
add a comment |
$begingroup$
var longestCommonPrefix = function(strs) {
if (!strs || strs.length === 0) {
return '';
}
var smallest = strs.reduce((min, max) => {
return min < max ? min : max
}, strs[0]);
var longest = strs.reduce((min, max) => {
return min > max ? min : max
}, strs[0]);
var res = '';
for (var i = 0; i < smallest.length; i++) {
if (smallest[i] !== longest[i]) {
return smallest.substr(0, i);
}
else {
res += smallest[i];
}
}
return res;
};
New contributor
$endgroup$
add a comment |
$begingroup$
var longestCommonPrefix = function(strs) {
if (!strs || strs.length === 0) {
return '';
}
var smallest = strs.reduce((min, max) => {
return min < max ? min : max
}, strs[0]);
var longest = strs.reduce((min, max) => {
return min > max ? min : max
}, strs[0]);
var res = '';
for (var i = 0; i < smallest.length; i++) {
if (smallest[i] !== longest[i]) {
return smallest.substr(0, i);
}
else {
res += smallest[i];
}
}
return res;
};
New contributor
$endgroup$
var longestCommonPrefix = function(strs) {
if (!strs || strs.length === 0) {
return '';
}
var smallest = strs.reduce((min, max) => {
return min < max ? min : max
}, strs[0]);
var longest = strs.reduce((min, max) => {
return min > max ? min : max
}, strs[0]);
var res = '';
for (var i = 0; i < smallest.length; i++) {
if (smallest[i] !== longest[i]) {
return smallest.substr(0, i);
}
else {
res += smallest[i];
}
}
return res;
};
New contributor
New contributor
answered 17 mins ago
1061763184qqcom1061763184qqcom
1
1
New contributor
New contributor
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%2f182217%2flongest-common-prefix%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