Win a copy of Think Java: How to Think Like a Computer Scientist this week in the Java in General forum!
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

How Elegant Is This Solution? Can You Offer Me Recommendations?

 
Nolan Lefler
Greenhorn
Posts: 8
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Can you tell me how elegant this solution is for the problem? I tried using as little code as required. This is basically listing how many times a word appears in a line of text. I have converted the text to a string array.

 
Ryan Sykes
Ranch Hand
Posts: 58
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I'll let someone more experienced comment on elegance, but I wanted to point out that your code fails to identify words that are adjacent to punctuation marks. The presence of a period, comma or question mark next to a word will give you faulty results.
 
Nolan Lefler
Greenhorn
Posts: 8
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Ryan Sykes wrote:I'll let someone more experienced comment on elegance, but I wanted to point out that your code fails to identify words that are adjacent to punctuation marks. The presence of a period, comma or question mark next to a word will give you faulty results.


I've fixed that issue. However, if somebody enters a word that has punctuation in the middle of the word, such as 'it's' or 'jack-in-the-box' it will make these words 'its' and 'jackinthebox'

 
Tuah Fadzli
Greenhorn
Posts: 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
How about this? it will show all words that found in sentences
 
Nolan Lefler
Greenhorn
Posts: 8
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Mr Tuah wrote:How about this? it will show all words that found in sentences


That looks nice to me Mr Tuah, but my code requires the number of times a word occurs in a string to be counted.
 
Campbell Ritchie
Sheriff
Posts: 48917
58
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Welcome to the Ranch

Both examples are very poor because they have too long a main method. Ideal length of main method: one statement.
Use of static members appears inappropriate.
Why are you using an array to store the Strings? There are far more appropriate data structures.
Failure to close readers. Failure to handle exceptions locally.
 
Matthew Brown
Bartender
Posts: 4567
8
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
You're also looping through the array far more than is necessary. You've got a main loop, but inside that you keep looping up to the current point to find out if you've found the word yet, then loop from that point onwards to count them.

My first thought would be to use a HashMap, containing words mapped against the number of times they occur. Then you only have to loop through the array once - check if the word is in the map (a very fast operation - HashMaps are designed to be good at that), add it if not, and increment the count if it is. Then loop once through the map to print the values.
 
James Boswell
Bartender
Posts: 1051
5
Chrome Eclipse IDE Hibernate
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Matthew beat me to the suggestion of a HashMap:

HashMap<String, int> wordMap;

where String is a word found and int is the number of occurrences.
 
Matthew Brown
Bartender
Posts: 4567
8
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
James Boswell wrote:HashMap<String, int> wordMap;

Make that HashMap<String, Integer> - you can't use primitives as a generic type.
 
Winston Gutkowski
Bartender
Pie
Posts: 10417
63
Eclipse IDE Hibernate Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Matthew Brown wrote:Make that HashMap<String, Integer> - you can't use primitives as a generic type.

A HashMap<String, AtomicInteger> allows for the counts to be incremented rather than replaced, and so might be a bit faster; or you could create your own Counter class.

Winston
 
James Boswell
Bartender
Posts: 1051
5
Chrome Eclipse IDE Hibernate
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks for correction Winston, obviously I was just testing you!
 
Campbell Ritchie
Sheriff
Posts: 48917
58
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
If you search for the Java Tutorials and find the section about the Map interface, there is an example of counting with a Map.
 
Nolan Lefler
Greenhorn
Posts: 8
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks for the suggestions. I haven't encountered HashMaps yet, but I'll look into them.

But I've never heard the main method should only being one line of code. That's quite new to me.
 
Campbell Ritchie
Sheriff
Posts: 48917
58
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
The main method is intended for starting your application. You can often do that with a class you instantiate and call one method onOne line. I know people will disagree with me on that.

You want a class which runs your application, and you want objects which you manipulate. You are nowhere creating an object which runs your application. In the case of a little application like that, you can get away with few objects; a larger application will require many objects to run it.
 
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic