Clarity of encryption class

I have recently gotten back into development and I am wondering if the script I have just created is clearly documented and easily understandable throughout each step. Is it easy to understand?

```
class Encryption {

public function Encrypt ($String){
/*
* Create an Encrypted String.
* @Var string
*
*/
$Size = mcrypt_get_iv_size(MCRYPT_CAST_256,MCRYPT_MODE_CBC);
$iv = mcrypt_create_iv($Size,MCRYPT_RAND);
$Hash = strlen($String);
return
array(
"EncryptedString" => openssl_encrypt($String, "AES-256-CBC",$Hash,0,$iv),
"IV" => $iv,
"Hash" => $Hash
);
}

public function Merge($Array){
/*
* Merge The Array Returned From the String Encryption into a string
*
* Information:
* 1) Convert Each Element from the array into their personal variables
* 2) Create an Array from the first element. Will be used at the later stages
* 3) Get the length of the current IV passed
* 4) Create an empty string to be manipulated using the attaching loop
*
*/
# $Array = $this->Encrypt($String);
$Encrypted_String = $Array['EncryptedString'];
$IV = $Array['IV'];
$Hash = $Array['Hash'];
$Hash_Count = count(str_split($Hash));
$EncStr_Arr = str_split($Encrypted_String);
$Count = strlen($IV);
$Increment = 0;
$String = "";
// NEW: Appending the Hash Count, to be used in later functions. So The correct Hash can be obtained.
$String .= $Hash_Count.$Count.$Hash; // Append the Count and Hash (as EncryptedString and IV are different lengths. Used to compensate
While ($Increment $Value){
/*
* Through each iteration of the array (using the keys) decides if the key is odd or even.
* If

In my opinion, no your class is not clearly documented and easily understandable.

Here are the main reasons I find this code... dirty:

• The naming is misleading.

• The naming is unusual.

• There are too many comments.

• Inconsistencies.

Is that it? Well, let me explain:

Misleading Names

-
The first thing I did was look at the class name, because that alone should tell you exactly what the class will do. No ifs-ands-or-buts. Sweet, the class is about encryption.

Now, by definition, "Encryption" is:


In cryptography, encryption is the process of encoding messages or information in such a way that only authorized parties can read it.

This was simply from Googling "define encryption". How is this relevant? Well, if you check out the last bit of your code, it's decrypting (decoding if you want a polar of the quote). This seems like a stupid thing to point out, but in all honesty, I was expecting two classes, an encryption and a decryption class! This is what I mean by misleading names.

-
The next greatest glaring thing is the Merge and UnMerge functions (both of which I need to criticize later). When I saw Merge, I got lost. What in the world could an encrypting class need to merge? And why is it so prominent in the code?

So I take a look at the contents, and it's A) defining variables to equal other variables, and B) obfuscating the encryption in a strange manner. In a completely education attitude, I find this function a waste of space and time. I think it takes up unnecessary space by appending a mish-mash of characters to an already encoded string, and I feel like the variable tide grew and washed up a bunch of unnecessary variables! What's the point in the whole while loop? If you have a reasonable explanation, I'd be more than welcome to be enlightened by the way.

As of now though, I see absolutely no reason for this to be it's own method.

Would I say the same about the UnMerge method? Most definitely. Without the Merge method, this un-merger wouldn't exist.

Naming is one of the most important things a programmer can get right. Here's a couple pointers for naming classes:

• Class names are always nouns, not verbs. (Kudos to you)

• Class names should be descriptive in nature without implying implementation. Since implementation can change, to imply implementation in the name forces the class name and all references to it to change or else the code can become misleading. Remember: "describing but not specifying".

Unusual Naming

Now, here's the fun part. Hehe I've got a lot to say! But before I begin, I want you to know that it's important to choose a code formatting standard that the readers will understand. There are many, many, many standards to choose from for PHP, and the choice is up to you. The three I listed are quite similar in style.

Having a style that people recognize makes reading almost 200% easier. Since I wasn't used to your style, it took me longer to read your code than it normally would have. Here's a couple notes (taken from PHP.net):

-

Method names follow the 'studlyCaps' (also referred to as 'bumpy case'
or 'camel caps') naming convention, with care taken to minimize the
letter count. The initial letter of the name is lowercase, and each
letter that starts a new 'word' is capitalized.

-

Variable names must be meaningful. ... Variable names should be in
lowercase. Use underscores to separate between words.

I focus on white space in the Inconsistencies section.

Too Many Comments

This one could be debated in certain situations, but in your case, I find your comments detrimental to the readability and conciseness of your code.

The comments are non-standard (PHPdoc), very spread out, in inappropriate places, and I think far too long.

PHPdocs are almost identical to JavaDocs, if you're familiar with that. If not, I suggest this standard to get you started.

I think you too much white space in between comments and inside comments. It forces the eyes to jump around.

If the variables names, method names, and class names are all excellent, comments are only needed in essential places. I think that because the names of these objects are vague, you're having to do a lot more commenting than necessary!

I think you're giving too much information. With the PHPdocs, it's all in that one place, compact, uniform, and concise.

There are however, times when inline comments are actually necessary, and it's perfectly acceptable to use them then. Comments aren't code, so of course it really doesn't matter what you do. I'm just thinking about us readers ;)

Inconsistencies

These are the little things we often forget about, which is why Code Review is here!

• Compare Encrypt ($String) and Merge($Array). Do you want one space or none? Choose.

• Compare $iv, $Size, and $IV. You need to decide what capitalization pattern should be followed.

• Is it While or while? Don't fix it if it ain't broke. Nowhere in the PHP docum

StackExchange Code Review Q#57489, answer score: 5