2
\$\begingroup\$

I have this script which is for a news feed. A button click will grab more data from the server if there is any and then add it to the box. It doesn't allow any more than 10 pages of results currently and how to properly store that page counter is what this question is about. loadMoreFeedItems() is the function attached to the onclick event of the aforementioned button.

<script type="text/javascript">

    window.page = 0;

    function loadMoreFeedItems() {
        var options = {};
        options.page = window.page;
        $.ajax({
            url: "home/feed",
            data: options,
            dataType: "html",
            success: function (data) {
                // Code to append the data received into the feed element
                window.page++;
                // Code to hide "load more" button if window.page exceeds 10
            }
        });
    }

    (function () {
        loadMoreFeedItems();
    })();

</script>

So I did a little reading about global variables in JavaScript[1][2][3] and while this works, I got the vibe that this could be problematic, so I'm wondering if there is a better way. One alternative I thought of was to create some kind of hidden that stores the value in page rather than the browser, and then I'd modify/retrieve it using a jQuery selector, but that seems even dirtier.

\$\endgroup\$
1
\$\begingroup\$

This doesn't have anything to do with ajax or the options exactly, just about "counting up" so i will show my example using a function called countUp which logs ever increasing numbers.

In short you can make page a member of countUp. You will recall that in javascript that functions are very much first class citizens and they are objects. Its easier to see this of course if you write your function definitions like:

var countUp = function () { /* .. function body */ }

So the variable countUp can have in it any number of members. The member we are interested in is page or more precisely this.page. So lets put this all together.

var countUp = function ()
{
    this.page = (typeof this.page !== "undefined") ? this.page : 0;
    this.page++;
    console.log(this.page);
}

countUp();
countUp();
countUp();
// logs 1 2 3
\$\endgroup\$
  • \$\begingroup\$ This is a very bad recommendation. Creating a class that when instantiated increases a counter? That is completely illogical and a huge misuse of JavaScript's OO features. All the code needs to do when counting is increment. \$\endgroup\$ – SirPython Aug 16 '15 at 20:28
  • \$\begingroup\$ @SirPython sorry but you are very unclear about a few things. One is posted as a comment to your Answer. And the other all functions are objects in javascript. The only difference is that shorthand function foo() peculates to the top of the scope, while var foo = function() does not. They are both objects of type Function. Objects have members this is a very basic principle of OOP. \$\endgroup\$ – Victory Aug 16 '15 at 20:39
  • \$\begingroup\$ @SirPython another point of misunderstanding you have is that talk about "Classes" in javascript When in reality javascript uses prototypes for an inheritance model. \$\endgroup\$ – Victory Aug 16 '15 at 20:44
  • \$\begingroup\$ When talking about JavaScript, "classes" usually refer to functions that are used to instantiate new objects. And yes, I know how JavaScript works and that "everything is an object". \$\endgroup\$ – SirPython Aug 16 '15 at 20:55
  • \$\begingroup\$ Well i am obviously not instantiating a new class on each call. See the terinary expression, i am reusing this.page exactly because countUp is an object. At any rate I think its time for me to step away from this thread, but hopefully I can make it constructive for everyone by pointing out this great resource on really understanding javascript. "You Don't Know Javascript" github.com/getify/You-Dont-Know-JS \$\endgroup\$ – Victory Aug 16 '15 at 21:05
0
\$\begingroup\$
var options = {};
options.page = window.page;

This is kind of a dirty way to populate an object. It's just like creating a blank array only to populate it's indexes in the line after, like this:

var arr = [];
arr[0] = ...;
arr[1] = ...;

That's a little gross looking, right? It would look a lot more clean if you just setup the page property as soon as the object was created, like this:

var options = {page:window.page};

Why are you storing the page variable in the window object? There is no need (at least with the code that is shown) to have that variable in scope for every other script in that page: only this script is accessing it, so it should be declared with var.

That would look like this:

var page = 0;

Now, when accessing the page variable, you no longer use window.page, you just use page.


\$\endgroup\$
  • 1
    \$\begingroup\$ If you change window.page to var page; on the same line as Ty has it, you are not changing anything in the design, page would still be global in either case. This answer is wrong. \$\endgroup\$ – Victory Aug 16 '15 at 20:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.