• 1

• 6.5k
• 18
• 41

#Implementation

• using namespace std; is considered bad practice and should be avoided.
• Naming: Many variable names tell nothing about their meaning. For example, what does value2 do? How is it different from value? Better names enhance the readability of the code!
• The IntFile constructor should probably take the path to the file as a parameter (there might be other files than "test.txt").
• The IntFile constructor never initializes IntFile::value.
• IntFile::file and IntFile::value should probably be private.
• file is never checked if it actually is a valid file handle.
• Rule of 3/5 violation: IntFile acquires a resource and has a custom destructor. As such, the copy constructor, move constructor, copy assignment operator and move assignment operator should also be manually implemented or deleted (the compiler generated ones will very likely do the wrong thing).
• getValue and setValue assume that sizeof(int) will always be 4 bytes. This is not the case! (e.g. if compiled for 64bit, sizeof(int) == 8)
• getValue and setValue aren't aware of endianess. This can mean that writing a value 0x00000001 on one platform will be read as 0x01000000 on another.
• setValues return value is always the same. Maybe its return type should be changed to void?
• friend struct iterator does not refer to IntFile::iterator, but to std::iterator instead (reason: no forward declaration for IntFile::iterator and using namespace std; combined with #include <vector>).
• iterator::iterator(IntFile*, int, int) never initializes iterator::value2.
• iterator::iterator(IntFile*, int, int) unnecessarily calls fseek.
• iterator::iterator(int) does not initialize iterator::ptr and iterator::index, thus setting every subsequent operation up for undefined behavior (e.g. dereferncing iterator::ptr).
• iterator &iterator::operator=(const iterator&) never assigns iterator::ptr and iterator::index.
• int &iterator::operator=(int) never returns an int value.
• iterator &iterator::operator++() never returns a iterator&.
• iterator &iterator::operator--() never returns a iterator&.
• iterator &iterator::operator+=(int) never returns a iterator&.
• I can't imagine what iterator::operator+(const iterator&) or iterator::operator-(const iterator&) would be used for.
• IntFile::end(int) never checks if Number is out of bounds (could be negative or too large).
• IntFile::rbegin() and IntFile::rend(int) should probably return std::reverse_iterator<iterator> instead (currently iterating backwards has to be done with --iter instead of the expected ++iter.
• Proxy::index should be of the same type as the parameter index of IntFile::setValue and IntFile::getValue to prevent slicing errors (sizeof(long) might be greater than sizeof(int)).
• The call to print to std::cout in Proxy::operator int() is never reached.

#Design

• IntFile::setValue: I'd expect the first parameter to be the index and the second parameter to be the value).
• IntFile could provide an overload for operator[].