Is this a correct way to utilize classes and methods in JAVA?

Question

I am currently learning JAVA. I barely have any coding experience. I wrote a program which asks a user for a sentence. It then swaps the words of the sentence (first with last, second with second to last, etc.). Program will then output the new sentence.

I know that this is not the most optimal way this task could be solved so please ignore that. My question is: "Do I use classes and methods correctly? If not, how would I adjust them in this code? In particular, I feel my main method looks weird (the way parameters are added to the methods)."

The code compiles and works as intended. My question is more in regards to style.

I add the code, in 2 classes

main class:

package com.test;
public class ND2_Task1 {
 public static void main(String[] args) {
 ND2_Task1_Core run = new ND2_Task1_Core();
 run.getInput(); // gets userSentence
 run.getWordCount(run.getUserSentence());
 run.checkValidity(run.getWordCount());
 run.toArray(run.getUserSentence()); // transforms String into an array
 run.reverseArray(run.getSeperatedSentence());
 run.showResult(run.getSeperatedSentence());
 }
}

2nd. class:

package com.test;
import java.util.Arrays;
import java.util.Scanner;
public class ND2_Task1_Core {
 private static String userSentence;
 private static int wordCount;
 private static String[] seperatedSentence;
 private static String[] reversedSentence;
 public void getInput() {
 System.out.println(
 "Please enter a sentence.\nFirst word will be swapped with the last one.\nSecond word will be swapped with second to last\netc.");
 setUserSentence(new Scanner(System.in).nextLine());
 }
 public void getWordCount(String userSentence) {
 wordCount = 1;
 for (int i = 0; i < userSentence.length(); i++) {
 if (userSentence.charAt(i) == ' ') {
 wordCount++;
 }
 }
 }
 public void checkValidity(int wordCount) {
 if (wordCount < 2) {
 System.out.println(
 "You have entered an insufficient number of words. Enter at least 2 words for program to swap them");
 System.out.println("Exiting now");
 System.exit(0);
 }
 }
 public void toArray(String userSentence) {
 setSeperatedSentence(userSentence.split(" "));
 }
 public void reverseArray(String[] seperatedSentence) {
 reversedSentence = new String[seperatedSentence.length];
 reversedSentence = seperatedSentence;
 for (int i = 0; i < reversedSentence.length / 2; i++) {
 String temp = reversedSentence[i];
 reversedSentence[i] = reversedSentence[reversedSentence.length - i - 1];
 reversedSentence[reversedSentence.length - 1 - i] = temp;
 }
 }
 public void showResult(String[] reversedSentence) {
 System.out.println(Arrays.toString(reversedSentence));
 }
 public String getUserSentence() {
 return userSentence;
 }
 public void setUserSentence(String userSentence) {
 ND2_Task1_Core.userSentence = userSentence;
 }
 public String[] getSeperatedSentence() {
 return seperatedSentence;
 }
 public void setSeperatedSentence(String[] seperatedSentence) {
 ND2_Task1_Core.seperatedSentence = seperatedSentence;
 }
 public int getWordCount() {
 return wordCount;
 }
 public void setWordCount(int wordCount) {
 ND2_Task1_Core.wordCount = wordCount;
 }
}
```

• markspace: If you're just starting, then it doesn't matter too much. Simple programs like this don't have an obvious "correct" way to write them. They're mostly procedural anyway and don't really benefit from object oriented design. When your assignments become more complex you'll see more opportunities to usefully use objects and breakdown code into structured design elements. One criticism is that you've put all your code into a single object, which is normally a poor choice. However the resulting object really isn't that complex, so it's fine.

Answer 1

I have some suggestion for you.

ND2_Task1 class

Instead of adding comment near the methods calls, I suggest that you rename the method to a name that explains what it's doing and remove the comment. Also, most of the method that has parameters can be refactored to use the internal reference of the variable.

The best way to do this, is to think the ND2_Task1_Core class as a container that doesn’t expose the variables with getter or setters, but instead have methods to handle the data directly, so the ND2_Task1 class can use them.

getInput Method

I suggest that you rename this method to askAndStoreUserSentenceInput and remove the comment.

public class ND2_Task1 {
 public static void main(String[] args) {
 //[...]
 ND2_Task1_Core run = new ND2_Task1_Core();
 run.askAndStoreUserSentenceInput();
 //[...]
 }
}

getWordCount method

This method is more than a getter, since it updates the wordCount; the name can be confusing, I suggest updateWordCount.

public class ND2_Task1 {
 public static void main(String[] args) {
 //[...]
 ND2_Task1_Core run = new ND2_Task1_Core();
 run.updateWordCount();
 //[...]
 }
}
 

checkValidity method

Same for other methods, you can remove the parameter and uses the local variable.

public class ND2_Task1 {
 public static void main(String[] args) {
 //[...]
 ND2_Task1_Core run = new ND2_Task1_Core();
 run.checkValidity();
 //[...]
 }
}
public class ND2_Task1_Core {
 //[...]
 public void checkValidity() {
 if (wordCount < 2) {
 System.out.println("You have entered an insufficient number of words. Enter at least 2 words for program to swap them");
 System.out.println("Exiting now");
 System.exit(0);
 }
 }
 //[...]
}

toArray method

In my opinion, the name is confusing; since it does more than convert to an array (also set the result).

Personally, I suggest that you make a new method called convertUserSentence in the class ND2_Task1_Core that handle the logic.

ND2_Task1

public class ND2_Task1 {
 public static void main(String[] args) {
 //[...]
 run.convertUserSentence();
 //[...]
 }
}

ND2_Task1_Core

public class ND2_Task1_Core {
 //[...]
 public void convertUserSentence() {
 String userSentence = getUserSentence();
 setSeperatedSentence(userSentence.split(" "));
 }
 //[...]
}

reverseArray method

You can remove the parameter and uses the local variable and rename the name to reverseSentenceArray.

ND2_Task1

public class ND2_Task1 {
 public static void main(String[] args) {
 //[...]
 run.reverseSentenceArray();
 //[...]
 }
}

ND2_Task1_Core

public class ND2_Task1_Core {
 //[...]
 public void reverseSentenceArray() {
 for (int i = 0; i < reversedSentence.length / 2; i++) {
 String temp = reversedSentence[i];
 reversedSentence[i] = reversedSentence[reversedSentence.length - i - 1];
 reversedSentence[reversedSentence.length - 1 - i] = temp;
 }
 }
 //[...]
}

showResult method

ND2_Task1

public class ND2_Task1 {
 public static void main(String[] args) {
 //[...]
 run.showResult();
 //[...]
 }
}

ND2_Task1_Core

public class ND2_Task1_Core {
 //[...]
 public void showResult() {
 System.out.println(Arrays.toString(reversedSentence));
 }
 //[...]
}

ND2_Task1_Core class

1. The use of the static keyword on the variables is useless in this case, since you have only one instance of the class ND2_Task1_Core. The static is used to share the same value over all the instances of the same class.

//[...]
private String userSentence;
private int wordCount;
private String[] seperatedSentence;
private String[] reversedSentence;
//[...]

2. Instead of creating a new instance of the Scanner each time you call the getInput method, I suggest that you store it in a variable.

public class ND2_Task1_Core {
 private Scanner scanner;
 public ND2_Task1_Core() {
 scanner = new Scanner(System.in);
 }
 public void getInput() {
 System.out.println("Please enter a sentence.\nFirst word will be swapped with the last one.\nSecond word will be swapped with second to last\netc.");
 setUserSentence(scanner.nextLine());
 }
}

Refactored code

ND2_Task1

public class ND2_Task1 {
 public static void main(String[] args) {
 ND2_Task1_Core run = new ND2_Task1_Core();
 run.askAndStoreUserSentenceInput();
 run.updateWordCount();
 run.checkValidity();
 run.convertUserSentence();
 run.reverseSentenceArray();
 run.showResult();
 }
}

ND2_Task1_Core

public class ND2_Task1_Core {
 private String userSentence;
 private int wordCount;
 private String[] seperatedSentence;
 private String[] reversedSentence;
 private Scanner scanner;
 public ND2_Task1_Core() {
 scanner = new Scanner(System.in);
 }
 public void askAndStoreUserSentenceInput() {
 System.out.println("Please enter a sentence.\nFirst word will be swapped with the last one.\nSecond word will be swapped with second to last\netc.");
 setUserSentence(scanner.nextLine());
 }
 public void updateWordCount() {
 wordCount = 1;
 for (int i = 0; i < userSentence.length(); i++) {
 if (userSentence.charAt(i) == ' ') {
 wordCount++;
 }
 }
 }
 public void checkValidity() {
 if (wordCount < 2) {
 System.out.println("You have entered an insufficient number of words. Enter at least 2 words for program to swap them");
 System.out.println("Exiting now");
 System.exit(0);
 }
 }
 public void reverseSentenceArray() {
 for (int i = 0; i < reversedSentence.length / 2; i++) {
 String temp = reversedSentence[i];
 reversedSentence[i] = reversedSentence[reversedSentence.length - i - 1];
 reversedSentence[reversedSentence.length - 1 - i] = temp;
 }
 }
 public void showResult() {
 System.out.println(Arrays.toString(reversedSentence));
 }
 public String getUserSentence() {
 return userSentence;
 }
 public void setUserSentence(String userSentence) {
 this.userSentence = userSentence;
 }
 public String[] getSeperatedSentence() {
 return seperatedSentence;
 }
 public void setSeperatedSentence(String[] seperatedSentence) {
 this.seperatedSentence = seperatedSentence;
 }
 public int getWordCount() {
 return wordCount;
 }
 public void setWordCount(int wordCount) {
 this.wordCount = wordCount;
 }
 public void convertUserSentence() {
 String userSentence = getUserSentence();
 setSeperatedSentence(userSentence.split(" "));
 }
}

• Kleronomas: Thank you! This is exactly what I was looking for and it will help me improve my understanding of how coding works a lot!

Answer 2

You seem to be under the impression that efficient code and good design are two separate things. A good design will help you write efficient code.

In this particular case, you need to learn about separation of concern. Specifically what is the user concerned about and what should be handled by the class?

The user basically just wants the words in the supplied string swapped. Everything else should be handled inside the class.

In the class itself, you don't really need all those properties and helper methods. The main purpose of the code can easily be summed in one function. I would suggest a utility class(StringUtils?) with a public static function to swap the words and return a string.

The solution could look like this:

Main.java

import java.util.Scanner;
class Main {
 public static void main(String[] args) {
 String sentence;
 System.out.println(
 "Please enter a sentence.\nFirst word will be swapped with the last one.\nSecond word will be swapped with second to last\netc.");
 try (Scanner sc = new Scanner(System.in)) {
 sentence = sc.nextLine(); 
 } 
 String swapped = StringUtils.swapWords(sentence);
 System.out.println(swapped);
 }
}

StringUtils.java

public class StringUtils {
 public static String swapWords(String input) {
 String[] usersWords = input.split(" ");
 if(usersWords.length < 2){
 inputError();
 }
 String[] words = new String[usersWords.length];
 int mid = (words.length / 2) + 1;
 for (int front = 0, back = words.length - 1; front < mid; ++front, --back) {
 words[front] = usersWords[back];
 words[back] = usersWords[front];
 }
 return String.join(" ", words);
 }
 private static void inputError(){
 System.out.println(
 "You have entered an insufficient number of words. Enter at least 2 words for program to swap them");
 System.out.println("Exiting now");
 System.exit(0); 
 }
}

Answer 3

I'm agree with all considerations expressed by @tinstaafl in his answer, so I'm adding just two thoughts.

First point: I noticed that you used the Arrays class in your code so if you want to reverse the String words array without any loop but just using the standard library you can do in one line with the help of Collections class like below:

Collections.reverse(Arrays.asList(words));

The cost is the creation of one List object , but it is a concise solution.

Second point : you called your classes ND2_Task1 and ND2_Task1_Core ; following the java code conventions for class names:

Class names should be nouns, in mixed case with the first letter of each internal word capitalized.

So you could rename them Nd2Task1 and Nd2Task1Core.