I have to disagree with palacsint about using HashMap
, if anything, I think that you should use a Set
implementation such as HashSet
. You're never using the value that is stored in the map, and you have no reason to use it either. Therefore, I would go with a Set
. (The fact that a HashSet internally uses a HashMap is an entirely different subject) Palacsint is correct though that HashMap is a better alternative than Hashtable.
Reading lines like this makes me sea-sick:
if( (!myTable.containsKey(minVal-1)) && minVal-1>0){
Use spacing properly and remove unnecessary parameters and it becomes:
if(!myTable.containsKey(minVal - 1) && minVal - 1 > 0) {
Overall, I think that you shouldn't need to use Objects for this (except an array of course, which technically is an object). I would only use primitives and use more self-documenting variable names only use primitives and use more self-documenting variable names.
An unnecessary operation in your code is that on each iteration you check containsKey
twice. Once to see if value - 1
is set and once to see if value + 1
is set. Why not just check if value itself is set?
Your code can be rewritten and optimized into this: (This is a fixed version of ChrisW's approach, I had started the approach when he posted his answer and then I did some modifications to make sure it matched your code).
public int firstMissingPositive(int[] array) {
boolean[] foundValues = new boolean[array.length];
for (int i : array) {
if (i > 0 && i <= foundValues.length)
foundValues[i - 1] = true;
}
for (int i = 0; i < foundValues.length; i++) {
if (!foundValues[i]) {
return i + 1;
}
}
return foundValues.length + 1;
}
I have to disagree with palacsint about using HashMap
, if anything, I think that you should use a Set
implementation such as HashSet
. You're never using the value that is stored in the map, and you have no reason to use it either. Therefore, I would go with a Set
. (The fact that a HashSet internally uses a HashMap is an entirely different subject) Palacsint is correct though that HashMap is a better alternative than Hashtable.
Reading lines like this makes me sea-sick:
if( (!myTable.containsKey(minVal-1)) && minVal-1>0){
Use spacing properly and remove unnecessary parameters and it becomes:
if(!myTable.containsKey(minVal - 1) && minVal - 1 > 0) {
Overall, I think that you shouldn't need to use Objects for this (except an array of course, which technically is an object). I would only use primitives and use more self-documenting variable names.
An unnecessary operation in your code is that on each iteration you check containsKey
twice. Once to see if value - 1
is set and once to see if value + 1
is set. Why not just check if value itself is set?
Your code can be rewritten and optimized into this: (This is a fixed version of ChrisW's approach, I had started the approach when he posted his answer and then I did some modifications to make sure it matched your code).
public int firstMissingPositive(int[] array) {
boolean[] foundValues = new boolean[array.length];
for (int i : array) {
if (i > 0 && i <= foundValues.length)
foundValues[i - 1] = true;
}
for (int i = 0; i < foundValues.length; i++) {
if (!foundValues[i]) {
return i + 1;
}
}
return foundValues.length + 1;
}
I have to disagree with palacsint about using HashMap
, if anything, I think that you should use a Set
implementation such as HashSet
. You're never using the value that is stored in the map, and you have no reason to use it either. Therefore, I would go with a Set
. (The fact that a HashSet internally uses a HashMap is an entirely different subject) Palacsint is correct though that HashMap is a better alternative than Hashtable.
Reading lines like this makes me sea-sick:
if( (!myTable.containsKey(minVal-1)) && minVal-1>0){
Use spacing properly and remove unnecessary parameters and it becomes:
if(!myTable.containsKey(minVal - 1) && minVal - 1 > 0) {
Overall, I think that you shouldn't need to use Objects for this (except an array of course, which technically is an object). I would only use primitives and use more self-documenting variable names.
An unnecessary operation in your code is that on each iteration you check containsKey
twice. Once to see if value - 1
is set and once to see if value + 1
is set. Why not just check if value itself is set?
Your code can be rewritten and optimized into this: (This is a fixed version of ChrisW's approach, I had started the approach when he posted his answer and then I did some modifications to make sure it matched your code).
public int firstMissingPositive(int[] array) {
boolean[] foundValues = new boolean[array.length];
for (int i : array) {
if (i > 0 && i <= foundValues.length)
foundValues[i - 1] = true;
}
for (int i = 0; i < foundValues.length; i++) {
if (!foundValues[i]) {
return i + 1;
}
}
return foundValues.length + 1;
}
I have to disagree with palacsint about using HashMap
, if anything, I think that you should use a Set
implementation such as HashSet
. You're never using the value that is stored in the map, and you have no reason to use it either. Therefore, I would go with a Set
. (The fact that a HashSet internally uses a HashMap is an entirely different subject) Palacsint is correct though that HashMap is a better alternative than Hashtable.
Reading lines like this makes me sea-sick:
if( (!myTable.containsKey(minVal-1)) && minVal-1>0){
Use spacing properly and remove unnecessary parameters and it becomes:
if(!myTable.containsKey(minVal - 1) && minVal - 1 > 0) {
Overall, I think that you shouldn't need to use Objects for this (except an array of course, which technically is an object). I would only use primitives and use more self-documenting variable names.
An unnecessary operation in your code is that on each iteration you check containsKey
twice. Once to see if value - 1
is set and once to see if value + 1
is set. Why not just check if value itself is set?
Your code can be rewritten and optimized into this: (This is a fixed version of ChrisW's approach, I had started the approach when he posted his answer and then I did some modifications to make sure it matched your code).
public int firstMissingPositive(int[] array) {
boolean[] foundValues = new boolean[array.length];
for (int i : array) {
if (i > 0 && i <= foundValues.length)
foundValues[i - 1] = true;
}
for (int i = 0; i < foundValues.length; i++) {
if (!foundValues[i]) {
return i + 1;
}
}
return foundValues.length + 1;
}
I have to disagree with palacsint about using HashMap
, if anything, I think that you should use a Set
implementation such as HashSet
. You're never using the value that is stored in the map, and you have no reason to use it either. Therefore, I would go with a Set
. (The fact that a HashSet internally uses a HashMap is an entirely different subject) Palacsint is correct though that HashMap is a better alternative than Hashtable.
Reading lines like this makes me sea-sick:
if( (!myTable.containsKey(minVal-1)) && minVal-1>0){
Use spacing properly and remove unnecessary parameters and it becomes:
if(!myTable.containsKey(minVal - 1) && minVal - 1 > 0) {
Overall, I think that you shouldn't need to use Objects for this (except an array of course, which technically is an object). I would only use primitives and use more self-documenting variable names.
I have to disagree with palacsint about using HashMap
, if anything, I think that you should use a Set
implementation such as HashSet
. You're never using the value that is stored in the map, and you have no reason to use it either. Therefore, I would go with a Set
. (The fact that a HashSet internally uses a HashMap is an entirely different subject) Palacsint is correct though that HashMap is a better alternative than Hashtable.
Reading lines like this makes me sea-sick:
if( (!myTable.containsKey(minVal-1)) && minVal-1>0){
Use spacing properly and remove unnecessary parameters and it becomes:
if(!myTable.containsKey(minVal - 1) && minVal - 1 > 0) {
Overall, I think that you shouldn't need to use Objects for this (except an array of course, which technically is an object). I would only use primitives and use more self-documenting variable names.
An unnecessary operation in your code is that on each iteration you check containsKey
twice. Once to see if value - 1
is set and once to see if value + 1
is set. Why not just check if value itself is set?
Your code can be rewritten and optimized into this: (This is a fixed version of ChrisW's approach, I had started the approach when he posted his answer and then I did some modifications to make sure it matched your code).
public int firstMissingPositive(int[] array) {
boolean[] foundValues = new boolean[array.length];
for (int i : array) {
if (i > 0 && i <= foundValues.length)
foundValues[i - 1] = true;
}
for (int i = 0; i < foundValues.length; i++) {
if (!foundValues[i]) {
return i + 1;
}
}
return foundValues.length + 1;
}
I have to disagree with palacsint about using HashMap
, if anything, I think that you should use a Set
implementation such as HashSet
. You're never using the value that is stored in the map, and you have no reason to use it either. Therefore, I would go with a Set
. (The fact that a HashSet internally uses a HashMap is an entirely different subject) Palacsint is correct though that HashMap is a better alternative than Hashtable.
(Palacsint's answer seems to have been deleted now though...)
Reading lines like this makes me sea-sick:
if( (!myTable.containsKey(minVal-1)) && minVal-1>0){
Use spacing properly and remove unnecessary parameters and it becomes:
if(!myTable.containsKey(minVal - 1) && minVal - 1 > 0) {
Overall, I think that you shouldn't need to use Objects for this (except an array of course, which technically is an object). I would only use primitives and use more self-documenting variable names.
I have to disagree with palacsint about using HashMap
, if anything, I think that you should use a Set
implementation such as HashSet
. You're never using the value that is stored in the map, and you have no reason to use it either. Therefore, I would go with a Set
. (The fact that a HashSet internally uses a HashMap is an entirely different subject) Palacsint is correct though that HashMap is a better alternative than Hashtable.
(Palacsint's answer seems to have been deleted now though...)
Reading lines like this makes me sea-sick:
if( (!myTable.containsKey(minVal-1)) && minVal-1>0){
Use spacing properly and remove unnecessary parameters and it becomes:
if(!myTable.containsKey(minVal - 1) && minVal - 1 > 0) {
Overall, I think that you shouldn't need to use Objects for this (except an array of course, which technically is an object). I would only use primitives and use more self-documenting variable names.
I have to disagree with palacsint about using HashMap
, if anything, I think that you should use a Set
implementation such as HashSet
. You're never using the value that is stored in the map, and you have no reason to use it either. Therefore, I would go with a Set
. (The fact that a HashSet internally uses a HashMap is an entirely different subject) Palacsint is correct though that HashMap is a better alternative than Hashtable.
Reading lines like this makes me sea-sick:
if( (!myTable.containsKey(minVal-1)) && minVal-1>0){
Use spacing properly and remove unnecessary parameters and it becomes:
if(!myTable.containsKey(minVal - 1) && minVal - 1 > 0) {
Overall, I think that you shouldn't need to use Objects for this (except an array of course, which technically is an object). I would only use primitives and use more self-documenting variable names.