11

Compiler Warning This line gives you a compiler warning because it's not using generics: List activeList = listOfLists.get(pageNumber); it should be List<T> activeList = listOfLists.get(pageNumber); Empty list input If the input is an empty list, then the output will be a list containing one empty list. I think it would be more reasonable if the ...


11

Bug You're code has a bug-ish. When using this, if instead of hitting enter when asked to enter a character you enter some other letter and then you press enter, then next newline will be skipped and you will have two lines merged into one: Fooa BarHello, World! Spama Eggs where the text file was: Foo Bar Hello, World! Spam Eggs To be honest, I'm not ...


9

List<char[]> c = new List<char[]>(); private int pagedepth; private long pagesize; private long mpagesize; // https://stackoverflow.com/questions/11040646/faster-modulus-in-c-c private int currentPage = 0; private int currentPosInPage = 0; Some of these names are rather cryptic. I'm not sure why c isn't private. And ...


8

Some notes: Be consistent with the use of parens. Use ternary expressions (?) to reduce short conditionals. If you are using Ruby >= 2.0, check named keywords. The resulting expression of an assignment is its value, so you can remove the last @result What looks gross to me are those in-place updates of @result, but this goes beyond the scope of the question....


8

There have been a number of answers, and they all provide very good input. Both Simon and Janos suggest answers that are similar, you have a list-of-lists, and you have a 'current' list that you will with values. The concept is good, but, if you reverse the logic slightly, you can neaten up the code and remove the 'terminal' if-condition. Consider this code:...


8

Naming A better name for this functionality would be partition, like it is in Guava's Lists.partition. Handling invalid / strange arguments There are several possible invalid / strange inputs to consider: size < 1: invalid input, the natural response is to throw IllegalArgumentException original list is empty or null: strange input, you might even ...


8

int num What does num represent exactly? Number of pages? Number of posts per page? Current page number? Properly naming the variable would help a lot in clarity here. var postsPerPage = 3; Turn it into a parameter with default value 3. That way you can always decide later that the user can change this without much trouble. Instead of using 0-based pages ...


8

1. Review The function needs a docstring to explain what it does, with maybe some examples. The explanation in your post is very good: this is exactly the kind of thing that should go into the docstring. The number 5 is arbitrary, so I would make it a parameter to the function. The implementation takes time proportional to the number of entries in the page ...


7

There are many ways to test this. Mostly, though, it'd be nice to avoid having to create 50+ records, since it slows down your tests. If you use a request spec, though, it's probably best to create 50+ records, since it's a high-level test, so you'll want to be close to the "real" usage scenario. But you can cheat a little in other places. For instance, if ...


7

while loop Your while loop would work much better as a for loop, as you are iterating over index anyways (you would get rid of two extra lines of code, and the code would be clearer as well). raw list It's always a bit ugly to have raw types, so List activeList could be List<T> activeList. Naming pageNumber isn't very generic, listNumber or ...


7

Styling and readability The styling of your code is inconsistent. You sometimes use spaces after a comma, you sometimes do not. You sometimes use a space between the curly brace and the thing it belongs to, you sometimes do not. You sometimes use whitespace around operators, and you sometimes do not. This makes the code harder to read, and I recommend ...


6

Yes, I would have written this completely differently (so I decided to do just that). I'll follow my solution with a hopefully scientific analysis of both solutions. Finally I'll give some personal recommendations. Note: The reason for the in-depth analysis is that I was already interested in pagination and thinking about an implementation for a framework ...


6

Let's start with the BUG. if (entity.Count() == 0 || entity == null) If entity is null, then you'll get a NullReferenceException when Count is called, so there's currently no possible way the second half of this statement will ever be called. I think you meant to do this. if (entity == null || entity.Count == 0) Which works, but isn't great. I would ...


6

You have a surprise bug: what happens when I do page.php?Name=EBrown? Well $_GET is not empty but your page_number sure isn't set. That said, you should be doing something like the following: $page_number = 1; if (isset($_GET["page_number"})) { $page_number = $_GET["page_number"]; } or $page_number = $_GET["page_number"]; if (!isset($page_number)) {...


4

Well, first of. This code is not at all secure. you don't check wether the $_GET['page'] var is actually an int. But I don't think that was the point here. Second: sometimes you put the page numer in the id, this is a number and is thus invalid HTML, and sometimes you simply put it in the href. Why not always that last thing? Third: read the specs about ...


4

Okay, I’m going to list anything and everything that came in my mind while reading your code. Start documenting your code for better maintainability Validate every and any user input Use proper camelCase notation for property and method names Separate your code into more methods (especially in __construct) Keep everything protected unless you have a good ...


4

You don't need to worry about sparse arrays in JavaScript and there is no need to instantiate with the Array constructor. If you insert into a random index then the rest of the array is just padded with undefined references: var items = []; items[5] = 'foo'; console.log(items); // [5: "foo"] items.toString(); // ",,,,,foo" To add new items into the ...


4

If you look at your code you have a lot of if pageCount.has_key(page): else:. This can be replaced with a defaultdict. pages defaults to a list, as if pages[page] does not exist you manually default it to a list. And page_count defaults to zero. As a simple example the following is how you could use default-dicts: from collections import defaultdict pages ...


4

A function has parameters It allows for more flexibility and reusability. There is absolutely no need to declare scoreResult as a global variable, it forces you to use that variable name each time you want to use this function. Instead declare it as a parameter of the function. Same goes for PageLimit; except for this one you can use a default value to ...


4

Your linq in linq in linq statement is written as a one-liner, with a single semicolon: return (from b1 in ((from b2 in ((from b3 in dbSet.Include(b3 => b3.Category).Include(b3 => b3.User) orderby b3.CreationDateStamp ascending where categories.Contains(b3.CategoryId) ...


4

If you’re using HTML5 <nav> element, it’s not necessary to apply a role of ‘navigation’ – certain HTML elements inherently pass their relevant roles to the accessibility API. If you were wrapping the navigation menu in a <div>, for example, then you’d need to apply a role of ‘navigation’. Role="menubar" has been correctly applied to the <ul&...


4

//countQuery mirrors query through the filter assignments I don't think that's necessary. You could have only one query for the first part of the method, then do recordCount = query.Count(); and then keep using query as you do now. This way, you have less code and less chance of the count getting out of sync. query.Where("IsActive = true") I realize ...


4

The intention When I have all matching objects - I pass them to a "Paging" method within the service (I think I have to do this in the service rather than the repository because a generic repo paging method won't work across some resources e.g "orderBy" a property nested within a class in an aggregate object). I'm apprehensive of your design decision ...


4

We can improve readability of your code as follows: using type annotations partially looks incomplete, when we can be more descriptive: from typing import (Iterable, TypeVar) ElementType = TypeVar('ElementType') def iter_group(iterable: Iterable[ElementType], batch_size: int) -> Iterable[Iterable[ElementType]]: ....


4

Design considerations In the comments you mentioned that you're going to analyze and manipulate this string? If so, that could be a problem, because replacing, inserting and removing text requires moving all subsequent characters to make room or to fill the gap. With this much data, you can expect that to have a major impact on performance. That is why I ...


4

This code is safe from the SQL injection standpoint but it is likely prone to XSS because of the untreated output, and also to possibly leak the sensitive information due to the error message unconditionally spat out. Regarding other improvements I would suggest a more robust connection code and to avoid bindParam calls thanks to disabled emulation mode. ...


4

From a short review; You use globals instead of parameters for activePage and pageNeighbours You need at least one comment stating what pageNeighbours stands for, without knowing this the code looks meaningless to me You need at least one comment on the top explaining what the range is supposed to contain On the whole I think this could use more comments ...


3

First, putting all HTML into echo does not necessarily make things cleaner. More typically, it makes them full of escaped quotes and difficult to edit in an IDE. I personally only use echo for output that will span less than a full line. I use php tags and heredocs for multiple lines. A lot of people don't know about heredocs, so I will use them in my ...


3

Your code duplication in BindResourceRepeater (2x in there) and btnSearch_Click is around the differences of how the keyword is handled and how the prev/next links are enabled/disabled. Lets look at the link enabling first. To me the logic of disabling the links should be very simple: The prev link should be enabled on every page except the first one The ...


3

The code looks fine to me. The only things I would personally change is the use of the live() method (which has been deprecated, and was slow anyway) todelegate() and the use of the :nth() selector, which again I believe is not very fast - although I've not specifically tested it. In place of that I'd use eq(). $('.modal-pagination').delegate('a', 'click', ...


Only top voted, non community-wiki answers of a minimum length are eligible