1
\$\begingroup\$

The problem is stated in Encrypt this!:

Your message is a string containing space separated words. You need to encrypt each word in the message using the following rules:

  • The first letter needs to be converted to its ASCII code.

  • The second letter needs to be switched with the last letter

  • Keepin' it simple: There are no special characters in input.

Kata.encryptThis("Hello") => "72olle"

Kata.encryptThis("good")` => "103doo"

Kata.encryptThis("hello world") => "104olle 119drlo"

class NewEnpc{
    public static String encryptThis(String text) {
        StringBuilder sb=new StringBuilder();
        String[] arry=text.split(" ");
        if(text!=""){
            for(int j=0;j<arry.length;j++){
                char[] sd=arry[j].toCharArray();
                int number=(int)sd[0];
                if(sd.length>1){
                    for(int k=0;k<sd.length;k++){
                        if(k==0){
                            sb.append(number);
                        }
                        else if(k==1){
                            int len=sd.length-1;
                            char c=sd[len];
                            sb.append(c);
                        }
                        else if(k==sd.length-1){
                            char c=sd[1];
                            sb.append(c);
                        }
                        else{
                            char c=sd[k];
                            sb.append(c);
                        }
                    }
                }
                else{
                    sb.append(number);
                }
                sb.append(" ");
            }
        }
        return sb.toString().trim();
    }
}

public class EncryPt {
    public static void main(String[]args){
        System.out.println("Hello");
         
      System.out.println(NewEnpc.encryptThis("A 
        wise old owl lived in an oak"));
    }
}

Can someone help me improve my coding standards. I have solved the problem.

\$\endgroup\$
1
  • 3
    \$\begingroup\$ Did you write any unit tests for this code? You should do this, and add them to your code, to show us examples of plain text and the corresponding "encrypted" text. By reading the unit tests, we can also tell you which corner cases you forgot to test. The one I'm thinking about has several spaces in a row, like "three spaces" (I actually wrote 3 spaces between the words, but they are not shown as such here). Your code should not crash while encrypting this text. And what about the text "hello, world", is it encrypted as you expect it to be? \$\endgroup\$ – Roland Illig Nov 24 '19 at 7:24
4
\$\begingroup\$

I would suggest following:

  1. Do not compare strings by ==, use String.equals(). And in Your case is better String.isEmpty()

    text != ""!text.isEmpty()

  2. Use constants for numbers and strings. It increases readability and futher refactoring.

    private static final String WORD_SEPARATOR = " ";

  3. There is no need to use indexed iterations

    for(int j=0;j<arry.length;j++)for (String s : arry)

  4. Use IDE with spelling checker:

    arryarray

  5. Use meaning variable names.

    String sString word

  6. No need to use cast:

    int number = (int) sd[0];int number = sd[0];

  7. Decrease level of visibility

    public static String encryptThisstatic String encryptThis

  8. Use method extraction to decrease nesting level

    private static StringBuilder encrypt(char[] wordChars, int number)

  9. You add first char in any case so condition can be omitted.

  10. You could use chain of method .append(...) calls for StringBuilder

  11. It is nice to move conditions like k==1 to meaningful named method:

    See isSecondChar(...)

  12. It is nice to validate input params early and prevent any actions if it is not valid:

    if(text!="")if (text.equals("")) then return empty string;

I would strongly suggest you to read great Joshua Bloch 'Effective Java' book.

Have a look at the refactored code:

class NewEnpc {

    private static final String WORD_SEPARATOR = " ";

    static String encryptThis(String text) {
        if (text.isEmpty()) {
            return text;
        }
        StringBuilder result = new StringBuilder();
        String[] words = text.split(WORD_SEPARATOR);
        for (String word : words) {
            char[] wordChars = word.toCharArray();
            result.append((int) wordChars[0])
                    .append(encrypt(wordChars))
                    .append(" ");
        }
        return result.toString().trim();
    }

    private static StringBuilder encrypt(char[] wordChars) {
        StringBuilder encrypted = new StringBuilder();
        for (int index = 1; index < wordChars.length; index++) {
            if (isSecondChar(index)) {
                int len = wordChars.length - 1;
                encrypted.append(wordChars[len]);
            } else if (isLastChar(index, wordChars.length - 1)) {
                encrypted.append(wordChars[1]);
            } else {
                encrypted.append(wordChars[index]);
            }
        }
        return encrypted;
    }

    private static boolean isLastChar(int currentIndex, int prevCharIndex) {
        return currentIndex == prevCharIndex;
    }

    private static boolean isSecondChar(int currentIndex) {
        return isLastChar(currentIndex, 1);
    }
}

public class EncryPt {
    public static void main(String[] args) {
        System.out.println("Hello");
        System.out.println(NewEnpc.encryptThis("A wise old owl lived in an oak"));
    }
}
\$\endgroup\$
1
\$\begingroup\$

The solution I'm proposing is based on these rules and examples, so I suppose a generic message contains words of length more than 1 character and composed of letters and numbers. The edge cases like empty strings or others to pass other tests on the site can be added to my code. My class contains a main method including the tests:

public class NewEnpc {

    public static void main(String[] args) {
        assertEquals(encryptThis("Hello"), "72olle");
        assertEquals(encryptThis("good"), "103doo");
        assertEquals(encryptThis("hello world"), "104olle 119drlo");
    }
}

The method encryptThis splits the message by space into words, encrypts every single word and finally return the encrypted message: this can be done like the code below:

public static String encryptThis(String message) {
    String sep = " ";
    String[] words = message.split(sep);
    StringJoiner joiner = new StringJoiner(sep);

    for (String word : words) {
        String encryptedWord = encryptWord(word);
        joiner.add(encryptedWord);
    }

    return joiner.toString();
}

I used the class StringJoiner to obtain the encrypted message. The method encryptWord takes a word as a parameter and encrypt it using rules I quoted at the beginning of my answer:

private static String encryptWord(String word) {
    char[] arr = word.toCharArray();
    final int n = arr.length;
    int asciiFirstChar = arr[0];
    char secondChar = arr[n - 1];
    char lastChar = arr[1];

    StringBuilder builder = new StringBuilder(Integer.toString(asciiFirstChar));
    builder.append(secondChar);
    builder.append(word.substring(2, n - 1));
    builder.append(lastChar);

    return builder.toString();
}

Below all the code of the class:

public class NewEnpc {

    public static void main(String[] args) {
        assertEquals(encryptThis("Hello"), "72olle");
        assertEquals(encryptThis("good"), "103doo");
        assertEquals(encryptThis("hello world"), "104olle 119drlo");
    }

    public static String encryptThis(String message) {
        String sep = " ";
        String[] words = message.split(sep);
        StringJoiner joiner = new StringJoiner(sep);

        for (String word : words) {
            String encryptedWord = encryptWord(word);
            joiner.add(encryptedWord);
        }

        return joiner.toString();
    }

    private static String encryptWord(String word) {
        char[] arr = word.toCharArray();
        final int n = arr.length;
        int asciiFirstChar = arr[0];
        char secondChar = arr[n - 1];
        char lastChar = arr[1];

        StringBuilder builder = new StringBuilder(Integer.toString(asciiFirstChar));
        builder.append(secondChar);
        builder.append(word.substring(2, n - 1));
        builder.append(lastChar);

        return builder.toString();
    }

}
\$\endgroup\$

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.