Parallelized Sobel algorithm using OpenMP I implemented the Sobel algorithm sequentially, and got decent performance (1.49 s) but wanted to improve it. I parallelized it with OpenMP and got a ~3x speedup (0.523 s). I'd like to do better, but I'm not sure how.

// -------  C/C++ includes ------
#include <iostream>
#include <stdio.h>

#include <omp.h>
#include <time.h>

// ------ OpenCV includes ------
#include <opencv2/core.hpp>
#include <opencv2/imgproc.hpp>
#include <opencv2/highgui.hpp>
#include <opencv2/opencv.hpp>

using namespace std;
using namespace cv;

// dimension of kernel
int x[3][3];
int y[3][3];

/*----- OpenMP -----*/
int num_of_threads, i ,j;
double start, end;

int main( int argc, char** argv )
{

    Mat initialImage = imread(argv[1], 0); // imread gray-scale image
    Mat finalImage = Mat::zeros(initialImage.size(), initialImage.type());

    if (finalImage.type() == initialImage.type() )
    {
        cout << "YES" << endl;
    }

    if(argc != 2 || !initialImage.data)
    {
        cout << "No image data or Usage: ./sobel imagePath" << endl;
        return -1;
    }
    else
      cout << "Image OK!" << endl;

    //x direction
    x[0][0] = -1;  x[0][1] = 0;  x[0][2] = 1;
    x[1][0] = -2;  x[1][1] = 0;  x[1][2] = 2;
    x[2][0] = -1;  x[2][1] = 0;  x[2][2] = 1;

    //y direction
    y[0][0] = -1;  y[0][1] = -2;  y[0][2] = -1;
    y[1][0] =  0;  y[1][1] =  0;  y[1][2] =  0;
    y[2][0] =  1;  y[2][1] =  2;  y[2][2] =  1;

    num_of_threads = 8;//omp_get_num_procs();
    omp_set_num_threads(num_of_threads);

    start = omp_get_wtime();
    for(j = 0; j < initialImage.rows - 2; j++ ){
      #pragma omp parallel for private(i)
      for(i = 0; i < initialImage.cols -2; i++ ){
        // applay karnel in x direction
        int xValOfPixel =
         (x[0][0] * (int)initialImage.at<uchar>(j, i    )) + (x[0][1] * (int)initialImage.at<uchar>(j + 1, i    )) + (x[0][2] * (int)initialImage.at<uchar>(j + 2, i    )) +
         (x[1][0] * (int)initialImage.at<uchar>(j, i + 1)) + (x[1][1] * (int)initialImage.at<uchar>(j + 1, i + 1)) + (x[1][2] * (int)initialImage.at<uchar>(j + 2, i + 1)) +
         (x[2][0] * (int)initialImage.at<uchar>(j, i + 2)) + (x[2][1] * (int)initialImage.at<uchar>(j + 1, i + 2)) + (x[2][2] * (int)initialImage.at<uchar>(j + 2, i + 2));

        // apply karnel in y direction
        int yValOfPixel =
         (y[0][0] * (int)finalImage.at<uchar>(j, i    )) + (y[0][1] * (int)finalImage.at<uchar>(j + 1, i    )) + (y[0][2] * (int)finalImage.at<uchar>(j + 2, i    )) +
         (y[1][0] * (int)finalImage.at<uchar>(j, i + 1)) + (y[1][1] * (int)finalImage.at<uchar>(j + 1, i + 1)) + (y[1][2] * (int)finalImage.at<uchar>(j + 2, i + 1)) +
         (y[2][0] * (int)finalImage.at<uchar>(j, i + 2)) + (y[2][1] * (int)finalImage.at<uchar>(j + 1, i + 2)) + (y[2][2] * (int)finalImage.at<uchar>(j + 2, i + 2));

         int sum = abs(xValOfPixel) + abs(yValOfPixel);
         if(sum > 255)
          sum = 255;

        finalImage.at<uchar>(j, i) = (uchar)sum;
      }
    }
    end = omp_get_wtime();

    cout << "Time: " << end - start << endl;

    // display the images
    namedWindow("Initial Image", WINDOW_AUTOSIZE);
    namedWindow("Final Image", WINDOW_AUTOSIZE);
    imshow("Initial Image",initialImage);
    imshow("Final Image",finalImage);

    waitKey(0);
    return 0;
}

Avoid using namespace
Especially for large namespaces such as std, bringing so many identifiers into scope is dangerous.  Prefer to import just the identifiers you need, and/or qualify at use; as it is, I get a compilation failure:
195834.cpp: In function ‘int main(int, char**)’:
195834.cpp:81:5: error: reference to ‘end’ is ambiguous
     end = omp_get_wtime();
     ^~~

Don't include <stdio.h> and <time.h>.
When writing new code, prefer <cstdio> and <ctime>, which brings the names into the std namespace.  Even better, omit these includes entirely, as we don't need them.
And do include <cmath>, so we have a declaration for std::abs().
Don't put everything in main()
There's three parts to this program.  It's easier to follow if we split the input and output from the processing.
Avoid global variables
Part of the problem with end above is that it's declared global when it has no right to be.  Keep the scope of your variables as small as reasonably possible (and give them more meaningful names!).  Similarly, the kernels can be declared within the function:
static const int x[3][3] = { { -1, 0, 1 }, { -2, 0, 2 }, { -1, 0, 1 }};
static const int y[3][3] = { { -1, -2, -1 }, { 0, 0, 0 }, { 1, 2, 1 }};

Use the correct output streams
Use std::cerr for reporting errors, and std::clog for other diagnostics.
Check argv[1] is valid before using it.
We have
Mat initialImage = imread(argv[1], 0); // DANGER

// ...

if(argc != 2 || !initialImage.data)

We hit undefined behaviour if we access argv[1] before discovering that argc < 2.
Use the correct image for the y-gradient
We should be using initialImage, not finalImage to compute yValOfPixel, otherwise we'll always get zero.
Use a bigger work unit
You'll get better parallelisation if you make the outer loop a parallel-for, rather than the inner loop.  Since each row is the same length, you're better off dividing the work into bigger chunks
Hard-code the Sobel coefficients
Since this isn't a general kernel convolution, we can help the compiler by using the constants in-place.  This allows the multiplication by small constants to be replaced by addition or subtraction (or omitted entirely for the six 0s).
Use standard functions to limit values
Instead of the ad-hoc test for sum exceeding 255 (and no test for below zero), we can use std::clamp() (from <algorithm>).

Modified code
#include <algorithm>
#include <cmath>

#include <opencv2/core.hpp>
#include <opencv2/highgui.hpp>

cv::Mat sobel_transform(const cv::Mat& input)
{
    cv::Mat finalImage = cv::Mat::zeros(input.size(), input.type());

#pragma omp parallel for
    for (int j = 0; j < input.rows-2;  ++j) {
        for (int i = 0;  i < input.cols-2;  ++i) {
            // applay karnel in x direction
            int xValOfPixel =
                -     (int)input.at<uchar>(j, i    ) +     (int)input.at<uchar>(j + 2, i    )
                - 2 * (int)input.at<uchar>(j, i + 1) + 2 * (int)input.at<uchar>(j + 2, i + 1)
                -     (int)input.at<uchar>(j, i + 2) +     (int)input.at<uchar>(j + 2, i + 2);

            // apply karnel in y direction
            int yValOfPixel =
                - (int)input.at<uchar>(j, i    ) - 2 * (int)input.at<uchar>(j + 1, i    ) - (int)input.at<uchar>(j + 2, i    )
                + (int)input.at<uchar>(j, i + 2) + 2 * (int)input.at<uchar>(j + 1, i + 2) + (int)input.at<uchar>(j + 2, i + 2);

            int sum = std::clamp(std::abs(xValOfPixel) + std::abs(yValOfPixel), 0, 255);

            finalImage.at<uchar>(j, i) = (uchar)sum;
        }
    }

    return finalImage;
}

#include <omp.h>
#include <iostream>
int main(int argc, char** argv)
{
    if (argc != 2) {
        std::cerr << "Usage: ./sobel imagePath" << std::endl;
        return 1;
    }

    const cv::Mat initialImage = cv::imread(argv[1], cv::IMREAD_GRAYSCALE);
    if (!initialImage.data) {
        std::cerr << "Failed to read image" << std::endl;
        return 1;
    }

    double start_time = omp_get_wtime();
    const cv::Mat finalImage = sobel_transform(initialImage);
    double end_time = omp_get_wtime();

    std::clog << "Time: " << end_time - start_time << std::endl;

    // display the images
    cv::namedWindow("Initial Image", cv::WINDOW_AUTOSIZE);
    cv::namedWindow("Final Image", cv::WINDOW_AUTOSIZE);
    cv::imshow("Initial Image", initialImage);
    cv::imshow("Final Image", finalImage);

    cv::waitKey(0);
}


AT&T assembly - Basic loop & write - follow-up This is a follow-up question to this one: AT&T Assembly - Basic loop & write

The code loops to display "Hello, World!" ten times.

I implemented the syscall instead of int $0x80, used a decrementing loop to avoid a useless instruction and commented the code. Is there any way to make it better?

When debugging with GDB, it appears exit $0 is part of l. However, I would like it to be part of _start (since l represents the loop only). Is that possible?

.section .data
  hello: .string "Hello, world!\n"
  len = . -hello

  .equ EXIT, 60
  .equ WRITE, 1
  .equ STDOUT, 1

.section .bss
  # Write a str of length len on the standard output.
  .macro write str, len
    movq $WRITE, %rax
    movq $STDOUT, %rdi
    movq \str, %rsi
    movq \len, %rdx
    syscall
  .endm

  # Exit with the specified error code
  .macro exit code
    movq $EXIT, %rax
    movq \code, %rdi
    syscall
  .endm

.section .text
  .globl _start

  # Loops to display "Hello, World!" ten times.
  _start:
     movq $10, %r8               # Counter
    l:
      write $hello, $len
      dec %r8
      jnz l

    exit $0

This is much better.

Since this is v2, let me introduce you a couple more slightly advanced concepts.

First: The second time you execute write, what will be in (most of) those registers (rdi, rsi, rdx)?

It's easy to think in terms of C (or practically every other high level language), and how variables have 'scope.'  But asm's registers (even when used with call) don't work that way.

The is nothing 'wrong' with using macros like this.  It makes reading / maintaining much easier.  And in more complex examples, you probably wouldn't be able to assume that these registers haven't been modified since the last write.  But since we are critiquing this particular case, be aware that the code is slightly less efficient than it could be (since it re-assigns values that are already there 3 * 9 times).

Second (follows from the first):

write modifies a LOT of registers (as a percentage of how many registers there are).  In addition to the ones you see (rax, rdi, rsi, rdx), there are also the registers 'clobbered' by using syscall (rcx, r11).  It's going to be difficult to compose complex code if every time you want to write something, that many registers get destroyed.

This leads us to the concept of 'calling conventions' (something I've written about before).  In short, you come up with (or use an existing) standard set of rules about what registers your macros/routines are allow to modify, then avoid using those 'volatile' registers in the routines that call them (or save the contents of the registers yourself before making the call).  And if the macros/routines need more registers than your 'rules' allow, they must push/pop all the additional registers to preserve their contents.

Yes, this is starting to get a bit deeper.  But register handling is something you need to start thinking about if you want to read/write asm.  Additionally, it starts to explain some of the 'junk' you see when you disassemble your C code.  Those push/pops you see around the calls and at the top of functions?  This is why they are there.

I could talk about the cost/benefits of macros vs routines, but this is probably enough

A few more thoughts:

Add a comment at the top of the file.  Even something simple like The code loops to display "Hello, World!" ten times.
Stylistically, perhaps . - hello?  Negative hello seems weird.

Yes, nitpicky.  But that's what you get when you ask for code reviews on such basic code.

Edit1:

Hmm.  You haven't accepted an answer yet.  Were you looking for something else?  Or perhaps I wasn't clear about what I was trying to explain?  Let me take a shot at explaining this again:

Using macros isn't the same as defining routines.  Instead of defining the code in one place and calling it from other places, it pastes the entire macro in where ever it is invoked.

So your code basically expands to:

movq $10, %r8
l:
  movq $1, %rax
  movq $1, %rdi
  movq $hello, %rsi
  movq $15, %rdx
  syscall
  dec %r8
  jnz l

movq $60, %rax
movq $0, %rdi
syscall

If you were to call write twice in a row, you'd get:

movq $10, %r8
l:
  movq $1, %rax
  movq $1, %rdi
  movq $hello, %rsi
  movq $15, %rdx
  syscall
  movq $1, %rax
  movq $1, %rdi
  movq $hello, %rsi
  movq $15, %rdx
  syscall
  dec %r8
  jnz l

movq $60, %rax
movq $0, %rdi
syscall

Now, if I'm not mistaken, the syscall you are using overwrites rax (uses it as a return value), but leaves the other parameters alone.  Such being the case, it would be slightly more efficient to write:

movq $1, %rdi
movq $hello, %rsi
movq $15, %rdx
movq $10, %r8
l:
  movq $1, %rax # or perhaps movq %rdi, %rax?
  syscall
  dec %r8
  jnz l

movq $60, %rax
movq $0, %rdi
syscall

This doesn't lend itself well to macros.  But if performance was more important to you than "maintainability," this would be (slightly) better.

Alternately, you could do this as a routine.  The existing code for write uses 6 of the 15 x64 registers, stomping on their existing values in order to make the call.  Registers are a precious and limited resource.  If you were doing anything much more complex, you would start to run out.  Using routines allows you to bundle code in such a way that only a limited number of registers get modified, causing a minimum amount of disruption in the code that calls it.

For example, if you were to use the Microsoft x86 'fastcall' calling convention (a poor choice for 64bit linux, but useful as an illustration), then the first parameter gets placed in rcx, the second in rdx, and the return value (if any) goes in rax.  rcx, rdx and rax can all be changed by the routine, but all other registers must be returned unchanged.

So, re-working write with this in mind, we get something like this:

# On entry:
#   rcx points to the string to print
#   rdx contains the length of the string

push %rsi  # Save the non-volatile registers we modify
push %rdi
push %r11

movq %rcx, %rsi # Move the string pointer to the correct register

movq $WRITE, %rax
movq $STDOUT, %rdi
syscall

# At this point:
#   rax contains the return value from the call
#   rcx/r11 have been clobbered

pop %r11 # Restore the registers and return
pop %rdi
pop %rsi
ret

You can call it like this:

mov $hello, rcx
mov $len, rdx
call write
# At this point, the contents of rcx and rdx are undefined.

Note that when I say "rcx and rdx are undefined," I mean they are undefined by definition.  Yes, you can look at write and see what they would contain, but you pretend like you don't.  This way someone can modify write to work slightly differently, and every place that calls it will still work correctly.  As long as everybody follows the agreed-upon 'rules.'

The implications here define a lot of how registers actually get used, both by compilers when they generate asm and by people who write their own.  If you know how registers will be treated when you make a call, that helps you choose which registers to use for what.  For example, write needs the length in rdx.  So if you needed to count how many bytes were in a string before passing it to write,  then using rdx when doing the count suddenly makes a lot more sense.  And using rcx to hold your 1-10 loop counter would obviously be a poor choice, since it gets wiped out during each write.

To sum up:

Using the write macro allows for easy-to-read code.  It also allows you to call the macro (somewhat) generically.  However, it has some limitations that may make it a bad choice for more complicated code, or if performance it a primary consideration.

That's what I see when I read this code.


AJAX call with repetitive success/failure handlers There's some repetition in here. I tried making it into a function but then the response wasn't filling up the #project-container div. Is there a way to condense this code to make it more elegant?

// Activate projects
$('.post-link').click(function(e) {
    e.preventDefault();

    var post_id = $(this).attr('rel'),
        ajaxURL = site.ajaxurl; // Ajax URL localized from functions.php

    function projectShow() {

        // Initiate the Ajax call
        $.ajax({
            type: 'POST',
            url: ajaxURL,
            data: {'action': 'load-content', post_id: post_id },
            beforeSend: function() {
                $('').insertBefore($(e.currentTarget));
            },
            success: function(response) {
                $('.loading-icon').remove();

                if ($(window).scrollTop() != 0) {
                    $('html, body').animate({
                        scrollTop : 0
                    },200, function() {
                        $('#project-wrapper').addClass('activated');

                        // Keep the rest of the projects grey while #project-wrapper is activated
                        $('article').addClass('grayscale grayscale-fade').css('opacity', '0.4');

                        // Load up the response from the Ajax call
                        $('#project-container').html(response);

                        // Make the max-height of the container exact for smoother animations
                        var containerHeight = $('#project-container').outerHeight();
                        $('#project-wrapper.activated').css('max-height', containerHeight);

                        // CSS effects
                        $('.post-container').addClass('fadeInUp');
                        $('.close-button').addClass('fadeInDown');

                        // Remove pesky, sticky 'hover' class
                        $('article').removeClass('hover');

                        // Shuffle letters and update the title of the page
                        $('#project-wrapper .entry-title').shuffleLetters({
                            callback:function(){
                                var title = $(e.currentTarget).closest('#main').find('h1').text();
                                $('head').find('title').text('Keebs | ' + title);
                            }
                        });
                    });
                } else {
                    $('#project-wrapper').addClass('activated');

                    // Keep the rest of the projects grey while #project-wrapper is activated
                    $('article').addClass('grayscale grayscale-fade').css('opacity', '0.4');

                    // Load up the response from the Ajax call
                    $('#project-container').html(response);

                    // Make the max-height of the container exact for smoother animations
                    var containerHeight = $('#project-container').outerHeight();
                    $('#project-wrapper.activated').css('max-height', containerHeight);

                    // CSS effects
                    $('.post-container').addClass('fadeInUp');
                    $('.close-button').addClass('fadeInDown');

                    // Remove pesky, sticky 'hover' class
                    $('article').removeClass('hover');

                    // Shuffle letters and update the title of the page
                    $('#project-wrapper .entry-title').shuffleLetters({
                        callback:function(){
                            var title = $(e.currentTarget).closest('#main').find('h1').text();
                            $('head').find('title').text('Keebs | ' + title);
                        }
                    });
                }

                return false;
            }
        });
    }

    // The call
    projectShow();
});

Here's a shot at it. I don't have a demo environment to test with, so let me know if I had any typos etc. I also wrapped it in a self-executing function to keep things from being in the global scope. If you already have it scoped then the self-executing function is unnecessary.

I pulled out the duplicated code into a function so the code is no longer duplicated. This helps with maintainability and shrinks the file size.

(function() {
    function doWork(event, response) {
        $('#project-wrapper').addClass('activated');

        // Keep the rest of the projects grey while #project-wrapper is activated
        $('article').addClass('grayscale grayscale-fade').css('opacity', '0.4');

        // Load up the response from the Ajax call
        $('#project-container').html(response);

        // Make the max-height of the container exact for smoother animations
        var containerHeight = $('#project-container').outerHeight();
        $('#project-wrapper.activated').css('max-height', containerHeight);

        // CSS effects
        $('.post-container').addClass('fadeInUp');
        $('.close-button').addClass('fadeInDown');

        // Remove pesky, sticky 'hover' class
        $('article').removeClass('hover');

        // Shuffle letters and update the title of the page
        $('#project-wrapper').find('.entry-title').shuffleLetters({
            callback: function(){
                var title = $(event.currentTarget).closest('#main').find('h1').text();
                $('head').find('title').text('Keebs | ' + title);
            }
        });
    }

    // Activate projects
    $('.post-link').click(function(e) {
        e.preventDefault();

        var post_id = $(this).attr('rel'),
            ajaxURL = site.ajaxurl; // Ajax URL localized from functions.php

        // Initiate the Ajax call
        $.ajax({
            type: 'POST',
            url: ajaxURL,
            data: {'action': 'load-content', post_id: post_id },
            beforeSend: function() {
                $('').insertBefore($(e.currentTarget));
            },
            success: function(response) {
                $('.loading-icon').remove();

                if ($(window).scrollTop() != 0) {
                    $('html, body').animate({
                        scrollTop : 0
                    }, 200, function() {
                        doWork(e, response);
                    });
                } else {
                    doWork(e, response);
                }

                return false;
            }
        });
    });
})();


Storing and processing large arrays to compute dot products Problem Statement (from HackerRank)

  We have two arrays \$a\$ and \$b\$ of length \$N\$, initially all values equals to
  zero. We have \$Q\$ operations. Let's define three types of operations on
  this arrays:

  1 l r c: Increase \$a_l, a_{l+1}, \ldots, a_r\$ by \$c\$.
  2 l r c: Increase \$b_l, b_{l+1}, \ldots, b_r\$ by \$c\$.
  3 l r: Print \$a_l\cdot b_l + a_{l+1}\cdot b_{l+1} +\ldots+ a_r\cdot b_r\$ in modulo \$1000000007\$.

  Input Format

  First line of the input consists of \$N\$ and \$Q\$. Next \$Q\$ lines contain one of the three types of operations.

  Constraints

  \$1 \le N \le 10^9\$
  \$1 \le Q \le 200000\$
  \$1 \le c \le 10000\$
  \$1 \le l \le r \le N\$

  Output Format

  Whenever you get a type 3 operation, you should print the answer in a
  new line.

  Sample Input 1

5 3
1 1 5 5
2 2 4 2
3 3 4

  Sample Output 1

20

  Sample Input 2

10 20
1 9 9 6768
2 5 5 2202
3 7 7
2 3 9 1167
2 1 7 8465
3 1 5
2 1 1 1860
3 9 9
2 5 5 2153
1 5 7 749
3 1 1
2 8 10 3129
3 1 1
1 2 10 2712
2 1 8 79
1 1 6 4645
1 7 7 1358
3 2 10
1 9 9 8677
3 8 10

  Sample Output 2

0
0
7898256
0
0
506356461
98353320

How can I efficiently store and process data for this problem?

#include<stdio.h>
#include<string.h>
int main()
{
    long n,q,ch,l,r,c,i,j;
    /* n, q, l, r, c works as per problem statement.
       ch is used to scan the first digit of operation.
       i and j are used to control the loops. */
    scanf("%ld %ld",&n,&q);
    long a[n],b[n];
    memset(&a, 0, sizeof a);
    memset(&b, 0, sizeof b);
    for(i=0;i<n;i++)    //Init to 0
    {
        a[i]=0;
        b[i]=0;
    }
    for(i=0;i<q;i++)
    {
        scanf("%ld ",&ch);  //Look for the first digit
        switch(ch)
        {
            case 1:
                scanf("%ld %ld %ld",&l,&r,&c);
                l--;
                for(j=l;j<r;j++)
                    a[j]+=c;    //Adds c to every element of a
                break;
            case 2:
                scanf("%ld %ld %ld",&l,&r,&c);
                l--;
                for(j=l;j<r;j++)
                    b[j]+=c;    //Adds c to every element of b
                break;
            case 3:
                scanf("%ld %ld",&l,&r);
                l--;
                c=0;
                for(j=l;j<r;j++)
                    c+=a[j]*b[j];   //Adds the product
                printf("%ld\n",c%1000000007);   //Prints the value after using mod
                break;
        }
    }
}

This is a fairly straightforward implementation that seems to get the job done at least for the simple input you posted. Overall, it's easy to read and understand, given the problem description. Here are some ways I think it could be improved.

Naming

You've named all the variables similarly to how they are named in the problem statement. That's fine for an assignment or exercise, but it would be a real pain to maintain later on. You have a comment describing the names, but it assumes the person reading it has the problem statement. That's rarely the case in the real world.

The variable names n, i, and j aren't terrible. I'd leave i and j, but rename n to something descriptive like numElements (or, if you prefer num_elements).

The names q, l, ch, r, and c are terrible and confusing. I'd make q something like numLines or numInputLines or something like that. l and r are the start and end of the range of indexes you want to modify, so I'd name them start and end.

c and ch are likely to be confused. Also, ch seems like it's a remnant from an earlier version when it was declared as a char type instead of a long. It's used to get the user's input, so you could name it userInput or mode or something more appropriate. c is the value you're adding to all the inputs. Since this is an exercise, it's just an abstract value that doesn't have any real meaning. Normally, I'd suggest calling it whatever it is, but that's hard in this case. Something generic like value or operand could work.

Functions

Your code does a few different things:

Gets the size of the arrays and number of lines of input
Creates and initializes the arrays
Processes the inputs

You should separate those into functions like this:

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

int getNumberOfElementsAndLines (size_t* outElements, size_t* outLines)
{
    long numScanned = scanf("%zu %zu", outElements, outLines);
    int result = (numScanned == 2);
    if ((*outElements < 1) || (*outElements > 1000000000) || (*outLines < 1) || (*outLines > 200000))
    {
        result = 0;
    }

    return result;
}

int allocateAndInitArrays (const size_t numElements, long** outA, long** outB)
{
    // Make sure the inputs are valid
    if ((outA == NULL) || (outB == NULL) || (numElements == 0))
    {
        return 0;
    }

    // Allocate and clear A and if successful, allocate and clear B
    *outA = calloc (numElements, sizeof(**outA));
    *outB = calloc(numElements, sizeof(**outB));

    if (*outB == NULL)
    {
        free(*outA);
        *outA = NULL;
    }

    return ((*outA != NULL) && (*outB != NULL));
}

int getAndProcessInput (long* a, long* b, const size_t numElements, const size_t numLines)
{
    for (size_t i = 0; i < numLines; i++)
    {
        long mode   = 0;
        int numScanned = scanf("%ld ", &mode);
        if (numScanned != 1)
        {
            return 0;
        }

        long l, r, c;
        switch (mode)
        {
            case 1:
                scanf("%ld %ld %ld",&l,&r,&c);
                l--;
                for(long j = l; j < r; j++)
                    a[ j ] += c;    //Adds c to every element of a
                break;

            case 2:
                scanf("%ld %ld %ld",&l,&r,&c);
                l--;
                for(long j = l; j < r; j++)
                    b[ j ] += c;    //Adds c to every element of b
                break;

            case 3:
                scanf("%ld %ld",&l,&r);
                l--;
                c=0;
                for(long j = l; j < r; j++)
                    c += a[ j ] * b[ j ];   //Adds the product
                printf("%ld\n",c%1000000007);   //Prints the value after using mod
                break;

            default:
                // Error in input
                return 0;
                break;
        }
    }

    return 1;
}

Then your main would look something like this:

int main()
{
    int     result  = 0;
    size_t  numElements = 0;
    size_t  numLines = 0;
    result = getNumberOfElementsAndLines(&numElements, &numLines);

    long*   a = NULL;
    long*   b = NULL;
    if (result == 1)
    {
        result = allocateAndInitArrays(numElements, &a, &b);
    }

    if (result == 1)
    {
        result = getAndProcessInput(a, b, numElements, numLines);
    }

    free(a);
    free(b);

    return result;
}

Memory

You'll notice above that I used calloc() to allocate the memory on the heap instead of the stack. Stack space is limited, and there's a lot more memory available on the heap. That may fix your issue getting a segfault.

Error Handling

I added a little bit of error handling. It needs more. You should check to make sure you got valid results from functions like scanf(), for example. What if the user entered a negative number? What if they entered 0? Etc.


Retrieving three separate sorted lists of items I need to search for records based on their area, postcode, city and sort each group separately based on the value of their code field.

Table is as follows:

ID   Code Name    Area        PostCode   City
1    15    BV     Shore        343444    Toronto
2    2     BB     North        545555    London
3    3     AA     North        343444    Munich
4    4     KH     South Shore  959595    London
5    1     XX     North Shore  343444    Toronto

I want to show following list against 'North 343444 London':

2  BB   North         545555       London
3  AA   North         343444       Munich
--------------------------------------------
1  XX   North Shore   343444       Toronto
15 BV   Shore         343444       Toronto
--------------------------------------------
4  KH   South Shore   959595       London

BB and AA will be returned and sorted for matched area (PostCode and City are ignored)
XX, BV will be returned and sorted for matched postcode. (Area and City are ignored)
KH will be returned for matched city. (Area and Postcode are ignored)

As you can see code of XX is 1 but in result list it comes up after BB and AA because it is sorted in its own group (XX and BV).

In controller I have:

List<Area> matchedArea = searchService.findAreaMatches(area);
List<Area> matchedPostcode = searchService.findPostcodeMatches(area);
List<Area> matchedCity = searchService.findCityMatches(area);

In the Service class I have:

@Override
@Transactional(readOnly = true)
public List<Area> findAreaMatches(String area){
     return searchRep.findAreaMatches(area);
}

@Override
@Transactional(readOnly = true)
public List<Area> findPostcodeMatches(int postcode){
     return searchRep.findPostcodeMatches(postcode);
}

@Override
@Transactional(readOnly = true)
public List<Area> findCityMatches(String city){
     return searchRep.findCityMatches(city);
}

Is there any better approach to this? Is that fine to have three different methods in which each has its own @Transactional annotation?

If you're concerned about repeating yourself, you can look into the Java workaround/equivalent of C# delegates, though it might be more trouble than it's worth with regard to clarity and conciseness (http://www.javacamp.org/javavscsharp/delegate.html).

Everything below is just a hyper-abridged and incomplete explanation of one use of delegates.

Think of delegates as a stand-in for any method with a specific signature.  In this case, you could use it for searchRep.findAreaMatches and searchRep.findCityMatches (searchRep.findPostcodeMatches has a different signature).  At a high level, it might do something like this:

delegate List<Area> functionDelegate(string s);
functionDelegate f;
if (areaMatch):
    f = searchRep.findAreaMatches
else if (cityMatch):
    f = searchRep.findCityMatches
else:
    f = /* Your choice of what goes here */
functionDelegate(areaOrCity)

As a bit of an semi-related side-note, languages like Python allow for similar behavior without the need to explicitly declare something a delegate and without worrying about signatures, though you do have to make sure your types are correct.

def findMatches(arg):
    func = someFunction
    if areaMatch:
        func = searchRep.findAreaMatches
    elif postcodeMatch:
        func = searchRep.findPostcodeMatches
    elif cityMatch:
        func = searchRep.findCityMatches
    return func(arg)

Apologies if I totally missed the mark on what you were asking, I'm new to this.  Hope it helps!


Listing prime numbers up to the given number import java.util.*;

public class PrimeNumber {
    private static ArrayList<Integer> listOfPrime = new ArrayList<Integer>();

    public static void main(String args[]) {
        Scanner userInput = new Scanner(System.in);
        System.out.print("Please enter a number to checked the list of prime number >  ");
    //Request user to input  number.
        String input = userInput.nextLine();
        int number = 0;
        boolean check = false;
    //Check if integer entered is it integer
        while(check != true) {
            try {
    // if input can be parse into integer, shows that its number so can change check into true to exit the while loop

                number = Integer.parseInt(input);
                check = true;
            }
            catch(NumberFormatException e) {
                System.out.println("Please enter numbers only  >  ");
                input = userInput.nextLine();
            }
        }

    //Check if input enter is 0 or 1, as prime number cannot be those.
        if(number < 2) {
            System.out.println("Prime number cannot be 0 or 1");
        }else {
            checkPrime(number);
        }

    }

    //Check for all prime numbers within the number user input.
    private static void checkPrime(int input) {
        boolean primeValidation = true;
    //first for loop to check from small to biggest integer in the input
        for(int i = 1; i <= input;i++) {
            if(i !=1) {

    //Second for loop to check from prime number in the small to big number.

    //If the number i is 2 or 3, the loop will exit and add into the list as the default is prime, and no option changes it to not prime number.

                for(int x = 2; x < i ; x++) {
                    if(i%x == 0) {
    //If the number is divisable by any number to 0, set it to not prime.
                        primeValidation = false;
                    }
                }
            }

//if the input is either 0 or 1 set it to false and 0 and 1 is not prime

            else {
                primeValidation = false;
            }
            if(primeValidation == true) {
                listOfPrime.add(i);
            }else {
                primeValidation = true;
            }

        }

        System.out.println("The list of prime numbers of the number " + input + " are "+listOfPrime);

    }

}

The first thing I'd suggest is break out code segments into functions.  This keeps your Main free of clutter and makes the flow easier to understand.

You don't mention which IDE you're using.  If it doesn't have the option for you to format your code according to acceptable guidelines, I would seriously consider changing programs.  There are several well established IDE's freely available.

Your, check for prime, algorithm, while naive, could do with some optimization.  Check for 0,1, and 2 separately.  If the number is bigger than 1 then add 2 to the list.  The loop should start at 3 and step by 2.  As was pointed out, all primes larger than 2 are odd.

There are much better algorithms for getting a list of prime numbers, not the least of which is the Sieve of Eratosthenes.  With this algorithm it is very easy to make a master list of primes then build the sublist that you want.


Replacing the second word of some matching lines in a file I have a file that at some point contains a line of the following form:

OutputFilename output.root

I want to write a Python function that replaces the text after "OutputFilename" and saves to a new file.

I have made the following attempt, which works, but I'm sure there must be a more efficient, Pythonic way to do this.

def set_cuts_configuration_output_filename_configuration(
    cuts_filename                 = "cuts.txt",
    output_filename_configuration = "output_new.root",
    output_cuts_filename          = "cuts_new.txt"
    ):
    # Access the cuts configuration.
    with open (cuts_filename, "r") as input_file:
        cuts_file = input_file.readlines()
    # Find the output filename line.
    modified_file = []
    for line in cuts_file:
        if "OutputFilename" in line:
            modified_file.append(line.split(" ")[0] + " " + output_filename_configuration + "\n")
        else:
            modified_file.append(line)
    # Save the new file content.
    output_cuts_file = open(output_cuts_filename, "w")
    for line in modified_file:
        output_cuts_file.write(line)
    output_cuts_file.close()

Here are some comments to your solution:

Try avoiding reading the entire files into memory – Using readlines() means you read the entire file into memory, which can be eat a lot of memory. Usually it is better to read and process the file line by line
Use with open(...) for pythonic file reading/writing – This can be used both for reading and writing files, and can also be combined into a double with in your case to read and write at the same time
Make functions a little more generic – To have a function to change just one explicitly named config value seems a little strange. Two natural extensions: 1) Allow for an arbitrary config value to be changed, 2) Allow for multiple config values to be changed in one go.
Note that "OutputFilename" in line matches in the entire line – If you write "OutputFilename" at end of line it matches, or at start, or as part of another word. This could lead to a lot of failing changes.
Set the new config line directly – If you have a positive hit on the line you want to change, you know how the line is supposed to look, so no need to split, append and change it. Set it directly to the correct value.
Add some blank lines in your script to make it easier to read – A lot of people are afraid of having blank lines in script, and that is said. It can make the script a lot easier to read.

Refactored code

Here is two functions (within a program) to illustrate a better approach:

def change_configuration_option(
    option,
    new_value,
    sourcefile = "cuts.txt",
    destinationfile = "cuts_new.txt"):
    """Change option_name in sourcefile to new_value in destinationfile."""

    with open(sourcefile) as source, open(destinationfile, "w") as destination:
        for line in source:

            if option in line: # Buggy match on line...
                destination.write('{} {}\n'.format(option, new_value))
            else:
                destination.write(line)

def change_configuration_options(
    change_options,
    sourcefile="cuts.txt",
    destinationfile = "cuts_multiple.txt"):
    """From sourcefile to destinationfile do change_options."""

    with open(sourcefile) as source, open(destinationfile, "w") as destination:
        for line in source:

            # For every line, match against new_options
            for option in change_options:
                if line.startswith(option + ' '):
                    destination.write('{} {}\n'.format(option, change_options[option]))
                    break

            else:
                destination.write(line)

def main():
    """Replace output.root with new_output.root in config file."""

    change_configuration_option("OutputFilename", "new_output_file.txt")

    change_configuration_options(
        { "OutputFilename" : "new_output_file.txt",
          "YetAnother" : "bites the dust",
          "AnotherConfig" : "And another one",
         })

if __name__ == '__main__':
    main()

The change_configuration_options() takes a dictionary of new configuration option changes, and it uses a little trick within the inner loop to check write non-matching options to the output file. That trick is the for ... else loop. If the for loop completes normally, the else will execute, but if a matching option is found and it break's out of the loop then the else: will not be executed.

Also note that in the multiple variant, I've changed the matching on the line to be line.startswith(option + " "), which limits the option to be at the start of the line, and a complete word followed by a space. This avoids false hits, as are present in your original code and in my change_configuration_option() for changing the single option. (Left as proof of concept, see tests below)

Test run

I tested this using the following cuts.txt file:

MyOutputFilename Not_this_file
OutputFilename this_file_should_change
# Next line should have value "no change"
OutputFilenameAndSome no change
# Multiple
AnotherConfig AnotherValue
YetAnother config value with multiple spaces
# Flawed
FinalFlaw What if the value has OutputFilename as part of text?

And after running, I got the file cuts_new.txt:

OutputFilename new_output_file.txt
OutputFilename new_output_file.txt
# Next line should have value "no change"
OutputFilename new_output_file.txt
# Multiple
AnotherConfig AnotherValue
YetAnother config value with multiple spaces
# Flawed
OutputFilename new_output_file.txt

Notice the duplicated line near start, due to falsely changing the OutputFilenameAndSome option, and the changed bottom line when comparing to original file.

And finally the file cuts_multiple.txt:

MyOutputFilename Not_this_file
OutputFilename new_output_file.txt
# Next line should have value "no change"
OutputFilenameAndSome no change
# Multiple
AnotherConfig And another one
YetAnother bites the dust
# Flawed
FinalFlaw What if the value has OutputFilename as part of text?

This file has only the intended changes. The latter option should be a better alternative for changing lines in a file, and should be quite efficient as well. A further optimisation, could be to remove already changed option from the change_options dict after doing that line, but then again that could be unneccessary micro optimisation.

The provided solutions reads and writes the file in one go, and only keeps one line of the file in memory (except internal cacheing). In most operating systems, this should be the better option for handling an arbitrarily sized file. (Earlier Windows version could loose some efficiency due to the almost simultaneously read and write to two different places/files at the same time.)


jQuery serializeObject function I am working on a small serialization function which serializes form input to json.

Here is the fiddle:
http://jsfiddle.net/jdQfj/2/

I have currently no idea how to split up the array declarations let alone a combination of array and object declaration. However the function works good with infinite object nestings and normal values. But I could believe that there is optimization room.

jQuery.fn.serializeObject = function() {
    var o = {};
    var a = this.serializeArray();

    $.each(a, function() {

        // check if object
        if (this.name.indexOf('.') !== -1) {
            var path = this.name.split('.');

            var current = o;
            for (var i = 0; i < path.length; i++) {
                if (i === (path.length - 1)) {
                    current[path[i]] = this.value;
                } else {
                    if (current[path[i]] === undefined) {
                        current[path[i]] = {};
                    }
                }
                current = current[path[i]];
            }

        // check if array
        } else if (this.name.indexOf('[') !== -1 && this.name.indexOf(']')) {
            console.log(this.name + ' is an array');

            // has to get implmented

        // normal value
        } else {
            o[this.name] = this.value;
        }

    });

    return o;
};

Here's what I came up with:

var addNestedPropToObj = function (obj, name, value) {
    var path = name.split('.'),
    current = obj,
    len = path.length - 1,
    i = 0;
    for (; i < len; i++) {
        current[path[i]] = current[path[i]] || {};
        current = current[path[i]];
    }
    if ( 0 < path[i].indexOf( "[]" ) ) {
        name = path[i].replace('[]', '');
        current[name] = current[name] || [];
        current[name].push(value);
    } else {
        current[path[i]] = value;
    }
    return obj;
};
jQuery.fn.serializeObject = function () {
    var o = {},
    a = this.serializeArray(),
    i = 0,
    len = a.length;
    for (; i < len; i++) {
        o = addNestedPropToObj(o, a[i].name, a[i].value);
    }
    return o;
};

Demo and test cases

Tips:

Always try to separate most of the logic from the plugin into small testable functions.
Use a regular loop instead of the $.each().
String.prototype.split() returns an array of the entire string if a match isn't found. So the else condition isn't required.

// not needed
else {
    o[this.name] = this.value;
}

Instead of checking to see if you're at the end of the loop, iterate to path.length - 1 then afterwards perform the statement, current[path[i]] = this.value;.

As for the array format contained within the name property, for simplicity I suggest that the array notation should be an endpoint where the value is the push to the current property. So for a value to a added to an array, then [] must be at the end of the name.

Here's an example:

var func = function( str ){
    return JSON.stringify( addNestedPropToObj( {}, str, 1));
};
func("a[].b") === '{"a[]":{"b":1}}';
func("a.b[]") ===  '{"a":{"b":[1]}}';

Seen as an array: a[] or a.b[]

Seen as a string: a[b], a[b.c]


Is the sum of 2 numbers in list k I got a coding problem to solve and was wondering if I got a commonly acceptable and efficient/clean solution. If there is a better way I'd gladly hear it.

  given a list with numbers return if the sum of 2 elements in the list = k

def is_sum_of_2nums_in_list_k(list, k):
    for i in list:
        if k - i in list:
            return True
    return False

They also mentioned you get bonus points if you do it in one 'pass'. What does this mean?

You are using one explicit pass over the list (for i in list), and one implicit pass over the list (k - i in list).  This means your algorithm is \$O(N^2)\$.

Note: You can reduce your implementation to one line:

return any(k - i in list for i in list)

But you have a bug.   is_sum_of_2nums_in_list_k([5], 10) returns True.

A one pass algorithm

starts with an empty set,
takes each number in the list,

tests if k-i is in the set,

return true if it is,

adds i to the set.

returns false


Object-oriented Linux networking library I needed to perform network communications in my Linux C++ project. I thought that it was a good idea to operate on a higher level of abstraction than raw system calls. Also I love OO design. So I'm implementing an object-oriented high-level Unix network open-source library (There are issues with tab-size if you watch source code on github, but the tabs are aligned well if watching in a text editor).

At the moment the library is partially implemented and is in a process of development. I've already tested some parts of it in real project and I find it's high level of abstraction extremely convenient.

The library checks all system calls for errors, so user shouldn't worry about checking every function's return status (thou exception system is still under construction). It allows easily convert socket addresses and strings in both directions. Socket classes provide convenient methods to get and set their properties, messages are sent and received as strings. TCP socket can recieve messages separated by terminator character or messages determined by required message size e.t.c.

I would like to know community's opinion if I made some design or implementation mistakes, if there could be done some improvements and if the library could be useful in some way to somebody. Any feedback is appreciated.

Here is a working example of a UDP echo-server implementation using my library.

#include <iostream>
#include <Unet/UdpSocket.hpp>
#include <Unet/Ipv4Address.hpp>

int main ( int argc , char** argv )
{

    //  Check program parameters to be provided
    if ( argc != 3 )
    {
        std::cout << "Usage: UdpEchoServer $(SERVER_IP) $(SERVER_PORT)" << std::endl;
        return -1;
    }

    //  Create and open UDP socket with Internet family address
    Unet::UdpSocket socket;
    socket.open();
    std::cout << "Server socket was opened successfully." << std::endl;

    //  Set server socket to reuse address
    socket.setOption(SO_REUSEADDR,1);
    std::cout << "Server socket option \"SO_REUSEADDR\" was set to \"1\"." << std::endl;

    //  Bind socket to the specified local address
    socket.bind(Unet::Ipv4Address(argv[1],argv[2]));
    std::cout << "Server socket was bound successfully." << std::endl;

    //  Datagram which will be recieved and sent back
    Unet::Datagram datagram;

    while ( true )
    {
        try
        {
            if ( socket.hasUnreadData() )
            {
                //  Recieve incomming datagram
                datagram = socket.recieveDatagram();
                std::cout << "[ <-- ] [" << Unet::Ipv4Address(datagram.address).toString() << "] - " << datagram.message << std::endl;

                //  And send it back
                socket.sendDatagram(datagram);
                std::cout << "[ --> ] [" << Unet::Ipv4Address(datagram.address).toString() << "] - " << datagram.message << std::endl;
            }
        }

        //  Catch and print all exceptions
        catch ( Unet::Exception exception )
        {
            std::cout << "Unet exception: " << exception.getMessage() << std::endl;
        }
        catch ( std::exception exception )
        {
            std::cout << "Standard exception: " << exception.what() << std::endl;
        }
        catch ( ... )
        {
            std::cout << "Unknown exception" << std::endl;
        }
    }
}

P.S. I'm not a Linux specialist, so I doubt which name to choose for the library Unix-net, Linux-net or Posix-net. Which is more appropriate and logical?

As Loki stated: don't re-invent the wheel. In the interest of learning and sharing, here are some ideas to consider:

if ( argc != 3 )

Use an API already designed for parsing the command line. There must be dozens.

socket.setOption(SO_REUSEADDR,1);

Since this is an object, I would hide the dependency as follows:

socket.reuseAddress();

Since this is a networking object, I would prefer not to write the above statement at all. Rather, if the socket mustn't reuse the address, the API should provide a method to disable the behaviour.

socket.hasUnreadData()

A socket typically doesn't offer direct data reading and writing. The reason is that a data stream is a much more reusable concept. Were a developer to use your API, they would have to code functionality separately for reading from a socket vs. reading from a file. This restriction seems to limit the potential of software developed. Rather:

InputStream *dataIn = new SocketInputStream();
OutputStream *dataOut = new SocketOutputStream();
socket.setSource( dataIn );
socket.setDestination( dataOut );

Now functionality can be written that reads data from any type of stream, making the source of the data independent (socket, file, database). If you absolutely must have the socket read/write datagrams, then use a terser method name:

socket.isReady()

This line:

socket.bind(Unet::Ipv4Address(argv[1],argv[2]));

Could be:

socket.bind(argv[1],argv[2]);

This simplifies the API for the developer. Otherwise all developers of the API will have to reference Ipv4Address. They shouldn't have to care. It's great to have the flexibility of putting in an Ipv6Address, but that can be handled automatically.

Regarding the exception handling:

        std::cout << "Unet exception: " << exception.getMessage() << std::endl;
        std::cout << "Standard exception: " << exception.what() << std::endl;

Having the API override what(), eliminates the duplication:

        std::cout << "Exception: " << exception.what() << std::endl;

The what() method will differentiate the exception classes in its message.

One more pet peeve:

while ( true )

Since the program doesn't actually continue forever, the loop should have a logical termination condition that tells the next person reading the source code when the program actually terminates. Some examples:

while ( socket.isOpen() )      // Exit upon socket closure
while ( isRunning() )          // Exit upon user request
while ( !command.is( EXIT ) )  // Exit when the QUIT command is received

Regarding the bind method, consider the following pseudo-code:

void Socket::bind( String host, String port ) {
  Address *address = AddressFactory.newInstance( host );
  bind( address, atoi( port ) );
}

Since the socket class already knows about Unet::Ipv4Address, adding a convenience method makes the class easier to use. Java gained popularity for a several reasons, including the simplicity of its networking API for trivial tasks. Using an AddressFactory, internally, will allow programs to work with either IPv4, IPv6, or any as-of-yet-invented protocol in the future. In C++, the application might have to be recompiled, but in Java, porting from IPv4 to IPv6 addresses has been mostly transparent.

If developers are forced to hard-code a reference to IPv4 in their application, then they will have to do extra work to make the software use a different Internet Protocol version.


Spotify's "Reversed Binary Numbers" Problem I wrote some Java code for Spotify's Reversed Binary Numbers puzzle and I tested it on my machine and I am almost positive that it behaves as expected. However, when I send it to puzzle AT spotify.com it kicks it back saying I have the "wrong answer", and nothing else.

Frustrating as this is, I can't for the life of me seem to figure out why the code I have wouldn't work. I even went so far as to remove all error messages that result from bad input, thinking that maybe that was causing it to fail. I was wondering if anyone here with a keener eye than my own could possibly help me hunt down the bug?

Here is a snippet of the most relevant portion of the code:

/**
 * Reverse the bits of an integer between 1 ≤ n ≤ 1000000000.
 *    i.e. 11 (1011) becomes 13 (1101)
 *
 * @param n The integer to reverse.
 * @return The reversed integer.
 */
public static int reverseBits(int n) {
  // 1-indexed MSB Position
  int msbPosition = findMSBPosition(n);
  int reversed = 0;

  for (int i = 0; i < msbPosition; i++) {
    int mask = 1 << i;
    int bit = (n & mask) >> i;
    reversed |= (bit << (msbPosition - i - 1));
  }

  return reversed;
}

/**
 * Find the 1-indexed position of the MSB of a number.
 *    i.e. For the number 11 (1011), findMSBPosition() would return 4.
 *
 * @param n The number to find the MSB for.
 * @return The 1-indexed position of the MSB.
 * @protected
 */
protected static int findMSBPosition(int n) {
  int msbPos = 0;
  int testNum = 1;
  while (testNum <= n) {
    msbPos++;
    testNum <<= 1;
  }

  return msbPos;
}

The full code can be found in this gist.

As a caveat and as information I am NOT APPLYING TO SPOTIFY NOR HAVE ANY INTENTION TO. I'm just doing this to stay sharp and since "wrong answer" is less than helpful output I'd love to know where I messed up. Thank you so much.

I do not see any problems inside the given range. I would have used already existing methods:

public static int reverse2(final int n) {
    return Integer.valueOf(new StringBuilder(Integer.toBinaryString(n)).reverse().toString(), 2);
}

Which is probably to best readable version.

If we want to do it by ourself and do not want to create a string and stringbuilder:

With recursion:

public static int reverse3(final int newNumber, final int number) {
    if (number == 0)
        return newNumber;
    return reverse3((newNumber << 1) | (number & 1), number >> 1);
}

Without recursion:

public static int reverse4(int number) {
    int newNumber = 0;
    while (number != 0) {
        newNumber = (newNumber << 1) | (number & 1);
        number >>= 1;
    }
    return newNumber;
}

Hint: I have only considered the given range. I did not think about negative numbers.

You could try to avoid Scanner and use Integer.parseInt inside a try/catch. Who knows what they throw at you at the command line. The nextInt from Scanner does a Integer.parseInt, but it also tries to match tokens.


Gson to Model containing several return types for one key i just had a problem converting the JSON from Phillips Hue Lights (Response) to a Model. Since it may contain several "unknown" keys in success and/or may contain just a String for success i had to write/use a TypeAdapter and a deserializer aswell. This works but im pretty sure there is a better way to solve this.

Dummy JSONS:

[{"success":{"/config/name":"My bridge"}}]

and

[{
    "success":{"id": "3"}
}]

and

[
{"success":{ "address": "/groups/1/action/on", "value": true}},
{"success":{ "address": "/groups/1/action/effect", "value":"colorloop"}},
{"success":{ "address": "/groups/1/action/hue", "value":6000}}
]

And the JSON with just a String

[{
    "success": "/config/whitelist/1234567890 deleted."
}]

It may also contain "error" but this isnt the problem. Since the object in address is unknown i use a Adapter from https://stackoverflow.com/questions/17627869/simple-way-to-strip-outer-array-of-responses-in-gson which works fine as long as its an Object. Since it may also contain a String i had to create a JsonDeserializer which validates if its a JSONObject and if not, it sets another variable in the Model with the message. It works, but i always have to revalidate if the message or success it set.

This is my JSONDeserializer

public class HueResponseDeserializer implements JsonDeserializer<HueResponse> {
    private final String TAG = HueResponseDeserializer.class.getSimpleName();

    @Override
    public HueResponse deserialize(JsonElement jsonElement, Type type, JsonDeserializationContext context) throws JsonParseException {

        HueResponse response = new HueResponse();
        JsonObject jsonObject = jsonElement.getAsJsonObject();
        if (jsonObject.get("success").isJsonObject()) {
            for (Map.Entry<String, JsonElement> entry : jsonObject.getAsJsonObject("success").entrySet()) {
                response.setSuccess(new HashMap<String, Object>() {{
                    put(entry.getKey(), entry.getValue());
                }});
            }
        } else {
            response.setSuccessMessage(jsonObject.get("success").getAsString());
        }

        if (jsonObject.has("error") && jsonObject.get("error").isJsonObject()) {
            for (Map.Entry<String, JsonElement> entry : jsonObject.getAsJsonObject("error").entrySet()) {
                response.setError(context.deserialize(entry.getValue(), HueResponse.Error.class));
            }
        }
        return response;
    }
}

And my Model

public class HueResponse {

    private Map<String, Object> success;
    private Error error;
    private String successMessage;

    public String getSuccessMessage() {
        return successMessage;
    }

    public void setSuccessMessage(String successMessage) {
        this.successMessage = successMessage;
    }

    public Map<String, Object> getSuccess() {
        return success;
    }

    public Error getError() {
        return error;
    }

    public void setSuccess(Map<String, Object> success) {
        this.success = success;
    }

    public void setError(Error error) {
        this.error = error;
    }

    public static class Error {
        private int type;
        private String address;
        private String description;

        public int getType() {
            return type;
        }

        public String getAddress() {
            return address;
        }

        public String getDescription() {
            return description;
        }

        @Override
        public String toString() {
            return "Type: " + this.type
                    + " Address: " + this.address
                    + " Description: " + this.description;
        }
    }

}

Iterating through an entrySet()

If what you really want is to put all the JSON objects into a success map, rather than overwriting each of them with every iteration, there's a way to stream it:

Map<String, Object> map = object.entrySet().stream()
                                .collect(Collectors.toMap(Entry::getKey,
                                            entry -> (Object) entry.getValue()));
result.setSuccess(map);

In any case, a more fluent implementation of HashMap<>() {{ ... }} is to use Collections.singletonMap():

object.entrySet().forEach(entry -> {
    Map<String, Object> map = Collections.singletonMap(entry.getKey(), entry.getValue());
    // ...
});

Modeling

You can consider have a more domain-based modeling of the successful results, instead of a simple Map<String, Object> in your HueResponse. For example, consider a Group of Action objects that have a state consisting of enabled (aka on), effect and hue? That seems to adhere closer to the results, giving you a more meaningful application.


Validating partially filled objects The following classes are a simplification of an auto-generated code of an ORM (targeting Microsoft's Dynamics CRM):

public class Entity
{
    public Dictionary<string, object> Fields { get; set;}
    public virtual T GetFieldValue<T>(string fieldName)
    {
         return (T)Fields[fieldName];
    }
    protected virtual void SetFieldValue(string fieldName, object value)
    {
         Fields[fieldName] = value;
    }
}
partial class Account : Entity
{
    public string Name
    get
    {
         return GetFieldValue<string>("Name");
    }
    set
    {
         SetFieldValue("Name", value);
    }
    public string Email
    get
    {
         return GetFieldValue<string>("Email");
    }
    set
    {
         SetFieldValue("Email", value);
    }
}

When an Entity's derived class is retrieved from the database, the base class Fields property is automatically assigned (again, this is not something in my control) with the selected fields.

I'd like a review of the following class, intended to validate "partially" filled objects:

public class EntitySubset<T> : Entity where T : Entity
{
    public EntitySubset(T subset)
    {
        foreach (var field in subset.Fields)
        {
            SetFieldValue(field.Key, field.Value);
        }
    }
    public override U GetFieldValue<U>(string fieldName)
    {
        if (!Fields.ContainsKey(fieldName))
        {
            throw new Exception($"{typeof(T).Name} must contain {fieldName} as a key!");
        }
        return base.GetFieldValue<U>(fieldName);
    }
}

an example usage:

private List<Account> GetSimiliarAccounts(EntitySubset<Account> targetAccount)
{
    var similarityFields = new[] { "Name", "Email" };
    var similiarAccounts = db.Where(account =>
       similarityFields.All(f =>
          account.GetFieldValue<object>(f) == targetAccount.GetFieldValue<object>(f))
    ).ToList();
    return similiarAccounts;
}

so that if GetSimiliarAccounts is called as follows:

GetSimiliarAccounts(new EntitySubset<Account>(new Account { Email = "b" }));

then the output is:

System.Exception: 'Account must contain Name as a key!'

Note that EntitySubset<T> and its error messaging is intended for developers to "fail fast", namely: to quickly spot and fix bugs stemming from a partially filled object.

What do you think of EntitySubset<T> as a validation implementation?

Review

You should avoid hardcoded strings. Take advantage of nameof. Also, expression-bodied members (arrow notation) makes code cleaner to read.

public string Name
{
    get => GetFieldValue<string>(nameof(Name));
    set => SetFieldValue(nameof(Name), value);
}

EntitySubset<T>'s method GetFieldValue makes sure a clean (but technical) error message is thrown whenever an asked field is not available.

public override U GetFieldValue<U>(string fieldName)
{
    if (!Fields.ContainsKey(fieldName))
    {
        throw new Exception($"{typeof(T).Name} must contain {fieldName} as a key!");
    }
    return base.GetFieldValue<U>(fieldName);
}

However, don't fallback to this method to perform business rules checks. The business layer should check that mandatory fields are filled in, before mapping the entities to the data layer.

You should also optimize ContainsKey and base.GetFieldValue. You don't want to perform 2 lookups here. Implement something like a TryGetFieldValue.


Reading in different datatypes from a file I'm making a database program that reads from a file info on tennis players. Everything works, but is there a simpler way of doing this?  Is there anything I can improve here?

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

const int MAX_PLAYERS = 5;

struct player {
    char *fullname[2];
    int age;
    double rating;
    char *racketBrand;
}; typedef struct player tPlayer;

void readData(tPlayer *player){
    FILE *file = fopen("database", "r");
    if(!file){
        perror("File not found");
        return;
    }

    int idx = 0;
    char buffer[50];
    while(fgets(buffer, sizeof(buffer), file)){
        player[idx].fullname[0] = malloc(20);
        player[idx].fullname[1] = malloc(20);
        player[idx].racketBrand = malloc(20);
        sscanf(buffer, "%s %s %d %lf %s", player[idx].fullname[0], player[idx].fullname[1],
            &player[idx].age, &player[idx].rating, player[idx].racketBrand);
        idx++;
    }
    fclose(file);
}

void printInfo(tPlayer *player){
    int idx;
    for(idx = 0; idx < MAX_PLAYERS; idx++){
        printf("***Name: %s %s ***Age: %d ***Rating: %.1lf ***Racket Brand: %s\n", player[idx].fullname[0],
            player[idx].fullname[1], player[idx].age, player[idx].rating, player[idx].racketBrand);
    }
}

int main(){
    tPlayer player[MAX_PLAYERS];
    readData(player);
    printInfo(player);
}

This part hardly makes sense to me:

    player[idx].fullname[0] = malloc(20);
    player[idx].fullname[1] = malloc(20);
    player[idx].racketBrand = malloc(20);

If you are always allocating 20 bytes for your strings, why bothering with a dynamic allocation? Simply declare your strings as char whatever[20];. If you want your dynamically allocated strings to actually allocate as much memory as is needed, things will go far more complicated (see my old answer here, although you'll need to adapt it).

Your code does not allow for a tennis player to have more than one name and one surname, but I'll assume that this is on purpose. However, if we assume that your data file is not broken in a very specific way (i.e., having one player's data spread across more than one line), we could simply do this (assuming we also abandon dynamic memory allocation):

while (fscanf(file, "%19s %19s %d %lf %19s", player[idx].fullname[0], player[idx].fullname[1], &player[idx].age, &player[idx].rating, player[idx].racketBrand) == 5) idx++;

Unless there is a reason to assume that there are at most MAX_PLAYERS in the file, it would be better to dynamically allocate player (the main array), using realloc. This is, of course, more complicated than your attempt, but it is also less limiting for the user.

If you want to keep the limit on the number of players, you should also check it when reading the data, i.e., you should replace idx++ with

if (idx++ >= MAX_PLAYERS) break;

And a minor technical notion. You can replace

struct player {
    char *fullname[2];
    int age;
    double rating;
    char *racketBrand;
}; typedef struct player tPlayer;

with

typedef struct player {
    char *fullname[2];
    int age;
    double rating;
    char *racketBrand;
} tPlayer;

I am all for using typedef, but it is fair to warn you that there are some disagreements about it.


Testable authentication handler for web API I need to unit test my authentication handler. I don't really want do an assert against the text message returned by the handler. How could this be improved ?

protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
{
    // Check for API key
    if (!request.Headers.Contains(AuthConfig.ApiKeyHeader))
    {
        return SendResponseText("ApiKey is required");
    }

    // Check for timestamp
    if (request.GetQueryNameValuePairs().Where(k => k.Key.ToLower() == "timestamp").Count() != 1)
    {
        return SendResponseText("Timestamp is required");
    }

    // Validate timestamp
    if (IsValidTimestamp(request) == false)
    {
        return SendResponseText("Invalid timeStamp");
    }

    // Check for signature
    if (request.Headers.Authorization == null || request.Headers.Authorization.Scheme != AuthConfig.AuthenticationScheme)
    {
        return SendResponseText("Signature is required");
    }

    // Validate API Key
    string apikey = request.Headers.GetValues(AuthConfig.ApiKeyHeader).First();
    UserDTO user = _userRepo.GetUserAuthInfo(apikey);
    if (user == null)
    {
        return SendResponseText("Invalid API key");
    }

    // Validate signature
    string signature = _signatureCalculator.GetSignature(user.Secret, request.RequestUri.OriginalString);
    if (request.Headers.Authorization.Parameter != signature)
    {
        return SendResponseText("Invalid signature");
    }

    return base.SendAsync(request, cancellationToken);
}

private Task<HttpResponseMessage> SendResponseText(string text)
{
    Logger.Error(text);

    var response = new HttpResponseMessage(HttpStatusCode.Unauthorized);
    response.ReasonPhrase = text;
    response.Content = new StringContent(text);

    var tcs = new TaskCompletionSource<HttpResponseMessage>();
    tcs.SetResult(response);

    return tcs.Task;
}

So far, I have put the custom reason phrases in a constant

public sealed class AuthError
{
    public const string ApiKeyMissing = "ApiKey is required";
    ...
}

And my tests look like this

[TestMethod]
public void SecurityHandler_ApiKeyMissingFromHeader_ReturnsUnauthorizedHttpStatus()
{
    var handler = new SecurityHandler(new Mock<IClientRepository>().Object, new Mock<ISignatureCalculator>().Object);

    var httpRequestMessage = new HttpRequestMessage(HttpMethod.Get, "http://localhost");

    var client = new HttpClient(handler);
    var result = client.SendAsync(httpRequestMessage).Result;

    Assert.AreEqual(result.StatusCode, HttpStatusCode.Unauthorized);
    Assert.AreEqual(result.ReasonPhrase, SecurityHandler.AuthError.ApiKeyMissing);
}

I'm not too happy with this, and I'm doing 2 asserts which is bad practice.. Any ideas ?

Basically you need somehow confirm that

StatusCode == HttpStatusCode.Unauthorized
ReasonPhrase == someDefinedConstant

If you only Assert the first, you can't be sure that the logic is flawless. If you only test the second you can't be sure that StatusCode == HttpStatusCode.Unauthorized.

But again, proper unit tests should fail exactly for one reason.

As others had this problem also, Omer Rauchwerger has created a NUnit addin which can be found here: http://rauchy.net/oapt/

Comments

Comments should be used to describe why something is done. The code itself should describe what is done.
So comments like // Check for API key should be removed and the code following should be extracted to a separate method.

timestamp validation

As the message will be "Timestamp is required" for a failed validation, I guess that you need to check if at least one key is found. There it is better to use Any() over Count().

Style

You should be consistent in your coding style. Here you one time use if (!condition) and another time you use if (condition == false).

Refactoring

Extracted validation methods

private bool ContainsApiKey(HttpRequestMessage request)
{
    return request.Headers.Contains(AuthConfig.ApiKeyHeader);
}

private bool ContainsTimeStamp(HttpRequestMessage request)
{
    return request.GetQueryNameValuePairs().Any(k => k.Key.ToLower() == "timestamp");
}

private bool ContainsSignature(HttpRequestMessage request)
{
    return (request.Headers.Authorization != null && request.Headers.Authorization.Scheme == AuthConfig.AuthenticationScheme);
}

private UserDTO GetUser(HttpRequestMessage request)
{
    string apikey = request.Headers.GetValues(AuthConfig.ApiKeyHeader).First();
    return  _userRepo.GetUserAuthInfo(apikey);
}

private bool IsValidUser(UserDTO user)
{
    return user != null;
}

private bool IsValidSignature(UserDTO user, HttpRequestMessage request)
{
    string signature = _signatureCalculator.GetSignature(user.Secret, request.RequestUri.OriginalString);
    return  request.Headers.Authorization.Parameter == signature
}

and the cleaned up former method

protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
{

    if (!ContainsApiKey(request))
    {
        return SendResponseText(SecurityHandler.AuthError.ApiKeyMissing);
    }

    if (!ContainsTimeStamp(request))
    {
        return SendResponseText(SecurityHandler.AuthError.TimeStampMissing);
    }

    if (!IsValidTimestamp(request))
    {
        return SendResponseText(SecurityHandler.AuthError.TimeStampInvalid);
    }

    if (!ContainsSignature(request))
    {
        return SendResponseText(SecurityHandler.AuthError.SignatureMissing);
    }

    UserDTO user = GetUser(request);
    if (!IsValidUser(user))
    {
        return SendResponseText(SecurityHandler.AuthError.ApiKeyInvalid);
    }

    if (!IsValidSignature(user,request))
    {
        return SendResponseText(SecurityHandler.AuthError.SignatureInvalid);
    }

    return base.SendAsync(request, cancellationToken);
}


High performance wrapper for CSPRNG arc4random(3) This function is a wrapper for arc4random(3), a cryptographic pseudo-random number generator on macOS and BSDs, but you can also get it by installing libbsd-devel on most Linux distros.

It serves the same purpose as arc4random_uniform(3), the official wrapper for arc4random(3), i.e. to generate a nonnegative random integer less than range.

static inline unsigned random_uniform(unsigned range) {
    static uint32_t random_32b;
    static uint32_t full_32b;

    if (full_32b < range) {
        random_32b = arc4random();
        full_32b = UINT32_MAX;
    }

    unsigned result = random_32b % range;
    random_32b /= range;
    full_32b /= range;

    return result;
}

You can find the implementation of arc4random_uniform(3) on macOS at the bottom of this page. Basically, it makes use of the rejection method to produce uniform deviates and eliminate the modulo bias. On the other hand, my implementation tries to eliminate the modulo bias by keeping track of the maximum possible range of the remaining random bytes in full_32b. What's more, it saves the unused random bytes in a static variable for future use, as a call to arc4random(3) can be costly. It appears to be faster according to my benchmark, but I'm not sure if it's correct.

result = random % range

You can only do this if range is constrained to be much smaller than the maximum value of random.  Imagine a 2-bit PRNG that returns a value from 0 to 3.   When your function is invoked with range 3, return value 0 will be twice as common as 1 or 2.

random_32b /= range;
full_32b /= range;

This is going to exhibit obvious bias when the result has a small number of bits.  Imagine a range of 3 and a 4-bit random pool.  On exit, full_32b will equal 5 but random_32b will only equal 5 one time in 15.

There are three good ways to go about this:

choose an acceptable margin of bias (expressed as a fraction);
divide range by that fraction;
select enough bits to make max(random) larger than the result of the division;
return a simple modulus.

For example, 1% bias is 0.01.  If the range is 10, the random number needs a maximum larger than \$10/0.01 = 1000\$.
That's ten bits.

select enough bits to represent range
repeat until the result is smaller than range.

Example: for a range of 10, consume four bits and discard results larger than 9.

select enough bits to represent range, plus a couple more, to represent a multiple of range.  Call the multiplier \$i\$.
repeat until the result is smaller than \$i * \$range
divide result by \$i\$

This last approach combines the advanatages of first two: you get bias-free numbers and mostly predictable runtime.

Example: approach #2 with a range of 9 consumes four bits per attempt, and discards almost half of the results.  With 5 bits, instead of 4, you keep results from 0-26 (i.e., range\$*3\$) and divide them by 3.  Discard 27 or larger—that's only 5/32 attempts wasted.  With 6 bits and range of 9, values 0-62 are usable (range\$*7\$), and only one possible value (63) will be discarded.

In all cases, consume bits by bit-shifting (or dividing by powers of two).  Integer division with arbitrary denominators won't work.


Getting a set of subscribers from a subscriberMap In my Java event bus project, I have a private generic method that retrieves a Set<Subscriber<E>> from a private Map<Class<? extends Event>, Set<Subscriber<? extends Event>>> (the subscriberMap).

/**
 * A map of Event types and Subscriber Sets.
 *
 * That is, the Set of Subscribers set to an Event type
 * is assumed to be used for that Event type.
 */
private final Map<Class<? extends Event>, Set<Subscriber<? extends Event>>> subscriberMap = new ConcurrentHashMap<>();

private <E extends Event> void checkSubscriberMap(Class<E> type)
{
    if (!subscriberMap.containsKey(type))
    {
        subscriberMap.put(type, new HashSet<>());
    }
}

/**
 * Gets a Set of Subscribers by an Event.
 *
 * @param event the event
 * @param <E> the type of event
 * @return a Set of Subscribers
 */
@SuppressWarnings("unchecked")
private <E extends Event> Set<Subscriber<E>> getSubscribers(E event)
{
    return (Set<Subscriber<E>>) (Object) getSubscribers(event.getClass());
}

/**
 * Gets a Set of Subscribers by an Event type.
 *
 * @param type the event class
 * @param <E> the type of event
 * @return a Set of Subscribers
 */
@SuppressWarnings("unchecked")
private <E extends Event> Set<Subscriber<E>> getSubscribers(Class<E> type)
{
    checkSubscriberMap(type); // checks if subscriberMap.get(type) is null

    return (Set<Subscriber<E>>) (Object) subscriberMap.get(type);
}

Assuming nobody uses reflection to modify subscriberMap, the public methods that modify subscriberMap only work if the Subscriber<? extends Event> is the same type as the Class<? extends Event>. Therefore, I can be sure that getSubscribers() will always return a Set of the same type that you give it.

/**
 * Registers a single event handler under an explicit Event type.
 * Returns true if the handler was added.
 *
 * @param subscriber the Subscriber
 * @param type the type of Event
 */
public <E extends Event> boolean register(Subscriber<E> subscriber, Class<E> type)
{
    if(type == Event.class)
    {
        throw new IllegalArgumentException("The provided type is not a subclass of Event.");
    }

    return getSubscribers(type).add(subscriber);
}

However, it's bothering me how getSubscribers() looks. Why does it look I'm doing a hacky workaround? Is it necessary that I have to cast subscriberMap.get(type) twice? Is this the right way to go about what I'm trying to do?

private <E extends Event> void checkSubscriberMap(Class<E> type)

This method checks nothing. It ensures that there's a non-null value. Or it fixes or prepares something.

Note that you're using both contains and get every time. I guess, a method like

private <E extends Event> Set<E> safeGet(Class<E> type) {
   Set<E> result =
           (Set<Subscriber<E>>) (Object) subscriberMap.get(type);
   if (result == null) {
        result = new HashSet<>();
        subscriberMap.put(type, result);
   }
   return result;
}

would be better.

Consider using Guava's Multimap as it's been solved there.

@SuppressWarnings("unchecked")
private <E extends Event> Set<Subscriber<E>> getSubscribers(E event)
{
    return (Set<Subscriber<E>>) (Object) getSubscribers(event.getClass());
}

I'm afraid, that's right. The ugly double cast is needed as there's no way to tell the compiler that it works. Maybe the intermediate cast could be (non-generic) (Set).

  Assuming nobody uses reflection

If somebody does, that's not your fault.

What you did is similar in idea to the MutableClassToInstanceMap, except that you need Set<Subscriber<E>> instead of E itself and therefore you can't use type.cast(object) and have to resolve to an unchecked cast.


Stack Exchange User Activity Score About

This is a SEDE query which will calculate your activity and participation on a site. It's geared for Programming Puzzles & Code Golf where questions are values more, but the values can be modified to fit general Q&A SE sites too.

Try it online!

So I did what all programmers do and I looked up how to most of this stuff on Stack Overflow and mashed the snippets from SO into my program until it worked!

Essentially this program takes input for the user's User ID and their score is calculated by::

1  * <# of Upvotes>
2  * <# of Downvotes>
10 * <# of Questions>
5  * <# of answers>
1  * <# of Comments>

The actual values of what <# of ...> is multiplied is stored in variables which you can see I've declared.

Then within the FROM the Upvotes, Downvotes, etc. are all counted. The outmost SELECT is used so I can manipulate these also

Code

DECLARE @UID int = ##UserId##

DECLARE @Up      int = 1
DECLARE @Down    int = 2
DECLARE @Q       int = 10
DECLARE @A       int = 5
DECLARE @Comment int = 1

SELECT Score = Upvotes   * @Up +
          Downvotes * @Down +
          Questions * @Q +
          Answers   * @A +
          Comments  * @Comment,
       Upvotes, Downvotes, Questions, Answers, Comments
FROM (
  SELECT TOP 1
    (
      SELECT UpVotes
      From Users
      WHERE Id = @UID
    ) as Upvotes,
    (
      SELECT DownVotes
      From Users
      WHERE Id = @UID
    ) as Downvotes,
    (
      SELECT COUNT(*)
      FROM Posts
      WHERE OwnerUserId = @UID and PostTypeId = 1
    ) as Questions,
    (
      SELECT COUNT(*)
      FROM Posts
      WHERE OwnerUserId = @UID and PostTypeId = 2
    ) as Answers,
    (
      SELECT COUNT(*)
      FROM Comments
      WHERE UserId = @UID
    ) as Comments
  FROM Users, Comments, Posts
) data

I'd appreciate any comments on better ways to write this program, if I can restructure this better, my formatting, and really anything because I suck at SQL.

TOP 1

When selecting values that correspond to Unique fields referenced by constants (OwnerUserId), you're only really selecting one field, meaning this is redundant.

UVN

Unexplained Variable Names

Don't use names like Q and A, they're confusing at best and unreadable at worst.

DECLARE @Q
DECLARE @A

DECLARE @Questions
DECLARE @Answers

Yo dawg, heard you like SELECTs

SELECT --...
FROM (
  SELECT TOP 1
    (
      SELECT UpVotes
      From Users
      WHERE Id = @UID
    ) as Upvotes,

SQL is not intended to be used like that.

You don't need to create pseudo tables to select from to store your data.

You can use a base table, and then specify certain fields calling other tables from that.

SELECT
  UpVotes + DownVotes as [Total Votes],
  (SELECT COUNT(*) FROM Posts WHERE OwnerUserId = Users.Id) as [Post Count]
FROM Users

Formatting

This formatting is a bit weird:

SELECT Score = Upvotes   * @Up +
          Downvotes * @Down +
          Questions * @Q +
          Answers   * @A +
          Comments  * @Comment,

You might want to break these up a little more.

Naming

These aren't the actual values, they're type weights, so I'd add the word weight to the end of the variable names:

DECLARE @Up      int = 1
DECLARE @Down    int = 2
DECLARE @Q       int = 10
DECLARE @A       int = 5
DECLARE @Comment int = 1

Into:

DECLARE @UpVotesWeight     int = 1
DECLARE @DownVotesWeight   int = 2
DECLARE @QuestionsWeight   int = 10
DECLARE @AnswersWeight     int = 5
DECLARE @CommentsWeight    int = 1


Decimal number to binary number conversion in JavaScript I made this decimal to binary conversion as an exercise for myself and because it is obviously often asked during job interviews / tests.

Any hints concerning possible improvements welcomed.

// -- START : Testing the function --------
var display = document.querySelector('div');
var result = '';
var n = 255;
var maxSpace = n.toString().length;

// Assigning wrong parameter.
console.log('Result empty: ' + getBinaryNumber());
console.log('Result string: ' + getBinaryNumber('abc'));

// Helper function: Avoiding "stairs".
function getSpaces(count, someChar) {

  if (!count) return '';

  var ret = '';

  for (var i = 0; i < count; i++) {
    ret += someChar;
  }

  return ret;
}

var spaces = getSpaces(maxSpace, '0');

for (var i = 0; i <= n; i++) {
  result +=
    (spaces + i).slice(- maxSpace) +
    ' : ' + getBinaryNumber(i, ' ') + '
';
}

display.innerHTML = result;
// -- END : Testing the function --------

// Converts a decimal number to a binary number.

// -- Parameter --------------------------
// Number - The decimal number to convert.

// -- Return -----------------------------
// String - The number in it's binary
// represention.
// On error:
// Returns an empty string.

function getBinaryNumber (decimalNumber, separator) {
  // Parameter check
  if (decimalNumber === 0) return 0;

  if (!decimalNumber) return '';

  if (isNaN(decimalNumber)) return '';

  separator = separator || '';
  // Local variables
  var ret = [];
  var bit = decimalNumber % 2;

  ret.push(bit);

  decimalNumber = ~~(decimalNumber / 2);

  while (decimalNumber > 1) {
    bit = decimalNumber % 2;
    ret.push(bit);

    decimalNumber = ~~(decimalNumber / 2);
  }

  if (decimalNumber > 0) ret.push(decimalNumber);

  return ret.reverse().join(separator);
}

Comment says, that function will return a string, and then returns a number.

if (decimalNumber === 0) return 0

Best improvement would be using a native method

function getBinaryNumber(num, sep) {
    if (typeof num != "number") num = parseFloat(num)
    return sep ? num.toString(2).split("").join(sep) : num.toString(2)
}

I would remove separator parameter, as it makes no sense to combine this functionality.

function getBinaryNumber(num) {
    return (typeof num == "number" ? num : parseFloat(num)).toString(2)
}


Find the highest number satisfying a condition in an infinite range I wrote the below function for doing a variation of bisection search that I came up with. This can be useful when the higher bound is not known. It assumes that as number increases the result of the condition is also increasing.

For example - find the largest classroom size where the probability of two students sharing the same birthday is less than 0.99.

If I have a function that finds the probability of two students sharing the same birthday based on number of classrooms then I can pass that function and perform a bisection search over an infinite range to find the highest number.

def largest_satisfying_condition(condition):
    lo, hi = 1, 3
    mid = None
    lowest_hi_false = float('inf')
    while lo != hi:
        mid = (lo + hi) / 2
        if condition(mid):
            lo, hi = mid, hi * 2

            if hi > lowest_hi_false:
                #Because any value greater than this will give false result
                hi = lowest_hi_false - 1
        else:
            lo, hi = lo, mid
            if hi < lowest_hi_false:
                #No need to go over this when increasing hi
                lowest_hi_false = hi
    return mid

I wanted to know whether this can be written in a better way - according to either algorithm or Python.

if/else inside the loop doesn't look clean. Consider splitting the code into two parts:

    # Find the "good" range

    while condition(hi):
        lo, hi = hi, hi * 2

    # At this point it is known that condition(lo) is True,
    # and condition(hi) is False. A normal bisection search
    # follows.

   return binary_search(lo, hi, condition)

It would be also worthy to factor the setup loop out in its own function.


Inserting new rows into an existing table I want to insert about 10000 new rows into an existing table which already contains 10000 rows. I need to get the insertid for the query and to use that id in another function inside the loop.

foreach($values as $val){
   $sql = "INSERT INTO wp_posts (`post_author`, `post_date`, `post_content`, `post_title`, `post_excerpt`, `post_status`,`post_modified`,`post_type`)
   VALUES('".$val[author]."',NOW(),'".$val[content]."','".$val[title]."','".$val[excerpt]."','published',NOW(),'post')";
   $result = mysql_query($sql);
   $insertid = mysql_insert_id();
   add_post_meta($insertid, $val[metakey], $val[metaval], true );
}

I am fetching the values for the $values array from a CSV file. As of now, I can insert only 20 new rows per 3 minutes. Is there any way to speed up this insertion?

Before I continue, the following must be said:

Stop using mysql_query.

Seriously.  There have been very good replacements for going-on a decade now.  And considering this code does not appear to escape anything at all, you'd do well to learn to use parameterized queries so that it doesn't have to.

With all that said, though, the SQL might not even be necessary.  Is there a good reason you're not using wp_insert_post to, well, insert a post?  Looks like it lets you set everything you want to set, and then some.  It also returns the post ID, so you don't even have to touch the database directly to get it.

It sounds like the problem is not SQL or WordPress, really.  This code should not be as slow as you're describing.  If you have a DB server that's not your web server (which is generally the case in shared hosting and such), there will naturally be a bit of slowdown.  Generally not that much delay, though, unless the DB is hosted in a totally different place (ie: not from your web hosting company).  That, and the possibility that the server (DB or web server, either one) is simply overloaded, are more likely possibilities.


Disposing SQL objects inside another using block In one sql connection I have multiple sql commands and I wonder what approach is better - create and dispose SqlCommand for each operation or use only one sql command?

Eg:

using (SqlConnection connection = new SqlConnection(ConnectionString))
{
   connection.Open();

   using(SqlCommand cmd = new SqlCommand("select...", connection))
   {
       cmd.ExecuteNonQuery();

       cmd.CommandText = @"another query...";
       cmd.ExecuteNonQuery();

       //and so on
   }
}

or

using (SqlConnection connection = new SqlConnection(ConnectionString))
{
   connection.Open();

   using(SqlCommand cmd = new SqlCommand("select...", connection))
   {
       cmd.ExecuteNonQuery();
   }

   using(SqlCommand cmd = new SqlCommand("another query...", connection))
   {
       cmd.ExecuteNonQuery();
   }
}

If SqlCommand(s) are inside another using block (in this cae SqlConnection), disposing will be done after close connection or immediately after running SqlCommand?

I'm asking because I have web api which receives a few hundred requests per minute and my goal is to optimize application for better garbage collector working - there might be memory leak

Yes, you can do this, and the garbage collector will do the job well. This is very close to your original code (snippet 1), but notice the parameter clear instructions.

using (var sqlConnection = new SqlConnection(ConnectionString))
{
   sqlConnection.Open();

   using (var command = new SqlCommand("select...", sqlConnection))
   {
       command.ExecuteNonQuery();
       command.Parameters.Clear();  // <-- IMPORTANT BEFORE RUNNING NEXT COMMAND

       command.CommandText = @"another query...";
       command.ExecuteNonQuery();
       command.Parameters.Clear();  // <-- IMPORTANT BEFORE RUNNING NEXT COMMAND

       // ...etc
   }
}


Implementing Conway's Game of Life Here is an implementation of Conway's Game of Life. My code is working fine but I want you to review and give me your suggestions of how to improve it.

These are the steps of my code:

Initialize the grid cells with zeros (all cells are dead at the start)
Then randomly change some of the grid cells to one (bring some cells to life)
Draw the grid
Update the grid, by checking which cells are going to live or die
Go to step 3

gameOfLife.h

#ifndef GAME_OF_LIFE_H
#define GAME_OF_LIFE_H

#include <vector>
#include <random>

class gameOfLife
{
public:
    typedef std::vector< std::vector<int>> gridVector;

private:
    int gridSize;
    gridVector mainGrid;
    gridVector *gridCopy;

public:
    gameOfLife();

public:
    void run();
    void initGrid();
    void drawGrid();
    void updateGrid();
    void copyGrid ();
    int checkNeighbors(gridVector *grid, int x, int y);
    void nextGeneration(int x, int y, int lod);
    int randGenerate(int start, int end);

private:
    std::random_device randDevice;
    std::mt19937 gen;
};

#endif

implementation.cpp

#include <iostream>
#include "gameOfLife.h";

gameOfLife::gameOfLife() : gridSize(15), gen(randDevice())
{
}

void gameOfLife::initGrid()
{
    for(int i = 0; i < gridSize; i++)
    {
        for(int j = 0; j < gridSize; j++)
        {
            mainGrid[i].push_back(0);
        }
    }

    for(int i = 1; i < gridSize ; i++)
    {
        for(int j = 1; j < gridSize ; j++)
        {
            int randSize     = randGenerate(2 , gridSize);
            int state       = randGenerate(0 , 2);
            int indexRow    = randGenerate(1, randSize );
            int indexColumn = randGenerate(1, randSize );

            mainGrid[indexColumn][indexRow] = state;
        }
    }
}

int gameOfLife::randGenerate(int start, int end)
{
        std::uniform_int_distribution<int> genenNum(start, end - 1);

        return genenNum(gen);
}

void gameOfLife::drawGrid()
{
    char ch = 254;
    for(int i = 1; i < gridSize - 1; i++)
    {
        for(int j = 1; j < gridSize - 1; j++)
        {

                if(mainGrid[i][j])
                {
                    std::cout << " "<<ch;
                }
                else
                {
                    std::cout << "  ";
                }

        }
        std::cout<<std::endl;
    }
}

void gameOfLife::copyGrid ()
{
    gridCopy = &mainGrid;
    (*gridCopy).resize(gridSize);
}

void gameOfLife::updateGrid()
{
    copyGrid();

    for(int i = 1; i < gridSize - 1 ; i++)
    {
        for(int j = 1; j < gridSize - 1; j++)
        {
            int lod = checkNeighbors(gridCopy, i, j);
            nextGeneration( i, j, lod);

        }

    }
}

int gameOfLife::checkNeighbors(gridVector *gridCopy, int x, int y)
{
    int lod = 0;
     for(int i = -1; i < 2; i++)
     {
         for(int j = -1; j < 2; j++)
         {
             if(!(i == 0 && j == 0))
             {
                 if((*gridCopy)[ x + i ][ y + j ])
                 {
                     lod++;
                 }
             }
         }
     }

     return lod;
}

void gameOfLife::nextGeneration(int x, int y, int lod)
{
    if(lod < 2)
    {
        mainGrid[x][y] = 0;
    }
    else if(lod == 3)
    {
        mainGrid[x][y] = 1;
    }
    else if(lod > 3)
    {
        mainGrid[x][y] = 0;
    }
}

void gameOfLife::run()
{
    mainGrid.resize(gridSize);

    initGrid();

    while(true)
    {
        drawGrid();
        updateGrid();
        system("cls");
    }
}

int main()
{
    gameOfLife gol;
    gol.run();

    std::cin.get();

    return 0;
}

#includes should not end with a semicolon:

#include "gameOfLife.h";

The reason this is incorrect is because this is a preprocessor directive, which do not follow the same syntax as compiled lines.
Instead of using a typedef:

typedef std::vector< std::vector<int>> gridVector;

you can now use C++11's type alias:

using std::vector<std::vector<int>> = gridVector;

If you don't need the default constructor, you can either leave it out or make it explicitly defaulted:

gameOfLife() = default;

There's really no need for multiple public and private sections.  If you prefer to organize or separate the members in some way, you can just add line breaks.
I'm not sure why you have a *gridCopy member.  Not only would it be more natural to use a copy constructor in such cases, but having a raw pointer as a member would "force" you to have an overloaded copy constructor.  This is because the default copy constructor performs shallow copies, which will copy the pointer instead of the value at that pointer.

After removing this member, you should also remove copyGrid().
I'm not too familiar with GoL, but I don't think that while loop in run() is supposed to run infinitely.  This type of while loop will not stop unless a break or return is encountered.  You may need to modify it in such a way that it can end.