Review

[oleksandr-jr]

Hello!

I see that you have done a great job. Many decisions in the code are well thought out. The project structure is well designed. There are certain things that can be improved in terms of architecture. However, we have not yet taught these topics. I spent a lot of time reviewing the project and it was worth it. You did a great job.

Some tips for improvement:

First of all make an pull request from develop brunch to the main.

There is some problem which I haven't solved. It is passing a key to the method when a BRUTE FORCE command is needed. Especially since we don't need any key with brute force command starts, but my method is universal for all commands ENCRYPT, DECRYPT and BRUTE FORCE and I have just passed some random number at all. Please, could you give me some hint how make it better.

There is many possible solutions for this case. For example:

• add encrypt, decrypt, bruteForce methods with different params to the Encrypter interface and call them when its needed. in this case your Provider can be a facade implementation.
• break the method into multiple methods, each which require only a subset of the parameters. You can use method overloading.
• create helper classes to hold group of parameters (typically static member classes). And pass an object which contains all params instead of passing those params separately.
• adapt the Builder pattern from object construction to method invocation.
• adapt factory method.

Your brute force algorithm gives all possible variations of the decrypted text, which is not exactly what was expected. But it works, and I can say that it's fine.

in command enum you can implement equals method.

public enum Command {
    ENCRYPT, DECRYPT, BRUTE_FORCE;

    public boolean equals(String value){
        return this.name().equalsIgnoreCase(value);
    }
} 

From this: if (command.equals(String.valueOf(Command.ENCRYPT))) you will get this: if (Command.ENCRYPT.equals(command))

It's much better and safe way to compare commands.

I noticed that you forget to indicate the groupId for maven-compiler-plugin https://github.com/AndriiChipets/Encrypter/blob/6b4bef094259533587aebc7ce813dc89f01a3479/pom.xml#L52-L58

Should be like this:

<plugin>
    <groupId>org.apache.maven.plugins</groupId>
    <artifactId>maven-compiler-plugin</artifactId>
    <version>${maven.compiler.plugin.version}</version>
    <configuration>
        <release>${java.version}</release>
    </configuration>
</plugin>

Tests:

Its very cool that you wrote all this test for your project. Have you ever had experience with testing before?)

My draft test results:

Good luck :)

[AndriiChipets-Java]

Dear, Oleksandr. Thanks a lot for checking my code and especially for your pieces of advice. I really appreciate it! I am going to implement all of them for refactoring my code. I have had testing experience before with the FoxMinded IT course. I have studied there from January this year. I have made 4 small projects and all of them have been made with tests. Thanks one more time and have a nice day!

[AndriiChipets-Java]

Hi, Oleksandr. I have done everything that you told me. Now my BRUTE FORCE method should work according to technical specification and it should pass your BRUTE_FORCE test. Also it is really weird that my program hasn't passed your validation tests (negative key and File do not exist), I can suggest that it is because I catch an exception which the program throws and throw another exception which your test program doesn't expect. Have a nice day!