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

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

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

"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

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

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.

"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

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.

"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.

"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
:-)