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 penyaskito
Full name Christian López Espínola
Date 2008-11-29 05:23:03 PST
Message On Sat, Nov 29, 2008 at 2:08 PM, Bob Tarling <bob dot tarling at gmail dot com> wrote:
> 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.

I've just committed this with the required changes in the (new) unit tests too.

> 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>
>>
>
> --------------------​--------------------​--------------------​---------
> 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 | 4 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: