views:

362

answers:

3

This seems like it might be a bad idea, but I can't figure out why:

I have a class, cXYZ, with properties A, B and C. It also has a method 'sGetData' that loads those three properties from the database, and a method 'sSaveData' which saves it back.

class cXYZ

  public property A as string...
  public property B as string...
  public property B as string..

  public sub sGetData()...
  public sub sSaveData()...

end class

A webform has the following property:

private property xyz() as cXYZ
get
    return session("myXYZ")
end get
set (value as cXYZ)
    session("myXYZ")=value
end set
end property

And the following events:

Protected Sub Page_Load(ByVal sender As Object, ByVal e As System.EventArgs) Handles Me.Load

if not ispostback() then
    xyz=new cXYZ()
end if

end sub

Protected Sub ButtonLoad_Click(ByVal sender As Object, ByVal e As System.EventArgs) Handles ButtonLoad.Click

   //Can now reference the class
  txtA.text=xyz.A
  txtB.text=xyz.B
  txtC.text=xyz.C

end sub


 Protected Sub ButtonSave_Click(ByVal sender As Object, ByVal e As System.EventArgs) Handles ButtonSave.Click

  //Can now reference the class
  xyz.A=txtA.text
  xyz.B=txtA.text
  xyz.C=txtC.text

  xyz.sSaveData()

end sub

I can see some overhead with serializing/deserializing for each property reference- it might be worth doing this:

 Protected Sub ButtonSave_Click(ByVal sender As Object, ByVal e As System.EventArgs) Handles ButtonSave.Click

  dim localxyz as cXYZ=xyz

  localxyz .A=txtA.text
  localxyz .B=txtA.text
  localxyz .C=txtC.text

  xyz=localxyz

end sub

Other than that, views on why this is good or bad? The class is not large, it maintains the form state. Webforms suck, etc is not very useful..

A: 

I think its ok. I would addicionaly keep a variable to store the object to improve performance.

Something like this:

private _xyz as cXYZ = nothing 
private property xyz() as cXYZ 
   get
      if not _xyz is nothing then 
         return _xyz
      else
         return session("myXYZ")
      end if 
   end get 
   set (value as cXYZ)
      _xyz = value
      session("myXYZ")=value 
   end set
end property
Sergio
A: 

I would improve that little bit:

 private _xyz as cXYZ = nothing 

 private property xyz() as cXYZ 
   get
      if _xyz is nothing then _xyz = TryCast(session("myXYZ"), cXYZ)
      return _xyz
   end get 
   set (value as cXYZ)
      _xyz = value
      session("myXYZ")=_xyz 
   end set
end property
Tomas Kirda
Why the trycast option if the local variable is nothing? The only reason I can think of is to automatically pick up the session copy of the object if it had not been instantiated in the event model. (which may be valid- say on a second webform that picks up data from the first)
ScottK
You may use DirectCast to increase performance, but if variable is nothing DirectCast or TryCast will cast it successfully, unless datatype is not reference type.
Tomas Kirda
A: 

As long as your object is serializable you are OK.

Just don't hold any unmanaged object references in your session - otherwise you'll get into trouble once you move away from 'in process' session state to a web farm.

chris