At 12:47 2007-02-25, John Gunnarsson wrote:
>Okay, I have studied the RAII ideome, as a realworld test I made a small
>application where
>I have wrapped a c api handle for a directory inside a raii class. Could you
>provide some
>weedback on the code below, have I understood RAII correctly?
well, it's clear that you still "think in C"
I see NO use of references here,
>If something else look weired in my code please tell me that aswell, I'm
>very good at taking critisism :)
>
>(the application is enumerating files recurivly from a strarting dir)
>
>//John
>
>----------------------------------------------
>
>#include <iostream>
>#include <sys/types.h>
>#include <sys/stat.h>
>#include <sys/dir.h>
>#include <vector>
>#include <string>
>
>
>using namespace std;
>
>// RAII class for opendir
>class DirectoryHandle
>{
> public:
> DirectoryHandle(string directory)
rather than pass by value, why not:
DirectoryHandle(string const& directory)
> {
> dirhandle = opendir(directory.c_str());
> }
>
> ~DirectoryHandle()
> {
> closedir(dirhandle);
> }
>
> direct *readNext()
> {
> return readdir(dirhandle);
> }
>
> bool canAccess()
> {
> return (NULL != dirhandle);
> }
>
> private:
> DIR *dirhandle;
>};
>
>
>
>void enumerateFileSystem(vector<string> *fileList, string directory)
again, I think that using a reference would be better than a pointer.
also, you're NOT going to modify "directory", so why not tell
everyone using const ,,, thusly
void enumerateFileSystem(vector<string>& fileList, string const& directory)
>{
>
> struct direct *dirEntry;
> struct stat fileInfo;
> string path;
>
>
> // open directory for scanning (using RAII object)
> DirectoryHandle dirHandle(directory);
>
>
> while (dirHandle.canAccess() && ((dirEntry = dirHandle.readNext()) !=
>NULL))
> {
> // exclude . and .. directories
> if (strcmp(dirEntry->d_name,".") != 0 &&
>strcmp(dirEntry->d_name,"..") != 0)
> {
> path = directory + dirEntry->d_name;
>
>
>
> if (stat(path.c_str(), &fileInfo) != -1)
> {
>
>
> // check if this is a directory
> if (S_ISDIR(fileInfo.st_mode)) {
>
>
> string subDir = path + "/";
>
> // start enumerating that directory recursivly
> enumerateFileSystem(fileList, subDir);
>
>
>
> }
> else {
> // nope, this is a file
> // add to list
> fileList->push_back(path);
fileList.push_back(path);
> }
>
>
>
> }
>
>
>
> }
>
>
>
> }
>
>
>
> return;
>};
>
>
>
>int main(int argc, char *args[])
>{
> vector<string> files;
>
>
> enumerateFileSystem(&files, "/etc/");
enumerateFileSystem(files, "/etc/");
> for (size_t i = 0; i < files.size(); i++)
> {
> cout << files[i] << endl;
> }
and learn to use the standard library.......
#include <algorithm>
#include <iterator>
copy(files.begin(), files.end(), ostream_iterator<string>(cout, "\n"));
instead of the for loop.
> return 0;
>}
>
>
>
>
>On 2/11/07, Milan Babuskov <[EMAIL PROTECTED]> wrote:
> >
> > --- John Gunnarsson wrote:
> > > As you can see the cleanup code is duplicated 3 times (not very DRY)
> > > If I would have implemented the same scenario in c# the cleanupcode
> > > would have been placed in a "finally" clause only once, and would be
> > > executed wither an exception occured or not.
> > >
> > > Since I'm quite new to c++ I'm sure I just missed something very
> > > vital here, and would like to know how you solve scenarios
> > > like this in C++.
> >
> > Generally it is not a good practice to have stuff like:
> >
> > try
> > {
> > ...* x= new Something;
> > ...
> > delete x;
> > }
> > catch()
> > {
> > }
> >
> > The solution is to create objects on the stack, or use some kind of
> > wrapper object (a proxy, smart pointer, whatever) and have it's
> > constructor allocate the memory and have it's destructor free that memory.
> >
> > --
> >
> > Milan Babuskov
> > http://www.guacosoft.com
> >
> >
> >
>
>
>[Non-text portions of this message have been removed]
>
>
>
>
>To unsubscribe, send a blank message to
><mailto:[EMAIL PROTECTED]>.
>Yahoo! Groups Links
>
>
>
Victor A. Wagner Jr. http://rudbek.com
The five most dangerous words in the English language:
"There oughta be a law"
