Loughter for Developers

[chuck_starchaser]

By the way, here's a different case of non-member functions used to improve encapsulation; --mediators:

"Coupling

Finally, let's see how the use of non-member functions affects object coupling. It is a good design practice to promote loose coupling between objects. The less objects know about each other, the better. On the other hand, objects need to interact in order to make system functional. To solve this problem, Mediator Design Pattern has been introduced. Mediators are classes that handle interaction between different types and thus promote loose coupling. However, there are cases when a non-member function is enough to serve the role of a mediator. For instance, assume we have a class sample_db that wraps a database engine:

Code: Select all

class sample_db {...};

Now, imagine that in an application of ours we need to write sample_string objects to a sample_db database. The first thing that may come to our mind would be to add a function to sample_db that would take care of that:

Code: Select all

class sample_db 
{
    ...
public:
    void write_sample_string (const sample_string& str);
};

That would work, but it requires sample_db to know about sample_string, and that is bad. If we want to reuse sample_db in some other project, we would need to include sample_string even if we don't need it. A better approach would be to make a non-member function in a separate compilation unit that would serve as a mediator between those two classes:

Code: Select all

void write_sample_string_to_db (const sample_string& str, sample_db& db);

Of course, in order to make this work, sample_db would need to have a public function that writes some form of data to a database. "

from:
http://www.codeproject.com/gen/design/nonmemberfnoo.asp


As a general rule, I don't like funny assignment operators, like,

Code: Select all

class gif;
class jpeg
{
    ....
    jpeg( gif const & );
    jpeg& operator=( gif const & );
    ....
};

Transforming assignments, in my book, are non-member functions:

Code: Select all

image_transforms.h

  void operator=( jpeg &; gif const & );

Of course, this is a particularly bad example in the sense that there's data loss going from gif to jpeg. One rule I once proposed (to the creator of the Eiffel language, when I was into it), was to make the languge implicitly mandate the satisfaction of equality after assignment. In C++ that would be a matter of discipline, adding an assert to assignments, like, just being poetic with my template notation, here,

Code: Select all

template <typename U, typename V>
U& operator=( U& u, V const & v )
{
    u = v;
    assert( u == v );
};

[chuck_starchaser]

Anyways, you said you disagreed with "most" of what I was proposing. So I thought I'd look for some references so that you know where they come from, and to save myself typing


About private/protected virtual functions:
Traditionally, we would write base classes using public virtual functions to directly and simultaneously specify both the interface and the customizable behavior. For example, we might write:
Example 1: A traditional base class.

Code: Select all

class Widget
{
public:
  // Each of these functions might
  // optionally be pure virtual, and if
  // so might or might not have an
  // implementation in Widget; see
  // Item 27 in [1].
  //
  virtual int  Process( Gadget& );
  virtual bool IsDone();
  // ...
};

The problem is the “simultaneouslyâ€

[rockstar]

No offense here, but... fucking freakheads you are
At least this thread got some colors... good job!

[chuck_starchaser]

I'm not taking offense, but I am taking damage. When someone posts something serious and someone posts a joke after, first of all it pushes the post back, so it's less likely to be read; secondly it weakens the intended effect, which is to induce ideas that will be remembered. A joke tends to make the reader forget what was said, and after spending several hours collecting links for that post, it IS disappointing to have a post with a joke sitting below it. I'm not angry or resentful; just using the opportunity to mention this because it happens all too often. Then again, it is my fault that I'm writing this in the off-topic forum. Just the way things went...

[Halleck]

Er yes... a joke thread in the offtopic forum is probably not the best place for dead-serious code discussions, chuck.

[klauss]

Anyway... my previous post, with the nifty solution... was no solution. Sorry, last-minute post. I'll correct shortly (by editing).

And yes... I didn't use the operator+ since, in that case, I also find it friendlier to use a friend function... mostly because they take two arguments. A member function, taking only one, is misleading.

[chuck_starchaser]

I looked at it again and I miss the error...

Anyways, at one time I came to a conclusion that a class should either be a "data type" class, I called it, with lots of operators so that it behaves like a built-in type, sort of; or else be a "functional class" and be coded as separate implementation and interface. The former are aggregational in inheritance, and should not be used polymorphically; whereas the latter are "functional" and therefore inherit without changing size, and are more suitable for polymorphism. Not sure I still believe that totally (there are many other categories, I suspect), but pretty much still do.
And I was dreaming of coming up with a C++ metalanguage that would distinguish between these types of classes. Data-type classes would an associated "transformations.h" header with global functions that implement all needed transformations (assignments).
One principle I follow is to look very suspiciously at classes that mix inline data members and pointer or reference members. Such classes are for the most part disasters waiting to happen if they have copy constructor and assignment. If a class only has a pointer or pointers, there's usually some due attention paid to copy policy. And if they only have inlined data, there's not much risk; but when they mix the two, watch out.

[klauss]

The compiler was not preventing the derived class from using the base class' private members, and it was hiding public members on the base class as well.

Oh... another pitfall of the final proposal is that it basically disallows public member data on the implementation, at least without accessors.

[chuck_starchaser]

You lost me now... which derived class?

[klauss]

punchimball was inheriting punchimball_impl as private... that didn't stop punchimball from accessing punchimball_impl's protected members, and did stop outsiders from accessing punchimball_impl's public members.

Of course... you could say you wanted protected members to be accessible... hm... so I guess it isn't that bad. I'll add both.

[chuck_starchaser]

I must be blind; I just don't see the inheritance...

Code: Select all

/* Implementation class */
class punchimball_impl
{
   int x,y;

public:
   T dosomething(const punchimball &a) { ... }
   T dosomeother(...) { ... }
}

/* Interface class */
class punchimball
{
private:
   punchimball_impl *impl;

public:
   punchimball() : impl(new punchimball) {} //Although... make sure it isn't inlined
   ~punchimball() { delete impl; } //again, non-inline

   (wrap functions in impl, inline form)

   punchimball doveryusefulthing(...)
   {
      ...
      impl->dosomething(...);
      ...
      impl->dosomeother(...);
      ...
   }
} 

All I can see is a standard pimpl implementation; no inheritance :-/

In any case, klauss, pimpl's incurr overhead, due to the pointer; so for high level things they are okay and very desirable; but for objects that are in the thick of things, that can be prohibitive. Like you wouldn't implement an iterator as a pimple; or a 3D vector, for that matter. And in a 3D vector, dot product and cross product are functions that should use the friend definition idiom, just like operator+().

[klauss]

Because I edited to remove the inheritance.
I just edited back to include the 'inherited' option, read it again.
(editing is confusing, isn't it? I just wanted an accurate post, so that we can refer to it).

[chuck_starchaser]

Ah, I see; but if you do that, you give up the biggest advantage of the pimpl idiom, which is to elliminate a header dependency.

Like, in many cases, a header is needed only because some class needs to know the *size* of a class; so by hiding the implementation (and data members) the size of the public interface doesn't change. But if you inline or inherit the implementation, you give up that advantage.

And frankly, I find the friend definition idiom quite terse, easy to code, and expressive; not to speak of being the standard way to solve the problem.

Not that I have anything against pimpl's, but as I was mentioning in the previous post, while editing it, was that you can't always use pimpl. As in class3Dvector you want dot_prod() and cross_prod() to be binary like '+' and you don't want to go through a pointer all the time. Ergo

Code: Select all

class c3Dvector
{
    ....
    friend float dot_prod( c3Dvec const & a, c3Dvec const & b )
    {
        ....
    }
};

[chuck_starchaser]

Ah, sorry, cross_prod should be a public function defined outside the class and declared friend, precisely NOT a candidate for this idiom, since it accesses private members.
Sorry for the confusion.

Anyways, changing topics a bit, I'd like also to put asserts at the entry and exit points of functions, to implement Design by Contract (DBC). As an ex-Eiffel programmer I'm fanatical about pre/post conditions. Also, for each class there should be a protected virtual function that only compiles in debug mode, called "bool invariants()" and testing the coherence of the class with asserts. Then, public functions assert it upon entry and exit.

[klauss]

Anyways, changing topics a bit, I'd like also to put asserts at the entry and exit points of functions, to implement Design by Contract (DBC). As an ex-Eiffel programmer I'm fanatical about pre/post conditions. Also, for each class there should be a protected virtual function that only compiles in debug mode, called "bool invariants()" and testing the coherence of the class with asserts. Then, public functions assert it upon entry and exit.

I like that.
But it shouldn't be overdone, when the invariants and/or pre/post conditions are too complex, because otherwise it would make debugging too painful (unacceptable performance decrease).

[chuck_starchaser]

Thing is, if pre/post conditions, not to speak of invariants, are too complex, the design is too complex and needs breaking into finer pieces.
Note that you don't need to assert coherence of a class passed in as argument to a function, for example; as the DBC system should already have taken care of it.

A case where preconditions get too complex to express would be in a function receiving a dozen parameters, where some of them might assume special values to signify certain things, which might in fact affect the meaning of the value of another parameter...
Well, that's just BAD code. If so many parameters are being passed, and they have tight inter-dependenciex, they might as well be members of a struct or class that is passed to the function by reference or value. Then, the invariants of that class will assert coherence between memebers' states.

But also, the purpose of DBC is to express the purpose of a function in the simplest terms. Usually, looking at the asserts you can tell what the function does, faster than looking at the code. But they are nowhere nearly as thorough as the code, of course.
Typically you might assert that a pointer passed in is not null; that an array index is not negative; that a filename string is not empty, that a file you're about to open for reading exists...

About the latter:
A file not being there is a common example given for the application of exceptions (try/catch). I'd like to take exception with that tradition, or merely qualify it: If you're writing an i/o routine for a desktop appication, you might want the code to deal with a missing file in a non-catastrophic way. But for that very reason, about 90% of the code in commercial apps is exception handling. A game being a different animal, where a file "should" be there, the proper time to deal with the opposite is during debugging. At run time, let all hell break lose. If it happens, it is a BUG, and NOT an "exceptional situation", anyways.

[klauss]

Didn't mean that.
Imagine I said that one invariant in the System class is that no unit winds up inside another unit. Which wouldn't be outrageous to think. Though I wouldn't impose such a restriction, should something the likes arise... how long would it take to verify it upon every function call? Nuts.

[chuck_starchaser]

Ah, for sure; agreed.

And nor would we check that Tolwyn's first name is either Kevin or Geoffrey. No, basically what DBC concerns itself is a legal contract a function offers to the outside world: "If the caller meets these set of preconditions, I promise that on exit I will meet this set of postconditions."

And besides, DBC can never be thorough without language support for it. The fact is, the preconditions should only involve variables that are within the scope of the caller of a function....

Code: Select all

class foo
{
    int x;
public:
    int difference( int y )
    {
        //preconditions
        assert( y > 0 ); //OK
        assert( x > y ); //NOT OK, the caller cannot know x; not fair!
        //code
        int result = x - y;
        //postconditions
        assert( temp > 0 ); //OK, this concerns the caller.
        assert( x = 5 ); //BAD, none of the caller's business
        return result;
    }
};

In Eiffel, the bad examples would result in compile-time errors.

[klauss]

Don't ever think that the previous invariant isn't a sensible DBC invariant: "If no unit in the system is inside another, I can guarantee physics accuracy upon the execution of a physics cycle." It's only an invariant that is hard to verify.

[chuck_starchaser]

That IS an interesting concept, but not how DBC is applied. DBC is applied at the entry and exit of a function, rather than on a continuously executing universe of changing data streams. DBC would not in fact apply to streams' integrity vis a vis thread-safety, I wouldn't think. And you could put pre and postconditions in main(), but not mid-conditions somewhere in the middle. Not sure if I'm making any sense. But if a function is in charge of computing collision between two objects, however, you could definitely put a precondition that one of the objects isn't inside the other....

[klauss]

You're missing the fact that System::UpdatePhysics() is a function... that enters... and exits

[chuck_starchaser]

I was missing the fact, indeed.

But I imagine it must call other functions? In any case, postconditions are what a function wants to promise, and preconditions are whatever demands it can get away with. If there are data conditions that can break the algorithm, but they are too hard to detect, then we have to just live with it. However, if those horrible conditions could be prevented by starting with a healthy data set, there's nothing to prevent us from writing a data analysis routine that does a pre-check of all data files involved, and is called from the constructor, but only compiles in debug mode.

[klauss]

However, if those horrible conditions could be prevented by starting with a healthy data set, there's nothing to prevent us from writing a data analysis routine that does a pre-check of all data files involved, and is called from the constructor, but only compiles in debug mode.

Nice one.

[chuck_starchaser]

Hey, klauss; whaddya think, maybe this thread could use a healthy snip and move to some place where other coders might start depressing at all the changes acoming...? I know you like thread surgery...

[chuck_starchaser]

What do you think of this?

Code: Select all

#include <stdio.h>
#include <cassert>
#include <string>
#define invariant_assert(x)     assert((x))
#define precondition_assert(x)  assert((x))
#define postcondition_assert(x) assert((x))

class abstract_text_file : private boost::noncopyable
{
protected:
    mutable FILE *pfile_;
private:
    virtual void  open_( std::string const & fn ) const = 0;
    virtual void  read_() const = 0;
    virtual void write_() = 0;
    virtual void close_() const { ::fclose( pfile_ ); pfile_=0; }
public:
    bool is_open() const { return pfile_ != 0; }
    abstract_text_file() : pfile_(0)
    {
        postcondition_assert( ! is_open() );
    }
    virtual ~abstract_text_file()
    {
        precondition_assert( ! is_open() );
        if( pfile_ ) close();
    }
    void  open( std::string const & fn ) const
    {
        precondition_assert( ! is_open() );
        precondition_assert( ! fn.is_null() );
        open( fn );
        postcondition_assert( is_open() );
    }
    void  read() const
    {
        precondition_assert( is_open() );
        read_();
    }
    void close() const
    {
        precondition_assert( is_open() );
        close_();
        postcondition_assert( ! is_open() );
    }
};
class abstract_read_only_text_file : public abstract_text_file
{
    //to do: move implementations to cpp file...
    virtual void  open_( std::string const & fn ) const { ::fopen( pfile, "r" ); }
    virtual void write_(); //leave unimplemented; shouldn't be used.
};
class abstract_read_write_text_file : public abstract_text_file
{
    //to do: move implementations to cpp file...
    virtual void  open_( std::string const & fn ) const { ::fopen( pfile, "r+" ); }
    void write()
    {
        precondition_assert( is_open() );
        write_();
    }
};

class read_only_obj_file_t : public abstract_read_only_text_file
{
    ...
};