Mapping values. Should a single use variable be declared as a variable at all?

Question

Where I have, let us say an object, that I will only use once, what is considered general best practice in terms of declaring it as var vs. simply putting the object directly into the method it will be used in? I'm thinking this is going to come down to "which do you think is more readable", but given my lack of experience thought I'd see if there was a consensus.

This came about when looking at the following code:

      var aiPersonalities = model.aiPersonalities();

      var newPersonalities = {
        qCasual: {
          ai_path: "/pa/q_casual",
          display_name: "!LOC:Q-Casual",
          metal_drain_check: 0.64,
          energy_drain_check: 0.77,
          metal_demand_check: 0.95,
          energy_demand_check: 0.92,
          micro_type: 0,
          go_for_the_kill: false,
          priority_scout_metal_spots: true,
          enable_commander_danger_responses: false,
          neural_data_mod: 2,
          adv_eco_mod: 0.5,
          adv_eco_mod_alone: 0,
          factory_build_delay_min: 0,
          factory_build_delay_max: 12,
          per_expansion_delay: 60,
          personality_tags: ["queller"],
          min_basic_fabbers: 10,
          min_advanced_fabbers: 3,
        },
        // imagine another 11 objects of the same size here
      };

      var baseline = aiPersonalities.Absurd;

      newPersonalities = _.mapValues(
        newPersonalities,
        function (personality, name) {
          var result = _.assign(_.clone(baseline), personality);
          result.name = name;
          return result;
        }
      );

      _.assign(aiPersonalities, newPersonalities);

      model.aiPersonalities.valueHasMutated();

This could be written in a way which removes aiPersonalities, baseline, and avoids newPersonalities referencing itself:

      var newPersonalities = _.mapValues(
        {
          qCasual: {
            ai_path: "/pa/q_casual",
            display_name: "!LOC:Q-Casual",
            metal_drain_check: 0.64,
            energy_drain_check: 0.77,
            metal_demand_check: 0.95,
            energy_demand_check: 0.92,
            micro_type: 0,
            go_for_the_kill: false,
            priority_scout_metal_spots: true,
            enable_commander_danger_responses: false,
            neural_data_mod: 2,
            adv_eco_mod: 0.5,
            adv_eco_mod_alone: 0,
            factory_build_delay_min: 0,
            factory_build_delay_max: 12,
            per_expansion_delay: 60,
            personality_tags: ["queller"],
            min_basic_fabbers: 10,
            min_advanced_fabbers: 3,
          },
        // imagine another 11 objects of the same size here
        },
        function (personality, name) {
          var result = _.assign(
            _.clone(model.aiPersonalities().Absurd),
            personality
          );
          result.name = name;
          return result;
        }
      );

      _.assign(model.aiPersonalities(), newPersonalities);

      model.aiPersonalities.valueHasMutated();

I'm just interested in how people would approach this. My instinct is that aiPersonalities and baseline could be ditched, per approach 2, but that keeping the initial declaration of the newPersonalities object may make it easier to see what the _.mapValues bit is doing.

There aren't any style guidelines or anything, this is a purely me project.

Answer 1

In this specific scenario, I would definitely go with your first approach. Putting a large amount of data right in the middle of your logic makes it extremely difficult to follow the logic.

To answer the general question of "should data associated with a specific function go inside its body or outside?" - there's never one right answer. I like to think of it as though there are various pressures going on. If your function is pretty cluttered but your module is pretty empty, then moving some stuff out of the function and into the module helps improve the readability of that function without sacrificing much. On the other hand, if there's already a whole lot of stuff in that module, and the function is really simple to follow then it might be worth it to move stuff into the function (or split the module up into multiple modules, if the containing folder isn't too cluttered). One important task of a programmer is to know when and how to shuffle clutter around to prevent any one spot from feeling too overwhelming.