Cfrgtkky

I had a discussion, if the code for calling information from a database can have a switch to show also deleted entries.

Simplyfied the code (C#) look like this:

void searchEntry(string searchValue, bool searchDelete = false)

Usually, the most part of the code base doesn't want to show up the deleted entries, but if you want to undelete an entry (and thats my only case) I want to get the deleted to look, if they should be undeleted.

I came to discussion that "boolean" should be avoided in this method,

Surely, I can make a enum like:

enum DeletionFlag {

   DontSearchDeleted,

   SearchDeleted

}

But makes this the code really more readble:

void searchEntry(string searchValue, DeletionFlag searchDelete = DeletionFlag.SearchDeleted)

The database table has a bool value for "isDeleted" to define, if the entry is marked as deleted.

So could use the kind of calls:

    db.searchEntry("somesearch", DeletionFlag.DontSearchDeleted);

    db.searchEntry("somesearch", DeletionFlag.SearchDeleted);

Would this be a better solution, or is it ok, to stay with boolean for this?

What are your suggestions?

My advice is to avoid optional parameters, especially when they are just switches. Instead, create two functions that clearly describe what they do:

void searchEntry(string searchValue)

void searchEntryIncludeDeleted(string searchValue)

That way, you avoid a boolean parameter, avoid the need for an enum, avoid optional parameters and make the code easier to read as a bonus.

To avoid code duplication, both methods then just call a private method that does the work. In the case of the private method, that boolean parameter becomes acceptable as the "distance" between the declaration and the caller is very small, thus avoiding the issues with it being non obvious what the boolean represents, eg:

public void searchEntry(string searchValue)

{

    searchEntries(searchValue, false);

}



public void searchEntryIncludedDeleted(string searchValue)

{

    searchEntries(searchValue, true);

}



private void searchEntry(string searchValue, bool includeDeleted)

{

    ...

}

And because it's private and only used by the two public wrapper methods, there is no need for an optional parameter.

The usual advice is to avoid many value type parameters and bool in particular as you can end up with arguably hard to read code

var x = searchEntry(

    true,

    false,

    "helloworld"

    )



var y = searchEntry(

    false, //super important that this is different!!

    false,

    "goodbye"

    )

But c# now has named arguments (sometimes incorrectly refered to as named parameters)

var y = searchEntry(

    includeDeleted: false,

    recursive: false,

    searchTerm: "goodbye"

    )

Which addresses the issue without having to create extra class/enums