Search

TypeConverter for value types (such as int, bool, short, etc.) return true to IsValid, even if the value is not valid by DanielKzu

Closed
as Fixed Help for as Fixed

154
1
Sign in
 to vote
Type: Bug
ID: 93559
Opened: 2/22/2005 3:53:12 PM
Access Restriction: Public
2
Workaround(s)
29
User(s) can reproduce this bug
TypeConverter for most value types return true to IsValid even if the value is invalid. This should not be the case, and a TryParse should be performed instead.
Details (expand)
Product Language
English
Version
Community Technology Preview December 2004
Category
Base Class Libraries and APIs
Subcategory
 
Operating System
Windows XP Professional
Steps to Reproduce
On a console application, type the following code:

TypeConverter converter = TypeDescriptor.GetConverter(typeof(int));
Debug.Assert(!converter.IsValid("asdfasdf"), "Invalid value reported as valid");

Repeat the test for bool, short, and other bulit-in value types.
Actual Results
Assert always fails, meaning that the converter is reporting as valid a value that is not.
Expected Results
Return false from converter.
Sign in to post a comment.
Posted by mcm_ham on 11/28/2011 at 3:25 PM
"TypeDescriptor.GetConverter(typeof(DateTime)).IsValid()" is not checking to see what locale you are in so it incorrectly returns false for "29/11/2011" and true for "11/29/2011" when it should be the other way around for say the "en-NZ" locale.
Posted by mcm_ham on 11/7/2011 at 6:24 PM
"TypeDescriptor.GetConverter(typeof(Unit)).IsValid(null);" returns true when it should be false.
Posted by Microsoft on 5/17/2010 at 2:47 AM
This indeed has been fixed in .NET 4.
Posted by Craig Gemmill on 4/17/2010 at 8:11 PM
Great, now all of my code that didn't work... works.
Posted by PJ on Development on 4/17/2010 at 3:56 PM
Well, I think we then can consider this bug fixed.

Just a reminder to Microsoft to close it down.
Posted by PJ on Development on 3/13/2010 at 3:25 PM
YAY!!! That's good news indeed. No more wrong behavior.

It took almost FIVE years to fix...

I guess we should count ourselves blessed because it didn't took 25 years ( http://www.osnews.com/story/19731/The-25-Year-Old-UNIX-Bug )... hehe
Posted by Maximilian Haru Raditya on 3/10/2010 at 10:15 PM
It seems it's been documented here as well:
http://msdn.microsoft.com/en-us/library/tez461eb(v=VS.100).aspx
http://msdn.microsoft.com/en-us/library/5z76f169(VS.100).aspx

See the remarks section.
Posted by PJ on Development on 3/10/2010 at 10:22 AM
@yongyi781 I'm at work right now (where we're not using .NET4), but I'll check this as soon as I'm back home. If that's the case it will be a wonderful thing.

In the meantime, can anyone verify this?
Posted by yongyi781 on 3/7/2010 at 8:24 AM
It appears the behavior has changed in .NET Framework 4.0. From what I can tell, TypeConverter.IsValid now returns false for most arguments, including arguments of that type itself. The only time I can tell it returns true is for BooleanConverters, where strings "true" and "false" return true, and for StringConverters, where any string returns true.
Posted by Mick N on 3/6/2010 at 6:30 PM
Martin.Robins idea of a dedicated warning class for these type of issues sounds excellent.
Posted by Martin.Robins on 2/26/2010 at 4:06 AM
If this were to be deferred, which I suspect is the best case scenario, then there should at least be a compiler warning that the methods in question return incorrect results. This would then further expand the message to developers that there is a problem so that they themselves can be addressing the problem before any fixes are finally released.
Posted by Mick N on 2/22/2010 at 4:14 PM
At the very least, broken features not being fixed for fear of breaking existing code should be marked as such and a view available to make them all discoverable.

This will reduce people wasting time with these known issues, reduce the chance of new breakable code being written and give the option to discuss how to better deal with issues of this nature collectively.
Posted by Mick N on 2/22/2010 at 4:05 PM
I appreciate the trade off here between Microsoft's position and the communities position.

I lean toward's fixing what is broken and to address Microsoft's concern with breaking changes I have a proposal that I believe will break the deadlock and establish a sensible precedent for all issues of this nature.

Firstly, as Microsoft has tried to do here, defer to release these changes to be available early in the cycle of a new major release. I agree introducing this late in the cycle is a bad strategy.

What Microsoft then does is tag these issues as "Breaking changes intended to fix critical broken features".

A process of communication is then undertaken to get the community accustomed to familiarising with these short list of issues. The list would be available at any time by a view into this database with a direct link that can be easily published and shared. The list can be sorted, filtered and direct linked by target release. The list itself for any one target release should be short or empty as only critically broken things should make the cut.

Microsoft could even consider charting the size of this list as a KPI for the history of successful feature releases and tune this over time.

The community is then considered to be on notice well in advance of breaking changes being released. Dev teams can communicate this list of issues internally, advising developers to avoid writing could that will be broken when broken features are fixed.

Communicating with developers about known broken feature has other time saving advantages because developers will know to steer clear of problems areas rather than waste time head scratching and repeating this discovery.

Dev teams can, at a convenient, unpressured time, search and review their code for effected usage and protect themselves from the breaking changes ahead of release.

Dev teams can create test cases for code that is using broken features ahead of rectification so that when the changes are first released in the targeted major release, tests can be quickly re-executed to confirm their code is not broken.

The short story is these changes seem to need to be done, we just need to coordinate a little better with some agile communication.
Posted by Martin.Robins on 2/19/2010 at 11:50 AM
Like those before me, I fail to see how fixing a broken API can itself be a breaking change. If anybody has gone to the trouble of using the API, they will already have seen that it does not work and will by now be using an alternative implementation of their own. If they have not noticed that it does not work, they clearly do not test properly.
The time has come to bite the bullet and accept that this needs to be fixed for once and for all.
Posted by androidi on 1/14/2010 at 2:59 PM
Should've been fixed back in 2005 when .NET wasn't very popular yet. The lesson here is, fix this type of dumb thing immediately and add A WARNING TO IDE OF BREAKING CHANGE... So it will warn anyone relying on the old broken logic. Then there could be a dialog to say "I have fixed/looked for this, stop warning me now".
Posted by PJ on Development on 1/3/2010 at 6:13 AM
I was browsing the Connect site and got a glimpse of this reported bug.

Digging a bit in the Framework I discovered that EVERY TypeConverter that does not override the bool IsValid(ITypeDescriptorContext, Object) function will always report IsValid as true.

The bug resides in the base TypeConverter class that has the following code:

public virtual bool IsValid(ITypeDescriptorContext context, object value)
{
    return true;
}

Hence, every single TypeDescriptor that doesn't override it, will fail the most basic tests.

Shame on you MS
Posted by Craccoon on 12/9/2009 at 10:40 AM
Clearly someone does not understand the nature of this bug… or want to sabotage BCL… :P

Shame on you, shame!
Posted by Andrey Shchekin on 12/2/2009 at 11:19 PM
I think breaking change can be solved by allowing opt-out through assembly-level attribute and *.config.
Posted by Carl Daniel on 10/26/2009 at 11:39 AM
NEarly four years after I first raised this concern (and others did too), I still agree with the common sentiment here: Backward compatibility with an API that has been horribly broken (to the point of being useless) since its inception is not a problem. Please - Break It!

The current implementation is useless and any code depending on it is proably already broken due to developer expectations that are not met by the current implementation.

At the least, deprecate this API and make a new one TypeConverter.IsReallyValid, as I and others have suggested multiple times over the past several years.
Posted by Daniel Smith on 10/13/2009 at 3:39 PM
Clearly, backwards compatibility is not an issue for people here. We'd rather have the bug fixed. Please don't make a mountain out of a mole hill.
Posted by straightener on 8/5/2009 at 7:44 AM
This defect required us to e-fix our production code today. It sure would be nice if you at least updated the documentation to reflect broken code, or mark it with a deprecated attribute or something.
Posted by Michael Maluck on 11/3/2008 at 5:59 AM
Can you please provide an example how the fix might break existing code? Please fix this issue.
Posted by DanielKzu on 5/20/2008 at 12:33 PM
After the overwhelming feedback, would you reconsider implementing this?
Posted by bluemaf on 8/24/2007 at 2:18 AM
I still participate the position of the other developers. Fixing a broken API is NOT a breaking change.
Posted by FullMetal on 11/6/2006 at 9:02 AM
So, make a new api: IsReallyReallySeriouslyValid OK?
Posted by Marc C Brooks on 7/25/2006 at 10:18 PM
Can you really still stand by this rediculous statement? Fixing a broken API is NOT a breaking change.
Posted by Microsoft on 11/30/2005 at 4:46 PM
We do not have the luxury of making that risky assumption that people will not be affected by the potential change. I know this can be frustrating for you as it is for us. Thanks for your understanding in this matter.
Posted by DanielKzu on 11/25/2005 at 2:37 PM
I fail to see how it can be a breaking change to leave a behavior that doesn't work and therefore nobody could be depending on.

That is, if someone is currently depending on the IsValid property returning always true, I don't see the point. If they are depending on it thinking that it's working as expected when it's actually not, I'd say they have a time-bomb waiting to explode.

In either case I fail to see how someone would oppose to have the right behavior in a method that is otherwise absolutely useless (for people not relying on converters for behavior, that is). It's clearly not a useless
method for me because in our project (the patterns & practices Guidance Automation Toolkit) we use extensively TypeConverters and UITypeEditors. For us this is a major showstopper, unless we reimplement converters for all primitve types and override them all the time. Not exactly what I'd love to do, but the only workaround for now (thankfully we do allow overriding the converter even of built-in types).

I urge you to reconsider, please. No one can be depending on the broken functionality in any sensible way (unless I'm missing something).

Thanks.
Posted by Microsoft on 11/21/2005 at 9:26 AM
Now that Visual Studio 2005 has shipped we're in the planning stages for our next release. As part of our normal planning process we are going through and re-evaluating all bugs that were postponed for Visual Studio 2005. We re-reviewed this bug as part of that process.

To ensure that applications developed with Visual Studio 2005 will continue to work well with our next release, we are refraining from making changes that would break compatibility with Visual Studio 2005. Based on our investigation, the most reasonable fix for this bug would break compatibility. This could cause existing applications or source to fail. Ideally, we would fix all customer bugs, but we believe that by fixing this bug would cause problems for more customers than we would help. Therefore, we are resolving it as “Won’t Fix”.

We very much value your comments and look forward to further feedback from you in the future.
Posted by Microsoft on 10/24/2005 at 6:14 PM
This issue has been reactivated as we begin planning for the next version of Visual Studio. Over the coming months we will reconsider feedback that was previously postponed. We welcome your comments and participation in this process.

-- Visual Studio Team
Posted by Microsoft on 4/12/2005 at 8:53 AM
Since TypeConverters are used so widely, changing the Int32Converter (and the other TypeConverters mentioned) to override IsValid will likely break applications, but it is far too destabilizing to try this late in Whidbey. We will investigate this for a future version.
Posted by DanielKzu on 4/1/2005 at 6:07 AM
Potential existing code relying on this to-be-broken-feature-if-you-fix-it could be:

if (intConverter.IsValid("blah"))
{
// As it **always** passes we assume it's a valid int?
int value = (int)intConverter.ConvertFrom("blah");
}

Of course, with the current implementation, there's no way that code actually works, and the conversion will always throw an exception. Hence, what would be broken?!?
Posted by DanielKzu on 4/1/2005 at 6:02 AM
I'm starting to worry about these "won't fix as it worked that (wrong) way before", as it's starting to appear on all bugs, no matter how evidently wrong a feature is implemented.

I completely understand that you're running out of time to fix these bugs. But at least you should postpone their resolution, and link them to the PS database you're using to track future features you expect for Orcas.

In this particular case, I'm not asking you to throw anything. The IsValid method on the converter should return a sensible value according to the input, that's all.

Now, do you think there's people around using the IsValid method when they KNOW the following won't work?:

if (intConverter.IsValid("blah"))
{
// **always** passes!!!
}

As there's no way such a code even exists, because it would cause runtime exceptions to whoever is using it, I hardly see it as a breaking change. Actually, it's a breaking change for something that was effectively broken and now starts working. I fail to see what's bad or wrong with that.
Posted by Microsoft on 3/29/2005 at 1:48 PM
Since this is the same behavior as previous versions, chaning the TypeConverters to throw the proper exception would be a breaking change.
Posted by Microsoft on 2/22/2005 at 11:18 PM
The Microsoft Sub-status is now "Reproduced"

Thanks for reporting this bug, we have been able to repro this issue and are investigating. Thank you, Prabhu, VS2005 Product Team
Sign in to post a workaround.
Posted by DanielKzu on 12/7/2005 at 7:33 AM
>Resolved as Won't Fix by Microsoft on 2005-11-30 at 16:46:06
>     
> We do not have the luxury of making that risky assumption that people will
> not be affected by the potential change. I know this can be frustrating for
> you as it is for us. Thanks for your understanding in this matter.

If the reasoning behind this is not breaking applications that do this:
if (!intConverter.IsValid("blah"))
{
     throw new BlahException("weird, this never happened before...");
}
you should *FIX* this bug.

The probability of existing applications doing this AND NOT USING the
"validated" value and throwing an ArgumentException inmediately after
is almost ludicrous, and I can't even imagine people complaining about
you fixing this since this is the *expected* behavior, this is what the
developer intended it to do when the code was written.




-
Posted by DanielKzu on 12/6/2005 at 8:34 AM
I understand your desire not to break existing code, but what this specific case addresses is a function that cannot possibly be in use in its current form. Anyone using STILL this method has not tested it, and thus is ignorant to the cause of the failures they are getting. If someone calls IsValid and it (falsely) returns true, the very next thing they are going to do is to call the Convert method, which will throw. This obsfucates the real error, and prevents code that could have handled the invlalid input.

Leaving this method in place is worse than deprecating it, but even that is better than the current patently WRONG behaviour. You could, if you insist on retaining this broken behaviour, expose a switch (like you did for the HTML compatible rendering).

Fix the method, Microsoft!

(p.s. Ladybug comments are broken, or that is where this would be)
© 2013 Microsoft Corporation. All rights reserved. Microsoft Connect Terms of Use | Trademarks | Privacy Statement