Why does this code work?

  • Thread starter Thread starter Julie
  • Start date Start date
J

Julie

I'm supporting an application at work. Below are some code segments
that I can't understand how they work. First let me say, I would never
code this standard. I'm just really creeped out that it works.

Here's the setup. I have a function, called GetEmployeeCertifications,
that is going to make a call to a SQL DB and return a result set. This
module calls another function, called FillParameters, to determine if
SQL parameters need to be appended to the command object. The
FillParameters function...

does not have a return value
the command object is not globally defined
and the command object is passed by value not reference

But yet the command object, in the GetEmployeeCertifications function,
has appended to it the parameter objects from the FillParameters
function. How can that be?

Public Shared Function GetEmployeeCertifications(ByVal criteria As
SearchCriteria) As EmployeeCertificationCollection
Dim tblEmployees As New DataTable
Dim connection As New SqlConnection(connectionString)
Dim cmdGetEmployees As New SqlCommand
Dim myDataAdapter As New SqlDataAdapter
Dim employees As New EmployeeCertificationCollection

Try
connection.Open()
cmdGetEmployees.Connection = connection
cmdGetEmployees.CommandText = "GetEmployeeCertifications"
cmdGetEmployees.CommandType = CommandType.StoredProcedure

FillParameters(criteria, cmdGetEmployees) '<---Here is the
call with no assignment
myDataAdapter.SelectCommand = cmdGetEmployees
myDataAdapter.Fill(tblEmployees)

Catch ex As Exception

Finally
'convert the matching data rows to an
EmployeeCertificationCollection
If Not tblEmployees Is Nothing Then
Dim row As DataRow
For Each row In tblEmployees.Rows
employees.Add(GetEmployeeCertification(row))
Next
End If
End Try

'If they searched by cost center, then return the results
sorted by cost center.
'The results come back from the database ordered by last name.
If criteria.CostCenterFrom.Length > 0 Or
criteria.CostCenterTo.Length > 0 Then
employees.ApplySort("CostCenter",
ComponentModel.ListSortDirection.Ascending)
End If
Return employees

End Function

Private Shared Function FillParameters(ByVal criteria As
SearchCriteria, ByVal cmd As SqlCommand)
If criteria.LastName.Length > 0 Then
cmd.Parameters.Add(New SqlParameter("@LastName",
SqlEscape(criteria.LastName)))
End If

If Not (criteria.SiteCode = "ALL" Or criteria.SiteCode.Length =
0) Then
cmd.Parameters.Add(New SqlParameter("@SiteCode",
criteria.SiteCode))
End If

End Function
 
Julie said:
I'm supporting an application at work. Below are some code segments
that I can't understand how they work. First let me say, I would never
code this standard. I'm just really creeped out that it works.

Here's the setup. I have a function, called GetEmployeeCertifications,
that is going to make a call to a SQL DB and return a result set. This
module calls another function, called FillParameters, to determine if
SQL parameters need to be appended to the command object. The
FillParameters function...

does not have a return value
the command object is not globally defined
and the command object is passed by value not reference
Click to expand...

You're confused about parameter passing, basically. See
http://www.pobox.com/~skeet/csharp/parameters.html

It's written in terms of C#, but the principle is the same - even when
a parameter is passed by value, if it's a reference type (such as
SqlCommand) it's the *reference* which is passed. Changes to the object
made via that reference are still visible to the caller after the
method has terminated.
 
FillParameters(criteria, cmdGetEmployees) '<---Here is the
call with no assignment
Click to expand...

This is a call by reference. A Command object is by reference always.

So the parameters are filled using the reference.

Jesse
 
Jon,

Thanks for the refresher on reference types vs value types. The
confusion comes in on the variable definition in the function name with
the explicit ByVal keyword.

A variable can be assigned a reference. A second variable can be
assigned the value of first. "They are still, however, independent
variables themselves. Changing the value of first will not change the
value of second". But change the value of the object using the first
variable will change the second.

StringBuilder first = new StringBuilder();
StringBuilder second = first;
first.Append ("hello");
first = null;
Console.WriteLine (second);

Hello will still print because second is a variable whose address
points to an area in core that contains hello. While first has
released its address.

HERE IS MY DISCONNECT

The variable cmd is defined as a SQL command type object. But the
ByValue keyword threw me as we are passing the pointer not creating a
duplicate object. It is possible to create and object in memory that
has the same contents as another object in memory. Thus cmd and
cmdGetEmployees are two different variables pointing to the same
object.

So in the FillParameters function what would have happened if I had
ByVal with a new keyword?

Private Shared Function FillParameters(ByVal criteria As
SearchCriteria, ByVal cmd As new SqlCommand)

Thanks for the whack upside the head.
J
 
So in the FillParameters function what would have happened if I had
ByVal with a new keyword?

Private Shared Function FillParameters(ByVal criteria As
SearchCriteria, ByVal cmd As new SqlCommand)
Click to expand...

I'm afraid I don't know enough VB.NET to understand that syntax - it
doesn't seem to make sense judging by the documentation.
 
Jon,
No, the object isn't passed at all. There's a different between passing
an object by reference and passing a reference by value. See
Click to expand...

That was in my idea not written by Jesse. I think that we are blind as soon
as we read "reference".

A reference type is always passing in any way an address as the reference
not the complete object.

:-)

Cor
 
Julie,

Do you know why this methode declaration bellow does not give an error. You
write this as a Sub, which should work, but better is it to avoid this in
this kind of situations where you are changing values inside a Sub.

Private Shared Function FillParameters(ByVal criteria As
SearchCriteria, ByVal cmd As SqlCommand)

Try to avoid this, try to return what has to be returned, that makes your
program at least more readable.

Private Shared Function FillParameters(ByVal criteria As
SearchCriteria, ByVal cmd As SqlCommand) as SqlCommand
....
return cmd

And than in your code
cmdGetEmloyees = FillParameters(criteria, cmdGetEmployees)

However the strangest in your code is for me that function whithout return
value.

I hope this helps,
 
Cor Ligthert said:
That was in my idea not written by Jesse. I think that we are blind as soon
as we read "reference".

A reference type is always passing in any way an address as the reference
not the complete object.
Click to expand...

Yes, but Jesse claimed that the object was passed by reference. It
isn't. It isn't passed at all. There are well-defined meaning to "pass
by reference", and ByVal semantics aren't the same, even when what is
passed is a reference.
 
Cor Ligthert said:
Do you know why this methode declaration bellow does not give an error. You
write this as a Sub, which should work, but better is it to avoid this in
this kind of situations where you are changing values inside a Sub.

Private Shared Function FillParameters(ByVal criteria As
SearchCriteria, ByVal cmd As SqlCommand)

Try to avoid this, try to return what has to be returned, that makes your
program at least more readable.

Private Shared Function FillParameters(ByVal criteria As
SearchCriteria, ByVal cmd As SqlCommand) as SqlCommand
...
return cmd

And than in your code
cmdGetEmloyees = FillParameters(criteria, cmdGetEmployees)

However the strangest in your code is for me that function whithout return
value.
Click to expand...

Why is that strange? To me, it would be very odd to return the command
when there's no need for it. Why have a return value when it's
unnecessary? *That* would make the code less readable IMO.

Rather than pretending that the return value is important, it's better
to understand reference type semantics. What would you do when the
method modified the objects referred to by two of its parameters? You
can't return them both, so you get back to the same problem of
understanding.
 
Jon,

Did you try this ever in C: to call a function

Void(Skeet skeet){}
in VB as I have this right almost the same as
Function(byval skeet as Skeet)

Maybe this goes in C# but it is no VB code
(As many things in VB it goes with Option Strict Off but that I have in my
opions already set on On)

In VB you use for the void methods SUB

(There are voices to change that and make a return with an empty return a
Sub)
But there is very much nostaligy in VB. By instance I don't see any reason
to use the Dim.
In 20% of the syntax it is told already by the "As" keyword.
(For a as integer etc)

To return more parameters,
Use a class object

What would you do if this class is a seperate DLL how do you decribe your
intelisence.
Something as "your values are changed in the DLL".

Cor
 
Cor Ligthert [MVP] wrote:

<snip>

It seems that you have two concerns:

1) The contents of the parameter being modified.
2) A function being used instead of a sub.

The second point is reasonable, but the best approach is to turn the
function into a sub, rather than to add a pointless return value. Now,
for the first point:
To return more parameters,
Use a class object
Click to expand...

So you'd start encapsulating every permutation of types which change,
just for the sake of returning them *when they don't even need to be
returned in the first place*?

Moreover, you're then confusing things, because it *looks* like you
won't actually change the objects which are referred to by the
parameters. Suppose I did this:

filledCommand = FillParameters (unfilledCommand)

That looks like unfilledCommand won't be changed, even though it will!
What would you do if this class is a seperate DLL how do you decribe your
intelisence.
Something as "your values are changed in the DLL".
Click to expand...

Well, they're not changed "in the DLL" but the values of the object are
indeed changed, wherever the type has been loaded from. Look at the
documentation for DataAdapter.Fill for a perfect example of this. You
pass in a reference to a DataSet, and that DataSet gets filled. The
number of rows added or refreshed happens to be returned, but that's
pretty much incidental. Why is that any different to filling a Command
object?

Jon
 
Jon,


Look at the
documentation for DataAdapter.Fill for a perfect example of this. You
pass in a reference to a DataSet, and that DataSet gets filled. The
number of rows added or refreshed happens to be returned, but that's
pretty much incidental. Why is that any different to filling a Command
object?
Click to expand...
Exactly.

While with filling is meant in this black box adding and not returning the
asked resultset as one of the misunderstanding of this black box, but it
keeps the newsgroups and forums filled.

:-)

Cor
 
Jon/Cor

Thank you both for your input. Remember I did write this code, but I
do have to support it. And the one thing I LOVE about this industry is
your always learning.

I believe at this point, the thread has become more about programming
style. Should the Function be a Subroutine? Yes, IMO it should be
because it doesn't return a value but does affect the area of core
being pointed to by two different variables. Having been an Assembler
programmer (and yes I did do C/C++/Java a lifetime ago), I can
appreciate not duplicating stored values and using reference. But
there were two other programmers at the site that were confused by the
code (one of them has is roots in C#.Net and is trying to learn VB.Net)

In the technical design and construction of any application there is
the bread vs. bullets syndrome, except its call Maintainability verses
Efficiency. Is it more readable to have a return type even if the
values are modified by reference? Yes. Again, IMO it is more readable
and helps the other developers visual see the flow. I have used
reference before, so I should have figured this out without the slap
upside the head =).

If there is one thing I want to take away from this is "it doesn't take
a genius to know everything, just know where to find the right
answers". And if you do get an answer, learn from it. If you still
don't understand the answer, look at the question again.

Thank you both. I love this group.

J
 
Cor said:
Look at the
Exactly.

While with filling is meant in this black box adding and not returning the
asked resultset as one of the misunderstanding of this black box, but it
keeps the newsgroups and forums filled.
Click to expand...

Whereas pretending that you're going to only return a different object
while keeping the original one pristine, but actually modifying it
in-place to keep things efficient would be better, would it?

It's fair enough that people don't always understand parameter passing
semantics, but they're good semantics which aren't that hard to explain
when people *do* misunderstand them, and which allow much more
efficient designs than would otherwise be possible. Not using those
semantics is flying in the face of the philosophy of .NET.

Jon
 
Julie said:
In the technical design and construction of any application there is
the bread vs. bullets syndrome, except its call Maintainability verses
Efficiency. Is it more readable to have a return type even if the
values are modified by reference? Yes. Again, IMO it is more readable
and helps the other developers visual see the flow. I have used
reference before, so I should have figured this out without the slap
upside the head =).
Click to expand...

I still find it hard to accept that it's more readable. If a method
returns something, I will assume there's a reason for it returning it -
that it gives me more information than I had before. If a method is
returning an object which is of the same type as one of the parameters,
I may make an assumption that it's returning a reference to a *new*
object, and that the "old" has remained untouched. That wouldn't be the
case here. At this point, you've effectively misled the caller - which
is exactly what readable code is meant to avoid.

The idiom of modifying parameters is pretty common, and I don't think
we should try to avoid people having to learn it by pretending we're
doing something we're not.

Jon
 
Jon,

What do I always write about maintainability and you about effeciency?

That efficiency is probably here however even a piece of not to measurable
time, even if you do it a million time. As you wrote yourself, you are only
passing a reference.

I am with those who find this not so a nice philosopyy of .Net.

But I thought that I once read that it is the toy of the the architect of
that.

Cor
 
Cor Ligthert said:
What do I always write about maintainability and you about effeciency?
Click to expand...

I think you must have missed the many, many times I've written about
maintainability and readability. I am interested in efficiency in
coarse terms (like not copying objects unnecessarily) but
maintainability is key. I think your suggestion *reduces*
maintainability rather than increasing it though.
That efficiency is probably here however even a piece of not to measurable
time, even if you do it a million time. As you wrote yourself, you are only
passing a reference.
Click to expand...

But in order to make the method do what you *imply* it does (i.e. leave
the original object alone) you need to clone the object. If you do
that, it's inefficient. If you don't do that, your method is
effectively lying to the reader, which is reducing the readbility.
I am with those who find this not so a nice philosopyy of .Net.
Click to expand...

What, reference type semantics? Can you find any articles on the web
suggesting that .NET shouldn't have reference types, or be able to
modify objects whose references are passed as parameters?
But I thought that I once read that it is the toy of the the architect of
that.
Click to expand...

Not sure what you mean by that.
 
Jon,

What it takes extra is one time a set of the addressvalue (reference) to the
address (stack) that it already contains. I does not take the time to paint
the result of that, when you do that a billion times, on the screen.

You can call this inefficiency. I thinkt that consistent use makes programs
it better to understand.

I have used that black box method myself until I forgot what I was doing in
a method.

It is for me a very classic way of programming as done in Cobol, probably
very usefull for programmers who still think in that way and won't give up
those "handy" things.

I think it should be prohibited that it is possible to use. It is for me the
same use as Cobol that had indexes (pointers) that holds the references too
adresses, good used it was ideal but you had to know what you was doing and
almost not to explain. (I have used this extremely often by the way).

I have read on Internet that I am not the only one who does not like it, but
as I have understand those articles well, seems it to be a hobby of somebody
to keep this Cobol like behaviour in Net.

However as forever, feel free to disagree with me.

Just my idea,

Cor



..
 
Cor Ligthert said:
What it takes extra is one time a set of the addressvalue (reference) to the
address (stack) that it already contains. I does not take the time to paint
the result of that, when you do that a billion times, on the screen.
Click to expand...

I think you've completely missed my point.

If you think people aren't going to understand that when you pass a
reference in, the object referred to might change, then you ought to
honour that assumption. That means that if you *do* want to effectively
change the object (and return it), you need to create a new object with
the same data as the old object. *That* is much more than copying a
reference.

If, however, you don't do that, and you just modify the object in-
place, then it's confusing to the person who assumes that the object
*won't* be modified.
You can call this inefficiency. I thinkt that consistent use makes programs
it better to understand.
Click to expand...

No - you either introduce a significant penalty by working against the
point of the framework, or you introduce *inconsistency* by implying an
invariant that you don't uphold.
I have used that black box method myself until I forgot what I was doing in
a method.
Click to expand...

That's just a case of poor documentation then. The description of a
method should certainly specify if it's going to change the contents of
an object which has been provided using a reference. Given an
appropriate description, there's no reason for it to be confusing at
all.
It is for me a very classic way of programming as done in Cobol, probably
very usefull for programmers who still think in that way and won't give up
those "handy" things.
Click to expand...

I haven't used Cobol, but it sounds like it was using value type
semantics. That's okay, but you need to understand the efficiency
penalties of doing that - and acknowledge that it's *not* the .NET way
of doing things. Why fight against what the framework promotes?
I think it should be prohibited that it is possible to use. It is for me the
same use as Cobol that had indexes (pointers) that holds the references too
adresses, good used it was ideal but you had to know what you was doing and
almost not to explain. (I have used this extremely often by the way).
Click to expand...

Just because you've used it doesn't make it a good idea.
I have read on Internet that I am not the only one who does not like it, but
as I have understand those articles well, seems it to be a hobby of somebody
to keep this Cobol like behaviour in Net.
Click to expand...
However as forever, feel free to disagree with me.
Click to expand...

I'm afraid you just don't seem to be seeing my point at all. What
you're proposing is confusing in the extreme - it implies that you
don't make changes to the object whose reference is passed in. Either
you need to make sure that you honour that, in which case you have the
penalty of copying data, or you don't, in which case you create
confusing code because someone using that implication will be confused
if the object *does* change.
 
Back
Top