C#•12mo 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

this_is_pain•12mo 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•12mo 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•12mo ago

i was about to do the exact same thing ^

stigzlerOP•12mo 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

this_is_pain•12mo 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•12mo ago

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

stigzlerOP•12mo 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);
}

this_is_pain•12mo 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

stigzlerOP•12mo 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

this_is_pain•12mo ago

then, you know, "keep it stupid simple"

stigzlerOP•12mo ago

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

Clément•12mo ago

From my perspective the second version is harder to read

stigzlerOP•12mo ago

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

Clément•12mo ago

I think the truth is somewhere between

stigzlerOP•12mo ago

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

Clément•12mo ago

:Ok:

this_is_pain•12mo 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

stigzlerOP•12mo ago

yeah - that's the original dev's code

bighugemassive3•12mo 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

Did you find this page helpful?