4

This question already has an answer here:

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?

marked as duplicate by gnat, amon, BobDalgleish, Community Dec 14 '18 at 17:36

This question has been asked before and already has an answer. If those answers do not fully address your question, please ask a new question.

  • 1
    I like it. So you can expand it in the future, how about making it an optional array of SearchFlags instead? Since SearchDeleted is the default behavior, it probably doesn't need its own flag. – bitsoflogic Dec 14 '18 at 15:16
  • Slightly off-topic: If something's deleted, you cannot search for it. And if something's searchable, it's definitely not deleted. I don't know for which data your database fakes deletion, but if it's anything like personalized data, you are very likely violating the GDPR with your database. – cmaster Dec 14 '18 at 17:09
  • 1
    Another thing to be aware of is that C# has named parameters, so you can also do searchEntry("somesearch", searchDelete: true); (handy if you have to deal with a 3rd party API that makes use of optional parameters the purpose of which may be unclear in a call). – Filip Milovanović Dec 14 '18 at 20:20
13

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.

  • 2
    +1, speaking method names are the way to go as long as you have few different options. If you have many orthogonal ways of calling a method, it's time to create a parameter class or even a method object. – Kilian Foth Dec 14 '18 at 15:25
  • 2
    @KilianFoth, good point. searchEntryIncludedDeletedInLastThreeDaysMatchingSuppliedPatternIgnoringCase might be taking things too far :) – David Arno Dec 14 '18 at 15:27
  • Thanks, that looks good. If this is not a new question: I want to avoid code doublication, since exclusion of "deleted" is additional query to get all elements. Would it make sense that "SearchEntry" calls "SearchEntryIncludeDeleted" and remove the deleted in its part only? – Christian Müller Dec 14 '18 at 15:31
  • @ChristianMüller Oh yes, absolutely! Otherwise you'd be repeating an awful lot of code - which would be much worse than an occasional boolean parameter. – Kilian Foth Dec 14 '18 at 15:34
  • 1
    This can become ugly fast imo, if you have several switches. In that case an options object is sometimes used or optional parameters if the language supports them – Alvaro Dec 14 '18 at 17:48
5

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

  • Thank you ewan. That's what also I thinking about :) But I was in discussion, that it would be not good "clean-code" practice to use it like this and change the code. So it would be nice, if you can give some arguments, why this maybe clean enough (as I think). – Christian Müller Dec 14 '18 at 15:34
  • 1
    hmm, well 'clean code' really comes down to what you find easy to understand. I would argue that in the named parameters case, we have nice long plain english names which make sense in combination with the method name and avoid the problem of many parameters making for an overly long method name. I dont think anyone would argue that it wasnt 'clean' the argument is whether a long method name is 'cleaner' – Ewan Dec 14 '18 at 15:43
  • Thank you ewan also for your answer. I like it, too. I go with David's solution, since it gives another direction I was thinking :) I wish I could approve both, so I had to decide.... – Christian Müller Dec 14 '18 at 15:55
  • omg this is just like "UKs Got Talent" all over again! – Ewan Dec 14 '18 at 16:02
  • Yes, but all could be glad, that I don't want to try singing :D – Christian Müller Dec 14 '18 at 16:07

Not the answer you're looking for? Browse other questions tagged or ask your own question.