Calculate difference of indices The challenge is to reconstruct an n-digit number using the following information:

  On each step, you choose an index x from 1 to n.  For all indices y (y
  < x), you calculate the difference by = ax - ay.

  You then calculate B1 — sum of all by which are greater
  than 0 and B2 — sum of all by which are less than 0.

  The answer for this step is B1 - B2.

  Input

  The first line contains two integers n and m, denoting the number
  of digits and number of steps. The second line contains n digits
  (without spaces) a1, a2, ..., an.
  Each of the next m lines contains a single integer x denoting the index for
  the current step.

  Output

  For each of m steps print single number in a line - answer of the
  step.

Can someone please help me optimize this code or provide a better, less time-consuming solution?

import java.io.*;
import java.util.*;
public class Chefs
{
    //BufferedReader in;
    DataInputStream in;
    PrintWriter out;
    HashMap<Integer,Integer> mymap=new HashMap<Integer,Integer>();
    int a[],n,m,b1,b2;
    public Chefs()
    {
        //in=new BufferedReader(new InputStreamReader(System.in));
        in=new DataInputStream(System.in);
        out=new PrintWriter(System.out,true);
        readInput();
        calchef();
    }
    void readInput()
    {
        try
        {
            String digit=in.readLine();
            String split[]=digit.split("\\s+");
            n=Integer.parseInt(split[0]);
            m=Integer.parseInt(split[1]);
            a=new int[n];

            digit=in.readLine();

            for(int i=0;i<digit.length();i++)
                a[i]=digit.charAt(i);
        }
        catch(Exception e)
        {
            e.printStackTrace();
        }
    }
    void calchef()
    {
        for(int i=0;i<m;i++)
        {
            try
            {
                b1=0;
                b2=0;
                int index=Integer.parseInt(in.readLine());
                if(mymap.containsKey(index))
                {
                    int ans=mymap.get(index);
                    out.println(ans);
                }
                else
                {
                    for(int j=0;j<index-1;j++)
                    {
                        if(a[index-1]-a[j]>0)
                        {
                            b1+=(a[index-1]-a[j]);
                        }
                        else
                        {
                            b2+=(a[index-1]-a[j]);
                        }
                    }
                    int ans=b1-b2;
                    out.println(ans);
                    mymap.put(index,ans);
                }
            }
            catch(Exception e)
            {
                e.printStackTrace();
            }
        }
    }
    public static void main(String[] args)
    {
        //long startTime = System.currentTimeMillis();
        Chefs c=new Chefs();
        //long endTime   = System.currentTimeMillis();
        //long totalTime = endTime - startTime;
        //System.out.println(totalTime);
    }
}

1) I would change the inner for loop this way (very little optimization, unlikely it will bring better performance)

int ans=0;
int y=index-1;
for(int j=0; j<y; j++)
{
    int diff = a[y] - a[j];

    if(diff > 0)
        ans += diff;
    else
        ans -= diff;
}

2) You never store ans in the map.


Rewrite conditional statement I have this if else block code

 if($_POST["item"]=="All"){
        $notificatioSetting->AllNotification=$_POST["status"];
    }
    else if($_POST["item"]=="Accept"){
        $notificatioSetting->AcceptTrackRequest=$_POST["status"];
    }
    else if($_POST["item"]=="Deny"){
        $notificatioSetting->DenyTrackRequest=$_POST["status"];
    }

this condition continues for almost 12 another else if condition. Beside this and switch, is there a better way?

If you have a lot of conditions I recommend you to use an HashMap (associative array in php) to store the conditions, like this :

$notificationsItems = [
    "All" => "AllNotification",
    "Accept" => "AcceptTrackRequest",
    "Deny" => "DenyTrackRequest"
];

When you have your pair, you can test all the conditions through a simple foreach:

foreach ($notificationsItems as $postValue => $objectAttr) {
    if ($_POST["item"] == $postValue) {
        $notificatioSetting->{$objectAttr} = $_POST["status"];
        break; //we make an early termination
    }
}

This version mimic exactly the elseifcode. As itzmukeshy7 put in his response, there is a solution with fewer lines of code :

if(in_array($_POST["item"], $notificationsItems)) {
    $notificationSetting->{$notificationsItems[$_POST["item"]]} = $_POST["status"];
}

In this way adding more conditions will be really easy while making the code more extensible and more decoupled. In addition you'll make explicit the association between a post value and the corresponding attribute in the object, which is good for your futur self or colleague(s).


Find repeated substrings of length 3 in a string The following method receives a string and finds all occurrences of repeated substrings with a length of three characters, including spaces. Is this a good approach?

public Map<String, Integer> findOccurences(String str) {
        Map<String, Integer> map = new HashMap<String, Integer>();
        int counter = 0;
        String sub;
        int stringLen = str.length() - 2;
        for (int i = 0; i < stringLen ; i++) {
            sub = str.substring(i, i + 3);
            if (map.containsKey(sub)) {
                counter = map.get(sub);
                counter++;
                map.put(sub, counter);
                counter = 0;
            } else {
                map.put(sub, 1);
            }
        }
        return map;
    }

Input

This This

Output

 Th 1    <<< includes space
s T 1
his 2
Thi 2
is  1    <<< includes space

It's quite fine. The logic is simple and clear.
There might be a more clever solution (I don't know),
but probably it will be less clear.

A few improvements would be good though:

counter is set to 0 at two places, both unnecessary, as the value is overwritten later
It's a good practice to declare variables in the smallest scope where they are used. For example counter and sub would be better to declare inside the loop.
stringLen is not a great name, because it's not really the "length" of anything. Even limit would be better
Instead of the magic number 3, and str.length() - 2, it would be better to make the target length a method parameter.
Instead of calling .containsKey followed by get and put, I prefer to get, check null and then put, whenever possible. This is not always possible, for example when null is a meaningful entry, but that's not the case here.

Based on the above suggestions, the implementation could become a bit simpler:

public Map<String, Integer> findOccurences(String str, int length) {
    Map<String, Integer> map = new HashMap<>();
    int limit = str.length() - length + 1;
    for (int i = 0; i < limit; i++) {
        String sub = str.substring(i, i + length);
        Integer counter = map.get(sub);
        if (counter == null) {
            counter = 0;
        }
        map.put(sub, ++counter);
    }
    return map;
}

And it would be good to add a unit test for it:

@Test
public void test_This_This_() {
    Map<String, Integer> map = new HashMap<>();
    map.put("his", 2);
    map.put("Thi", 2);
    map.put(" Th", 1);
    map.put("s T", 1);
    map.put("is ", 2);
    assertEquals(map, findOccurences("This This ", 3));
}


Create nested named object using variables as key I am trying to create a new object with nested value pairs. My initial challenge is that I have a huge object containing lots of junk data, and I'd like to strip away the value pairs that I do not need as well as renaming certain values. Performance is vital as this will iterate over thousands of items. I assume that creating a new object gives better performance than deleting and replacing values within the original object.

I've also put the following code in this JSFiddle: http://jsfiddle.net/mk3cH/2/

var data = [{
  "ID": 20,
  "Title": "Test item",
  "Category": "1;#Local",
  "ContentType": "Item",
  "FileLeafRef": "20;#20_.000",
  "FileRef": "20;#km/Lists/Meny/20_.000",
  "_ModerationStatus": "0",
  "_Level": "1",
  "UniqueId": "20;#{447FEC10-E7E2-4561-A7B3-0433C8C90D31}",
  "FSObjType": "20;#0",
  "PermMask": "0x400001f07fff1bff",
  "Modified": "2013-01-10 11:56:05",
  "owshiddenversion": "8",
  "Created": "2012-04-12 11:53:07"
}, {
  "ID": 27,
  "Title": "Test item 2",
  "ContentType": "Item",
  "FileLeafRef": "27;#27_.000",
  "FileRef": "27;#km/Lists/Meny/27_.000",
  "_ModerationStatus": "0",
  "_Level": "1",
  "UniqueId": "27;#{F7965302-AB39-45AE-9D9E-99BCF39F03B9}",
  "FSObjType": "27;#0",
  "PermMask": "0x400001f07fff1bff",
  "Modified": "2013-04-14 14:00:30",
  "owshiddenversion": "18",
  "Created": "2012-04-20 14:39:02"
}]
var new_object = {}
var keysToIgnore = ['FileLeafRef', 'UniqueId', 'PermMask', 'owshiddenversion', 'Created', '_Level', '_ModerationStatus'].reduce(function (res, key) {
  res[key] = true;
  return res;
}, {});
var object_initiated = false
document.writeln("Desired content of object:
");
for (var i in data) {
  for (var key in data[i]) {
    if (key in keysToIgnore) continue;
    if (key == "ID") {
      new_object[i] = {
        "ID": keydata
      }
    } else {
      var keyname = key;
      var keydata = data[i][key]
      document.writeln(keyname + ": " + keydata + "
");
      new_object[i][keyname] = keydata
    }
  }
}
document.writeln("
Actual new object content
");
document.writeln(JSON.stringify(new_object));

New Object != Better Performance

First of all, your notion of "new object is faster" isn't "right" all the time. This might be trivial for smaller objects, but for thousands of records and possibly nested objects, this isn't right.

Though you do create lesser code by reconstructing a new object, but not done right, you might end up eating more memory.

If you don't dereference the other object properly, it may sit there and just eat up memory. This will give you the "It's dead" page in Chrome.
If you do successfully dereference it, due to the size of the object (like you said, thousands), the GC will take time clearing it from memory, seconds at a time. In this event, there will be noticable "choking" of the browser, and stuttering or worse, freezing of animations. Been there, not good user experience.

Use the almighty server

If you are getting this data from the server, it's better that the server handle this operation. Servers are usually faster than the browser, use it to your advantage. That way, you do less on the browser, and prioritize more important tasks, like giving the user a good UI and fast interaction.

Doing it all client side

Now if you can't do it somewhere else but the server, there are ways to speed it up, or at least not freeze the server.

Web Workers

A modern way of doing it is handing it over to another "thread" by making Web Workers do it.

Keep them small

Keeping your loops small. I usually notice a significant lag when iterations go more than 500 (it's not a magical number, just an observation). Keep your loops small, and avoid that long freeze.

Chunk it up

In addition to keeping the data small, if you have more data to churn than just the 500, consider using timers. Timers with 1ms intervals don't really execute 1ms later. They execute at least 1ms later, and when JS is not busy. You can use that to your advantage and schedule short tasks at intervals without breaking the UI. Of course, the operation becomes asynchronous, so you might want to build a crude callback system for this.

var intervalChurn = setInterval(function(){...loop 100 at a time...},0);

Keep the data flat

Nesting introduces more complexity to the data. Say you have an array of 10 objects. If you loop through that array, it's just 10 iterations. But what if that 10 items have 5 nested properties that you also want to loop through. So thats 5 nested properties times 10 items, giving you 50. Nesting more will introduce more iterations.

Use a library

Some of the bright minds in the development world might have already developed functionality similar to what you are Trying to do. Don't reinvent the wheel. In fact, you are doing something similar to Underscore's omit().


Monkeypatching __builtin__ in tests for mocking I'm testing a little magic behaviour of my script that automatically reads from ~/.ghtoken to do automagic authentication on GitHub API requests. I want to test that this works properly by checking the headers in my mock requests to GitHub's API. I'm using HTTPretty to mock HTTP requests.

GitHub

def test_authentication(monkeypatch):
    import os.path
    import io
    import __builtin__

    auth_token = 'fhqwhgads\n'

    # Err... Just ignore how needlessly complicated this is.
    def monkeypatch_on_path_match(mod, name):
        original = getattr(mod, name)

        def patch(function):
            def patched(path, *args, **kwargs):
                if path == os.path.expanduser('~/.ghtoken'):
                    return function(path, *args, **kwargs)
                return original(path, *args, **kwargs)
            monkeypatch.setattr(mod, name, patched)
        return patch

    # Monkey-Patch exists() to say that the token exists.
    @monkeypatch_on_path_match(os.path, 'exists')
    def intercept_exists(path, *args, **kwargs):
        return True

    # Monkey-Patch open() to return specific file content.
    @monkeypatch_on_path_match(__builtin__, 'open')
    def intercept_open(path, *args, **kwargs):
        return io.BytesIO(auth_token)

    # Enable HTTPretty and register the index path.
    httpretty.enable()

    body = iter(mock_data.abbrev_search_bodies)

    def request_callback(request, uri, headers):
        headers['Content-Type'] = 'application/json; charset=utf-8'
        headers['X-RateLimit-Remaining'] = 10
        headers['Link'] = (
            '<https://api.github.com/search/repositories?'
            'q=language%3Apython&sort=stars&page=1>; rel="last"')
        return 200, headers, next(body)

    httpretty.register_uri(httpretty.GET,
                           "https://api.github.com/search/repositories",
                           body=request_callback)

    # Simply issue the request...
    ghdwn.get_github_list('java')
    assert httpretty.last_request().headers[
        'Authorization'] == 'token fhqwhgads'

    original_exists = os.path.exists

    # Now pretend that file DOES NOT exist!
    @monkeypatch_on_path_match(os.path, 'exists')
    def intercept_exists_fails(path, *args, **kwargs):
        return False

    @monkeypatch_on_path_match(__builtin__, 'open')
    def intercept_open_failure(path, *args, **kwargs):
        raise IOError('Could not find file!')

    # Issue the same request again:
    ghdwn.get_github_list('java')
    assert 'Authorization' not in httpretty.last_request().headers

    # Disable HTTPretty
    httpretty.disable()
    httpretty.reset()

My favourite part is the inexplicable monkeypatch_on_path_match decorator.

In the end, I feel like I did this because I could; not because it ends up testing whether or not my magic "authorize on the existence of a file" actually works.

I'm assuming you're fine with assert, generally I'd recommend using
the unittest
library.

Now for the monkey patching the other choice you have is to restructure
how the credentials are loaded.  At the moment the credentials are
loaded every time a request is performed - instead of that the
credentials could be loaded once, for the whole module as globals,
before invoking any other functionality.  Yes, globals are bad, but
unless you have another layer of indirection it'd be okay for this
purpose.

Done that way, you'd call e.g. ghdwn.load_credentials(), then issue
the call, then ghdwn.drop_credentials() and repeat.

If you stay with the current approach, I'd only move the patch method
out of the test into a proper function, as it's quite reusable, and give
it a better name maybe.

I'm not quite sure if there are other ways to open a file in Python,
regarding your concern whether you're actually testing the right thing;
it might also make sense to test two separate things: the function to
load credentials (from a custom path, so you can load from "/tmp/...")
and calling that function.  Then you wouldn't mock Python standard
functions, but instead only the credential loader, while you'd test the
loading of credentials with an actual test file.


Secure CURL to & handle response from Payment Gateway I'm using a payment gateway and am trying to implement the post and response handling with CURL so that it all happens on one page.

The following is tested and works but I want to double check it's secure. I'm not storing the card details but I'm essentially posting them to myself (via AJAX) and then curling that through to the gateway. I can't AJAX post it directly to the gateway because of cross domain restrictions.

So, this all happens in https://mysite.com/payment.php:

HTML

<form method="post" action="https://mysite.com/payment.php">
    <input type="hidden" name="Amount" value="1.00">

    <!-- fingerprint is a hash of my gateway passwords and fields on this form -->
    <input type="hidden" name="Gateway_Fingerprint" value="<?php echo $fingerprint ?>">
    <input type="hidden" name="Gateway_ReturnURL" value="https://mysite.com/payment.php">
    <input type="hidden" name="Card_Number" value="">
</form>

PHP

// when payment form is submitted
if(isset($_POST['Card_Number'])){

   // CURL the data to payment gateway
   foreach($data as $key => $value){
       $fields .= $key . '=' . $value . '&';
   }

   rtrim($fields, '&');

   $post = curl_init();

   curl_setopt($post, CURLOPT_VERBOSE, true);
   curl_setopt($post, CURLOPT_URL, 'https://gatewayurlhere');
   curl_setopt($post, CURLOPT_POST, count($data));
   curl_setopt($post, CURLOPT_POSTFIELDS, $fields);
   curl_setopt($post, CURLOPT_RETURNTRANSFER, 1);

   $result = curl_exec($post);

   // output result from gateway as JSON
   header('Content-Type: application/json');

   echo $result;

   curl_close($post);

   exit();
}

// result returned from gateway, this will be the CURL result data
if(isset($_POST['restext']))
{
   echo json_encode($_POST);
   exit();
}

Well, security from what type of threat? You need to figure out what you want to protect against before you can protect against it! Since I'm unsure of what you're protecting against, I'll point out relevant setopt flags.

CURLOPT_VERBOSE - This is fine for a developer, but make sure the client doesn't see this returned information. It could lead to a possible exploit. More on the output later on.
CURLOPT_POST - You're using this incorrectly. The value should be true or an alternate HTTP POST.
CURLOPT_RETURNTRANSFER - Using 1 does result in TRUE, but it's easier to read if the explicit boolean is used.

Here are some flags you may way to consider in order to help secure this connection even more. Each can be found in the docs.

bool CURLOPT_FAILONERROR - Just in case the page you fetch is "out of order".
bool CURLOPT_FORBID_REUSE - Force the connection to close on finish, and prevent the reuse of a connection. Also see bool CURLOPT_FRESH_CONNECT.
Make sure you keep bool CURLOPT_SSL_VERIFYPEER set to TRUE.
int CURLOPT_TIMEOUT - Timeout if the connection takes too long.


FizzBuzz in Objective-C In my quest for learning Objective-C, I have created the common FizzBuzz code in it.  This is due to a suggestion/challenge from @nhgrif to re-implement anything he does in Swift in Objective-C.

#import <Foundation/Foundation.h>

int main(int argc, const char* argv[])
{
    @autoreleasepool
    {
        for (int i = 1; i <= 100; ++i)
        {
            if (i % 15 == 0) NSLog(@"FizzBuzz");
            else if (i % 3 == 0) NSLog(@"Fizz");
            else if (i % 5 == 0) NSLog(@"Buzz");
            else NSLog(@"%d", i);
        }
    }
}

I don't like that you've implemented logic in Main. Move the logic into a FizzBuzz method and call that from Main. (Even though it seems really silly for such a simple program, it's not best practice to have it there.)

I'm also not a fan of hard coding 15 as the "FizzBuzz" case. What if we decide that Fizz should be 2 and Buzz should be 5? Now 15 as your first case makes no sense and Mr. Maintainer has to figure out why 15 is a case at all. It's easier to understand and maintain if (i % 3 == 0 && i % 5 == 0) {.

This might be clearer if you consider 15,3, & 5 for what they really are. They're magic numbers and as such should be stored as constants. I don't know Obj-C, so I'll provide a pseudo code example of how I think it should be done.

const int fizz = 3
const int buzz = 5

if (value % fizz == 0 && value % buzz == 0) {
    Print("FizzBuzz")
} else if (value % fizz == 0) {
    Print("Fizz")
} else if (value % buzz == 0) {
    Print("Buzz")
} else {
    Print(value)
}

This allows us to easily update the conditions.

Or alternatively (thanks to Nhgrif and David K)

int fizzbuzz = lcm(fizz,buzz);
for (value = 1; value <= 100; value++) {
    if (value % fizzbuzz == 0) {
        Print("FizzBuzz")
    }
    //etc.
}

Where lcm() is some function that returns the lowest common multiple. At that point we may be overthinking the problem, but it does result in a robust solution.


Substring search on multiple attributes public function getData ($term) {
        $db = Database::getInstance();

        $term = htmlentities($term);

        //Kill the script if the term is blank
        if ($term == "") {
            die();
        }

        $query = "SELECT * FROM coils WHERE name like '%$term%' or
                                            resistance like '%$term%' or
                                            wraps like '%$term%' or
                                            wire_one like '%$term%' or
                                            wire_two like '%$term%' or
                                            wire_three like '%$term%' or
                                            wire_four like '%$term%' or
                                            wire_five like '%$term%' or
                                            wire_six like '%$term%'
                                            LIMIT 25";
        $prepareQuery = $db->getConnection()->prepare($query);
        $result = $prepareQuery->execute();
        $this->rowCount = $prepareQuery->rowCount();

        //Return the Query
        return $prepareQuery;
    }

You should create full text search indexes for these fields and use MATCH ... AGAINST

This way you can change this line to:

$query = "SELECT * FROM coils MATCH ( name, resistance, wraps, wire_1, wire_2, wire_3, wire_4, wire_5, wire_6) AGAINST ('$term') LIMIT 25";

Updated: If you want stick to string substitution when mounting your $query (like you did), make sure you have sanitized your $term variable in order to prevent SQL injection attacks. But it's highly recommended using bind solution nicely expanded below by @greg-burghardt.

More information about MATCH AGAINST can be found here: https://dev.mysql.com/doc/refman/5.7/en/fulltext-natural-language.html


Parallelizing bitmap methods I created a class called QuickBitmap to quickly get/set pixels in a Bitmap object. Using intertops I'm able to safely lock/unlock the bits of the Bitmap and with the use of methods like SaveBits, LoadBits, UpdateUnderlayingBitmap, GetPixel and SetPixel. I can quickly and easily manipulate the image.

The problem:

One of my methods changes the bitdepth to 32 bits. If I use it I'm no longer able to quickly determine if the image I'm manipulating was, originally, composed of only black and white colors.

I created the following methods do it:

private IEnumerable<Color> AllPixels()
{
    for (int i = 0; i < Width;i++ )
    {
        for(int j=0; j<Height; j++)
        {
            yield return GetPixel(i, j);
        }
    }
}

public bool IsBlackAndWhiteParallel()
{
    return AllPixels().AsParallel().All(x => x.IsBlackOrWhite());
}

But as you can guess, it's quite a slow method. So I googled a little and found out that I can break a parallel.for loop.
So I rewrote my method do this:

public bool IsBlackAndWhiteParallel()
{
    bool ret = true;
    Parallel.For(0, Width, (i, loopstate) =>
    {
        for(int j=0; j<Height; j++)
        {
            if(!GetPixel(i,j).IsBlackOrWhite())
            {
                ret = false;
                loopstate.Stop();
            }
        }
    });

    return ret;
}

I benchmarked the code using the benchmark class (I found it in an old question on SO) with 100 iterations:

public static class Benchmarker
{
    public static void Profile(string description, int iterations, Action func)
    {
        // warm up
        func();

        Stopwatch watch = new Stopwatch();

        // clean up
        GC.Collect();
        GC.WaitForPendingFinalizers();
        GC.Collect();

        watch.Start();
        for (int i = 0; i < iterations; i++)
        {
            func();
        }
        watch.Stop();
        Console.Write(description);
        Console.WriteLine(" average time: {0} ms", watch.Elapsed.TotalMilliseconds/iterations);
    }
}

Results:

First code: 1170 ms
Second code: 99 ms

This is how the getpixe/setpixel methods are implemented for 32 bits image:

    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    public Color GetPixel(int x, int y)
    {
        int pos;
        int r, g, b, a;

        pos = (y * stride) + (x * 4);

        b = bytes[pos];
        g = bytes[pos + 1];
        r = bytes[pos + 2];
        a = bytes[pos + 3];

        return Color.FromArgb(a, r, g, b);
    }

    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    public void SetPixel(int x, int y, Color c)
    {
        int pos;

        pos = (y * stride) + (x * 4);

        bytes[pos] = c.B;
        bytes[pos + 1] = c.G;
        bytes[pos + 2] = c.R;
        bytes[pos + 3] = c.A;
    }

I'm satisfied with the result, but I'd like to know if you have any suggestions for improvements.

By using BitConverter.ToInt32 you can simplify the GetPixel() method to

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public Color GetPixel(int x, int y)
{
    return Color.FromArgb(BitConverter.ToInt32(bytes, y * stride + x * 4));
}

Also there is no need to place () arround the multiplications because they will be done first.

Because GetPixel and SetPixel are public you should add some validation to them. This can be done by adding a private method GetPosition() like

private int GetPosition(int x, int y)
{
    if (x > stride) { throw new ArgumentOutOfRangeException("x"); }

    int position = y * stride + x * 4;

    if (position >= bytes.Length) { throw new ArgumentOutOfRangeException("y"); }
    return position;
}

now the changed GetPixel() method looks like

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public Color GetPixel(int x, int y)
{
    return Color.FromArgb(BitConverter.ToInt32(bytes, GetPosition(x, y)));
}

Basically you don't need the Color to determine if a pixel is black or white. You can just check if the argb value of that pixel matches the values for Color.Black or Color.White like

private const int black = -16777216;
private const int white = -1;

public bool IsBlackAndWhite(int x, int y)
{
    int argb = BitConverter.ToInt32(GetPosition(x, y));
    return argb == white || argb == black;
}

You should let your variables some space to breathe.

for(int j=0; j<Height; j++)

should be

for(int j = 0; j < Height; j++)

Your IDE will usually have a keyboard shortcut for formatting the code.

Declaring multiple variables in one line makes your code less readable.


GCD/LCM calculator in x86 NASM assembly After creating a similar program in C++, I've decided to try to write it in x86 assembly language (the type of assembly I was taught in college).

I'd like to receive feedback regarding syntax and clarity.  The macros used here were provided by my college, so I don't think that's a concern.

%include "macros.s"

.DATA

num_lbl:   DB   "> Numbers (2): ", 0
gcd_lbl:   DB   "*** GCD: ", 0
lcm_lbl:   DB   "*** LCM: ", 0

num1:   DD   0
num2:   DD   0

num1_cpy:   DD   0
num2_cpy:   DD   0

gcd:   DD   0
lcm:   DD   0

.CODE
.STARTUP

xor       EAX, EAX
xor       EBX, EBX
xor       ECX, ECX
xor       EDX, EDX
xor       EDI, EDI
xor       ESI, ESI

main:
     nwln
     nwln
     PutStr   num_lbl
     nwln
     nwln
     GetLInt   [num1]
     GetLInt   [num2]

     mov   EAX, [num1]
     mov   [num1_cpy], EAX
     mov   EBX, [num2]
     mov   [num2_cpy], EBX

     call   calc_euclidean
     call   calc_lcm

     nwln
     PutStr   gcd_lbl
     PutLInt   [gcd]
     nwln
     PutStr   lcm_lbl
     PutLInt   [lcm]
     nwln

     .EXIT

calc_euclidean:
               mov   EAX, [num2]
               cmp   EAX, 0
               jne   chk_swap

               mov   EAX, [num1]
               mov   [gcd], EAX

               ret

calc_lcm:
         mov   EAX, [num1_cpy]
         mov   EDX, [num2_cpy]
         mul   EDX

         mov   EDI, EAX

         xor   EBX, EBX

         mov   EDX, EDI
         shr   EDX, 16
         mov   EAX, EDI
         mov   BX, [gcd]
         div   BX

         mov   SI, AX
         mov   [lcm], SI

         ret

chk_swap:
         mov   EAX, [num1]
         mov   EBX, [num2]
         cmp   EAX, EBX
         jl    swap

after_check:
            jmp   loop

swap:
     mov   EAX, [num1]
     mov   EBX, [num2]

     ; temp
     mov   ECX, [num2]

     ; num2 = num1
     ; num1 = temp
     mov   EBX, EAX
     mov   EAX, ECX

     mov   [num1], EAX
     mov   [num2], EBX

     jmp   after_check

loop:
     mov   EDX, [num1]
     shr   EDX, 16
     mov   EAX, [num1]
     mov   BX, [num2]
     div   BX

     mov   EDI, [num1]
     mov   ESI, [num2]
     mov   EDI, ESI
     mov   [num1], EDI
     mov   [num2], EDX

     jmp   calc_euclidean

1) In a language like C or C++, you wouldn't do something like this:

foo           (1,2);
x = bar       (2.5);

It makes it harder to read the code because the function name makes no sense without its parameters. The same is true for assembly. Basically, this:

xor       EAX, EAX

Should be this:

xor EAX, EAX

2) In a language like C or C++, you wouldn't do something like this:

b = a * 5 + 2298;
c = b / 9;

Instead, you might do this:

kelvin_x9 = fahrenheit * 5 + 2298;
kelvin = kelvin_x9 / 9;

In assembly, because you can't give registers descriptive names (in the same way that you can give variables descriptive names in higher level languages) and because often you do things that are semantically wrong (e.g. using shifts when you mean multiplication, using LEA when you're not loading an effective address, etc) it's extremely important for code maintainability to use comments. Basically this:

calc_lcm:
     mov   EAX, [num1_cpy]
     mov   EDX, [num2_cpy]
     mul   EDX

     mov   EDI, EAX

     xor   EBX, EBX

     mov   EDX, EDI
     shr   EDX, 16
     mov   EAX, EDI
     mov   BX, [gcd]
     div   BX

     mov   SI, AX
     mov   [lcm], SI

     ret

Should be this:

calc_lcm:
     mov EAX, [num1_cpy]   ;eax = number1
     mov EDX, [num2_cpy]   ;edx = number2
     mul EDX               ;edx:eax = number1 * number2

     mov EDI, EAX          ;edi = number1 * number2, high 32-bits of number discarded

     xor EBX, EBX

     mov EDX, EDI          ;edx = number1 * number2
     shr EDX, 16           ;edx = number1 * number2 / 65536, dx:ax = number1 * number2
     mov EAX, EDI          ;eax = number1 * number2, (can be deleted as EAX already contained this value)
     mov BX, [gcd]         ;bx = GCD
     div BX                ;ax = number1 * number2 / GCD, dx = number1 * number2 % GCD

     mov SI, AX            ;si = number1 * number2 / GCD
     mov [lcm], SI
     ret

For assembly there's only 2 types of bugs - comments that don't describe sane logic, and code that doesn't do what the comments say it should. This allows you to debug the code effectively and drastically reduces the chance of missing mistakes (in addition to the hopefully obvious massive improvement in code readability).

3) In a language like C or C++, you would create lots of bugs caused by things like integer overflows because the language doesn't provide a sane/easy way to detect these bugs. In assembly you should not create lots of bugs caused by things like integer overflows because you can easily test for overflows.

For example, this:

calc_lcm:
     mov EAX, [num1_cpy]   ;eax = number1
     mov EDX, [num2_cpy]   ;edx = number2
     mul EDX               ;edx:eax = number1 * number2

     mov EDI, EAX          ;edi = number1 * number2, high 32-bits of number discarded

Should be this:

calc_lcm:
     mov EAX, [num1_cpy]   ;eax = number1
     mov EDX, [num2_cpy]   ;edx = number2
     mul EDX               ;edx:eax = number1 * number2

     test EDX, EDX         ;Will the result fit in 32 bits?
     jne .overflow         ; no, error
     mov EDI, EAX          ;edi = number1 * number2

Note: A better idea would be to do "movzx ebx,word [GCD]" and "div ebx" to divide EDX:EAX by the GCD instead of pointlessly converting the 64-bit value you had in EDX:EAX into a 32-bit value in DX:AX.

4) A compiler will check if (e.g.) functions are being called with the correct number of parameters, and will make sure that various registers are saved/restored, etc where necessary. In assembly there is none of that; so you have to do it manually, so you have to be able to do it manually. To be able to do it manually you need to document routines properly. For example, this:

calc_lcm:

Should be this:

;Calculate Lowest Common Multiple
;
;Input:
; [num1_cpy]  Some number
; [num2_cpy]  Some other number
; [GCD]       The GCD
;
;Output:
; [lcm]       The result
;
;Trashed:
; eax, edx, ebx, esi, edi

calc_lcm:

Normally, you'd pass parameters in registers and return parameters in registers to avoid wasting time on loading/storing where avoidable. For example, the entire "calc_lcm" code might be:

;Calculate Lowest Common Multiple
;
;Input:
; eax         Some number
; edx         Some other number
; ebx         The GCD
;
;Output:
; eax         The result
;
;Trashed:
; edx

calc_lcm:
     mul EDX               ;edx:eax = number1 * number2
     div EBX               ;eax = (number1 * number2) / GCD, edx = (number1 * number2) % GCD
     ret

5) If you're writing assembly you should write assembly; and not attempt to obfuscate the code by inventing your own language that makes it difficult read the code and impossible to effectively optimise the code.

For example, this code:

 nwln
 nwln
 PutStr   num_lbl
 nwln
 nwln
 GetLInt   [num1]
 GetLInt   [num2]

Is 100% meaningless gibberish.

I don't know what this code should be (and am completely unable to see ways of optimising it that would have been obvious if it was assembly) because I have no idea what any of these macros do. I can't tell which registers get trashed, I can't see which pushes/pops/moves are redundant. I might be able to assume that it'd be safe to delete the first 2 nwln lines and change the string to num_lbl:   DB   "\n\n> Numbers (2): ", 0 but then that could introduce all sorts of bugs because I didn't bother with the hassle of finding and reading the macros that may do more (or less) than whatever I've assumed they might do.


Simple dispatcher implementation I need a simple dispatcher implementation(invoke methods on one thread from many threads)
once you dispatch method curent thread should wait for results, so I'm thinking about something like this:

public class Dispatcher
{
    private readonly BlockingCollection<Tuple<Delegate, object[]>> runQueue = new BlockingCollection<Tuple<Delegate, object[]>>();
    private readonly BlockingCollection<object> resultQueue = new BlockingCollection<object>();
    private readonly CancellationTokenSource source = new CancellationTokenSource();
    private readonly Task task;
    public Dispatcher()
    {
        Task.Run(() =>
        {
            using (source)
            using (runQueue)
            using (resultQueue)
            {
                Debug.WriteLine("Dispatcher started with thread {0}", Thread.CurrentThread.ManagedThreadId);
                while (!source.IsCancellationRequested)
                {
                    var run = runQueue.Take(source.Token);
                    resultQueue.Add(run.Item1.DynamicInvoke(run.Item2));
                }
                Debug.WriteLine("Dispatcher ended");
            }
        });
    }

    public void Stop()
    {
        source.Cancel();
    }

    [MethodImpl(MethodImplOptions.Synchronized)]
    public object Invoke(Delegate @delegate, params object[] @params)
    {
        runQueue.Add(new Tuple<Delegate, object[]>(@delegate, @params));
        return resultQueue.Take(source.Token);
    }
}

I had test it with such code and it seems too work well ...

class Program
{
    static void Main(string[] args)
    {
        Func<int, int, int> func = (sleep, i) =>
        {
            CC.WriteLine(i, "\tTask {0} will sleep for {1}, thread {2}", i, sleep, Thread.CurrentThread.ManagedThreadId);
            Thread.Sleep(sleep);
            CC.WriteLine(i, "\tTask {0} woke up", i, sleep);
            return i; //return task number
        };
        var rnd = new Random();
        var disp = new Dispatcher();
        {
            var tasks = Enumerable.Range(0, 10).Select(i =>
                            Task.Run(() =>
                            {
                                CC.WriteLine(i, "Task {0} started, thread {1}", i, Thread.CurrentThread.ManagedThreadId);
                                CC.WriteLine(i, "Task {0} ended with {1}", i, disp.Invoke(func, rnd.Next(3, 15) * 100, i));
                            })).ToArray();
            Task.WaitAll(tasks);
            disp.Stop();
        }

        Console.ReadKey();

    }
    static class CC
    {
        public static readonly List<ConsoleColor> COLORS = Enum.GetValues(typeof(ConsoleColor)).Cast<ConsoleColor>().ToList();
        static CC()
        {
            COLORS.RemoveAt(0);
            COLORS.RemoveAt(0);
        }
        public static void WriteLine(int colorIndex, string format, params object[] args)
        {
            lock (COLORS)
            {
                Console.ForegroundColor = COLORS[colorIndex];
                Console.WriteLine(format, args);
            }
        }
    }
}

Is a BlockingCollection a good choice?
Or maybe it can be done with something simpler? (SlimSemaphore)?

Because the task within the Dispatcher is a long running task, you should use the option TaskCreationOptions.LongRunning. Otherwise the operation blocks a thread-pool thread that is usually used for short operations. Alternatively you could also use a Thread.
Instead of using the blocking collection for returning the result, you could use a TaskCompletionSource. That would change the return value of the method Invoke to Task<T>. Therfore the calling code could use the async/await-pattern.
Consider to throw an exception if the Dispatcher was stopped and Invoke is called. Otherwise the call blocks forever, right?


User input and reading contents of file For full disclosure: this is an assignment for my programming class and I just want tips or advice on some of the code.

Assignment details just for background information purposes:

  Write a program that will read a number 1-100 from the user, and the
  name of a data file, and will tell the user what word is in the file
  and how many times the numbers shows up in the data file. Validate
  input number (keep asking until valid) and validate the file was
  successfully open.

  text file contents: Darling 10 20 21 19 20

#include <iostream>
#include <fstream>
#include <string>
#include <cstdlib>  // Needed for exit()
using namespace std;

int main()
{
    ifstream inputFile;
    string fileName;
    int value;

    cout << "Enter the file name: ";
    getline(cin, fileName);

    // Open the file
    inputFile.open(fileName.c_str());

    // Check for successful opening
    if(inputFile.fail())
    {
       cerr << "Error Opening File" << endl;
       exit(1);
    }

    cout << "\nThe file has been successfully opened for reading.\n";

    cout << "Pick a number between 1 through 100. ";
    cin >> value;

    if (value >= 1 && value <= 100)
    {
     cout << value << " is in the acceptable range.\n";
    }
    else
    {
     cout << "Please enter a number from 1 to 100. ";
     cin >> value;
    }

    string word;
    int number;
    int count = 0;

    infile >> number;

    // Read a file until you've reached the end
    while (!inputFile.eof())
    {
        if (number == x)
        {
            sum1++
        }
        infile >> number;
    }
    cout << sum1;

    while (!inputFile.eof())
    {
       inputFile >> word;
        if (word == "input")
        {
            count++;
        }
    }

    // Close the file
    inputFile.close();
    return 0;
}

Any tips or advice would be greatly appreciated since I'm lost when it comes to the fstream portion of this assignment right now.

UPDATED

#include <iostream>
#include <fstream>
#include <string>
#include <cstdlib>  // Needed for exit()
using namespace std;

int main()
{
    ifstream inputFile;
    string fileName;
    int value, x;

    cout << "Enter the file name: ";
    getline(cin, fileName);

    // Open the file
    inputFile.open(fileName.c_str());

    // Check for successful opening
    if(inputFile.fail())
    {
       cerr << "Error Opening File" << endl;
       return(1);
    }

    cout << "Pick a number between 1 through 100. ";
    cin >> value;

    do
{
    cout << "Pick a number between 1 through 100. ";
    int value;
    cin >> value;

} while (value < 1 || value > 100);

    string word;
    int number;
    int count = 0;

    infile >> number;

    // Read a file until you've reached the end
    while (!inputFile.eof())
    {
        if (number == x)
        {
            sum1++
        }
        inputFile >> number;
    }
    cout << sum1;

    while (!inputFile.eof())
    {
       inputFile >> word;
        if (word == "input")
        {
            count++;
        }
    }

    // Close the file
    inputFile.close();
    return 0;
}

Despite what you may have been taught, you should not get into the habit of using using namespace std, especially when you start writing larger programs.
Since you're in main(), you should just return 1 instead of calling exit(1).  The latter is only necessary in other places from which you will need to terminate early.
You don't need to display an output if the file was successfully opened.  You'll know it failed if the program terminates after asking for a filename.
Do not use std::eof() for detecting the end of the file.  It will look for the eof bit after reading the end of the stream (and will not even consider errors in reading (i.e. bad bit)).  Read this for more information.

Instead, read from the file and check for the bad bit using std::getline():

while (std::getline(std::cin, line))
{
    // ...
}

You ask the user for input, but you only validate it once.  If the user enters an invalid value a second time, the program will continue, causing problems.  All of that should be in a loop that will terminate once the user provides valid input.

do
{
    std::cout << "Pick a number between 1 through 100. ";
    std::cin >> value;

} while (value < 1 || value > 100);

You don't need return 0 at the end of main().  Reaching the end already implies a successful termination, so the compiler will do this return for you.


How to use lambdas in "template" I have an event handling code, something like
void handleEventType1(EventType1 event1) {
    if (someCheck()){
        function1(event);
        postCheckTrue();
    } else {
        function2(event);
        postCheckFalse();
    }
}

void handleEventType2(EventType2 event2) {
    if (someCheck()){
        function3(event);
        postCheckTrue();
    } else {
        function4(event);
        postCheckFalse();
    }
}

I was hoping, I can do something like
handleEvent(event1, Handler::function1, Handler::function2);
handleEvent(event2, Handler::function3, Handler::function4);

that will somehow call functions (1 or 2) or (3 or 4), but I'm somehow not able to solve it to my satisfaction.

First, lets use inheritance. We will inherit the specific event types from the event interface.
interface Event {}

class EventType1 implements Event{}

class EventType2 implements Event{}

Secondly, lets define the type of functions that we will call in the handleEvent() method. In this case it is Consumer<Event>
Now our handleEvent() function will look like this
void handleEvent(Event event,
                 Consumer<Event> option1,
                 Consumer<Event> option2) {
   if (someCheck()) {
      option1.accept(event);
   } else {
       option2.accept(event);
   }
}

Now we can call this function and pass instance of event interface and two lambdas or method references
Handler.handleEvent(new EventType1(),
    event -> {
       //do something
    },
    event -> {
       //do something else;
    });
}


Storing recent search phrases I have a search on a very simple spring-powered website and I want to display 10 recent inputs:

import java.util.Collection;
import java.util.Map.Entry;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;

import org.springframework.beans.factory.DisposableBean;
import org.springframework.stereotype.Component;

@Component
public class RecentPhrases implements DisposableBean {
    static final int SIZE = 10;
    private final ExecutorService offeringService = Executors.newFixedThreadPool(1);
    private final ConcurrentMap<String, Long> phrases = new ConcurrentHashMap<>();

    @Override
    public void destroy() {
        offeringService.shutdownNow();
    }

    public void offer(final String phrase) {
        offeringService.execute(() -> {
            phrases.put(phrase, System.currentTimeMillis());
            while (phrases.size() > SIZE) {
                long oldestAge = Long.MAX_VALUE;
                String oldestPhrase = null;
                for (Entry<String, Long> entry : phrases.entrySet()) {
                    if (entry.getValue() < oldestAge) {
                        oldestAge = entry.getValue();
                        oldestPhrase = entry.getKey();
                    }
                }
                phrases.remove(oldestPhrase);
            }
        });
    }

    public Collection<String> getRecent() {
        return phrases.keySet();
    }

}

Is there some simpler solution that will be lightweight enough to not update the underlying collection in a separate thread?

A LinkedHashSet would be simpler:

When the size reaches the limit, simply remove the first entry, no need to iterate
When adding an entry, if it already exists, you'd have to reinsert it to update the order: remove and then add
No more need to worry about the time of insert

Another advantage of this method is that the entries will be naturally ordered. In your current solution they are unordered, which could appear odd to users. This is not a big advantage though, because you could easily sort before returning the collection.


JobTypeLogic and JobType classes in MVC business logic I am working on an MVC solution.  My DAL layer I could solve with Repository classes, everything is working great.

But in my BLL layer I have repetitive code:

My CRUD is the same, my fields and consructor are different.
I can also have some extra methods.

Is there a way to solve this properly?

Class 1

public class JobTypeLogic
{
    #region Fields
    public JobType JobType { get; set; }
    private UnitOfWork unitOfWork = new UnitOfWork();
    public Repository<JobType> JobTypeEngine { get; set; }
    #endregion

    #region Constructor
    public JobTypeLogic()
    {
        JobType = new JobType();
        JobTypeEngine = unitOfWork.Repository<JobType>();
    }
    #endregion

    #region CRUD

    public void Add()
    {
        JobTypeEngine.Add(JobType);
    }

    public JobType Get(long id)
    {
        return JobType = JobTypeEngine.Get(id);
    }

    public void Edit()
    {
        JobTypeEngine.Edit(JobType);
    }

    public void Delete()
    {
        JobTypeEngine.Delete(JobType);
    }

    public List<JobType> List()
    {
        return JobTypeEngine.List.ToList();
    }

    #endregion

}

Class 2

public class JobLogic
{
    #region Fields
    public Job Job { get; set; }
    public IEnumerable<SelectListItem> JobTypeList { get; set; }
    private UnitOfWork unitOfWork = new UnitOfWork();
    public Repository<Job> JobEngine;
    private Repository<JobType> JobTypeEngine;
    #endregion

    #region Constructor
    public JobLogic()
    {
        Job = new Job();
        JobEngine = unitOfWork.Repository<Job>();
        JobTypeEngine = unitOfWork.Repository<JobType>();
        JobTypeList = GetJobTypeList();
    }
    #endregion

    #region CRUD

    public void Add()
    {
        JobEngine.Add(Job);
    }

    public Job Get(long id)
    {
        return Job = JobEngine.Get(id);
    }

    public void Edit()
    {
        JobEngine.Edit(Job);
    }

    public void Delete()
    {
        JobEngine.Delete(Job);
    }

    public List<Job> List()
    {
        return JobEngine.List.ToList();
    }

    #endregion

    #region Methode

    private IEnumerable<SelectListItem> GetJobTypeList()
    {
        JobTypeEngine = unitOfWork.Repository<JobType>();
        var jobs = JobTypeEngine.List
                    .Select(x =>
                            new SelectListItem
                            {
                                Value = x.ID.ToString(),
                                Text = x.Name
                            });

        return new SelectList(jobs, "Value", "Text");
    }

    #endregion

}

This feels bad to me:

private UnitOfWork unitOfWork = new UnitOfWork();

public JobTypeLogic()
{
    JobTypeEngine = unitOfWork.Repository<JobType>();
}

This begs for IoC/DI, otherwise I cannot see how you'd test your code.

JobTypeEngine.List.ToList();: I don't know what's worse, that you've got a property named List, or that it apparently isn't a List at all.

public IEnumerable<SelectListItem> JobTypeList { get; set; }: Does a SelectListItem belong in the BLL? Why isn't that a JobType instead, and why aren't you calling it JobTypes (especially since it is an IEnumerable and not a List)?

GetJobTypeList() doesn't seem to be used. It also again assigns JobTypeEngine even though this already happened in the JobLogic constructor.


Setting and clearing JavaScript intervals depending on browser state I'm trying to build an application which automatically refreshes the page with fresh stuff from the server. I was trying out the approach with something basic like printing out the time, to test out my approach. I go to the page, and it starts printing the time every 5 seconds in the console. When I switch to offline mode, the auto-printing should stop, and when I come back to online mode, the auto-printing should resume again. So far, I have this:

timing = setInterval(function() {
    urlToBeCalled = '/autoupdate/';
    var d = new Date();
    console.log(d.toLocaleTimeString());
    /*$.ajax(url: urlToBeCalled);*/
}, 5000);

window.addEventListener("offline", function(e) { console.log("offline"); clearInterval(timing);});

window.addEventListener("online", function(e) { console.log("online"); timing = setInterval(function() {.... blah... blah... blah...}, 5000)

(The "blah"s are the same code from the timing variable above)

It works, but I don't think this is a very proper way to do things. Is there any better way to accomplish this?

I can think about two ways of doing this.

Using server

You can make your browser work as a 'listener' from the server using the
Push API, its probably the best way, but sometimes you will not have access to make changes in the server.

Using client

Create a run loop.

var interval;
var hasBeenOffline = true;
var loop = function() {
   // this will be to function that will run every X seconds
   if(online) {
      // if the server are online
      if (hasBeenOffline) {
          interval = setInterval(function() { /* code */ } )
      }
   } else {
      // server offline
      clearInterval(interval)
   }
};

setInterval(loop, 1000);

In this way you will have the a better control of the application flow, and will not be required to create the same interval twice in your code.

Some frameworks use a similiar approach, read about the Ember Run Loop to know more about the concept.


First attempt at weighted search My concern is speed, since this may be used in an e-commerce website I want it to be as quick as possible. Where can I improve?

def get_queryset(self):
    search_terms = self.request.GET.getlist('search', None)

    if not search_terms:
        return []

    # removing trailing slash on restangular calls
    search_terms[0] = search_terms[0].lower().replace('/', '')

    terms = [term.split(" ") for term in search_terms][0]

    # Query that will go through each item to see if their name or description match contain the search terms
    results = reduce(operator.or_,
                     (Item.objects.filter
                      (Q(name__icontains=term) | Q(description__icontains=term))
                      for term in terms))

    # Using enumerate so I can get the index, storing index at end of list for future reference
    # Concats the item name and the item description into one list, using that for the items weight in the result
    results_split = [list(set(item.name.lower().split() + item.description.lower().split() + list((index,))))
                     for index, item in enumerate(results)]

    # Builds weight for each term
    # Example: The search has 3 terms, Red, Shoes, Pants
    # Red would have a weight of 3 since it is the first word, shoes would be 2 and pants would be 1
    query_with_weights = [(term, len(search_terms[0].split()) - search_terms[0].split().index(term))
                          for term in terms]

    # This section will go through and weigh each item based on name and description weight.
    # This may be problematic if the description uses the key word multiple times.
    #  It could result in things being weighed incorrectly. See the example below for more details.
    # Example 2: 'red pants' is the query.
    # We have, in no particular order, a red shoe item, a blue pants item, a red pants item, a red swim trunks item.
    # Each items description is sweet {{ item.name }} bro
    # The resulting weight would be Red: 2, Pants: 1
    # However, the returned result would be, in this order, [Red Pants, Red Shoe, Red Swim Trunks, Blue Pants]
    get_weight = lambda x: ([weight for y, weight in query_with_weights if y == x] or [0])[0]
    sorted_results = sorted([(item, sum([(get_weight(term)) for term in item])) for item in results_split],
                            key=lambda lst: lst[1], reverse=True)

    # Using the index stored from before I am able to access the original results list in order and
    #  create a new list that is now sorted based on the weight of each item in the search.
    # I am planning to expand this purely for educational purposes to include tags in the weighing and filtering process.
    all_results = [results[result[0][result[0].index(term)]] for result in sorted_results for term in result[0] if
                    type(term) is int]

    # Gets the top level item for each sub item that hit in the results.
    #   If multiple sub items FK back to the a single master item we want to only display that master item once.
    top_level_sorted = list(set([Item.objects.get(id=item.get_top_level_item().id)
                                    for item in all_results if item.is_variant]))
    return top_level_sorted

I agree with @Gareth-Rees, so I'm going to comment on the comments as
well:  Most of them are fine if not a bit too detailed.  It could make
sense to move them into docstrings and separate functions if there
really is that much to say about it.  I kind of expected the whole
function to have a docstring anyway, since at the moment neither input
nor output is really documented.

In general, if you care about performance it'd be best to use a profiler
to find problematic parts.  That said the construction of all these
lists is probably not the best thing to do.  E.g. results_split is
only iterated over once.  Another general suggestion would be to avoid
linear search and rather use dictionaries for faster lookups.

The construction of terms is confusing.  If the only value used is
the first one, then just use that all the time.  Also, is the split(" ") significant, or could just split() be used as well (since it will remove any whitespace instead of just a single space, c.f. the docs)?

  str.split([sep[, maxsplit]])

  If sep is not specified or is None, a different splitting algorithm is applied: runs of consecutive whitespace are regarded as a single separator, and the result will contain no empty strings at the start or end if the string has leading or trailing whitespace.

I'm going to assume so.
The None default value in getlist is probably not necessary, as
the empty list already counts as a false value.
The definition of get_weight should be done with def as well.
Additionally, avoiding the list creation by using a normal loop may be
faster.
results can probably be calculated more efficiently by combining all
the queries instead of combining all the results?  In any case I'd
move this into a separate method.
list((index,)) is a very verbose way of saying [index].
The comment above results_split is wrong insofar as the index is
added to the set, thus isn't necessarily at the end of the resulting
list.  Changing the parenthesis slightly would fix this - however it
doesn't seem like the index is used anywhere else, so both adding the
index, as well as using enumerate aren't necessary.
search_terms[0] is used more than once, so it makes sense to store
the calculated values and reuse them.
The sort method can be used instead of sorted to avoid additional
allocations.
top_level_sorted sounds like it does something different than what
the code says, as there is nothing preventing an unsorted list in
those lines.  Also, it would be better to fetch all objects at once by
ID instead of fetching all of them and then removing duplicates with
the set.
all_results is bad.  It's searching for the index value in a list
that was explicitely constructed with that number at the end, so it
should be rewritten to make use of that fact.  Honestly I can't really
follow the logic here, I'll leave it out in the code below.

I tried to rewrite this a bit according to those points, however there's
a trade-off between readability and faster code.  Best thing would be to
merge all the steps that operate on the same lists one after another and
extract functionality into separate functions to make up for the lack of
readability.

def get_matching_items(self, term):
    return Item.objects.filter(Q(name__icontains=term) | Q(description__icontains=term))

def get_queryset(self):
    search_terms = self.request.GET.getlist('search')

    if not search_terms:
        return []

    terms = search_terms[0].lower().replace('/', '').split()
    terms_length = len(terms)

    # Builds weight for each term
    # Example: The search has 3 terms, Red, Shoes, Pants
    # Red would have a weight of 3 since it is the first word, shoes would be 2 and pants would be 1
    query_with_weights = {term: terms_length - index for index, term in enumerate(terms)}

    # Query that will go through each item to see if their name or description match contain the search terms
    results = get_matching_items(terms[0])
    for term in terms[1:]:
        results |= get_matching_items(term)

    # This section will go through and weigh each item based on name and description weight.
    # This may be problematic if the description uses the key word multiple times.
    #  It could result in things being weighed incorrectly. See the example below for more details.
    # Example 2: 'red pants' is the query.
    # We have, in no particular order, a red shoe item, a blue pants item, a red pants item, a red swim trunks item.
    # Each items description is sweet {{ item.name }} bro
    # The resulting weight would be Red: 2, Pants: 1
    # However, the returned result would be, in this order, [Red Pants, Red Shoe, Red Swim Trunks, Blue Pants]
    sorted_results = []
    for item in results:
        terms = set(item.name.lower().split())
        terms.update(item.description.lower().split())

        weighted_sum = sum(query_with_weights.get(y, 0) for term in terms)

        sorted_results.append((item, weighted_sum))

    sorted_results.sort(key=lambda lst: lst[1], reverse=True)

    ...

    # Gets the top level item for each sub item that hit in the results.
    #   If multiple sub items FK back to the a single master item we want to only display that master item once.
    top_level_ids = set()
    top_level_sorted = []
    for item in all_results:
        if not item.is_variant:
            continue
        top_level_id = item.get_top_level_item().id
        if top_level_id in top_level_ids:
            continue
        top_level_item = Item.objects.get(id=top_level_id)
        top_level_ids.add(top_level_id)
        top_level_sorted.append(top_level_item)

    return top_level_sorted


Efficient way of reading in list of string csvs' into lists I have a list of strings. Each string is a comma separated list of numbers. I want to quickly convert each of these strings into a list. So far I have done the following:

My Solution So Far

def convertCommaSeperatedStringToList(data):

    s_lists = []

    for s in data:
        s_list =  [int(x) for x in s.split(',')]
        s_lists.append(s_list)

Example Dataset

# Here we have 600 very long lists to process
import numpy as np

data = []

for x in range(0,600):
    a = (np.random.randint(1000,size=100000)).tolist()
    b = ','.join(str(x) for x in a)
    data.append(b)

Just some background if you're wondering why I would have such a wierd list, I am reading this list from a downstream database.

Fastest way would be to use a list-comprehension here. This would avoid the repetitive lookups to list.append and will use the LIST_APPEND byte code.

s_lists = [[int(x) for x in row.split(',')] for row in data]

Note that for function names underscores are preferred over camel-case.


Solution to Chef and Squares challenge, timing out in Java but not in C++ I was solving a problem on codechef (online judge and contest organizer). I was writing my code in Java and was getting TLE at last testcase where as I wrote the same code in C++ and it was accepted. Now I know C++ is faster then java but If anyone could help me optimize my Java code ?

Here is the question

  Chef has finished his freshman year in college. As a present, his
  parents gave him a new problem to solve: Chef has to fill a K x K
  square grid of integers in a certain way. Let us say that such a grid
  is valid if:

  Each cell contains an integer from 1 and K (inclusive).
  No integer appears twice in the same row or the same column.

  Let F(K) be maximum possible distance between the center of the square and the closest
  cell that contains 1, among all possible squares with the side length
  K.

  Here, we use the following notions:

  The distance between cell (x, y) and (i, j) is equal to |x-i|+|y-j|.
  The center of a K × K square is cell ((K+1)/2, (K+1)/2) for odd K.

  Input

  The first line of input contains a single integer T denoting the
  number of test cases. Each test case consists of a single line
  containing a single odd integer K.

  Output

  For each test case, print K lines each consisting of K space separated
  integers giving some square grid where the distance from the center of
  the grid to the nearest 1 is exactly F(K). If there's more than 1
  possible answer output any of them.

  Constraints

  Ki is odd.

  Subtask #1 (10 points):

  1 ≤ T ≤ 50
  1 ≤ Ki ≤ 5

  Subtask #1 (90 points):

  1 ≤ T ≤ 10
  1 ≤ Ki < 400

  Example

  Input:

2
1
3

  Output:

1
3 2 1
1 3 2
2 1 3

Here is my code in Java

import java.util.*;
import java.io.*;

public class Main {
    public static void main(String[] args) throws IOException{
        BufferedReader sc = new BufferedReader(new InputStreamReader(System.in));
        int t = Integer.parseInt(sc.readLine());
        while(t-- > 0){
            int n = Integer.parseInt(sc.readLine());
            int center = (n + 1)/2 ;
            for(int i = 0; i < n; i++){
                int temp = center;
                for(int j = 0; j < n; j++){
                    System.out.print(temp+" ");
                    temp++;
                    if(temp > n)
                        temp = 1;
                }
                center--;
                if(center < 1)
                    center = n;
                System.out.printf("%n");
            }
        }
    }
}

Here is my code in c++

#include <iostream>
using namespace std;

int main(void){
    int t;
    cin>> t;
    while(t--){
        int n;
        cin>>n;
        int center = (n + 1)/2 ;
        for(int i = 0; i < n; i++){
            int temp = center;
            for(int j = 0; j < n; j++){
                cout<<temp<<" ";
                temp++;
                if(temp > n)
                    temp = 1;
            }
            center--;
            if(center < 1)
                center = n;
            cout<<"\n";
        }
   }
   return 0;
}

My first observation was the same as @EmilyL.'s: that you're performing unneeded string concatenations.  Upon investigation, however, it turned out to be a loser to substitute two invocations of System.out.print() methods for a string concatenation plus one method invocation -- the result ran about 70% slower than the original code for me.

It took a while for the lightbulb to turn on, but that slowdown is a key symptom of the underlying problem: System.out is unbuffered, at least when it's not connected to a terminal.  In a comment you described addressing the problem by building the output in a StringBuilder and then printing it all at once.  That's a viable mechanism for buffering manually, but cleaner and easier to integrate into your original solution would have been to wrap a buffered stream around System.out and print to that.  In other words, at the top of main() add ...

PrintStream out = new PrintStream(new BufferedOutputStream(System.out));

... and everywhere else print to out instead of to System.out().  Doing that cut my run time by more than 50% relative to the original code, for a maximal-size test set.


Best way to pass parameters to Factory class? So I have a series of objects, which I will call Impl1, Impl2, and Impl3.  They each implement an interface, called IImpl.  I have a Factory class who's task is to retrieve the ImplX which is appropriate for a given circumstance, and pass it on to its callers.  So the code in the factory looks like:

public IImpl GetInstance(params object[] args)
{
    if (args[0]=="Some Value")
         return new IImpl1();
    else if (args[0]=="Other Value")
         return new IImpl2(args[1]);
    else
         return new IImpl3(args[1]);
}

So depending on the arguments passed in, different instances are selected.  All well and good and works ok.  The problem now is, that I have a class which needs to call this factory method.  It has no references to IImplX, which is good, but it winds up having to know exactly how to construct the input array to GetInstance, in order to ensure it receives the correct kind of instance.  Code winds up looking like:

switch (_selectedInputEnum)
{
    case InputType.A:
        myIImplInst = Factory.GetInstance("Some Value");
    case InputType.B:
        myIImplInst = Factory.GetInstance("Other Value",this.CollectionB);
    case InputType.C:
        myIImplInst = Factory.GetInstance("Third Value",this.CollectionC);
}

This feels very redundant, and off somehow.  What would be the best way to abstract the actual parameters of the factory?  I feel like with the above switch statement, I am strongly coupled to the implemenations of IImplx, even if I don't have a direct reference.

How about some kind of intermediary blackboard that the client code constructs before calling the factory, and each concrete Impl can poll to construct itself?

// client code:

Blackboard blackboard = new Blackboard();
blackboard.pushCollectionB(collectionB);
blackboard.pushCollectionC(collectionC);
blackboard.pushFoo(foo);

IImpl myInstance = Factory.GetInstance(b);

///////////////////////////

// in Factory.GetInstance():

return new Impl3(blackboard);

////////////////////////////

// in Impl3:

Impl3(Blackboard b) { process(b.getCollectionC()); }

I've hidden the switch statement in the client code, but you could move that into the blackboard as well.

What data each concrete Impl needs is now hidden from both the Factory and the client code. However if you need more data in your Blackboard for Impl(x+1) you will need to update every place in your code that creates a Blackboard.

Depending on application, over-constructing the Blackboard like this may be expensive for you. You could construct a cached version at startup, or you could make the Blackboard an interface and have your client code derive from it.


Randomizing all class variables in constructor c++ I'm playing around with making random items and decided to implement the following structure.

#include <stdlib.h> //uints
#include <limits>
#include <random>
#include <iostream>

class RandomItem {
private:
    struct _physical_properties { uint16_t length; uint16_t width; uint16_t sqew; uint16_t weight; };
    struct _color { uint8_t r; uint8_t g; uint8_t b; uint8_t a;};
    //etc
public:
    _physical_properties physical;
    _color main_color;
    _color secondary_color;
    uint16_t durability;
    uint32_t type;

    RandomItem() = default;
    RandomItem(uint32_t _type) {
        union thing {
            RandomItem item;
            uint8_t buf[sizeof(RandomItem) / sizeof(uint8_t) ];
        };
        thing bad{};
        std::random_device dev;
        std::mt19937 gen(dev());
        std::uniform_int_distribution<> dist(0, std::numeric_limits<uint8_t>::max());
        for (auto& b : bad.buf) {
            b = static_cast<uint8_t>(dist(gen));
        }
        *this = bad.item;
        this->type = _type;
    }
};

int main() {
    RandomItem a(1);
    RandomItem b(2);
    std::cout << "item type: " << a.type << " Some values: " << (int)a.physical.length << ", " << a.durability << std::endl;
    std::cout << "item type: " << b.type << " Some values: " << (int)b.physical.length << ", " << b.durability << std::endl;
    return 0;
}

(edited slightly to post here in a more minimal form...using iostream instead of fmt sticking in one file etc)

seeing as how:

The total number of properties in the class can increase and have (potentially) different types
I don't particularly feel like

prop1 = dist(gen);
prop2 = dist(gen);
prop3 = dist(gen);
//etc

is very scalable...

I wrote the displayed constructor and that's specifically what I'm asking for feedback on (rest of the dummy class is just for the example)

It uses a union between the item and a buffer and fills that buffer with random values then sets the current object equal to the unionized item.

This feels hacky however might be better than alternatives (individually setting each variable manually or the other end of the gross hack scale making a macro that expands to setting each variable)

Are there any major gotchas with doing this?

Am I relying on undefined behavior and or will this style of code introduce bugs down the line? (as far as I can see it doesn't quite matter what the endianness of the computer is 'cause the value is random)

Is this the best way to implement my goal? (set all the variables and nested variables to random values on construction).

I'm posting here instead of StackOverflow because it appears to work (compiles and  and I assume any answers would just be opinion-based.
I'm not working with other people and for now, I'm just experimenting with things. (this isn't being used in a project but I might add it into one in the future.).

Is it "better" to make the class nothing but POD without a constructor and then implement a function that does pretty much the same code and return the constructed item? (something like)

Item& randomItem(uint32_t type){
    union thing{
        Item item;
    //snip
    ...
    return item;
}

To avoid the hacky *this = item; code (smell?)

Of course, if I was actually writing a game or similar program that implemented a class of random variables I'd probably have some constraints for what the random values for different types of items could be. (For example, length/width might be a distribution between (300-1000) instead of (0-maxValueOfType) so maybe this is just a contrived example. However, if anyone has any feedback on the rest of the concept I'd be interested.

In theory, I think this is allowed because your Items are trivial types, but this is pretty ugly.

I think you can do better using std::array.

struct _color { uint8_t r; uint8_t g; uint8_t b; uint8_t a;};

could be

struct _color {
    auto& r() { return data[0]; }
    // etc
    void randomize() { /* fill data with random stuff */ }
    private:
        std::array<uint8_t, 4> data;
};

P.S. I think you will find that uniformly randomly setting r, g, b, and a doesn't look random to humans.

P.P.S.  There is a proposal to add something like this:

_color color;
for... (auto& component : std::meta::members(color)) {
    // do stuff with component
}

I haven't followed this proposal but it sounds like it would help here.


Compact game of life implementation I am learning F# and implemented Conway's Game of Life as an exercise (There are also other implementations on CodeReview). I have tried to forget all my object-oriented background and practice "functional thinking".

Any feedback is welcome, especially related to alternative (more functional) solutions.

open System

[<EntryPoint>]
let main argv =

    let rows = 30
    let columns = 30
    let size = rows*columns

    let combine listX listY = [
        for x in listX do
        for y in listY do
        yield x, y ]

    let createRandomBoard =
        let rnd = new Random(10)
        [1..size]
        |> List.map (fun x -> if rnd.NextDouble() < 0.2 then 1 else 0)
        |> List.toArray

    let board = createRandomBoard

    let toBoardLine (line : int[]) : string =
        line |> Array.fold (fun agg i -> agg + (if i = 1 then "X" else ".")) ""

    let drawBoard () =
        [0..rows-1]
        |> List.iter (fun y -> printfn "%A" (board.[y*rows..y*rows+columns-1] |> toBoardLine))

    let indexToCoordinates index = (index % columns + 1, index / columns + 1)
    let coordinatesToIndex (x, y) = (y - 1) * columns + (x - 1)

    let getLivingNeighboursCount idx =
        let x, y = indexToCoordinates idx
        let (minX, maxX) = (if x = 1 then columns else x - 1), (if x = columns then 1 else x + 1)
        let (minY, maxY) = (if y = 1 then rows else y - 1), (if y = rows then 1 else y + 1)
        combine [minX; x; maxX] [minY; y; maxY]
        |> List.filter (fun com -> com <> (x,y))
        |> List.map coordinatesToIndex
        |> List.map (fun x -> board.[x])
        |> List.sum

    let indexToNewState idx =
        let state = board.[idx]
        let livingNeigbours = getLivingNeighboursCount idx
        (if livingNeigbours = 3 || (livingNeigbours = 2 && state = 1) then 1 else 0)

    let updateState () =
        [0..size-1]
        |> List.iter (fun idx -> (Array.set board idx (indexToNewState idx)))

    while true do
        Console.Clear();
        updateState ()
        drawBoard ()
        System.Threading.Thread.Sleep(200);

    Console.ReadLine()
    0

Some remarks :

your combine function is in fact List.zip, you might want to explore the Array and List sections of the standard library, they are full of very useful functions that will speed-up your development.
createRandomBoard is not a function and thus will always be the same array (arrays being mutable, you might come across some unwanted side effects if you use it twice in the same code). It could be rewritten using Array.init :

let createRandomBoard () =
   let rnd = new Random(10)
   Array.init size (fun _ -> if rnd.NextDouble() < 0.2 then 1 else 0)

you are using '0' and '1' to model your two states, why not booleans ? It would eliminate the need for some tests.
Array.set ar ind x can be rewritten as ar.[ind] <- x (but, as Foggy Finder said, you would lose some type inference if ar has not been identified as an array before)
forms such as :

[1..n] |> List.iter f

are indeed functional but inefficient and, I believe, not as readable as using a loop. F# is especially powerful because it is a hybrid language (and not a purely functional one) : don't hesitate to leverage arrays and loops when it makes sense.


Printing stars using nested loops I just got back from my interview where I had to solve the following problem:

  Write a C++ program that prints the following pattern using nested for loops.

*
***
*****
***
*

Here is my solution:

int inc = 2;
for (int i = 1; i >= 1; i += inc){
    for (int j = 0; j < i; j++ )
        std::cout<<'*';
    std::cout<<endl;
    if ( i == 5 )
        inc = -2;
}

I'm just curious to know if there are better solutions (any criteria for better) or just different approaches.

Edit Oh well, not using nested for... Sorry about that

int inc = 2;
for (int i = 1; i >= 1; i += inc){
    std::cout<<std::string(i,'*')<<endl;
    if ( i == 5 )
        inc = -2;
}


Project Euler 78: Coin Partitions in Python I was trying to solve Project Euler Problem 78: Coin Partitions.

  Let p(n) represent the number of different ways in which n coins can be separated into piles. For example, five coins can be separated into piles in exactly seven different ways, so p(5)=7. Find the least value of n for which p(n) is divisible by one million.

My solution is in Python. It uses the formula described here in the Wikipedia article, a simple formula really. It takes around 12 seconds, but ideally I'd get it around two or three seconds if not quicker.

Here's my code. I would appreciate any comments or help people could offer.

from timeit import default_timer as timer

start = timer()
partitions = [1, 1, 2, 3, 5, 7, 11, 15, 22, 30, 42]
n = 11
partition = 42
pentagonals = []

for k in range(1, 2000):
    pentagonals.append(k*(3*k-1)/2)
    k *= -1
    pentagonals.append(k*(3*k-1)/2)

while partition != 0:
    partition = 0
    pentagonal = 0
    index = 0
    temp = 0
    is_positive = False
    while pentagonal <= n:
       if is_positive: partition += temp
       else: partition -= temp
       pentagonal = pentagonals[index]
       temp = partitions[n - pentagonal]
       if index % 2 == 0: is_positive = not is_positive
       index += 1
    partition %= 1000000
    partitions.append(partition)
    n += 1

ans = n - 1
elapsed_time = (timer() - start) * 1000
print "Found %d as the answer in %d ms." % (ans, elapsed_time)

1. Debugging

The first version of your code was broken because you used the expression

(-1)**(k-1)

where k is sometimes negative. The documentation says:

  For int operands, the result has the same type as the operands unless the second argument is negative; in that case, all arguments are converted to float and a float result is delivered.

(my emphasis) so when k was negative, this expression evaluated to a float, either 1.0 or -1.0, and then the usual rules of arithmetic conversion caused all the other intermediate results to become floats too. Floats have limited precision, so eventually these were approximate rather than exact.

How did I discover this? Well, I ran the program under the Python debugger:

$ python -m pdb cr93311.py
> cr93311.py(1)<module>()
-> partitions = [1, 1, 2, 3, 5, 7, 11, 15, 22, 30, 42]

I wanted to see what values actually showed up in the partitions list, so I set a breakpoint on line 17 (just after the partitions.append):

(Pdb) break 17
Breakpoint 1 at cr93311.py:17

And then I continued to the breakpoint and printed the partitions array:

(Pdb) continue
> cr93311.py(17)<module>()
-> n += 1
(Pdb) p partitions
[1, 1, 2, 3, 5, 7, 11, 15, 22, 30, 42, 56.0]

The 56.0 stands out immediately as an error: it should be 56. (Possibly this was not obvious to you: in which case you might appreciate David Goldberg's article "What Every Computer Scientist Should Know About Floating-Point Arithmetic" or the beginner's version thereof.)

How did this floating-point number get there? Let's find out by restarting the program and setting a breakpoint on line 14, just after the computation of pentagonal and temp:

(Pdb) run
Restarting cr93311.py with arguments:
        cr93311.py
> cr93311.py(1)<module>()
-> partitions = [1, 1, 2, 3, 5, 7, 11, 15, 22, 30, 42]
(Pdb) break 14
Breakpoint 2 at cr93311.py:14
(Pdb) continue
> cr93311.py(14)<module>()
-> if k > 0: k *= -1
(Pdb) p pentagonal, temp
(1, 42)

Nothing wrong there; let's try the next iteration of the loop:

(Pdb) continue
> cr93311.py(14)<module>()
-> if k > 0: k *= -1
(Pdb) p pentagonal, temp
(2, 30.0)

So it's the computation of temp that wrongly converted to float. Let's check the sub-expressions of that computation:

(Pdb) p k, (-1)**(k - 1), pentagonal, partitions[n - pentagonal]
(-1, 1.0, 2, 30)

So it's the (-1)**(k - 1) that's the culprit.

2. Review

It is easier to test and measure code if it is organized into functions. Functions can be tested and timed independently of each other, making it possible to isolate an error or a performance problem.
Functions should be given docstrings describing what they do, so that it's possible to check the implementation against the documentation.
When a function produces a sequence of results it's usually convenient in Python to generate those results using the yield statement. That way, we don't have to commit in advance as to how many results we are going to compute (as we would if the function returned a list), and we don't have to store all the results in memory at once.
It is easier to test functions if they have adjustable parameters (instead of fixed constants like 1000000) so that we can run and check them on small inputs.

So I would break up the code into functions, like this:

def pentagonal():
    """Generate the generalized pentagonal numbers."""

def partitions_modulo(m):
    """Generate the partition numbers modulo m."""

def problem78(m=10**6):
    """Return the least value of n for which the partition number p(n) is
    divisible by m."""

This makes the code easy to test on small instances:

>>> from itertools import islice
>>> list(islice(pentagonal(), 16))
[1, 2, 5, 7, 12, 15, 22, 26, 35, 40, 51, 57, 70, 77, 92, 100]
>>> list(islice(partitions_modulo(100), 16))
[1, 1, 2, 3, 5, 7, 11, 15, 22, 30, 42, 56, 77, 1, 35, 76]
>>> problem78(23)
32

and easy to measure:

>>> from timeit import timeit
>>> timeit(problem78, number=1)
9.393578507006168

3. Performance

Your best bet is to use PyPy — this kind of simple numerical code is where PyPy really shines. I get more than ten times speedup simply from running the code in the post under PyPy.

If you're determined to stick with CPython, then here are some techniques I found that were helpful:

Only generating as many generalized pentagonal numbers as necessary: it turns out that we only need the first 200 or so, not the first 2000.
The generalized pentagonal numbers are $$ 0, 1, 2, 5, 7, 12, 15, 22, 26, 35, 40, 51, 57, 70, \dotsc $$ If we take the differences between consective elements, we get the sequence $$ 1, 1, 3, 2, 5, 3, 7, 4, 9, 5, 11, 6, 13, \dotsc $$ whose \$n\$th item is \$n\$ if \$n\$ is odd, or \$n\over 2\$ if \$n\$ is even. This means that the generalized pentagonal numbers can be generated using itertools.count and itertools.accumulate, like this:

accumulate(k if k % 2 else k // 2 for k in count(1))

The signs repeat in the cycle $$ 1, 1, -1, -1, 1, 1, -1, -1, \dotsc $$ and so they can be generated using itertools.cycle, like this:

cycle((1, 1, -1, -1))

Using sum instead of explicit accumulation.

But the revised code is only about twice as fast as the original:

from itertools import accumulate, count, cycle, islice

def pentagonal():
    """Generate the generalized pentagonal numbers starting at 1.

    >>> list(islice(pentagonal(), 16)) # http://oeis.org/A001318
    [1, 2, 5, 7, 12, 15, 22, 26, 35, 40, 51, 57, 70, 77, 92, 100]

    """
    return accumulate(k if k % 2 else k // 2 for k in count(1))

def partitions_modulo(m):
    """Generate the partition numbers modulo m.

    >>> list(islice(partitions_modulo(1000), 16)) # http://oeis.org/A000041
    [1, 1, 2, 3, 5, 7, 11, 15, 22, 30, 42, 56, 77, 101, 135, 176]

    """
    signs = 1, 1, -1, -1
    partitions = [1]          # Partition numbers up to p(n)
    pentagonals = []          # Generalized pentagonal numbers <= n
    pentagonal_it = pentagonal() # Iterator over generalized pentagonal numbers
    next_pentagonal = next(pentagonal_it)

    yield 1
    for n in count(1):
        if next_pentagonal <= n:
            pentagonals.append(next_pentagonal)
            next_pentagonal = next(pentagonal_it)
        p = sum(sign * partitions[-pent]
                for sign, pent in zip(cycle(signs), pentagonals)) % m
        partitions.append(p)
        yield p

def problem78(m=10**6):
    """Return the least value of n for which the partition number p(n) is
    divisible by m.

    >>> problem78(23)
    32

    """
    return next(i for i, p in enumerate(partitions_modulo(m)) if p % m == 0)


Docstring generator in Google type Hello everyone!
In this task I had to create docstring generator. I'm just wondering how you see this problem, I have to "take data" from the dictionary
{'Args': None, 'Returns': None, 'Raises': None, 'Attributes': None, 'Summary': None, 'Description': None}

and then generate docstring in the correct order.
This is my solution:
import textwrap

def gendoc(config):
    docstring = []
    if ("Summary" in config) and (config["Summary"] is not None):
        docstring.append(config["Summary"])
    if ("Args" in config) and (config["Args"] is not None):
        args = ["Args:"] + [f"\t{arg}: Function argument" for arg in config["Args"]]
        docstring += args
    if ("Attributes" in config) and (config["Attributes"] is not None):
        attr = ["Attributes:"] + [f"\t{attr}: Information about parameter {attr}" for attr in config["Attributes"]]
        docstring += attr
    if ("Raises" in config) and (config["Raises"] is not None):
        raises = ["Raises: "] + [f"\t {config.get('Raises')}"]
        docstring += raises
    if ("Returns" in config) and (config["Returns"] is not None):
        returns = ["Returns: "] + [f"\t {config.get('Returns')}"]
        docstring += returns
    if ("Description" in config) and (config["Description"] is not None):
        value = config.get('Description')
        wrapper=textwrap.TextWrapper(subsequent_indent= '\t', width = 120)
        description = ["Description: "] + [f"\t{wrapper.fill(value)}"]
        docstring += description
    if ("Todo" in config) and (config["Todo"] is not None):
        star = "*"
        todo = ["Todo:"] + [f"\t{star} {todo}" for todo in config["Todo"]]
        docstring += todo

    docstring = "\n\n".join(docstring)
    return docstring

My output:
The Pear object describes the properties of pears.

Args:

    a: Function argument

    b: Function argument

Attributes:

    a: Information about parameter a

    b: Information about parameter b

Raises:

     AttributeError: The ``Raises`` section is a list of all exceptions that are relevant to the interface.

Returns:

     The return value. True for success, False otherwise.

Description:

    The description may span multiple lines. Following lines should be indented. The type is optional. The description may
    span multiple lines. Following lines should be indented. The type is optional.The description may span multiple lines.
    Following lines should be indented. The type is optional.The description may span multiple lines. Following lines
    should be indented. The type is optional.

Todo:

    * Do something

    * Something else

Any tips how to make this SHORTER and more neat, closer to a advanced or just better solution will be definitely on point. Thanks in advance!
Have a great day!

if ("Summary" in config) and (config["Summary"] is not None):

Can be simplified to:
if config.get("summary") is not None:

args = ["Args:"] + [f"\t{arg}: Function argument" for arg in config["Args"]]
attr = ["Attributes:"] + [f"\t{attr}: Information about parameter {attr}" for attr in config["Attributes"]]
todo = ["Todo:"] + [f"\t{star} {todo}" for todo in config["Todo"]]

Can all be moved into a function.

You always add f"{Name}:",
You always iterate through config[Name], and
You always format the value. Which can be provided as a format string (not an f-string).

def format_values(config, name, fmt):
    return [f"{name}:"] + [fmt.format(value) for value in config[name]]

format_values(config, "Args", "\t{}: Function argument")

docstring.append(config["Summary"])
raises = ["Raises: "] + [f"\t {config.get('Raises')}"]
returns = ["Returns: "] + [f"\t {config.get('Returns')}"]

Can all be moved into a function.

You add f"{Name}: " except for Summary,
You always add config[Name], and
You always format the value. (Summary would be "{}")

def format_value(config, name, fmt, head=True):
    return ([f"{name}: "] if head else []) + [fmt.format(config[name])]

format_value(config, "Summary", "{}", False)

value = config.get('Description')
wrapper=textwrap.TextWrapper(subsequent_indent= '\t', width = 120)
description = ["Description: "] + [f"\t{wrapper.fill(value)}"]
docstring += description

To be able to handle Description we can change format_value (and optionally format_values for consistency) to take a lambda to decide how to display the value.
def format_value(config, name, fmt, head=True):
    return ([f"{name}: "] if head else []) + [fmt(config[name])]

wrapper = textwrap.TextWrapper(subsequent_indent='\t', width=120)
docstring += format_value(config, "Description", lambda value: f"\t{wrapper.fill(value)}")

Lets look at how the code looks now:
import textwrap

def format_value(config, name, fmt, head=True):
    return ([f"{name}: "] if head else []) + [fmt(config[name])]

def format_values(config, name, fmt):
    return [f"{name}:"] + [fmt(value) for value in config[name]]

def gendoc(config):
    docstring = []
    if None is not config.get("Summary"):
        docstring += format_value(config, "Summary", lambda summary: summary, False)
    if None is not config.get("Args"):
        docstring += format_values(config, "Args", lambda arg: f"\t{arg}: Function argument")
    if None is not config.get("Attributes"):
        docstring += format_values(config, "Attributes", lambda attr: f"\t{arg}: Information about parameter {arg}")
    if None is not config.get("Raises"):
        docstring += format_value(config, "Raises", lambda raises: f"\t {raises}")
    if None is not config.get("Returns"):
        docstring += format_value(config, "Returns", lambda returns: f"\t {returns}")
    if None is not config.get("Description"):
        wrapper = textwrap.TextWrapper(subsequent_indent='\t', width=120)
        docstring += format_value(config, "Description", lambda value: f"\t{wrapper.fill(value)}")
    if None is not config.get("Todo"):
        star = "*"
        docstring += format_values(config, "Todo", lambda todo: f"\t{star} {todo}")

    docstring = "\n\n".join(docstring)
    return docstring

if None is not config.get({name}):
    docstring += format_value(config, {name}, ...)

We can see we are duplicating the above pseudocode.
Lets ignore the fmt and head arguments to format_value and format_values.
We could change the code to:
docstring = []
for name, fn in ...:
    if config.get(name) is not None:
        docstring += fn(config, name)

We can change format_value and format_values so we provide the fmt and head.
We then return another function which takes config and name to call in the loop.
def format_value(fmt, head=True):
    def inner(config, name):
        return ([f"{name}: "] if head else []) + [fmt(config[name])]
    return inner

format_value("{}")(config, "Summary")

Next we can define a dictionary to store the result of the first call to format_value and format_values.
And then change the code to iterate over the dictionary.
Final code:
import textwrap

def format_value(fmt, head=True):
    def inner(config, name):
        return ([f"{name}: "] if head else []) + [fmt(config[name])]
    return inner

def format_values(fmt):
    def inner(config, name):
        return [f"{name}:"] + [fmt(value) for value in config[name]]
    return inner

wrapper = textwrap.TextWrapper(subsequent_indent='\t', width=120)
star = "*"

FORMATS = {
    "Summary": format_value(lambda summary: f"{summary}", False),
    "Args": format_values(lambda arg: f"\t{arg}: Function argument"),
    "Attributes": format_values(lambda attr: f"\t{arg}: Information about parameter {arg}"),
    "Raises": format_value(lambda raises: f"\t {raises}"),
    "Returns": format_value(lambda returns: f"\t {returns}"),
    "Description": format_value(lambda value: f"\t{wrapper.fill(value)}"),
    "Todo": format_values(lambda todo: f"\t{star} {todo}"),
}

def gendoc(config):
    docstring = []
    for name, fn in FORMATS.items():
        if config.get(name) is not None:
            docstring += fn(config, name)
    return "\n\n".join(docstring)


Group list items into pages, rows and columns I have a list of labels for printing. Now I have go over the list and print the labels. The layout is specified by how many labels per page and per row on a page I can print.

@{
    var labels = new List<int> { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17 };
    var labelsPerPage = 10;
    var labelsPerRow = 2;
    var labelCount = labels.Count;
}

@for (var i = 0; i < labelCount; i++) {
    if (i % labelsPerPage == 0) {
        @Html.Raw("
")
    }

    if (i % labelsPerRow == 0) {
        @Html.Raw("
")
    }

    @PrintLabel(labels[i])

    if (i % labelsPerRow == (labelsPerRow - 1)) {
        @Html.Raw("
")
    }

    if (i % labelsPerPage == (labelsPerPage - 1)) {
        @Html.Raw("
")
    }
}

@helper PrintLabel(int label) {
    @label
}

Is there a better method (eg. using LINQ or nested for/foreach loops) for going through the list and doing something at the start and end of each page and each row?

I would prefer a nested structure to a flat one so it can be used inside a Razor template. The flat version fails here because Razor requires HTML tags to be closed at the end of a code block.

I'm not looking for performance here, I'd rather have more readable code and possibly getting rid of any Html.Raw() calls.

You're closing tags only when a row or table is full. Is data always going to fit the table size?

if (i % labelsPerRow == (labelsPerRow - 1)) {
    @Html.Raw("
")
}

if (i % labelsPerPage == (labelsPerPage - 1)) {
    @Html.Raw("
")
}

You could make a functional helper and use that instead. Notice that after the loop, the row and table are ensured to be closed. The purpose is to allow the consumer to pick how to build a TAggregate from TSource items and observer methods. We're observing every item, table start/end and row start/end.

public static class GridRenderer
{
    public static TAggregate Render<TSource, TAggregate>(IEnumerable<TSource> source, TAggregate seed, int pageSize, int rowSize,
        Func<TSource, TAggregate, TAggregate> itemObserver,
        Func<TAggregate, TAggregate> beginPageObserver,
        Func<TAggregate, TAggregate> endPageObserver,
        Func<TAggregate, TAggregate> beginRowObserver,
        Func<TAggregate, TAggregate> endRowObserver)
    {
        if (source == null) throw new ArgumentNullException(nameof(source));
        if (pageSize <= 0) throw new ArgumentOutOfRangeException(nameof(pageSize));
        if (rowSize > pageSize) throw new ArgumentOutOfRangeException(nameof(rowSize));
        // .. check observers for null

        var items = source.ToList();
        var result = seed;

        if (!items.Any()) return result;

        for (var i = 0; i < items.Count; i++)
        {
            if (i % pageSize == 0)
            {
                result = beginPageObserver(result);
            }

            if (i % rowSize == 0)
            {
                result = beginRowObserver(result);
            }

            result = itemObserver(items[i], result);

            if ((i + 1) % rowSize == 0)
            {
                result = endRowObserver(result);
            }

            if ((i + 1) % pageSize == 0)
            {
                result = endPageObserver(result);
            }
        }

        if (items.Count % rowSize != 0)
        {
            result = endRowObserver(result);
        }

        if (items.Count % pageSize != 0)
        {
            result = endPageObserver(result);
        }

        return result;
    }
}

Let's say your data does not fit the table size and row size.

var renderer = new StringBuilder();
GridRenderer.Render(new[] {1, 2, 3, 4, 5}, renderer, 4, 2,
    (item, cur) => cur.AppendLine($"    {item}	"),
    cur => cur.AppendLine("
"),
    cur => cur.AppendLine("
"),
    cur => cur.AppendLine("
"),
    cur => cur.AppendLine("
"));
var layout = renderer.ToString();

The grid still gets created correctly.

    1
    2

    3
    4

    5

Note I've used a StringBuilder, but you could also use any other class to render the output. You could also change the flow a bit if you want an empty table when no data is available.


Unique values of an array I'm curious if there is another way to code this. Could it be done with a while loop for instance?

public static int numUnique(double[] list) {
    int uniques = 0;
    if(list.length == 0){return 0;}
    for(int i = 0; i < list.length; i++){
        int dups = 0;
        for(int j = i + 1; j < list.length; j++){
            if(list[i] == list[j]){
                dups = dups + 1;
            }
        }
        if(dups == 0){
            uniques = uniques + 1;
        }
    }
    return uniques;
}

No need for any loops with Java 8 streams.

Get a DoubleStream from the array with Arrays.stream(double[] array) so that you can
get the distinct() elements from that
which you can count() to retrieve your result.

public static long numUnique(double[] list)
{
    return Arrays.stream(list).distinct().count();
}

As count() returns a long, I modified the signature of your method to return long, too. It's within the responsibility of the user of your method then to perform any cast on the result.

Full example program:

import java.util.Arrays;

public class UniqueDoubles
{
    public static void main(String[] args)
    {
        double[] doubles = {1, 2, 3.5, 3.5, 2, 345345.345345};

        System.out.println(numUnique(doubles));
    }

    public static long numUnique(double[] list)
    {
        return Arrays.stream(list).distinct().count();
    }
}