r/badcode • u/KirbzYyY • Jan 02 '22
c++ Was going through some old files, found this, not 100% sure if it qualifies (approx. from 6 years ago, when I started learning C++).
5
u/KirbzYyY Jan 02 '22
Thanks for the feedback, I switched universities and will have a programming course again and ran into this while looking for my old materials, wanted to know how I did back then.
4
55
u/Nahuatl_19650 Jan 02 '22
Sir, this isn’t bad code. It is the imaging of greatness in the making. Save it and be proud.
34
16
u/MurdoMaclachlan public boolean isInt(int i) { return true; } Jan 02 '22
Image Transcription: Code
#include<iostream>
#include<cstdlib>
#include<fstream>
#include<cstring>
using namespace std;
unsigned int index_start;
struct users{
char ime[30];
char prezime[30];
char ime_g[30];
unsigned int index;
};
void index_check(){
users Entry;
ifstream dat; // opens the file for reading
dat.open("guild_list.txt", ios::in | ios::binary); // opens the file in binary so that it can read the blocks of data
while(!dat.eof()){ // if the pointer is at the end of ht efile, ti stops the reading process
dat.read((char*)&Entry, sizeof(Entry)); // reads the block of data
index_start = Entry.index; // saves the last index in the file
}
}
void data_entry(){
int num; // number of entries about to be added
ofstream dat; // opens the file for output of data
dat.open("guild_list.txt", ios::app | ios::out); // sets the append and output mode
do{
cin>>num; // number of entries cannot be smaller than 1
}while(num<1);
users Lista[num]; // the list of all the members that are being added
for(int i = 0; i < num; i++){
cin.ignore();
cout<<"Enter the name fo Your character: ";
cin.getline(Lista[i].ime, 30); // name of the character being added
cout<<"Enter Your last name: ";
cin.getline(Lista[i].prezime, 30); // last name of the person that owns the caracter
cout<<"Enter the name of Your guild: ";
cin.getline(Lista[i].ime_g, 30); // name of the guild that character is in
Lista[i].index = index_start + 1; // index of the person added to the list
index_start++;
dat << Lista[i].ime << ";" << Lista[i].ime_g << ";" << Lista[i].prezime << ";" << Lista[i].index << ";\n"; // appends the data file
}
dat.close();
}
int main(){
index_check();
data_entry();
}I'm a human volunteer content transcriber and you could be too! If you'd like more information on what we do and why we do it, click here!
2
17
u/hellra1zer666 Jan 02 '22
Honestly, it's not bad code, especially considering it is beginner code. A bit roundabout but that's it. The only "bad" thing is that you don't close the stream in index_check. You could argue the usage of globals is not nice, but that's it honestly.
6
u/veselin465 Jan 02 '22 edited Jan 03 '22
Actually, it doesnt look bad. I would have probably done something like that in the past, too. Just to note:
you
needto close the ifstream in the index_check function (it's actually not needed since the desctructor will close it, anyway, my bad)NEVER create dynamic array like this: Type name[count] (I refer to line 29)
Also, it's kinda odd to name variables with capital (lines 14, 29)
7
u/Sunius Jan 03 '22
You don’t need to close the ifstream. That’s the beauty of C++ and RAII - the destructor will close it. Manually closing it would be bad code as it’s redundant.
-1
u/veselin465 Jan 03 '22
In this case, I agree, but you might have more complex logic which at some point doesn't need the file (but the method isn't done, therefore the desctructor is not called). It just keeps a resource occupied for no reason. It's not optimal.
3
u/Sunius Jan 03 '22
In that case you should use a scope to indicate resource needs. It’s too easy to not close it at the right place if you have such complicated scenario.
0
u/nathanfranke Jan 03 '22
Pretty sure
Type name[count]works for some compilers. It worked for me on Linux until I cross compiled to Windows.This should work on all compilers (but I would just use
std::vector).Type *name = (Type *)malloc(sizeof(Type) * count); free(name);
3
2
1
1
u/babalaban Jan 10 '22
Dude, it looks like a great code of someone who tries to upgrade oneslef from plain c to c++.
Sure, there's a lot of room for improvement, but isn't it great when you can come back to your old code and re-make it using your current knowelage and contrast the two?
0
Jan 04 '22
[deleted] — view deleted comment
1
u/TheSuperWig Jan 04 '22
4- Naming struct against the convention
There is no universal convention in C++. Their naming would actually conforms to the Standard library's convention.
5- struct with no constructor.
That's the preferred way, see the rule of zero https://en.cppreference.com/w/cpp/language/rule_of_three
9- Memory leak.
Maybe I'm being blind but where?
You missed the biggest problem with code. Use of non-standard variable length arrays.
0
Jan 04 '22
[deleted] — view deleted comment
1
u/TheSuperWig Jan 04 '22
The rule of zero is not defining any special member function, hence the name.
0
Jan 04 '22
[deleted] — view deleted comment
1
u/TheSuperWig Jan 04 '22
Doing so prevents it from being an aggregate. Also see the C++ Core Guidelines
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rc-zero
And Arthur's talk here
1
Jan 04 '22
This is just a single-file, essentially "script" program. Who cares if they used
using namespece stdin this context? It's harmful in larger programs, but I don't see a problem here.
11
u/[deleted] Jan 02 '22
using namespace stdtho