Null Ref and Missing using statement #171

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
2 participants
Contributor

CreepyGnome commented Dec 17, 2012

Null exception if value is null
Changed double if statements in ValidatesPresenceOf into an if/else statement, because if the value is null it would through a NullReferenceException when it called ToString on it.

In ValidateIsCurrency added a return after value == null check is true as we don't need to process the rest of the method and if you did and value was null he ToString call would throw a NullReferenceException.

Missing using statement
Using statement is missing, this should be disposing the reader when done. It is in sycn with the other Query method now as well.

Rodney Foley added some commits Dec 17, 2012

Null exception if value is null
Changed double if statements in ValidatesPresenceOf into an if/else
statement, because if the value is null it would through a
NullReferenceException when it called ToString on it.

In ValidateIsCurrency added a return after value == null check is true
as we don't need to process the rest of the method and if you did and
value was null he ToString call would throw a NullReferenceException.
Missing using statement
Using statement is missing, this should be desposing the reader when
done. It is in sycn with the other Query method now as well.
Contributor

CreepyGnome commented Dec 17, 2012

I broke these out from my other changes so they are only the bug fixes without the features. I believe this is one of the issues you had with my first pull request a while back is it was mixed above other things.

Contributor

robconery commented Dec 23, 2012

The Null exception is not readable as the DIFF is solid red/green. The missing using statement - I'm wondering why that's needed? The reader is in the scope of the connection, when the connection goes out of scope so does the reader - why do I need this?

Contributor

CreepyGnome commented Dec 23, 2012

I don't know why the Oracle and PostgreSQL they have the same changes from SQLite and the main Massive file in them is all. As GitHub's Window's and Mac app isn't having any issue showing the diffs clearly on all the files, so not sure why GitHub's Web Diff tool is smoking very odd.

If you don't want to the using statement you should call Close on the reader before it goes out of scope.

Basically if you don't call Close on the Reader nothing will. According to the MSDN doc's if you own the reader you must explicitly call Close when you are through with it. The close method has some clean up it does as described in the SqlDataReader.Close docs.

Maybe things you have been doing so far didn't rely on any of the work being done by the Close however it is reasonable to assume that its there for a reason and someone will rely on it, and it would be a hell of a issue to debug for them.

Contributor

CreepyGnome commented Jan 2, 2013

I separated out the null exceptions from the missing using statement for now and submit a pull request. That way you can accept those if you are still not sure about the missing using statement yet and need more time.

@CreepyGnome CreepyGnome closed this Jan 4, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment