How do I properly write these functions?

Question

So far my weakest side has been proper code structuring and organization. I wanted to know how I can properly apply separation of concerns to the following code to make it more organized.

I have a function deleteCurrentList().

It does two things: deletes the currently selected list and navigates back to the previous page.

function deleteCurrentList() {
     deleteList(current_list_id);
     navigate("..");
}

From my understanding, this is bad code because

A: It does more than one thing.
B: Its name doesn't describe fully what it does.

So one idea that I had in mind is to rename this function to deleteCurrentListAndNavBack(), but that is a long name, and it still doesn't solve the issue of the function doing more than one thing.

So I wanted to know, what should I do in this situation?

How do I split these two actions while still being able to delete the list and navigate back in the same sequence?

score 1 · Answer 1 · answered May 23 '22 at 06:48

1

The long name at least tells you what the function does. So that is an improvement.

Now let me guess that with the current list deleted you can’t stay on the same page and you have to navigate somewhere.

So maybe call the function “closeCurrentPage” whose single responsibility it is to close the current page plus of course take all actions necessary for it. And necessary actions are deleting related data (but the caller doesn’t need to know it’s a list), and navigating from the page that is closed to some other page.

answered May 23 '22 at 06:48

gnasher729

42,090
4
59
119

The function is called when the "delete list" button is clicked, so it doesn't make sense to name it `closeCurrentPage`, as its main purpose is to delete the list. – Good guy May 23 '22 at 07:11
@Goodguy But does the "delete list" button essentially close the page? The name of the function does not necessarily have to match what the user sees on a button. – JMekker May 25 '22 at 16:07
@JMekker But the function's main purpose is to delete the list, not to close the page. You could omit the action of closing the page, and the function would still fulfill its purpose. – Good guy May 27 '22 at 05:33

score 1 · Accepted Answer · answered May 23 '22 at 06:49

If the deleteCurrentList is bound on any event then I recommend renaming it to one of onHover, onClick, etc. That will describe that the sequence of operations is done on the corresponding event. In that case, it will be more callback than a function and it is alright to do more than one action.

function onEvent() {
     deleteList(current_list_id);
     navigate("..");
}

How do I properly write these functions?

2 Answers2