C#•3mo ago

Refactor this if statement without an additional method

Sorry folks. I'm a bit codeblind atm. What's the neatest way to do the below without another Method (which would contain the repeated code). This feels like a code smell atm:

KryptonWorkspaceCell? cell = SpaceControl?.ActiveCell;
if (cell == null)
{
    // ...create a new cell and place at the end of the root collection
    cell = new KryptonWorkspaceCell();
    SpaceControl!.Root.Children!.Add(cell);
}
else if (cell.Pages.Count() == 0)
{
    // first remove the empty cell
    SpaceControl!.Root.Children!.Remove(cell);
    // now create a new cell and place at the end of the root collection
    cell = new KryptonWorkspaceCell();
    SpaceControl!.Root.Children!.Add(cell);
}
// cell is then used from hereon in

KryptonWorkspaceCell? cell = SpaceControl?.ActiveCell;
if (cell == null)
{
    // ...create a new cell and place at the end of the root collection
    cell = new KryptonWorkspaceCell();
    SpaceControl!.Root.Children!.Add(cell);
}
else if (cell.Pages.Count() == 0)
{
    // first remove the empty cell
    SpaceControl!.Root.Children!.Remove(cell);
    // now create a new cell and place at the end of the root collection
    cell = new KryptonWorkspaceCell();
    SpaceControl!.Root.Children!.Add(cell);
}
// cell is then used from hereon in

19 Replies

WEIRD FLEX•3mo ago

the only thing i can think of right now -- not tested at all

var anyPages = cell?.Pages.Any();
if (anyPages != true) {
  if (anyPages == false)
    SpaceControl!.Root.Children!.Remove(cell);
  cell = new KryptonWorkspaceCell();
  SpaceControl!.Root.Children!.Add(
}

var anyPages = cell?.Pages.Any();
if (anyPages != true) {
  if (anyPages == false)
    SpaceControl!.Root.Children!.Remove(cell);
  cell = new KryptonWorkspaceCell();
  SpaceControl!.Root.Children!.Add(
}

Clément•3mo ago

if (cell is not null)
{
    return; // I guess ?
}

if (cell?.Pages.Count == 0)
{
    SpaceControl!.Root.Children!.Remove(cell);
}

// ...create a new cell and place at the end of the root collection
cell = new KryptonWorkspaceCell();
SpaceControl!.Root.Children!.Add(cell);

if (cell is not null)
{
    return; // I guess ?
}

if (cell?.Pages.Count == 0)
{
    SpaceControl!.Root.Children!.Remove(cell);
}

// ...create a new cell and place at the end of the root collection
cell = new KryptonWorkspaceCell();
SpaceControl!.Root.Children!.Add(cell);

Jimmacle•3mo ago

i was about to do the exact same thing ^

stigzler•3mo ago

ah dammit, sorry folks, forgot to add the cell initiator. Updated OP. Think that kyboshes Clement's Yep - thought of something similar, but wanted to avoid the nested if

WEIRD FLEX•3mo ago

there's more stuff that you could do, maybe, i don't think i would make code more difficult to understand than that

Clément•3mo ago

The original could be better, but at least is readable and is clear on what it's doing

stigzler•3mo ago

There's this, but it still looks smelly:

if (cell == null || cell.Pages.Count() == 0)
{
    if (cell?.Pages.Count == 0) SpaceControl!.Root.Children!.Remove(cell);

    // ...create a new cell and place at the end of the root collection
    cell = new KryptonWorkspaceCell();
    SpaceControl!.Root.Children!.Add(cell);
}

if (cell == null || cell.Pages.Count() == 0)
{
    if (cell?.Pages.Count == 0) SpaceControl!.Root.Children!.Remove(cell);

    // ...create a new cell and place at the end of the root collection
    cell = new KryptonWorkspaceCell();
    SpaceControl!.Root.Children!.Add(cell);
}

WEIRD FLEX•3mo ago

exactly if you make an extension you could ortographically avoid the if, like cell?.RemoveSpaceControlChildren(); or something but at that point i would probably rethink the whole thing instead of doing that

stigzler•3mo ago

Yeah. Problem is, it's someone else's code - working on a github fork bug shoot Don't really know the etiquette for that

WEIRD FLEX•3mo ago

then, you know, "keep it stupid simple"

stigzler•3mo ago

Think I'll stick with my second version - less repetative

Clément•3mo ago

From my perspective the second version is harder to read

stigzler•3mo ago

hmm - which is conventionally the bigger sin? More repetition or less readable?

Clément•3mo ago

I think the truth is somewhere between

stigzler•3mo ago

😆 Well, I'll post it with the alt version and see what the devs think

Clément•3mo ago

:Ok:

WEIRD FLEX•3mo ago

i feel like this SpaceControl!.Root.Children!.Remove(cell); shouldn't go 4 levels deep to do his thing that's why to me it looks like it could have its own class with a specific method to do this

stigzler•3mo ago

yeah - that's the original dev's code

br4kejet•3mo ago

This is one way I find myself doing a lot:

KryptonWorkspaceCell? cell = SpaceControl?.ActiveCell;
bool hasZeroPages = false;
if (cell == null || (hasZeroPages = cell.Pages.Count() == 0))
{
    if (hasZeroPages)
        SpaceControl!.Root.Children!.Remove(cell)

    cell = new KryptonWorkspaceCell();
    SpaceControl!.Root.Children!.Add(cell);
}

KryptonWorkspaceCell? cell = SpaceControl?.ActiveCell;
bool hasZeroPages = false;
if (cell == null || (hasZeroPages = cell.Pages.Count() == 0))
{
    if (hasZeroPages)
        SpaceControl!.Root.Children!.Remove(cell)

    cell = new KryptonWorkspaceCell();
    SpaceControl!.Root.Children!.Add(cell);
}

Gaming

Programming

Refactor this if statement without an additional method