Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I have to parse a file line-by-line and in single line I have split by ",". The first string is the name and the second is the count.

Finally, I have to display the Key and Count, for example:


I should display as Peter 5 and Smith 8.

I confused about choosing between BufferedReader vs Scanner. I went through this link and came up with these two approaches. I would like to get your reviews.

  1. BufferedReader

    private HashMap<String, MutableLong> readFile(File file) throws IOException {
        final HashMap<String, MutableLong> keyHolder = new HashMap<>();
        try (BufferedReader br = new BufferedReader(new InputStreamReader(
                new FileInputStream(file), "UTF-8"))) {
            for (String line; (line = br.readLine()) != null;) {
                // processing the line.
                final String[] keyContents = line
                if (keyContents.length == 2) {
                    final String keyName = keyContents[0];
                    final long count = Long.parseLong(keyContents[1]);
                    final MutableLong keyCount = keyHolder.get(keyName);
                    if (keyCount != null) {
                        keyHolder.put(keyName, keyCount);
                    } else {
                        keyHolder.put(keyName, new MutableLong(count));
        return keyHolder;
    private static final String COMMA_DELIMETER = ",";
    private static volatile Pattern commaPattern = Pattern.compile(COMMA_DELIMETER);

    I have used MutableLong, since I don't want to create a BigInteger each time. Again, it may be a very big file and I don't have control on how max a key can occur.

  2. Scanner and two delimiters

    private static final String LINE_SEPARATOR_PATTERN = "\r\n|[\n\r\u2028\u2029\u0085]";
    private static final String LINE_PATTERN = ".*(" + LINE_SEPARATOR_PATTERN
            + ")|.+$";
    private static volatile Pattern linePattern = Pattern.compile(LINE_PATTERN);

I have went through the hasNext in Scanner, and to me, there is no harm in switching the Pattern. And I believe from Java 7, Scanner does have a limited buffer and can be enough for this kind of file.

Does anyone prefer Approach 2 over Approach 1, or do we have any other option than this? I just did sop for testing purpose. Obviously the same code in approach 1 would replace here. Using split in Approach 1 would create multiple String instances, which can be avoided here, by scanning a char sequence.

private HashMap<String, BigInteger> readFileScanner(File file)
            throws IOException {
        final HashMap<String, BigInteger> keyHolder = new HashMap<>();
        try (Scanner br = new Scanner(file, "UTF-8")) {
            while (br.hasNext()) {
        return keyHolder;
share|improve this question

I think that splitting a string on commas is the worse approach for serious CSV parsing.

There is no single standard format, but RFC 4180 comes close to being a de facto standard. As described in Section 2, CSV fields may be double-quoted (and if so, a literal " would be represented as a pair of " characters). Simply splitting a line on commas does not give you the ability to ever implement support for interpreting quoting.

With a Scanner, you can use tools such as useDelimiter(Pattern) and findInLine(Pattern) to simultaneously look for " and , characters, giving you a chance to write a real parser based on the ABNF grammar in RFC 4180.

Of course, unless are deliberately , you should just use an existing library, such as Apache Commons CSV or opencsv.

share|improve this answer
Thanks , i may be misguided by adding CSV in the Question. It is just comma Separated and no Quotes involved. I agreed that i should avoid reinvent and use opencsv / Apache Commons. Assume i am given only to use native API . in that case if i understood correctly you prefer to use Scanner and findInLine(Pattern)- correct me if i wrong – Mani Apr 6 '14 at 18:02
I particularily agree with the suggestion to use the apache commons CSV library; after all, he already is using apache commons with the MutableLong anyway. – AJMansfield Apr 6 '14 at 18:20
SuperCSV is also a good choice. – palacsint Apr 6 '14 at 19:06

Which iterator to use
I believe it is a matter of taste which iterator to use, though I find the first option is better because:

  • It is more readable - you understand what is going on at every line - your readLine() each time, and then split() it. In the second option you keep changing the delimiter which is not very intuitive or readable, and the code reader would have to be familiar with Scanner to understand what is going on.
  • It is less brittle - in the first option you actually check the validity of your input (that it has two elements in each line). In the second option, you can't really do that, and if you get a corrupt file, you are going to get unexpected results.

Mutable Long?
I did not follow you explanation about MutableLong, for one thing since I can't see that it even supports BigInteger... Also, when in each line you have numbers in the range of 2 and 5, how big you expect the files to be? The max long number is 9,223,372,036,854,775,807...

Magic Constants
A constant like COMMA_DELIMETER = "," is not very useful. Using the literal is readable enough, and resolves any ambiguity it might have by hiding it in a constant:

keyContents = line.split(",");
share|improve this answer
Agreed with magic Constants. Thanks for the your view on Option 1 and Option 2. I do even prefer the first one. Only thing i worried is , readLine would create new String and Split would create two more Strings . so for a line it would create 3 Strings. If the file is huge it would be expensive spending cycle in GC. How ever will it do always new String / intern from pool ( i dont know) – Mani Apr 6 '14 at 17:27
And for the Mutable Long. Same Reason. BigInteger is not Mutable, so each time when the same key found it would create another one. That why went to Mutable Long. I could have used AtomicLong but it has more than what i needed. so i choosed MutableLong from apache it just wraps the long – Mani Apr 6 '14 at 17:30
Scanner will create two strings too. Anyway native long what is wrong with it? It's 64bit and you avoid object overhead and the part to check if it's null. (P.S AtomicLong is a total different thing!) – Marco Acierno Apr 6 '14 at 17:38
Yes. but i cant use native long since i have to keep the count in map (any storage) . so i have to use Object wrapped by long any way – Mani Apr 6 '14 at 17:41

Your Answer


By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.