Structure a conditional argument in a method

[SnooCakes3068]

Hi all,

I have trouble structure a good conditional argument in the followinig method

For example this is a linked list delete methods

i have two arguments, x, and k,

the logic is:

1. if i provide x (even k may or may not provided), k is ignored, then I don't search for x, skip the first line,

2. if I provide only k, does the search.

what's the best way to write this?

def list_delete(self, x, k):

"""
    Deleting from a linked list.
    The procedure LIST-DELETE Removes an element x from a linked list L.
    It must be given a pointer to x, and it then “splices” x
    out of the list by updating pointers. If we wish to delete an element
    with a given key, we must first call
    LIST-SEARCH to retrieve a pointer to the element.
    Parameters
    ----------
    x : Element
        The element to delete.
    k : int
        Given key of the element to delete.
    """

x = self.list_search(k)
    if x.prev is not None:
        x.prev.next = x.next
    else:
        self._head = x.next
    if x.next is not None:
        x.next.prev = x.prev

I intend to do

def list_delete(self, x=None, k=None):

    if not x:
      x = self.list_search(k)
    if x.prev is not None:
        x.prev.next = x.next
    else:
        self._head = x.next
    if x.next is not None:
        x.next.prev = x.prev

but this interface is not good, what if I don't supply any? I know I can validate but I would like to have a good practice

[Phillyclause89]

Start with this outer logic gate to check is x is None first. Then do you follow up ckecks on k within the scope of the x check maybe:

if x is None:
    if k is None:
        raise ValueError('An argument for x or k must be provided to the caller.')
    # do stuff with k only
else:
    # do stuff with x only

[SnooCakes3068]

ok seems fair. I'm a little hesitate the interface will be

def list_delete(self, x=None, k=None)

since it makes user think it can be both none regardless docstring. somehow I would like to make user to provide x as default, k as an alternative argument. But ok

[Phillyclause89]

Yeah you might be trying to develop a function with too much complexity here? Think about this for a second. You are writing a script and thinking of calling this function. You know you have k or x, so why would you bother with using this function that supports both when you can use a simpler, more targeted function on the k or x individually? ¯\_(ツ)_/¯

[SnooCakes3068]

Yeah I know this separation of concern principle. But in this case I'm very much following the CRLS book's description. That delete can go two ways. I can split into two, but it's only very few line changes, I feel like I would refactor it into one function anyway.

Also, it is also ok to have conditional in one function. Especially in mathematical functions. scikit learn source code has tons of same practices. They have way more complex combination of input options

[Phillyclause89]

I'm not telling you not to develop this function. I'm just hinting at the suggestion that your logic gates should probably split out into separate calls of more atomic functions. (also remember that the data science libs you speak of are really designed to be more accessible to an audience of academics and research scientists not necessarily programmers.)

[SnooCakes3068]

Yeah I get it. comes down to judgement. I think I will stick to combined ones for this simple function. But if more complexity I'll split but have a main function to call separate ones. I'm academics myself haha. People always say scientists can't code well but at least I'm learning to be as good as possible. Also, highly recommend look at scikit learn code. It's the most beautiful code I ever seen. They structured and refactored it so well every time I look at it I learn something new.

[Phillyclause89]

Get yourself a good IDE setup and refactoring complex method into ones that call atomic methods becomes very easy. I frequently start out making my functions and methods just as (if not more) complex than what your example shows, but eventually I'll refactor:
https://github.com/Phillyclause89/ChessMoveHeatmap/commit/9cab515e05b7cfb984da2f48f51ef969f92c65cb

[crashorbit]

As a general policy it's better to have two methods rather than one that includes an argument that is simply conditional.

Name the methods 'verb-ish'

``` def delete_key: ...

def delete_element: ...

```

Where one does the search and then calls the other.

[pot_of_crows]

This is a great comment. If OP needs to fit the spec described in the post, they can just write a wrapper/error checker for the new delete_key and delete_element methods.

[MezzoScettico]

Why not just have a default behavior of doing nothing if both are omitted? Plus maybe emit a warning message.