Is this correct?

[nothing]

This book I'm reading has this exercise and I wrote an answer for it but I would like to know if I got it right.

"Your program should input a four-digit integer and encrypt it as follows. Replace each digit by ( the sum of that digit plus 7 ) modulus 10. Then, swap the first digit with the third, swap the second digit with the fourth and print the encrypted number."


#include <iostream>

using namespace std;

int main()
{
int number;
int firstDigit;
int secondDigit;
int thirdDigit;
int fourthDigit;
int originalFirstDigit;
int originalSecondDigit;
int originalThirdDigit;
int originalFourthDigit;

cout << "Enter a four-digit number to be encrypted."
<< "\nTo end, type -1: ";
cin >> number;

while ( number != -1 ) {

if ( number < 1000 || number > 9999 ) {
cout << "The program only works with four-digit numbers."
<< "\nEnter a four-digit number to be encrypted."
<< "\nTo end, type -1: ";
cin >> number;
}

else {

// Separates the numbers into their individual digits
originalFirstDigit = ( number - ( number % 1000 )) / 1000;

originalSecondDigit = ( number % 1000 - ( number % 100 )) / 100;

originalThirdDigit = ( number % 100 - ( number % 10 )) / 10;

originalFourthDigit = ( number % 10 - ( number % 1 )) / 1;

// Replaces each digit by the sum of that digit + 7 % 10

originalFirstDigit = ( ( originalFirstDigit * 2 ) + 7 ) % 10;
originalSecondDigit = ( ( originalSecondDigit * 2 ) + 7 ) % 10;
originalThirdDigit = ( ( originalThirdDigit * 2 ) + 7 ) % 10;
originalFourthDigit = ( ( originalFourthDigit * 2 ) + 7 ) % 10;

// Swaps the first digit with the third, the second digit with the fourth

firstDigit = originalThirdDigit;
secondDigit = originalFourthDigit;
thirdDigit = originalFirstDigit;
fourthDigit = originalSecondDigit;

cout << "The encrypted number is " << firstDigit << secondDigit << thirdDigit << fourthDigit << endl;

cout << "\nEnter a four-digit number to be encrypted."
<< "\nTo end, type -1: ";
cin >> number;
}

}

return 0;
}


I know that there is a problem because if the user inputs say 0999, the program will complain because it is less than 1000 but it is still a four-digit number. I have to fix that but I don't know how. Please, tell me if, besides that problem, the code looks good or not and help
me check for input lenght. Thanks.

[stuka]

Personally, I think I would use a string input, and use the handy string indexing abilities to change values, etc. Saves on much of the math, and you can swap digits much more easily.

[nothing]

The problem is that I don't have that knowledge yet. Do you know another way to check the input lenght besides what you suggested?

[stuka]

Well, that depends - what tools do you have available? Using cin >> number puts you at the mercy of the input stream's automatic translation, so that a leading zero is going to be removed every time. If you were to use something like this: #include <iostream>
#include <string>

int main(){
std::string original, encoded;

std::cout << "Enter the number: "
std::cin >> original;
if (original.length() != 4){
std::cout << "Number must be 4 digits!"
exit(0);
}
for (int i = 0; i < 4; i++){
if (!std::isdigit(original[i])){
std::cout << "Not a valid number!"
exit(0);
}
}
//Here you can use math on individual digits, knowing that '0' - 48 = 0
// to modify the digits, then use indexes to swap them
} you could test all the necessary things (length, is it a number, etc.)

[nothing]

What did you mean by "knowing that '0' - 48 = 0" ?

[stuka]

Ah...it's an old trick. In C/C++, a char can be treated as a 1-byte integer, so that if I have char c = '0';
int x = c - 48;
cout << x;
the result printed out is 0 (the integer, not the char). Or, to put it in a loop:int x;
for (char c = '0'; c <= '9'; c++){
x = c - 48;
cout << x << endl;
} This will print out the digits from 0 to 9, after being converted from the chars '0' through '9'.

[nothing]

But why the number 48?

[stuka]

Oh...that's because in the ASCII code system, the code assigned to the character '0' is 48, and the other digits follow in order. A google search for 'ASCII chart' should provide plenty of answers.

[nothing]

This program is all wrong. Let's say that the program only encrypts the numbers but don't swap them with each other. If the user inputs 1234, the program would print 9135. Notice how 1 became 9. Now, if the user inputs 6234, the program would also print 9135. I am mad :angry:

[DNAunion2000]

nothing: This program is all wrong. Let's say that the program only encrypts the numbers but don't swap them with each other. If the user inputs 1234, the program would print 9135. Notice how 1 became 9. Now, if the user inputs 6234, the program would also print 9135. I am mad

DNAunion: Perhaps instead of calling it encryption (which implies that the encrypted sequence can be decrypted to unambiguously recreate the original) they should have simply called it a one-way hashing algorithm.

[nothing]

Originally posted by Stuka
Well, that depends - what tools do you have available? Using cin >> number puts you at the mercy of the input stream's automatic translation, so that a leading zero is going to be removed every time. If you were to use something like this: #include <iostream>
#include <string>

int main(){
std::string original, encoded;

std::cout << "Enter the number: "
std::cin >> original;
if (original.length() != 4){
std::cout << "Number must be 4 digits!"
exit(0);
}
for (int i = 0; i < 4; i++){
if (!std::isdigit(original[i])){
std::cout << "Not a valid number!"
exit(0);
}
}
//Here you can use math on individual digits, knowing that '0' - 48 = 0
// to modify the digits, then use indexes to swap them
} you could test all the necessary things (length, is it a number, etc.)

Stuka, can you explain why you used exit(0) inside the if and the for statements, why you used for (int i = 0; i < 4; i++) and why you used (original[i]) (actually, I didn't understand the [i] thing. Thank you and sorry for all the questions.

[DNAunion2000]

DNAunion: If I may be allowed to reply.

nothing: Stuka, can you explain why you used exit(0) inside the if and the for statements,...

DNAunion: exit() terminates program execution immediately. The numeric argument indicates to the caller (here, probably the OS) under what conditions the program terminated. 0 typically indicates no problem (here, the operating system does not need to do anything to handle the fact that the user entered a invalid value, so 0 is sent). You can think of exit(0); as being about the same as the return 0; statement found at the end of main() functions, except that exit(0); ends the program immediately.

nothing: ... why you used for (int i = 0; i < 4; i++) and why you used (original[ i ]) (actually, I didn't understand the [ i ] thing.

DNAunion: i is the name Stuka chose to assign to his integer variable - he could have called it index or subscript or loopControlVariable or myInteger or what have you. But when a variable is used only in one small part of the code and its purpose is clear from the surrounding context, short names such as x or y are often times used. (One little side note: using i as an index for C-style languages can fool a web site such as this, which interprets [ i ] to be markup for "begin italics here").

Its purpose is to "walk" through the individual character elements that make up the string original that was input by the user. Here's the code.


for (int i = 0; i < 4; i++){
if (!std::isdigit(original[ i ])){
std::cout << "Not a valid number!"
exit(0);
}
}


DNAunion: Assume the user typed the number 5678 and then pressed enter. The variable original, being a string, would contain the value "5678". The very first element in the character array that represents original is "5", the second is "6", the third is "7", and the fourth is "8". Since arrays are zero-based in C-style languages, the very first element of an array is accessed by using an index of 0, not 1. So the very first, single-character element of the array is referenced as the array name, followed by the [ symbol, the integer index, and the ending ]: that is, original[0]; That's why Stuka started with an index of 0 and ended BEFORE using 4 (i.e., before trying to access the "nonexistent" element original[4];).

What Stuka wants to do is to see if each of the 4 characters in the array represents a digit ("0", "1", "2", "3", ...). If any one of them doesn't, Stuka will inform the user of the problem and then exit the program. He could have performed this check as follows:


if (!std::isdigit(original[0])){
std::cout << "Not a valid number!"
exit(0);
}
if (!std::isdigit(original[1])){
std::cout << "Not a valid number!"
exit(0);
}
if (!std::isdigit(original[2])){
std::cout << "Not a valid number!"
exit(0);
}
if (!std::isdigit(original[3])){
std::cout << "Not a valid number!"
exit(0);
}


But that would have been "inelegant", repetitive, overly verbose, and inflexible. Note that everything is exactly the same for each of the repeated if blocks except for the value of the index, which is incremented by one each time. This is an ideal candidate for writing as a loop, which is what Stuka did from the start.

[nothing]

Thank you very much DNAunion2000!

[nothing]

When I try to compile the following code, I get two erros:

--------------------Configuration: 2.37 - Win32 Debug--------------------
Compiling...
2.37.cpp
C:\Documents and Settings\Cadu\My Documents\C++\2.37.cpp(27) : error C2676: binary '!=' : 'class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> >' does not define this operator or a conversion to a type acceptable to
the predefined operator
C:\Documents and Settings\Cadu\My Documents\C++\2.37.cpp(27) : fatal error C1903: unable to recover from previous error(s); stopping compilation
Error executing cl.exe.

2.37.obj - 2 error(s), 0 warning(s)

I'm using Microsoft's Visual C++


#include <iostream>
#include <string>

using namespace std;

int main()
{
string number;
int firstDigit;
int secondDigit;
int thirdDigit;
int fourthDigit;
int originalFirstDigit;
int originalSecondDigit;
int originalThirdDigit;
int originalFourthDigit;

cout << "Enter a four-digit number to be encrypted."
<< "\nTo end, type -1: ";
cin >> number;

if ( number.length() != 4 )
cout << "Number must be four-digit." << endl;

else {

while ( number != -1 ) {

// Separates the numbers into their individual digits
originalFirstDigit = ( number - ( number % 1000 )) / 1000;

originalSecondDigit = ( number % 1000 - ( number % 100 )) / 100;

originalThirdDigit = ( number % 100 - ( number % 10 )) / 10;

originalFourthDigit = ( number % 10 - ( number % 1 )) / 1;

// Replaces each digit by the sum of that digit + 7 % 10

originalFirstDigit = ( originalFirstDigit + 7 ) % 10;
originalSecondDigit = ( originalSecondDigit + 7 ) % 10;
originalThirdDigit = ( originalThirdDigit + 7 ) % 10;
originalFourthDigit = ( originalFourthDigit + 7 ) % 10;

// Swaps the first digit with the third, the second digit with the fourth

firstDigit = originalThirdDigit;
secondDigit = originalFourthDigit;
thirdDigit = originalFirstDigit;
fourthDigit = originalSecondDigit;

cout << "The encrypted number is " << firstDigit << secondDigit << thirdDigit << fourthDigit << endl;

cout << "\nEnter a four-digit number to be encrypted."
<< "\nTo end, type -1: ";
cin >> number;
}

}

return 0;
}


I can't figure out why I'm getting these errors. Help please.

[stuka]

Because there is no comparison between a std::string and an integer.

[nothing]

How can I fix it? :(

[stuka]

Well, remember...you're not dealing with a number now, you're dealing with a string. Each 'digit' can be accessed by doing 'number[0]' for example.

[nothing]

I was following the example you gave me.


#include <iostream>
#include <string>

int main(){
std::string original, encoded;

std::cout << "Enter the number: "
std::cin >> original;
if (original.length() != 4){
std::cout << "Number must be 4 digits!"
exit(0);
}
for (int i = 0; i < 4; i++){
if (!std::isdigit(original[i])){
std::cout << "Not a valid number!"
exit(0);
}
}


Now I am lost again
:redfaced:

[nothing]

Sorry Stuka. Nevermind my last post. :redfaced:

[sans-hubris]

Suggestion, go look up atoi().

[nothing]

The atoi() function converts str into an integer, and returns that integer. str should start with some sort of number, and atoi() will stop reading from str as soon as a non-numerical character has been read. For example,

i = atoi( "512.035" );

would result in i being set to 512.

Nice, very nice :D

[nothing]

Now I got my program to check input length but I also got a new problem :sick: When the program is working correctly, if I input number 1234, the program will encrypt it and the result will be number 0189. After the changes, if I input number 1234 or any other four-digit number, the program will print 7777. Why is this happening?


#include <iostream>
#include <string>
#include <cstdlib>

using namespace std;

int main()
{
string num;
int number;
int firstDigit;
int secondDigit;
int thirdDigit;
int fourthDigit;
int originalFirstDigit;
int originalSecondDigit;
int originalThirdDigit;
int originalFourthDigit;

cout << "Enter a four-digit number to be encrypted: ";
cin >> num;

while ( 1 ) {

if ( num.length() != 4 ) {
cout << "ERROR! Number must be four-digit: ";
cin >> num;
}

else {

// Transforms string num into int number
number = atoi( "num" );

// Separates the number into its individual digits
originalFirstDigit = ( number - ( number % 1000 )) / 1000;

originalSecondDigit = ( number % 1000 - ( number % 100 )) / 100;

originalThirdDigit = ( number % 100 - ( number % 10 )) / 10;

originalFourthDigit = ( number % 10 - ( number % 1 )) / 1;

// Replaces each digit by ( digit + 7 ) % 10

originalFirstDigit = ( originalFirstDigit + 7 ) % 10;
originalSecondDigit = ( originalSecondDigit + 7 ) % 10;
originalThirdDigit = ( originalThirdDigit + 7 ) % 10;
originalFourthDigit = ( originalFourthDigit + 7 ) % 10;

// Swaps the first digit with the third, and the second digit with the fourth

firstDigit = originalThirdDigit;
secondDigit = originalFourthDigit;
thirdDigit = originalFirstDigit;
fourthDigit = originalSecondDigit;

cout << "The encrypted number is " << firstDigit << secondDigit << thirdDigit << fourthDigit << endl;

cout << "\nEnter a four-digit number to be encrypted: ";
cin >> num;
}
}

return 0;
}

[DNAunion2000]

DNAunion: Nothing, I have three comments that don't deal with the actual algorithm.

1) How does the user end the program? You've coded an infinite loop using while (1) and nothing inside the loop causes control to exit that loop. You could change your prompts to


cout << "\nEnter a 4-digit number to be encrypted (0 to exit)";

and then use


while ( atoi(num) )


to control the loop.


2) You can eliminate the extra level of nesting by using a continue statement.


if ( num.length() != 4 )
{
cout << "\nERROR! Number must be four-digits.";
cout << "\nEnter a 4-digit number to encrypt (0 to exit)";
cin >> num;
continue;
}


Because continue causes control to immediately pass back to the top of the loop, you don't need the else portion and the bulk of the code will be indented one level less. (Some believe continue goes against structured programming, but that seems a bit nitpicky to me here).

3) Your final cout statement is very long and won't fit in my word processor without wrapping, even when I set margins to 0. You could split the one statement up onto two or more lines, or into two or more statements.


cout << "The encrypted number is " << firstDigit << secondDigit;
cout << thirdDigit << fourthDigit << endl;

[DNAunion2000]

DNAunion: Nothing, here's a problem.


// Transforms string num into int number
number = atoi( "num" );


DNAunion: The atoi() function accepts a string as a parameter and the string you pass as an argument consists of the three letters 'n' and 'u' and 'm'. What you intend to pass is not that sequence of letters but the value stored in the num variable (1234 or what have you). The variable num is already a string, so its name is all that is needed in the invocation (no double quotes).


// Transforms string num into int number
number = atoi( num );

[DNAunion2000]

DNAunion: One more. I believe code clarity can be improved by changing the math used to pick apart the individual original digits.


// Separates the number into its individual digits
originalFirstDigit = ( number - ( number % 1000 )) / 1000;

originalSecondDigit = ( number % 1000 - ( number % 100 )) / 100;

originalThirdDigit = ( number % 100 - ( number % 10 )) / 10;

originalFourthDigit = ( number % 10 - ( number % 1 )) / 1;



DNAunion: Instead:


// Separates the number into its individual digits
originalFirstDigit = number / 1000;
number %= 1000;

originalSecondDigit = number / 100;
number %= 100;

originalThirdDigit = number / 10;

originalFourthDigit = number % 10;


DNAunion: It’s a matter of taste, really, but I found the second method easier to follow.

[DNAunion2000]

DNAunion: Another thought is to use an array instead of 4 individual integer variables.


int originalFirstDigit;
int originalSecondDigit;
int originalThirdDigit;
int originalFourthDigit;


becomes


int originalDigits[4];



*********************************

DNAunion: Then the code that picks apart the individual digits could use a for loop, greatly reducing the complexity of the algorithm.


// Separates the number into its individual digits
for (int lcv = 0; lcv < 4; ++lcv)
{
originalDigits[lcv] = number / pow(10, 3 – lcv);
number %= pow(10, 3 – lcv);
}


DNAunion: In this example, using the array doesn’t really gain all that much. But, it would be the much better solution if you wanted to be able to handle larger numbers or have the ability to change the size of the numbers at will. Suppose you changed the program to require 8 digit numbers. Here’s how the code would compare then.


int originalFirstDigit;
int originalSecondDigit;
int originalThirdDigit;
int originalFourthDigit;
int originalFifthDigit;
int originalSixthDigit;
int originalSeventhDigit;
int originalEighthDigit;


becomes


int originalDigits[8];



DNAunion: Note also that the code that picks apart the individual digits would need only a trivially simple change to handle the extra digits (whereas the original code would require several not-so-simple changes).


// Separates the number into its individual digits
for (int lcv = 0; lcv < 8; ++lcv)
{
originalDigits[lcv] = number / pow(10, 3 – lcv);
number %= pow(10, 3 – lcv);
}


DNAunion: In fact, you have a “magic number” in your program...the number of digits (4 in the original) is referred to many places and controls several things. It should be made into a constant variable, which would work perfectly for the array implementation. For the program to handle 6-digits numbers…


const int DIGITS = 6;

…

int originalDigits[DIGITS];

…

cout << "\nEnter a " << DIGITS << "-digit number (0 to exit):";
...

if ( num.length() != DIGITS)

...

// Separates the number into its individual digits
for (int lcv = 0; lcv < DIGITS; ++lcv)
{
originalDigits[lcv] = number / pow(10, 3 – lcv);
number %= pow(10, 3 – lcv);
}

...

// Replaces each digit by ( digit + 7 ) % 10
for (int lcv = 0; lcv < DIGITS; ++lcv)
{
originalDigits[lcv] = (originalDigits[lcv] + 7 ) % 10;
}

...

// swaps digits from one end with its partner at the other end
for(int lcv = 0, int offset = DIGITS / 2; lcv + offset <= DIGITS; ++lcv)
{
int temp = originalDigits[lcv];
originalDigits[lcv] = originalDigits[lcv + offset];
originalDigits[lcv + offset] = temp;
}

...

for(int lcv = 0; lcv < DIGITS; ++lcv)
cout << originalDigits[lcv];

...



DNAunion: And by making a single change -- modifying DIGITS -- the program will work with 1-, 2-, 3-, 4-, 5-, 6-, 7-, 8-, or 9-digit numbers.

[nothing]

:o I really really really appreciate the help. Eventhough I'm a beginner and there are things that are didn't get, it will help me in the future. I will work on the program with your ideas and I will post the results soon. Thank you so much :D