Idiomatic refactoring into subfunctions?

help
1

What is idiomatic way to refactor long function into subfunction?

fn main() {
    #[derive(Debug)]
    struct Struct1 {
        a: i32,
    }
    let mut struct1 = Struct1 { a: 1 };

    let mut step = || {
        struct1.a += 1;
    };

    step();
    dbg!(&struct1);
    step();
    dbg!(&struct1);
}

Assumed closure step uses much more than single variable from the context.

Options I see:

  • pass all arguments -- that is bad option because context could have dozen of variables
  • make a struct having all variables and pass it as an argument to the subfunction -- not perfect option
  • use submacro instead of subroutine -- not perfect option

Is there better solution?

Playground

2

Well of course, nothing is perfect.
However, the second option is the general solution to this problem, regardless of programming language.
The first option, as you mention, is going to be bad for anything more than a few variables.

2 Likes
3

I think the question is too general. If a function has become very long, that's usually because of poor design, perhaps the function is doing more than one thing for example. But really, the answer has to be "it all depends", there is no general solution.

4 Likes
4

Something you need to keep in mind is that refactoring is a thing you do for the benefit of humans. A compiler really won't care if you put your entire application within a single function and have a hundred variables called variable_1, variable_2, and so on.

I'm sure you already know that, but it bears mentioning anyway... The entire process depends on context and how your brain relates concepts, so providing a generic code snippet containing a main() function with generic variables and types makes it hard to give you a concrete answer - Struct1, a, and step() all mean nothing to me and therefore it's hard to know how they relate or could be restructured to make the code easier to understand.

That said, here are some general pieces of advice for refactoring code:

  • Keep related things close together
  • Keep unrelated things far apart
  • Listen to your code - if it feels awkward to use something then maybe it needs refactoring/redesigning
  • What is your ownership story? Who owns what and how should access be mediated? Where are the boundaries between components/concepts/systems?
  • If it can't fit on your screen it probably can't fit in your head
  • The bigger the interface, the weaker the abstraction
  • Refactor early and refactor often
  • You can use closures to concisely hide a bunch of state and tricky logic behind a simple interface
  • If there are a lot of details floating around, you can often reduce complexity by adding another level of indirection (e.g. wrap a bunch of related variable in a struct and hide them behind a couple methods)
  • The Gang of Four get a lot of hate, but some of their techniques are actually pretty useful
  • Write lots of code that uses your code - writing tests and examples is often the best way to come up with a good design
  • Keep your functions and structs small, if a function takes more than about 3 arguments or a struct has more than about 4 fields that's probably a hint that it's trying to do too much

Some Rust-specific points:

  • Good use of the ? operator can make a lot of complicated logic just.... disappear
  • Don't reach for complex generics unless your code actually needs to be complex and generic
  • Lifetimes are most useful for short-lived objects
  • Lifetimes and generics can be infections - if a field has a lifetime/type parameter then the containing struct will need one, too (and so on, all the way up the chain)
  • Repetitive, boring boilerplate (e.g. implementing a trait for tuples of varying sizes or the various integer types) can often be abstracted away using declarative macros
  • Run clippy. Often.
  • You can ask people to review your code. I've found this forum and the community Discord to be rather helpful

Some less concrete tips which will help you write better code in general:

  • Learn your paradigms
    • Procedural code is great for straight-line code where there isn't much need for flexibility and everything is simple
    • Object Oriented Programming is good for high level orchestration because you can think in terms of abstract actions and not care about the details
    • Functional Programming is good when you can phrase your problem as a series of transformations over some data
  • Learn what "tiers" of abstraction you have available to you - the further you go the more power you have, but with increased flexibility comes increased cognitive load
    1. Variables
    2. Functions
    3. Classes/Structs
    4. Interfaces and Generics
    5. Code generation (i.e. macros and build scripts)
    6. Runtime reflection
  • Read other people's code, as much as you can find, and pick it to pieces
    • Ask yourself why something feels elegant
    • Find the code that looks like garbage and ask yourself what makes it so
    • Put yourself in the shoes of the author and try to guess why they may have done things a certain way and what look for the hidden constraints that may have forced things to be that way
  • You need to come to terms with the fact that not all code can be (or needs to be) "clean"
    • Some problems really are just complex and there is no elegant way to solve them
    • If I'm going to throw this code away in a week or have a deadline to hit I probably won't care about designing it well
  • It always depends... Writing code is subjective and very context-sensitive, what worked for you probably won't work for me, or me the 6 months from now

(EDIT: sorry for the wall of text, this was mostly a brain dump of things I've learned in my time. I'm sure I'll write it up properly one day :sweat_smile:)

12 Likes
5

Question about refactoring into subroutines not about function been too long. Code above works in JS and most script languages, but does not in Rust. This is a case when something very simple is not reachable easily in Rust.

6

Wow. Thanks. That's good general guide.

7

Is there a better way? In JS code written above does just work, so struct is not so general.

8

I didn't quite get you. What doesn't work? What is the code that you have in JS that works but in Rust doesn't? What's the issue?

9

This code. I know why, but not sure what is the best workaround in Rust.

10

Does this code help?

2 Likes
11

Thanks. I don't see a better option.

12

Looking at that code again, I think this is actually a conceptual/design issue and unrelated to refactoring... In JS it's common to create lots of closures that close over variables and mutate them when they get called, but this practice goes almost exactly against Rust's "shared read XOR single write" rules so you run into a lot of friction/frustration.

Instead of having a long-lived closure which borrows something mutably, you should prefer to have short-lived borrows so other things get a chance to access your struct1 variable. Normally I would go with @RedDocMD's suggestion of a step() method which only borrows for the duration of the method call.

2 Likes
13

I see. Thanks for clarification.

14

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.