proper programming? (events)

[giddy]

hi ,

ok , i have a programming background but i'm new to C# . i'm also self
taught so :

i have a datagridview that should act differently depending on which
user has signed in

now is it more effecient/correct if i do this:

if(user=="user1") {
this.dataGridView2.CellBeginEdit += new
System.Windows.Forms.DataGridViewCellCancelEventHandler(this.dataGridView2_CellBeginEdit_User1);
}else {
this.dataGridView2.CellBeginEdit += new
System.Windows.Forms.DataGridViewCellCancelEventHandler(this.dataGridView2_CellBeginEdit_User2);
}

as opposed to check for which user in ONE cellBegin event??

Thanks
Gideon

 

[Tom Shelton]

hi ,

ok , i have a programming background but i'm new to C# . i'm also self
taught so :

i have a datagridview that should act differently depending on which
user has signed in

now is it more effecient/correct if i do this:

if(user=="user1") {
this.dataGridView2.CellBeginEdit += new
System.Windows.Forms.DataGridViewCellCancelEventHandler(this.dataGridView2_CellBeginEdit_User1);
}else {
this.dataGridView2.CellBeginEdit += new
System.Windows.Forms.DataGridViewCellCancelEventHandler(this.dataGridView2_CellBeginEdit_User2);
}

as opposed to check for which user in ONE cellBegin event??

Thanks
Gideon

I'm not sure I like either method. Checking for the actual user would
seem to make things pretty hard to maintain. What kinds of things are
you looking to do different? There is almost certainly a better way to
accomplish what you want...

 

[Dave Sexton]

Hi Gideon,

I prefer to place that logic in the event handler, or if the logic is complex
and dynamic then a business object. i.e., if the logic in the event handler
may be configured for each user and changed at a later time based on database
properties, for example, then that complexity may warrant the use of a more
complex system as well to prevent code bloat in the event handler method and
to encapsulate the logic so it can be reused in other UI components and
systems.

Here's a simple BO model that you might want to adopt if you have "dynamic"
needs, such as the one I mentioned above:

this.dataGridView2.CellBeginEdit += dataGridView2_CellBeginEdit;

void dataGridView2_CellBeginEdit(object sender, SomeEventArgs e)
{
// you should ensure that your application assigns the User.Current
// property before it is queried here, or you can make the property a
// singleton field and initialize it automatically upon first request
using a
// type constructor (static).

User.Current.BeginCellEdit(sender, e);
}


class User
{
private static User current;
public static User Current { get { return current; } set { current=
value; } }

public UserUICapabilities Caps { get { return caps; } }

private readonly UserUICapabilities caps;
private readonly int id;

public User(int id)
{
this.id = id;
caps = new UserUICapabilities(this);
}

public void BeginCellEdit(object sender, SomeEventArgs e)
{
caps.BeginCellEdit(sender, e);
}
}

class UserUICapabilities
{
private readonly User user;
private bool initialized; // lazy initialization

public UserUICapabilities(User user)
{
this.user = user;
}

void EnsureInitialized()
{
if (initialized)
return;

// TODO: load user caps from database

initialized = true;
}

public void BeginCellEdit(object sender, SomeEventArgs e)
{
EnsureInitialized();

// Each User object with a distinct "id" will have an
// instance of a UserCapabilities object that can provide a
// different implementation here.

// TODO: begin cell edit based on the "user" field in this class
}
}


If the BeginCellEdit method needs more information about the UI elements that
it can manipulate, then you can create a data context class. An instance of
your data context class may be passed to BeginCellEdit instead of the sender
and event args:

class BeginCellEditContext
{
private DataGridView grid;
private DataGridView Grid { get { return grid; } }

private Label label;
private Label Label { get { return label; } }

public BeginCellEditContext(DataGridView grid, Label label)
{
this.grid = grid;
this.label = label;
}
}

That class may be the only argument to the BeginCellEdit method:

public void BeginCellEdit(BeginCellEditContext context) { ... }

The method may be called as such:

BeginCellEditContext context = new BeginCellEditContext(theGrid, theLabel);

User.Current.BeginCellEdit(context);


Another option is to use an MVP architecture, but that's going to add even
more complexity. You'll have to create command objects for every possible
task that the BeginCellEdit method may perform.

The simplest approach that meets all your needs will be the best one; in some
cases that is just a simple event handler routine, so choose wisely

 

[giddy]

oops. .i'm sorry , by user i meant user for my application , my app
mantains users wth passwords , i'm not talking about windows user
accounts

basically i did'nt want to make it confusing but , there are levels ...
each user is given some privilege and retricted in some way , eg .
some users might not be allowed to edit , but only allowed make new
entries , some users would be allowed do the opposite.. etc... .the
celBeginedit is'nt the only event i have to check for this...

i'm just asking if its CORRECT to give it a different delegate to
different funciton depending on what privelages the use has...

becuase checking for it in ONE event might get cumbersome and reduce
efficency??

also is Program.cs file , with "class program , and the main()" the
place to have complete global variables? that can be accessed by all
forms and dialogs??? (i'm an ex vb! , so i'm asking is program.cs like
a module file?)

Gideon

 

[Dave Sexton]

Hi Gideon,

basically i did'nt want to make it confusing but , there are levels ...
each user is given some privilege and retricted in some way , eg .
some users might not be allowed to edit , but only allowed make new
entries , some users would be allowed do the opposite.. etc... .the
celBeginedit is'nt the only event i have to check for this...

In that case, the code I posted in my original response should be useful to
use as the foundation for a User business object to encapsulate information
about a particular user, and a UserUICapabilities object that encapsulates a
User's permissions. You may even want to use roles that encapsulate groups of
permissions so that you can write code as such the following:

if (User.Current.IsAdministrator) // role-check
{
// do admin things such as allow edit AND decrypt related information
}
else if (User.Current.CanEdit) // permission-check for non-admins
{
// allow edit
}

i'm just asking if its CORRECT to give it a different delegate to
different funciton depending on what privelages the use has...

For the example you posted, I wouldn't recommend it at all. As I already
stated, I think that logic should be in the event handler (or better yet, be
abstracted into a business object).

becuase checking for it in ONE event might get cumbersome and reduce
efficency??

The same would be true in the method that checks the user name to determine
which delegate to be registered as an event handler.

Try to separate the UI logic from the business logic. That is, encapsulate
the business logic in separate components that can be utilized by the UI code.
Only provide one place to handle a UI event, and then perform the required
processing in the appropriate business object. If the process itself will
vary by state that is retrieved from a database, such as user permissions,
then this design approach will make it much easier for you to change things in
the future without having to change the business logic in all of the UI
classes in which it may have been hard-coded. This is especially true if you

if (user == "user1")

What happens if "user1" is renamed to "user_1", or deleted, or their
permissions are changed?

In that case, you'd have to recompile your code to handle the change in the
data. That's something that should by dynamic, and accounted for ahead of
time so that simple changes in data will not require the entire solution to be
modified and redeployed.

Business objects can be the first step to providing the abstraction you need
from your UI code to your business logic code so that you don't have to
hard-code user names, permissions and business logic in every event handler in
which it's required.

also is Program.cs file , with "class program , and the main()" the
place to have complete global variables? that can be accessed by all
forms and dialogs??? (i'm an ex vb! , so i'm asking is program.cs like
a module file?)

Try to use as little global variables as possible. Instead, try to think more
OOP.

That said, there are definitely times where a global variable might be of use
outside of any business object, but it's rare (if even appropriate at all).

But I do use the Program class in my projects to aggregate certain singleton
data such as the current user of the application or the current client context
in which all of the Forms in my application will process information across
the entire system, for example:

static class Program
{
public static User CurrentUser { get { return User.Current; } }
public static Client CurrentClient { get { return Client.Current; }
}

But realize that the Program class doesn't store the data in this case, it
just creates a common interface for accessing global data. The data itself is
stored in the appropriate business object. In some cases, a DataContext
object may be useful if you need to store multiple contexts, simultaneously.

I like using the Program class because it just feels natural to code against
data with application scope like this:

if (Program.CurrentUser.IsAdministrator)
{
if (Program.CurrentClient.InvoiceRequired)
{
Invoice invoice = Invoice.PrepareInvoice(Program.CurrentClient);
}
}

This is really just a matter of opinion. Some people, I'm sure, prefer to
access the User and Client objects directly for singleton data. I just feel
that User.Current isn't as clear as Program.CurrentUser.

 

[PS]

Hi Gideon,

I prefer to place that logic in the event handler, or if the logic is
complex and dynamic then a business object. i.e., if the logic in the
event handler may be configured for each user and changed at a later time
based on database properties, for example, then that complexity may
warrant the use of a more complex system as well to prevent code bloat in
the event handler method and to encapsulate the logic so it can be reused
in other UI components and systems.

Here's a simple BO model that you might want to adopt if you have
"dynamic" needs, such as the one I mentioned above:

this.dataGridView2.CellBeginEdit += dataGridView2_CellBeginEdit;

void dataGridView2_CellBeginEdit(object sender, SomeEventArgs e)
{
// you should ensure that your application assigns the User.Current
// property before it is queried here, or you can make the property a
// singleton field and initialize it automatically upon first request
using a
// type constructor (static).

User.Current.BeginCellEdit(sender, e);
}

You make some very good points Dave. When it comes to authorizations it is
also possible to have a heirarchy of view/add/edit/delete. I often split the
responsibilities between how the events are hooked up from what is handled
in the event handler. If a user's authorization is read only then I usually
do not hook up any events and make the control read only. The event handler
is used for when the user has some editing authorizations.

In some respects the OP's delegation to different event handlers at "load"
time is a halfway between using a single event handler that handles
different levels of authorizations, and using a separate business object
that delgates accordingly. He has just shifted the delegation decision to
"before" the event has occured.

PS

 

[Dave Sexton]

Hi,

You make some very good points Dave. When it comes to authorizations it is
also possible to have a heirarchy of view/add/edit/delete. I often split the
responsibilities between how the events are hooked up from what is handled
in the event handler. If a user's authorization is read only then I usually
do not hook up any events and make the control read only. The event handler
is used for when the user has some editing authorizations.

Agreed. It definitely makes sense to disable or even remove a control based
on the current user, but in some cases it's just not that simple. I assumed
that this is one of those not-so-simple cases since the OP didn't mention if
certain users would be restricted from editing all data or just certain cells.

In some respects the OP's delegation to different event handlers at "load"
time is a halfway between using a single event handler that handles
different levels of authorizations, and using a separate business object
that delgates accordingly. He has just shifted the delegation decision to
"before" the event has occured.

Yes, and I don't think that's a reasonable option.

The event is going to occur no matter what, so adding that logic to choose
which handler is invoked is not a clear separation of UI logic and business
logic unless another class handled the delegation for the UI component, but
that would be extra work with no apparent value. What do you think?

 

[PS]

Yes, and I don't think that's a reasonable option.

The event is going to occur no matter what, so adding that logic to choose
which handler is invoked is not a clear separation of UI logic and
business logic unless another class handled the delegation for the UI
component, but that would be extra work with no apparent value. What do
you think?

I am always in two minds over the "separation" aspect. On simple forms I
find that the code behind provides sufficient separation. I generally have a
setup region, a display region and an event handling region. There is very
little that can be shifted to another object because so much involves an
interaction between a UI object and a business object. e.g.

this.deleteEmployeeButton.Enabled = user.Role.IsAdminUser;

Moving this to a seperate object is "extra work with no apparent value". In
fact the new object ends up looking like your code behind file did. It is
just a facade that adds no value.

At a certain point a form goes beyond these simple UI object / business
object interactions. Decisions about UI state become more complicated. UI
controls need to react to the events raised by the use of other UI controls.
UI controls may be disabled if the user does not have authorization, if the
business object is read only, if a particular tab is not selected etc. This
is when I use a separate object. The code behind remains simple and the form
once again returns to simple UI control / object interaction, e.g.
this.deleteEmployeeButton.Enabled = myObject.EmployeeDeletionAllowed;

PS

 

[Dave Sexton]

Hi,

I am always in two minds over the "separation" aspect. On simple forms I
find that the code behind provides sufficient separation. I generally have a
setup region, a display region and an event handling region. There is very
little that can be shifted to another object because so much involves an
interaction between a UI object and a business object. e.g.

this.deleteEmployeeButton.Enabled = user.Role.IsAdminUser;

Moving this to a seperate object is "extra work with no apparent value". In
fact the new object ends up looking like your code behind file did. It is
just a facade that adds no value.

In this simple case, I would agree. The standard MVC architecture used in
WinForms is common and simple to understand.

Here, you're performing an action that could easily be done using the standard
property-binding mechanism. Once binding is no longer a possibility, I
believe that complexity may warrant abstraction.

At a certain point a form goes beyond these simple UI object / business
object interactions. Decisions about UI state become more complicated. UI
controls need to react to the events raised by the use of other UI controls.
UI controls may be disabled if the user does not have authorization, if the
business object is read only, if a particular tab is not selected etc. This
is when I use a separate object. The code behind remains simple and the form
once again returns to simple UI control / object interaction, e.g.
this.deleteEmployeeButton.Enabled = myObject.EmployeeDeletionAllowed;

I agree, but in this example you still only need to set an Enabled property.
That could be done using simple property-binding.

My example allowed for complex business logic on the UI components through a
"capabilities" class. It is not an appropriate design if the OP only needs
the simplicity in your examples, but I tried to make that clear in my original
response. Since the OP didn't state the requirements in detail, I assumed
that they may be complex enough where giving an answer such as, "put it in the
event handler" might have been too basic.

I think we agree here, though, that the standard MVC architecture in WinForms
may still be appropriate in certain circumstances.

Do we also agree that coding the logic to choose an appropriate event handler
instead of having a single event handler, in the OP's particular case, is a
bad idea? (that was actually my original question to you

 

[PS]

Hi,



In this simple case, I would agree. The standard MVC architecture used in
WinForms is common and simple to understand.

Here, you're performing an action that could easily be done using the
standard property-binding mechanism. Once binding is no longer a
possibility, I believe that complexity may warrant abstraction.

A good guideline to use.

I agree, but in this example you still only need to set an Enabled
property. That could be done using simple property-binding.

Yes, but the logic required to work out if the button should be enabled is
something that should be encapsulated outside of the code behind in it's own
object. I guess I use the Mediator pattern.

"Define an object that encapsulates how a set of objects interact. Mediator
promotes loose coupling by keeping objects from referring to each other
explicitly, and it lets you vary their interaction independently"

My example allowed for complex business logic on the UI components through
a "capabilities" class. It is not an appropriate design if the OP only
needs the simplicity in your examples, but I tried to make that clear in
my original response. Since the OP didn't state the requirements in
detail, I assumed that they may be complex enough where giving an answer
such as, "put it in the event handler" might have been too basic.

I think we agree here, though, that the standard MVC architecture in
WinForms may still be appropriate in certain circumstances.

Do we also agree that coding the logic to choose an appropriate event
handler instead of having a single event handler, in the OP's particular
case, is a bad idea? (that was actually my original question to you

I would have to agree with you on that. If the OP wants to use separate
methods then he should call them from the single event handler.

Good discussion,

Regards,

PS

 

[Dave Sexton]

Hi,

Yes, but the logic required to work out if the button should be enabled is
something that should be encapsulated outside of the code behind in it's own
object. I guess I use the Mediator pattern.

"Define an object that encapsulates how a set of objects interact. Mediator
promotes loose coupling by keeping objects from referring to each other
explicitly, and it lets you vary their interaction independently"

Yes, we're on the same page here as well.

The mediator pattern is interesting. Here's some more information I found on
the subject in the context of an article on the Message Broker Pattern [link
after sig]:

"Mediator [Gamma95]. The Mediator pattern separates objects so that they are
only aware of the mediator but not each other. The Broker deals with similar
concerns, but it can only be used in the context of enterprise applications."

So your example does seem like the mediator pattern. And if the actual type
of "myObject" was the "UserUICapabilities" class from my original response,
that would make even more sense. It would just need a property named,
"EmployeeDeletionAllowed". Since it was created by a User object, it would
act as the mediator between the User and the UI.

<snip>

Thanks for the discussion.

 

[giddy]

Hi Dave ,

i didnt see your post before posting my second reply! i hit the reply
button , went to have lunch! and wrote my post only in response to Toms
post! lol and i did'nt have time to replay back! sorry....

yes , your approach makes whole lot of sense , i did'nt even give a
thought to making seperate classes! infact i've decided to seperate a
lot more .. .thanks

although the complexity at which i'm working is not much. Each user has
about 5 levels of privileges..... basically i thought it would be more
effecient to register an event with the right delegate in the first
place(when the form loads) so i would'nt have to go checking INSIDE the
function (when the event fires) , for what privilege the user has!
anyway i think the object method is more effecient! and a lot more
cleaner... .

thanks again!

Gideon