1
\$\begingroup\$

I've been doing a Java for beginners course for a few weeks (2 hours a week), we had an exam question.

Take a sentence as an input from a user for twitter. Using only String Arrays and String Buffers (do not use split() or any other methods not covered in class yet). Separate the sentence into words and add to an array. Then change any instance of the word "cannot" with "can't". Must use an instantiable class and all relevant set, compute, get methods covered. Comment all code to show you understand what everything you've used is doing.

This is the code I submitted (it works), looking for feedback before the results come out to see what I can expect.

Thanks

App Class enter code here

public class tweetApp{
    public static void main(String arg[]){
        //decalre variables
        String sentence;
        //create an array for the new sentence
        String [] newSentence;

        //instaniate classes 
        tweet myTweet;
        Scanner input;
        //creat classes
        myTweet = new tweet();
        input = new Scanner(System.in);


        System.out.print("Enter a sentence: ");
        sentence = input.nextLine();

        //set method for the sentence inputted from the user
        myTweet.setSentence(sentence);
        //compute method for chaging the sentence given by the user
        myTweet.computeSentence();
        //set the array to the new changed sentence
        newSentence = myTweet.getNewSentence();
        //output each section of the array and a space, to print out the entire message
        for(int y = 0; y < newSentence.length; y++){
            System.out.print(newSentence[y]+" ");

        }
        //end the program by adding a blank print line to make sure the command line looks ok.
        System.out.println();
    }
}

enter code here

Instantiable class

enter code here
public class tweet{
    //declare variables
    private String sentence;
    private int strlen;
    private StringBuffer word;
    private String newSentence[];
    private String newWord;
    private int wordCount;
    //create constuctor
    public tweet(){
        sentence = " ";
        strlen = 0;
        word = new StringBuffer();
        wordCount = 1; //starting at 1 if user enters 1 word
        newSentence = new String[wordCount];
        newWord = " ";  
    }
    //set method
    public void setSentence(String sentence){
        this.sentence = sentence;
    }
    //compute method
    public void computeSentence(){
        strlen = sentence.length();//get length of sentence 
        for(int x = 0; x < strlen; x++){//transverse the string,
            if(sentence.charAt(x)==' '){//calculate word count  
                wordCount = wordCount +1;
            }
        }
        newSentence = new String[wordCount]; //use word count to set size of the array 
        for(int j = 0; j < newSentence.length; j++){//transverse the array,
            for(int i = 0; i< strlen; i++){//transverse string,
                if(sentence.charAt(i) == ' '){//check if char is a space,
                    j = j+1;//if so,increment j by 1
                    word = new StringBuffer();//start a new buffer
                }
                else{
                    word.append(sentence.charAt(i));//char at i, append to buffer,  
                    newWord = word.toString();//convert buffer to string,
                    newSentence[j] = newWord;//add to j'th place of array
                    }
            }
        }
        for(int x = 0; x < newSentence.length; x++){//change any instance of cannot to can't
            if(newSentence[x].equalsIgnoreCase("cannot")){
                newSentence[x] = "can't";
            }
        }
    }
        //get method, returning new sentence to app class
        public String[] getNewSentence(){
            return newSentence;
        }
    }
\$\endgroup\$
1
\$\begingroup\$

Thanks for sharing your code,

This looks very good for just having a few weeks' experience.

Let's start with a some small things,

1 Initialization vs declaration

this line of code tweet myTweet; declares a variable, while this line of code myTweet = new tweet(); instantiates it. When possible it's preferable to do both on the same line. This has a few benefits.

  • There's less change for things to go wrong. In a larger application, there could be many things happening between declaration and initialization leading to potentially difficulty to track down bugs.
  • It makes the code easier to follow for other developers (and future you)

for these reasons, I would prefer to see

  • tweet myTweet = new tweet();
  • Scanner input = new Scanner(System.in);
  • String sentence = input.nextLine();

etc.

2 Java naming conventions

Classes should start with a capital letter. You'll notice all the standard library classes begin with a capital letter (String, List, Scanner etc.) while primitive types (int, float, double) start with a lower case letter. With this in mind, it would be better to have your class called Tweet

3-0 Add setters to classes only if you absolutely need to

If your code, you call myTweet.setSentence(sentence); setters generally exist when you need to update the state of an object some time after its creation, in this case, we update it right away! Why not simply pass sentence as another constructor argument, it could look like this.

Scanner input = new Scanner(System.in);
String sentence = input.nextLine();
Tweet myTweet = new Tweet(sentence);

No need to update the Tweet once it has been created.

3-1

Staying on the topic of unnecessary mutation, we call myTweet.computeSentence(); the first thing I notice about this method is that it has a void return type. This usually means one thing. side effects. In this context, a side effect means we call a method and something else is happening (in this case, we're mutating the state of the object.)

One other potential problem, is that in order for me to get the expected result from myTweet.getNewSentence(); I need to remember to call myTweet.computeSentence(); first! In a small program, not a big deal, but as our objects get more and more complicated, this becomes more of an issue. What if my entire main method could look like this.

public static void main(String[] args) {
    Scanner input = new Scanner(System.in);
    System.out.print("Enter a sentence: ");
    String sentence = input.nextLine();
    Tweet myTweet = new Tweet(sentence);
    String [] newSentence = myTweet.getNewSentence();
    for(int y = 0; y < newSentence.length; y++){
        System.out.print(newSentence[y]+" ");
    }
    System.out.println();
}

This is very similar to what you already have. But there area few notable changes. 1. I declare and initialize variables at the same time. 2. I pass all dependencies to the Tweet constructor (the sentence) at creation time. 3. I don't need to remember to call certain setup methods (computeState) 4. Conform to Java naming standards (Tweet vs tweet)

4 Storing unneeded instance variables

Let's take a quick look at the Tweet class, so we store all these

private String sentence;
private int strlen;
private StringBuffer word;
private String newSentence[];
private String newWord;
private int wordCount;

I was curious as to why we need to store all these. In our constructor, we initialize a lot of them. Why not just do that in the getNewSentence method instead

We end up with a Tweet class that looks like

public class Tweet {

    private String sentence;

    public Tweet(String sentence){
        this.sentence = sentence;
    }

    //get method, returning new sentence to app class
    public String[] getNewSentence(){
        StringBuffer word = new StringBuffer();
        String newWord = " ";
        int wordCount = 1;


        int strlen = sentence.length();//get length of sentence
        for(int x = 0; x < strlen; x++){//transverse the string,
            if(sentence.charAt(x)==' '){//calculate word count
                wordCount = wordCount +1;
            }
        }
        String[] newSentence = new String[wordCount]; //use word count to set size of the array
        for(int j = 0; j < newSentence.length; j++){//transverse the array,
            for(int i = 0; i< strlen; i++){//transverse string,
                if(sentence.charAt(i) == ' '){//check if char is a space,
                    j = j+1;//if so,increment j by 1
                    word = new StringBuffer();//start a new buffer
                }
                else{
                    word.append(sentence.charAt(i));//char at i, append to buffer,
                    newWord = word.toString();//convert buffer to string,
                    newSentence[j] = newWord;//add to j'th place of array
                }
            }
        }
        for(int x = 0; x < newSentence.length; x++){//change any instance of cannot to can't
            if(newSentence[x].equalsIgnoreCase("cannot")){
                newSentence[x] = "can't";
            }
        }
        return newSentence;
    }
}

With this class, we don't need to worry about who used it before, or how they used it. It will never be in a bad state we can call getNewSentence all day long and it will always return the same thing, no need to worry about set up with setSentence or computerState

Hopefully you found this review useful, I'd like to reiterate again that this looks very good for just a few weeks! Keep it up!

\$\endgroup\$
  • \$\begingroup\$ Thank you for the review, extremely useful! Even though I've already submitted the code for grading I'm going to rewrite with the pointers you've given me and hopefully I can learn a bit more from it, very much appreciated. \$\endgroup\$ – Zap-Brannigan May 1 at 0:47

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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