Putting array items into hashmap with 2 string

Question

Say, I have an array similar to this,

{"A", "1", "B", "2", "C", "3"}

I want to put it to HashMap like pairs, I tried few ways, but none seem to work. I created a method isDigit() to check if an item is digit,

private static boolean isDigit(String str){
    try {
        Integer.parseInt(str);
    }
    catch (NumberFormatException nfe){
        return false;
    }
    return true;
}

Then I tried separating into 2 arrays, numbers and letters.

for (int i = 0; i <= parts.length; i++) {
    if (isDigit(parts[i])) {
        numbers[i] = parts[i];
    } else {
        abcs[i] = parts[i];
    }
}

and finally,

for (int i = 0; i < numbers.length ; i++) {
    map.put(abcs[i], numbers[i]);
}

and I print them with,

for (String each: map.keySet()) {
   System.out.println(each + ":" + map.get(each));
}

which prints something like,

1
A : null
2
B : null

What it should print is,

A : 1
B : 2
C : 3

`isDigit("10")` would return true, but 10 isn't a digit. `isInteger` would be a better name. — weston, Apr 15 '17 at 19:47
You're making it much harder (and slower) than necessary. Why not just iterate through the original array, add the current element and the next one to the map, and then add 2 to the loop counter? `for (int i = 0; i < array.length; i += 2) { map.put(array[i], array[i + 1]); }` — JB Nizet, Apr 15 '17 at 19:48
@JB Nizet already tried that, let me edit the question with that as well. — Simeon Aleksov, Apr 15 '17 at 19:49
There are ways to do this in various ways, your way is one of them, though its not the best one. Still as a learning you must try to know the error or issue in your code. I have added my answer and tried to explain the issue with the solution to resolve the issue. Hope it will be helpful. http://stackoverflow.com/a/43430604/504133 — nits.kk, Apr 15 '17 at 19:55

score 2 · Answer 1 · answered Apr 15 '17 at 19:48

2

The problem is with your for loop:

for (int i = 0; i <= parts.length; i++) {
    if (isDigit(parts[i])) {
        numbers[i] = parts[i];
    } else {
        abcs[i] = parts[i];
    }
}

You are iterating from 0 to 5, so what happens is you are placing valies into indexes offset by 1. It goes like this: abcs[0] -> numbers[1] -> abcs[2] -< numbers[3]. A simpler solution is to use lists and use the add method.

answered Apr 15 '17 at 19:48

Sasang

1,261
9
10

Agreed. Using a List would be better here. – Glen Pierce Apr 15 '17 at 19:49

weston · Accepted Answer · 2017-04-15T20:08:08.600

2

Why not just iterate through the parts in pairs by incrementing the loop counter in 2s:

for (int i = 0; i < parts.length -1; i += 2) {
    map.put(parts[i], parts[i + 1]);
}

edited Apr 15 '17 at 20:08

answered Apr 15 '17 at 19:49

weston

54,145
21
145
203

Already tried this, say if I do, `map.get("A");`, prints `2 \n B`. – Simeon Aleksov Apr 15 '17 at 19:53
I don't think you'll want to do this as you'll have to start keeping track of iterating by twos which is not common. While this option might work, it wouldn't pass code review. – Glen Pierce Apr 15 '17 at 19:54
You need to post the bit of code that creates the array. That sounds like the problem. `"2 \n B"` must be in the array for that to happen. – weston Apr 15 '17 at 19:55
@GlenPierce It's just once to get it into a map, then you're done. Your sweeping code review comment makes no sense. Code reviews are done by developers. I am a developer. I would pass this. So how can you say that? – weston Apr 15 '17 at 19:59
My original array is split from a string `"A:1 \n B:2 \n C:3 \n ...` It prints okay when I print it. – Simeon Aleksov Apr 15 '17 at 20:05
I create parts by splitting with `:`. – Simeon Aleksov Apr 15 '17 at 20:05
Yeah you need to split by `\n` as well. – weston Apr 15 '17 at 20:07
@weston I don't want to get into a fight with you. Yes, you're correct that you only have to do this for the purpose of running the map method, but you're creating other problems, like that this code will produce a null pointer exception if that array happens to have an odd number of elements. – Glen Pierce Apr 15 '17 at 20:07
1

@GlenPierce `IndexOutOfBoundsException` you mean. I have now fixed that with a `-1` – weston Apr 15 '17 at 20:09
1

@GlenPierce haha :D – weston Apr 15 '17 at 20:13
@weston Yep. That is the problem, however how do I split twice? – Simeon Aleksov Apr 15 '17 at 20:21
http://stackoverflow.com/questions/5993779/java-use-split-with-multiple-delimiters – weston Apr 15 '17 at 20:22

score 1 · Answer 3 · answered Apr 15 '17 at 19:46

Your iterating over the array unevenly.

for (int i = 0; i <= parts.length; i++) {
    if (isDigit(parts[i])) {
        numbers[i] = parts[i];
    } else {
        abcs[i] = parts[i];
    }
}

Assuming isDigit() is behaving, you're creating two arrays, but they aren't in order when you create them.

So when i == 0, isDigit(parts[0]) is false, so you create abcs[0] = parts[0], but the next item you create in abcs isn't abcs[1], it's abcs[2] because you skipped 1 when it was a digit.

score 0 · Answer 4 · edited May 23 '17 at 12:17

There is the issue of indexing in your case. Though looking at the input type there are other ways (One such way is mentioned in an another answer) to do this but I am editing your code so that it could work as intended by you.

int inputLength = parts.length; // assuming it is always even as it contains a pair (alphabet, number)
int[] numbers = new int[inputLength/2];
char[] abcs = new char[inputLength/2;

int keyIndex = 0;
int valueIndex = 0;

for (int i = 0; i <= parts.length; i++) {
    if (isDigit(parts[i])) {
        numbers[keyIndex] = parts[i];
        keyIndex++;
    } else {
        abcs[valueIndex] = parts[i];
        valueIndex++;
    }
}

score 0 · Answer 5 · edited May 23 '17 at 12:09

0

As it is mentioned in one of the answers Your iterating over the array unevenly the problem is with indexing in a loop. But I would like also to point out, that you are using exception handling in a wrong way in your method isDigit(String str) Using exception handling for control flow is considered as a bad practice. A good answer to the question about this topic is here: Exceptions as control flow I would refactor your method to something like this:

private static boolean isDigit(String str){
    if (Character.isDigit(str.charAt(0))) { //assuming that the length of str is always 1
        return true;
    }
    return false;
}

Hope this helps.

edited May 23 '17 at 12:09

Community

1
1

answered Apr 15 '17 at 20:08

Vasiliy Vlasov

3,316
3
17
20

The length of string is not always 1. – Simeon Aleksov Apr 15 '17 at 20:08
Ok, then you may use regex. Something like this in `isDigit` method body: `return str.matches("[-+]?\\d*\\.?\\d+");` It will consider strings like "120" or "0012" or "00.45" as valid digits. Probably you will adjust regex to your concrete needs. Anyway my key idea was to kindly inform you that exception handling as control flow is an anti pattern. – Vasiliy Vlasov Apr 15 '17 at 20:23

Putting array items into hashmap with 2 string

5 Answers5