-2

I am learning how to use arrays and I want to write a method that takes the given array(The alphabet), encrypts the array based on a formula, and prints the new array. My code is as follows:

import java.util.Scanner;

public class Encryption {
    public static void main(String[] args){
        String[] alphabet = {"A","B","C","D","E","F","G","H","I","J","K","L","M","N","O","P","Q","R","S","T","U","V","W","X","Y","Z"};
        Scanner in = new Scanner(System.in);
        System.out.println("Please select a number from 0-25: ");
        int key = in.nextInt();
        System.out.println(encryption(alphabet,key));

    }

    public static String[] encryption(String[] alphabet, int key){
        String[] newalph = new String[25];
        int alpha = 0;
        for(String a: alphabet){
            if ((0 <= a.indexOf(a) + key) && (a.indexOf(a) + key< 26)){
                 newalph[alpha] = alphabet[a.indexOf(a) + key];
                 alpha++;
            }
            else if ( a.indexOf(a) + key >= 26){
                newalph[alpha] = alphabet[((a.indexOf(a)) + key) - 26];
                alpha++;
            }
        }

        return newalph;
    }


}

I suspect that it's something inside the for loop but I'm not entirely sure.

Kingofthezachs
  • 9
  • 1
  • *I suspect that it's something inside the for loop but I'm not entirely sure.* - what do you mean you are not sure? The stack trace will tell you the statement that causes the problem. So display the value of your indexes. This means statements like `a.indexOf(a) + key` should be be assigned to a varaible. Then you use that variable as an index of the array. That way you can display the value of the index to see if it what you expect. – camickr Apr 18 '21 at 21:52
  • For what it's worth, this expression is always zero: `a.indexOf(a)`, where A is some String. It looks for "A" in "A", or "B" in "B", etc. And the index of any string in itself is zero. Always. – user15187356 Apr 18 '21 at 21:53
  • Thus the first `if` collapses to `if (key >= 0 && key < 26)`. The following statement becomes `newalph[alpha] = alphabet[key];` Both of these are a lot easier to understand than as written. – user15187356 Apr 18 '21 at 22:03
  • @user15187356 So how would I go about fixing the loop so that it doesn't print the same letter 26 times? – Kingofthezachs Apr 18 '21 at 22:16

3 Answers3

1

The length of your newalph array is only 25, whereas there are 26 letters in the alphabet.

String[] newalph = new String[26];

should fix the IndexOutOfBounds Error.

It's really easy to get confused since you would use [0] to access the first element of an array, so you might assume that when you create a new array, the number should be the actual length - 1. However, this is not the case when it comes to creating new arrays:

new String[n] creates an array with n elements, and the first element has index 0 while the last element has index n-1.

As mentioned in the comments by @user15187356, there are other issues in this program as well. a.indexOf(a) will always be zero. (Edit: Based on what your code is doing, you can safely remove it)

Parzival
  • 2,051
  • 4
  • 14
  • 32
  • Ahh, ok thanks. But now that that is fixed, when I try to print the array, it just prints the same letter 26 times, I'll look into that one now – Kingofthezachs Apr 18 '21 at 22:08
  • @Kingofthezachs Sorry I might be misinterpreted your program. You can simply delete `a.indexOf(a)` and it should work – Parzival Apr 18 '21 at 22:11
  • All it's really doing is checking that the key is in range 0 to 25 (inclusive) before using it as an index, or else if the key exceeds 25, reducing it by 26. (The case where the key exceeds 51 is still problematic). That much is ok. But *the bug* is that, each time around the loop, it selects `alphabet[key]` (or key-26), and key never changes. – user15187356 Apr 18 '21 at 22:14
  • I suspect what is intended is that the selected character is `alphabet[(alphabet.indexOf(a) + key) % 26]`. That is, it's a caesar cipher where the offset is the value of 'key'. OP, you should perhaps read about the remainder operator '%', it avoids the need for 'if' herre. – user15187356 Apr 18 '21 at 22:19
  • sorry @user15187356 last question, where would "alphabet[(alphabet.indexOf(a) + key) % 26]" go?(Sorry if I formatted that incorrectly) I've read your comments and thank you for being so helpful, I'm just a little confused on where/how that last part would be implemented – Kingofthezachs Apr 18 '21 at 22:29
  • The entire body of the for-loop can become `newalph[alpha++] = alphabet[(alphabet.indexOf(a) + key) % 26]` – user15187356 Apr 18 '21 at 22:41
0

You’re initialising newAlpha as 25 (0-24) when it should be 26 (0-25) so all you need to do is change newAlpha to 26

lukeyp
  • 13
  • 4
0

For your edification, I offer a more compact version of your encryption method. The main thing that makes this more compact is that, since deriving the encrypted letter requires indexing, then we use indexing throughout the method.

For each character in the alphabet (0..25) we determine the encrypted form by getting the character that's 'key' entries away, modulo the size of the alphabet.

public static String[] encryption(String[] alphabet, int key) {
    String[] newalph = new String[alphabet.length];
    for (int j=0; j<alphabet.length; j++) 
        newalph[j] = alphabet[(j+key) % alphabet.length];
    return newalph;
}

Other improvements:

  • Remove assumption that the 'alphabet' argument has exactly 26 letters; there's no need for that assumption, we can use the actual size.

  • Use of the remainder operator to avoid 'if' statements. Note that this also makes it safe if the key is 'large' compared to the alphabet, say greater than 52. (Behaviour with key < 0 should be checked; I didn't consider it).

user15187356
  • 807
  • 3
  • 3