2
\$\begingroup\$

I am learning java and would like to create a very simple application but which uses SOLID principles.

The required functionality is: generate the alphabet with a possibility to select specific letters. For now there must be an option to select only even/odd letters but the project must be open for extra functionalities (which are unknown at the moment but may be defined in the near future).

I created a code but I got a feedback that my code is not using the Single Responsibility Principle. Could you please help me understand which part of my code is not in line with that principle and what can be done better?

abstract class Characters {
    private ArrayList<Character> array;

    public ArrayList<Character> getArray() {
        return array;
    }

    public Characters() {
        array = new ArrayList<>();
        for (int i = 0; i < 26; i++) {
            array.add((char) ('A' + i));
        }
    }

    public abstract void getModifiedArray(String arg);
}



class CharactersParity extends Characters {

    @Override
    public void getModifiedArray(String arg) {
        String arg2 = arg.trim().toLowerCase();

        switch (arg2) {
        case "even": {
            for (char c : getArray()) {
                if (c % 2 == 0) {
                    System.out.println(c);
                }
            }
        }
            break;
        case "odd": {
            for (char c : getArray()) {
                if (c % 2 != 0) {
                    System.out.println(c);
                }
            }
        }
            break;
        default:
            System.out.println("wrong parameter");
        }
    }
}
\$\endgroup\$
0

1 Answer 1

2
\$\begingroup\$

This may get hard, but you requested it... ;o)


Your program fails the SOLID principle (at least) in this ways:

Single responsibility principle

You mix the "business logic" with the user interface (UI).

The getModifiedArray method does the separation of the values and displays the result. Along the way it also fails to do what its name expresses.

Open/closed principle

Your class hierarchy is questionable.

OCP is about future changes, so imagine you want to enhance this program so that the user can select the number of characters to process.

SoC does not allow to put a int maxChars = new Scanner(System.in).nextInt(); into your Characters class.

So you have to either create a setter Metthod in Characters which extends the interface of CharactersParity too (violates Interface seggaration), or you create a parameterized constructor which forces you to create the same constructor in CharactersParity too.

The better way would be that CharactersParity does not inherit from Characters but gets a Charactersobject as constructor parameter to use:

class CharactersParity  {
   private final Characters characters;

   CharactersParity(Characters characters){
     this.characters = characters;
   }

public void getModifiedArray(String arg) {
    String arg2 = arg.trim().toLowerCase();

    switch (arg2) {
    case "even": {
        for (char c : characters.getArray()) {

    // ....

Liskov substitution principle

IMHO not applicable .

Interface segregation principle

The declaration of method getModifiedArray() in class Characters is not needed. It makes the interface of class Characters wider as is should be.

I guess the aim was to force other AnythingCharacters classes to have this method. In that case this method should be in a separate interface class:

public interface CharacterArrayModifier {
   void getModifiedArray(String arg);
}
class CharactersParity implements  CharacterArrayModifier{
   private final Characters characters;

   CharactersParity(Characters characters){
     this.characters = characters;
   }

@Override
public void getModifiedArray(String arg) {
// ...

Dependency inversion principle

Your class CharactersParity has 2 dependencies: Systen.out and Characters.

You get the first via static access and the second is bound via inheritance.

DI suggests to gather the dependencies outside the class needing it and injecting them into the using class. The using class stores them in internal variables.

This injection can be done in three ways:

  1. direct assignment
    The dependency object is directly assigned to the variable in the using object.

  2. setter injection
    the using object has a setter method to receive the dependency.

  3. constructor injection
    the using object gets the dependency as constructor parameter (as shown above).

IMHO the priority in usage should be 3, 1, 2.

The main advantage of "constructor injection" is that the dependency cannot be null by accident, (i.e. you simply did not type the assignment). The compiler forces you to pass a value. If this value is null then it is your intention...


As I indicated you program as some improper naming, but I think it is enough for now...

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.