Code needs some cleanup.

Question

1.00/5 (1 vote)

See more:

Hi, I am sure following code doesn't look professional so please give some suggestions to make it short.

C#

if (((DevExpress.Xpf.LayoutControl.GroupBox)sender).State == DevExpress.Xpf.LayoutControl.GroupBoxState.Maximized)
{
   Employee emp = (((DevExpress.Xpf.LayoutControl.GroupBox)sender).Content) as Employee;
   if (emp != null)
   {
      User user = emp.user;
      ((DevExpress.Xpf.LayoutControl.GroupBox)sender).Content = new Employee(user);
   }
   else
   {
      EmployeeMin empMin = (((DevExpress.Xpf.LayoutControl.GroupBox)sender).Content) as EmployeeMin;
      User user = empMin.user;
      ((DevExpress.Xpf.LayoutControl.GroupBox)sender).Content = new Employee(user);
   }
}
else
{
   Employee emp = (((DevExpress.Xpf.LayoutControl.GroupBox)sender).Content) as Employee;
   if (emp != null)
   {
      User user = emp.user;
      ((DevExpress.Xpf.LayoutControl.GroupBox)sender).Content = new EmployeeMin(user);
   }
   else
   {
      EmployeeMin empMin = (((DevExpress.Xpf.LayoutControl.GroupBox)sender).Content) as EmployeeMin;
      User user = empMin.user;
      ((DevExpress.Xpf.LayoutControl.GroupBox)sender).Content = new EmployeeMin(user);
   }
}

Posted 14-Jan-13 6:23am

Qadeer Ahmed Khan

Updated 14-Jan-13 7:36am

fjdiewornncalwe

v2

Add a Solution

Comments

Jibesh 14-Jan-13 12:29pm

You know its doesnt look good they why cant you re-arrange that by closely looking at the variables and its usages

2 solutions

Solution 2

As Adam R Harris you can use local variables to store data the code access more than once; in your code the variable most used is sender cast to DevExpress.Xpf.LayoutControl.GroupBox; the second one is its Content property.

The followin is a possible arrangement:

C#

var box = sender as DevExpress.Xpf.LayoutControl.GroupBox;
if (box != null)
{
    var oldContent = box.Content;
    /* the following would be better as ...
    ContentType newContent = null;
    */
    var newContent = oldContent;

    var emp = oldContent as Employee;
    if (emp == null)
        emp = oldContent as EmployeeMin;

    if (box.State == DevExpress.Xpf.LayoutControl.GroupBoxState.Maximized)
    {
        newContent = new Employee(emp.user);
    }
    else
    {
        newContent = new EmployeeMin(emp.user);
    }

    if (newContent != oldContent)
        box.Content = newContent;
}

I used var to reduce type name appearance. There is only one instruction where you should explicitly put the type name, but I don't know it so I can't do it for you; I had sorrounded it with a block comment with ContentType as the name of the type I don't know (maybe object, or Employee or a base type).

Regards,
Daniele.

Posted 14-Jan-13 7:00am

Daniele Rota Nodari

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Jibesh · Accepted Answer · 2013-01-14T06:33:00

my suggestions are

Keep this '((DevExpress.Xpf.LayoutControl.GroupBox)sender) in a location variable say mySender

replace all the occurrence of '((DevExpress.Xpf.LayoutControl.GroupBox)sender)' with mySender

if you want go further define another variable for mySender.Content as myContent and replace all the occurrence of '((DevExpress.Xpf.LayoutControl.GroupBox)sender).Content' with myContent

arh.. I just noticed the tag 'Optimization' if you are re-arranging this code block to improve the program optimization then you are wrong, your compiler already doing this for you.