Re: Complexity - C# Discussion Boards

16-Jul-15 22:37

Ive create a method that will handle keydown event

it has a lot of "if" statment and it say i has a high complexity and its scores 33

is there way to decrease its complexity
actually the method is not finish

here is the code

private static void FlyouCommandEvent(object sender, KeyEventArgs e)
{
    var fd = (FlyoutDialog)sender;
    if ((e.KeyCode == Keys.A || (e.Alt && e.KeyCode == Keys.A)) && fd.CanExecuteCommand(Helpers.FlyoutCommandAbort))
    {
        fd.ExecuteCommand(Helpers.FlyoutCommandAbort);
    }
    else if (e.KeyCode == Keys.Escape && fd.CanExecuteCommand(Helpers.FlyoutCommandCancel))
    {
        fd.ExecuteCommand(Helpers.FlyoutCommandCancel);
    }
    else if ((e.KeyCode == Keys.I || (e.Alt && e.KeyCode == Keys.I)) && fd.CanExecuteCommand(Helpers.FlyoutCommandIgnore))
    {
        fd.ExecuteCommand(Helpers.FlyoutCommandIgnore);
    }
    else if ((e.KeyCode == Keys.N || (e.Alt && e.KeyCode == Keys.N)) && fd.CanExecuteCommand(Helpers.FlyoutCommandNo))
    {
        fd.ExecuteCommand(Helpers.FlyoutCommandNo);
    }
    else if ((e.KeyCode == Keys.N || (e.Alt && e.KeyCode == Keys.N)) && fd.CanExecuteCommand(Helpers.FlyoutCommandNone))
    {
        fd.ExecuteCommand(Helpers.FlyoutCommandNone);
    }
    else if (e.KeyCode == Keys.Enter && fd.CanExecuteCommand(Helpers.FlyoutCommandOK))
    {
        fd.ExecuteCommand(Helpers.FlyoutCommandOK);
    }
    else if ((e.KeyCode == Keys.R || (e.Alt && e.KeyCode == Keys.R)) && fd.CanExecuteCommand(Helpers.FlyoutCommandRetry))
    {
        fd.ExecuteCommand(Helpers.FlyoutCommandRetry);
    }
    else if ((e.KeyCode == Keys.Y || (e.Alt && e.KeyCode == Keys.Y)) && fd.CanExecuteCommand(Helpers.FlyoutCommandYes))
    {
        fd.ExecuteCommand(Helpers.FlyoutCommandYes);
    }
}

any suggestion is highly appreciated
thank you

modified 17-Jul-15 4:47am.

Godhaniketan17-Jul-15 0:07

Godhaniketan

17-Jul-15 0:07

there is switch....catch option is availabel as alternate option of if condition

Gilbert Consellado17-Jul-15 2:15

17-Jul-15 2:15

Thank you so much, but i guess switch cant handle expressions. Correct if I am wrong.

Pete O'Hanlon17-Jul-15 1:07

Pete O'Hanlon

17-Jul-15 1:07

There's a heck of a lot of code going on here, and it certainly appears as though you're violating SRP here. Rather than doing things like this, I would look to implement separate classes that do what you need to do - each class would then decide whether it was the one that handled the case. There's even a name for this, as a pattern; it's called the Chain Of Responsibility. So, you would end up with something like this:

public abstract class HandlerBase
{
  private IHandler successor;
  public void SetSuccessor(IHandler successor)
  {
    this.successor = handler;
  }
  public virtual void HandleRequest(KeyEventArgs e)
  {
    if (CanHandleRequest(e))
    {
      return;
    }
    if (successor != null)
    {
      successor.HandleRequest(e);
    }
  }

  public abstract bool CanHandleRequest(KeyEventArgs e);
  public abstract void HandleRequest(FlyoutDialog fd);
}

public class FlyoutAbort : HandlerBase
{
  public virtual bool CanHandleRequest(KeyEventArgs e)
  {
    return (e.KeyCode == Keys.A || (e.Alt && e.KeyCode == Keys.A)) && fd.CanExecuteCommand(Helpers.FlyoutCommandAbort);
  }

  public abstract void HandleRequest(FlyoutDialog fd)
  {
    fd.ExecuteCommand(Helpers.FlyoutCommandAbort); // In reality, I would look to move the command from Helpers, into this class.
  }
}

public class FlyoutCancel : HandlerBase
{
  public virtual bool CanHandleRequest(KeyEventArgs e)
  {
    return e.KeyCode == Keys.Escape && fd.CanExecuteCommand(Helpers.FlyoutCommandCancel;
  }

  public abstract void HandleRequest(FlyoutDialog fd)
  {
    fd.ExecuteCommand(Helpers.FlyoutCommandCancel);
  }
}

Then, in your main class, you simply do this:

private FlyoutAbort flyoutAbort;
private FlyoutCancel flyoutCancel;

private void InitializeChain()
{
  flyoutAbort = new FlyoutAbort();
  flyoutCancel = new FlyoutCancel();
  flyoutAbort.SetSuccessor(flyoutCancel);
}

Finally, all you need to do to handle the request (if it can be handled that is), is call:

flyoutAbort.HandleRequest(e);

Gilbert Consellado17-Jul-15 2:17

17-Jul-15 2:17

Nice one, let see what I can do. Thank you so much.

Gilbert Consellado17-Jul-15 5:18

17-Jul-15 5:18

Its all works now. Thank you so much.

Alan N17-Jul-15 4:37

Alan N

17-Jul-15 4:37

Off topic with regards to your question but is your key testing code correct?

I assume the intention of the following code is to allow A or Alt-A

if ((e.KeyCode == Keys.A || (e.Alt && e.KeyCode == Keys.A))

The first condition does not test for modifier keys so would be true for A with any combination of modifiers keys (including none). Due to shortcut evaluation the second condition will be evaluated only if the first is false, that is if the Key is NOT A and I'm sure you can see the problem with that. Even if it was evaluated the condition does not exclude additional modifiers so Ctrl-Alt-A would be allowed.

The solution is to test the key and the modifiers separately. Specifically the modifiers test should be for none or just Alt.

if ((e.KeyCode == Keys.A) && (e.Modifiers == Keys.None || e.Modifiers == Keys.Alt))

Hope that helps, Alan.

Gilbert Consellado17-Jul-15 5:22

Pass Anonymous Type To Worker

17-Jul-15 5:22

Oh I didn't realize that. Thank you so much

Kevin Marois16-Jul-15 12:58

Kevin Marois

16-Jul-15 12:58

I have this:

List<long> checkedIds = Sensors.Where(x => x.IsChecked).Select(x => x.ID).ToList();

var filterArgs = new 
{
    Ids = checkedIds,
    DateRangeStart = FilterRangeStart,
    DateRangeEnd = FilterRangeEnd
};

myWorker.RunWorkerAsync(filterArgs);

How do I get the values in the myWorker_DoWork?

private void myWorker_DoWork(object sender, DoWorkEventArgs e)
{
  // e is type object.. How do I get my anonymous type values?
}

If it's not broken, fix it until it is

Re: Pass Anonymous Type To Worker

Dave Kreskowiak16-Jul-15 14:49

Dave Kreskowiak

16-Jul-15 14:49

I really hate using anonymous types because they just make your code harder to debug and understand. I strongly suggest replacing the anonymous type with an actual class. I see no reason why you need to use a anonymous type here.

Anonymous types are good for use inside the method where it's defined, not really for passing around to other methods.

The reason being is that you have to define your anonymous type all over again just so you can cast the value back to that type. That's doubling your work and introducing another opportunity to introduce bugs into your code. This is especially true in this case because if you change the type definition, you have to do it in multiple places. YUK!

A guide to posting questions on CodeProject

Click this: Asking questions is a skill.
Seriously, do it.

Dave Kreskowiak

Re: Pass Anonymous Type To Worker

Kevin Marois16-Jul-15 14:52

Kevin Marois

16-Jul-15 14:52

You're right

If it's not broken, fix it until it is

Re: Pass Anonymous Type To Worker

F-ES Sitecore16-Jul-15 21:45

F-ES Sitecore

16-Jul-15 21:45

Whilst I agree you should really use a concrete class for anything that leaves the context in which it was created, to answer the question you could use this

private void myWorker_DoWork(object sender, DoWorkEventArgs e)
{
    dynamic arg = (dynamic) e.Argument;
    Console.WriteLine(arg.DateRangeStart);
}

form 1 and form 2

jamesmc153516-Jul-15 4:08

Transferring information between two forms, Part 1: Parent to Child[^]

16-Jul-15 4:08

hi i want to have text(textbox8) show on form 1 (frmLogin)
shown on form2(files) textbox (textbox1)
my code: textbox1.Text = textbox8.text ,,, error,,,,name(textbox8) login form does not exist in current context

Re: form 1 and form 2

Wes Aday16-Jul-15 4:26

Wes Aday

16-Jul-15 4:26

You can lead a developer to CodeProject, but you can't make them think.
The Theory of Gravity was invented for the sole purpose of distracting you from investigating the scientific fact that the Earth sucks.

i get a error invalid expresion

jamesmc153516-Jul-15 2:57

16-Jul-15 2:57

okay this is my code "if (string.IsNullOrEmpty(txtUser.Text)) || (string.IsNullOrEmpty(txtPassword.Text)" i get a invalid expresion on( "||" ) but i have used it before and it worked

Richard MacCutchan16-Jul-15 3:02

16-Jul-15 3:02

Should be:

if (string.IsNullOrEmpty(txtUser.Text) || string.IsNullOrEmpty(txtPassword.Text))

Use the IDE to check where your matching parentheses are, or should be.

jamesmc153516-Jul-15 3:09

16-Jul-15 3:09

if (string.IsNullOrEmpty(txtPassword.Text) || (string.IsNullOrEmpty(txtUser.Text)))
MessageBox.Show("field empty.", "Error", MessageBoxButtons.OK, MessageBoxIcon.Warning);
Files mf = new Files();
Hide();
mf.ShowDialog();
Close();

}
*theres my code ,its practicly a button that opens another form,and if the fields are empty popup error but when i click ok , the other form still opens, whats the addon code, (== DialogResult.Ok)??

Richard MacCutchan16-Jul-15 4:54

16-Jul-15 4:54

Yes, you should check the result of the MessageBox.Show message, and perhaps put a better message, and extra button, so the user can continue or cancel. Alternatively, if this error is unrecoverable, you should just exit aftger the message is dismissed by the user.

jamesmc153516-Jul-15 4:56

16-Jul-15 4:56

i fixed it my fault was i put the ) in but shudve put it iin the end, how do i close it im new here Poke tongue | ;-P

Richard MacCutchan16-Jul-15 4:58

16-Jul-15 4:58

jamesmc1535 wrote:
how do i close it

Close what?

jamesmc153516-Jul-15 5:00

16-Jul-15 5:00

well not close , u said if its marked as dismissed or something like that i should exit did you mean the program $Shucks | :-\$

Richard MacCutchan16-Jul-15 5:06

16-Jul-15 5:06

I don't know, it all depends on what the application is doing at the time, and what it is supposed to do in these circumstances. You may want to close the main form and then open some other form for further work, or you may just want to close the program.

help with login

jamesmc153516-Jul-15 2:49

16-Jul-15 2:49

okay so this is my problem , i have a button on a form and 2 textboxes,and what i want to do is "create a admin account" where i type into the text boxes "example" username :admin password :admin " and then have it moved on to the next form and if the password/username is incorrect have a message popping up. the messagebox isnt a problem, i just dont know about the username,and will i be able to do it without sql (temporary)?

Re: help with login

Richard MacCutchan16-Jul-15 2:59