Computing even huger Fibonacci numbers in Java - follow-up See the previous and initial iteration.

I have incorporated almost all the suggestions by Peter Taylor:

The actual method returns a BigInteger instead of String.
The actual method can deal with Fibonacci numbers with negative indices, so there is no any need to throw IllegalArgumentException.
The only base case that is checked is \$n = 0\$.
fibonacciMatrix renamed to pow.
main is now more sensible.

(The matrix power method is still recursive.)

See what I have:

import java.math.BigInteger;

public class LargeFibonacciNumbers {

    public static BigInteger fibonacci(int n) {
        if (n == 0) {
            return BigInteger.ZERO;
        }

        BigInteger[][] matrix = new BigInteger[2][2];

        matrix[0][0] = BigInteger.ONE;
        matrix[0][1] = BigInteger.ONE;
        matrix[1][0] = BigInteger.ONE;
        matrix[1][1] = BigInteger.ZERO;

        BigInteger tmp = pow(matrix, Math.abs(n))[0][1];

        if (n > 0 || ((n & 1) == 1)) {
            return tmp;
        } else {
            return tmp.negate();
        }
    }

    private static BigInteger[][] multiply(BigInteger[][] matrix1,
                                           BigInteger[][] matrix2) {
        BigInteger[][] ret = new BigInteger[2][2];

        ret[0][0] = matrix1[0][0].multiply(matrix2[0][0])
               .add(matrix1[0][1].multiply(matrix2[1][0]));

        ret[0][1] = matrix1[0][0].multiply(matrix2[0][1])
               .add(matrix1[0][1].multiply(matrix2[1][1]));

        ret[1][0] = matrix1[1][0].multiply(matrix2[0][0])
               .add(matrix1[1][1].multiply(matrix2[1][0]));

        ret[1][1] = matrix1[1][0].multiply(matrix2[0][1])
               .add(matrix1[1][1].multiply(matrix2[1][1]));

        return ret;
    }

    private static BigInteger[][] pow(BigInteger[][] matrix, int n) {
        if (n == 1) {
            // End the recursion.
            return matrix;
        }

        BigInteger[][] tmp = pow(matrix, n >> 1);
        tmp = multiply(tmp, tmp);

        if ((n & 1) == 1) {
            tmp = multiply(matrix, tmp);
        }

        return tmp;
    }

    public static void main(String[] args) {
        if (args.length > 0) {
            int n;

            try {
                n = Integer.parseInt(args[0]);
            } catch (NumberFormatException ex) {
                System.err.println("Could not parse \"" + args[0] +
                                   "\" as an integer.");
                return;
            }

            System.out.println(fibonacci(n));
        } else {
            System.out.println("Usage: java -jar File.jar N");
            System.out.println("Where N is the index of the Fibonacci number " +
                               "to compute.");
        }
    }
}

So, what do you think?

Theory

As described in a related Python answer, it's possible to refactor the recursion into a for loop and to use matrix arithmetics without using matrices.

Code

import java.math.BigInteger;

public class LargeFibonacciNumbers
{

    public static BigInteger fibonacci(int n) {
        BigInteger f_n = BigInteger.ZERO;
        BigInteger f_n_plus_1 = BigInteger.ONE;
        BigInteger two = BigInteger.valueOf(2);
        BigInteger f_n_plus_1_squared, f_2n, f_n_squared, f_2n_plus_1;
        for (int i = Integer.SIZE - Integer.numberOfLeadingZeros(n); i >= 0; i--) {
            f_n_squared = f_n.multiply(f_n);
            f_n_plus_1_squared = f_n_plus_1.multiply(f_n_plus_1);
            f_2n = f_n.multiply(f_n_plus_1).multiply(two).subtract(f_n_squared);
            f_2n_plus_1 = f_n_squared.add(f_n_plus_1_squared);
            if ((n >> i & 1) == 1) {
                f_n = f_2n_plus_1;
                f_n_plus_1 = f_2n.add(f_2n_plus_1);
            } else {
                f_n = f_2n;
                f_n_plus_1 = f_2n_plus_1;
            }
        }
        return f_n;
    }

    public static void main(String[] args) {
        int n = 10000000;
        long start = System.nanoTime();
        System.out.println(fibonacci(n).bitLength());
        long finish = System.nanoTime();
        System.out.println((finish - start) / 1000000 + " ms");
    }
}

Comments

bitLength() isn't defined for ints, so this method has been used instead.

Performance

This code calculates the 10-millionth Fibonacci number in less than 1.5s on my computer. In comparison, the matrix version needs almost 4s.

Note that the test code only displays the bit length of the result, not the result itself. fibonacci(1E7) has more than 2 million digits, and it takes much longer to display it in the console than to calculate it.


Saving a sales order - too many repositories? I am wondering if I have overdone it with repositories in the following code, which is to save a sales order.

I understand that the purpose of a repository is to decouple the domain layer from the persistence layer, but this code seems to be replicating what EF does anyway.

public SalesOrder FindSalesOrder(int id)
{
    using (var uow = new UnitOfWork<LogContext>())
    {
        using (var repository = new SalesOrderRepository(uow))
        {
            var SalesOrder = repository.Find(s => s.Id == id, s => s.Site, s => s.User);
            if (SalesOrder != null)
            {
                using (var repository2 = new OrderLineRepository(uow))
                {
                    var OrderLines = repository2.GetList(o => o.SalesOrderId == id);
                    foreach (var OrderLine in OrderLines)
                    {
                        OrderLine.SalesOrder = SalesOrder;
                        SalesOrder.OrderLines.Add(OrderLine);

                        using (var repository3 = new LineDetailRepository(uow))
                        {
                            var LineDetails = repository3.GetList(v => v.OrderLineId == OrderLine.Id, v => v.PropertyName);
                            foreach(var LineDetail in LineDetails)
                            {
                                LineDetail.OrderLine = OrderLine;
                                OrderLine.LineDetails.Add(LineDetail);
                            }
                        }
                    }
                }
            }

            return SalesOrder;
        }
    }
}

[Update]
I had a chat to my co-developer and think that perhaps the code is OK.

Our reasoning is that EF itself is rather complicated so we want to have a wrapper around what we allow the UI layer to use.

We did realize that we don't need the lineDetail repository as there are no instances where we need to access the lineDetails separate from the orderLines. That means we could make the orderLines return containing the lineDetails pre-populated.

However that would then mean some of our repository's Find methods return entities with children and some don't.  Thus I am thinking maybe we need a FindGraph method in our repositories which will return the full graph.

However now I wonder whether the FindSalesOrder method should be in the SalesOrder repository itself instead of the UI layer?

I find it interesting that you want to decouple layers, but that the method you have here is tightly coupled with UnitOfWork<LogContext> (and thus with LogContext as well), OrderLineRepository and OrderDetailRepository.
To answer the question directly, yes, it's way overdone.
I'd put a method like SalesOrder FindSalesOrder(int id) in a service layer:
public class SalesOrderService
{
    private readonly IUnitOfWork _uow;

    public SalesOrderService(IUnitOfWork uow)
    {
        _uow = uow;
    }

    public SalesOrder FindSalesOrder(int id)
    {
        return _uow.Set<SalesOrder>() // get repository
                   .Where(order => order.Id == id) // filter by id
                   .Include(order => order.OrderLines) // eager-load details
                   .ToList(); // materialize
    }
}

What's IUnitOfWork, you'll ask? Just an abstraction that looks something like this:
internal interface IUnitOfWork
{
    DbSet<TEntity> Set<TEntity>();
    void Commit();
}

The EF context itself implements it:
public class MyDbContext : DbContext, IUnitOfWork
{
    public IDbSet<SalesOrder> Orders { get; set; }

    public void Commit()
    {
        SaveChanges();
    }
}

Entity Framework's DbContext is a unit-of-work implementation, and IDbSet<T> is a repository.
Constructor-injecting an IUnitOfWork in your service class effectively decouples your service class from EF, at least good enough for unit testing (you can mock what Set<T>() returns)... "swapping the ORM" will require lots of work anyway, and is highly hypothetical - the cost vs benefits ratio doesn't add up in favor of abstracting EF with repositories.

Your Code
There are a number of itchy spots with the code you've provided:

Casing is annoyingly inconsistent for local variables. Should be camelCase, always: var LineDetails becomes var lineDetails, for example.
using blocks should be stacked whenever possible, to reduce unnecessary nesting:

using (var uow = new UnitOfWork<LogContext>())
{
    using (var repository = new SalesOrderRepository(uow))
    {
        var SalesOrder = repository.Find(s => s.Id == id, s => s.Site, s => s.User);
        if (SalesOrder != null)
        {

Becomes:
using (var uow = new UnitOfWork<LogContext>())
using (var repository = new SalesOrderRepository(uow))
{
    var SalesOrder = repository.Find(s => s.Id == id, s => s.Site, s => s.User);
    if (SalesOrder != null)
    {


Image processing: rotation I've made two functions for image processing:

First one is for rotating an image
Second one is to set all pixels of one color to another color

Rotate Image:

private static void roateImage(BufferedImage pic1) throws IOException {
    int width = pic1.getWidth(null);
    int height = pic1.getHeight(null);

    double angle = Math.toRadians(90);
    double sin = Math.sin(angle);
    double cos = Math.cos(angle);
    double x0 = 0.5 * (width - 1);     // point to rotate about
    double y0 = 0.5 * (height - 1);     // center of image

    WritableRaster inRaster = pic1.getRaster();
    BufferedImage pic2 = new BufferedImage(width, height, BufferedImage.TYPE_INT_RGB);
    WritableRaster outRaster = pic2.getRaster();
    int[] pixel = new int[3];

    // rotation
    for (int x = 0; x < width; x++) {
        for (int y = 0; y < height; y++) {
            double a = x - x0;
            double b = y - y0;
            int xx = (int) (+a * cos - b * sin + x0);
            int yy = (int) (+a * sin + b * cos + y0);

            if (xx >= 0 && xx < width && yy >= 0 && yy < height) {
                outRaster.setPixel(x, y, inRaster.getPixel(xx, yy, pixel));
            }
        }
    }
    ImageIO.write(pic2, "bmp", new File("Images/Output2.bmp"));
}

Please help me to optimize this code.

Math is fun; why waste it on the computer?

    double angle = Math.toRadians(90);
    double sin = Math.sin(angle);
    double cos = Math.cos(angle);

This is the same as

    int sin = 1;
    int cos = 0;

So

            int xx = (int) (+a * cos - b * sin + x0);
            int yy = (int) (+a * sin + b * cos + y0);

simplifies to

            int xx = (int) (- b + x0);
            int yy = (int) (+a + y0);

which simplifies to

            int xx = (int) (y0 + x0) - y;
            int yy = x + (int) (y0 - x0);

And you don't need a or b at all.  But you could precalculate

    int sum = (int) (y0 + x0);
    int difference = (int) (y0 - x0);

right after you calculate y0 and x0 (i.e. before the loop).  Then in the loop, you just need

            int xx = sum - y;
            int yy = difference + x;

Then you could rewrite

            if (xx >= 0 && xx < width && yy >= 0 && yy < height) {
                outRaster.setPixel(x, y, inRaster.getPixel(xx, yy, pixel));
            }

as

            if (sum >= y && sum - width < y && x >= -difference && x < height - difference) {
                outRaster.setPixel(x, y, inRaster.getPixel(xx, yy, pixel));
            }

But then we can change

    for (int x = 0; x < width; x++) {
        for (int y = 0; y < height; y++) {

to

    int right = Math.min(width, height - difference);
    int bottom = Math.min(height, sum + 1)
    for (int x = Math.max(0, -difference); x < right; x++) {
        for (int y = Math.max(0, sum - width + 1); y < bottom; y++) {

Now we don't have to check if xx and yy fit in the bounds.  We only iterate over the pixels where they do.  So we can drop the if and just say

            outRaster.setPixel(x, y, inRaster.getPixel(xx, yy, pixel));

TL;DR

private static void rotateImage(BufferedImage pic1) throws IOException {
    int width = pic1.getWidth(null);
    int height = pic1.getHeight(null);

    // point to rotate about center of image
    double x0 = 0.5 * (width - 1);
    double y0 = 0.5 * (height - 1);

    int sum = (int) (y0 + x0);
    int difference = (int) (y0 - x0);

    WritableRaster inRaster = pic1.getRaster();
    BufferedImage pic2 = new BufferedImage(width, height, BufferedImage.TYPE_INT_RGB);
    WritableRaster outRaster = pic2.getRaster();
    int[] pixel = new int[3];

    // rotation
    int right = Math.min(width, height - difference);
    int bottom = Math.min(height, sum + 1)
    for (int x = Math.max(0, -difference); x < right; x++) {
        for (int y = Math.max(0, sum - width + 1); y < bottom; y++) {
            int xx = sum - y;
            int yy = difference + x;

            outRaster.setPixel(x, y, inRaster.getPixel(xx, yy, pixel));
        }
    }

    ImageIO.write(pic2, "bmp", new File("Images/Output2.bmp"));
}

I also changed the spelling to rotateImage.

Now the only math done inside the loops is incrementing the index variables and simple integer subtraction and addition.

And we only increment over pixels that we're copying.

It's odd to take an image as input and then write a different image to a file.  Why not return the image instead?  Then the caller can save it.

Consider if you can make inRaster a Raster instead.  I don't think that it needs to be writable, just outRaster.


Accessing the only element of Java ResultSet Does this code look OK?

public Map<String, String> getById(long id) {
    String sql = "select * from " + this._TABLE_NAME + " where id = ?";
    PreparedStatement p;
    Map<String, String> result = new HashMap<String, String>();
    try {
        p = conn.prepareStatement(sql);
        p.setLong(1, id);
        ResultSet rs = p.executeQuery();

        ResultSetMetaData rsmd = rs.getMetaData();
        int fieldsCount = rsmd.getColumnCount();
        // is this ok?
        rs.next();

        for (int i=1;i<fieldsCount+1;i++) {
            String cName = rsmd.getColumnName(i);
            result.put(cName, rs.getString(cName));
        }

    } catch (SQLException e) {
        e.printStackTrace();
    }

    return result;

}

Here is a bit reworked version of you code. Some explanation:

Assume _TABLE_NAME is a constant, so there is no need to build query in every method call.
Force getting connection and close resources to avoid side effects.
Propagate SQLException to caller (caller should decide what to do with thrown exception).

Code:

private final static String GET_DATA_BY_ID = "select * from " + _TABLE_NAME + " where id = ?";

public Map<String, String> getById(long id) throws SQLException {

    Connection conn = null;
    Map<String, String> result = new HashMap<String, String>();

    try {
        conn = getConnection();

        PreparedStatement preparedStatement = null;

        ResultSet rs = null;
        try {
            preparedStatement = conn.prepareStatement(GET_DATA_BY_ID);
            preparedStatement.setLong(1, id);
            try {
                rs = preparedStatement.executeQuery();

                if (rs.next()) {
                    ResultSetMetaData rsmd = rs.getMetaData();
                    int fieldsCount = rsmd.getColumnCount();

                    for (int i = 1; i < fieldsCount + 1; i++) {
                        String cName = rsmd.getColumnName(i);
                        result.put(cName, rs.getString(cName));
                    }
                }
            } finally {
                if (rs != null)
                    rs.close();
            }
        } finally {
            if (preparedStatement != null)
                preparedStatement.close();
        }
    } finally {
        if (conn != null)
            conn.close();
    }
    return result;
}


Scan the user input and then work with these letters The program scans a maximum of 10 chars from user and then offers you 4 options what you can do with that array. Options are:

0 = quit the program,
1 = prints whole array,
2 = count all the small letters,
3 = prints only the numbers,
4 = change the small letters with the big ones and via versa.

This is my homework for school, so the interface needs to be in Czech. I think that overall it's quite good but I want to improve!!

#include <stdio.h>

int main() {

    int char_count = 10;
    char input_chars[char_count];
    int i = 0;
    int applied_numb = 1;
    int last_index;
    int amount_small_char = 0;
    int ascii_code;

    printf("Zadejte maximalne 10 znaku" );
    do {
        printf(" \n Zadejte %d z %d: ", i, char_count);
        input_chars[i] = getchar();
        getchar();
        i++;
            if(i == char_count) {
                printf("Zadali jste maximalni pocet znaku. \n");
        }
    } while(i < char_count && input_chars[i - 1] != '.' );

    if(input_chars[i - 1] == '.')
    {
        // in case of last element is "." substract 2 from last index(because we used do while cycle (i++))
        last_index = i - 2;
    }
    else
    {
        last_index = i - 1;
    }

    while (applied_numb != 0 && applied_numb <= 4 && applied_numb >= 0)
    {
        printf("Vyberte si co chcete s danymi cisli provest: \n 0) konec programu \n 1) vypis \n 2) pocet malych pismen \n 3) vypsat jen cislice \n 4) zmenit mala/velka pismena\n");
        scanf ("%i", &applied_numb);

        switch (applied_numb)
        {
            case 0:
                printf ("KONEC PROGRAMU, SEE YOU SOON :) ");
                break;
            case 1:
                printf("Vypis vaseho vstupuje: ");
                for(i = 0; i <= last_index; i++)
                {
                    printf(" %c",input_chars[i]);
                }
                break;

            case 2:
                for (i = 0; i <= last_index; i++)
                {
                    ascii_code = (int)input_chars[i];
                    if (ascii_code >= 97 && ascii_code <= 122)
                    {
                        amount_small_char = amount_small_char + 1;
                    }
                }
                printf("Pocet malych pismen ve vasim vstupu je:  %d", amount_small_char);
                break;
            case 3:
                printf("Tyto cislice jsou ve vasem vstupu:");
                for (i = 0; i <= last_index; i++)
                {
                    ascii_code = (int)input_chars[i];
                    if (ascii_code >= 48 && ascii_code <= 57)
                    {
                        printf(" %c", input_chars[i]);

                    }
                }
                break;
            case 4:
                printf("Zmena na mala/velka pismena vypada takto:");
                for (i = 0; i <= last_index; i++)
                    {
                        ascii_code = (int)input_chars[i];
                        if (ascii_code >= 97 && ascii_code <= 122)
                        {
                            ascii_code = ascii_code - 32;
                            printf(" %c", ascii_code);
                        }
                        else if(ascii_code >= 65 && ascii_code <= 90)
                        {
                            ascii_code = ascii_code + 32;
                            printf(" %c", ascii_code);
                        }
                        else if((ascii_code <= 64 && ascii_code >= 0) ||
                                (ascii_code >= 91 && ascii_code <= 96) ||
                                (ascii_code >= 123))
                        {
                            printf(" %c", input_chars[i]);
                        }
                    }
                break;
        }

        printf("\n  \n ------------------------------------- \n \n ");
    }
    return 0;
}

You just wrote this program and it's perfectly clear in your mind, imagine to come back after 1 year: you will need to re-read everything to understand what it is doing because everything is in one big function (which is also hard to test, something you will definitely love to have when writing production code).

First of all let's split the logic into smaller functions, do it in small steps:

#define MAX_INPUT_LENGTH 10

int main()
{
    char input[MAX_INPUT_LENGTH + 1] = {0};
    if (!read_input_text(input, MAX_INPUT_LENGTH))
        return EXIT_FAILURE;

    while (1)
    {
        print_menu();

        switch (read_menu_option())
        {
            case 0:
                puts("KONEC PROGRAMU, SEE YOU SOON :) ");
                return EXIT_SUCCESS;
            case 1:
                print_input(input);
                break;
            case 2:
                print_count_lowercase_letters(input);
                break;
            case 3:
                print_digits(input);
                break;
            case 4:
                print_input_toggled(input);
                break;
            default:
                puts("Invalid option.");
        }
    }

    return EXIT_SUCCESS;
}

You can further split this code and menu may be well handled with a dictionary of functions to call (it will be easier to extend) but I think you understand the point. Few things to note:

I initialized input to zero, now you do not need to last_index because you simply can use strlen(). Also to print the string is as simple as puts(input) because you  have a valid null terminated C string (note the extra array item added to hold the string terminator).
Variable-length arrays are a subtle thing, you may avoid them unless they're absolutely required.
I'm using EXIT_SUCCESS instead of 0, it's a small thing but I think it helps to make the intent clear (and it's easier to search...)
I'm using puts() instead of printf() when I simply need to print a string to stdout: it's faster, easier and better communicate the intent (compiler may also detect what you're doing and use puts() for you, BTW).

Now let's examine the core logic. As you can already imagine the input function might be simplified:

bool read_input_text(char* input, size_t maximumLength)
{
    for (size_t i=0; i < maximumnLength; ++i)
    {
        int c = getchar();

        if (c == EOF)
            return false;

        if (c == '.')
            return true;

        input[i] = (char)c;
    }

    return true;
}

Few notes:

I'm using a for loop to read needed characters, I think it's shorter and easier to understand than the equivalent while.
I check for EOF, user may press CTRL+D/CTRL+Z or stdin may be redirected and input file may be too short. As Chux noted you may consider EOF when string is still empty as a valid input (returning true).
When user type . I do not write it to input and because it's already zeroed then I simply can exit.
I'm using bool and other macros defined in stdbool.h.
I'm not allocating memory in this function, you may also change it to be char* read_input_text() but it's less error-prone (in bigger programs) to have a clear ownership for dynamically allocated memory.
read_input_text() is assuming that input is already zeroed. In true production code this should not be the case (less function interface requirements mean less bugs...) You may try to change it to append a trailing 0 where appropriate (tnx Chux, again, to point this out.)

How to count for lowercase letters? You're doing that manually but it's little bit hard to read because you're using magic numbers, let's try to write a small function:

int count_lowercase_characters(char* text)
{
    int count = 0;

    for (size_t i=0; i < strlen(text); ++i)
    {
        if (text[i] >= 'a' && text[i] <= 'z')
            ++count;
    }

    return count;
}

Using 'a' and 'z' instead of their ASCII values makes code easier to understand (I personally do not remember ASCII codes...) and less error-prone. But, hey...isn't it a very common task? There is a standard function for that, let's use it:

int count_lowercase_characters(const char* text)
{
    int count = 0;

    for (size_t i=0; i < strlen(text); ++i)
    {
        if (islower((unsigned char)text[i])
            ++count;
    }

    return count;
}

Note:

I'm directly using strlen() and loop variable is size_t. Note that it's not mandatory for size_t to be equivalent to int!
I'm using int even if unsigned (or even better size_t) might be more appropriate, don't go into academic discussions about this...
I declare text as const char*, I'm not going to modify the pointed value. Later you may want to declare it as const char* const but no need to go that far, now (also consider to declare it as char const *, it may be easier to pick when beginning).
I'm casting text[i] to unsigned char, if your compiler is aliasing char to unsigned char then you do not need it. See also Do I need to cast to unsigned char before calling toupper?

To write print_count_lowercase_letters() should now be trivial. As you can imagine there is also a similar function isdigit() to determine if a char is a digit (0..9) to use in print_digits().

The last function we have to go through is the one to toggle the case:

if (islower(text[i]))
    text[i] = toupper(text[i]);
else if (isupper(text[i]))
    text[i] = tolower(text[i]);

In this case I'm toggling the original string but you may simply write it to stdout. Note that you're using printf() to print a single character, there is an easier way:

if (islower((unsigned char)text[i]))
    putchar(toupper(text[i]));
else if (isupper((unsigned char)text[i]))
    putchar(tolower(text[i]));
else
    putchar(text[i]);

Notes:

I'm using putchar() to print a single character to stdout. Easier and faster.
I'm using islower() and isupper() to determine the case of the character. If you can't use these library functions then write your own function to reuse.
I'm using tolower() and toupper() to perform case conversion. Again if you can't use them then write your own, you will gain in legibility.
Last else has been greatly simplified, string is user input then it already contains printable characters, everything else which is not a letter should simply go untouched.
I didn't write it but the loop is similar to the previous function for (size_t i=0; i < strlen(text); ++i). See very last paragraph for more info.

In this examples I didn't consider any optimization, you will soon learn that strlen(text) as condition in a for loop is relatively terrible from a performance point of view (because outside very simple cases compiler isn't able to determine that it can be optimized and evaluated just once). The same loop may then be rewritten (few solutions ordered for complexity+speed).

To evaluate strlen(text) just once:

for (size_t i=0, n = strlen(text); i < n; ++i)

To manually stop when encountering string terminator:

for (size_t i=0; text[i] != 0; ++i)

To fully use pointers arithmetic:

for (char *p = text; *p; ++p)


Implement `fold` on a "Rose Tree" Given the following algebraic data type, a Rose Tree:

data Tree a = Node {
    rootLabel :: a,
    subForest :: [Tree a]
}

I attempted a foldTree function to fold over this list: (credit to this class's homework from 2013:

treeFold :: (b -> [b] -> b) -> (a -> b) -> Tree a -> b
treeFold f g tree = f (g (rootLabel tree)) (map (g . rootLabel) (subForest tree))

test

*Party> let tree = Node { rootLabel = 100, subForest = [] }
*Party> let tree2 = Node { rootLabel = 200, subForest = [tree] }
*Party> add tree2
300

Please review this implementation.

Given my definition of treeFold, I don't see how I could fold over a Tree Char, producing a [Char]/String result.

My understanding is that, for the return type, b, it can't be [a].

The implementation isn't correct, because it doesn't traverse sub-trees. Consider

Node 1 [Node 2 [Node 3 []]]

Then your folding function will only fold over 1 and 2, but not over 3.

If you have a recursive structure like this, a folding function over it must also be recursive. Otherwise it won't be able to traverse arbitrarily large recursive structure.

For the other question: If you specialize the folding function as

treeFold :: ([Char] -> [[Char]] -> [Char]) -> (Char -> [Char])
         -> Tree Char -> [Char]

by setting b = [Char], you get what you're looking for - converting a Tree Char to String. You just need to supply the two function for folding, for example

treeFold (\x ys -> x ++ concat ys) (: [])

Update: The signature also isn't correct. The general rule is that the folding function should have one additional argument for each constructor of the data type where recursive types (here Tree a) are replaced by the result of the fold:

treeFold :: (a -> [b] -> b) -> Tree a -> b

For example for a list you have 2 constructors: (:) :: a -> [a] -> [a] and [] :: [a], so its folding function is

foldr :: (a -> b -> b) -> b -> [a] -> b


Python: keeping track of info across classes by storing a variable to self I wrote the following code to test a score-keeping class.  The idea was to keep score of a game such that the score was kept across multiple classes and methods.

I'm looking for any input on being more efficient, more 'pythonic' and/or just better.

import os

class Foo():
    def __init__(self):
        self.stored_end = 0

    def account(self, a, b):
        c = float(a) + b
        print a
        print b
        print c
        self.stored_end = c
        print self.stored_end

    def testy(self, q, v):
        print "\n"
        print " _ " * 10
        z = float(q) + v
        print self.stored_end
        self.stored_end = self.stored_end + z
        print " _ " * 10
        print self.stored_end

class Bar():
    def __init__(self):
        pass

    def zippy(self, a, b):
        print " _ " * 10
        print "this is zippy"
        foo.testy(a, b)

class Baz():
    def __init__(self):
        pass

    def cracky(self, g, m):
        y = g + m
        print " _ " * 10
        print "calling stored_end"
        foo.stored_end = foo.stored_end + y
        print " _ " * 10
        print "this is cracky"
        print "y = %r" % y
        print foo.stored_end

os.system("clear")
foo = Foo()
foo.account(5, 11)
foo.testy(100, 100)
bar = Bar()
bar.zippy(10, 100)
baz = Baz()
baz.cracky(1000, 1)

I think what you may be looking for is the Borg design pattern. It's meant for a single class to maintain state between multiple instances. Not exactly what you're looking for but you could modify it to also maintain state across multiple classes, perhaps by specifying a global for shared state:

## {{{ http://code.activestate.com/recipes/66531/ (r2)
class Borg:
    __shared_state = {}
    def __init__(self):
        self.__dict__ = self.__shared_state
    # and whatever else you want in your class -- that's all!
## end of http://code.activestate.com/recipes/66531/ }}}

Here are some links to various code examples of this, non chronological:

Python borg pattern problem
Singleton? We don't need no stinkin' singleton: the Borg design pattern (Python recipe)
Usage example


Testimonial section with HTML tags I created this testimonial on CodePen

I am a bit skeptical about a few things in my HTML structure. For example, I typically see testimonials enclosed in
s with custom classes. In my case I used a
 but had to overwrite a lot of rules.

Also wondering if enclosing the author in  tags was wise.

HTML

    &ldquo;Such cool. Much awesome. WOW&rdquo;

    &ndash;
    Doge,
    The Moon

CSS

/* == resets == */
body { margin: 0; padding: 0; }
/* == project == */
body {
  background: none repeat scroll 0% 0% rgb(240, 240, 240);
  color: rgb(102, 102, 102);
  font-family: "Helvetica Neue", Helvetica, Arial, Sans-serif;
  font-size: 22px;
}
.wrapper {
  width: 600px;
  margin: 24px auto;
}

blockquote {
  background-color: rgb(255, 255, 255);
  border-radius: 6px;
  font-family: Georgia, serif;
  font-size: 22px;
  line-height: 1.4;
  margin: 0;
  padding: 17px;
}
p.author {
  background-color: transparent;
  font-weight: 500;
  font-size: 22px;
  line-height:22px;
  margin: 24px 0 0 18px;
}
strong {
  color: rgb(68, 68, 68);
}

a {
  color: rgb(64, 131, 169);
  text-decoration: none;
}

I would say that for it to be semantically accurate, the author should be a part of the blockquote, perhaps using a footer.
You should include a cite attribute if the quote has a source.
The quote content should be inside a paragraph element.
I guess now you don't need the wrapper any more.

&ldquo;Such cool. Much awesome. WOW&rdquo;

       &ndash;
       Doge,
       The Moon

To make it look the same I had to change these bits of CSS:

blockquote {
  font-size: 22px;
  margin: 0;
}

blockquote p {
  background-color: rgb(255, 255, 255);
  border-radius: 6px;
  font-family: Georgia, serif;
  line-height: 1.4;
  padding: 17px;
}

footer.author {
  background-color: transparent;
  font-weight: 500;
  font-size: 22px;
  line-height:22px;
  margin: 24px 0 0 18px;
}

Here's the result.


Longest Common Prefix of string Can anyone help me remove TLE (time limit exceeded) from this?

  Given integer n and array of string of length n. In each query (q of them) we are given l and r and we need to find LCP in this subsequence. Q and N <= 10^5. The sum of lengths of strings <= 10^5.

#include <bits/stdc++.h>

using namespace std;

typedef long long ll;
typedef pair<int, int> pi;
typedef pair<ll, ll> pl;
typedef vector<int> vi;
typedef vector<ll> vl;
typedef vector<double> vd;
typedef vector<bool> vb;
typedef vector<char> vc;
typedef vector<string> vs;
typedef vector<pi> vp;
typedef vector<pl> vpl;

int lcp(string a, string b){
    int sol = 0;
    for (int i = 0; i < min(b.size(), a.size()); ++i){
        if (a[i] == b[i])
            ++sol;
        else
            break;
    }

    return sol;
}

int d[10000][10000];

int main()
{
    ios_base::sync_with_stdio(false);
    cin.tie(nullptr);
    cout.tie(nullptr);
    cerr.tie(nullptr);

    int t;
    cin >> t;

    for (int e = 0; e < t; ++e){
        int n;
        cin >> n;

        vs s(n);
        for (auto& i : s)
            cin >> i;

        int q;
        cin >> q;

        for (int i = 0; i < q; ++i){
            int l, r;
            cin >> l >> r;
            --l; --r;

            int sol = 0;
            for (int i = l; i < r; ++i){
                for (int j = i + 1; j <= r; ++j){
                    if (!d[i][j]){
                        d[i][j] = lcp(s[i], s[j]);
                        if (d[i][j] == 0)
                            d[i][j] = -1;
                    }
                    sol = max(sol, d[i][j]);
                }
            }

            cout << sol << '\n';
        }
    }
}

First of all, absurd terseness does not imply lightning speed. Quite the contrary, it makes understanding and thus optimizing that much more difficult.

So, get some useful names.

Anyway, if you want to go for reasonable brevity, or just remove some confusion, remove all the unused typedefs. And then remove the rest of them too, as they also have negative value.

Now, a tip for getting a stunning performance-boost:
Stop copying things, especially if doing so needs dynamic allocation.
That means lcp() should receive its arguments by constant reference.

Next, hopefully your compiler hoists min(b.size(), a.size()) out of the loop. Still, do you really want to rely on that?

Are you sure you need all the headers, and want to live with the unportability?
Read "Why should I not #include <bits/stdc++.h>?".

And also, don't pollute the global namespace with all kinds of things, which you are unlikely to know for now, let alone the future.
See Why is “using namespace std” considered bad practice? for more elaboration on that.


Checking form input for an AJAX request I've made a few functions to check form input for an AJAX request. I am still getting used to JavaScript and some of these AJAX requests. I am looking for any suggestions to better work with JavaScript functions, and passing variables through a function level scoping.

//Form Handler
$("#form-submit").submit(function(e) {
    e.preventDefault();
    $.ajax({
        beforeSend: function(){
            return (checkAll());
            return (passwordCheck());
        },
        complete: function(){
        },
           type: "POST",
           url: '',
           data: $("#form-submit").serialize(),
           success: function(data)
           {
           //window.location = '/dashboard.php';
           // Set Session Php Etc
           alert('php ran');
           return true;
           }
         });
});

//Pushing all Input Values to an Array
function checkAll() {
    var arr = [];
    $('#form-submit :input').each(function() {
        arr.push($(this).val());
    });
    return (checkArray(arr));
}

//Checks Array for empty strings
function checkArray(arr){
    for(var i=0; i < arr.length; i++) {
        console.log(arr[i]);
        if (arr[i].trim() == '') {
            alert('Please Enter All Fields');
            return false;
        } else {
            return true;
        }
    }
}

//Matches Password
function passwordCheck() {
    var pass1 = $('#password').val();
    var pass2 = $('#password-check').val();
    if(pass1 != pass2) {
        $('#password').addClass('highlight');
        $('#password-check').addClass('highlight');
        alert("Passwords don't match");
        return false;
    } else {
        $('#password').removeClass('highlight');
        $('#password-check').removeClass('highlight');
        return true;
    }
}

Bugs

You may think this code works, but it probably doesn't.

Example 1:

beforeSend: function () {
    return (checkAll());
    return (passwordCheck());
},

The passwordCheck function will never be called. Most probably you wanted this:

return checkAll() && passwordCheck();

Example 2:

for (var i = 0; i < arr.length; i++) {
    console.log(arr[i]);
    if (arr[i].trim() == '') {
        alert('Please Enter All Fields');
        return false;
    } else {
        return true;
    }
}

This for statement doesn't loop: it returns after checking the first value in arr, it won't check the others. Most probably you wanted this:

for (var i = 0; i < arr.length; i++) {
    console.log(arr[i]);
    if (arr[i].trim() == '') {
        alert('Please Enter All Fields');
        return false;
    }
}
return true;

Also, a sligthly simpler way to check for blank value:

if (!arr[i].trim()) {

Efficiency

In passwordCheck you lookup $('#password') and $('#password') twice. It would be better to lookup only once and cache in a local variable, for example:

function passwordCheck() {
    var $password = $('#password');
    var $password_check = $('#password-check');
    var pass1 = $password.val();
    var pass2 = $password_check.val();
    if (pass1 != pass2) {
        $password.addClass('highlight');
        $password_check.addClass('highlight');
        alert("Passwords don't match");
        return false;
    } else {
        $password.removeClass('highlight');
        $password_check.removeClass('highlight');
        return true;
    }
}


Better approach to online builder I have a windows forms application which needs customized dll files to be built and for that I am using an online builder using asp.net. I am in no way a asp.net expert not even near a beginner but this is what I have come up with so far:

All the work is done by the Handler:

public void ProcessRequest(HttpContext context)
    {
        var assembly = AssemblyDefinition.ReadAssembly(context.Server.MapPath("D2.dll"));

        var ctor = assembly.MainModule.Types.FirstOrDefault(t => t.FullName.Contains("ClassName") && t.Namespace.Contains("NameSpace"))
            .Methods.FirstOrDefault(m => m.FullName.Contains("ctor"));

        // Update Instructions
        ctor.Body.Instructions[1].Operand = context.Request["A"];
        ctor.Body.Instructions[4].Operand = context.Request["GM"];
        ctor.Body.Instructions[7].Operand = context.Request["GI"];
        ctor.Body.Instructions[10].Operand = context.Request["U"];
        ctor.Body.Instructions[13].Operand = context.Request["CT"];
        ctor.Body.Instructions[16].Operand = context.Request["MT"];
        ctor.Body.Instructions[19].Operand = context.Request["D"];

        // Optimize the Body - Required to allow proper build
        ctor.Body.OptimizeMacros();
        ctor.Body.SimplifyMacros();

        // Write the Assembly to Stream so it can be converted to Byte[]
        var memstream = new MemoryStream();
        assembly.Write(memstream);

        // Clear existing response.
        context.Response.Clear();

        // Write new response.
        context.Response.ContentType = "application/octet-stream";
        context.Response.AddHeader("Content-Disposition", "attachment; filename=Build.dll");
        context.Response.AddHeader("Content-Length", memstream.Length.ToString());
        context.Response.BinaryWrite(memstream.ToArray());
    }

Currently using this approach/model I have a lot of problems mainly debugging and fixing errors.

I want to improve on this but due to my lack of knowledge in web applications I see no clear path to choose. I have looked at SignalR and it seems like a better approach as it can provide features such as Progress Responses from the server. But still I am confused.

This looks good to me. A nice use of space, Everything is very clear and easy to read, just a couple of problems:

Magic Numbers / Strings

You have a few of these dotted about, a couple of examples:

ctor.Body.Instructions[1].Operand = context.Request["A"];

I have no idea what A signifies here, nor why it should be the first instruction's operand. Swap A with a const that gives that string a name describing its purpose. You can probably do the same for 1 in this scenario too, unless A's purpose is only FirstInstructionOperandCode or something similar.

Commenting

Your comments don't add anything to your code. A lot of them just describe what you're doing, which any maintenance programmer can work out by reading the actual code. Instead you should use your comments to describe why you're doing something, particularly if it breaks from the expected norms.

The most egregious example is this:

// Clear existing response.
context.Response.Clear();

Structure

The Write New Response section could be refactored out into a separate method, but unless you're writing these responses elsewhere, it's probably not going to give you any significant gains.


Extracting data from a string I created a method that takes a string value with a fixed length of 9 characters and extracts three values, house (4 characters), level (2), door (3). If the input value has another length than 9 then the method should return nil.

I found several ways to implement the method, but which implementation should I prefer and why?

def extract(value)
  return nil unless value.length == 9
  [value[0..3].to_i, value[4..5].to_i, value[6..8].to_i]
end

require 'scanf'

def extract(value)
  return nil unless value.length == 9
  value.scanf('%4d%2d%3d')
end

def extract(value)
  value.match(/^(\d{4})(\d{2})(\d{3})$/).captures.map(&:to_i) rescue nil
end

p extract('123456789')
#=> [1234, 56, 789]
p extract('123456789012')
#=> nil

My Thoughts

I haven't profiled any of these, but I'm going to assume speed is not a priority.

I like #2 the best.  It is clean and simple, and easy to understand what the code is doing at a glance.

I think #1 is ugly, and the sequential array slicing makes me nervous and seems easy to break.  If your number sizes change at some point in the future, this one would be the hardest to modify.

I think #3 is better, but not as clear or concise as #2.


Adding comments to images I'm working on an interview code sample in Javascript, which is not my first language.  The task is simple enough:  Given a webpage with a variety of images, allow the users to add comments.  The code to generate and manage the images was given to me; my task was to build out comment.js and comments.controller.js.  Everything works and I'm happy with it, but since JS is not my first choice I don't know if there's any glaring errors I'm missing.  Please be as nitpicky as possible.

Most of my code is in the following two files.  The rest can be viewed at https://github.com/paulnicholsen27/flatiron but I do not need feedback on any of those (I didn't write them)--it just may be helpful to run the entire program rather than piecemeal.  I don't know how you brave altrusitic wizards operate most effectively.

comment.js

function Comment(imageId, text) {
    this.id = this.constructor.all.length;
    this.imageId = imageId;
    this.text = text;
    this.constructor.all.push(this);
}

Comment.all = [];

Comment.prototype.buildCommentEl = function() {
    return $(`<li id='comment-${this.id}'>${this.text}</li>`);
}

comments.controller.js

class CommentsController {
  constructor() {
    this.$addCommentForm = $('.add-comment')
  }

  init() {
    $(".add-comment").each((index, element) => {
        element.addEventListener("submit", this.addCommentFormListener);
    })
  }
}

CommentsController.prototype.addCommentFormListener = function(e){
    e.preventDefault();
    var image = $(this).closest(".image");
    var imageId = Number(image.find("ul").attr("data-id"));
    var commentForm = $(this).find('input[name="comment-description"]')
    var commentContents = commentForm.val();
    if (commentContents.length) {
       var newComment = new Comment(imageId, commentContents);
       CommentsController.render(newComment);
       commentForm.val("");  // reset comment form
    }
}

CommentsController.render = function(newComment){
    var imageId = newComment.imageId;
    var image = Image.all.find(image => image.id === Number(imageId));
    image.comments.push(newComment);
    var commentList = $(`#comments-${imageId}`);
    var commentEl = newComment.buildCommentEl();
    commentList.append(commentEl);
}

Your link has an assignment description, which I assume you have to follow, and I'll do so too. It says to use ES6 and class, but you have only used class for one of these, and even then you still use .prototype. You also shouldn't be using var, use let and const instead.

Comment class

Use class syntax. Save the image object instead of the imageId. Make the findImage function. commentEl should return a string, not a jQuery object.

class Comment {
    constructor(imageId, text) {
        this.id = Comment.all.length;
        this.image = this.findImage(imageId);
        this.text = text;
        Comment.all.push(this);
    }

    findImage(imageId) {
        const image =  Image.all.find(image => image.id === Number(imageId));
        image.comments.push(this);
        return image;
    }

    commentEl() {
        return `<li id='comment-${this.id}'>${this.text}</li>`;
    }
}

Comment.all = [];

CommentsController class

Use class syntax. Remove $addCommentForm since it's never used. addCommentFormListener doesn't add a listener, it is a listener (or handler, if you will). init is the one actually adding the listener. render can be simplified, since part of it is being done elsewhere. It is also being used like a static method.

class CommentsController {
    constructor() {
        this.addCommentFormListener();
    }

    addCommentFormListener() {
        $(".add-comment").each((index, element) => {
            element.addEventListener("submit", commentFormHandler);
        });
    }

    static render(commentObject) {
        const commentList = $(`#comments-${commentObject.image.id}`);
        commentList.append(commentObject.commentEl());
    }
}

// This could also be part of the class
function commentFormHandler(e) {
    e.preventDefault();
    const image = $(this).closest(".image");
    const imageId = Number(image.find("ul").attr("data-id"));
    const commentForm = $(this).find('input[name="comment-description"]')
    const commentContents = commentForm.val();
    if (commentContents.length) {
        const newComment = new Comment(imageId, commentContents);
        CommentsController.render(newComment);
        commentForm.val("");
    }
}

I'm not a fan of the way you're getting the imageId and comment. It's tied too much to the DOM and a bit confusing. But that's beyond this question.


Using std::chrono to make custom "SI unit" types std::chrono::duration is a great example of how to create "units" using std::ratio. std::ratio by itself is not very useful for a "units" library, and the two choices are either something like Boost.Unit (which can be very complicated to use) or to write it yourself (why reinvent the wheel?) Therefore I'm trying to write a wrapper around std::chrono::duration for some very basic types, like meter, feet, inches and centimeters.

Right now I feel like the way I've constructed the types is sloppy. It took me a while to get it right (at least for the test cases.)

I also haven't figured out a good way to "rename" the types either. I've considered creating a templated struct that inherits from duration and then just renames the typedefs.

Another concern I have is whether I should use double instead of float and if there's a better way to avoid floating point errors.

#include <iostream>
#include <chrono>
#include <ratio>
#include <cmath>  /* for std::abs(double) */

// https://isocpp.org/wiki/faq/newbie#floating-point-arith
// "Why doesn’t my floating-point comparison work?"
constexpr inline bool isEqual(double x, double y)
{
  const double epsilon = 1e-5;
  return std::abs(x - y) <= epsilon * std::abs(x);
  // see Knuth section 4.2.2 pages 217-218
}

int main()
{
    using meters = std::chrono::seconds;

    // There are 100 centimeters per meter
    using centimeters = std::chrono::duration<float, std::ratio_divide<meters::period, std::ratio<100>>>;

    // There are 2.54 centimeters per inch
    // and 39.3701 inches per meter
    using inches = std::chrono::duration<float, std::ratio_divide<centimeters::period, std::ratio<100, 254>>>;

    // There are 12 inches per foot
    using feet = std::chrono::duration<float, std::ratio_multiply<std::ratio<12>, inches::period>>;

    static_assert(inches(1) * 12 == feet(1), "");
    static_assert(centimeters(2.54) == inches(1), "");

    /* Thanks to rounding, inches(1) / 2.54 is 0.393701.
     * To get 0.3937007874, we need to use inches(1 / 2.54)
     * instead.
     */
    static_assert(inches(1 / 2.54) == centimeters(1), "");
    static_assert(centimeters(100) == meters(1), "");

    static_assert(inches(centimeters(100)) == meters(1), "");
    static_assert(isEqual(inches(centimeters(100)).count(), 39.3701), "");
}

Meters are Not Seconds

using meters = std::chrono::seconds;

I know it's obvious, but it bears stating. A meter is not a second. You compare a meter and a second, you cannot add them, etc. They are different types completely. So what you want is to make a different type:

template <typename T, class Ratio = std::ratio<1>>
class distance;

using meters = distance<int64_t>;

I'll leave the implementation of distance up to you. But it should absolutely be a different class than seconds!

Use the Standard Ratios

<ratio> actually provides a bunch of standard ratios for your use. You should use them:

using centimeters = distance<int64_t, std::centi>;

You'll note that std::centi is std::ratio<1, 100>, which makes more sense for centimeters than std::ratio<100>.

Use integral types

You'll notice I used int64_t instead of float or double. Prefer using integers whenever possible to simply sidestep the issue of floating point arithmetic altogether.


Assign Names to Phone Numbers using text file Assign all names to a phone number. Nothing much else to explain. I think there are a lot of improvements that can be done with this code. Not really sure what though.

here is the Name/Phone Number List

phone = open("Phone.txt", "r");

name_list = []
phone_list = []

append_data = "name"

iterate_first = True
for data in phone.readlines():
    if (data == '\n'):
        append_data = "phone"

    if (append_data == "name"):
        name_list.append(data.strip())
    else:
        if (iterate_first == False):
            phone_list.append(data.strip())
        else:
            iterate_first = False

for data in name_list:
    name_index = name_list.index(data)
    print("%s is associated with the phone number %s" % (data, phone_list[name_index]))

Use with when opening files. It automatically closes the file, which you didn't do. And allows readers to easily know the scope of the file.
You can loop through a file, so you don't need to use readlines.
Rather than using iterate_first you can use continue. This is as you skip the line which is empty. It also makes your code simpler, as it explicitly says what that variable does.
Rather than using name_list.index(data), you could instead use enumerate, or even better, zip.
Rather than using %s formatting, you can use str.format. This has a simpler interface, and is the same style as f-strings, which are the new best way to format strings. However f-strings are Python 3.6+.
Your names are mostly good, name_list, but using plurals may be better, names.

Using all the above can allow you to get:

with open("Phone.txt", "r") as phone:
    names = []
    numbers = []
    append_data = "name"
    for data in phone:
        data = data.strip()
        if (data == ''):
            append_data = "phone"
            continue

        if (append_data == "name"):
            names.append(data)
        else:
            numbers.append(data)

for name, number in zip(names, numbers):
    print("{} is associated with the phone number {}".format(name, number))

The above is good, however, I'd personally use itertools.takewhile, and a function. You may find the code harder to read, however I don't so you may not want to use the following.
And so I'd write:

from itertools import takewhile

def read_phone_file(phone_file):
    with open(phone_file, "r") as f:
        lines = (item.strip() for item in f)
        return list(takewhile(bool, lines)), list(lines)

names, numbers = read_phone_file("Phone.txt")
for name, number in zip(names, numbers):
    print("{} is associated with the phone number {}".format(name, number))


Performance Byte[] to Generic I'm in need to make a byte[] -> T extension method and need it to be fast (no need for it being pretty)

This function will be called 100's of 1000's of times in very short succession in an absolute performance critical environment.

We're currently optimizing on "ticks" level, every tick translates to a couple milliseconds higher in the callstack, thus the need of raw speed over maintainability (not how I like to design software, but the reasoning behind this is out of scope).

Consider the following code, it's clean and maintainable, but it's relatively slow (probably due to boxing and unboxing), Can this be optimized to be faster?

public static T ConvertTo<T>(this byte[] bytes, int offset = 0)
{
    var type = typeof(T);
    if (type == typeof(sbyte)) return bytes[offset].As<T>();
    if (type == typeof(byte)) return bytes[offset].As<T>();
    if (type == typeof(short)) return BitConverter.ToInt16(bytes, offset).As<T>();
    if (type == typeof(ushort)) return BitConverter.ToUInt32(bytes, offset).As<T>();
    if (type == typeof(int)) return BitConverter.ToInt32(bytes, offset).As<T>();
    if (type == typeof(uint)) return BitConverter.ToUInt32(bytes, offset).As<T>();
    if (type == typeof(long)) return BitConverter.ToInt64(bytes, offset).As<T>();
    if (type == typeof(ulong)) return BitConverter.ToUInt64(bytes, offset).As<T>();

    throw new NotImplementedException();
}

public static T As<T>(this object o)
{
    return (T)o;
}

And yes it needs to be generic, sadly

First, you seem to have a minor typo on this line:

if (type == typeof(ushort)) return BitConverter.ToUInt32(bytes, offset).As<T>();

That should be:

if (type == typeof(ushort)) return BitConverter.ToUInt16(bytes, offset).As<T>();

Just as well, you have a bug in here that is a pretty big one (causes exception on any attempts to convert anything to sbyte with your method):

if (type == typeof(sbyte)) return bytes[offset].As<T>();

Should be:

if (type == typeof(sbyte)) return ((sbyte)bytes[offset]).As<T>();

If you really need speed, you should probably not use the BitConverter class in this situation. Use bitwise operators, as they are much faster.

See this answer for a comparison.

You should also not use .As<T> either, and instead cast within the method using: (T)(object). This eliminates unnecessary stack overhead.

Two improvements we can make immediately:

Replace all the BitConverter work with bitwise work.
Replace all the .As<T>() with (T)(object) casts instead.

Performance Evaluation

The performance difference, when those changes are made is significant for certain types of T.

ConvertTo1<byte> on 50000000 rounds, time (in ms) taken: 3465
ConvertTo1<short> on 50000000 rounds, time (in ms) taken: 4217
ConvertTo1<int> on 50000000 rounds, time (in ms) taken: 5586
ConvertTo1<long> on 50000000 rounds, time (in ms) taken: 7665
ConvertTo2<byte> on 50000000 rounds, time (in ms) taken: 4995
ConvertTo2<short> on 50000000 rounds, time (in ms) taken: 5775
ConvertTo2<int> on 50000000 rounds, time (in ms) taken: 6945
ConvertTo2<long> on 50000000 rounds, time (in ms) taken: 8492
ConvertToInt on 50000000 rounds, time (in ms) taken: 1092

Verifying results of conversions are same:
SByte test: 1, 1: True
Byte test: 1, 1: True
Short test: 10497, 10497: True
UShort test: 10497, 10497: True
Int test: 755378433, 755378433: True
UInt test: 755378433, 755378433: True
Long test: 4041804345027995905, 4041804345027995905: True
ULong test: 4041804345027995905, 4041804345027995905: True

Now, to explain, ConvertTo1<T> is the optimized method, ConvertTo2<T> is the original, and ConvertToInt is a strongly-typed method for converting directly to int.

By removing .As<T>() and replacing with (T)(object), and replacing all the BitConverter work with bitwise work, we cut our work down to:

69% original time for byte
73% original time for short
80% original time for int
90% original time for long

Also note that we returned the exact same values for all situations.

Just how bad is performance dragging because of the boxing?

Badly. If we consider another method, ConvertTo3 which is strongly typed around an int parameter, we can come up with the following result:

ConvertToInt on 50000000 rounds, time (in ms) taken: 1034

Now that is with a method that only returns an int value.

This leads us to be able to conclude that, due to the boxing of the generic types, and recasting them, we lose a lot of performance. (Around 500% or so right-off-the-bat.) The issues isn't the fact that you use generics, the issue is that you use boxing several times within, which creates extra overhead.

Using an .As<T>() method

While it would make sense to think that this would be fairly quick, with no drawbacks, this is actually much slower than inline calls due to the overhead of having an additional method call. Replacing .As<T>() with (T)(object) right out of the gate gave us significant performance back. It may even be acceptable to only make that change and continue using BitConverter.

In fact, upon further research, it seems that if you replace the ConvertTo2 method I supplied with this variant:

    public static T ConvertTo2<T>(this byte[] bytes, int offset = 0)
    {
        var type = typeof(T);
        if (type == typeof(sbyte)) return (T)(object)((sbyte)bytes[offset]);
        if (type == typeof(byte)) return (T)(object)bytes[offset];
        if (type == typeof(short)) return (T)(object)BitConverter.ToInt16(bytes, offset);
        if (type == typeof(ushort)) return (T)(object)BitConverter.ToUInt16(bytes, offset);
        if (type == typeof(int)) return (T)(object)BitConverter.ToInt32(bytes, offset);
        if (type == typeof(uint)) return (T)(object)BitConverter.ToUInt32(bytes, offset);
        if (type == typeof(long)) return (T)(object)BitConverter.ToInt64(bytes, offset);
        if (type == typeof(ulong)) return (T)(object)BitConverter.ToUInt64(bytes, offset);

        throw new NotImplementedException();
    }

The performance difference between ConvertTo1 and ConvertTo2 is very minimal. (Though, removing the BitConverter work still does seem to make it slightly faster, the difference is very insignificant at this level.)

Food for thought.

Code

Here's the code I used for conversions:

public static class Extensions
{
    public static T ConvertTo1<T>(this byte[] bytes, int offset = 0)
    {
        var type = typeof(T);
        if (type == typeof(sbyte)) return (T)(object)((sbyte)bytes[offset]);
        if (type == typeof(byte)) return (T)(object)bytes[offset];
        if (type == typeof(short)) return (T)(object)((short)(bytes[offset + 1] << 8 | bytes[offset]));
        if (type == typeof(ushort)) return (T)(object)((ushort)(bytes[offset + 1] << 8 | bytes[offset]));
        if (type == typeof(int)) return (T)(object)(bytes[offset + 3] << 24 | bytes[offset + 2] << 16 | bytes[offset + 1] << 8 | bytes[offset]);
        if (type == typeof(uint)) return (T)(object)((uint)bytes[offset + 3] << 24 | (uint)bytes[offset + 2] << 16 | (uint)bytes[offset + 1] << 8 | bytes[offset]);
        if (type == typeof(long)) return (T)(object)((long)bytes[offset + 7] << 56 | (long)bytes[offset + 6] << 48 | (long)bytes[offset + 5] << 40 | (long)bytes[offset + 4] << 32 | (long)bytes[offset + 3] << 24 | (long)bytes[offset + 2] << 16 | (long)bytes[offset + 1] << 8 | bytes[offset]);
        if (type == typeof(ulong)) return (T)(object)((ulong)bytes[offset + 7] << 56 | (ulong)bytes[offset + 6] << 48 | (ulong)bytes[offset + 5] << 40 | (ulong)bytes[offset + 4] << 32 | (ulong)bytes[offset + 3] << 24 | (ulong)bytes[offset + 2] << 16 | (ulong)bytes[offset + 1] << 8 | bytes[offset]);

        throw new NotImplementedException();
    }

    public static T ConvertTo2<T>(this byte[] bytes, int offset = 0)
    {
        var type = typeof(T);
        if (type == typeof(sbyte)) return ((sbyte)bytes[offset]).As<T>();
        if (type == typeof(byte)) return bytes[offset].As<T>();
        if (type == typeof(short)) return BitConverter.ToInt16(bytes, offset).As<T>();
        if (type == typeof(ushort)) return BitConverter.ToUInt16(bytes, offset).As<T>();
        if (type == typeof(int)) return BitConverter.ToInt32(bytes, offset).As<T>();
        if (type == typeof(uint)) return BitConverter.ToUInt32(bytes, offset).As<T>();
        if (type == typeof(long)) return BitConverter.ToInt64(bytes, offset).As<T>();
        if (type == typeof(ulong)) return BitConverter.ToUInt64(bytes, offset).As<T>();

        throw new NotImplementedException();
    }

    public static int ConvertToInt(this byte[] bytes, int offset = 0)
    {
        return (bytes[offset + 3] << 24 | bytes[offset + 2] << 16 | bytes[offset + 1] << 8 | bytes[offset]);
    }

    public static T As<T>(this object o)
    {
        return (T)o;
    }
}

And the test code:

public class CR_101636
{
    public static void _Main(string[] args)
    {
        var rounds = 50000000;
        var bytes = new byte[] { 1, 41, 6, 45, 163, 95, 23, 56 };

        Stopwatch sw = new Stopwatch();
        sw.Start();
        for (int i = 0; i < rounds; i++)
        {
            byte result = bytes.ConvertTo1<byte>(0);
        }
        sw.Stop();
        Console.WriteLine("ConvertTo1<byte> on {0} rounds, time (in ms) taken: {1}", rounds, sw.ElapsedMilliseconds);
        sw.Reset();

        sw.Start();
        for (int i = 0; i < rounds; i++)
        {
            short result = bytes.ConvertTo1<short>(0);
        }
        sw.Stop();
        Console.WriteLine("ConvertTo1<short> on {0} rounds, time (in ms) taken: {1}", rounds, sw.ElapsedMilliseconds);
        sw.Reset();

        sw.Start();
        for (int i = 0; i < rounds; i++)
        {
            int result = bytes.ConvertTo1<int>(0);
        }
        sw.Stop();
        Console.WriteLine("ConvertTo1<int> on {0} rounds, time (in ms) taken: {1}", rounds, sw.ElapsedMilliseconds);
        sw.Reset();

        sw.Start();
        for (int i = 0; i < rounds; i++)
        {
            long result = bytes.ConvertTo1<long>(0);
        }
        sw.Stop();
        Console.WriteLine("ConvertTo1<long> on {0} rounds, time (in ms) taken: {1}", rounds, sw.ElapsedMilliseconds);
        sw.Reset();

        sw.Start();
        for (int i = 0; i < rounds; i++)
        {
            byte result = bytes.ConvertTo2<byte>(0);
        }
        sw.Stop();
        Console.WriteLine("ConvertTo2<byte> on {0} rounds, time (in ms) taken: {1}", rounds, sw.ElapsedMilliseconds);
        sw.Reset();

        sw.Start();
        for (int i = 0; i < rounds; i++)
        {
            short result = bytes.ConvertTo2<short>(0);
        }
        sw.Stop();
        Console.WriteLine("ConvertTo2<short> on {0} rounds, time (in ms) taken: {1}", rounds, sw.ElapsedMilliseconds);
        sw.Reset();

        sw.Start();
        for (int i = 0; i < rounds; i++)
        {
            int result = bytes.ConvertTo2<int>(0);
        }
        sw.Stop();
        Console.WriteLine("ConvertTo2<int> on {0} rounds, time (in ms) taken: {1}", rounds, sw.ElapsedMilliseconds);
        sw.Reset();

        sw.Start();
        for (int i = 0; i < rounds; i++)
        {
            long result = bytes.ConvertTo2<long>(0);
        }
        sw.Stop();
        Console.WriteLine("ConvertTo2<long> on {0} rounds, time (in ms) taken: {1}", rounds, sw.ElapsedMilliseconds);
        sw.Reset();

        sw.Start();
        for (int i = 0; i < rounds; i++)
        {
            int result = bytes.ConvertToInt(0);
        }
        sw.Stop();
        Console.WriteLine("ConvertToInt on {0} rounds, time (in ms) taken: {1}", rounds, sw.ElapsedMilliseconds);
        sw.Reset();

        Console.WriteLine("");
        Console.WriteLine("Verifying results of conversions are same:");

        Console.WriteLine("SByte test: {0}, {1}: {2}", bytes.ConvertTo1<sbyte>(), bytes.ConvertTo2<sbyte>(), bytes.ConvertTo1<sbyte>() == bytes.ConvertTo2<sbyte>());
        Console.WriteLine("Byte test: {0}, {1}: {2}", bytes.ConvertTo1<byte>(), bytes.ConvertTo2<byte>(), bytes.ConvertTo1<byte>() == bytes.ConvertTo2<byte>());
        Console.WriteLine("Short test: {0}, {1}: {2}", bytes.ConvertTo1<short>(), bytes.ConvertTo2<short>(), bytes.ConvertTo1<short>() == bytes.ConvertTo2<short>());
        Console.WriteLine("UShort test: {0}, {1}: {2}", bytes.ConvertTo1<ushort>(), bytes.ConvertTo2<ushort>(), bytes.ConvertTo1<ushort>() == bytes.ConvertTo2<ushort>());
        Console.WriteLine("Int test: {0}, {1}: {2}", bytes.ConvertTo1<int>(), bytes.ConvertTo2<int>(), bytes.ConvertTo1<int>() == bytes.ConvertTo2<int>());
        Console.WriteLine("UInt test: {0}, {1}: {2}", bytes.ConvertTo1<uint>(), bytes.ConvertTo2<uint>(), bytes.ConvertTo1<uint>() == bytes.ConvertTo2<uint>());
        Console.WriteLine("Long test: {0}, {1}: {2}", bytes.ConvertTo1<long>(), bytes.ConvertTo2<long>(), bytes.ConvertTo1<long>() == bytes.ConvertTo2<long>());
        Console.WriteLine("ULong test: {0}, {1}: {2}", bytes.ConvertTo1<ulong>(), bytes.ConvertTo2<ulong>(), bytes.ConvertTo1<ulong>() == bytes.ConvertTo2<ulong>());
    }
}

In your Program (or wherever):

CR_101636._Main(args);


Rolling loop with an array of data points I have a NumPy array of about 2500 data points. This function is called on a rolling basis where 363 data points is passed at a time.

def fcn(data):
    a = [data[i]/np.mean(data[i-2:i+1])-1 for i in range(len(data)-1, len(data)-362, -1)]
    return a

This takes about 5 seconds to run. I think the bottleneck is the list slicing. Any thoughts on how to speed this up?

range returns a list.  You could use xrange instead.

range([start,] stop[, step]) -> list of integers

  Return a list containing an arithmetic progression of integers.

vs

xrange([start,] stop[, step]) -> xrange object

  Like range(), but instead of returning a list, returns an object that
  generates the numbers in the range on demand.  For looping, this is
  slightly faster than range() and more memory efficient.

The other thing that strikes me is the slice in the argument to np.mean.  The slice is always of length 3.  Assuming this is an arithmetic mean, you could turn the division into

(3.0 * data[i] / (data[i - 2] + data[i - 1] + data[i]))

So putting it together

def fcn(data):
    return [(3.0 * data[i] / (data[i - 2] + data[i - 1] + data[i])) - 1
            for i in xrange(len(data) - 1, len(data) - 362, -1)]

and you could further optimize the sum of last three values by recognizing that if

x = a[n] + a[n+1] + a[n+2]

and you have already computed

y = a[n - 1] + a[n] + a[n + 1]

then

x = y + (a[n - 1] - a[n + 2])

which helps whenever a local variable access and assignment is faster than accessing an element in a series.


Applying scalar multiplication to matrix This is one of my first programs in C. Please review and help me improve it. I create a matrix and apply scalar multiplication on it. Then I print the result.

#include <stdio.h>

int main(void)
{
    void scalarMultiply(int matrix[3][5], int scalar);
    void displayMatrix (int matrix[3][5]);

    int sampleMatrix[3][5] = {
        7,16,55,13,12,12,10,52,0,7,-2,1,2,4,9
    };

    printf("Original matrix:\n");
    displayMatrix(sampleMatrix);

    scalarMultiply (sampleMatrix, 2);
    printf ("\nMultiplied by 2:\n");
    displayMatrix (sampleMatrix);
    scalarMultiply (sampleMatrix, -1);
    printf ("\nThen multiplied by -1:\n");
    displayMatrix (sampleMatrix);
    return 0;

}

// Function to multiply a 3 x 5 array by a scalar
void scalarMultiply (int matrix[3][5], int scalar)
{
    int row, column;
    for ( row = 0; row < 3; ++row )
        for ( column = 0; column < 5; ++column )
            matrix[row][column] *= scalar;
}
void displayMatrix (int matrix[][5])
{
    int row, column;
    for ( row = 0; row < 3; ++row) {
        for ( column = 0; column < 5; ++column )
            printf ("  %i", matrix[row][column]);
        printf ("\n");
    }
}

Braces aren't required as initialization will proceed by row.
However it is much easier to read if you initialized your
two-dimensional array like this:

int sampleMatrix[3][5] = {
    {7,16,55,13,12},
    {12,10,52,0,7},
    {-2,1,2,4,9}
}

Also note, it is not required that the entire array be initialized.
If you use inner pairs of braces you can force initialization to
whatever you want. Unspecified values are set to zero by default.

int sampleMatrix[3][5] = {
    {7,16,55,13},  //Last element in each row is zero ; test it yourself
    {12,10,52,0},
    {-2,1,2,4}
}

Use variables to represent the number of rows and columns for the
matrices. Magic numbers are bad. This will also allow you to pass
matrices of different dimensions.

void scalarMultiply(int nRows, int nCols, int matrix[nRows][nCols], int scalar);
void displayMatrix (int nRows, int nCols, int matrix[nRows][nCols]);

It is best practice to keep your function prototypes outside of
main(). Keeping it in main() makes it harder to find.
The outout layout looks a bit messy. Change your printf statement
to include a field width specifier.

     printf ("%5i", matrix[row][column]);

%5i tells printf to display integers and their sign that take up 5 columns.
Use proper spacing:

    scalarMultiply (sampleMatrix, 2);

    printf ("\nMultiplied by 2:\n");
    displayMatrix (sampleMatrix);

    scalarMultiply (sampleMatrix, -1);

    printf ("\nThen multiplied by -1:\n");  //Keep related functions together
    displayMatrix (sampleMatrix);


Targeting only 1 event in DOM In the picture below, I want to open a modal when a user clicks anywhere within in the yellow block, including if they click on the white ‘+’ sign. The problem is, the click event happens on a different target depending on whether the user clicks on the yellow portion, or on the white ‘+’ sign.

In my JavaScript code, I have been handing it like this so far, but I wonder if there is a better, shorter way of doing this that doesn’t require the II operator:

function openModal(e) {
  if (e.target.classList.contains('project-box__picture-box') || e.target.classList.contains('project-box__icon')) {
    modalEl.classList.add('open');
  }
}

I can give you two ways to approach this: via JavaScript or CSS.
JavaScript
You can use the .closest() method of the HTMLElement interface. You need to pass it a CSS selector, and it will find you the closest DOM ancestor of the element that matches the given selector, or null if no such ancestor was found.
In this case, you could use it like this:
function openModal(e) {
  if (e.target.closest('.project-box__picture-box')) {
    modalEl.classList.add('open');
  }
}

(full method reference here)
CSS
The other option is to use the pointer-events CSS property. If you set this to none on all elements inside the clickable box, then those elements will not be allowed to be the targets of mouse events; instead, the target will be the closest ancestor that has pointer events enabled.
In this case, you'd need to add this CSS:
.project-box__picture-box * {
  pointer-events: none;
}

And this JS:
function openModal(e) {
  if (e.target.classList.contains('project-box__picture-box')) {
    modalEl.classList.add('open');
  }
}

(full property reference here)


Finding the most common DNA patterns of some given length I'm working on an algorithm to search through 100,000+ lines of dna sequencing, with the fastest time possible. Here is my current code, I was wondering if there's any ways to make this run faster:

public static void mostCommonKmer(String s, int k) {
    // your code here
    HashMap<String, Integer> m = new HashMap<>();
    int l = 0, g;
    String n = "", ss;
    for (int i = 0; i < s.length() - k; i++) {
        ss = s.substring(i, i+k);
        g = m.getOrDefault(ss, 1);
        m.put(ss, g + 1);
        if (l < g) {
            l = g;
            n = ss;
        }
    }
    System.out.println("Most Frequent " + k + "-mer = " + n);
    System.out.println("frequency = " + l);
}

s is the 100,000+ character string passed to the method and k is the length of pattern to look for

To be clear, it runs fast (0.08s for 100,000 chars), but I want to see how fast I can get it to go.

You can optimize a bit by pre-calculating s.length() - k

for (int i = 0, max = s.length() - k; i < max; i++) {
    ...
}

Furthermore, you can cut a bit of execution time by replacing the auto boxing and unboxing you do by using a mutable value class, something like this:

private static class MutableInt {
    public int value;

    public MutableInt(int value) {
        this.value = value;
    }
}

public static void mostCommonKmer3(String s, int k) {
    HashMap<String, MutableInt> m = new HashMap<>();
    int l = 0;
    MutableInt g;
    String n = "", ss;
    for (int i = 0, max = s.length() - k; i < max; i++) {
        ss = s.substring(i, i+k);

        // note: computeIfAbsent over getOrDefault in this case, so that
        // creation will only take place when necessary
        g = m.computeIfAbsent(ss, key -> new MutableInt(1));
        g.value++;
        if (l < g.value) {
            l = g.value;
            n = ss;
        }
    }
    //System.out.println("Most Frequent " + k + "-mer = " + n);
    //System.out.println("frequency = " + l);
}

Raw execution time for 1000 repetitions (with proper ramp-up) using 100000 chars and k = 150 went from 3308 ms for the original code to 3190 using both changes on my machine.

And as you are on codereview, one additional remark: longer variable names (longest, subsequence, n?) do not hurt execution time, but have the potential to make the code more readable.... ;-)


Eager load additional data about some instructors Below is a snippet of code of a little practice project. I am still pretty new to EF and I was wondering if there is a way to write the below statement so that all required data is eager loaded since I have the ID parameter.

public ActionResult Index(int? id)
    {
        var viewModel = new InstructorIndexData
        {
            Instructors = _db.Instructors
                .Include(i => i.OfficeAssignment)
                .Include(i => i.Courses.Select(c => c.Department))
                .OrderBy(i => i.LastName)
        };

        if (id != null)
        {
            ViewBag.InstructorId = id.Value;
            viewModel.Courses = viewModel.Instructors.Single(i => i.InstructorId == id.Value).Courses;
        }

        return View(viewModel);
    }

When this action runs it grabs the instructor data and includes a few other collections because they are required for the initial view of the page. After selecting an instructor the page comes back with the instructor and the course the instructor is assigned to but this requires two calls to the database. Is there a way I can simple add the course to the initial query before it goes out to pull the instructor since that doesnt occur till the return?

The query to the database wont't be executed by writing this linq statement

var viewModel = new InstructorIndexData
{
    Instructors = _db.Instructors
        .Include(i => i.OfficeAssignment)
        .Include(i => i.Courses.Select(c => c.Department))
        .OrderBy(i => i.LastName)
};

if the InstructorIndexData.Instructors property setter isn't executing any of the following actions

enumerating the elements by a for..each loop
calling ToArray(), ToDictionary() or ToList() on the IEnumerable
calling a linq operator like First(), Any

See: https://msdn.microsoft.com/en-us/data/jj573936.aspx

So, if you call ToList() on the enumerable shown above, only one call to the database will be made.

 var viewModel = new InstructorIndexData
 {
     Instructors = _db.Instructors
         .Include(i => i.OfficeAssignment)
         .Include(i => i.Courses.Select(c => c.Department))
         .OrderBy(i => i.LastName)
         .ToList()
 };

The call to

viewModel.Courses = viewModel.Instructors.Single(i => i.InstructorId == id.Value).Courses;

will then operate on the retrieved data and won't need a trip to the database anymore.


Iterating lists and updating dictionary for corresponding match logic improvement I have two lists:

Users - [<UserObject1>, <UserObject2>, ...]
Contributions - [<ContributionObject1>, <ContributionObject2>, ...]

Every ContributionObject further can have single or multiple UserObject in it which are mentioned as person_links here and both the objects that are ContributionObject and UserObject has certain methods and attributes.

UserObject has an attribute affiliation.

I have to check whether UserObject from Users and have the same affiliation to one of the UserObject from ContributionObject from Contributions.

If yes, I have to make a dictionary where key will be the user from Users and value will be an array of ceratin ContributionObject attributes that is title and url.

I am able to do it with the following logic.

I wanted to ask if this logic can be improved further?

If there is another efficient way to do this task, do mention that. Thanks for all the help. :)

conflicts = dict()
for user in users:
    if user.affiliation:
        for contribution in contributions:
            if not conflicts.get(user):
                conflicts[user] = [
                    (
                        contribution.title,
                        contribution.url,
                    )
                    for person in contribution.person_links
                    if user.affiliation in person.affiliation
                ]
            else:
                conflicts[user] += [
                    (
                        contribution.title,
                        contribution.url,
                    )
                    for person in contribution.person_links
                    if user.affiliation in person.affiliation
                ]

I tried to find out better ways to update dict values on SO but they were mostly about updating the existing values(overriding) not about adding(appending) to an existing value.

Considering DRY principal, this

if not conflicts.get(user):
    conflicts[user] = [
                (
                    contribution.title,
                    contribution.url,
                )
                for person in contribution.person_links
                if user.affiliation in person.affiliation
            ]
else:
    conflicts[user] += [
                (
                    contribution.title,
                    contribution.url,
                )
                for person in contribution.person_links
                if user.affiliation in person.affiliation
            ]

can and should be replaced with a defaultdict:

from collections import defaultdict

conflicts = defaultdict(list)
...
for contribution in contributions:
    conflicts[user].extend((contribution.title, contribution.url)
                           for person in contribution.person_links
                           if user.affiliation in person.affiliation)

Or at least take advantage of dict.setdefault (while preferring a dict literal over a call to dict to save method look-up time):

conflicts = {}
...
for contribution in contributions:
    conflicts.setdefault(user, []).extend((contribution.title, contribution.url)
                                          for person in contribution.person_links
                                          if user.affiliation in person.affiliation)


jQuery plugin for easy CSS3 transformation I wrote it more for educational reasons and less as something that can compete with existing alternatives.

But I enjoyed writing it and wish to get some feedback, JavaScript is my second language :)

(I hope the main issue is lack of comments, but I wonder if there is anything else hairy about the code)

Here is the main plugin code:

(function ($) {
  'use strict';

  /*
   * Base function for all transformations
   */
  function base(property, transformer, params) {

    var value = $.map(params, function (item, index) {
      return transformer.call(this, item);
    }).join(", ");

    return this.each(function () {
      var $this = $(this);
      var existingTransformation = $this.css("transform");
      if (existingTransformation === 'none') {
        existingTransformation = '';
      }
      if (existingTransformation !== '') {
        existingTransformation += ' ';
      }
      $this.css({
        transform: existingTransformation + property + "(" + value + ")"
      });
    });
  };

  /*
   * Numeric to string conversion internal helper functions
   */

  function suffixNumber(param, suffix){
    if (typeof param === "number") {
      param += suffix;
    }
    return param;
  };
  function toDeg(param){
    return suffixNumber(param, "deg");
  }

  function toPx(param){
    return suffixNumber(param, "px");
  }

  function toPc(param){
    return suffixNumber(param, "%");
  }

  function toStr(param){
    return suffixNumber(param, "");
  }

 /*
  * The plugin's outer world functions
  */

  var functionParamTypes = {
    number:toStr,
    length:toPx,
    angle:toDeg
  }

  var functionNames = [
    {
      "name":"matrix",
      "type":functionParamTypes.length
    },
    {
      "name":"translate",
      "type": functionParamTypes.length
    },
    {
      "name":"translateX",
      "type": functionParamTypes.length
    },
    {
      "name":"translateY",
      "type": functionParamTypes.length
    },
    {
      "name":"scale",
      "type": functionParamTypes.number
    },
    {
      "name":"scaleX",
      "type": functionParamTypes.number
    },
    {
      "name":"scaleY",
      "type": functionParamTypes.number
    },
    {
      "name":"rotate",
      "type": functionParamTypes.angle
    },
    {
      "name":"skew",
      "type": functionParamTypes.angle
    },
    {
      "name":"skewX",
      "type": functionParamTypes.angle
    },
    {
      "name":"skewY",
      "type": functionParamTypes.angle
    },
    {
      "name":"matrix3d",
      "type": functionParamTypes.length
    },
    {
      "name":"translate3d",
      "type": functionParamTypes.length
    },
    {
      "name":"translateZ",
      "type": functionParamTypes.length
    },
    {
      "name":"scale3d",
      "type": functionParamTypes.length
    },
    {
      "name":"scaleZ",
      "type": functionParamTypes.length
    },
    {
      "name":"rotate3d",
      "type": functionParamTypes.angle
    },
    {
      "name":"rotateX",
      "type": functionParamTypes.angle
    },
    {
      "name":"rotateY",
      "type": functionParamTypes.angle
    },
    {
      "name":"rotateZ",
      "type": functionParamTypes.angle
    },
    {
      "name":"perspective",
      "type": functionParamTypes.angle
    }
  ];

  $.each(functionNames,function(index, item){
    var name = item.name, type = item.type;
    $.fn[name] = function () {
      return base.call(this, name, type, arguments);
    }
  });

})(jQuery);

Latest version can be found here
Live demo can be found here
Usage example can be found here

I like the demo, from a once over:

the functionNames make my eyes glaze over, data tables variables should be sorted and tabbed in my mind:

  var functionNames = [
    { name: "matrix",      type: functionParamTypes.length},
    { name: "matrix3d",    type: functionParamTypes.length},
    { name: "perspective", type: functionParamTypes.angle },
    { name: "rotate",      type: functionParamTypes.angle },
    { name: "rotate3d",    type: functionParamTypes.angle },
    { name: "rotateX",     type: functionParamTypes.angle },
    { name: "rotateY",     type: functionParamTypes.angle },
    { name: "rotateZ",     type: functionParamTypes.angle },
    { name: "scale",       type: functionParamTypes.number},
    { name: "scale3d",     type: functionParamTypes.length},
    { name: "scaleX",      type: functionParamTypes.number},
    { name: "scaleY",      type: functionParamTypes.number},
    { name: "scaleZ",      type: functionParamTypes.length},
    { name: "skew",        type: functionParamTypes.angle },
    { name: "skewX",       type: functionParamTypes.angle },
    { name: "skewY",       type: functionParamTypes.angle },
    { name: "translate",   type: functionParamTypes.length},
    { name: "translate3d", type: functionParamTypes.length},
    { name: "translateX",  type: functionParamTypes.length},
    { name: "translateY",  type: functionParamTypes.length},
    { name: "translateZ",  type: functionParamTypes.length}
  ];

It is preferred to have 1 single comma-separated var statement:

var $this = $(this),
    existingTransformation = $this.css("transform");

It is preferred to have either all single or all double quoted strings, I would suggest single quoted strings
You have some missing and some unnecessary semicolons, please check out http://jshint.com/
Other than that, a fun piece of code


Custom ArrayList class Please review my custom ArrayList class I constructed from scratch and add any suggestions or constructive criticisms. The point of this template class is give the user a lightweight ArrayList with optimal control over how it operates.

It is automatically created on the heap with an optional seed value which determines how much space to give it upon being recreated after filling up. It comes with a refine function which cuts off the garbage values at any time and it can also be reseeded. Seed default is 10 new array spots for each recreation.

#ifndef MEMORYLIST_H
#define MEMORYLIST_H

//Class is designed to be a basic array list off the stack
template <class Heap> class MemoryList {
private:
    Heap *array;    //ArrayList on Heap
    int length;     //Max Length of ArrayList
    int counter;    //Current Position on ArrayList
    int seed;       //amount to add by when limit is breached

    //Increase list if needed
    inline void spaceCheck() {
        if (counter == length) {
            int i;

            Heap *temp = new Heap[length];

            for (i = 0; i < length; ++i) {
                temp[i] = array[i];
            }

            delete [] array;

            length += seed;
            array = new Heap[length];

            for (i = 0; i < length - 10; ++i) {
                array[i] = temp[i];
            }

            delete[] temp;
        }
    }

    inline void shiftRight(int place) {
        for (int i = length - 1; i >= place + 1; --i) {
            array[i] = array[i - 1];
        }
    }

public:
    MemoryList(int seed = 10) {
        this->seed = seed;
        length = seed;
        counter = 0;
        array = new Heap[length];
    }

    //just shows what value is stored in what position
    Heap showValue(int position) {
        return array[position];
    }

    //tack a value to the end
    void newValue(Heap variable){
        spaceCheck();

        array[counter] = variable;
        ++counter;
    }

    //tack a value anywhere you want and array will self adjust
    void insertValue(int position, Heap variable) {
        spaceCheck();
        shiftRight(position);

        array[position] = variable;
        ++counter;
    }

    //delete any value you'd like and array will self adjust
    void deleteValue(int position) {
        for (int i = position; i < length - 1; ++i) {
            array[i] = array[i + 1];
        }

        --counter;
    }

    //Overwrite any value you'd like, no need to adjust anything
    void overWrite(int position, Heap variable) {
        array[position] = variable;
    }

    //swap any two array points
    void swap (int position1, int position2) {
        Heap temp;
        temp = array[position1];
        array[position1] = array[position2];
        array[position2] = temp;
    }

    //reseed amount of space you'd like to allocate when you're running out
    //of room
    void reSeed(int seed) {
        this->seed = seed;
    }

    //is item in array?
    bool search(Heap item) {
        //this might make it faster ::shrug::
        for (int i = 0, j = counter; i < counter; ++i, --j) {
            if (array[i] == item || array[j] == item) return true;
        }
        return false;
    }

    //Creates perfect ArrayList stored on heap without garbage
    void refine() {
        int i;

        length = counter;
        Heap *temp = new Heap[length];

        for (i = 0; i < length; ++i) {
            temp[i] = array[i];
        }

        delete [] array;
        array = new Heap[length];

        for (i = 0; i < length; ++i) {
            array[i] = temp[i];
        }

        delete[] temp;
    }

    void finished(){
        delete[] array;
    }
};

#endif // MEMORYLIST_H

A couple of things jump out:

Use size_t for the all allocation sizes/indices. I have had real-world bugs because of this in third-party libraries.

Use exponential, not linear, reallocations. Doubling each time is the best way. This allows .newValue to append in \$O(1)\$ amortized time, as opposed to the current \$O(n)\$.


Verifying that three cards make a "Set" The game of Set involves cards with four properties: number, symbol, shading, and colour.

Three cards make a "Set" if the property is either all the same or all different across the three cards, for each of the four properties.

I wrote some code to verify if three cards constitute a "Set". But I'm not satisfied with the final function isSet. I feel there should be some way to iterate over the four properties instead of writing four separate lines. Any help would be appreciated - I'm fairly new to F#.

type Number = | One | Two | Three
type Symbol = | Round | Wavy | Diamond
type Shading = | Empty | Lines | Full
type Colour = | Red | Green | Blue

type Card = {number: Number;
            symbol: Symbol;
            shading: Shading;
            colour: Colour}

let all =
    List.reduce (=)

let numUnique list =
    list
    |> List.distinct
    |> List.length

let allSame list =
    list
    |> numUnique
    |> (=) 1

let allUnique list =
    let length = List.length list
    list
    |> numUnique
    |> (=) length

let allSameOrUnique list =
    allSame list || allUnique list

let isSet card1 card2 card3 =

    let check1 = allSameOrUnique [card1.number; card2.number; card3.number]
    let check2 = allSameOrUnique [card1.symbol; card2.symbol; card3.symbol]
    let check3 = allSameOrUnique [card1.shading; card2.shading; card3.shading]
    let check4 = allSameOrUnique [card1.colour; card2.colour; card3.colour]

    all [check1; check2; check3; check3; check4]

let card1 = {number = Two; symbol = Round; shading = Lines; colour = Green}
let card2 = {number = One; symbol = Round; shading = Empty; colour = Green}
let card3 = {number = Three; symbol = Round; shading = Full; colour = Green}

The final function returns a boolean for verifying the set:

> isSet card1 card2 card3

val it : bool = true

I feel there should be some way to iterate over the four properties instead of writing four separate lines.

You can use a higher-order function that takes in a function that returns a card property. Here is an example of how that could look. I've also simplified the code to the extent that none of the helper functions are needed, and changed some formatting to more idiomatic F#:

type Number = One | Two | Three
type Symbol = Round | Wavy | Diamond
type Shading = Empty | Lines | Full
type Colour = Red | Green | Blue

type Card =
    { Number: Number
      Symbol: Symbol
      Shading: Shading
      Colour: Colour}

let isSet card1 card2 card3 =
    let sameOrUniqueBy prop =
        let unique = [ card1; card2; card3 ] |> List.distinctBy prop |> List.length
        unique = 1 || unique = 3

    sameOrUniqueBy (fun c -> c.Number)
    && sameOrUniqueBy (fun c -> c.Symbol)
    && sameOrUniqueBy (fun c -> c.Shading)
    && sameOrUniqueBy (fun c -> c.Colour)

In this code, sameOrUniqueBy is a higher-order function because it takes a function as an argument. It is defined as a local function inside isSet so that the cards don't have to be passed in each time it's used.

Testing:

let card1 = { Number = Two; Symbol = Round; Shading = Lines; Colour = Green }
let card2 = { Number = One; Symbol = Round; Shading = Empty; Colour = Green }
let card3 = { Number = Three; Symbol = Round; Shading = Full; Colour = Green }

isSet card1 card2 card3 = true // ✔

isSet card1 card1 card3 = false // ✔


Sorting entities by Z in every call to Draw My implementation contains a SpriteComponent class which has a Z property; I use this to determine the order in which to draw my entities (higher Z = drawn on top). It's a simple integer.

What I'm worrying about (premature optimization, I suspect) is the runtime complexity of the function which gets the list of all the SpriteComponents to draw, in order of their Z attribute:

List<IEntity> entitiesToDraw = currentScreen.Entities
    .Where(e => e.HasComponent<SpriteComponent>())
    .OrderBy(e => e.GetComponent<SpriteComponent>().Z)
    .ToList();

From what I've read, OrderBy uses QuickSort, which is \$O(n log n)\$, but I'm not sure about Where.

I can reasonably assume:

1000+ entities can concurrently exist
90% or more of entities have a SpriteComponent in them
Changing the Z of a sprite happens fairly infrequently for most sprites

My questions are:

Should I be worrying about this call at all?
If so, how can I reduce the runtime complexity, and/or cache the results reliably?

Not sure about Where

Where method (if applied to IEnumerable<T>) is O(N) operation, and it's a lazy operation so it will do the filtering while consumer reads the result.

  Should I be worrying about this call at all?

The best way to answer this question is to measure the performance and see if it satisfies you. If not - then start thinking about caching results and refreshing them instead of calculating from scratch.

  If so, how can I reduce the runtime complexity, and/or cache the results reliably?

It's hard to suggest good solution based on the information you've provided... Will the number of entities change over time? Can they "loose" SpriteComponent?

Assuming that the number of entities is the same and only Z will slightly change you can cache the sorted list from previous run, and apply TimSort or Insertion sort (you would need to implement it yourselves or grab from Wikipedia) to nearly-sorted list.