FAQFAQ   SearchSearch   MemberlistMemberlist   UsergroupsUsergroups   RegisterRegister 
 ProfileProfile   Log in to check your private messagesLog in to check your private messages   Log inLog in 

asserts are useless
Goto page Previous  1, 2, 3  Next
 
Post new topic   Reply to topic     Forum Index -> gtkD
View previous topic :: View next topic  
Author Message
ShprotX



Joined: 28 Aug 2007
Posts: 24
Location: Ukraine

PostPosted: Wed Dec 05, 2007 12:40 pm    Post subject: Reply with quote

satelliittipupu wrote:

ShprotX wrote:
In spite of all that, I still disagree with you proposition to check if the variable is null in the constructor because there will be unnicessary operations: class allocation and initialization. Please, point me a place in the code from where such things are originating.

Hmm. I'm not sure if I understand that correctly.

Yes, you've understood correctly. Is it because of my bad English knowledge?

satelliittipupu wrote:
Code:
class Button
{
    this(GtkButton* gtkButton)
    {
        if(gtkButton is null)
        {
            //do something to inform the user that the creation of her gtkD object
            //failed.
        }
    }
}

So, are you suggesting that there was no such checking at all. And if that would be the case, there would be no way for the user to know if the C-object was created or not. (Except checking if( myObject.gtkButtonStruct is null), but that is ugly).
Or are you suggesting that we should throw an exception in that case (if the C-object was null on the D-objects constructor)?

The problem is that every GTK+/GLib function checks if it's arguments are null. If some function returns null and the application doesn't check this, then other functions will just print messages. But tell me please, is it abnormal for g_object_new to return null if object_type wasn't null? IMHO, it is. But it's not a runtime exception. It's programming mistake. My suggestion is to stop program execution on such kind of errors. Maybe, we could raise an exception to do so.
For example, unexpected abscence of some widget would almost always be an error if programmer didn't forsee that.
Back to top
View user's profile Send private message
Auria



Joined: 29 May 2006
Posts: 44

PostPosted: Wed Dec 05, 2007 1:40 pm    Post subject: Reply with quote

When returning null does not mean a programming error, and just means "there was no such thing", etc. then i think it's correct to return null too. In cases where it reflects actual errors i still think exceptions are better. But then, if following the current model is easier to code and would make gtkD move forward more quickly then i don't really care either
Back to top
View user's profile Send private message
ShprotX



Joined: 28 Aug 2007
Posts: 24
Location: Ukraine

PostPosted: Wed Dec 05, 2007 2:31 pm    Post subject: Reply with quote

ShprotX wrote:
But tell me please, is it abnormal for g_object_new to return null if object_type wasn't null? IMHO, it is. But it's not a runtime exception. It's programming mistake.

Oops, it is not a programming mistake, it's really bad runtime exception. But we still need to stop execution.
Back to top
View user's profile Send private message
kaarna



Joined: 03 Apr 2006
Posts: 92
Location: Finland

PostPosted: Thu Dec 06, 2007 8:11 am    Post subject: Reply with quote

Sorry, this is a long post again.

The D objects wrap the C objects. The D objects are not always new and they are not always created by the user. So, the g_objects_new isn't always called, when a D constructor creates a D wrapper object. The object could already exist. Let me explain this with a couple of examples:

For example if you have a Button. A gtkD Button inherits from Bin, so it can contain one widget. That would be an Image. When you first create a button it doesn't have a child Image, until you put one there. So, in some situation you might want to ask the Button, what Image do you have inside you. You'd call this method on Button:
Code:
public Widget getImage()
   {
      // GtkWidget* gtk_button_get_image (GtkButton *button);
      return new Widget( gtk_button_get_image(gtkButton) );
   }


This is not as good example as it could be, as it returns a Widget rather than an Image. (If a button can only contain images, then this of course should be changed in gtkD...) But anyway.

If there was an Image inside the button (the C object would already exist, so there would be no g_object_new involved) the constructor of Widget would get called, and you'd have a new D object.
But it is perfectly normal, and even the default case for a Button to not have an Image. In this case the gtk_button_get_image would return null. So the Widget this() constructor get's null as it's argument in the line:
return new Widget( gtk_button_get_image(gtkButton) );

Now the user who asked the Button about it's Image get's a D object (which is of the type Widget). And if there was no GtkImage inside the GtkButton, the D Widget will still exist and not be null, even the C object inside it is null.

It's perfectly normal, and even common in GObject to have methods that return null. The gtkD wrapping doesn't only happen when a new gobject is created. (And to be precise, only the gobjects that the user is interested are wrapped, not all of them. So you have a bunch of gobjects created all the time which don't have a corresponding D wrapper object until the user - and by that I mean the programmer - want's to do something with them in D.) The usual case when a D wrapper object is created is any kind of container objects, that contain other gobjects. The gobject is already existing inside some other gobject, and then we call for example that button.getImage() and we get a newly created D wrapper object, which wraps the underlying gobject.

If we put an exception inside every D wrapper object's constructor, our code would constantly have to check for exceptions even when doing normal stuff like the getImage above. If there would not be an Image inside the Button, our program would end if we didn't catch that exception. As it is not critical or wrong, or a programming error for our Button to not have an Image, then we should not raise an exception.

And if we do nothing, the user might check if the Image was null, and then the D wrapper object would not be null, even though the C object was null, and then the user might call some function on the Image, and then null would be passed to the corresponding C object. That would not be that bad, as the gobject's would just print a Gtk-CRITICAL error message on stdout and do nothing. (But it's not nice to have your program outputting error messages with the word critical in them...)

As you said, if this was a critical thing for the program, and something would have gone wrong, we should raise an exception and terminate the program if the user doesn't catch the exception. But, as this is just normal behaviour, and comes from the way wrapping of the C object has to be made, then I think we don't need to end the program.

The reason why I'm so concerned about this is, that the current situation where we raise the arithmetic exception by dividing by zero, can't even be caught by the user. So currently if you have a Button without an Image, and you call get Image, your program will crash even if you try to catch exception. That's crearly wrong behaviour. And there are dozens of similar methods and situations in gobject, and mostly with gstreamerD too.

But, I hope this post explains the situation a bit more. (Writing this made me realize more aspects about the whole issue, so it was good for me too!)

Here's the code I used for testing the issue:
Code:

module hello;

import gtk.MainWindow;
import gtk.Label;
import gtk.Button;
import gtk.Widget;
import gtk.GtkD;

import gtk.VBox;
import gtk.HButtonBox;

version(Tango)
   import tango.io.Stdout;
else
   import std.stdio;

import gtkc.gtk;
import gtkc.gtktypes;

class HelloWorld : MainWindow
{
   this()
   {
      super("GtkD");
      setBorderWidth(10);
      
      mybox = new VBox(false, 0);
      myButton = new Button("My button.");
      myButton.addOnClicked( &onMyButton );
      
      add( mybox );
      
      mybox.packStart( myButton, false, false, 0 );
      mybox.packStart( new Label("Hello World"), false, false, 0 );
      
      showAll();
   }
   
   VBox mybox;
   Button myButton;
   
   void onMyButton( Button unused_button )
   {
      Stdout("Button clicked.").newline;
      
      //To show you the Gtk-CRITICAL error message this will give you:
      gtk_button_clicked(null);
            
            
      Widget an_image;
      
      try
      {
         an_image = myButton.getImage();
      }
      catch(Exception ex)
      {
         //Exception catching doesn't help with the divide by zero.
         //That's nasty.
         version(Tango)
            Stdout("The image was null.").newline;
         else
            writefln("The image was null.");
      }
      
      //It will crash here, and there's nothing you can
      //do about it currently. The arithmetic exception
      //will be raised.
      
      //And here's what I propose.
      //This is what you'd do normally.
      //If an_image wouldn't exist.
      if( an_image !is null )
      {
         version(Tango)
            Stdout("The image was OK.").newline;
         else
            writefln("The image was OK.");
      }
      else
      {
         version(Tango)
            Stdout("The image was null.").newline;
         else
            writefln("The image was null.");
      }
   }
   
}

void main(char[][] args)
{
   //I've just noticed that these work too!:
   Gtk.init(args);
   new HelloWorld();
   Gtk.main();
}



And here's the dsss.conf file for those of you who've installed their gtkD with DSSS:
Code:

name = hello

requires = gtkD

[hello.d]
type = binary
target = hello
buildflags = -debug=Tango -I/usr/local/include/d/src -L-ldl
Back to top
View user's profile Send private message AIM Address MSN Messenger
ShprotX



Joined: 28 Aug 2007
Posts: 24
Location: Ukraine

PostPosted: Thu Dec 06, 2007 11:53 am    Post subject: Reply with quote

satelliittipupu wrote:
Code:
public Widget getImage()
   {
      // GtkWidget* gtk_button_get_image (GtkButton *button);
      return new Widget( gtk_button_get_image(gtkButton) );
   }

Code:
public Widget getImage()
   {
      // GtkWidget* gtk_button_get_image (GtkButton *button);
      GtkWidget* widg;
      widg = gtk_button_get_image(gtkButton);
      return widg ? new Widget( widg ) : NULL;
   }


satelliittipupu wrote:
And if we do nothing, the user might check if the Image was null, and then the D wrapper object would not be null, even though the C object was null, and then the user might call some function on the Image, and then null would be passed to the corresponding C object. That would not be that bad, as the gobject's would just print a Gtk-CRITICAL error message on stdout and do nothing.

If C object is null then D object will be null too and if user tried to call some method he would get segfault (or Access Violation) and that would be right!
Back to top
View user's profile Send private message
kaarna



Joined: 03 Apr 2006
Posts: 92
Location: Finland

PostPosted: Fri Dec 07, 2007 12:05 pm    Post subject: OT: satelliittipupu is now known as kaarna... Reply with quote

Ok. I'll just post this here, so that nobody gets confused:
I've changed my nick from satelliittipupu to kaarna. Just because that old nick was so awful, a bit funny and hard to type.
I'm also now known as kaarna in the #d and #d.tango channels in freenode, so that you know that I now have a more consistent net-identity.
Smile
Back to top
View user's profile Send private message AIM Address MSN Messenger
Mike Wey



Joined: 07 May 2007
Posts: 428

PostPosted: Tue Dec 18, 2007 1:24 pm    Post subject: Reply with quote

The dividing by zero in the constructor doesn't seen to be a good option.
The following code fails with a 'Floating point exception' on the getImage, to me it whould be better it getImage just returned null.

Code:
import gtk.MainWindow;
import gtk.Button;
import gtk.GtkD;
import gtk.Image;

class ButtonTest : MainWindow
{
   this()
   {
      super("Button Test");
      setDefaultSize(50, 30);
      Button btn = new Button();
      btn.setLabel("Test");
      Image image = cast(Image)btn.getImage();
      add(btn);
      showAll();
   }
}

void main(char[][] args)
{
   GtkD.init(args);
   new ButtonTest();
   GtkD.main();
}


Edit: Ticket #3
Back to top
View user's profile Send private message
JJR



Joined: 22 Feb 2004
Posts: 1104

PostPosted: Tue Dec 18, 2007 9:23 pm    Post subject: Reply with quote

Yes, I noticed this too... in fact, one of the demos explicitly tests this "feature".

I don't really know why it's done this way. It should probably be changed to throwing a more clearly "named" exception.

I also responded to the ticket.

-JJR
Back to top
View user's profile Send private message
JJR



Joined: 22 Feb 2004
Posts: 1104

PostPosted: Wed Dec 19, 2007 12:33 am    Post subject: Reply with quote

Ha, I guess the very first post in this topic explains why the "divide by zero" exception was added. It accomodates stack tracing in the debugger.

Perhaps there's a cleaner way about this?
Back to top
View user's profile Send private message
JJR



Joined: 22 Feb 2004
Posts: 1104

PostPosted: Wed Dec 19, 2007 12:59 am    Post subject: Reply with quote

In fact, I just went back and read many of the posts in this topic.

I see that this issue is already being thoroughly discussed. Smile
Back to top
View user's profile Send private message
Pse



Joined: 13 Dec 2007
Posts: 36

PostPosted: Thu Dec 20, 2007 11:09 am    Post subject: Reply with quote

I've posted my thoughts on this in ticket #3.
Once everybody has posted their thoughts, I think a poll would be in place.
Right now options seem to be:
a) Return null objects when GTK returns null pointers.
b) Raise exceptions only when constructors get called by client code (members other than constructors will not throw exceptions, but instead return null objects).
c) Return to the assert mechanism and let everything halt execution if not properly caught.

I think option 'a' is the right way to go as long as there is no need to handle more than one specific construction error. Since this is GTK, that's probably the case.

Edit:
There might be an option 'd':
d) Take advantage of D's delegates and let the user optionally specify the exception handler in the constructor. If no handler is specified, use the provided one and only print an error message to the console. Default behaviour for the handler (halt/continue/etc.) may be changed at compile time.

I'll try to provide a working example later, if it is actually doable in D.
Back to top
View user's profile Send private message
Pse



Joined: 13 Dec 2007
Posts: 36

PostPosted: Thu Dec 20, 2007 2:25 pm    Post subject: Reply with quote

So, option 'd' might look something like:
Code:
import std.stdio;

alias int GtkWidget; //Simulate GtkWidget.

/* A global ExceptionHandler class allows for
 * minimum code duplication and easily configurable
 * default behaviour. */
class ExceptionHandler
{
    void defaulthandler(Exception exception)
    {   
        version(AutoException)
        {
            throw exception;
        }
        else
        {
            writefln(exception.toString());
        }
       
    }
}

class Widget
{
    this(GtkWidget* widget, void delegate(Exception exception) handler=null)
    {
        if(widget) this.m_widget = widget;
        else
        {
            if(handler is null) handler = &(new ExceptionHandler).defaulthandler;
            //We do return a null value since the object is invalid.
            this = null;
            //According to the errors above we may pass different exceptions
            //to the handler. This is the real power of exceptions. Let the user
            //handle more than one error condition.
            handler(new Exception("Whoa! Something went wrong!"));
        }
    }
   
private:
    GtkWidget* m_widget;
}

int main()
{
    /* No handler, let gtkd handle the error. */
    Widget g = new Widget(null);
    /* We do get a null object. */
    if(g is null) writefln("The widget is null.");

    /* Specify a construction error handler.
     * Note the user needn't check for the return value as he can
     * specify what to do directly in the handler (think about
     * nested functions, etc..).
     */
   
    void NoWidget(Exception exception){ writefln("We have no widget :(."); }
    Widget s = new Widget(null, &NoWidget);

    /* We may throw exceptions with a function literal. */
    try Widget d = new Widget(null, (Exception exception){ throw exception; });
    catch{ writefln("Exception discarded."); }

    /* We can let the exception slide past us. */
    Widget d = new Widget(null, (Exception exception){ throw exception; });

    /* The obvious downside to all of this is that exceptions still have to be
     * manually thrown. We can change that behaviour at compile time with a simple
     * version statement (see ExceptionHandler declaration). */

    return 0;
}


It might give us the best of both worlds.
So what are the advantages next to something as simple as:
Code:
gtkd code:
    return pointer ? pointer : null;
Client code:
    if(Widget is null) throw new Exception("null");

a) We can easily give the user information about the error (each error can be handled separately), and only if the user requests it. Moreso, if the user wants to know such information, he has to correctly and cleanly keep it inside its own codeblock.
b) The way gtkD handles errors can easily be changed at compile time by a switch.
c) The user is in no way obliged to handle the error (unless gtkD is set up to always throw exceptions). Code like this is perfectly valid:
Code:
Widget w = new Widget();
if(w is null) writefln("No widget.");


All in all, this is nothing more than a simple errorhandler. However, the power of D's delegates and nested functions make it easy and clean to handle non-critical error conditions (exceptions), while keeping perfect compatibility with GTK's model of 'null' pointers. GTK programmers will feel right at home. D programmers can leverage the power of nested error handling and throwing exceptions.
Back to top
View user's profile Send private message
JJR



Joined: 22 Feb 2004
Posts: 1104

PostPosted: Thu Dec 20, 2007 8:06 pm    Post subject: Reply with quote

Looks like there are some promising solutions to this one. Nice!

Option (d) looks very tempting... bonus points for matching the language name, no less. Wink
Back to top
View user's profile Send private message
ShprotX



Joined: 28 Aug 2007
Posts: 24
Location: Ukraine

PostPosted: Thu Dec 20, 2007 8:29 pm    Post subject: Reply with quote

Let's look at the reason, why constructor with gobject as an argument can be called.
1. From within gtkD library. Some method have gotten a null gobject and is trying to wrap it instead of returning null. It is inadmissible because there is no big difference between "return Widget(get_some_widget());" and "GtkWidget* w = get_some_widget(); return w ? Widget(w): null;" and we don't need unnecessary memory allocations.
2. From within other program/library. User gets a gobject and passes it without having been checked to the wrapping class contructor. In this case there are two ways:
a) "new" returns null. If user is going to use null instance then every non-static method (don't forget that user may create his own methods) must check if instance is null. This is not a D-style (read "not a simple way"). Much more simplier would be to check if instance is null at the begginning (i.e. right after "new" is called). Good, but unnecessary memory allocation happens.
b) constructor raises an exception, i.e. what have you done, stupid user? In this case user must check if gobject is null, not allowing any exceptions to be raised. Do not forget that user manipulates glib/gtk structures bypassing gtkD at his own risk, so exception is just a light punishment for such kind of carelessness! Other question is what kind of exception will be raised?
All this time I was trying to say that there is a way to avoid passing null gobject to the class constructor so there will be no unnecessary memory allocations.
Here is my sample code (some functions/methods names are fabricated; I don't know how localization is made in gtkD, so using GTK+ style):
Code:


class EGlibAssert: Exception
{
   this(char[] msg)
   {
      super(msg);
      version(FatalExceptions)
      {
         Stderr(msg).newline;
         exit(1);
      }
   }
}

class EGObjectNullInConstructor: EGlibAssert;
class EGObjectCreation: EGlibAssert;

class Widget: ObjectGtk
{
   protected GtkWidget* gtkWidget;

   public this (GtkWidget* gtkWidget)
   {
      if (!gtkWidget)
         throw new EGObjectNullInConstructor(_("Null gobject has been passed to the constructor of Widget"));
      super(cast(GtkObject*)gtkWidget);
      this.gtkWidget = gtkWidget;
   }

   public this(Gtype type)
   {
      GtkWidget* tmpobj = gtk_widget_new(type);
      if (!tmpobj)
         throw new EGObjectCreation(_("Couldn't create Widget"));
      this(tmpobj)
   }

   public Clipboard getClipboard(GdkAtom selection)
   {
      // GtkClipboard* gtk_widget_get_clipboard (GtkWidget *widget,  GdkAtom selection);
      GtkClipboard* tmpobj = gtk_widget_get_clipboard(gtkWidget, selection);
      return tmpobj ? new Clipboard( tmpobj ) : null;
   }
}

int main()
{
   /* Widget w = new Widget(null); // wrong */

   Widget w1 = new Widget(GTK_WIDGET_TYPE);
   /* if (w1 is null) // impossible */

   GtkWidget* temp_widget = gtk_widget_new(GTK_WIDGET_TYPE);
   if (temp_widget)
   {
      w2 = new Widget(temp_widget);   
      Clipboard = w2.getClipboard(somevar);
      if (Clipboard !is null)
         // Use clipboard
   }
   /* else
      // widget hasn't been created */

   try
   {
      w3 = new Widget(some_gtype);
   }
   catch (EGObjectCreation o)
   {
      Stderr(_("Couldn't create a widget of some_gtype")).newline;
   }
   
   return 0;
}


PS: btw, what the hell? Current implementation of constructor is
Code:
   public this (GType type, char[] firstPropertyName, ... )
   {
      // GtkWidget* gtk_widget_new (GType type,  const gchar *first_property_name,  ...);
      this(cast(GtkWidget*)gtk_widget_new(type, Str.toStringz(firstPropertyName)) );
   }


Last edited by ShprotX on Fri Dec 21, 2007 2:43 am; edited 2 times in total
Back to top
View user's profile Send private message
Pse



Joined: 13 Dec 2007
Posts: 36

PostPosted: Thu Dec 20, 2007 9:45 pm    Post subject: Reply with quote

JJR wrote:
Looks like there are some promising solutions to this one. Nice!

Option (d) looks very tempting... bonus points for matching the language name, no less. Wink


It does provide an alternative, however there are three things I cannot wrap (pun intended) my head around about option 'd':
1) It implies code pollution. Member functions' arguments should not be polluted with gtkD specific implementation code (error handler delegates).
2) It's confusing to put the code that handles the exception before the code that may throw it (nested functions cannot be forward referenced).
3) You need to explicitly state the errorhandler for each call.

So, I've gone back and tackled this in a KISS manner: runtime exception handler selection (defaulting to GTK style error handling).

Here's the example code:
Code:
import std.stdio;

alias int GtkWidget; //Simulate GtkWidget.

class ExceptionHandler
{
    static void enableExceptions(){ writefln("Exceptions are enabled.");
                                    callHandler = &throwException; }
    static void disableExceptions(){ writefln("Exceptions are disabled.");
                                     callHandler = &printException; }
 
    static void printException(Exception exception)
    {
        exception.print();
    }
    static void throwException(Exception exception)
    {
        throw exception;
    }

    static void function(Exception exception) callHandler = &printException;   
}

class Widget
{
    this(GtkWidget* widget)
    {
        if(widget) this.m_widget = widget;
        else
        {
            scope(exit) this = null;
            ExceptionHandler.callHandler(new Exception("No widget for you."));
        }
    }
private:
    GtkWidget* m_widget;
}

int main()
{
    Widget g = new Widget(null);
   
    /* No exceptions are thrown, default behaviour. */
    /* We need to manually check for our new object: */
    if(g is null) writefln("No widget for us.");

    /* But now, we can also leverage the power of real exceptions: */
    ExceptionHandler.enableExceptions();
    /* We can tell D to automatically disable exceptions in case we forget. */
    scope(exit) ExceptionHandler.disableExceptions(); /* Cool! */
    /* Even cooler: we can now create a series of objects and forget about
     * manually checking for null pointers for each and every one of them!
     * GTK made easy! */
    Widget a,b,c;
    try
    {
        a = new Widget(new int);
        b = new Widget(new int);
        c = new Widget(null);
    }
    catch(Exception e){ writefln("Wow, caught an exception"); }

    /* The user can have his own custom handler! */
    static void CustomHandler(Exception e){ writefln("Custom handler called!"); }
    ExceptionHandler.callHandler = &CustomHandler;
    Widget d = new Widget(null);

    ExceptionHandler.enableExceptions();
    /* We let the exception pass through! */
    Widget e = new Widget(null);

    return 0;
}

This includes:
* GTK style error handling as the default behaviour (option 'a'):
* D style error handling (exceptions) on demand.
* The power of nested handlers (option 'd') as custom handlers.

We also get a few extras that perfectly suit GTK development:
* We can 'try' full blocks of code without resorting to 'if' checks for each statement (look at the 'try' block in the example, this is typical of GTK development).
* Code clarity. Look at widgets 'a', 'b' and 'c' and how clear is their declaration before the 'try' block.

What could be added:
* Support for delegates as handlers.
* Per object custom error handlers (!).
Back to top
View user's profile Send private message
Display posts from previous:   
Post new topic   Reply to topic     Forum Index -> gtkD All times are GMT - 6 Hours
Goto page Previous  1, 2, 3  Next
Page 2 of 3

 
Jump to:  
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum


Powered by phpBB © 2001, 2005 phpBB Group