{
get { return ValidationRules.IsValid && (_fieldManager != null || FieldManager.IsValid()); }}
The simplified object model I'm dealing with is an individual which contains a collection of individual names.
FieldManager.IsValid is false, but it doesn't matter because _fieldManager is not null. So when an individual name is invalid, individual is still returning that it's valid.
Certainly it only looks like _fieldManager is assigned if FieldManager is ever first referenced, so I assume this should be == null?
Chris
*Bump*
I just wanted to make sure that Rocky notices this as I saw him swipe by with a few posts yesterday. I checked the SVN in advance but it's still unchanged -- just want to make sure this doesn't get lost in case it's a bug as a suspect it is.
Yes, I think you are right - there's a bug. Here's a truth table that shows a problem (if I did it right - been a long time...):
T & (T | T) = T
T & (F | T) = T (but would blow up)
T & (T | F) = T (WRONG)
F & (T | T) = F
F & (F | T) = F
F & (T | F) = F
I think this is what it should be:
T & (T & T) = T
T & (F & T) = F
T & (T & F) = F
F & (T & T) = F
F & (F & T) = F
F & (T & F) = F
So if I am doinig this right, the problem is the OR should be an AND.
For IsDirty it would be:
T | (T & T) = T
T | (F & T) = T
T | (T & F) = T
F | (T & T) = T
F | (F & T) = F
F | (T & F) = F
Do you agree?
So it's currently:
ValidationRules.IsValid && (_fieldManager != null || FieldManager.IsValid());
You're proposing:
ValidationRules.IsValid && (_fieldManager != null && FieldManager.IsValid());
I initially suggested (and think is probably the right answer):
ValidationRules.IsValid && (_fieldManager == null || FieldManager.IsValid());
I initially suggested what I did because I wasn't entirely clear if _fieldManager got initialized when dealing with a class with no properties. I figured if it's the case that users who don't use managed properties don't get this initialized, _fieldManager == null would be a "valid" supporting condition.
It looked like there were many cases in the code where you were hitting FieldManager only if _fieldManager was not null - that is, it looked like perhaps you only wanted the property setting/getting to establish a FieldManager - kind of an on-demand thing.
There were a couple cases in code where however it was going right to FieldManager without checking if _fieldManager was null - effectively triggering an instance for _fieldManager - below are two:
Child_ListChanged
Child_PropertyChanged
I don't quite follow why you would change it to an "and" to be honest.... maybe I'm missing something?
Chris
RockfordLhotka:
Yes, I think you are right - there's a bug. Here's a truth table that shows a problem (if I did it right - been a long time...):
T & (T | T) = T
T & (F | T) = T (but would blow up)
T & (T | F) = T (WRONG)
F & (T | T) = F
F & (F | T) = F
F & (T | F) = FI think this is what it should be:
T & (T & T) = T
T & (F & T) = F
Shouldn't this be True. There's no field manager to check, so it should = T... (F & T) can't exist, it will always blow up.
Chris,
The problem with using OR is that it can blow up (and it is wrong if my table is correct). If the first part of OR returns false then the second part gets evaluated - which is exactly what we don't want!
Andy,
That bolded statement (T & (F | T)) is both False and safe. It is False because the inner expression returns False (F&T = F), and it is safe because the OR short-circuits and the second part is never evaluated because the first is False.
I see what you are saying, that's correct. Perhaps this can't be done in a single line. It should probably be this:
bool result = false;
if (ValidationRules.IsValid)
{
if (fieldManager == null)
result = true;
else if (FieldManager.IsValid)
result = true;
}
return result;
OK, so I think the original suggestion was right (change != to
==).
This isn’t that hard, but I’m so busy I’m only
getting a minute here and there to think about it, which is really not fun…
Rocky
From: ajj3085
[mailto:cslanet@lhotka.net]
Sent: Wednesday, March 12, 2008 10:14 AM
To: rocky@lhotka.net
Subject: Re: [CSLA .NET] Bug in 3.5 BusinessBase IsValid()?
You're right, it won't blow up, but shouldn't IsValid still
return true if ValidationRules.IsValid is true and the fieldManger is
null?
( _fieldManager != null && fieldManager.IsValid ) always returns false
if _fieldManager is null, so the entire expression will be false.
So the object will always say its not valid if _fieldManager is null if you use
T & ( F & T ). Or am I missing something?
Thank you Rocky, no hurries, just didn't want it to get lost.
I have checked in a fix for the IsValid issue. IsDirty was already correct, and oddly enough so was the VB code on both counts. But the fix is in, so things should be good now.
Actually if _fieldManager is not null (i.e. _fieldManager == null is false) don't you want to test IsValid for FieldManager?
I must be still missing something.
I like it but w/ _fieldManager :)
Chris
Andy,
This one looks OK, but I don't see the functional difference with Rocky's last solution.
Cheers,
Herman
If ValidationRules.IsValid is false, why bother checking _fieldManager.IsValid, since the end result will be false anyway!
**nevermind**
Copyright (c) Marimer LLC