Menu
▾
▴
rails-devel
|
From: Erik V. <eri...@xs...> - 2011-07-21 19:52:31
|
Brett,
I don't think this is a good change, because 'optionValue' may have been
assigned between the first and the second identical ifs.
This second if [ if (optionValue == null) ] should be read as: if
optionValue is *still* null, then...
Erik.
> -----Original Message-----
> From: Brett Lentz [mailto:wak...@us...]
> Sent: Thursday, July 21, 2011 7:02 PM
> To: rai...@li...
> Subject: [Rails-commits] rails/common
>
> rails/common/parser/Tag.java | 3 ---
> 1 file changed, 3 deletions(-)
>
> New commits:
> commit 6b7255acdcdfd3217884ae2701975166df7350b8
> Author: Brett Lentz <bl...@cl...>
> Date: Thu Jul 21 09:58:42 2011 -0700
>
> Tag: simplify conditional logic.
>
> This is really a spurious commit to test Egit.
>
> diff --git a/rails/common/parser/Tag.java b/rails/common/parser/Tag.java
> index 22c3c0a..7a7d46e 100644
> --- a/rails/common/parser/Tag.java
> +++ b/rails/common/parser/Tag.java
> @@ -326,9 +326,6 @@ public class Tag {
> break;
> }
> }
> - }
> -
> - if (optionValue == null) {
> // Take the default value
> GameOption go = GameOption.getByName(name);
> optionValue = go != null ? go.getDefaultValue() :
"";
>
>
>
>
----------------------------------------------------------------------------
--
> 5 Ways to Improve & Secure Unified Communications Unified
> Communications promises greater efficiencies for business. UC can improve
> internal communications as well as offer faster, more efficient ways to
> interact with customers and streamline customer service. Learn more!
> http://www.accelacomm.com/jaw/sfnl/114/51426253/
> _______________________________________________
> Rails-commits mailing list
> Rai...@li...
> https://lists.sourceforge.net/lists/listinfo/rails-commits
|
|
From: brett l. <bre...@gm...> - 2011-07-21 20:28:41
|
Really?
My initial reaction is that it sounds like a design flaw if the XML
properties can change in the span of 3 lines, especially given that
our current init code isn't multi-threaded.
Can you identify a case where this actually happens (and is a good thing)?
---Brett.
On Thu, Jul 21, 2011 at 12:52 PM, Erik Vos <eri...@xs...> wrote:
> Brett,
>
> I don't think this is a good change, because 'optionValue' may have been
> assigned between the first and the second identical ifs.
> This second if [ if (optionValue == null) ] should be read as: if
> optionValue is *still* null, then...
>
> Erik.
>
>> -----Original Message-----
>> From: Brett Lentz [mailto:wak...@us...]
>> Sent: Thursday, July 21, 2011 7:02 PM
>> To: rai...@li...
>> Subject: [Rails-commits] rails/common
>>
>> rails/common/parser/Tag.java | 3 ---
>> 1 file changed, 3 deletions(-)
>>
>> New commits:
>> commit 6b7255acdcdfd3217884ae2701975166df7350b8
>> Author: Brett Lentz <bl...@cl...>
>> Date: Thu Jul 21 09:58:42 2011 -0700
>>
>> Tag: simplify conditional logic.
>>
>> This is really a spurious commit to test Egit.
>>
>> diff --git a/rails/common/parser/Tag.java b/rails/common/parser/Tag.java
>> index 22c3c0a..7a7d46e 100644
>> --- a/rails/common/parser/Tag.java
>> +++ b/rails/common/parser/Tag.java
>> @@ -326,9 +326,6 @@ public class Tag {
>> break;
>> }
>> }
>> - }
>> -
>> - if (optionValue == null) {
>> // Take the default value
>> GameOption go = GameOption.getByName(name);
>> optionValue = go != null ? go.getDefaultValue() :
> "";
>>
>>
>>
>>
> ----------------------------------------------------------------------------
> --
>> 5 Ways to Improve & Secure Unified Communications Unified
>> Communications promises greater efficiencies for business. UC can improve
>> internal communications as well as offer faster, more efficient ways to
>> interact with customers and streamline customer service. Learn more!
>> http://www.accelacomm.com/jaw/sfnl/114/51426253/
>> _______________________________________________
>> Rails-commits mailing list
>> Rai...@li...
>> https://lists.sourceforge.net/lists/listinfo/rails-commits
>
>
> ------------------------------------------------------------------------------
> 5 Ways to Improve & Secure Unified Communications
> Unified Communications promises greater efficiencies for business. UC can
> improve internal communications as well as offer faster, more efficient ways
> to interact with customers and streamline customer service. Learn more!
> http://www.accelacomm.com/jaw/sfnl/114/51426253/
> _______________________________________________
> Rails-devel mailing list
> Rai...@li...
> https://lists.sourceforge.net/lists/listinfo/rails-devel
>
|
|
From: Erik V. <eri...@xs...> - 2011-07-21 22:37:16
|
As the comment in the code says: the first 'if' is there for backwards compatibility.
I don't remember the exact point of it, and most likely it's obsolete by now. I'll have to investigate that; I guess I had somehow fiddled with option names.
(What that code does is look for an option with a longer name than the one at hand. )
But either that first 'if' must be completely removed, or it must be retained as is. Your 'fix' makes no sense. Read the code.
(The danger of removing it is that some of my old saved files may no longer work).
Erik.
> -----Original Message-----
> From: brett lentz [mailto:bre...@gm...]
> Sent: Thursday, July 21, 2011 10:28 PM
> To: Development list for Rails: an 18xx game
> Subject: Re: [Rails-devel] [Rails-commits] rails/common
>
> Really?
>
> My initial reaction is that it sounds like a design flaw if the XML properties can
> change in the span of 3 lines, especially given that our current init code isn't
> multi-threaded.
>
> Can you identify a case where this actually happens (and is a good thing)?
>
> ---Brett.
>
>
>
> On Thu, Jul 21, 2011 at 12:52 PM, Erik Vos <eri...@xs...> wrote:
> > Brett,
> >
> > I don't think this is a good change, because 'optionValue' may have
> > been assigned between the first and the second identical ifs.
> > This second if [ if (optionValue == null) ] should be read as: if
> > optionValue is *still* null, then...
> >
> > Erik.
> >
> >> -----Original Message-----
> >> From: Brett Lentz [mailto:wak...@us...]
> >> Sent: Thursday, July 21, 2011 7:02 PM
> >> To: rai...@li...
> >> Subject: [Rails-commits] rails/common
> >>
> >> rails/common/parser/Tag.java | 3 ---
> >> 1 file changed, 3 deletions(-)
> >>
> >> New commits:
> >> commit 6b7255acdcdfd3217884ae2701975166df7350b8
> >> Author: Brett Lentz <bl...@cl...>
> >> Date: Thu Jul 21 09:58:42 2011 -0700
> >>
> >> Tag: simplify conditional logic.
> >>
> >> This is really a spurious commit to test Egit.
> >>
> >> diff --git a/rails/common/parser/Tag.java
> >> b/rails/common/parser/Tag.java index 22c3c0a..7a7d46e 100644
> >> --- a/rails/common/parser/Tag.java
> >> +++ b/rails/common/parser/Tag.java
> >> @@ -326,9 +326,6 @@ public class Tag {
> >> break;
> >> }
> >> }
> >> - }
> >> -
> >> - if (optionValue == null) {
> >> // Take the default value
> >> GameOption go = GameOption.getByName(name);
> >> optionValue = go != null ? go.getDefaultValue() :
> > "";
> >>
> >>
> >>
> >>
> > ----------------------------------------------------------------------
> > ------
> > --
> >> 5 Ways to Improve & Secure Unified Communications Unified
> >> Communications promises greater efficiencies for business. UC can
> >> improve internal communications as well as offer faster, more
> >> efficient ways to interact with customers and streamline customer
> service. Learn more!
> >> http://www.accelacomm.com/jaw/sfnl/114/51426253/
> >> _______________________________________________
> >> Rails-commits mailing list
> >> Rai...@li...
> >> https://lists.sourceforge.net/lists/listinfo/rails-commits
> >
> >
> > ----------------------------------------------------------------------
> > --------
> > 5 Ways to Improve & Secure Unified Communications Unified
> > Communications promises greater efficiencies for business. UC can
> > improve internal communications as well as offer faster, more
> > efficient ways to interact with customers and streamline customer service.
> Learn more!
> > http://www.accelacomm.com/jaw/sfnl/114/51426253/
> > _______________________________________________
> > Rails-devel mailing list
> > Rai...@li...
> > https://lists.sourceforge.net/lists/listinfo/rails-devel
> >
>
> ------------------------------------------------------------------------------
> 5 Ways to Improve & Secure Unified Communications Unified
> Communications promises greater efficiencies for business. UC can improve
> internal communications as well as offer faster, more efficient ways to
> interact with customers and streamline customer service. Learn more!
> http://www.accelacomm.com/jaw/sfnl/114/51426253/
> _______________________________________________
> Rails-devel mailing list
> Rai...@li...
> https://lists.sourceforge.net/lists/listinfo/rails-devel
|
|
From: brett l. <bre...@gm...> - 2011-07-21 22:48:08
|
That makes more sense from a historical perspective.
My thinking is that two back-to-back if-statements that are checking
the same condition is redundant and unnecessary. You've checked the
condition once and have a code block for it. There's no need to do the
exact same check again immediately afterward. The same backward
compatibility can be achieved with a comment block that identifies
which portions are for legacy functions and which portions are for
current functions.
But... that's sort of tangential and I'm not invested in being right.
We can revert it if needs be.
Ok... so, can you check if we even need the backwards compatible
portion anymore? I'd rather just drop that stuff to reduce the overall
complexity. If it's still needed, great, I'll revert my change.
---Brett.
On Thu, Jul 21, 2011 at 3:37 PM, Erik Vos <eri...@xs...> wrote:
> As the comment in the code says: the first 'if' is there for backwards compatibility.
> I don't remember the exact point of it, and most likely it's obsolete by now. I'll have to investigate that; I guess I had somehow fiddled with option names.
> (What that code does is look for an option with a longer name than the one at hand. )
>
> But either that first 'if' must be completely removed, or it must be retained as is. Your 'fix' makes no sense. Read the code.
> (The danger of removing it is that some of my old saved files may no longer work).
>
> Erik.
>
>> -----Original Message-----
>> From: brett lentz [mailto:bre...@gm...]
>> Sent: Thursday, July 21, 2011 10:28 PM
>> To: Development list for Rails: an 18xx game
>> Subject: Re: [Rails-devel] [Rails-commits] rails/common
>>
>> Really?
>>
>> My initial reaction is that it sounds like a design flaw if the XML properties can
>> change in the span of 3 lines, especially given that our current init code isn't
>> multi-threaded.
>>
>> Can you identify a case where this actually happens (and is a good thing)?
>>
>> ---Brett.
>>
>>
>>
>> On Thu, Jul 21, 2011 at 12:52 PM, Erik Vos <eri...@xs...> wrote:
>> > Brett,
>> >
>> > I don't think this is a good change, because 'optionValue' may have
>> > been assigned between the first and the second identical ifs.
>> > This second if [ if (optionValue == null) ] should be read as: if
>> > optionValue is *still* null, then...
>> >
>> > Erik.
>> >
>> >> -----Original Message-----
>> >> From: Brett Lentz [mailto:wak...@us...]
>> >> Sent: Thursday, July 21, 2011 7:02 PM
>> >> To: rai...@li...
>> >> Subject: [Rails-commits] rails/common
>> >>
>> >> rails/common/parser/Tag.java | 3 ---
>> >> 1 file changed, 3 deletions(-)
>> >>
>> >> New commits:
>> >> commit 6b7255acdcdfd3217884ae2701975166df7350b8
>> >> Author: Brett Lentz <bl...@cl...>
>> >> Date: Thu Jul 21 09:58:42 2011 -0700
>> >>
>> >> Tag: simplify conditional logic.
>> >>
>> >> This is really a spurious commit to test Egit.
>> >>
>> >> diff --git a/rails/common/parser/Tag.java
>> >> b/rails/common/parser/Tag.java index 22c3c0a..7a7d46e 100644
>> >> --- a/rails/common/parser/Tag.java
>> >> +++ b/rails/common/parser/Tag.java
>> >> @@ -326,9 +326,6 @@ public class Tag {
>> >> break;
>> >> }
>> >> }
>> >> - }
>> >> -
>> >> - if (optionValue == null) {
>> >> // Take the default value
>> >> GameOption go = GameOption.getByName(name);
>> >> optionValue = go != null ? go.getDefaultValue() :
>> > "";
>> >>
>> >>
>> >>
>> >>
>> > ----------------------------------------------------------------------
>> > ------
>> > --
>> >> 5 Ways to Improve & Secure Unified Communications Unified
>> >> Communications promises greater efficiencies for business. UC can
>> >> improve internal communications as well as offer faster, more
>> >> efficient ways to interact with customers and streamline customer
>> service. Learn more!
>> >> http://www.accelacomm.com/jaw/sfnl/114/51426253/
>> >> _______________________________________________
>> >> Rails-commits mailing list
>> >> Rai...@li...
>> >> https://lists.sourceforge.net/lists/listinfo/rails-commits
>> >
>> >
>> > ----------------------------------------------------------------------
>> > --------
>> > 5 Ways to Improve & Secure Unified Communications Unified
>> > Communications promises greater efficiencies for business. UC can
>> > improve internal communications as well as offer faster, more
>> > efficient ways to interact with customers and streamline customer service.
>> Learn more!
>> > http://www.accelacomm.com/jaw/sfnl/114/51426253/
>> > _______________________________________________
>> > Rails-devel mailing list
>> > Rai...@li...
>> > https://lists.sourceforge.net/lists/listinfo/rails-devel
>> >
>>
>> ------------------------------------------------------------------------------
>> 5 Ways to Improve & Secure Unified Communications Unified
>> Communications promises greater efficiencies for business. UC can improve
>> internal communications as well as offer faster, more efficient ways to
>> interact with customers and streamline customer service. Learn more!
>> http://www.accelacomm.com/jaw/sfnl/114/51426253/
>> _______________________________________________
>> Rails-devel mailing list
>> Rai...@li...
>> https://lists.sourceforge.net/lists/listinfo/rails-devel
>
>
> ------------------------------------------------------------------------------
> 5 Ways to Improve & Secure Unified Communications
> Unified Communications promises greater efficiencies for business. UC can
> improve internal communications as well as offer faster, more efficient ways
> to interact with customers and streamline customer service. Learn more!
> http://www.accelacomm.com/jaw/sfnl/114/51426253/
> _______________________________________________
> Rails-devel mailing list
> Rai...@li...
> https://lists.sourceforge.net/lists/listinfo/rails-devel
>
|
|
From: Erik V. <eri...@xs...> - 2011-07-22 10:08:06
|
> -----Original Message----- > From: brett lentz [mailto:bre...@gm...] > My thinking is that two back-to-back if-statements that are checking the > same condition is redundant and unnecessary. You've checked the condition > once and have a code block for it. There's no need to do the exact same > check again immediately afterward. Unless the condition may have changed in the meantime, which can be the case here if the first check is still needed at all. I'll check what the effect of removing it will be, and if it's harmless to my many old save files, I'll remove that code. Erik. |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X