• Forum
  • Lounge
  • Wrote this little program. Feed back ple

 
Wrote this little program. Feed back please

I wrote a text / txt file encryption program. I was wondering if I could get some feed back on my code. As in the good and bad programming techniques.

https://github.com/tibrado/txt_encryption


I'm also trying to understand github.
Last edited on
I personally like to put braces on all if statements, even if the conditional code is one line long. I think this is better because it gets people into the habit of putting braces in larger conditional codes, and also because it makes it easier if you want to change the code to have more in the braces.
Some thoughts in no particular order:
- Indent your sample by 4x more spaces or a tab on each line for it to be code-formatted on your README.
- one-letter function names like "d" and "c", and variables names like "c" and "m" makes code less readable and harder to understand, in most cases.
- ALLCAPS identifiers are usually reserved for macros. It is not usual practice to have all-caps class names like CRYPTION and CFILE.
- Since you're using C++, you should prefer to use C++ file streams std::ifstream/std::ofstream. You use this in your CFILE class, but then you use C-style file handling in your fhandler.cpp. This is inconsistent.
- Likewise, you use a weird combination of C-style const char *s, and std::strings. I would just use std::string.
1
2
3
4
5
6
7
8
9
10
11
	int i = 0;
	int j = 0;
	for (auto m : M) {
		if (j == K.length()) { j = 0; }

		if (v) { c[i] += e(m, K[j]); }
		else { c[i] += d(m, K[j]); }

		j++;
		i++;
	}

The above code using an odd combination foreach with i and j. It can also be simplified using the % (modulo) operator, by just doing i % K.length() instead. I would just make it a normal for loop since you already have the i variable, but that's up to you.
https://stackoverflow.com/questions/12556946/how-does-the-modulus-operator-work

1
2
3
4
5
6
CFILE::~CFILE() {
	// if file was not decrypted add count 
	// if file was not encrypted add count 
	// if file count is greater than 3 scrable
	// and delete file 
}

What is the point of this? If this is future plans, normally people put "FIXME" or "TODO" in the comment.
Last edited on
Topic archived. No new replies allowed.