Login | Register
My pages Projects Community openCollabNet

Discussions > dev > Re: [gef-dev] Re: svn commit: r1145 - trunk/src/org/tigris/gef/base/ModeManager.java

gef
Discussion topic

Back to topic list

Re: [gef-dev] Re: svn commit: r1145 - trunk/src/org/tigris/gef/base/ModeManager.java

Reply

Author bobtarling
Full name Bob Tarling
Date 2008-11-29 05:08:14 PST
Message Presumably if it had been called with null previously then there would
have been other bad side effects.

Personally I'd prefer that this threw in IllegalArgumtException so
that some client developer knew exactly what they did wrong where.

Bob.

2008/11/29 Christian López Espínola <penyaskito at gmail dot com>:
> Hi Dave,
>
> On Sat, Nov 29, 2008 at 9:51 AM, Dave Thompson <argouml at davet dot org> wrote:
>> Hi Christian,
>>
>> Is there a chance that a null mode might be passed into this class? This
>> seems an odd thing to do. Your new code will prevent bad things from
>> happening as a result, but won't it just mask bad code somewhere else?
>> Perhaps I'm missing something about modes. Maybe a mode being null is used
>> for some signalling purpose elsewhere.
>
> IMHO, a public method must check the arguments. The right thing should
> be launching a exception instead of just ignoring, but GEF never had
> exceptions and its introduction would be a huge change.
>
> About signalling with null, they shouldn't use exceptions for flow control.
>
>> I am curious because at the moment, I'm getting NPEs when trying to add a
>> new mode using Tom's new mechanism, but I thought that this was just my
>> fault!
>
> If this stops you advancing on anything, just revert my changes. But I
> thought that this would be an improvement.
>
>
>
>> Dave
>>
>> penyaskito at tigris dot org wrote:
>>>
>>> Author: penyaskito
>>> Date: 2008-11-28 15:05:01-0800
>>> New Revision: 1145
>>>
>>> Modified:
>>> trunk/src/org/tigris​/gef/base/ModeManage​r.java
>>>
>>> Log:
>>> Improved ModeManager for preventing NPE. Added count() for testability.
>>>
>>> Modified: trunk/src/org/tigris​/gef/base/ModeManage​r.java
>>> Url:
>>> http://gef.tigris.or​g/source/browse/gef/​trunk/src/org/tigris​/gef/base/ModeManage​r.java?view=diff​&rev=1145&p1=tru​nk/src/org/tigris/ge​f/base/ModeManager.j​ava&p2=trunk/src​/org/tigris/gef/base​/ModeManager.java​&r1=1144&r2=114​5
>>>
>>> ====================​====================​====================​==================
>>> --- trunk/src/org/tigris​/gef/base/ModeManage​r.java (original)
>>> +++ trunk/src/org/tigris​/gef/base/ModeManage​r.java 2008-11-28
>>> 15:05:01-0800
>>> @@ -101,6 +101,10 @@
>>> /** Add the given Mode to the stack if another instance
>>> * of the same class is not already on the stack. */
>>> public void push(FigModifyingMode newMode) {
>>> + if (newMode == null) {
>>> + LOG.info("A null mode was pushed. Ignoring.");
>>> + return;
>>> + } if(!includes(newMode​.getClass())) {
>>> _modes.addElement(newMode);
>>> //fireModeChanged();
>>> @@ -308,5 +312,12 @@
>>> m.paint(g);
>>> }
>>> }
>>> + + /**
>>> + * @return the number of modes in the stack.
>>> + */
>>> + public int count() {
>>> + return _modes.size();
>>> + }
>>> }
>>>
>>> --------------------​--------------------​--------------------​---------
>>> To unsubscribe, e-mail: commits-unsubscribe@​gef.tigris.org
>>> For additional commands, e-mail: commits-help at gef dot tigris dot org
>>>
>>
>> --------------------​--------------------​--------------------​---------
>> To unsubscribe, e-mail: dev-unsubscribe at gef dot tigris dot org
>> For additional commands, e-mail: dev-help at gef dot tigris dot org
>>
>>
>
>
>
> --
> Cheers,
>
> Christian López Espínola <penyaskito>
>

« Previous message in topic | 3 of 4 | Next message in topic »

Messages

Show all messages in topic

Re: svn commit: r1145 - trunk/src/org/tigris/gef/base/ModeManager.java dthompson Dave Thompson 2008-11-29 00:51:18 PST
     Re: [gef-dev] Re: svn commit: r1145 - trunk/src/org/tigris/gef/base/ModeManager.java penyaskito Christian López Espínola 2008-11-29 03:45:52 PST
         Re: [gef-dev] Re: svn commit: r1145 - trunk/src/org/tigris/gef/base/ModeManager.java bobtarling Bob Tarling 2008-11-29 05:08:14 PST
             Re: [gef-dev] Re: svn commit: r1145 - trunk/src/org/tigris/gef/base/ModeManager.java penyaskito Christian López Espínola 2008-11-29 05:23:03 PST
Messages per page: