HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Word duplicate finder and counter

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
duplicatefinderwordandcounter

Problem

```
import java.util.Collections;//for frequency of words
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.ArrayList;
import java.util.Scanner;
import java.util.Arrays;
/**
Output--In phrase count duplicate substrings or strings or with space
psuedo code---
1.splt phrase by particular length
get list of each particular splited length word
for each word in list
for each word count if its count> 1 after having counted at least one, remove duplicates from List
getword and its frequency
2.split phrase with space then get frequency of words
3.splt phrase by particular length space included
then get frequency of words
*/
public class DuplicateSubString {
//@SuppressWarnings("unchecked")
public static void main(String[] args) {

// String phrase = "a r b k c d se f g a d f s s f d s ft gh f ws w f v x s g h d h j j k f sd j e wed a d f";
//String phrase ="How do you do in How";
String phrase ="How do you do in Howdy How";

Scanner sc = new Scanner( System.in );
System.out.println( "Please enter how many characters of grouping between 0 to 5" );
int splitleng=splitleng = sc.nextInt();
if(splitleng>0){
int string_to_test=1;
System.out.println( "Please enter type 1 for with space or else without space " );
string_to_test=sc.nextInt();
if(string_to_test==1){
System.out.println( "split with specified group of characters space included");
getListwithFreq(listOfWords( phrase,splitleng));
}else{
System.out.println( "split phrase with space & check also in substring for repition");
getListwithFreq(wordRepitionList( phrase, splitleng));
}

}else{
System.out.println( "space split and get frequency of words");
getListwithFreq(wordRepitionList( phrase));
}

Solution

Spaces

I see many code segments like this:

splitleng>0
string_to_test=1
string_to_test=sc.nextInt()


Put spaces between your two expressions to result in this:

splitleng > 0
string_to_test = 1
string_to_test = sc.nextInt()


I consider the latter a lot more readable.

As a sidenote (but this is a matter of preference): I'm not a fan of adding spaces inside brackets like here:

getListwithFreq(wordRepitionList( phrase, splitleng));


That space doesn't add any readability and it is very inconsistently used in your code. I prefer this:

getListwithFreq(wordRepitionList(phrase, splitleng));


Even more about spaces: consider the following method declaration:

private   static List listOfWords(String phrase,int splitleng ){


Can you spot the 3 comments I will make about spacing?
Naming

Keep naming conventions in mind. A method should be in the form of ` with an obvious example for a getter: getFirstName(). At first glance I can immediately say what this method will do: it will return me data and I know what data.

Will I know what it does when its name is
listOfWords()? Using get- and set- prefixes are very common and you should try to use them wherever applicable.

Same goes for capitalization! In Java, we use
lowerCamelCase for our methods. This means that

getListwithFreq


becomes

getListWithFreq


Aside from that: write the entire word, don't shorten it.
Freq should become Frequency. Also try to make your name more descriptive: right now it seems as if your method will take 2 arguments, a list and a frequency. This is not reflected in your method's actual signature.

Another note: your class is named
DuplicateSubString. This bears no meaning in itself and thus I would suggest something that actually describes what it does: DuplicateSubStringFinder.
Encapsulation

Encapsulation exists in many forms but in the end they all have one thing in common: hide implementationd details.

You have a variable named
wordsWithCountHMap. What if you decide to use a TreeMap later on? Now you'll have to change the name of this variable! That's unacceptable, programmers are supposed to be lazy. Hide the actual implementation details by simply not putting it in the name (you're not having your firstName declared as firstNameString either).
Declaration

This code

ArrayList wordlist = new ArrayList();


Can be abbreviated to (notice the name- and spacingchanges)

List wordList = new ArrayList<>();


Assuming you use at least Java 7. For more information on this style, read this post.

You're already partially doing this here

Map wordsWithCountHMap=new HashMap();


so this can be turned just as well into

Map wordsWithCountHMap = new HashMap<>();


Keep to your own

Your method
printMap is declared as public but should only get called from inside your current class. Make it private instead.
Static

All your methods are declared as
static so they can directly be accessed from your main method. While it seems easier, it's also useless and curses at OO programming.

Instead, make them all instance methods (aka: remove the
static modifier) and create an instance of DuplicateSubString` instead. Now you can work with this class and it's a lot more OO-oriented.
Conclusion

All together you have many recurring code issues but because many of them can be categorized in the same aspects you will find it easy to pull your game to the next level if you adapt just a few changes.

Your code will become a lot easier to read and interpret by adding some spacing and choosing clearer names.

Code Snippets

splitleng>0
string_to_test=1
string_to_test=sc.nextInt()
splitleng > 0
string_to_test = 1
string_to_test = sc.nextInt()
getListwithFreq(wordRepitionList( phrase, splitleng));
getListwithFreq(wordRepitionList(phrase, splitleng));
private   static List<String> listOfWords(String phrase,int splitleng ){

Context

StackExchange Code Review Q#46257, answer score: 6

Revisions (0)

No revisions yet.