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

Omnissiah•8mo 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•8mo 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•8mo ago

i was about to do the exact same thing ^

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

Omnissiah•8mo 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•8mo ago

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

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

Omnissiah•8mo 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•8mo 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

Omnissiah•8mo ago

then, you know, "keep it stupid simple"

stigzlerOP•8mo ago

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

Clément•8mo ago

From my perspective the second version is harder to read

stigzlerOP•8mo ago

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

Clément•8mo ago

I think the truth is somewhere between

stigzlerOP•8mo ago

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

Clément•8mo ago

:Ok:

Omnissiah•8mo 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•8mo ago

yeah - that's the original dev's code

br4kejet•8mo 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