File APIs for Java Developers
Manipulate DOC, XLS, PPT, PDF and many others from your application.
http://aspose.com/file-tools
The moose likes Beginning Java and the fly likes Code critique: CSV to Tab-delimited Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login


Win a copy of The Java EE 7 Tutorial Volume 1 or Volume 2 this week in the Java EE forum
or jQuery UI in Action in the JavaScript forum!
JavaRanch » Java Forums » Java » Beginning Java
Bookmark "Code critique: CSV to Tab-delimited" Watch "Code critique: CSV to Tab-delimited" New topic
Author

Code critique: CSV to Tab-delimited

Bruce Lentz
Greenhorn

Joined: Sep 16, 2012
Posts: 6
I have been asked to provide a code sample for an employer, for an entry-level position. I have been asked to write a java class that takes a tab-separated file and creates a CSV file, replacing the tabs with commas and reversing the order of the rows. It also must perform this operation in reverse with the second method. What I have created below works, but I am looking for a critique just to see if there are any places that I could improve my code.

Jeanne Boyarsky
internet detective
Marshal

Joined: May 26, 2003
Posts: 30361
    
150

It's fine the way it is. If you wanted to change something, you could merge the exception blocks. They aren't really doing anything different. Also, there is duplicate code in finally. Consider extracting to a separate method.


[Blog] [JavaRanch FAQ] [How To Ask Questions The Smart Way] [Book Promos]
Blogging on Certs: SCEA Part 1, Part 2 & 3, Core Spring 3, OCAJP, OCPJP beta, TOGAF part 1 and part 2
Bruce Lentz
Greenhorn

Joined: Sep 16, 2012
Posts: 6
Jeanne Boyarsky wrote:Also, there is duplicate code in finally. Consider extracting to a separate method.


Thanks for the reply Jeanne, the interface I am implementing only contains two methods - would it be considered bad form to add additional methods from the interface you are implementing?
Jeanne Boyarsky
internet detective
Marshal

Joined: May 26, 2003
Posts: 30361
    
150

Absolutely not. This would be a private method anyway. Which means it couldn't appear in an interface.
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 38474
    
  23
Welcome to the Ranch

I don’t understand the question about additional methods from the interface. If you implement an interface, you must implement all its methods or declare your class abstract. Remember all interface methods are implicitly public and not static.
Jeanne Boyarsky
internet detective
Marshal

Joined: May 26, 2003
Posts: 30361
    
150

Campbell,
He is saying "If I implement an interface, can my implementing class have more methods than my interface defines." And I don't see anything static in the original poster's example.
Bruce Lentz
Greenhorn

Joined: Sep 16, 2012
Posts: 6
Jeanne Boyarsky wrote:Campbell,
He is saying "If I implement an interface, can my implementing class have more methods than my interface defines." And I don't see anything static in the original poster's example.


Correct Jeanne, that was the question I was asking. Thanks for the answer.

This is actually my second go-around trying to submit this sample to the employer, the first time they told me it could have been implemented better using advanced algorithms (???). I guess I can't help but feel like there is a better way to get this task done.

Could my above code be improved using NIO?
Jeanne Boyarsky
internet detective
Marshal

Joined: May 26, 2003
Posts: 30361
    
150

It would be more elegant using Scanner. I didn't say anything because yours isn't wrong. Also, if you can use Java 7, you can condense the exception handling parts.

As far as advanced algorithms, the algorithm isn't complex as is. Making it more complicated isn't clearer. And I can't think of how to do so anyway.
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 38474
    
  23
Jeanne Boyarsky wrote:Campbell,
He is saying "If I implement an interface, can my implementing class have more methods than my interface defines." And I don't see anything static in the original poster's example.
So that is what it means. What I said about public and static was simply a reminder.
Olivier Legat
Ranch Hand

Joined: Nov 17, 2007
Posts: 176

Bruce Lentz wrote:This is actually my second go-around trying to submit this sample to the employer, the first time they told me it could have been implemented better using advanced algorithms (???). I guess I can't help but feel like there is a better way to get this task done.

Howdy Bruce,

Your code is genuinely good. I do however have one suggestion to make on how to improve it. From what I can tell convertFromTabToCSV(File inputFile) and convertFromCSVToTab(File inputFile) are essentially identical. I don't know if this is what Jeanne was referring to in his first post but here's how I'd have done it:



Hope this helps.


Olly
Bruce Lentz
Greenhorn

Joined: Sep 16, 2012
Posts: 6
Hope this helps.


Thanks for the help cowboy. I never thought to write the code like this.
Matthew Brown
Bartender

Joined: Apr 06, 2010
Posts: 4370
    
    8

Just to throw a spanner in the works....

...can you guarantee the original input doesn't contain any commas? I hope you can see how it would complicate things if you can't.
Olivier Legat
Ranch Hand

Joined: Nov 17, 2007
Posts: 176

Bruce Lentz wrote:
Hope this helps.


Thanks for the help cowboy. I never thought to write the code like this.


I'm glad I could be of assistance
Bill Clar
Ranch Hand

Joined: Sep 21, 2006
Posts: 150

My only suggestion is to use Generics with your Stack class. Other than that, it looks good.
Richard Tookey
Ranch Hand

Joined: Aug 27, 2012
Posts: 1044
    
  10

Acting as Devil's Advocate - what happens if any of the fields of your tab delimited file contain a comma?
Bruce Lentz
Greenhorn

Joined: Sep 16, 2012
Posts: 6
Richard Tookey wrote:Acting as Devils advocate - what happens if any of the fields of your tab delimited file contain a comma?


Great question, although the interface assumes that the .csv file fields contain no commas other than the separators.

I would guess you would have to ensure that the .csv fields were enclosed in quotes or double quotes, and then only separate each field by commas outside of the quotes?
Richard Tookey
Ranch Hand

Joined: Aug 27, 2012
Posts: 1044
    
  10

And then what happens if any field already contains a quote (pair matched or otherwise)?

P.S. I'm not suggesting you modify your code to deal with these problems but if I were interviewing someone I would expect them to have have at least considered them and be able to propose a solution.
Bruce Lentz
Greenhorn

Joined: Sep 16, 2012
Posts: 6
Richard Tookey wrote:And then what happens if any field already contains a quote (pair matched or otherwise)?

P.S. I'm not suggesting you modify your code to deal with these problems but if I were interviewing someone I would expect them to have have at least considered them and be able to propose a solution.

Then you use the vast amount of libraries that are already available to you? ;).

I give up.
dennis deems
Ranch Hand

Joined: Mar 12, 2011
Posts: 808
Most (if not all) of those instance fields would be better as method-local fields. To decide whether something makes sense as an instance field, consider whether it makes sense for that thing to be kept around for the entire life-cycle of the object. Once we have converted and returned the output file, do we really need to hold onto it? Similarly, the String and Stack fields are just tools we're using during the conversion -- they aren't important attributes of a Converter. After the conversion is completed we should throw those things away; they are just cluttering up our work space. In general, it's best to declare variables in the smallest necessary scope.
Matthew Brown
Bartender

Joined: Apr 06, 2010
Posts: 4370
    
    8

Bruce Lentz wrote:
Then you use the vast amount of libraries that are already available to you? ;).

Is the right answer. Or at least, it would be if I was interviewing. .

One more suggestion. Stack is a bit of a legacy class (extends Vector, should be an interface but isn't so you're tied to that implementation). I'd recommend using something that implements the Deque interface.
 
I agree. Here's the link: http://aspose.com/file-tools
 
subject: Code critique: CSV to Tab-delimited