Global variable or parameter?

Question

Is it correct that homeitem should be declared inside the OnLoad method and overgiven to all the render methods instead of just declaring it as a global variable and accessing it by all the render methods?

public partial class Default : Page
{
    /// <summary>
    /// Initializes the module.
    /// </summary>
    /// <param name="e">
    /// The event arguments.
    /// </param>
    protected override void OnLoad(EventArgs e)
    {
        var homeItem = Sitecore.Context.Database.GetItem(Sitecore.Context.Site.StartPath);

        // some code...

        this.RenderHomeIcon(homeItem);
        this.RenderFacebookTags();
        this.RenderLanguageEntries();
        this.RenderServiceNavigation(homeItem);
        this.RenderMainNavigation();
        this.RenderCallToActionItems(homeItem, isHomeItem);
        this.RenderFooterMainNavigation(homeItem);
        this.RenderContact(homeItem);
        this.RenderFooterCulture(homeItem);

        if (isHomeItem)
        {
            this.InterfererToolbar(homeItem);
            this.RenderWebcamViews(homeItem);
            webcam.Visible = true;
        }

        // some more code...
    }
}

I'd just like to know what is correct / standard so I can stick to that!

rather blatant duplicate of [Why is Global State so Evil?](http://programmers.stackexchange.com/questions/148108/why-is-global-state-so-evil) — gnat, Aug 26 '16 at 10:37
Why would it need to be global? You could make it a private field inside `Default`. I'd be more worried about the name `Default`, the huge number of pointless `this.` suffixes and the length of the method (how much code is in `// some code...` and `// some more code...`), than choosing between passing `homeItem` as a parameter versus making it a field. — David Arno, Aug 26 '16 at 11:21
Not everything in programming has a "standard approach" or a "best practice." You have to evaluate the merits of each approach, and make a decision based on your specific requirements. — Robert Harvey, Aug 26 '16 at 14:00
@DavidArno: (just curious) how is this. prefix can be a problem to the point of overshadoing global variables. More precisely, what bugs, problems or side effects can prefixing this get us into, that is beyond the visual noise it can create on screen that would be worst than a global variable could potentially present. — Newtopian, Aug 26 '16 at 14:50
@Newtopian, because the OP is offering a false dichotomy of solutions. If the choice were genuinely between global state and parameters, then parameters would be the answer. However, a third way - using private fields - exists too. Sometimes fields are better; sometimes parameters. So the OP should worry less about choosing between them and focus more on improving the readability of the code (such as reducing the noise from all those `this.`'s. — David Arno, Aug 26 '16 at 14:56
@DavidArno: Thanks ! I thought that is what you meant but I had a nagging doubt there might have been something about this I did not know about. — Newtopian, Aug 26 '16 at 15:10

score 0 · Answer 1 · answered Aug 26 '16 at 15:09

From what I see here you are more asking about making homeItem a function variable or a class field, neither of which amount to a global variable.

Now, from your code it does seem that the homeItem is drawn from a global state of sorts, so I would answer your question with more questions ?.

How often does the original data change ? If it never change then you can safely make it a class level field. If it will change then it`s probably best to just get it every times you need it (leave it as a function variable as is now). Now reading the code, if your variables are named properly I doubt the start path would change much once the app starts.
How costly is it to get the value ? that is, is calling Sitecore.Context.Database.GetItem(Sitecore.Context.Site.StartPath); something that is expensive do to (memory, CPU, latency etc). If no then you could call it each time you need it, provided it won't change between calls (see first question).

Lastly, the concept of GlobalVariable is quite clear in the programmer world and very much frowned uppon. Most for very good reasons but that does not necessarily mean that it should be banned either. This is why I interpreted your GlobalVariable more as a private field for your class than a true global variable.

So basically here the best advice available on this whole page IMHO are the wise words of @RobertHarvey: Know what you need, know your system, get possible solutions and the right one should just emerge. Most of all, stay curious and never get religious about code.

Global variable or parameter?

1 Answers1