PC Review


Reply
Thread Tools Rating: Thread Rating: 1 votes, 5.00 average.

best way to handle null objects

 
 
mp
Guest
Posts: n/a
 
      15th Jan 2011
this is a method in a class to provide access to Excel
where _xlApp etc are private member vars for the respective objects
they shouldn't ever be null by the time this is called...but just in case...

public Range GetRange(string rangename)
{
Range rng=null ;
if (_xlApp != null)
{if (_xlWorkBook != null)
{if(_xlWorkSheet!=null)
{
rng = (Range)_xlWorkSheet.get_Range(rangename,
System.Reflection.Missing.Value);
_xlRange = rng;
}
}
}
return rng;
}

would it be advised to provide an else for each if
if (_xlApp != null)
{}
else
{throw exception or ???}
that would make the layered if clauses messy

or better to
if (_xlApp == null){...throw excep and bail}
if (_xlWorkBook == null){...throw excep and bail}
if (_xlWorkSheet == null){...throw excep and bail}
....aok so get the range...

thanks
mark


 
Reply With Quote
 
 
 
 
Arne Vajh°j
Guest
Posts: n/a
 
      15th Jan 2011
On 14-01-2011 19:30, mp wrote:
> this is a method in a class to provide access to Excel
> where _xlApp etc are private member vars for the respective objects
> they shouldn't ever be null by the time this is called...but just in case...
>
> public Range GetRange(string rangename)
> {
> Range rng=null ;
> if (_xlApp != null)
> {if (_xlWorkBook != null)
> {if(_xlWorkSheet!=null)
> {
> rng = (Range)_xlWorkSheet.get_Range(rangename,
> System.Reflection.Missing.Value);
> _xlRange = rng;
> }
> }
> }
> return rng;
> }
>
> would it be advised to provide an else for each if
> if (_xlApp != null)
> {}
> else
> {throw exception or ???}
> that would make the layered if clauses messy
>
> or better to
> if (_xlApp == null){...throw excep and bail}
> if (_xlWorkBook == null){...throw excep and bail}
> if (_xlWorkSheet == null){...throw excep and bail}
> ...aok so get the range...


If _xlApp, _xlWorkBook and _xlWorkSheet are initialized in the
constructor then drop the test. If someone later screwup the
initialization, then that will result in a NullReferenceException.
But they could also mess up things 717 other ways that you
can not protect against, so there are really no point in
trying.

If they may not have been set then you should test. I
suggest to code it like:

public Range GetRange(string rangename)
{
if (_xlApp == null || _xlWorkBook == null || _xlWorkSheet == null)
{
throw new FubarException();
}
Range rng = (Range)_xlWorkSheet.get_Range(rangename,
System.Reflection.Missing.Value);
_xlRange = rng;
return rng;
}

to keep the exception logic and the main logic better separated.

Arne
 
Reply With Quote
 
 
 
 
Arne Vajh°j
Guest
Posts: n/a
 
      15th Jan 2011
On 14-01-2011 20:14, Arne Vajh°j wrote:
> On 14-01-2011 19:30, mp wrote:
>> this is a method in a class to provide access to Excel
>> where _xlApp etc are private member vars for the respective objects
>> they shouldn't ever be null by the time this is called...but just in
>> case...
>>
>> public Range GetRange(string rangename)
>> {
>> Range rng=null ;
>> if (_xlApp != null)
>> {if (_xlWorkBook != null)
>> {if(_xlWorkSheet!=null)
>> {
>> rng = (Range)_xlWorkSheet.get_Range(rangename,
>> System.Reflection.Missing.Value);
>> _xlRange = rng;
>> }
>> }
>> }
>> return rng;
>> }
>>
>> would it be advised to provide an else for each if
>> if (_xlApp != null)
>> {}
>> else
>> {throw exception or ???}
>> that would make the layered if clauses messy
>>
>> or better to
>> if (_xlApp == null){...throw excep and bail}
>> if (_xlWorkBook == null){...throw excep and bail}
>> if (_xlWorkSheet == null){...throw excep and bail}
>> ...aok so get the range...

>
> If _xlApp, _xlWorkBook and _xlWorkSheet are initialized in the
> constructor then drop the test. If someone later screwup the
> initialization, then that will result in a NullReferenceException.
> But they could also mess up things 717 other ways that you
> can not protect against, so there are really no point in
> trying.
>
> If they may not have been set then you should test. I
> suggest to code it like:
>
> public Range GetRange(string rangename)
> {
> if (_xlApp == null || _xlWorkBook == null || _xlWorkSheet == null)
> {
> throw new FubarException();
> }
> Range rng = (Range)_xlWorkSheet.get_Range(rangename,
> System.Reflection.Missing.Value);
> _xlRange = rng;
> return rng;
> }
>
> to keep the exception logic and the main logic better separated.


And I don't like the assignment to _xlRange!

I assume this is saved for some other method that will
be fubar if this method is not called first.

Arne
 
Reply With Quote
 
mp
Guest
Posts: n/a
 
      15th Jan 2011

"Arne Vajh°j" <(E-Mail Removed)> wrote in message
news:4d30f53c$0$23762$(E-Mail Removed)...
> On 14-01-2011 20:14, Arne Vajh°j wrote:
>> On 14-01-2011 19:30, mp wrote:
>>> this is a method in a class to provide access to Excel

[...]

thanks for the input


> And I don't like the assignment to _xlRange!
>
> I assume this is saved for some other method that will
> be fubar if this method is not called first.
>
> Arne


yes, i save it so later calls dont' have to pass in a range object
thinking this simplifys my calling code
but i also returned it in case client wanted to do other things
that weren't built in using the "default" range object
just beginning to design all this so i was leaving my options open
maybe not good
mark


 
Reply With Quote
 
Arne Vajh°j
Guest
Posts: n/a
 
      15th Jan 2011
On 14-01-2011 20:26, mp wrote:
> "Arne Vajh°j"<(E-Mail Removed)> wrote in message
> news:4d30f53c$0$23762$(E-Mail Removed)...
>> And I don't like the assignment to _xlRange!
>>
>> I assume this is saved for some other method that will
>> be fubar if this method is not called first.

>
> yes, i save it so later calls dont' have to pass in a range object
> thinking this simplifys my calling code
> but i also returned it in case client wanted to do other things
> that weren't built in using the "default" range object
> just beginning to design all this so i was leaving my options open
> maybe not good


Passing the argument is much better.

More readable, less dependency between method calls,
not thread unsafe etc..

Arne

 
Reply With Quote
 
Registered User
Guest
Posts: n/a
 
      15th Jan 2011
On Fri, 14 Jan 2011 18:30:45 -0600, "mp" <(E-Mail Removed)> wrote:

>this is a method in a class to provide access to Excel
>where _xlApp etc are private member vars for the respective objects
>they shouldn't ever be null by the time this is called...but just in case...
>
>public Range GetRange(string rangename)
> {
> Range rng=null ;
> if (_xlApp != null)
> {if (_xlWorkBook != null)
> {if(_xlWorkSheet!=null)
> {
> rng = (Range)_xlWorkSheet.get_Range(rangename,
>System.Reflection.Missing.Value);
> _xlRange = rng;
> }
> }
> }
> return rng;
> }
>
>would it be advised to provide an else for each if
>if (_xlApp != null)
>{}
>else
>{throw exception or ???}
>that would make the layered if clauses messy
>

The code is already pretty messy

>or better to
>if (_xlApp == null){...throw excep and bail}
>if (_xlWorkBook == null){...throw excep and bail}
>if (_xlWorkSheet == null){...throw excep and bail}
>...aok so get the range...
>

Are all those nested if statements really needed? It appears the only
value which needs to be checked for null is the _xlWorkSheet member.
I'm guessing if that member isn't null then neither are the _xlApp and
_xlWorkBook members.

public Range GetRange(string rangename)
{
Range rng=null ;
// validate argument as well
if(_xlWorkSheet!=null && !String.IsNullOrEmpty(rangename))
{
rng = (Range)_xlWorkSheet.get_Range(rangename,
System.Reflection.Missing.Value);
_xlRange = rng;
}
return rng;
}

Another consideration should be the need to raise an exception. I
would suggest evaluating the possibility of simply returning null. The
caller can check the return value before proceeding.

regards
A.G.
 
Reply With Quote
 
mp
Guest
Posts: n/a
 
      15th Jan 2011

"Registered User" <(E-Mail Removed)> wrote in message
news:(E-Mail Removed)...
> On Fri, 14 Jan 2011 18:30:45 -0600, "mp" <(E-Mail Removed)> wrote:
>
>>this is a method in a class to provide access to Excel

[]
>> if (_xlApp != null)
>> {if (_xlWorkBook != null)
>> {if(_xlWorkSheet!=null)
>> {
>> rng = (Range)_xlWorkSheet.get_Range(rangename,

[]


>>

> Are all those nested if statements really needed? It appears the only
> value which needs to be checked for null is the _xlWorkSheet member.
> I'm guessing if that member isn't null then neither are the _xlApp and
> _xlWorkBook members.
>


good point. at this point i was debug.printing a statement for each
condidtion
just for my own use as i work with coding this project, but in reality what
you say makes sense...

[]

>
> Another consideration should be the need to raise an exception. I
> would suggest evaluating the possibility of simply returning null. The
> caller can check the return value before proceeding.
>
> regards
> A.G.


I originally was doing that, returning null, then (due to my inexperience)
I thought maybe that was tacky, as it made the caller have to do extra
checks???
But on further thought, the *only* way they could be null is if the caller
forgets
to "request" a workbook and worksheet before trying to get a range obect.
so i'm not really foisting extra work on the caller, just letting it know it
has
shirked its' duty somewhere upstream.

thanks for the input
mark


 
Reply With Quote
 
Registered User
Guest
Posts: n/a
 
      15th Jan 2011
On Sat, 15 Jan 2011 08:46:29 -0600, "mp" <(E-Mail Removed)> wrote:

>
>"Registered User" <(E-Mail Removed)> wrote in message
>news:(E-Mail Removed)...
>> On Fri, 14 Jan 2011 18:30:45 -0600, "mp" <(E-Mail Removed)> wrote:
>>
>>>this is a method in a class to provide access to Excel

>[]
>>> if (_xlApp != null)
>>> {if (_xlWorkBook != null)
>>> {if(_xlWorkSheet!=null)
>>> {
>>> rng = (Range)_xlWorkSheet.get_Range(rangename,

>[]
>
>
>>>

>> Are all those nested if statements really needed? It appears the only
>> value which needs to be checked for null is the _xlWorkSheet member.
>> I'm guessing if that member isn't null then neither are the _xlApp and
>> _xlWorkBook members.
>>

>
>good point. at this point i was debug.printing a statement for each
>condidtion
>just for my own use as i work with coding this project, but in reality what
>you say makes sense...
>
>[]
>
>>
>> Another consideration should be the need to raise an exception. I
>> would suggest evaluating the possibility of simply returning null. The
>> caller can check the return value before proceeding.
>>
>> regards
>> A.G.

>
>I originally was doing that, returning null, then (due to my inexperience)
>I thought maybe that was tacky, as it made the caller have to do extra
>checks???
>

There are many situations where null can be a valid return value. It
is seldom a bad idea for the caller validate a return value if the
return type is nullable.

One thing to be aware is an exception will be thrown if get_Range is
passed a bad argument e.g.
Range aRange = ws.get_Range("AG", "C7");
The happens because COM interop translates all error HRESULTS in to
COMExceptions. This can be avoided by using a regular expression to
validate the argument(s).

>But on further thought, the *only* way they could be null is if the caller
>forgets
>to "request" a workbook and worksheet before trying to get a range obect.
>so i'm not really foisting extra work on the caller, just letting it know it
>has
>shirked its' duty somewhere upstream.
>

I think this Excel wrapper type is a good learning exercise. I let you
discover the lessons as you go

>thanks for the input
>

You're most certainly welcome

regards
A.G.
 
Reply With Quote
 
mp
Guest
Posts: n/a
 
      15th Jan 2011

"Registered User" <(E-Mail Removed)> wrote in message
news:(E-Mail Removed)...
> On Sat, 15 Jan 2011 08:46:29 -0600, "mp" <(E-Mail Removed)> wrote:
>
>>
>>"Registered User" <(E-Mail Removed)> wrote in message
>>news:(E-Mail Removed)...
>>> On Fri, 14 Jan 2011 18:30:45 -0600, "mp" <(E-Mail Removed)> wrote:
>>>
>>>>this is a method in a class to provide access to Excel

>>[]


> One thing to be aware is an exception will be thrown if get_Range is
> passed a bad argument e.g.
> Range aRange = ws.get_Range("AG", "C7");
> The happens because COM interop translates all error HRESULTS in to
> COMExceptions. This can be avoided by using a regular expression to
> validate the argument(s).


oh no! <g> not regex again!
:-(
I'm so dumb when it comes to regex
(oh well, not *Just* regex <g>)

so i have to validate everything between "A1" up to "IV65536"
and everything in between
argghh
:-)
thanks for the suggestion though...i have way too much hair still left...
haven't pulled it *all* out yet
mark


[]
>>

> You're most certainly welcome
>
> regards
> A.G.



 
Reply With Quote
 
mp
Guest
Posts: n/a
 
      15th Jan 2011

"Peter Duniho" <(E-Mail Removed)> wrote in message
news:(E-Mail Removed)...
> On 1/15/11 12:07 PM, mp wrote:
>> [...]
>> so i have to validate everything between "A1" up to "IV65536"
>> and everything in between

>
> No, you don't have to do that. Let Excel do the validation. Yes, you'll
> get an exception. But that's by design.
>
> If the user is providing the range address, then be prepared to catch the
> exception and report it to the user. If the range address always coming
> from your own program, then just make sure you don't generate invalid
> addresses. If you do, you'll get an exception and your program will
> crash. Again, this is a _good_ thing. You don't want your program to
> keep running if it's not behaving correctly. You need to fix it so it
> does.
>
> Pete


much easier!
Thanks
mark
:-)


 
Reply With Quote
 
 
 
Reply

Thread Tools
Rate This Thread
Rate This Thread:

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
What is the best way to handle path to different objects in the database. Tony Johansson Microsoft C# .NET 1 4th Jan 2011 01:35 AM
How best to handle mouse events for custom drawn objects? =?Utf-8?B?TXJOb2JvZHk=?= Microsoft C# .NET 1 5th Sep 2006 05:21 PM
Objects, objects, so many objects! ;-) thechaosengine Microsoft Dot NET 11 18th Dec 2004 01:35 PM
"right" way to handle null date values? Dean Slindee Microsoft VB .NET 6 25th Nov 2003 02:11 AM
Null result when combining null field with non-null field in ADP View Lauren Quantrell Microsoft Access Form Coding 8 17th Nov 2003 03:34 AM


Features
 

Advertising
 

Newsgroups
 


All times are GMT +1. The time now is 05:11 PM.