A very simple, yet pratical class - C++

[Belzebuth]

Hello everyone,

so today I was in need of a vigenere encode/decode function for a simple application. The issues is that I didn't find any with a fully customizable alphabet. So I wrote one

A Vigenere coding is a very basic algorithm with consist on secret key and an list of characters to encode a piece of text. The idea is to take the first letter of your text and the first letter of your key, find their respective position in the char list, and sum the value. The result modulo the number of entry in your char list, is now the new encoded char. And you continue 'till the end of your text.

 

Here is the GitHub link : Vigenere-CPP

 

Let me know what you think about the class, and the Vigenere algorithm

[Nettly_]

It's nice

[Unimportant]

On 7/31/2019 at 4:16 PM, Belzebuth said:

Hello everyone,

so today I was in need of a vigenere encode/decode function for a simple application. The issues is that I didn't find any with a fully customizable alphabet. So I wrote one

A Vigenere coding is a very basic algorithm with consist on secret key and an list of characters to encode a piece of text. The idea is to take the first letter of your text and the first letter of your key, find their respective position in the char list, and sum the value. The result modulo the number of entry in your char list, is now the new encoded char. And you continue 'till the end of your text.

 

Here is the GitHub link : Vigenere-CPP

 

Let me know what you think about the class, and the Vigenere algorithm

Why do you declare all your variables in the beginning of a function asif this was old C ?

Declare them when you need them, in the scope you need them.

 

Should throw up a bunch of warnings for signed/unsigned comparisons.

 

It's also considered good practice to use braces even if there's only a single expression in them:

if (...)
{
  ++j;
}

 

[Sauron]

6 minutes ago, Unimportant said:

It's also considered good practice to use braces even if there's only a single expression in them

Depends on who you ask, the Linux kernel's coding style for instance says not to use the brackets.

[wanderingfool2]

7 hours ago, Unimportant said:

Why do you declare all your variables in the beginning of a function asif this was old C ?

Declare them when you need them, in the scope you need them.

 

Should throw up a bunch of warnings for signed/unsigned comparisons.

 

It's also considered good practice to use braces even if there's only a single expression in them:

 

I am a bit at odds at both comments.  It isn't always "good practice" to have braces on single line statements.  It all depends on what conventions you follow, and where you choose to exclude the braces.  In this case, I think leaving out the braces brings more clarity to the code (at least the one I skimmed through).

 

As for the declaring the variables at the beginning...I think he did it the right way personally...mainly that he wouldn't have had much choice of where to put it. 

 

[Belzebuth]

10 hours ago, Unimportant said:

Why do you declare all your variables in the beginning of a function asif this was old C ?

Declare them when you need them, in the scope you need them.

 

Should throw up a bunch of warnings for signed/unsigned comparisons.

 

It's also considered good practice to use braces even if there's only a single expression in them:

if (...)
{
  ++j;
}

 

Well to me it wasn't a big deal, the function are pretty clear on their own.

 

For unsigned/signed comparisons, indeed it throws warnings that could have been avoided with a long long unsigned int declaration. 

 

For the braces, I've heard both pros and cons... I find it more readable that way.

[Unimportant]

4 hours ago, wanderingfool2 said:

I am a bit at odds at both comments.  It isn't always "good practice" to have braces on single line statements.  It all depends on what conventions you follow, and where you choose to exclude the braces.  In this case, I think leaving out the braces brings more clarity to the code (at least the one I skimmed through).

It'll come back to bite you, or the one having to maintain your code, in the a**. How easy is it to accidentally misread or make a modification in the wrong place without the braces. Everyone who claims "they won't be making such mistakes" are probably the first to do so. Get all the help you can get!

Putting everything on a single line also does not allow putting a breakpoint on the conditional expression.

 

4 hours ago, wanderingfool2 said:

As for the declaring the variables at the beginning...I think he did it the right way personally...mainly that he wouldn't have had much choice of where to put it. 

No, you declare variables when you need them, in the scope you need them. That way you don't pollute a wider scope then necessary. The variable only lives as long as it needs to and no longer (which becomes critical for more complex types - RAII - so it should become a habit) and you can immediately initialize it with the proper value rather then a dummy value, which in turn allows more variables then you'd think to be const.

 

https://stackoverflow.com/a/3773458/1320881

Quote

Declaring variables "at the top of the function" is always a disastrously bad practice...

 

[Belzebuth]

3 hours ago, Unimportant said:

It'll come back to bite you, or the one having to maintain your code, in the a**. How easy is it to accidentally misread or make a modification in the wrong place without the braces. Everyone who claims "they won't be making such mistakes" are probably the first to do so. Get all the help you can get!

Putting everything on a single line also does not allow putting a breakpoint on the conditional expression.

While I do agree it can introduce user error, if the statement is single line like here, it's pretty straight forward. However, anything that is not single line should use braces. 

//This is fine for me
if(condition) statement;


//This is prone to user error
if(condition) 
    statement;

 

3 hours ago, Unimportant said:

No, you declare variables when you need them, in the scope you need them. That way you don't pollute a wider scope then necessary. The variable only lives as long as it needs to and no longer (which becomes critical for more complex types - RAII - so it should become a habit) and you can immediately initialize it with the proper value rather then a dummy value, which in turn allows more variables then you'd think to be const.

This is indeed a bad habit for me, keeping declaration in scope where needed is way better for stack optimisation. I will try to force me to do it the right way.

[mariushm]

What's with the strtoupper calls in the function?

What's the point of all those alphabet.find() calls ... just calculate those and put them in an array at the start of encoding or decoding and then just reference the array, instead of wasting time with function calls

How are you dealing with UTF-8 text where you may have bytes outside the latin encoding etc ... here's ﾑ刀 乇ﾒﾑﾶｱﾚ乇

edit: you could probably simplify the code a bit by always having the cypher/password padded to 128 or 256 bytes /characters , ex repeat the phrase until you reach this many characters.

So then you could just use shifts or bitwise and instead of divisions/mod/remainder etc

[Unimportant]

8 hours ago, Belzebuth said:

 

//This is fine for me
if(condition) statement;

 

Now put a debugging breakpoint on statement.

(Only on statement, not the entire if, so the breakpoint only trips when the condition is met).

[wanderingfool2]

18 hours ago, Unimportant said:

It'll come back to bite you, or the one having to maintain your code, in the a**. How easy is it to accidentally misread or make a modification in the wrong place without the braces. Everyone who claims "they won't be making such mistakes" are probably the first to do so. Get all the help you can get!

Putting everything on a single line also does not allow putting a breakpoint on the conditional expression.

 

No, you declare variables when you need them, in the scope you need them. That way you don't pollute a wider scope then necessary. The variable only lives as long as it needs to and no longer (which becomes critical for more complex types - RAII - so it should become a habit) and you can immediately initialize it with the proper value rather then a dummy value, which in turn allows more variables then you'd think to be const.

 

https://stackoverflow.com/a/3773458/1320881

 

Except having a { } on every single line if also add in an extra 2 lines (3 lines if you have the opening bracket on it's own line).  Which to me harms the readability a lot more than the potential of a miss read.

 

For the second point, my point still stands in the concept (again I skimmed the code)....Although I did read the code without skimming and did see the out of scope variables when I skimmed I had thought they had been reused.  (My argument wasn't for declaring at the beginning, but rather that I thought there wasn't really much other choice in scoping due to skimming it)

[Belzebuth]

On 8/2/2019 at 4:33 PM, mariushm said:

What's with the strtoupper calls in the function?

What's the point of all those alphabet.find() calls ... just calculate those and put them in an array at the start of encoding or decoding and then just reference the array, instead of wasting time with function calls

How are you dealing with UTF-8 text where you may have bytes outside the latin encoding etc ... here's ﾑ刀 乇ﾒﾑﾶｱﾚ乇

edit: you could probably simplify the code a bit by always having the cypher/password padded to 128 or 256 bytes /characters , ex repeat the phrase until you reach this many characters.

So then you could just use shifts or bitwise and instead of divisions/mod/remainder etc

The string to encode is set to uppercase to avoid the need of lowercase chars in the alphabet. 

The .find() function is up to linear in complexity, I don't feel it would be much faster/efficient to calculate those at the beginning, I will do the math.

I'm not dealing with non-latin chars.

This vigenere cypher is strictly the algorithm, the point is for it to be decodable by any other vigenere class from any language.

[wasab]

On 8/1/2019 at 5:14 PM, Unimportant said:

 

It's also considered good practice to use braces even if there's only a single expression in them:

if (...)
{
  ++j;
}

 

That's not the coding style in the book K&R. 

[wasab]

 

 

 

1

On 8/2/2019 at 5:03 AM, Unimportant said:

 

 

No, you declare variables when you need them, in the scope you need them. That way you don't pollute a wider scope then necessary.

https://stackoverflow.com/a/3773458/1320881

 

DIsagree. Declaring variables inside the loop can lead to inefficiencies in some cases e.g. function_two is much faster than function_one because sum(1000) is only called once as oppose to each loop. 

int sum(int arg){
	int retval = 0;
	for(int i = 0; i < 1000000000; i++){
		retval += arg;
	}
    return retval;
}


void function_one(){
    for(int i = 0; i < sum(1000); i++){
		// do some stuffs 
	}
}

void function_two(){
	int max = sum(1000); // this is so much faster
	for(int i = 0; i < max; i++){
		// do some stuffs 
	}
}

 

Edited August 21, 2019 by wasab
Spoiler tag and code tag do not work well together.

[Franck]

27 minutes ago, wasab said:

DIsagree. Declaring variables inside the loop can lead to inefficiencies in some cases e.g. function_two is much faster than function_one because sum(1000) is only called once as oppose to each loop. 

int sum(int arg){
	int retval = 0;
	for(int i = 0; i < 1000000000; i++){
		retval += arg;
	}
    return retval;
}


void function_one(){
    for(int i = 0; i < sum(1000); i++){
		// do some stuffs 
	}
}

void function_two(){
	int max = sum(1000); // this is so much faster
	for(int i = 0; i < max; i++){
		// do some stuffs 
	}
}

 

That's some crazy example lol but it does in fact shows as black and white how it cam impede to declare inside a loop.

[Unimportant]

1 hour ago, wasab said:

That's not the coding style in the book K&R. 

So? I don't see K&R anywhere assisting me when I have to deal with the countless problems caused by omitting the brackets.

 

52 minutes ago, wasab said:

DIsagree. Declaring variables inside the loop can lead to inefficiencies in some cases e.g. function_two is much faster than function_one because sum(1000) is only called once as oppose to each loop.

I said nothing about using/not using temporary variables. I said to not declare all variables in the top of a function asif this was old C code.

 

E.g. This:

//Some code...

const auto length = ...
const auto width = ...
const auto surface = length * width;

rather then this:

int length = 0;
int width = 0;
int surface = 0;

//Some code ...

length = ...
width = ...
surface = length * width;

You declare variables at the point you need them, not in the top of the function asif old C.

[wasab]

34 minutes ago, Unimportant said:

So? I don't see K&R anywhere assisting me when I have to deal with the countless problems caused by omitting the brackets.

Pythons have no brackets and python code is very readable. 

[Unimportant]

2 minutes ago, wasab said:

Pythons have no brackets and python code is very readable. 

Let's agree to disagree there.