Disclaimer: I'm a Rust beginner with a background in C and C++. Now that I've lost my credibility, let's have a look at your code.
Use rustfmt, clippy and rustc
There are (at least) three helpful tools when one writes Rust code: rustfmt, clippy and the compiler rustc itself.
Rustfmt provides a common ground
The compiler doesn't yield any warnings, but rustfmt will change your code.
As many Rust developers use rustfmt (or cargo fmt, which calls the former), you should also make a habit of using it. That way others will feel right at home if they look at your code, and the same will hold for you when you look at theirs.
Clippy's warnings
clippy on the other hand will catch common mistakes. In this case its mostly the redundant returns in your idx functions:
fn get_transpose_table(keyword: String) -> Vec<usize> {
keyword
.chars()
.map(|character| char_to_alpha_idx(character))
.collect()
}
fn char_to_alpha_idx(character: char) -> usize {
"abcdefghijklmnopqrstuvwxyz"
.find(character)
.expect("Bad char for char_to_alpha_idx")
}
fn alpha_idx_to_char(loc: usize) -> char {
"abcdefghijklmnopqrstuvwxyz"
.chars()
.nth(loc)
.expect("Bad loc for alpha_idx_to_char")
}
There is no need to return here. Next, you never consume the String in get_transpose_table, so we could borrow it instead with &String. But in the end, we never use any String-specific functionality, therefore we can just use &str.
While we're at it, map(|x| func(x)) is just map(func), so let's change that as-well:
fn get_transpose_table(keyword: &str) -> Vec<usize> {
keyword.chars().map(char_to_alpha_idx).collect()
}
All those small improvements were reported by clippy.
Use T::with_capacity if you know how many elements you will have
For your main algorithm, you should use String::with_capacity if you know the number of characters you want to store:
let mut output = String::with_capacity(input.len());
Nothing else in your code changes.
Closures and readability
Apart from better names (message instead of body, body_index or message_ index instead of input_index) that's all we can do with this algorithm. We could transform it into a closure:
let output: String = message
.char_indices()
.map(|(index, message_char)| {
let transpose_distance = transpose_table[index % transpose_table.len()];
let transposed_char_idx = char_to_alpha_idx(message_char) + transpose_distance;
alpha_idx_to_char(transposed_char_idx % 26)
})
.collect();
but we only removed mutability. So let's get back to the drawing table. Why do we need index? Because we want to know which usize we should use from the transpose_table. We would like to traverse a cycle of those values at the same time as our original message.
That's exactly what cycle() and zip() are for:
let output: String = transpose_table
.iter()
.cycle()
.zip(message.chars())
.map(|(transpose_distance, message_char)| {
let transposed_char_idx = char_to_alpha_idx(message_char) + transpose_distance;
alpha_idx_to_char(transposed_char_idx % 26)
})
.collect();
Whether that's easier to understand than your original variant is left to you. However, we can also get rid of the intermediate Vec:
let output: String = keyword
.chars()
.map(char_to_alpha_idx)
.cycle()
.zip(message.chars())
.map(|(transpose_distance, message_char)| {
let transposed_char_idx = char_to_alpha_idx(message_char) + transpose_distance;
alpha_idx_to_char(transposed_char_idx % 26)
})
.collect();
This is a handful, but it works exactly as your previous variant.
Encapsulate functionality in functions
Now, my main critique point is that you don't provide a single function for your cipher. It's hard to test your functionality at the moment, so we better add one and some documentation:
/// Applies [The Alphabet Cipher] given by `keyword` to `message`.
///
/// Both `message` and `keyword` must only contain lowercase alphabetic ASCII
/// characters.
///
/// # Panics
/// Panics if any non-lowercase or non-alphabetic ASCII character is encountered.
///
/// # Examples
/// The character 'a' in the keyword won't change the input:
/// ```
/// assert_eq!(alphabet_cipher("aaaaa", "hello"), "hello");
/// ```
///
/// They keyword gets cycled if its shorter than the message:
/// ```
/// assert_eq!(alphabet_cipher("a", "hello"), "hello");
/// assert_eq!(alphabet_cipher("b", "hello"), "ifmmp");
/// assert_eq!(alphabet_cipher("ab","hello"), "hflmo");
/// assert_eq!(alphabet_cipher("ba","hello"), "iemlp");
/// ```
///
/// [The Alphabet Cipher]: https://en.wikipedia.org/wiki/The_Alphabet_Cipher
fn alphabet_cipher(keyword: &str, message: &str) -> String {
keyword
.chars()
.map(char_to_alpha_idx)
.cycle()
.zip(message.chars())
.map(|(offset, message_char)| {
let index = char_to_alpha_idx(message_char) + offset;
alpha_idx_to_char(index % 26)
})
.collect()
}
The code snippets in the comments also get tested by cargo test, but we can also add some more tests to check that our functions work, for example:
#[test]
fn alphabet_cipher_rot_a_to_z() {
assert_eq!(
alphabet_cipher("abcdefghijklmnopqrstuvwxyz", "aaaaaaaaaaaaaaaaaaaaaaaaaa"),
"abcdefghijklmnopqrstuvwxyz"
);
}
There are more tests one can come up with, but those are left as an exercise.
User friendly interface
You use unwrap() quite a lot. However, that leaves a user with unhelpful error messages:
$ ./cypher test
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', ...
$ ./cypher test test12345
thread 'main' panicked at 'Bad char for char_to_alpha_idx', ...
Those error messages aren't that helpful. They're fine for toy programs, but you should view unwrap as "I know that this will be Ok or Some, and it's fine to panic if it isn't, because I don't know how to continue". However, even in those cases, the end user should know how to continue. Keep that in mind when you write larger programs.
