Calculating the last digit of a^b where a and b are very large numbers I have to make code that calculates the last digit of \$a^b\$. Here, \$a\$ can have up to 1000 digits whereas \$b\$ can be from 0 to 915*1015. Now I calculated the last digit of a and raised that single digit number to the power b and that number's last digit will be the answer.

#include <iostream>
#include <math.h>
#include <string>
using namespace std;
unsigned long long int foo (unsigned long long int a,unsigned long long int b );
int main (void)
{
    int test;
    cin>>test;
    while ( test != 0 )
    {
        string a;
        cin>>a;
        unsigned long long int b;
        cin>>b;
        char c;
        int e;
        string::iterator it = a.end()-1;
        c = *it;
        e = c -48;
        if ( e == 0 && b == 0 )
            cout<<"0"<<"\n";
        else
        {
            unsigned long long modu  = foo(e,b);
            cout<<modu<<"\n";
            test--;
        }
    }
    return 0;
}

unsigned long long int foo (unsigned long long int a,unsigned long long int b )
{
    unsigned long long int ret;
    if ( b == 0 )
        return 1;
    if ( b % 2 == 0 )
    {
        ret = foo(a,b/2);
        ret = ret*ret;
    }
    else if ( b % 2 != 0 )
    {
        ret = foo(a,b/2);
        ret = ret*ret;
        ret = ret*a;
    }
    return ret%10;
}

However, I don't know why this shows WA on submitting it? Any corner cases that I might be missing?

Wrong answer

$$ everything^0 = 1 => 0^0 = 1$$

    if ( e == 0 && b == 0 )
        cout<<"0"<<"\n";

Solution

Last digit relay only on last digit and you can not look at digits than goes before last one. As your power is up to \$10^{18}\$ you have to use modular binary power function.

Here is my solution:

#include <iostream>
#include <string>

typedef unsigned long long int ULLI;
typedef unsigned int UI;

UI bin_pow(UI a, ULLI n) {
    UI result = 1;
    while (n) {
        if (n & 1) {
            result = (result * a) % 10;
            n -= 1;
        } else {
            a = (a * a) % 10;
            n >>= 1;
        }
    }
    return result;
}

int main(int argc, char *argv[]) {
    int n;
    std::cin >> n;
    while (n--) {
        std::string a;
        ULLI b;
        std::cin >> a >> b;
        std::cout << bin_pow(a.back() - '0', b) << std::endl;
    }
}

  C++11 std::string

  Since C++11 std::basic_string had method .back() that returns last char in the string.

  To do this with C++98 you can use .rbegin() iterator.

Solution with complexity \$O(1)\$

Previous code can be acceptable solution, but if it will still not be enough, try to set predefined array for length of circle when

$$ a^{length} mod(10) = a | a = [0 \ldots 9]$$

I'd write some Python script, lets take a look at the results:

(2, [2, 4, 8, 6, 2, 4, 8, 6, 2, 4, 8])
(3, [3, 9, 7, 1, 3, 9, 7, 1, 3, 9, 7])
(4, [4, 6, 4, 6, 4, 6, 4, 6, 4, 6, 4])
(5, [5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5])
(6, [6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6])
(7, [7, 9, 3, 1, 7, 9, 3, 1, 7, 9, 3])
(8, [8, 4, 2, 6, 8, 4, 2, 6, 8, 4, 2])
(9, [9, 1, 9, 1, 9, 1, 9, 1, 9, 1, 9])

What does it means? First number is the digit which was processed. Array is array of lower digits from previous equation. As you can see, there are not too many circles of repetitions.

Lets take a look at digit 2:

with power of 1 we have \$2^1 mod(10)=2\$
with power of 2 we have \$2^2 mod(10)=4\$
with power of 3 we have \$2^3 mod(10)=8\$
with power of 4 we have \$2^4 mod(10)=6\$
with power of 5 we have \$2^5 mod(10)=2\$

that's it, we've got the circle for digit 2.

Also digits may be grouped by number of powers before lower digit will be repeated.

2, 3, 7, 8 repeated after 4 power operations
1, 5, 6 repeated after each power opetation
4, 9 repeated each 2nd power operation

const unsigned int predefined[10][4] = {
    {0},
    {1},
    {2,4,8,6},
    {3,9,7,1},
    {4,6},
    {5},
    {6,4},
    {7,9,3,1},
    {8,4,2,6},
    {9,1}
};

unsigned int bin_pow(const unsigned int& a, unsigned long long int n) {
    if (!n) return 1;

    switch(a) {
        case 0:
        case 1:
        case 5:
            return a;
        case 2:
        case 4:
        case 7:
        case 8:
            return predefined[a][n & 3];
        default:
            return predefined[a][n & 1];
    }
}


Computing the difference of the cross-totals of the diagonals of a matrix I have solved a seemingly easy problem:

  Given a square matrix, calculate the absolute difference between the sums of its diagonals.

For example, the square matrix  is shown below:
$$\begin{bmatrix} 1\, 1\, 1 \\ 2 \, 3\, 3 \\ 0\,1\,0\end{bmatrix}$$

The left-to-right diagonal = \$0+3+1=4\$. The right to left diagonal \$1+3+0=4\$. Their absolute difference is \$0\$.

However, I am a beginner in C and had some troubles. It took me way too long :D. Also, I tried to abstract the problem on a higher level - which does not mean it is high. However, I am not sure how far I should abstract the problem in such challenges.

How well/bad is this code written?
Is the code abstract enough?
How can you solve such challenges more quickly?

int addDiagonal(int from_X,int to_X,int from_Y, int to_Y, int** arr){
    int sum = 0;
    int dirX = to_X - from_X;

    if(dirX < 0) dirX = -1;
    else if(dirX > 0) dirX = 1;

    int dirY= to_Y - from_Y;
    if(dirY < 0) dirY = -1;
    else if(dirY > 0) dirY = 1;

    while(from_X != to_X){
        sum+=arr[from_X][from_Y];
        from_X += dirX;
        from_Y += dirY;
    }
    sum+=arr[from_X][from_Y];
    return sum;
}

int diagonalDifference(int arr_rows, int arr_columns, int** arr) {
    int left_to_right = addDiagonal(0,arr_columns-1,0,arr_rows-1,arr);
    int right_to_left = addDiagonal(0, arr_columns-1,arr_rows-1,0,arr);
    if(left_to_right - right_to_left > 0) return left_to_right - right_to_left;
    return right_to_left - left_to_right;
}

How well/bad is this code written?

A good first timer implementation.

  Is the code abstract enough?

Mostly. It does make unnecessary assumptions about range.  It assumes int math does not overflow.

To make more abstract, code could use typedef int TVS_int; to ease future type changes.

  How can you solve such challenges more quickly?

Take advantage that the matrix is square.

With computing along the diagonal of a square matrix, only the matrix and its one size parameter are needed.

Use const to hint the the compiler about certain potential optimizations and convey code's intent better.

size_t is the best size of array indexing, not too wide nor too narrow.

I like the idea of using a wider intermediate type to mitigate overflow problems.

typedef int TVS_int;
typedef long long TVS_int2;  // a type with wider range.

TVS_int diagonalDifference_alt(const TVS_int** arr, size_t n) {
  TVS_int2 sum_up = 0;
  TVS_int2 sum_dn = 0;

  for (size_t i = 0; i<n; i++) {
    // Notice both `arr` use the same `arr[i]` --> potentially easier to optimize
    sum_up += arr[i][n-i-1];
    sum_dn += arr[i][i];
  }
  TVS_int2 diff = sum_up - sum_dn;
  return (TVS_int) ((diff < 0) ? -diff : diff);  // OF possible here in range reduction
}

if(left_to_right - right_to_left > 0) unnecessarily incurs the potential for int overflow.  Simply compare instead.  if (left_to_right > right_to_left)


Making an accordion from a multi-level HTML list I am writing some code to turn a multi level UL into an accordion:

$(document).ready(function () {

$('.s4-ql li ul').each(function (index) {
    $(this).hide();
});

$('.s4-ql ul li ul').each(function (index) {
    var length = 0;
    length = $(this).children().length;
    var CurrentItem = $(this).parent().find('a:first');
    CurrentItem.find('.menu-item-text').append('(' + length + ')');
});
$('.s4-ql ul li').hover(
  function () {
      $(this).find('a:first').next().slideToggle('normal');
  },
  function () {
      $(this).find('a:first').next().slideToggle('normal');
  }
);
});

Can anyone tell me whether I am being relatively efficient or whether there is anywhere that I can "trim the fat", so to speak? The code also counts the number of children and displays that number on the main parent.

It'll be good if you can provide your HTML markup. Without it I can do this:

jQuery(function($) {
    $('.s4-ql li ul').hide();
    $('.s4-ql ul li').hover(function() {
        $(this).find('a:first').next().slideToggle();
    }, function() {
        $(this).find('a:first').next().slideToggle();
    }).find('ul').each(function(index) {
        var $this = $(this);
        $this.parent().find('a:first .menu-item-text').append(['(', $this.children().length, ')'].join(''));
    });;
});


Goldbach conjecture solution finder The Goldbach conjecture says: any even number greater than two can be represented by the sum of two prime numbers.

  G(n) = { (p1, p2) | p1 + p2 = n, p1 and p2 are prime numbers with p1 < p2 }

For example:

  G(6) = {(3, 3)}

  G(12) = {(5, 7)}

  G(26) = {(3, 23),(7, 19),(13, 13)}

So I have to do an algorithm that determines which three numbers n1, n2 and n3 are between 1,000,000 and 2,000,000 for which the
G (n1), G (n2) and G (n3) have the largest number of elements.

I do not know if I need to improve something in my code, or even optimize some part that is poorly written. It currently taking about 12 seconds.

public class Goldbach  {

    private static int j;
    private static int k;
    private static int [] primes;
    private static int [] just_primes;
    private static int [] conjunct;

    public static void main(String[] args)
    {

        primes = new int[2000000];
        for ( int i=0; i<2000000; i++ ) {
            primes[i] = i+1;
        }

        for ( int i=1; i<1000000; i++ ) {
            if ( primes[i] != 0 ) {
                j = primes[i];
                k = j;
                while ( k <= 2000000 ) {
                    k += j;
                    if ( k <= 2000000 ) {
                        primes[ k-1 ] = 0;
                    }
                }
            }
        }

        just_primes = new int [primes.length];
        for ( int i = 0; i< primes.length-1 ; i++ ) {
            just_primes[i] = 0;
        }

        k = 0;
        for ( int i = 1; i< primes.length-1 ; i++ ) {
            if ( primes[i] > 0 ) {
                just_primes[k] = primes[i];
                k++;
            }
        }

        conjunct = new int[1000001];
        for ( int i = 0; i<1000001 ; i++ ) {
            conjunct[i] = 0;
        }

        int p;
        for ( int i = 0; i< k ; i++ ) {
            for ( int j = i; j< k ; j++ ) {
                int w = just_primes[i]+just_primes[j];
                if ( w >= 1000000 && w <= 2000000 ) {
                    p = w - 1000000;
                    conjunct[p]++;
                }
            }
        }

        int big1 = 0;
        int big2 = 0;
        int big3 = 0;
        int n1 = 0;
        int n2 = 0;
        int n3 = 0;

        for ( int i = 0; i<1000001 ; i++ ) {
            if ( conjunct[i] >= big1 ) {
                big3 = big2;
                big2 = big1;
                big1 = conjunct[i];
                n3 = n2;
                n2 = n1;
                n1 = i;
            } else {
                if ( conjunct[i] >= big2 ) {
                    big3 = big2;
                    big2 = conjunct[i];
                    n3 = n2;
                    n2 = i;
                } else {
                    if ( conjunct[i] >= big3 ) {
                        big3 = conjunct[i];
                        n3 = i;
                    }
                }
            }
        }

        System.out.println( "Largest conjunct 1: " + n1 + " " + 1000000+big1 + " pairs" );
        System.out.println( "Largest conjunct 2: " + n2 + " " + 1000000+big2 + " pairs" );
        System.out.println( "Largest conjunct 3: " + n3 + " " + 1000000+big3 + " pairs" );

    }

}

Expected result:

Largest conjunct 1: 981980 100000027988 pairs
Largest conjunct 2: 951950 100000027802 pairs
Largest conjunct 3: 995630 100000027730 pairs

I think it looks pretty good. One possible way to make it faster is in this part:

for ( int i = 0; i< k ; i++ ) {
    for ( int j = i; j< k ; j++ ) {
        int w = just_primes[i]+just_primes[j];
        if ( w >= 1000000 && w <= 2000000 ) {
            p = w - 1000000;
            conjunct[p]++;
        }
    }
}

Notice that for each value of justprimes[i] you are currently testing all values of justprimes[j] and after this you are testing if the sum is in the desired interval.

One way to work around this is by using a two pointer method that tells us the biggest possible value j can take for any value of i, and also by breaking out of the loop as soon as j becomes to small.

int high = k-1;
    for ( int i = 0; i< k ; i++ ) {
        for ( int j = high; j>=i ; j-- ) {
            int w = just_primes[i]+just_primes[j];
            if(w< 1000000) break;
            if ( w <= 2000000 ) {
                p = w - 1000000;
                conjunct[p]++;
            }
            else{
                while( just_primes[i]+ just_primes[high] > 2000000 && high > 0){
                    high --;
                }
            }
        }
    }

This seems to give a 15% speed boost in my computer.


Parsing n lines to count vowels - HackerEarth I have written a haskell program for the following 'Code Monk' Challenge at HackerEarth.
Here is the challenge description. Basically, we are looking for the number of vowels in a string. The first input n is the number of string to parse, followed by n number of random strings.

And here is my implementation:

import Data.Char
main = do
    numberOfTestCases <- getLine
    processLine (read numberOfTestCases :: Int)

processLine 0 = return()
processLine n = do
    line <- getLine
    print $ countVowels line
    processLine (n-1)

countVowels :: String -> Int
countVowels line = length $ filter (`elem` ['a','e','i','o','u']) $ map toLower line

My code is working correctly but as I'm fairly new to functional programming and Haskell I would like some comments about my code. More specifically, I would like the processLine function to not have a side effect, but I don't know how can I do that. Any other tips or comments would be greatly appreciated!

Type signatures

You should add type signatures to all top-level bindings. processLine's returned value and main should have the same type, which can be checked with the type checker, if you add type signatures. The following code would compile, since main will have the type IO Int:

processLine 0 = return 0

But that's not what you've intended. Therefore, make sure to add type-signatures to all your top-level bindings:

main :: IO ()
main = …

processLine :: Int -> IO ()
processLine n = …

By the way, processLine's type signature makes n's type unambigous, and therefore you don't need an explicit type in (read numberOfTestCases :: Int) anymore.

Use syntactic sugar

['a','e','i','o','u'] is "aeiou", which is easier to change and to read (in my opionion). That way, we get:

countVowels :: String -> Int
countVowels line = length $ filter (`elem` "aeiou") $ map toLower line

By the way, if you use "aeiouAEIOU", you don't need to import Data.Char.

Keeping things pure(?)

  More specifically, I would like the processLine function to not have a side effect, but I don't know how can I do that.

At some point, you need to read the line. You've chosen processLine to do so. Note that it is not feasible to remove IO from processLine, because you would end up with processLine = countVowels.

Still, we can have a look at processLine to see what we can do. We can read the code literally:

If n is 0, do nothing
If n is greater than zero, do

read a line
count the vowels of the line
print the number of vowels
call processLine with n decreased by one

Or we can interpret it as

repeat the following action n times:

read a line
count the vowels of the line
print the number of vowels

return nothing

If we have a look at the modules in base, we'll notice that Control.Monad contains replicateM_, which does exactly the "repeat-n-times"-part:

processAllLines :: Int -> IO ()
processAllLines n = replicateM_ n processSingleLine

processSingleLine :: IO ()
processSingleLine = do
    line <- getLine
    print $ countVowels line

Note that processSingleLine can get shortened into getLine >>= print . countVowels, but I'm not sure whether you're ready for that yet.

Keeping things actually pure

Alright, we've seen how one could use standard functions to process all those lines. But it's still in IO. Can't we do anything?

Well, what would the alternative be? We still need to process a bunch of lines. A "bunch of something" can be easily expressed with a list, so let us write the type signature of our potential candidate.

processList :: [String] -> [Int]

The implementation comes rather easy:

processList = map countVowels

Now we just need to get a list of strings. Since we now want to get something from the user, we have to use IO:

getUserLines :: Int -> IO [String]

Seems familiar, doesn't it? "repeat-an-action-n-times". This time, it's getLine. Since we don't want to throw the result away, we use replicateM (note the missing _):

getUserLines n = replicateM n getLine

We're now missing two last ingredients. First, we need to print a list of Int so that the challenged site accepts it:

printSolutions :: [Int] -> IO ()

Exercise: write printSolution. Note that there is a function that might help you in the process.

We end up with the following main:

main :: IO ()
main = do
  numberOfTestCases <- readLn
  userLines         <- getUserLines numberOfTestCases
  printSolutions (processList userLines)

Further remarks

Search some of the base modules whether they provide the functions you're looking for. And have a look at Prelude. You've used read after getLine, which is provided as a single function readLn (see above).

For completion, here's how I would (probably) solve the challenge:

import Control.Monad (replicateM_)

main :: IO ()
main = do
  numberOfTest <- readLn
  replicateM_ numberOfTest $ getLine >>= print . countVowels

countVowels :: String -> Int
countVowels xs = length $ filter (`elem` "aeuioAEUIO") xs


Data encryption with python I am just trying to solve one of the puzzle which is explained below:

  The sentence if man was meant to stay on the ground god would have
  given us roots after removing spaces is 54 characters long, so it is
  written in the form of a grid with 7 rows and 8 columns.

  The encoded message is obtained by displaying the characters in a
  column, inserting a space, and then displaying the next column and
  inserting a space, and so on. For example, the encoded message for the
  above rectangle is:

imtgdvs fearwer mayoogo anouuio ntnnlvt wttddes aohghn sseoau

For more conditions please refer here:

from math import ceil, sqrt
from string import whitespace

def encrypt(string):
    enc_string = string.translate(None,whitespace)
    length = len(enc_string)

    #calculate the no of colums for the grid
    columns = int(ceil(sqrt(length)))

    #form a list of strings each of length equal to columns
    split_string = [enc_string[i:i+columns] for i in range(0, len(enc_string), columns)]

    #encrypted list of strings
    new_string = []
    for string in split_string:

        for i in range(len(string)):
            try:
                #replace the element in the list with the new string
                new_string[i] = new_string[i].replace(new_string[i], new_string[i] + string[i])
            except IndexError:
                #exception indicates that the string needs be added with new Index
                new_string.append(string[i])

    return " ".join(new_string)

if __name__ == "__main__":
    s = raw_input().strip()
    result = encrypt(s)
    print(result)

Please help me if multiple for loops can be avoided and any other improvements.

That seems like a very long method to splice and put together the string every even set of characters. You are essentially trying to build a square matrix using the characters from the given string. Python has a very nice slice-and-stride feature which can perform the same task in a single inline operation.

I took the liberty of rewriting your encrypt function:

from math import ceil
from string import whitespace

def square_encrypt(str_input): # avoiding the name `string` as python has a module `string`
    str_input = str_input.translate(None, whitespace) # making a copy and overwriting input parameter
    square_size = int(ceil(len(str_input)**0.5)) # or int(ceil(sqrt(len(str_input))))
    encrypted_list = [str_input[i::square_size] for i in xrange(square_size)]
    return " ".join(encrypted_list)


Simple calculator in Ruby I wrote a calculator in Ruby. These are the rules:

Write the first number, in a new line the      arithmetic operator, and in another line the last number.
Use + for addition, - for subtraction, * for multiplication and / for division.

Implementation:

val1 = gets.to_i
sym = gets.chomp
val2 = gets.to_i

def addition(val1, val2)
    return val1+val2
end

add = addition(val1, val2)

def subtraction(val1, val2)
    return val1-val2
end

sub = subtraction(val1, val2)

def division(val1, val2)
    return val1/val2
end

div = division(val1, val2)

def multiplication(val1, val2)
    return val1*val2
end

mult = multiplication(val1, val2)

if sym == "+"
puts add
elsif sym == "-"
puts sub
elsif sym == "/"
puts div
elsif sym == "*"
puts mult
else puts "error"
end

Instead of calculating all the function returns and then checking which one the user input why not just run the function after checking the users input.

For example instead of

add = addition(val1, val2)
if sym == "+"
    puts add

Why not

if sym == "+"
    puts addition(val1, val2)


Passing a large amount of diverse information in JSON using Symfony 2.1 and Doctrine 2 For a project I need to give a load of diverseinformation in JSON format. All the information will be used on the same page so a single call would result in the least overhead. The information all concerns the same database object and is all nescessary on the page. It basically is a collection of counts of the amount of objects that are of one or more of a certain type (the types are all booleans) and we require to know a lot of different variations of this. I used the code below but my co-worker believes the way I put it in the JSON list is a bit clunky and the code could be far more performant. How could I improve this code?

public function getContactsStatisticsAction()
{
    $response = new Response();
    $json = array();
    $em = $this->getDoctrine()->getEntityManager();
    $cr = $em->getRepository('BlaCoreBundle:Company');

    $json['numberOfCompanies'] = $cr->numberOfCompanies();
    $json['numberOfAccounts'] = $cr->numberOfCompanies(array("typeAccount" => true));
    $json['numberOfCompetitors'] = $cr->numberOfCompanies(array("typeCompetitor" => true));
    $json['numberOfSuppliers'] = $cr->numberOfCompanies(array("typeSupplier" => true));
    $json['numberOfOthers'] = $cr->numberOfCompanies(array("typeOther" => true));
    $json['numberOfUnassigned'] = $cr->numberOfCompanies(array("typeAccount" => false, "typeCompetitor" => false,"typeSupplier" => false,"typeOther" => false));

    $json['numberOfJustAccounts'] = $cr->numberOfCompanies(array("typeAccount" => true, "typeCompetitor" => false, "typeSupplier" => false));
    $json['numberOfJustCompetitors'] = $cr->numberOfCompanies(array("typeAccount" => false, "typeCompetitor" => false, "typeSupplier" => false));
    $json['numberOfJustSuppliers'] = $cr->numberOfCompanies(array("typeAccount" => false, "typeCompetitor" => false, "typeSupplier" => false));

    $json['numberOfCompetitorAndAccounts'] = $cr->numberOfCompanies(array("typeAccount" => true, "typeCompetitor" => true, "typeSupplier" => false));
    $json['numberOfCompetitorAndSuppliers'] = $cr->numberOfCompanies(array("typeAccount" => false, "typeCompetitor" => true, "typeSupplier" => true));
    $json['numberOfSupplierAndAccounts'] = $cr->numberOfCompanies(array("typeAccount" => true, "typeCompetitor" => false, "typeSupplier" => true));
    $json['numberOfCompaniesAndAccountsAndSuppliers'] = $cr->numberOfCompanies(array("typeAccount" => true, "typeCompetitor" => true, "typeSupplier" => true));

    $response->setContent(json_encode($json));
    return $response;
}

public function numberOfCompanies($filters = array())
{
    $qb = $this->getEntityManager()->createQueryBuilder();
    $qb->select('count(c.id)');
    $qb->from('BlaCoreBundle:Company', 'c');
    $sizeFilters = count ($filters);
    $keys = array_keys($filters);
    if($sizeFilters >= 1){
        $qb->where('c.' . $keys[0] . ' = ' . (int) $filters[$keys[0]]);
    }
    for($i = 1; $i < $sizeFilters; $i++){
        $qb->andWhere('c.' . $keys[$i] . ' = ' . (int) $filters[$keys[$i]]);
    }
    return $qb->getQuery()->getSingleScalarResult();
}

I think that what you have is probably fine. Personally, I'd move the construction of the result array to a view but this is not that big of a deal. That might help with the "clunkiness".

With regards to your concern about the size of the json response: You say that all of the information in it is needed by the app. It doesn't look all that big to me. Unless you are actually experiencing problems as a result of the size, I'd leave it alone. If it were too big, you could consider implementing appropriate caching (allow the browser to cache the results and/or use etags) or pagination.

Regarding the number of individual sql calls: I'm sure you could put it all into one query if need be.

Both of these potential issues, response size and number of queries, seem to me to be pre-optimization concerns. In the past, I've spent lots of time pre-optimizing stuff and never realized any real gain. Now, I implement what is needed and only optimize when the real need arises. The bottom line: I don't see any problems with what you've got and unless the result is unacceptably slow or resource intensive, don't worry about it.

Hope it helps.


Implementing Function that Maps over Stream I implemented the following function:

the ruler function
0, 1, 0, 2, 0, 1, 0, 3, 0, 1, 0, 2, 0, 1, 0, 4, . . .
where the nth element in the stream (assuming the first element
corresponds to n = 1) is the largest power of 2 which evenly
divides n

I'm giving it a vague name in the event that students try to cheat and search for it.
foo :: Stream Integer
foo = streamMap f startAtOne
    where f x        = if (odd x) then 0 else g x 0
          g y acc    = if y == 0 then acc else g (y-2) (acc+1)
          startAtOne = streamFromSeed 1 (+1)

Here's the streamFromSeed function and the Stream data type.
data Stream a = Stream (a, Stream a)
streamMap :: (a -> b) -> Stream a -> Stream b
streamMap f (Stream (x, rest)) = Stream (f x, streamMap f rest)

-- Given a start value and function, generate a stream by applying fn to each Stream value
streamFromSeed :: a -> (a -> a) -> Stream a
streamFromSeed x f = Stream $ (x, streamFromSeed (f x) f)

Note that this technically is homework, but from 2013. I'm working on this material since Prof. Yorgey provides excellent lectures and homework assignments. I'm not in this class nor a student at the school.

Since streamMap allows you to map over a stream, you can as well define a Functor instance for Stream. Actually, streams can also define an Applicative instance which zips two streams and also a Monad instance whose join computes the diagonal of a 2-dimensional stream.
Your definition of Stream is slightly inefficient as it wraps the (,) constructor in another constructor Stream. You could:

make it a newtype, or
use the UNPACK pragma, or
define just a 2-argument constructor like data Stream a = Cons a (Stream a).

Your solution deviates from the assignment that forbids any kind of divisibility testing. Function odd tests for divisibility by 2 and g implements (somewhat inefficiently) division by 2.
Without the full code it's a bit hard to test/judge correctness, but I believe your implementation is not correct. In particular, I believe for n=6 your code will yield 3, while the expected value is 1.


Reverse a linked list I was given 15 minutes to write the code to reverse a singly-linked list.

What do you think of the code style here?

using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace LinkedListQuestions
{
    [TestClass]
    public class ReverseLinkedList
    {
        [TestMethod]
        public void ReservseLinkedListTest()
        {
            Node<int> head = new Node<int>();
            Node<int> two = new Node<int>();
            Node<int> three = new Node<int>();
            Node<int> four = new Node<int>();
            head.Value = 1;
            head.Next = two;
            two.Value = 2;
            two.Next = three;
            three.Value = 3;
            three.Next = four;
            four.Value = 4;
            LinkedListHandler.Reserve(ref head);
            Assert.AreEqual(4, head.Value);
            Assert.AreEqual(3, head.Next.Value);
            Assert.AreEqual(2, head.Next.Next.Value);
            Assert.AreEqual(1, head.Next.Next.Next.Value);
        }
    }

    public class LinkedListHandler
    {
        public static void Reserve(ref Node<int> head)
        {
            Node<int> current = head;
            Node<int> previous = null;
            while (current != null)
            {
                Node<int> temp = current.Next;
                current.Next = previous;
                previous = current;
                current = temp;
            }
            head = previous;
        }
    }

    public class Node<T>
    {
        public T Value { get; set; }
        public Node<T> Next { get; set; }
    }
}

Test cases right off the bat are a good sign. You only have one, but it is now easier to expand other conditions. I like to add in conditions for list has 0 elements and list is null. You'll be able to prove that those work, or if they don't work, prove that changing it didn't break anything.

Two of the methods use the word Reserve instead of Reverse: ReservseLinkedListTest and Reserve.

For a 15 minute code interview, I'd give it a passing grade.


Adding classes with JavaScript I am working again and got this to work. Is this the best way to write this code?

$(window).load(function(){
var $window = $(window),
$sticky = $('#contentSideIner'),
elTop = $sticky.offset().top;

$window.scroll(function() {
$sticky.toggleClass('sticky', $window.scrollTop() > elTop);
});
$window.scroll(function() {
$sticky.toggleClass('stickyBottom', $window.scrollTop() > elTop + 4685);
});
});

I had such a hard time to get this function to work in the first place. For the longest time I was told it was not executing at all.

This can be simplified to this:

$(window).scroll(function() {
    var $sticky = $('#contentSideIner'),
        elTop = $sticky.offset().top,
        wTop = $(window).scrollTop();
    $sticky.toggleClass('sticky',  wTop > elTop)
        .toggleClass('stickyBottom', wTop > elTop + 4685);
});

The window object already exists, so no need to wait until it loads before attaching an event handler to it.
Don't cache objects outside of the event handler unless it's really needed (consumes more memory, code is less encapsulated, makes your page susceptible to problems if layout or DOM elements change, etc...)
Use jQuery chaining for multiple method calls on the same jQuery object
Only use one event handler and put both .toggleClass() calls in the same event handler.


Describing a date interval as text public String getDatePeriodtext(boolean userCanJoinAndLeave, Date startDate, Date endDate) {
    String text;

    if (!userCanJoinAndLeave) {
        text = getText(getLocale(), "no");
    } else if (startDate == null && endDate == null) {
        text = getText(getLocale(), "continuous");
    } else if (startDate == null && endDate != null) {
        text = getText(getLocale(), "to")
                + " "
                + formatDate(endDate);
    } else if (endDate == null) {
        text = getText(getLocale(), "from")
                + " "
                + formatDate(startDate);
    } else {
        text = formatDate(startDate)
                + " "
                + getText(getLocale(), "to")
                + " "
                + formatDate(endDate);
    }

    return text;
}

Some scenarios:

userCanJoinAndLeave = false ==> No
startDate = null and endDate = null ==> Continuous
startDate = 2016-10-31 and endDate = null ==> from 2016-10-31
startDate = null and endDate = 2016-11-01 ==> to 2016-11-01
startDate = 2016-10-31 and endDate = 2016-11-01 ==> 2016-10-31 to 2016-11-01

I am not sure if you like this one better but I personally prefer returning directly (and therefore omitting the else-clauses). I do not know if there was a better way.

public String getDatePeriodtext(boolean userCanJoinAndLeave, Date startDate, Date endDate) {

  if (!userCanJoinAndLeave)
    return getText(getLocale(), "no");

  if (startDate == null && endDate == null)
    return getText(getLocale(), "continuous");

  if (startDate == null && endDate != null)
    return getText(getLocale(), "to")
            + " "
            + formatDate(endDate);

  if (endDate == null)
    return getText(getLocale(), "from")
            + " "
            + formatDate(startDate);

  return formatDate(startDate);
          + " "
          + getText(getLocale(), "to")
          + " "
          + formatDate(endDate);
}


Python script to extract text from PDF with images I have the following Python script. The purpose of the script is to extract text from PDFs. I use textract for that because soon I realized there is no easy way to check if a page contains an image or not. So I extract the whole text using textract.

The workflow is like this. main() parses each pdf file from a folder, I extract the text, I search for keyword strikes and then I export the result to a csv file inside folder output_results.

What I can fix on my logic? What can I change in my code? I find it messy, how I can clean it up?

import textract
import os
import csv

class PdfMiner():

    path = os.getcwd() + '/folderForPdf/'
    output_path = os.getcwd() + '/output_results/'

    def __init__(self):
        pass

    def main(self):
        for self.filename in os.listdir(self.path):
            self.text = (PdfMiner().extract_text_from_pdf(self.path + self.filename))
            self.keyword_strike_dict = PdfMiner().keyword_strike(self.text)
            if bool(self.keyword_strike_dict):
                PdfMiner().output_to_csv(self.filename, self.keyword_strike_dict)

    def keyword_strike(self, text, keyword_strike_dict={}):
        '''keyword_strike function counts how many times a specific keyword occurs'''
        self.keyword_strike_dict = {}
        self.text = text
        self.keywords_list = PdfMiner().extract_keywords()
        for keyword in self.keywords_list:
            if keyword in text.decode('utf-8'):
                self.keyword_strike_dict[keyword] = text.decode('utf-8').count(keyword)
        return self.keyword_strike_dict

    def extract_keywords(self, keywords_list=None):
        '''function extract_keywords extract the keywords from file keywords.txt, into a list'''
        keywords_list = []
        with open('keywords.txt', 'r', encoding='utf8') as keywords_file:
            for keyword in keywords_file:
                keywords_list.append(keyword.strip('\n'))
        return keywords_list

    def extract_text_from_pdf(self, file_destination, text=None):
        '''extract_text_from_pdf'''
        self.file_destination = file_destination
        text = textract.process(self.file_destination, method='tesseract', language='eng', encoding='utf-8')
        return text

    def output_to_csv(self, *args, **kwargs):
        '''output_csv exports results to csv'''
        self.filename = args[0]
        self.keyword_strike_dict = args[1]
        self.output_file_path = PdfMiner().output_path + self.filename.strip('.pdf')
        with open(self.output_file_path + '.csv', 'w+', newline='') as csvfile:
            row_writer = csv.writer(csvfile, delimiter=',')
            row_writer.writerow(['keyword', 'keyword_count'])
            for keyword, keyword_count in self.keyword_strike_dict.items():
                print(keyword, keyword_count)
                row_writer.writerow([keyword, keyword_count])

if __name__ == "__main__":
    PdfMiner().main()

I don't see a good reason why this should be a class. You only have two things in your state, self.text, which you could pass as an argument, and self.path, self.output_path, which I would also pass as arguments, maybe with a default value.

Also, you are probably using classes wrong if your class has a main method that needs to instantiate new instances of the class on the fly.

Your algorithm is not very efficient. You need to run over the whole text twice for each keyword. Once to check if it is in there and then again to count it. The former is obviously redundant, since str.count will just return 0 if the value is not present.

However, what would be a better algorithm is to first extract all the words (for example using a regex filtering only letters) and then count the number of times each word occurs using a collections.Counter, optionally filtering it down to only those words which are keywords. It even has a most_common method, so your file will be ordered by number of occurrences, descending.

Instead of mucking around with os.getcwd() and os.listdir, I would recommend to use the (Python 3) pathlib.Path object. It supports globbing (to get all files matching a pattern), chaining them to get a new path and even replacing the extension with a different one.

When reading the keywords, you can use a simple list comprehension. Or, even better, a set comprehension to get in calls for free.

line.strip() and line.strip("\n") are probably doing the same thing, unless you really want to preserve the spaces at the end of words.

At the same time, doing self.filename.strip('.pdf') is a bit dangerous. It removes all characters given, until none of the characters is found anymore. For example, "some_file_name_fdp.pdf" will be reduced to "some_file_name_".

The csv.writer has a writerows method that takes an iterable of rows. This way you can avoid a for loop.

I would ensure to run only over PDF files, otherwise you will get some errors if a non-PDF file manages to sneak into your folder.

I have done all of this in the following code (not tested, since I don't have textract installed ATM):

from collections import Counter
import csv
from pathlib import Path
import re
import textract

def extract_text(file_name):
    return textract.process(file_name, method='tesseract', language='eng',
                            encoding='utf-8').decode('utf-8')

def extract_words(text):
    return re.findall(r'([a-zA-Z]+)', text)

def count_keywords(words, keywords):
    return Counter(word for word in words if word in keywords)

def read_keywords(file_name):
    with open(file_name) as f:
        return {line.strip() for line in f}

def save_keywords(file_name, keywords):
    with open(file_name, "w", newline='') as csvfile:
        writer = csv.writer(csvfile, delimiter=',')
        writer.writerow(['keyword', 'keyword_count'])
        writer.writerows(keywords.most_common())

def main():
    output_folder = Path("output_results")
    keywords = read_keywords('keywords.txt')

    for f in Path("folderForPdf").glob("*.pdf"):
        words = extract_words(extract_text(f))
        keyword_counts = count_keywords(words, keywords)
        save_keywords(output_folder / f.with_suffix(".csv"), keyword_counts)

if __name__ == "__main__":
    main()


Sequentially remove members "repelled" by previous members I was working on a project that need to remove extra data such that the remains are not "too close" to each other, and the algorithm should be as generalize as possible. To have more general usage, I try to write in std  format.

I'm not sure if the algorithm is the best way to accomplish the goal, or if the style of coding is optimal.

#ifndef _REPEL
#define _REPEL

#include <algorithm>
#if __cplusplus >= 201103L
#include <utility>
#endif

#if __cplusplus >= 201103L
template<class ForwardIt, class InputIt, class BinaryPredicate>
bool check_repelled(ForwardIt first, ForwardIt last, InputIt target, BinaryPredicate p){
    return last != std::find_if(first, last, [p, target](decltype(*first) i)->bool{ return p(i, *target); });
}
#else
template<class ForwardIt, class InputIt, class BinaryPredicate>
bool check_repelled(ForwardIt first, ForwardIt last, InputIt target, BinaryPredicate p){
    for(ForwardIt i = first; i != last; ++i)
        if(p(*i, *target))
            return true;
    return false;
}
#endif

template<class InputIt, class ForwardIt, class BinaryPredicate>
ForwardIt remove_copy_repelled(InputIt first, InputIt last, ForwardIt d_first, BinaryPredicate p){
    ForwardIt d_end = d_first;
    for(; first != last; ++first){
        if(!check_repelled(d_first, d_end, first, p))
            *d_end++ = *first;
    }
    return d_end;
}

template<class ForwardIt, class BinaryPredicate>
ForwardIt remove_repelled(ForwardIt first, ForwardIt last, BinaryPredicate p){
    ForwardIt i, j;
    for(i = first; i != last; ++i)
        if(check_repelled(first, i, i, p))
            break;
    if(i == last)
        return last;
    for(j = i; ++j != last;)
        if(!check_repelled(first, i, j, p))
#if __cplusplus >= 201103L
            *i++ = std::move(*j);
#else
            std::iter_swap(i++, j);
#endif
    return i;
}
#endif

Possible implementation:

std::vector<int>x = {1,3,2,8,5,6,7};
x.erase(remove_repelled(x.begin(), x.end(), [](int a,int b)->bool{std::abs(a-b) < 2;}),x.end());
//x == {1,3,8,5}

In all, there's not much to complain about, but I did find a few things that might be improved. All of my comments and testing are about C++11 mode, since that's what I have conveniently available.

Use const references for the predicate

Right now, the lambda in your predicate has values a and b passed by value, which is fine for primitive types such as int but could be relatively costly for classes such as std::complex<double> due to all of the overhead of construction and destruction of temporary objects.  With that said, it's only a minor change since std::abs creates a temporary itself, and this is a quibble with your example use of the the template rather than the template itself.  This is what I used for testing:

typedef std::complex<double> mytype;
auto pred = [](const mytype &a,const mytype &b)->bool{
    return abs(a-b) < 2;
};

Fix the C++11 version of the code

As has been mentioned in comments, the syntax isn't quite right for the C++ version of the lambda in the template.  It should be:

 [p, target](decltype(*target) i)->bool{return p(i, *target); }

Consider encapsulating the erase

It's a bit odd to have to use x.erase() outside of the templated function.  Putting it inside would make usage a little simpler.

Check for empty container in remove_copy_repelled

I tried to exercise the remove_copy_repelled code.  The first thing I tried was this:

std::vector<mytype>x = {1,3,2,8,5,6,7};
std::vector<mytype>x2;
x2.erase(remove_copy_repelled(
    x.cbegin(), x.cend(), x2.begin(), pred), x2.end());

I was rewarded with a core dump.  It doesn't seem to like an empty destination container.

std::vector<mytype>x2;
x2.reserve(x.size());

This worked, but isn't particularly general, so you could do a template specialization, but there may be a better way.  In particular, you might consider constructing the destination container, perhaps using a passed parameter that is a placeholder for push_back or whatever the caller wishes to use to add another object to the container.


Swift 3 Generic fetch request extension I implemented an extension for the NSManagedObjectContext that takes any CoreData class as a parameter and performs a fetch request on it.

I am new to CoreData and would very much appreciate any sort of feedback for this function

extension NSManagedObjectContext{
func fetchObjects <T: NSManagedObject>(_ entityClass:T.Type, sortBy: [NSSortDescriptor]? = nil, predicate: NSPredicate? = nil) throws -> [AnyObject]{
    var request: NSFetchRequest<NSFetchRequestResult>

    if #available(iOS 10.0, *) {
        request = entityClass.fetchRequest()
    } else {
        let entityClassName = NSStringFromClass(entityClass)
        let entityName = entityClassName.components(separatedBy: ".").last ?? entityClassName
        request = NSFetchRequest(entityName: entityName)
    }

    var fetchRequestError: Error?
    request.returnsObjectsAsFaults = false

    if let predicate = predicate {
        request.predicate = predicate
    }
    if let sortBy = sortBy {
        request.sortDescriptors = sortBy
    }

    var fetchedResult: [T]?

        do {
            fetchedResult = try self.fetch(request) as? [T]
        }
        catch let error {
            fetchRequestError = error
            //print(fetchRequestError)
        }

    guard let entityArray = fetchedResult else {
        throw fetchRequestError!
    }

    return entityArray
    }
}

Your code looks good, but there are some things which can be simplified.
Additionally, the method can be improved to return a array of the
actual managed object subclass type instead of AnyObject.

Simplifications and small improvements

var request: NSFetchRequest<NSFetchRequestResult>

request is assigned exactly once, therefore it should be a constant:

let request: NSFetchRequest<NSFetchRequestResult>

let entityClassName = NSStringFromClass(entityClass)
let entityName = entityClassName.components(separatedBy: ".").last ?? entityClassName

can be simplified to

let entityName = String(describing: entityClass)

which returns the unqualified class name of the given class
(in contrast to String(reflecting:) which returns the fully qualified
class name including the module name).

if let predicate = predicate {
    request.predicate = predicate
}
if let sortBy = sortBy {
    request.sortDescriptors = sortBy
}

The predicate and sortDescriptors of NSFetchRequest are optionals, so there is no need to conditionally unwrap
the given parameters. You can assign them directly:

request.predicate = predicate
request.sortDescriptors = sortBy

fetchedResult = try self.fetch(request) as? [T]

We know that the fetch request returns an array of T objects,
the cast cannot fail. Therefore the second half of the method
can be reduced to

do {
    let fetchedResult = try self.fetch(request) as! [T]
    return fetchedResult
} catch let error {
    print(error.localizedDescription)
    throw error
}

making the fetchRequestError variable obsolete. And if you don't
need to print the error locally then no do/catch is needed at all:

let fetchedResult = try self.fetch(request) as! [T]
return fetchedResult

A possible error will be re-thrown to the caller of the method.

Putting it all together, the method now looks like this:

extension NSManagedObjectContext{
    func fetchObjects <T: NSManagedObject>(_ entityClass:T.Type,
                       sortBy: [NSSortDescriptor]? = nil,
                       predicate: NSPredicate? = nil) throws -> [AnyObject] {

        let request: NSFetchRequest<NSFetchRequestResult>
        if #available(iOS 10.0, *) {
            request = entityClass.fetchRequest()
        } else {
            let entityName = String(describing: entityClass)
            request = NSFetchRequest(entityName: entityName)
        }

        request.returnsObjectsAsFaults = false
        request.predicate = predicate
        request.sortDescriptors = sortBy

        let fetchedResult = try self.fetch(request) as! [T]
        return fetchedResult
    }
}

Returning a "correctly" typed array

The return type is [AnyObject], which means that the return value has to be cast to the actual managed object subclass type

let objects = try context.fetchObjects(YourEntity.self) as! [YourEntity]

This can be improved by changing the return type to [T] and using
a typed fetched request NSFetchRequest<T>:

extension NSManagedObjectContext{
    func fetchObjects <T: NSManagedObject>(_ entityClass:T.Type,
                       sortBy: [NSSortDescriptor]? = nil,
                       predicate: NSPredicate? = nil) throws -> [T] {

        let request: NSFetchRequest<T>
        if #available(iOS 10.0, *) {
            request = entityClass.fetchRequest() as! NSFetchRequest<T>
        } else {
            let entityName = String(describing: entityClass)
            request = NSFetchRequest(entityName: entityName)
        }

        request.returnsObjectsAsFaults = false
        request.predicate = predicate
        request.sortDescriptors = sortBy

        let fetchedResult = try self.fetch(request)
        return fetchedResult
    }
}

This is now called as

let objects = try context.fetchObjects(YourEntity.self)

and objects is an array of YourEntity without additional cast.


Verifying and reducing Latin squares with Python 3 Similar to my previous post, I'm completing this challenge, which is to verify whether an n × n grid is a Latin square. I have also solved the bonus problem, which is to rearrange a valid Latin square so that its first row and first column are in order.

It is intended that both functions could be used independently. Any feedback is appreciated.

from itertools import chain

def is_latin(n, numbers):
    assert type(numbers) is str
    assert type(n) is str or type(n) is int

    n = int(n)
    list_numbers = [int(m) for m in numbers.split(" ")]

    valid_combination = set(range(1, n+1))
    rows = (set(list_numbers[m:m + n]) for m in range(0, n ** 2, n))
    columns = (set(list_numbers[m::n]) for m in range(n))

    for line in chain(rows, columns):
        if not(line == valid_combination):
            return False
        elif line == valid_combination:
            continue
        else:
            raise Exception("{}{}".format(line, valid_combination))
    return True

def reduce_latin(n, numbers):
    assert type(numbers) is str
    assert type(n) is str or type(n) is int

    n = int(n)
    latin = is_latin(n, numbers)
    if latin:
        return [[((i+m)%n)+1 for i in range(n)] for m in range(n)]
    elif not latin:
        return None
    else:
        raise Exception("{}{}".format(latin, numbers))

if __name__ == "__main__":
    print(reduce_latin("5", "1 2 3 4 5 5 1 2 3 4 4 5 1 2 3 3 4 5 1 2 2 3 4 5 1"))

You can make few minor improvements like:-

For comparing types use isinstance instead of comparing types using is. is won't work in case of multiple-inheritance. Plus for comparing against multiple types you can do isinstance(n, (str, int)). Read: Differences between isinstance() and type() in python

The elif and else block in is_latin function look redundant to me, you only need to check for mismatch there. Plus if line != valid_combination is more readable than: if not(line == valid_combination).
The else block in reduce_latin is also unreachable and you can remove it too.
Your code has couple of PEP 8 errors that you could check here: http://pep8online.com/


Using Poor Man's DI to inject helper class dependencies I had a rather large method in one public class that I refactored into 2 helper classes. The thing is though, that those 2 helper classes have dependencies. I refactored them into helper classes so I could mock and test them easily, which worked out perfectly.

However, the thing is I don't want to have to register my helper classes in the DI container, because I know the public class will always be using those specific implementations.

This is how I implemented the public class' constructors:

    /// <summary>
    /// Internal constructor used by tests for mocking.
    /// </summary>
    internal TranslationCompiler(ITranslationCatalogTransformer translationCatalogTransformer, ICompiledCatalogTransformer compiledCatalogTransformer)
    {
        if (compiledCatalogTransformer == null) throw new ArgumentNullException("compiledCatalogTransformer");
        if (translationCatalogTransformer == null) throw new ArgumentNullException("translationCatalogTransformer");
        _translationCatalogTransformer = translationCatalogTransformer;
        _compiledCatalogTransformer = compiledCatalogTransformer;
    }

    /// <summary>
    /// Public constructor that passes dependencies to concrete implementations of helper classes.
    /// </summary>
    public TranslationCompiler(IResourceService resourceService, ITranslationSerializer serializer)
    {
        if (resourceService == null) throw new ArgumentNullException("resourceService");
        if (serializer == null) throw new ArgumentNullException("serializer");
        _translationCatalogTransformer = new TranslationCatalogTransformer(resourceService);
        _compiledCatalogTransformer = new CompiledCatalogTransformer(serializer);
    }

Is this an acceptable use for poor man's DI? This way, the DI container only has to know the actual dependencies for the public class to work, while still being very testable.

I don't see anything wrong with your implementation - if it works for you, and does not interfere with the system - go for it!

I will even go further and claim that the guard conditions in your internal constructor are redundant because the constructor will only be used by the single public constructor, or by a test which:

Either counts on the helper - in which case a NullPointerException will occur and fail the test, or
Does not need the helper, in which case there is no need to mock it...

Final note: I guess that the public constructor actually uses resourceService and serializer in some code you deleted from it, otherwise, the guard conditions there are also redundant...


Alternatives for appending a div from a JS object I've built my first "real" web app which uses both server-side and client side functions to populate a list of courses a user can register for. Currently, the client receives an Object with the following structure:

Object

{
  "date": "1/1/2018",
  "time": "14:30",
  "title": "String Title 1",
  "desc": "A long string description with information about the workshop.",
  "cat": "Category 1, Category 2",
  "seats": "20",
  "type": "In Person",
  "location" : "Building A",
  "lock": true,
  "code": "ABCD",
  "hash": "Str1ng",
  "who": "Teachers, Admin"
} ...

Then, the client adds div elements built from the Object. I'm using .append() and inserting object keys at the correct point. It works fine, but I'm feeling like this is a really inefficient and frankly, messy, way to add the content.

Client script

var data = JSON.parse(Object);

$("#list").append(
    "
\
       " + data[j].time + "\
       " + data[j].date + ", \
       <input type='checkbox' name='wkshp' value='" + data[j].hash + "'/>\
       " + data[j].title + "\
       " + data[j].desc + "\

         " + data[j].location + "\
         " + data[j].who + "\
         " + data[j].cat + "\
         " + data[j].type + "\
         Seats: " + data[j].seats + "\

\

\

Registration Code
\
         <input type='text' name='code' value='' />

"
);

What improvements can I make to the Object? Is there a better way to parse the object by the client?

You could use template literals to avoid the breaking the strings with + and doing \ to newline. Alternatively, you could use template libraries which are more flexible. Mustache is a good example.
Consider using a web framework that does the bulk of the rendering for you. That way, you only have to worry about your data. Web frameworks also do optimizations to avoid re-rendering the entire chunk of HTML, do housekeeping, etc.

Other than that, there's not much to review.


Computing tangent space basis vectors for an arbitrary mesh This is more like a share and a request than a question. I converted Eric Lengyel's code, which calculates tangents of a mesh for the purpose of texturing and normal mapping, to support SIMD. For this I used DirectXMath library from windows SDK. I don't know if this is actually faster than regular floating point version (didn't test it yet). So, my request is: please let me know if you have any suggestions as to optimize the code even further.

void CalculateTangentsAndNormals( vector<Vertex>& verts, vector <U32>& idx )
{
    // Computing Tangent Space Basis Vectors for an Arbitrary Mesh
    //  http://www.terathon.com/code/tangent.html
    //

    const XMVECTOR NullVector = XMLoadFloat4A(&XMFLOAT4A(0.f, 0.f, 0.f, 0.f));
    const XMVECTOR W_Null = XMLoadFloat4A(&XMFLOAT4A(1.f, 1.f, 1.f, 0.f));
    const XMVECTOR signature = XMLoadFloat4A(&XMFLOAT4A(1.f,-1.f,-1.f,1.f));
    XMMATRIX st(NullVector, NullVector, NullVector, NullVector);

    vector <XMVECTOR> vTangents, vBitangents;
    const U32 NumberOfVertices = (U32)verts.size();

    vTangents.resize( NumberOfVertices , NullVector);
    vBitangents.resize( NumberOfVertices, NullVector);

    const U32 NumberOfIndices = (U32)idx.size();
    for ( U32 i = 0; i < NumberOfIndices; ++i )
    {
        const U32 i0 = idx[i];
        const U32 i1 = idx[++i];
        const U32 i2 = idx[++i];

        const XMVECTOR v0 = XMLoadFloat3A(&verts[i0].pos);
        const XMVECTOR v1 = XMLoadFloat3A(&verts[i1].pos);
        const XMVECTOR v2 = XMLoadFloat3A(&verts[i2].pos);

        const XMVECTOR e0 = XMVectorMultiply( v1 - v0, W_Null);
        const XMVECTOR e1 = XMVectorMultiply( v2 - v0, W_Null);

        const XMMATRIX e01( e0, e1, NullVector, NullVector);
        /*      | e0.x e0.y e0.z 0 |
                | e1.x e1.y e1.z 0 |
        e01 =   | 0    0    0    0 |
                | 0    0    0    0 |
        */
        const XMVECTOR t0 = XMLoadFloat2A(&verts[i0].tex);
        const XMVECTOR t1 = XMLoadFloat2A(&verts[i1].tex);
        const XMVECTOR t2 = XMLoadFloat2A(&verts[i2].tex);

        XMVECTOR s = XMVectorMergeXY(t1 - t0, t2 - t0); // s = (t1x - t0x, t2x - t0x, t1y - t0y, t2y - t0y)

        XMFLOAT4A d;
        XMStoreFloat4A(&d, s);

        const float DetInv = 1.0f/( d.x * d.w - d.z * d.y );

        s *= DetInv;
        s = XMVectorMultiply(s, signature); // s = (sx, -sy, -sz, sw)

        const U32 *const pSrc = reinterpret_cast<const U32 *const>(&s);
        U32* pDst = reinterpret_cast<U32*>(&st.r[0]);
        pDst[0] = pSrc[3];  // _00 = sw
        pDst[1] = pSrc[2];  // _01 = -sz
        pDst = reinterpret_cast<U32*>(&st.r[1]);
        pDst[0] = pSrc[1];  // _10 = -sy
        pDst[1] = pSrc[0];  // _11 = sz

        /*      | sw  -sz 0 0 |
                | -sy sx  0 0 |
        st =    | 0    0  0 0 |*DetInv
                | 0    0  0 0 |
        */

        const XMMATRIX uv( XMMatrixMultiply(st, e01) );

        vTangents[i0] += uv.r[0];
        vTangents[i1] += uv.r[0];
        vTangents[i2] += uv.r[0];

        vBitangents[i0] += uv.r[1];
        vBitangents[i1] += uv.r[1];
        vBitangents[i2] += uv.r[1];

    }
    // orthogonalize normals and tangents
    for ( U32 i = 0; i < NumberOfVertices; ++i )
    {
        const XMVECTOR normal = XMVector3Normalize(XMLoadFloat3A(&verts[i].normal));

        // normalize tangents and orthogonalize TBN
        XMVECTOR n0 = XMVector3Normalize( vTangents[i] - XMVector3Dot( normal, vTangents[i] ) * normal );
        //calculate handedness
        const XMVECTOR n1 = XMVector3Cross( normal, vTangents[i] );
        const float w = (XMVector3Less(XMVector3Dot(n1, vBitangents[i] ), NullVector)) ? -1.f : 1.f;

        n0 = XMVectorSetW(n0, w);
        XMStoreFloat4A(&verts[i].tangent, n0);
    }
}

Here's my pass at it; without the rest of your source I can't verify it's integrity so I don't know if this will compile correctly with the rest of your code, so hopefully it can help:

// outside function since they're consts, decreases run time as no more construction each function call
const XMVECTOR NullVector = XMLoadFloat4A(&XMFLOAT4A(0.f, 0.f, 0.f, 0.f));
const XMVECTOR W_Null = XMLoadFloat4A(&XMFLOAT4A(1.f, 1.f, 1.f, 0.f));
const XMVECTOR signature = XMLoadFloat4A(&XMFLOAT4A(1.f,-1.f,-1.f,1.f));

void CalculateTangentsAndNormals( vector<Vertex>& verts, vector <U32>& idx )
{
    // Computing Tangent Space Basis Vectors for an Arbitrary Mesh
    //  http://www.terathon.com/code/tangent.html

    const U32 NumberOfVertices = (U32)verts.size();
    const U32 NumberOfIndices = (U32)idx.size();
    U32 i = 0;

    XMMATRIX st(NullVector, NullVector, NullVector, NullVector);

    // explicit construction
    vector <XMVECTOR> vTangents( NumberOfVertices , NullVector);
    vector <XMVECTOR> vBitangents( NumberOfVertices, NullVector);

    {
        U32 i0, i1, i2;
        XMVECTOR v0, v1, v2;
        XMVECTOR t0;
        XMFLOAT4A d;
        const U32* pSrc = 0;
        U32* pDst = 0;
        while (i < NumberOfIndices) {
            i0 = idx[i];
            i1 = idx[++i];
            i2 = idx[++i];

            v0 = XMLoadFloat3A(&verts[i0].pos);
            v1 = XMLoadFloat3A(&verts[i1].pos);
            v2 = XMLoadFloat3A(&verts[i2].pos);

            // e0, e1, NullVector, NullVector
            const XMMATRIX e01( XMVectorMultiply( v1 - v0, W_Null),
                                XMVectorMultiply( v2 - v0, W_Null),
                                NullVector,
                                NullVector);
            /*      | e0.x e0.y e0.z 0 |
                    | e1.x e1.y e1.z 0 |
            e01 =   | 0    0    0    0 |
                    | 0    0    0    0 |
            */
            t0 = XMLoadFloat2A(&verts[i0].tex);

            // s = (t1x - t0x, t2x - t0x, t1y - t0y, t2y - t0y)
            XMVECTOR s = XMVectorMergeXY((XMLoadFloat2A(&verts[i1].tex) - t0),
                                         (XMLoadFloat2A(&verts[i2].tex) - t0));
            XMStoreFloat4A(&d, s);
            s *= (1.0f/( d.x * d.w - d.z * d.y )); //DetInv;
            s = XMVectorMultiply(s, signature); // s = (sx, -sy, -sz, sw)

            pSrc = reinterpret_cast<const U32*>(&s);
            pDst = reinterpret_cast<U32*>(&st.r[0]);
            pDst[0] = pSrc[3];  // _00 = sw
            pDst[1] = pSrc[2];  // _01 = -sz
            pDst = reinterpret_cast<U32*>(&st.r[1]);
            pDst[0] = pSrc[1];  // _10 = -sy
            pDst[1] = pSrc[0];  // _11 = sz

            /*      | sw  -sz 0 0 |
                    | -sy sx  0 0 |
            st =    | 0    0  0 0 |*DetInv
                    | 0    0  0 0 |
            */

            const XMMATRIX uv( XMMatrixMultiply(st, e01) );

            vTangents[i0] += uv.r[0];
            vTangents[i1] += uv.r[0];
            vTangents[i2] += uv.r[0];

            vBitangents[i0] += uv.r[1];
            vBitangents[i1] += uv.r[1];
            vBitangents[i2] += uv.r[1];

            ++i;
        }
    } // end scope for temp vars
    i = 0;
    XMVECTOR normal, n0, n1;
    float w;
    while (i < NumberOfVertices) {
        normal = XMVector3Normalize(XMLoadFloat3A(&verts[i].normal));
        // normalize tangents and orthogonalize TBN
        n0 = XMVector3Normalize( vTangents[i] - XMVector3Dot( normal, vTangents[i] ) * normal );
        //calculate handedness
        n1 = XMVector3Cross( normal, vTangents[i] );
        w = (XMVector3Less(XMVector3Dot(n1, vBitangents[i] ), NullVector)) ? -1.f : 1.f;
        n0 = XMVectorSetW(n0, w);
        XMStoreFloat4A(&verts[i].tangent, n0);
        ++i;
    }
}


Optimizing nested loop in run length encoding function I'm currently working on a tool that needs to decode, modify data, then re-encode that data using the Yaz0 format. Everything has been in a working state, but after running some code analysis on my program it turns out it is spending an obscene amount of time on the encoding portion (Over 95%). However, I cannot think of a way to make the loop in this function any more efficient than it already is.

So far I have compacted the inside for loop which used to have a break and two ifs inside it. I also tried using OpenMP, but since I am running async operations on all cores it just hindered performance. I haven't touched anything outside the loop because it is negligible as far as the analysis shows.

Here is a screenshot of the code analysis as well as the function in question. The code is compiled using Microsoft Visual Studio 2013 in debug mode and ran on Windows 8.1.

uint32_t simpleEnc(uint8_t *src, uint32_t size, uint32_t pos, uint32_t *pMatchPos)
{
  int32_t startPos = pos - 0x1000;
  uint32_t numBytes = 1;
  uint32_t matchPos = 0;
  int32_t diff = size - pos;

  if (startPos < 0)
  {
    startPos = 0;
  }

  for (int i = startPos; i < pos; i++)
  {
    for (int j = 0; j < diff && src[i + j] == src[j + pos]; j++)
    {
      if (j > numBytes)
      {
        numBytes = j;
        matchPos = i;
      }
    }
  }

  *pMatchPos = matchPos;

  if (numBytes == 2)
  {
    numBytes = 1;
  }

  return numBytes;
}

Are there any performance optimizations I've missed? I know there is some barrier where this will just take a fair amount of time, but since this is the biggest bottleneck in the program I want it to be as fast as possible.

Output arguments

You currently use a pointer for your output argument. While this is a matter of preference, I found it confusing considering the other pointer argument indicates an array. I would use a reference for pMatchPos instead of a pointer.

const-correctness and immutability

In a pointer-math-heavy function like this, it is especially important to document which values can and cannot be changed within the function body. Use const to specify that a value is not mutable. This allows the compiler to do more error-checking, and makes the code easier to reason about.

Most of your function parameters aren't modified inside the function, so they can be marked const. Note especially src, which has two const modifiers, indicating its value cannot be changed, nor can the data it points to.

Also, startPos is only mutated when it is clamped to zero. You can rewrite this using std::max() (from the algorithm header) to put the initialization on one line, and then make it const as well. Note that using max() requires pos be a signed type, otherwise there will be overflow errors.

An improved beginning of your function would look like this:

uint32_t simpleEnc(uint8_t const * const src, uint32_t const size,
                   int32_t const pos, uint32_t& pMatchPos) // note type changes, cf. above
{
    const int32_t startPos = std::max(0, pos - 0x1000);
    const int32_t diff = size - pos;

Magic numbers

I don't know what 0x1000 is supposed to represent, and neither will anyone else who doesn't intimately understand the algorithm. It would be better to assign it to a named constant. Something like

constexpr uint32_t BACKTRACK_AMOUNT = 0x1000; // Pick a more accurate name, though.

Naming

pMatchPos uses Hungarian notation. Please don't do this. Instead, try to come up with a more descriptive name, or just leave off the prefix.


Pythonic Substring Anagram Index I'm working on improving my Python skills and have been doing a daily code problem. For the problem:

Given a word W and a string S, find all starting indices in S which are anagrams of W

For example s = "abxaba" and w = "ab" should return [0, 3, 4],
s = "cbaebabacd", w = "abc" should return [0, 6]

Is there more efficient/pythonic solution than the code I wrote? Storing the queue as a list and casting it to set twice per loop feels like it must be inefficient.

def anagram_indices(w, s):

  w = set(list(w))

  queue = []
  index = []

  for x, i in enumerate(list(s)):
    queue.append(i)

    while(w.issubset(set(queue[1:]))):
      queue.pop(0)

    if w == set(queue):
      index.append(x-len(w)+1)

  return(index)

word = "ab"
string = "abxaba"

print(anagram_indices(word, string))

index is a singular noun that doesn't represent of group of things, therefor it isn't a good name for an array.

Your method is named anagram_indices and returns an array, I like to call the returned array results, so it's clear that the array are the anagram indices.

Using i and x is very confusing, so much that it made me question my own knowledge of the enumerate function. I expected i to be the index and x to be the letter, while it's the opposite. You should rename those with better variable names. Actually, using single letter variable names is almost never a good idea, there are some edge cases. I expect a variable named i to represent an index, for example.

Your algorithm itself is fine, but the naming is very confusing, which is kind of a problem.


Presenting and passing data to a modal view controller without using prepare(for:sender:) method I am using a toolbar button to present a modal view controller (in which I let the user export data as a PDF file). The main section of my app is a UITableViewController subclass embedded in a UINavigationController.

Here is a schematic of my layout.

The modal itself is embedded in a UINavigationController as I need it to have a bottom toolbar. It also has a transparent background and is presented using .overCurrentContext, so the main screen of the user's data blurs underneath.

I found that to get it to float over everything else (including the navigation bar etc), I had to present it from the UINavigationController (otherwise the main navigation bar and toolbar appeared on top of it).
The problem with this is that the UITableViewController method prepare(for:sender:) is not called.

I call the segue to the modal view controller like this (from the UITableViewController subclass):

// User taps EXPORT button
@objc func exportButtonTapped(_ sender: UIBarButtonItem) {
    self.navigationController?.performSegue(withIdentifier: "showExport", sender: nil)
}

In order to transfer the array of user data to the modal view controller, I have called the following code in the modal view controller:

override func viewDidLoad() {
    super.viewDidLoad()
    // Get data from array in main table view controller
    let masterNav = navigationController?.presentingViewController as! UINavigationController
    let myTableVC = masterNav.topViewController as! MyTableViewController
    self.userData = myTableVC.userData // This is of type: [MyObject]
}

The data is then rendered to a PDF (using HTML templating) in the modal view controller's viewWillAppear() method. This works as expected.

However, I have some concerns about doing it this way:

Is it guaranteed that viewDidLoad() will finish before viewWillAppear() is called? Will an even a larger data set be available for rendering as PDF in viewWillAppear()?
Is it acceptable to present modally from the UINavigationController?
Should I be subclassing the main UINavigationController and using its prepare(for:sender:) method (if this is even an option)?
In the performSegue(withIdentifier:sender:) method, does the sender argument make any difference?
Is it preferable to use present() rather than a segue?

I would of course be grateful for any other advice or refinements to the code. It seems to work as expected but I just want to make sure I following best practice as far as possible.

Is it guaranteed that viewDidLoad() will finish before viewWillAppear() is called? Will an even a larger data set be available for rendering as PDF in viewWillAppear()?

Yes. It needs to be loaded before it will appear.

  Is it acceptable to present modally from the UINavigationController?

I think it is.

  Should I be subclassing the main UINavigationController and using its prepare(for:sender:) method (if this is even an option)?

It sounds a bit complicated. prepare(for:sender:) is not a very clean way to do transfer data to begin with and only useful when you use segues in a regular way. Why don't you create the ModalViewController in code, set the value and then push it through code instead?

  In the performSegue(withIdentifier:sender:) method, does the sender argument make any difference?

I used it in rare occasions to understand where the push was coming from.

  Is it preferable to use present() rather than a segue?

I think in your case yes.


Number-guessing game in C++ I created a simple number-guessing game.  Do I need to divide this program into functions?  Please offer suggestions.

#include<iostream>
#include<string>
#include<ctime>

void drawLine(int n, char symbol);
void rules();

int main()
{
    int amount;
    int bettingAmount;
    int guess;
    int dice;
    std::string playerName;

    drawLine(60,'_');
    std::cout << "\n\n\n\t\tCASINO GAME\n\n\n\n";
    drawLine(60,'_');
    std::cout << "\n\nEnter Your Name : ";
    std::getline(std::cin, playerName);
    std::cout << "\n\nEnter Deposit amount to play game : ";
    std::cin >> amount;
    char choice;
    do
    {
        system("cls");
        rules();
        std::cout << "\n\nYour current balance is $ " << amount << "\n";

        do
        {
            std::cout <<playerName<<", enter money to bet ";
            std::cin >> bettingAmount;
            if(bettingAmount > amount)
                std::cout << "Your betting amount is more than your current balance\n\nRe-enter data\n ";
        }while(bettingAmount > amount);

        do
        {
            std::cout << "Guess your number to bet between 1 to 10 :";
            std::cin >> guess;
            if(guess <= 0 || guess > 10)
                std::cout << "Please check the number!! should be between 1 to 10\n\nRe-enter data\n ";
        }while(guess <= 0 || guess > 10);

        srand(time(0));
        dice = rand()%10 + 1;

        if(dice == guess)
        {
            std::cout << "\n\nGood Luck!! You won Rs." << bettingAmount * 10;
            amount = amount + bettingAmount * 10;
        }
        else
        {
            std::cout << "Bad Luck this time !! You lost $ "<< bettingAmount <<"\n";
            amount = amount - bettingAmount;
        }

        std::cout << "\nThe winning number was : " << dice <<"\n";
        std::cout << "\n"<<playerName<<", You have $ " << amount << "\n";
        if(amount == 0)
        {
            std :: cout << "You have no money to play ";
            break;
        }
        std::cout << "\n\n-->Do you want to play again (y/n)? ";
        std::cin >> choice;
    }while(choice =='Y'|| choice=='y');

    std::cout << "\n\n\n";
    drawLine(70,'=');
    std::cout << "\n\nThanks for playing game. Your balance amount is $ " << amount << "\n\n";
    drawLine(70,'=');

    return 0;
}

void drawLine(int n, char symbol)
{
    for(int i=0; i<n; i++)
        std::cout << symbol;
    std::cout << "\n" ;
}

void rules()
{
    system("cls");
    std::cout << "\n\n";
    drawLine(80,'-');
    std::cout << "\t\tRULES OF THE GAME\n";
    drawLine(80,'-');
    std::cout << "\t1. Choose any number between 1 to 10\n\t2. If you win you will get 10 times of money you bet\n\t3. If you bet on wrong number you will lose your betting amount\n\n";
    drawLine(80,'-');
}

// END OF PROGRAM

Very good.

A few things I would change:

Declare variables as close to the point of usage as possible.

int amount;
int bettingAmount;
int guess;
int dice;
std::string playerName;

This is very C like to declare all your variables at the top.
By declaring them close to the point of usage you don't need to look around for them they should be close to where you are using them. Also C++ has the concept of construction/destruction on objects which comes in useful and is applies at the point of creation.
Validate that your read worked.

std::cin >> amount;

If the user types in "Kevin" then your whole program will stop working. Any time you have user input it is best to read a line of text (as users interact with the terminal a line at a time). Then parse and validate that data out of line (so that failure does not set the bad bit on the stream (because you will need to reset that).

while(true)
{
    std::string   userInput;
    std::getline(std::cin, userInput);

    std::stringstream userInputStream(userInput);
    if (userInputStream >> amount)
    {
        // The user entered a valid number and not text.
        //
        // If you care you may want to check that they did not
        // enter text after the number like "10 Bob"
        // Personally I would not bother for version one

        break;   // exit the loop.
    }
    else
    {
        // invalid Input
        std::cerr << "Invalid Input: Try again\n";
        // Loop re-enters.
        // User trys again.
    }
}

I prefer all sub blocks to have {} around them

if(bettingAmount > amount)
    std::cout << "Your betting amount is more than your current balance\n\nRe-enter data\n ";

// I would write like this:
if(bettingAmount > amount)
{
    std::cout << "Your betting amount is more than your current balance\n\nRe-enter data\n ";
}

This makes it easier to read. Also when modifying the code and needing to add more statements you don't need to remember to add the {}.
Don't re-seed rand.

srand(time(0));

You only need to do this once in the application. Do it as main() is entered. Don't do it again. It breaks the randomness. It's not required for a simple application like this. But if you want to do better then I would look at using the new random number generates that are part of C++11. It will be good practice (rand() is not very good).

Even so the overall structure is still very C-Like.
You could practice your OO style here.

int main()
{
    Game       game;

    HumPlayer  human;
    while(!human.hadEnough())
    {
        game.play(human);
    }

    CompPlayer computer;
    while(!computer.hadEnough())
    {
        game.play(computer);
    }

    std::cout << (human.score() < computer.score())
               ? "The time of the computer overlords has arrived.\n";
               : "Still time before this silly machines are smart.\n";
}


Function for minimum non-zero with multiple additional conditions This function works, but seems unnecessarily complicated.  I would like to simplify but performance is vital as it is called many times.

The conditions are:

  1) If either value is negative, then result is -1

  2) If both values are zero, then result is zero

  3) If NeedMax is true, result is largest value

  4) If NeedMax is false, result is smallest non-zero value

CODE:

Public Function SelValue(ValOne As Long, ValTwo As Long, _
                    Optional NeedMax As Boolean = True) As Long

    If ValOne < 0 Or ValTwo < 0 Then
        SelValue = -1
    ElseIf ValOne = 0 And ValTwo = 0 Then
        SelValue = 0
    ElseIf NeedMax Then
        If ValOne > ValTwo _
            Then SelValue = ValOne _
            Else SelValue = ValTwo
    Else
        If ValOne = 0 Then
            SelValue = ValTwo
        ElseIf ValOne > ValTwo And ValTwo <> 0 Then
            SelValue = ValTwo
        Else
            SelValue = ValOne
        End If
    End If

End Function

Can anyone suggest a more direct, and hopefully faster, approach?

Try to use meaningful names. What's names (SelValue, ValOne,...) stands for? It don't give info on their purpose or usage.

Furthermore, you can simplify a lot your conditional branchments, using only this:

If ValOne < 0 Or ValTwo < 0 Then
    SelValue = -1
' remove this following condition to discard case 4
Else If ValOne = 0 Or ValTwo = 0 Then
    SelValue = ValOne + ValTwo
ElseIf ValOne > ValTwo <> NeedMax Then
    SelValue = ValTwo
Else
    SelValue = ValOne
End If

not fully tested, but should do the trick

Edit : fixed case (4), where NeedMax is false and one of values is 0.


Set of wrapper functions for POSIX signal APIs on Linux Please feel free to comment on the accuracy/validity of the following wrapper source for processing signals using the new POSIX sigaction API. If you feel I'm doing anything wrong or potentially dangerous, chime in.

Note: syserr is a custom function not shown to exit gracefully.

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

static struct sigaction *
handle_signal(int sig,
              int flags,
              void (*handler)())
{
static struct sigaction action;
static struct sigaction o_action;

/* init the sigaction structs */
memset(&action, 0, sizeof(action));
memset(&o_action, 0, sizeof(o_action));

/* setup our signal handler function to call when it fires */
action.sa_handler = handler;

/* initialize the signal mask */
sigemptyset(&action.sa_mask);

/* use the user passed in flags setting othersize init them */
if (flags != 0)
   action.sa_flags = flags;
else
   action.sa_flags = 0;

if (sigaction(sig, &action, &o_action) != 0)
   syserr();

return(&o_action);

}

static struct sigaction *
ignore_signal(int sig)
{
static struct sigaction action;
static struct sigaction o_action;

/* init the sigaction structs */
memset(&action, 0, sizeof(action));
memset(&o_action, 0, sizeof(o_action));

/* setup to ignore this signal */
action.sa_handler = SIG_IGN;

/* initialize the signal mask */
sigemptyset(&action.sa_mask);

action.sa_flags = 0;

if (sigaction(sig, &action, &o_action) != 0)
   syserr();

return(&o_action);

}

int
block_signal(int sig)
{
static sigset_t mask;
static sigset_t orig_mask;
int ret;

sigemptyset(&mask);
sigemptyset(&orig_mask);

sigaddset(&mask, sig);

ret = sigprocmask(SIG_BLOCK, &mask, &orig_mask);

return(ret);
}

int
unblock_signal(int sig)
{
static sigset_t mask;
static sigset_t orig_mask;
int ret;

sigemptyset(&mask);
sigemptyset(&orig_mask);

sigaddset(&mask, sig);

ret = sigprocmask(SIG_UNBLOCK, &mask, &orig_mask);

return(ret);

}

sigset_t *
get_signal_mask()
{
static sigset_t orig_mask;

sigemptyset(&orig_mask);

/* null in set arg means return current mask */
if (sigprocmask(SIG_SETMASK, NULL, &orig_mask) < 0)
   syserr();

return(&orig_mask);

}

static struct sigaction *
handle_signal(int sig,
              int flags,
              void (*handler)())
{

Two things here:

The signal handler function should have the signature void handler(int signum); [man page for sigaction] . Your compiler should — at the least — be warning you about assigning a function pointer of a different type further down.

The function returns a struct sigaction * containing the original disposition for the signal. None of the other functions shown accept a struct sigaction * as an argument, so it's not clear what you're doing with this information.
If you need this information elsewhere, I suggest passing in a struct sigaction * as an additional argument. The POSIX sigaction() API allows you to pass NULL as the third parameter, so this would leave it to the caller to decide whether or not they wanted to provide a pointer to a real struct sigaction. This also ties into the use of statically allocated structures which I address next.

static struct sigaction action;
static struct sigaction o_action;

You declare all of your <signal.h> structures with static storage class, so there is only one instance of each of them in the program's memory space. If the program using this API is multithreaded, and this API is called from multiple locations, you can overwrite the variables on one thread while they're being populated from another, causing you lots of confusion. Since the following code immediately zeros out their memory, you can remove the static from the declarations, so that they get allocated on the stack. That way if the function is called from multiple threads, they each get their own copy and don't conflict with each other.

/* init the sigaction structs */
memset(&action, 0, sizeof(action));
memset(&o_action, 0, sizeof(o_action));

/* setup our signal handler function to call when it fires */
action.sa_handler = handler;

/* initialize the signal mask */
sigemptyset(&action.sa_mask);

/* use the user passed in flags setting othersize init them */
if (flags != 0)
   action.sa_flags = flags;
else
   action.sa_flags = 0;

The else clause is taken if flags has the value 0, so action.sa_flags = 0 is equivalent to action.sa_flags = flags, so this whole if-statement could be collapsed to:
action.sa_flags = flags;

if (sigaction(sig, &action, &o_action) != 0)
   syserr();

return(&o_action);
}

Taken altogether, this function could be rewritten:
static struct sigaction *
handle_signal(int sig,
              int flags,
              void (*handler)(int),
              struct sigaction *o_action)
{
    struct sigaction action;

    memset(&action, 0, sizeof(action));
    /* Note: can't use sizeof(o_action) below because o_action is now a pointer */
    if(o_action != NULL)
        memset(o_action, 0, sizeof(action));

    /* setup our signal handler function to call when it fires */
    action.sa_handler = handler;

    /* initialize the signal mask */
    sigemptyset(&action.sa_mask);

    action.sa_flags = flags;

    if (sigaction(sig, &action, &o_action) != 0)
       syserr();

    return(o_action);
}

If you're using the original signal disposition to restore things to their defaults later on, you could potentially drop o_action altogether and install SIG_DFL as the handler for that signal.  Change the function to return void, only have the original three parameters, change the sigaction call to sigaction(sig, &action, &o_action) and remove the return statement at the end.

static struct sigaction *
ignore_signal(int sig)
{
    /* Code elided */
}

There's a lot of common code between this and handle_signal above; I'd suggest replacing the body of this function with a call to that function instead:
static struct sigaction *
ignore_signal(int sig)
{
    return handle_signal(sig, 0, SIG_IGN);
}

int
block_signal(int sig)
{
static sigset_t mask;
static sigset_t orig_mask;
int ret;

sigemptyset(&mask);
sigemptyset(&orig_mask);

sigaddset(&mask, sig);

ret = sigprocmask(SIG_BLOCK, &mask, &orig_mask);

return(ret);
}

You're not doing anything with the value of orig_mask after the return from sigprocmask(). This system call allows you to pass NULL [man page for sigprocmask], so I suggest you change it to:
ret = sigprocmask(SIG_BLOCK, &mask, NULL);

int
unblock_signal(int sig)
{
static sigset_t mask;
static sigset_t orig_mask;
int ret;

sigemptyset(&mask);
sigemptyset(&orig_mask);

sigaddset(&mask, sig);

ret = sigprocmask(SIG_UNBLOCK, &mask, &orig_mask);

return(ret);

}

Same advice about changing the sigprocmask() call as for block_signal() above. I also note that in the get_signal_mask() function below, you're calling your syserr() function if sigprocmask() fails. It seems to me that if you're exiting the program in that situation, you should also be doing it here where you're actually changing the mask, not merely querying its value.
Additionally, block_signal() and unblock_signal() differ only in the first argument to sigprocmask(), so I'd suggest refactoring the code in both functions into a single helper function that accepts the SIG_BLOCK or SIG_UNBLOCK argument.

sigset_t *
get_signal_mask()
{
static sigset_t orig_mask;

sigemptyset(&orig_mask);

/* null in set arg means return current mask */
if (sigprocmask(SIG_SETMASK, NULL, &orig_mask) < 0)
   syserr();

return(&orig_mask);

}

Again, you're returning a pointer to a statically allocated instance of sigset_t. On multithreaded programs, this can cause problems as already explained. I suggest changing it either to accept a sigset_t * argument, so that the caller must provide the instance to populate (this is the more C-like way of doing it):
sigset_t *
get_signal_mask(sigset_t *orig_mask)
{
    sigemptyset(orig_mask);
    /* null in set arg means return current mask */
    if (sigprocmask(SIG_SETMASK, NULL, orig_mask) < 0)
       syserr();

    return(orig_mask);
}

OR have the function return the sigset_t by value and remove the static from the declaration of orig_mask (this is the more C++-like way of doing it):
sigset_t
get_signal_mask()
{
    sigset_t orig_mask;
    sigemptyset(&orig_mask);

    /* null in set arg means return current mask */
    if (sigprocmask(SIG_SETMASK, NULL, &orig_mask) < 0)
       syserr();

    return(orig_mask);
}

Both methods are valid in both languages, so whichever you prefer.


Simple OpenGL-2D Program I have created mini-opengl program that draw simple chess board on screen by using SFML windows model only. The program runs fine. The main purpose of this program is to keep it as simple as possible and it is not rely on any of OpenGL helper functions like glOrtho or gluOrtho2D for converting coodinates. Also, it is not using any external library for loading textures.

I would like to know how can I improve it?

#include <SFML/OpenGL.hpp>
#include <SFML/Window.hpp>
#include <vector>

int main()
{
    sf::Window window(sf::VideoMode(800, 600), "OpenGL");

    // Set Orthographic
    auto width = static_cast<float>(window.getSize().x);
    auto height = static_cast<float>(window.getSize().y);
    auto a = 2.f / width;
    auto b = 2.f / height;
    std::vector<float> matrix =
    {
         a , 0, 0, 0,
         0, -b, 0, 0,
         0,  0, 1, 0,
        -1,  1, 0, 1
    };

    // Set the projection matrix
    glMatrixMode(GL_PROJECTION);
    glLoadMatrixf(matrix.data());

    // Initialize vertices:
    std::vector<float> vertices =
    {
        0,     0,
        0,     height,
        width, height,
        width, 0
    };

    // Initialize colors
    std::vector<float> colors =
    {
        1, 0, 0,
        0, 1, 0,
        0, 0, 1,
        0, 1, 1
    };

    // Initialize texture virtice
    std::vector<float> texCoord =
    {
        0, 0,
        0, 1,
        1, 1,
        1, 0
    };

    // Create texture: simple chess board 8x8
    auto numRows = 8u;
    auto numCols = 8u;
    auto character = 172u;
    auto remain = 255u - character;
    std::vector<unsigned char> texture(numCols * numRows);
    for (auto i = 0u; i < texture.size(); ++i)
        texture[i] = ((i + (i / numCols)) % 2) * remain + character;

    // Upload to GPU texture
    unsigned textureID;
    glGenTextures(1, &textureID);
    glBindTexture(GL_TEXTURE_2D, textureID);
    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
    glTexImage2D(GL_TEXTURE_2D, 0, GL_LUMINANCE, numCols, numRows, 0, GL_LUMINANCE, GL_UNSIGNED_BYTE, texture.data());

    // Initialize clear colors
    glClearColor(0.f, 0.f, 0.f, 1.f);

    // Activate necessary states
    glEnable(GL_TEXTURE_2D);

    glEnableClientState(GL_VERTEX_ARRAY);
    glEnableClientState(GL_COLOR_ARRAY);
    glEnableClientState(GL_TEXTURE_COORD_ARRAY);

    glVertexPointer(2, GL_FLOAT, 0, vertices.data());
    glColorPointer(3, GL_FLOAT, 0, colors.data());
    glTexCoordPointer(2, GL_FLOAT, 0, texCoord.data());

    while (window.isOpen())
    {
        sf::Event windowEvent;
        while (window.pollEvent(windowEvent))
        {
            if (windowEvent.type == sf::Event::Closed)
                window.close();
        }

        // render
        glClear(GL_COLOR_BUFFER_BIT);

        glDrawArrays(GL_QUADS, 0, 4);

        window.display();
    }
}

This is a pretty straightforward program. Overall, very easy to read, and makes sense. Here are a few things I'd do differently:

Use Types

You have several places where you use an unstructured vector of floats. While that's the formate you need to send to the GPU, you don't have to make it so hard on yourself (and anyone who's going to read your code in the future). You should make a 2D point structure, an RGB color structure, and a matrix class to handle those very different types of data. You might look into glm which has several data types that mimic those used in vertex and fragment shaders.

Or create your own:

struct Point2D {
    float x;
    float y;
};

struct RGBColor {
    float red;
    float green;
    float blue;
};

etc.

Use Functions

You code for creating and uploading the texture is really tangential to the rest of the code. You should put those into either a single other function, or 2 other functions (one to create the data and one to upload it). If it were me, I'd do 2, as I know that I upload a lot of textures when working with OpenGL and the code is going to mostly be the same.

Speaking of creating the texture, your code is difficult to understand. It would be simpler to do something like this:

// Create texture: simple chess board 8x8
const unsigned char kBlack = 0;
const unsigned char kWhite = 0;
auto numRows = 8u;
auto numCols = 8u;
std::vector<unsigned char> texture(numCols * numRows);
for (auto row = 0u; row < numRows; ++row)
{
    for (auto col = 0u; col < numCols; ++col)
    {
        bool evenRow = (row % 2) == 0;
        bool evenCol = (col % 2) == 0;
        if (evenRow == evenCol)
        {
            texture [ row * numCols + col ] = kWhite;
        }
        else
        {
            texture [ row * numCols + col ] = kBlack;
        }
    }
}

If You Set State, Use It

I notice that you're using a color array, but then you upload a texture, and you don't specify GL_MODULATE for the texture parameters. What is the point of setting the vertex colors if you aren't going to use them? (I don't see any shaders, so presumably you aren't using them there, either.) If you aren't going to use the vertex colors, don't waste time uploading them to the card.

Also, when you've completed drawing the arrays, you should disable the client state you set above, as when you inevitably expand this code in the future, those values will need to change for each object you draw. Your draw loop should look something like this:

while (window.isOpen())
{
    sf::Event windowEvent;
    while (window.pollEvent(windowEvent))
    {
        if (windowEvent.type == sf::Event::Closed)
            window.close();
    }

    // render
    glClear(GL_COLOR_BUFFER_BIT);

    glEnableClientState(GL_VERTEX_ARRAY);
    glEnableClientState(GL_COLOR_ARRAY);
    glEnableClientState(GL_TEXTURE_COORD_ARRAY);

    glVertexPointer(2, GL_FLOAT, 0, vertices.data());
    glColorPointer(3, GL_FLOAT, 0, colors.data());
    glTexCoordPointer(2, GL_FLOAT, 0, texCoord.data());

    glDrawArrays(GL_QUADS, 0, 4);

    glDisableClientState(GL_VERTEX_ARRAY);
    glDisableClientState(GL_COLOR_ARRAY);
    glDisableClientState(GL_TEXTURE_COORD_ARRAY);

    window.display();
}

And really all the drawing stuff should be in some sort of drawObjects() function.