Metadata item descriptions, using exceptions and polymorphism

classic Classic list List threaded Threaded
25 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Metadata item descriptions, using exceptions and polymorphism

Michael Hieke
Hi guys,

I propose the following to start cleaning up the code of the DBH objects:

1) Remove MetadataItem::getDescriptionSql() and
MetadataItem::getChangeDescriptionSql().

2) Modify the MetadataItem code as follows:



--- metadataitem.h: cut here ---

class MetadataItem: public Subject, public Element
...
private:
     void setDescriptionM(wxString description);
...
protected:
     virtual void loadDescription();
     void loadDescription(wxString loadStatement);
     virtual void saveDescription(wxString description);
     void saveDescription(wxString saveStatement, wxString description);
...
public:
     wxString getDescription();
     bool setDescription(wxString description);
...

--- metadataitem.h: cut here ---

--- metadataitem.cpp: cut here ---

//-----------------------------------------------------------------------------
wxString MetadataItem::getDescription()
{
     if (!descriptionLoadedM)
         loadDescription();
     return descriptionM;
}
//-----------------------------------------------------------------------------
void MetadataItem::loadDescription()
{
     setDescriptionM(wxT("Not available"));
}
//-----------------------------------------------------------------------------
void MetadataItem::loadDescription(wxString loadStatement)
{
     // FIXME: implement findDatabase() vs. getDatabase()
     Database* d = getDatabase();
     if (!(d))
         throw FRError(wxT("No database assigned"));

     IBPP::Database& db = d->getIBPPDatabase();
     IBPP::Transaction tr1 = IBPP::TransactionFactory(db, IBPP::amRead);
     tr1->Start();

     IBPP::Statement st1 = IBPP::StatementFactory(db, tr1);
     st1->Prepare(wx2std(loadStatement));
     st1->Set(1, wx2std(nameM));
     if (st1->Parameters() > 1) // table/view/SP name
         st1->Set(2, wx2std(getParent()->getName()));
     st1->Execute();
     st1->Fetch();

     string value;
     IBPP::Blob b = IBPP::BlobFactory(db, tr1);
     if (!st1->IsNull(1))
     {
         st1->Get(1, b);
         b->Load(value);
     }
     tr1->Commit();
     // set value, notify observers
     setDescriptionM(std2wx(value));
}
//-----------------------------------------------------------------------------
void MetadataItem::saveDescription(wxString description)
{
     wxString msg;
     msg.Format(wxT("Objects of type %s do not support descriptions"),
         getTypeName().c_str());
     throw FRError(msg);
}
//-----------------------------------------------------------------------------
void MetadataItem::saveDescription(wxString saveStatement,
     wxString description)
{
     // FIXME: implement findDatabase() vs. getDatabase()
     Database* d = getDatabase();
     if (!(d))
         throw FRError(wxT("No database assigned"));

     IBPP::Database& db = d->getIBPPDatabase();
     IBPP::Transaction tr1 = IBPP::TransactionFactory(db);
     tr1->Start();

     IBPP::Statement st1 = IBPP::StatementFactory(db, tr1);
     st1->Prepare(wx2std(saveStatement));

     if (!description.empty())
     {
         IBPP::Blob b = IBPP::BlobFactory(db, tr1);
         st1->Get(1, b);
         b->Save(wx2std(description));
     }
     else
         st1->SetNull(1);
     st1->Set(2, wx2std(nameM));
     if (st1->Parameters() > 2)
         st1->Set(3, wx2std(getParent()->getName()));
     st1->Execute();
     tr1->Commit();
     // set value, notify observers
     setDescriptionM(description);
}
//-----------------------------------------------------------------------------
bool MetadataItem::setDescription(wxString description)
{
     if (description.compare(getDescription()) != 0)
         saveDescription(description);
     return true;
}
//-----------------------------------------------------------------------------
void MetadataItem::setDescriptionM(wxString description)
{
     if (!descriptionLoadedM || (descriptionM.compare(description) != 0))
     {
         descriptionM = description;
         descriptionLoadedM = true;
         notifyObservers();
     }
}
//-----------------------------------------------------------------------------

--- metadataitem.cpp: cut here ---



3) Create (also protected) overridden methods loadDescription() and
saveDescription(wxString description) in descendent classes, like:

void Trigger::loadDescription()
{
     loadDescription(wxT("select rdb$description from rdb$triggers ")
         wxT("where rdb$trigger_name=?"));
}

void Trigger::saveDescription(wxString description)
{
     saveDescription(wxT("update rdb$triggers set rdb$description=? ")
         wxT("where rdb$trigger_name=?"), description);
}



This would (among other things) remove some of the uses of the NodeType
member, and write the description to the database only when changed.

Especially the idiom in MetadataItem::setDescriptionM() I would like to
see used consistently in FlameRobin source code.  Right now changes to
internal state and notification of observers are not coupled, which they
should be.  In an ideal world notifyObservers() wouldn't even be public...

Maybe the overridden methods should be called doLoadDescription() and
doSaveDescription(), that is what Nando used for similar code in
BaseFrame (see readConfigSettings() and doReadConfigSettings()).

Looking forward to your comments.

Thanks

--
Michael Hieke



-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server.
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: Metadata item descriptions, using exceptions and polymorphism

Milan Babuskov-5
Michael,

Michael Hieke wrote:
> I propose the following to start cleaning up the code of the DBH objects:
>
> 1) Remove MetadataItem::getDescriptionSql() and
> MetadataItem::getChangeDescriptionSql().
>
> 2) Modify the MetadataItem code as follows:
[snip]

Have you considered to use visitor pattern here? (just a thought,
perhaps it isn't suitable)

> wxString MetadataItem::getDescription()
> {
>     if (!descriptionLoadedM)
>         loadDescription();
>     return descriptionM;
> }
> This would (among other things) remove some of the uses of the NodeType
> member, and write the description to the database only when changed.

We would also need some way to "invalidate" the description. Or you plan
to call loadDescription() directly from statement parser?

--
Milan Babuskov
http://fbexport.sourceforge.net
http://www.flamerobin.org



-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server.
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: Metadata item descriptions, using exceptions and polymorphism

Michael Hieke-2
Milan,

Milan Babuskov wrote:

> Have you considered to use visitor pattern here? (just a thought,
> perhaps it isn't suitable)

at first I thought only about improving the current code.  The methods

     virtual wxString getDescriptionSql() const;
     virtual wxString getChangeDescriptionSql() const;

do not really belong into the class interface, as they are more about
implementation than about abstract interface issues.

But using the visitor pattern might indeed completely remove much of
this code from the DBH classes, and concentrate it in one place.  And as
soon as one thinks about the other places where different metadata item
classes use different statements to perform an action - visitor could
really work better.  Using this pattern to implement the creation or
removal of DBH objects, the editing of the description, and other such
actions would really simplify the code of the DBH objects.  I will look
into it.

> We would also need some way to "invalidate" the description. Or you
> plan to call loadDescription() directly from statement parser?

No, no.  The statement parser should only call a virtual invalidate() on
the DBH object that was changed by the parsed statement.  This
invalidate() method would (among other things) set descriptionLoadedM to
false, and finally notify the observers.  The first observer calling
getDescription() would trigger the reloading of the description.

Thanks

--
Michael Hieke


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server.
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: Metadata item descriptions, using exceptions and polymorphism

Michael Hieke-2
I wrote:

> But using the visitor pattern might indeed completely remove much of
>  this code from the DBH classes, and concentrate it in one place.
> And as soon as one thinks about the other places where different
> metadata item classes use different statements to perform an action -
> visitor could really work better.  Using this pattern to implement
> the creation or removal of DBH objects, the editing of the
> description, and other such actions would really simplify the code of
> the DBH objects.  I will look into it.

On further thinking about the visitor pattern I must say that there are
two cases here:

- It is applicable for e.g. creating or deleting database objects, since
such code is called in response to user interaction.  The calling code
can create the visitor, and let it work its magic.
- It does however not seem really applicable for the loading of the
metadata item description, or other such implementation details.  The
objects have a method to return properties (like the description), but
calling code should not know or even care whether they need to be loaded
from the database, or are already cached.  So the solution outlined in
the original posting is IMHO better suited.

Thanks

--
Michael Hieke


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server.
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: Metadata item descriptions, using exceptions and polymorphism

Nando Dessena
Michael,
sorry for being so late, but I've been reading and pondering about the
proposal and I like it. I also think that using the visitor pattern
might be warranted also in this case. Please read on.

>> But using the visitor pattern might indeed completely remove much of
>> this code from the DBH classes, and concentrate it in one place.
>> And as soon as one thinks about the other places where different
>> metadata item classes use different statements to perform an action -
>> visitor could really work better.  Using this pattern to implement
>> the creation or removal of DBH objects, the editing of the
>> description, and other such actions would really simplify the code of
>> the DBH objects.  I will look into it.

Rather that the editing of the description, you could use the visitor
pattern to implement the loading of the description. Just have MetadataItem
create a LoadDescriptionVisitor v and call v.visit(this). The problem
is, I've been told, that "this" is not polymorphic, so you'd have to
duplicate the code in all descendants. If this is true, then we're
stuffed. Otherwise the visitor would still help you keep some of the
code out of the DBH, and keep all description-loading SQL statements
together without the need for NodeType.

MH> On further thinking about the visitor pattern I must say that there are
MH> two cases here:

MH> - It is applicable for e.g. creating or deleting database objects, since
MH> such code is called in response to user interaction.  The calling code
MH> can create the visitor, and let it work its magic.
MH> - It does however not seem really applicable for the loading of the
MH> metadata item description, or other such implementation details.  The
MH> objects have a method to return properties (like the description), but
MH> calling code should not know or even care whether they need to be loaded
MH> from the database, or are already cached.  So the solution outlined in
MH> the original posting is IMHO better suited.

See above. An object can create a visitor and apply it to itself. I
don't think it's too much coupling if an object knows one or more of
its visitors... Or is it?

Ciao
--
Nando Dessena
http://www.flamerobin.org



-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server.
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: Metadata item descriptions, using exceptions and polymorphism

Milan Babuskov-5
In reply to this post by Michael Hieke-2
Michael Hieke wrote:
>> We would also need some way to "invalidate" the description. Or you
>> plan to call loadDescription() directly from statement parser?
>
> No, no.  The statement parser should only call a virtual invalidate() on
> the DBH object that was changed by the parsed statement.  This
> invalidate() method would (among other things) set descriptionLoadedM to
> false, and finally notify the observers.  The first observer calling
> getDescription() would trigger the reloading of the description.

I generally agree, but in order to be efficient (think working over a
modem line) we need a granularity for invalidation. For example, if you
add a column to the table, it shouldn't invalidate the description (and
every other property). This needs more thinking...

--
Milan Babuskov
http://fbexport.sourceforge.net
http://www.flamerobin.org



-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server.
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: Metadata item descriptions, using exceptions and polymorphism

Milan Babuskov-5
In reply to this post by Nando Dessena
Nando Dessena wrote:
> Rather that the editing of the description, you could use the visitor
> pattern to implement the loading of the description. Just have MetadataItem
> create a LoadDescriptionVisitor v and call v.visit(this). The problem
> is, I've been told, that "this" is not polymorphic, so you'd have to
> duplicate the code in all descendants.

This can be tricky.  <- a perfect sentence describing it.

You can "implement" visitor for reduced number of classes, as long as
you implement it for base class. For example:

LoadDescriptionVisitor::visit(MetadataItem&)

hides all the

MetadataItemVisitor::visit(...)

functions. So, yes, you can write v.visit(*this)

> See above. An object can create a visitor and apply it to itself.

Yep, that's one way to do it.


--
Milan Babuskov
http://fbexport.sourceforge.net
http://www.flamerobin.org



-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server.
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: Metadata item descriptions, using exceptions and polymorphism

Nando Dessena
Milan,

M> LoadDescriptionVisitor::visit(MetadataItem&)

M> hides all the

M> MetadataItemVisitor::visit(...)

M> functions. So, yes, you can write v.visit(*this)

No that's not what I am after. The visit() function should receive the
concrete type:

LoadDescriptionVisitor::visit(Table&)

then, you should be able to call it this way:

void MetadataItem::loadDescription()
{
    LoadDescriptionVisitor visitor;
    visitor.visit(this*);
}

Finally, if the current object in the code above is Table, the Table&
version of LoadDescriptionVisitor::visit() should be called. Is this
it?

Ciao
--
Nando Dessena
http://www.flamerobin.org



-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server.
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: Metadata item descriptions, using exceptions and polymorphism

Michael Hieke-2
In reply to this post by Milan Babuskov-5
Milan,

Milan Babuskov wrote:

> I generally agree, but in order to be efficient (think working over a
> modem line) we need a granularity for invalidation. For example, if
> you add a column to the table, it shouldn't invalidate the
> description (and every other property). This needs more thinking...

agreed, but at some time the new Column object has to be added to the
Table object, the code doing that seems to be the right place to do a
minimal invalidation of members, it has all the information it needs.
That would need no call to invalidate() at all, then.

And I agree of course, we should call invalidate() in such a way that
minimal updates are possible.  But let's not go the way to add something
like a parameter "aspect" to invalidate().

Maybe we should not have something like

     std::vector<ColumnConstraint> *getUniqueConstraints();

as part of the interface of Table, but something like

   ColumnConstraints& getUniqueConstraints();

instead.  Having such objects would allow us to call invalidate() on
them.  It would hide those implementation details, too.

Thanks

--
Michael Hieke


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server.
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: Metadata item descriptions, using exceptions and polymorphism

Michael Hieke-2
In reply to this post by Nando Dessena
Nando,

Nando Dessena wrote:

> No that's not what I am after. The visit() function should receive
> the concrete type:
>
> LoadDescriptionVisitor::visit(Table&)
>
> then, you should be able to call it this way:
>
> void MetadataItem::loadDescription()
> {
>     LoadDescriptionVisitor visitor;
>     visitor.visit(this*);
> }
>
> Finally, if the current object in the code above is Table, the Table&
> version of LoadDescriptionVisitor::visit() should be called. Is this
> it?

That seems only possible with languages like Smalltalk.  The C++
compiler needs to decide which method to call in
MetadataItem::loadDescription(), and at compilation time it does not
know of which class this will be.

Thanks

--
Michael Hieke


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server.
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: Metadata item descriptions, using exceptions and polymorphism

Milan Babuskov-5
In reply to this post by Nando Dessena
Nando Dessena wrote:
> The visit() function should receive the
> concrete type:
>
> LoadDescriptionVisitor::visit(Table&)

This would imply that there *is* a
LoadDescriptionVisitor::visit(Table&), just what you want to avoid
(having it made for each type).

> then, you should be able to call it this way:
>
> void MetadataItem::loadDescription()
> {
>     LoadDescriptionVisitor visitor;
>     visitor.visit(this*);
> }

And this gains us exactly what?

If I'm not mistaken, the issue here is that the code to load
descriptions is the same for most object types.

> Finally, if the current object in the code above is Table, the Table&
> version of LoadDescriptionVisitor::visit() should be called. Is this
> it?

I don't think so. AFAIU, the visit(table), visit(view), etc. would all
have the same code. We want to avoid that. The only thing that *does*
differ is the SQL statement used. Now, I looked into that, and the
statements could be built from type name (with exception of column and
parameter):

select rdb$description from rdb$TYPENAMEs where rdb$TYPENAME = ?
update rdb$TYPENAMEs set rdb$description = ? where rdb$TYPENAME = ?

Tables and views should return "RELATION" as typename, so that could be
done at Relation level, or another special case.

Well, this looks like a case for Visitor + something else. Use a visitor
to get the statement. Use "something else" to run the statement (as it
is the same for all types). I just can't figure out what would be the
most appropriate "something else".

--
Milan Babuskov
http://fbexport.sourceforge.net
http://www.flamerobin.org



-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server.
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: Metadata item descriptions, using exceptions and polymorphism

Nando Dessena
In reply to this post by Michael Hieke-2
Michael,

MH> Maybe we should not have something like

MH>      std::vector<ColumnConstraint> *getUniqueConstraints();

MH> as part of the interface of Table, but something like

MH>    ColumnConstraints& getUniqueConstraints();

MH> instead.  Having such objects would allow us to call invalidate() on
MH> them.  It would hide those implementation details, too.

this is and old-time issue and meant to be solved by Gregory's work on
Composite. The collections are inherited from the same base class as
the single items. Have a look at it.

Ciao
--
Nando Dessena
http://www.flamerobin.org



-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server.
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: Metadata item descriptions, using exceptions and polymorphism

Michael Hieke-2
In reply to this post by Nando Dessena
Nando, Milan,

Nando Dessena wrote:

> Rather that the editing of the description, you could use the visitor
> pattern to implement the loading of the description.

I understand this, but I don't really care much for it.  The visitor
pattern looks like a good idea when it implements a command, is created
outside of the DBH classes, and concentrates the differences between the
descendent classes in one place.
For the loading of the description OTOH we need descriptionM,
descriptionLoadedM and the accessors in the MetadataItem class.  I have
outlined a solution that has a minimal public interface (getter and
setter), and keeps everything else as implementation details.  Using a
visitor would need us to make some of those classes friends, or expose
more of the implementation details in the public interface.  Much effort
for little gain.  I must say that I would prefer the overridden methods
over the visitor.

> Just have MetadataItem create a LoadDescriptionVisitor v and call
> v.visit(this). The problem is, I've been told, that "this" is not
> polymorphic, so you'd have to duplicate the code in all descendants.
> If this is true, then we're stuffed.

I think we would get the same amount of code, only in one new file
instead of in those DBH classes files.

> See above. An object can create a visitor and apply it to itself. I
> don't think it's too much coupling if an object knows one or more of
> its visitors... Or is it?

It is possible, but looks to me a little like a solution in search of
the problem.  But if you two prefer the solution with the visitor, I
will not insist on the other one.

Thanks

--
Michael Hieke


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server.
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: Metadata item descriptions, using exceptions and polymorphism

Nando Dessena
In reply to this post by Milan Babuskov-5
Milan,

MB> Nando Dessena wrote:
>> The visit() function should receive the
>> concrete type:
>>
>> LoadDescriptionVisitor::visit(Table&)

MB> This would imply that there *is* a
MB> LoadDescriptionVisitor::visit(Table&), just what you want to avoid
MB> (having it made for each type).

are you joking? :-) Visitors visit the concrete types, not the
abstract types. I don't want to avoid just that, I want to avoid
having to define:

void Table::loadDescription
{
    LoadDescriptionVisitor visitor;
    visitor.visit(this);
}

and

void Column::loadDescription
{
    LoadDescriptionVisitor visitor;
    visitor.visit(this);
}

and so on, but only

void MetadataItem::loadDescription
{
    LoadDescriptionVisitor visitor;
    visitor.visit(this);
}

>> then, you should be able to call it this way:
>>
>> void MetadataItem::loadDescription()
>> {
>>     LoadDescriptionVisitor visitor;
>>     visitor.visit(this*);
>> }

MB> And this gains us exactly what?

perhaps LoadDescription is not a good name for an example.
GetReadDescriptionSQL might be a better name, in case of a visitor
that just retrieves the select statement.

MB> If I'm not mistaken, the issue here is that the code to load
MB> descriptions is the same for most object types.

Not quite. The code might be the same but the SQL instructions are
different. We want to factor out those. We can factor those out in
virtual functions or in a visitor, depending on the fact that we want
them scattered or all in the same place.

>> Finally, if the current object in the code above is Table, the Table&
>> version of LoadDescriptionVisitor::visit() should be called. Is this
>> it?

MB> I don't think so. AFAIU, the visit(table), visit(view), etc. would all
MB> have the same code.

See above, the example is not the best possible example. Of course the
code in those functions would be different, otherwise the visitor is
just clutter.

MB>  We want to avoid that. The only thing that *does*
MB> differ is the SQL statement used. Now, I looked into that, and the
MB> statements could be built from type name (with exception of column and
MB> parameter):

MB> select rdb$description from rdb$TYPENAMEs where rdb$TYPENAME = ?
MB> update rdb$TYPENAMEs set rdb$description = ? where rdb$TYPENAME = ?

This would mean way too much coupling and way too much guessing. It
relies on naming conventions which aren't even under our control.
Let's just stick to the idea that for each type of metadata item we
have a select statement.

MB> Tables and views should return "RELATION" as typename, so that could be
MB> done at Relation level, or another special case.

Special cases are the Evil itself. :-)

MB> Well, this looks like a case for Visitor + something else. Use a visitor
MB> to get the statement. Use "something else" to run the statement (as it
MB> is the same for all types).

It looks like we are in agreement after all and sent astray by the bad
example.

MB> I just can't figure out what would be the most appropriate
MB> "something else".

MetadataItem::loadDescription. It should:
- create a SqlStatement (something we haven't got yet).
- create a ReadDescriptionVisitor and have it visit "this",
passing the statement.
- the visitor returns the description.

for this to work, we need "this" to be polymorphic.

Ciao
--
Nando Dessena
http://www.flamerobin.org



-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server.
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: Metadata item descriptions, using exceptions and polymorphism

Nando Dessena
In reply to this post by Michael Hieke-2
Michael,

>> LoadDescriptionVisitor::visit(Table&)
>>
>> then, you should be able to call it this way:
>>
>> void MetadataItem::loadDescription()
>> {
>>     LoadDescriptionVisitor visitor;
>>     visitor.visit(this*);
>> }
>>
>> Finally, if the current object in the code above is Table, the Table&
>> version of LoadDescriptionVisitor::visit() should be called. Is this
>> it?

MH> That seems only possible with languages like Smalltalk.

and Java, and Delphi, and Python, and...

MH> The C++
MH> compiler needs to decide which method to call in
MH> MetadataItem::loadDescription(), and at compilation time it does not
MH> know of which class this will be.

Well, since it appears the compiler is dumber than we are, we could
add a virtual MetadataItem* getThis() function that returns this.
Would that work? I am confused.

Ciao
--
Nando Dessena
http://www.flamerobin.org



-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server.
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: Metadata item descriptions, using exceptions and polymorphism

Michael Hieke-2
In reply to this post by Nando Dessena
Nando,

Nando Dessena wrote:

> this is and old-time issue and meant to be solved by Gregory's work
> on Composite.

I take this as: it is not feasible in the foreseeable future :-(

> The collections are inherited from the same base class as
> the single items. Have a look at it.

You mean the code in collection.h, right?  I see a method

   virtual T* add()

with the following comment:

// Creates new item, adds it and returns pointer to it.
// notify() is *not* called since newly added object still doesn't
// have its properties set, so for example getName() would return
// empty wxString. It is responsibility of the caller to call notify
// after it has set the properties.

I think we could well call notify() in there, as long as the collection
is lock()ed before add() is called, it would only mark the internal flag
"notification needed", and only on unlock() the observers would be
notified.  That would eliminate the need for invalidate().  Or am I
missing something here?

Thanks

--
Michael Hieke


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server.
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: Metadata item descriptions, using exceptions and polymorphism

Nando Dessena
In reply to this post by Michael Hieke-2
Michael,

MH> I must say that I would prefer the overridden methods
MH> over the visitor.

I'm with you. Go on with your solution and let's find really useful
ways to exploit the visitor pattern.

Ciao
--
Nando Dessena
http://www.flamerobin.org



-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server.
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: Metadata item descriptions, using exceptions and polymorphism

Michael Hieke-2
In reply to this post by Nando Dessena
Nando,

Nando Dessena wrote:

> Well, since it appears the compiler is dumber than we are, we could
> add a virtual MetadataItem* getThis() function that returns this.
> Would that work?  I am confused.

well, either I am, or you are indeed.  This method would return a
MetadataItem*, so you still would not have a Table*.

>> That seems only possible with languages like Smalltalk.
>
> and Java, and Delphi, and Python, and...

It's not true with Delphi, at least.  Or please give me an example that
is equal to what you want for FR.

Thanks

--
Michael Hieke


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server.
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: Metadata item descriptions, using exceptions and polymorphism

Milan Babuskov-5
In reply to this post by Nando Dessena
Nando Dessena wrote:
> Michael,
>
> MH> I must say that I would prefer the overridden methods
> MH> over the visitor.
>
> I'm with you. Go on with your solution and let's find really useful
> ways to exploit the visitor pattern.

Agreed.

--
Milan Babuskov
http://fbexport.sourceforge.net
http://www.flamerobin.org



-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server.
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: Metadata item descriptions, using exceptions and polymorphism

Milan Babuskov-5
In reply to this post by Nando Dessena
Nando Dessena wrote:

>>>then, you should be able to call it this way:
>>>
>>>void MetadataItem::loadDescription()
>>>{
>>>    LoadDescriptionVisitor visitor;
>>>    visitor.visit(this*);
>>>}
> MB> And this gains us exactly what?
>
> perhaps LoadDescription is not a good name for an example.

Exactly.

> GetReadDescriptionSQL might be a better name, in case of a visitor
> that just retrieves the select statement.

Yes. We are thinking the same, loadDescription confused me.

> MB> If I'm not mistaken, the issue here is that the code to load
> MB> descriptions is the same for most object types.
>
> Not quite. The code might be the same but the SQL instructions are
> different. We want to factor out those. We can factor those out in
> virtual functions or in a visitor, depending on the fact that we want
> them scattered or all in the same place.

+1 for visitor for me.

> This would mean way too much coupling and way too much guessing. It
> relies on naming conventions which aren't even under our control.
> Let's just stick to the idea that for each type of metadata item we
> have a select statement.

Ok, good point.

> It looks like we are in agreement after all and sent astray by the bad
> example.

Exactly.

> MB> I just can't figure out what would be the most appropriate
> MB> "something else".
>
> MetadataItem::loadDescription. It should:
> - create a SqlStatement (something we haven't got yet).
> - create a ReadDescriptionVisitor and have it visit "this",
> passing the statement.
> - the visitor returns the description.
>
> for this to work, we need "this" to be polymorphic.

I see the problem now. Need to think about it though...

--
Milan Babuskov
http://fbexport.sourceforge.net
http://www.flamerobin.org



-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server.
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
12