By using this site, you agree to our updated Privacy Policy and our Terms of Use. Manage your Cookies Settings.
455,111 Members | 1,366 Online
Bytes IT Community
+ Ask a Question
Need help? Post your question and get tips & solutions from a community of 455,111 IT Pros & Developers. It's quick & easy.

Maybe own a resource: is this a good design?

P: n/a
I happened across this little library for playing with ELF files.

http://elfio.sourceforge.net/

It's written in C++ which makes it more attractive than most packages like
this, which are written in C. The design seems reasonably coherent.
Nonetheless, there are a few aspects which I decided to see if I could
change. One of these was the fact that it passes pointers to istreams
rather than references. It also does stuff like void** ppObj, which always
makes me think someone's trying to pull one over on me. It's pretty clear
to me that it's not making extensive, and direct use of RAII, os I decied
to see if I could change that.

While I was digging through the uses of std::istream trying to iliminate
pointers, I cam across this

ELFI::ELFI()
{
m_nRefCnt = 1;
m_nFileOffset = 0;
m_bOwnStream = false;
m_bInitialized = false;
std::fill_n( reinterpret_cast<char*>( &m_header ), sizeof( m_header ),
'\0' );
}

// all of this distructory stuff make me uncomfortable, and I'm going to try
// to find a cleaner way to do it. Not that it's terrible, or anything.

ELFI::~ELFI()
{
std::vector<const IELFISection*>::const_iterator it;
for ( it = m_sections.begin(); it != m_sections.end(); ++it ) {
delete const_cast<IELFISection*>( *it );
}

std::vector<const IELFISegment*>::const_iterator it1;
for ( it1 = m_segments.begin(); it1 != m_segments.end(); ++it1 ) {
delete const_cast<IELFISegment*>( *it1 );
}

//But this one stood out:
//vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
if ( m_bOwnStream ) {
((std::ifstream*)m_pStream)->close();
delete m_pStream;
}
}
//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

The things I don't like about it are that:

* it makes the class a "maybe file handle".
* the way it was implemented, it requires the streams be held by pointers,
* it's not using RAII "correctly", in the sense that it doesn't open the
file in the constructor, and it doesn't use the delete to close the file,
* It seems to me there should, at a minimum, some kind of smart pointer,
* If an instance of this class can own a stream, or it could just as easliy
hold a stream owned by another instance, it seems possible that someone
might deleate a busy stream.

There is some referencing counting going on for the whole ELFI class, which
may be sufficient. The desigin doesn't seem /that/ bad, it just "feels"
wrong. Any other opinions on this?

I acknowledge that I am evaluating it without much knowledge of the larger
context. In some ways I believe that is a correct approach. In otherways,
it's probably very dangerous. Part of the reason I'm stopping to analyze
it before I continue to examine the program is because I want to test how
valid my observations are from this limited perspective.

--
If our hypothesis is about anything and not about some one or more
particular things, then our deductions constitute mathematics. Thus
mathematics may be defined as the subject in which we never know what we
are talking about, nor whether what we are saying is true.-Bertrand Russell
Jul 31 '05 #1
Share this Question
Share on Google+
3 Replies


P: n/a
Steven T. Hatton sade:
The things I don't like about it are that:

* it makes the class a "maybe file handle".
* the way it was implemented, it requires the streams be held by pointers,
* it's not using RAII "correctly", in the sense that it doesn't open the
file in the constructor, and it doesn't use the delete to close the file,
* It seems to me there should, at a minimum, some kind of smart pointer,
* If an instance of this class can own a stream, or it could just as easliy
hold a stream owned by another instance, it seems possible that someone
might deleate a busy stream.


If the design is flawed by your standards, then why not join the project
and do a contribution, or at least direct your concerns to the developers?

Tobias
--
IMPORTANT: The contents of this email and attachments are confidential
and may be subject to legal privilege and/or protected by copyright.
Copying or communicating any part of it to others is prohibited and may
be unlawful.
Jul 31 '05 #2

P: n/a
Tobias Blomkvist wrote:
If the design is flawed by your standards, then why not join the project
and do a contribution, or at least direct your concerns to the developers?


I doubt that "I don't like this" will be considered a contribution.

--
Salu2
Jul 31 '05 #3

P: n/a
Tobias Blomkvist wrote:
Steven T. Hatton sade:
The things I don't like about it are that:

* it makes the class a "maybe file handle".
* the way it was implemented, it requires the streams be held by
pointers, * it's not using RAII "correctly", in the sense that it doesn't
open the
file in the constructor, and it doesn't use the delete to close the
file,
* It seems to me there should, at a minimum, some kind of smart pointer,
* If an instance of this class can own a stream, or it could just as
easliy
hold a stream owned by another instance, it seems possible that someone
might deleate a busy stream.


If the design is flawed by your standards, then why not join the project
and do a contribution, or at least direct your concerns to the developers?

Tobias


I was trying to determine if my observations were reasonable and valid.

--
If our hypothesis is about anything and not about some one or more
particular things, then our deductions constitute mathematics. Thus
mathematics may be defined as the subject in which we never know what we
are talking about, nor whether what we are saying is true.-Bertrand Russell
Aug 1 '05 #4

This discussion thread is closed

Replies have been disabled for this discussion.