Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have to parse file line by line and in single line I have split by ",". First String would be Name and Second would be count. Finaly I have to display the Key and Count

For example

Peter,2 
Smith,3
Peter,3
Smith,5

I should display as Peter 5 and Smith 8.

So I was in confusion to choose between BufferedReader vs Scanner. I went through link and came up with these two approaches. I would like to get your concerns.

Approach 1: use buffered Reader

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
                        .split(KeyCountExam.COMMA_DELIMETER);
                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) {
                        keyCount.add(count);
                        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 dont want to create BigInteger for each time . And again it may be very big file and i don't have control on how max a key can occur

Another approach: use Scanner and use two Delimiter

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 ( am I right), by scanning 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()) {
                br.useDelimiter(commaPattern);
                System.out.println(br.next());
                System.out.println(br.next());
                br.useDelimiter(linePattern);
            }
        }
        return keyHolder;
    }

UPDATED:

Of course the default choice is opencsv, since I could not use open source APIs other than Java native API that I haven't chosen. MutableLong is the class I created under my project by wrapping long primitive variable; it is not the one from Apache. I named it and gave a link so that it could easily be understood.

share|improve this question

2 Answers 2

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 at 18:02
2  
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 at 18:20
    
SuperCSV is also a good choice. –  palacsint Apr 6 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 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 at 17:30
1  
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 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 at 17:41

Your Answer

 
discard

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.