Prefer inline event handler to avoid indirection in code to improve human readability or use handler function?

user_261061 0
Viewed 269

This is two React components source code coding styles (put your attention on onSubmit):

Version 1:

...
const ContactsEditPage = ({ match }) => {
    ...

    const [updateContact] = useMutation(
        gql`mutation updateContactById ($id: UUID!, $input: ContactPatch!) {
            updateContactById(input: {id: $id, contactPatch: $input}) {
                clientMutationId
            }
        }`
    );

    return (
        ...
            <Formik
                initialValues={data.contactById}
                onSubmit={(values, actions) => {
                    updateContact({
                        variables: {
                            id: match.params.contactId,
                            input: values
                        }
                    })
                    .then(() => history.push('./'));
                }}
            >
                <Form>
                    <FieldGroup>
                        <FieldRow>
                            <FieldLabel>Email:</FieldLabel>
                            <FieldInput
                                as={Field}
                                type='email'
                                name='email'
                                placeholder='Email'
                            />
        ...
    );
};

Version 2:

...
const ContactsEditPage = ({ match }) => {
    ...

    const [updateContact] = useMutation(
        gql`mutation updateContactById ($id: UUID!, $input: ContactPatch!) {
            updateContactById(input: {id: $id, contactPatch: $input}) {
                clientMutationId
            }
        }`
    );

    function handleContactSubmit(values) {
        updateContact({
            variables: {
                id: match.params.contactId,
                input: values
            }
        })
        .then(() => history.push('./'))
    }

    return (
        ...
            <Formik
                initialValues={data.contactById}
                onSubmit={handleContactSubmit}
            >
                <Form>
                    <FieldGroup>
                        <FieldRow>
                            <FieldLabel>Email:</FieldLabel>
                            <FieldInput
                                as={Field}
                                type='email'
                                name='email'
                                placeholder='Email'
                            />
        ...
    );
};

The differences:

  • version 1 use inline Javascript code in event handler attribute onSubmit
  • version 2 use handleContactSubmit function instead inline Javascript code

In the first part of my coding life, I preferred to use handler function (version 2).
Now I have a preference for inline code (version 1) when:

  • this code isn't too long
  • and this code is used only once

Why: I prefer inline event handler code to avoid indirection in code to improve human readability (for coding review process).

There is also a cost to use a function (inderection). When a new reader encounters this code, they need to jump between many function definitions in many files. This non-linear reading process requires more mental focus than reading linear code.

I meet a majority of ReactJS coders who prefer to use handler function (produce indirection in code).

My question:

  • Am I part of a coder minority who prefer indirection in code?
  • Do I have to accept majority preference even if my non-linear reading process requires more mental focus than reading linear code? And add handler function rule use in the ReactJS style guide in my project?

Note:

Best regards,
Stéphane

reactjs coding-style
3 Answers

TL;DR It depends on the situation and personal preference.

  1. Using an arrow function in render creates a new function each time the component renders, which may break optimizations based on strict identity comparison.

This only applies to class components and is not relevant for functional components. In a functional component, the function is re-defined on every render regardless of if it's defined as function, const myFunc = ..., or onClick={() => ...} because it's scoped to your component-function (this could be solved with memo, but that's out of scope of your question).

  1. avoid indirection in code to improve human readability

IMO, I would say this depends. Sometimes, especially when e.g. reviewing a PR or returning to code I haven't seen in a while, I like to see a function name in the component's prop like onClick={doSomething} because then I can skim past the content I am not concerned with and focus solely on what I need to do for the task at hand. But, more importantly, it gives context to what is happening through the function name.

Yes, you're right that the dev has to scroll down to see where the functions are used and then scroll back up to see what the function does, but if I saw a component with multiple buttons, each with different handlers, something like below, I would have to actually read every single handler until I found the function I wanted to alter. If those were instead replaced with function names, I could easily just understand "Oh this button does this, whereas this other one does that" without having to scroll up to find the function declaration. This is especially true when the function is large or complex; re-reading a 30 line function that has no name just to try to remember what it does/why is very painful when you return to your code 6 months later.

return (
  <div>
    <button onClick={() => ...}>Some text</button>
    <button onClick={() => ...}>Some other text</button>
  </div>
);

Another thing to think about is levels of nesting. If you have a large render with many indentations, and then you add a complex function inline to props, suddenly you have to scroll left/right in your editor which essentially re-introduces the original issue you tried to solve.

  1. Am I part of a coder minority?

No, there are use cases for both inline and separately-declared handlers. I write both in my repositories, both in and out of work. My teammates do the same.

If the handler is small and it takes little effort to understand what it does (e.g. onClick={() => location.hash = "/redirectedPage"}), then it's totally reasonable, and probably better for readability, to use inline functions (why declare a new function redirectToPage when it's a one-line function?). For more complex functions, or deeply nested components, it is often helpful to declare a function name.

There are other factors that could come into play here, like maintaining consistency within a code base, lint rules, etc., but these are some of the main ones that I think apply outside of specific tunings like those.

user_57711070

My philosophy is to write code optimized for readibility that's why I try to keep the render / return as abstract as possible, it's easier to come back later (for debugging, new features...) and know quickly from the render / return what the function does from its name.

What seems to be a problem of the indirection is a false problem, because IMO:

  1. Your files & dirs are supposed to represent part (big or small depends to you) of the logic of the components and/or the app and if it's the case you naturally look for the right function in the right place (the naming should / is supposed to be explicit).
  2. It depends on your editor (in my case VScode) there are different shortcuts that take you to the definition of the function/var/const...or simply by alt-click
  3. Writing code is always a WIP, that's why even if I don't repeat the code I prefer the indirection (version2) because 90% of the time the function will be modified in the near future, so I prefer keep all the modification in one place and not within the render.
  4. Navigating files & dirs is not an option when the base code grows up, so you'll do it even if you use the inline style.
  5. It's more close to React logic and source code: reusable components.
  6. I personally prefer keeping the functions definitions not in the same place as the return/painting/rendering...
  7. It's / supposed to be part of your team / company agreed it on coding style.

For your questions:

Am I part of a coder minority who prefer indirection in code?

From what I've seen, in React world version 2 is the preferred and most used one.

Do I have to accept majority preference even if my non-linear reading process requires more mental focus than reading linear code? And add handler function rule use in the ReactJS style guide in my project?

IMHO, the most important thing is the team and what makes sense to its velocity. To me it's not a religion so my best answer is to choose the one that is more accurate to your/your team efficiency.

user_48749290

TL;DR when not obvious, agree with your team on a scope basis.

Some good answers have already been posted here and the consensus seems to be that it depends on the preferences of the team as well as "objective" factors such as:

  • if it's a one-line function, prefer in-line
  • if it's being reused, prefer handlers

But in your example, the handler is neither of the abov, does multiple things and is a bit more complex than a single state update. One could argue that this complexity justifies using a handler, or this non-reusability justifies a in-line style; it is difficult to draw the line as readability is quite personal and depends on the developer's background, personal preferences, IDE etc. as explained with detail in yuyu5's answer

Your team may not agree on what complex-enough-for-a-handler mean, but could agree on drawing the line according to the scope of the function. For example: does it stay within the component (state, form validation, UI-changes) or does it goes beyond (localStorage, user log in, and in this case, programmatic navigation)?

A scope-based agreement (whether you pick the one I described or another) is easier to enforce and would age better than a complexity-based one, as IDEs and preferences change, coding styles should remain the same.

user_14559050
Related