jQuery multiple events execute same function refactoring I am working a project which uses a function to show a modal dialog. The dialog can be hidden by calling the hideModal() function. This function is triggered by:

Pressing the ESC key
Clicking on the modal background
Clicking on the close button

My current code is:

 $("#modal").click(function() {
    hideModal();
});

$("#modal-object-container > a").click(function(e) {
    hideModal();
    e.preventDefault();
});

$(document).keydown(function (e) {

    if (e.keyCode == '27') {
        hideModal();
    }

});

I have the feeling there should be a faster way to bind all these events at once.

Your code isn't all that bad. However because you are binding key events on the document, you need to allow room, for other key events to be bound, for other tasks, so your code doesn't interfere with other ESC functions.

The concept is that you bind on click, and unbind upon hiding your modal. This way the key events are only applicable when your code is run.

// save your doc once, it's faster
var doc = $(document);
    // bind a function, which you can unbind later, so esc can be used in other cases as well, without triggering your function
    doc.bind('keydown', _processKey);

// on click binding for as many elements as you like.
// The preventDefault shouldn't influence other elements in this case.
$("#modal, #modal-object-container > a").click(function(ev) {
    ev.preventDefault();
    hideModal();
});

// when you hide your modal, you unbind the event, so the document is free for other events and functions
function hideModal() {
    doc.unbind('keydown', _processKey);
}

// have a 'private' function to do what you want. This way you can unbind it anytime you like
function _processKey(ev) {
    if (ev.keyCode == '27') {
        hideModal();
    }
}


Project Euler #10 in Java Project Euler #10:

  The sum of the primes below 10 is \$2 + 3 + 5 + 7 = 17\$.

  Find the sum of all the primes below two million.

My solution:

public class PrimeSumFinder {

    private static final int MAX = 2_000_000;

    public static void main(String[] args) {
        long time = System.nanoTime();
        long result = getSumOfPrimesBelowN(MAX);
        time = System.nanoTime() - time;
        System.out.println("Result: " + result
                + "\nTime used to calculate in nanoseconds: " + time);
    }

    private static long getSumOfPrimesBelowN(int n) {
        boolean[] isPrimeArray = new boolean[n + 1];
        for (int i = 2; i <= n; i++) {
            isPrimeArray[i] = true;
        }
        for (int i = 2; i * i <= n; i++) {
            if (isPrimeArray[i]) {
                for (int j = i; i * j <= n; j++) {
                    isPrimeArray[i * j] = false;
                }
            }
        }
        // Sum the primes
        int index = 0;
        long result = 0;
        for(boolean isPrime : isPrimeArray) {
            if(isPrime) {
                result += index;
            }
            index++;
        }
        return result;
    }

}

Output:

   Result: 142913828922
 Time used to calculate in nanoseconds: 87694113

Questions:

Is this the most efficient way? If not, what is a better way?
Does it have bad practices in it?

For your inner for-loop you have

for (int j = i; i * j <= n; j++) {
                isPrimeArray[i * j] = false;
            }

Once you find a prime number, you can start knocking out factors at i*i, since any multiple of i less than that would be a multiple by an existing smaller prime you already ran elimination on in a previous iteration!

Also, you have to go through the entire array, so your ending point would be j<=n

And lastly, you are iterating over multiples of i! So your loop can increment at j+=i at each step. Leaving the final fixed loop to be:

for (int j = i*i; j < n; j+=i) {
                isPrimeArray[j] = false;
            }

This should cut down on plenty of iterations.

Also you may want to consider breaking your outer loop into two for-loops.
First a for loop for multiples of 2

for(int i = 4; i < n; i+=2) //set to false

Since 2 is the only even prime, this means that in your main code (your old outer loop) can be rewritten to only go over odd numbers, cutting down half the iterations!

Another time saving tip is to also process your loop up to Math.sqrt(n) because once you hit the square root, anything after that is prime because multiples of those numbers fall outside of your sieve.

int limit = (int)Math.sqrt(n) + 1
for(int i = 3; i < limit ; i+=2) //inner loop revised above

So now you are not only skipping even numbers, but also only going up to the square root of n, rather than n itself.

Other than that your spacing and naming seems to be fine. Though I would recommend you separate the tasks out into different methods. Namely split your function into a generateSieve() function that makes the sieve, and then a sumOfPrimes function that takes the sieve as a parameter and outputs the sum.

I say this because the sieve itself is a widely used technique for Project Euler problems, and you won't just want to find the sum! So I recommend making generateSieve() (or whatever you may name it) it's own function.


Variable class from library Can I do something better with this code?  It's just a snippet of a library I wrote.

<?php
/*
 * Variable.class.php
 * (c) 2013 sinan eker`enter code here`
 * */
 class Variable extends Config {
     protected static $vars = array();
     final public static function variables(){
         return static::$vars;
     }
     final public static function clear(String $name){
         unset(static::$vars[$name->getString()]);
     }
     final public static function destruct(){
         unset(static::$vars);
         static::$vars = array();
     }
     final public static function override(String $name, $value){
         static::clear($name);
         static::register($name, $value);
     }
     final public static function get(String $name){
         $name = $name->getString();
         return (isset(static::$vars[$name]) ? static::$vars[$name] : null);
     }
     final public static function register(String $name, $value){
         return (!is_null(static::get($name)) ? false : static::$vars[$name->getString()] = $value);
     }
 }

First of all, method and variable docblocks are recommended. Provide a description of each method, its parameters and their return values.

I would give your getter function Variable::variables() an active name like getVariables() and rename your Variable::get() method to getVariable(). Consistency is important and you want to avoid confusion with the magic getter __get().

Are you intentionally using the non-magic method Variable::destruct instead of the PHP magic method __destruct()?

This is more of a syntax preference, but I would use array_key_exists() instead of isset() when referring to the property static::$vars since I find the verbage more aligned with what you are testing.

The docblocks are the only important thing. Good work.


Creating readline() function, reading from a file descriptor I've got a task to create a readline() function that would have the exact same behavior as read():

ssize_t read(int fd, void *buf, size_t count); (man read)

So it should return -1 on error and number of bytes read on success.
Also, if the function receives a string with multiple \n chars, it should return only the first line and store the rest for a future use.

I was scrolling through this article:
Reading a line ending with a newline character from a file descriptor

But I do not think the author had the same purpose - that is to work just as read()

I came up with this function:

//global buffer
char gBUF[BUFSIZ];

int readline2(int fd, char* buf){

    static char* resultBuffer = gBUF;
    int ret, mv;
    char temp[256];
    char* t;

    bzero(temp, sizeof(temp));

    t = strchr(resultBuffer,'\n');
    if(t != NULL){
        mv = t-resultBuffer+1;
        strncpy(temp,resultBuffer, mv);
        resultBuffer = resultBuffer + mv;

        temp[strlen(temp)] = '\0';
        strcpy(buf,temp);
        bzero(temp, sizeof(temp));
        ret = read(fd,&temp,256);
        temp[ret]='\0';
        strcat(resultBuffer,temp);

        return mv;
    }
    ret = read(fd,&temp,256);
    temp[ret]='\0';
    t = strchr(temp,'\n');
    mv = t-temp+1;
    strncpy(buf,temp,mv);

    strcat(resultBuffer,temp+mv);
    return ret;

}

and I mean, it works well, although I had some real struggles with pointers and copying addresses instead of values.

My question is, how to improve this? I still have a feeling there's something missing or that something could significantly improve it.

I guess I should put a while inside and check whether the read string actually has a \n char and only return when it does, right?

I also do not like the idea of a global buffer, maybe I should just create a class/struct, something like this:

struct {
char BUF[BUFSIZ];
char *p = BUF;
}dataStruct;

to store the buffer and a pointer to it, but I do not really know how I could use that properly

Thanks for any suggestions!

Reading max 256bytes is a purpose, that should be the max;

I could've probably added that this function is to be used after in a multiprocess/multithread client server project.
The server reads these lines from clients and should know what input came first, in what order the lines came, etc. The buffer should essentially work as a queue.

As we've basically worked out in the comments: when there's already a thing, use the thing. To get a line from a file descriptor, you can

Call fdopen on your file descriptor to get a FILE*
Check for NULL, perror and exit if necessary
Call fgets
Check for NULL again
Repeat 3-4 as necessary
fclose your FILE*
Do not call close on your file descriptor


Find paths whose sum equals a target value in a binary tree You are given a binary tree in which each node contains an integer value (which might be positive or negative). Design an algorithm to count the number of paths that sum to a given value. The path does not need to start or end at the root or a leaf, but it must go downwards (traveling only from parent nodes to child nodes).

The question asks for count only but I went one step ahead and found the paths too.

The following is my attempt at this code. Please let me know how I can improve it. Also I think the time complexity of the code is \$O(n^2)\$ because of the following reasoning:

findPathsWithSumHelper is called n times for each node in the tree.
This function goes through all the nodes below the root to find paths.
The total work done is \$n + 2 * \frac{n}{2} + 4 * \frac{n}{4} + \dots n * \frac{n}{n}\$ which equals \$O(n^2)\$.
The work done in copying the arrays using emplace_back is \$O(n)\$ at each node in the tree and adds a constant to \$O(n^2)\$.

#include <iostream>
#include <vector>
using namespace std;

struct Tree{
    struct Tree * left, *right;
    int data;
    Tree(int val): data(val), left(NULL), right(NULL){};
};

//insert nodes randomly in a BT
void insertNodeBT(Tree * &root, int val){

    Tree * newNode = new Tree(val);

    if(root==NULL){
        root = newNode;
        return;
    }

    int r = rand()%2;

    if(r)
        insertNodeBT(root->left, val);
    else
        insertNodeBT(root->right, val);
}

vector<vector<int>> findPathsWithSumHelper(Tree * root, int target, int prev, vector<int> path){

    vector<vector<int>> res;

    if(root == NULL){
        return res;
    }

    path.push_back(root->data);

    if(prev + root->data == target){
        res.push_back(path);
    }

    vector<vector<int>> temp;
    temp = findPathsWithSumHelper(root->left, target, prev+root->data, path);

    for(auto i : temp){
        res.emplace_back(move(i));
    }

    temp = findPathsWithSumHelper(root->right, target, prev+root->data, path);

    for(auto i : temp){
        res.emplace_back(move(i));
    }

    return res;

}

vector<vector<int>> findPathsWithSum(Tree * root, int target){

    vector<vector<int>> res;

    if(root == NULL){

        return res;
    }

    vector<int> path;
    res = findPathsWithSumHelper(root, target, 0, path);
    vector<vector<int>> temp;
    temp = findPathsWithSum(root->left, target);

    for(auto i : temp){
        res.emplace_back(move(i));
    }

    temp = findPathsWithSum(root->right, target);

    for(auto i : temp){
        res.emplace_back(move(i));
    }

    return res;

}

int main() {

    Tree *root = NULL;
    insertNodeBT(root, 4);
    insertNodeBT(root, 6);
    insertNodeBT(root, 2);
    insertNodeBT(root, 7);
    insertNodeBT(root, 5);
    insertNodeBT(root, 1);
    insertNodeBT(root, 3);
    insertNodeBT(root, -1);
    vector<vector<int>> res = findPathsWithSum(root, 6);

    for (auto i : res){
        for (auto j : i) {
            cout << j << " ";
        }
        cout<<endl;
    }
    return 0;
}

Following is a better solution but it only returns the count and not the actual paths. I feel it is \$O(nlogn)\$ but not sure.

int findPathsWithSum(Tree * root, int target, unordered_map<int, int> m){

    if(root==NULL)
        return 0;

    int res=0;

    for(auto &it : m){
        if(it.first+root->data == target)
            res++;
        m[it.first+root->data]++;
        m.erase(it.first);

    }

    if(root->data == target)
        res++;

    m[root->data]++;
    res += findPathsWithSum(root->left, target, m);
    res += findPathsWithSum(root->right, target, m);

    return res;

}

Simplification: remove temp vectors

Currently, your code spends a lot of effort merging partial return vectors with the final return vector.  You can eliminate the use of these temp vectors if you just pass in the final return vector as an argument to each recursive call.  Here is what your functions would look like with this change, which not only looks simpler but also speeds things up quite a bit:

void findPathsWithSumHelper(Tree * root, int target, int prev,
        vector<int> path, vector<vector<int>> &res)
{
    if (root == NULL)
        return;

    path.push_back(root->data);

    prev += root->data;
    if (prev == target)
        res.push_back(path);

    findPathsWithSumHelper(root->left,  target, prev, path, res);
    findPathsWithSumHelper(root->right, target, prev, path, res);
}

void findPathsWithSum1(Tree * root, int target, vector<vector<int>> &res)
{
    if(root == NULL)
        return;

    vector<int> path;
    findPathsWithSumHelper(root, target, 0, path, res);
    findPathsWithSum1(root->left,  target, res);
    findPathsWithSum1(root->right, target, res);
}

vector<vector<int>> findPathsWithSum(Tree * root, int target)
{
    vector<vector<int>> res;
    findPathsWithSum1(root, target, res);
    return res;
}

I tested with a complete binary tree of depth 16 (64K nodes) of all value 0, searching for value 0.  This returns 983041 paths (every path in the entire tree).  With the original code, this took 4 seconds.  With the simplified code, it took 0.78 seconds.

Simplification: Reduce recursions

Right now, your program recurses in two ways.  The first way recurses to the child, adding the child to an existing path.  The second way starts a new path at the current node and then recurses to the children.

You can reduce the amount of recursion by always adding to the existing path, but at each level, you must try each point in the path as a possible head of a path.  I rewrote your functions to do that and found that this change speeds up the program even further:

static void findPathsHelper(const Tree *node, int target, int depth,
        vector<int> &path, int pathSum, vector<vector<int>> &ret)
{
    if (node == NULL)
        return;

    path.push_back(node->data);
    pathSum += node->data;
    depth++;

    int sum = pathSum;
    // Try each point in the path as a new head of a path.
    for (int i=0; i<depth; i++) {
        if (sum == target) {
            vector<int> subPath(depth - i);
            for (int j=i; j < depth; j++) {
                subPath[j-i] = path[j];
            }
            ret.push_back(move(subPath));
        }
        sum -= path[i];
    }
    findPathsHelper(node->left,  target, depth, path, pathSum, ret);
    findPathsHelper(node->right, target, depth, path, pathSum, ret);
    path.pop_back();
}

vector<vector<int>> findPathsWithSum(const Tree *root, int target)
{
    vector<int> path;
    vector<vector<int>> ret;

    findPathsHelper(root, target, 0, path, 0, ret);
    return ret;
}

Test program and timings

Here is the code I used to test the various implementations.  I commented out the part that printed the results in order to reduce time spent on IO.  I mostly used complete binary trees with all nodes having value 0 and searching for value 0 in order to generate the worst case scenario, although I did test other cases including using randomized values (code not shown).

Tree *createCompleteBT(int depth, int value)
{
    if (depth == 0)
        return NULL;

    Tree *newNode = new Tree(value);
    newNode->left  = createCompleteBT(depth - 1, value);
    newNode->right = createCompleteBT(depth - 1, value);
    return newNode;
}

// Run with: depth value find
int main(int argc, char *argv[])
{
    int depth = 16;
    int value = 0;
    int find  = 0;

    if (argc > 1)
        depth = atoi(argv[1]);
    if (argc > 2)
        value = atoi(argv[2]);
    if (argc > 3)
        find  = atoi(argv[3]);

    Tree *root = createCompleteBT(depth, value);
    vector<vector<int>> res = findPathsWithSum(root, find);

    cout << res.size() << endl;
#if 0
    for (auto i : res){
        for (auto j : i) {
            cout << j << " ";
        }
        cout<<endl;
    }
#endif
    return 0;
}

Here are the timing results for depth 16 trees:

Original program    :  4.06 seconds
Removed temp vectors:  0.78 seconds
Reduced recursion   :  0.22 seconds

By the way, for a balanced tree, I believe the complexity is \$O(n \log^2 n)\$.  You need to visit each node once (\$n\$), and then for each node search a path from the root (\$\log n\$), and if the path matches the target, you need to add that path (\$\log n\$ again).  For a tree that is completely linear, the time would be \$O(n^3)\$.  For an arbitrary tree, the complexity should be \$O(n*d^2)\$ where \$n\$ is the number of nodes and \$d\$ is the depth of the tree.


Run all scripts in a sub-directory When git runs my post-receive hook, I'd like it to run all the scripts in the subdirectory post-receive.d.

My script is as follows

#!/bin/bash
SOURCE="${BASH_SOURCE[0]}"
DIR="$( dirname "$SOURCE" )"

for SCRIPT in $DIR/post-receive.d/*
do
  if [ -f "$SCRIPT" -a -x "$SCRIPT" ]
  then
    "$SCRIPT"
  fi
done

Source: https://github.com/alexchamberlain/githooks/blob/master/post-receive

Is this secure? Reliable?

I can foresee one problem. The hook supplies data using stdin, which should be copied to each subscript. I'm pretty sure this isn't happening at the moment. Any ideas?

for SCRIPT in $DIR/post-receive.d/*

You might want to limit that to only *.sh pattern, just for clarity.

Personally I'm a fan of the one-line-style, but that's personal preference:

for ITEM in SET; do
if [ CONDITION ]; then

And yes, input from stdin is at the moment ignored completely by your script. Reading from stdin can be done via the read command, like this.

#!/bin/sh

# This whill be our input variable
input=""

# Read the first line without adding
# a new line to the start.
read input

# Start reading from stdin
while read line; do
    # Concate the input into our variable
    input=$input"\n"$line
done

# Echo it...what else?
echo $input

After doing that you can pipe the variable into the scripts like this:

echo $INPUT | "$SCRIPT"

One word of warning: I'm not sure if this is BASH only or cross-shell compatible.


Getting prime numbers, custom example I was tasked this in class today, so the teacher could judge where our skills are.

I am not an experienced C++ programmer, and I made a mistake on my sheet, so I came home and wrote this.

I saw other examples online, but this is the solution I came up with. It works from what I can see, and for these test values.

Did I make any mistakes? How can I improve my coding style and algorithm?

#include <iostream>
#include <string>

    int main()
    {
      int intnumbers[10]= { 10,11, 17,20,37,53,44,88,90,42 };
      for(int i=0;i<10;i++)
      {
         if(intnumbers[i]%2==1){
          if((intnumbers[i]%3==1) || (intnumbers[i]%3==2) )
          {
              std::cout<<intnumbers[i] <<std::endl;      }
         }

      }
      }

Style improvements:

The use of whitespace here is inconsistent:

int intnumbers[10]= { 10,11, 17,20,37,53,44,88,90,42 };

It's good to pay attention to detail by keeping it consistent:

int intnumbers[10] = { 10, 11, 17, 20, 37, 53, 44, 88, 90, 42 };

The spacing in between will make it a little more readable, even though it may increase the horizontal width.
This could also use some whitespace:

for(int i=0;i<10;i++)

It's good to keep one space between operators and operands so that they can be easily distinguished at first glance.

for (int i = 0; i < 10; i++)

Stick to one length of indentation.  For instance, I see two below the first {, just one after the first if, and four after the second if.  Four is most common in C++, so consider using that everywhere.
You can increase performance a little by outputting '\n' instead of std::endl.  The latter also flushes the buffer, whereas the former does not.
Want to stay ahead of the C++ game a bit (at least in a classroom setting)?  Use a container class such as an std::vector in place of the C array:

const std::vector<int> intnumbers { 10, 11, 17, 20, 37, 53, 44, 88, 90, 42 };

It may not matter that much in this small program without any functions, but it's a good habit for C++ programming.  You'll even benefit from various member functions and bounds-checking.


Filename generator with Underscore The code generates all combinations of filename with underscore _:

def filename_generator(name, path=''):
    names = []

    for i in range(len(name)):
        p = '_'.join([x for x in name[i:]]) + path

        names.append(name[:i] + p)
        names += filename_generator(name[:i], p)

    return sorted(list(set(names)))

for e in filename_generator('longlonglonglonglonglongname'):
    print(e)

Example:

Input:

name

Output:

n_a_m_e
n_a_me
n_am_e
n_ame
na_m_e
na_me
nam_e
name

My code works well, but very slowly. How can I optimize the algorithm?

Note that there are 2n-1 different such names for a string of lengh n, as there can or can not be a _ between any two characters. For your long-long name that makes 134,217,728 variants!

Regardless of what algorithm you use, this will take a very long time. One way to work around this, depending on your actual use case, might be to use a generator function. This way, you do not need to calculate all the variants, if maybe you need just a few of them (maybe just enough to find one that does not yet exist?). For example, like this:

def filename_generator(name):
    if len(name) > 1:
        first, rest = name[0], name[1:]
        for rest2 in filename_generator(rest):
            yield first + rest2
            yield first + "_" + rest2
    else:
        yield name

Output:

>>> list(filename_generator("name"))
['name', 'n_ame', 'na_me', 'n_a_me', 'nam_e', 'n_am_e', 'na_m_e', 'n_a_m_e']

Or, building upon the idea by @jacdeh, using binary numbers to determine where to put a _.

def filename_generator(name):
    d = {"0": "", "1": "_"}
    for n in range(2**(len(name)-1)):
        binary = "{:0{}b}".format(n, len(name))
        yield''.join(d[s] + c for c, s in zip(name, binary))

Or similar, using bit shifting:

def filename_generator(name):
    for n in range(2**(len(name)-1)):
        yield ''.join(c + ("_" if n & 1<<i == 1<<i else "") for i, c in enumerate(name))

However, either both those implementation are pretty lousy, or it does not help much in the end: They are both about 10 times slower than the recursive function, according to timeit.

Here's some timing information, using IPython's %timeit magic method:

In [8]: %timeit filename_generator("longname")         # your original function
1000 loops, best of 3: 610 µs per loop
In [9]: %timeit list(filename_generator1("longname"))  # my recursive generator
10000 loops, best of 3: 22.5 µs per loop
In [10]: %timeit list(filename_generator2("longname")) # my binary generator
1000 loops, best of 3: 322 µs per loop
In [11]: %timeit partition_string("longname")          # Eric's binary function
1000 loops, best of 3: 200 µs per loop

Another possibility, as posted in comments, would be to use itertools.product to generate the "mask", similar to the binary number solution, but with less involved math.

from itertools import product, chain
def filename_generator4(name):
    for mask in product(("_", ""), repeat=len(name) - 1):
        yield ''.join(chain(*zip(name, mask + ("",))))

However, performance is about the same as for the binary solutions. I guess the bottleneck is the number of string concatenations: In the recursive solution, you "reuse" the partial solutions, while each of the latter solutions construct each solution "from scratch", so there are fewer total string concatenation in the recursive case. Not sure, though, so feel free to comment.


Storing global constants in app delegate or singleton I am converting a non ARC projecs to ARC. I just found that after login in my old app, the data is stored in app delegate objects, like Home_DataArray, various flags and some other objects.

I am bit confused at it, because my app delegate is looking like constant class and BoolDidFinishLaunch looks like parent of various code.

Old app Delegates.h:

@property (nonatomic, copy) void(^backgroundTransferCompletionHandler)();

@property(nonatomic)BOOL activatedFromBackground;
@property(nonatomic,strong)IBOutlet UIViewController *SplashViewController;
@property(nonatomic,strong)IBOutlet UIViewController *AppCodeViewController;
@property(nonatomic,strong)IBOutlet UINavigationController *navControl;
@property(nonatomic,strong)NSMutableArray *arrHomeProducts;
@property(nonatomic,strong)NSMutableArray *arrayDownloadProducts;
@property(nonatomic,strong)NSString *AppNIB;
@property(nonatomic,strong)NSBundle* languageBundle;
@property(nonatomic,strong)UIViewController* loginVC;
@property(nonatomic,assign)BOOL acceptedMessage_Status;

@property(nonatomic,strong)NSIndexPath *selectedWebsiteIndex;
@property(nonatomic,strong)NSString *selectedWebsiteID;
@property(nonatomic,strong)NSString *userID;
@property(nonatomic,strong)NSString *partID;
@property(nonatomic,strong)NSString *websiteID;
@property(nonatomic,strong)NSString *BrandName;
@property(nonatomic,strong)NSString *driverSide;
@property(nonatomic,strong)NSString *str_bundle;
@property(nonatomic,strong)NSString *str_BundleCost;
@property(nonatomic,assign)BOOL isCalledFromSafariBrowser;
-----------------------------------------------------------
------------------------------so on ---------------------------

and in similar allocation of array in didfinish launch.

I did same things and code is working but is it the right way of using objects in app delegate.

Please suggest how I should opt a singleton-based approach to declare, alloc and store various objects for various view controllers or something else.

Storing everything in a global singleton can be problematic, I wouldn't recommend that.

I worked on a project once where all data ever passed to view controllers from outside was stored a giant singleton class with a lot of properties. Essentially these properties were just global variables.

All this led to bugs where some view controller changed a variable at a wrong point in time and another view controller didn't expect that. If you ask anybody on the internet, they are probably going to say that singletons are bad. And in most situations they are, especially like this one.

So instead of using a singleton I would recommend passing parameters to view controllers somehow before you show them. It can be a as simple as setting a property:

@interface ViewController : UIViewController

@property (nonatomic) NSString *someParameter;

@end

@implementation ViewController

- (void)viewWillAppear:(BOOL)animated {
    [super viewWillAppear:animated];

    // Update your UI based on someParameter. For example:
    self.myAwesomeLabel.text = someParameter;
}

@end

If ViewController presents another view controller that happens to need someParameter too, what previously used to be a simple access to your singleton property now has to be replaced with code that will explicitly pass someParameter to the presented view controller like so:

AnotherViewController *vc = [[AnotherViewController alloc] init];
vc.someProperty = self.someProperty;
vc.anotherProperty = @"Another property";
[self presentViewController:vc animated:YES completion:nil];

I think this is called property injection (a kind of dependency injection). There is also constructor (initializer) injection:

@interface ViewController : UIViewController

@property (nonatomic, readonly) NSString *someProperty;

- (instancetype)initWithSomeProperty:(NSString *)someProperty;

@end

@implementation ViewController

- (instancetype)initWithSomeProperty:(NSString *)someProperty {
    if (self = [super initWithNibName:nil bundle:nil]) {
        _someProperty = someProperty;
    }
    return self;
}

@end

There is also a great article on objc.io about this stuff.


Entity class for a Slick2D game I'm working on a game with Slick2D and am just working through the Entity class. I'm making every variable protected and then accessing them with getters and setters. Yet it feels like I'm over-complicating things and it is more efficient to simply let some of these properties be public.

public abstract class Entity {
    protected Point location;
    protected Vector2f vector;
    protected boolean visible;
    protected Image image;

    public Entity() {
        location = new Point();
        vector = new Vector2f();
    }

    public void setLocation(int x, int y) {
        location.setX(x);
        location.setY(y);
    }

    public void setVelocity(int vx, int vy) {
        vector.x = vx;
        vector.y = vy;
    }
    public void update(){
        location.setX(Math.round(location.getX() + vector.x));
        location.setY(Math.round(location.getY() + vector.y));
    }

    public void setImage(String path) {
        try {
            image = new Image("res/" + path + ".png");
        } catch (Exception e) {
            e.printStackTrace();
        }
    }

    public int getX(){
        return location.getX();
    }
    public int getY(){
        return location.getY();
    }

    public Image getImage(){
        return image;
    }

Then in the main Game class:

public void render(GameContainer gc, Graphics g) throws SlickException {
    g.drawImage(player.getImage(), player.getX(), player.getY());
...
}

It just feels like additional work using so many methods as opposed to directly accessing the field.

Additionally, is there any particular reason Vector2f()'s x and y fields are public while Point's x and y must be accessed through getters/setters?

It just feels like additional work using so many methods as opposed to directly accessing the field.

Is it really? You have this:

g.drawImage(player.getImage(), player.getX(), player.getY());

With direct access, it would be:

g.drawImage(player.image, player.location.getX(), player.location.getY());

Which actually seems more complicated to me. It's longer, and you have to know of what type location is. Additionally, any decent IDE can generate fields plus getters and setters for you.

But the actual point of using getters and setters is that it encapsulates your code. For example, if you want to switch from using Point to using some other class. With getters/setters, you only have to change your Entity class, instead of that class plus your main plus maybe others.

Misc

correct indentation makes your code easier to read
your fields actually should not be protected, but private
I would not just print the stack trace inside the entity class, but thrown an exception and handle it elsewhere (just swallowing the exception will cause problems further on, because now the image is null). You could also try to recover by using a generic generated error image instead, so your application still works if one image file is damaged/lost


Queries for the number of users seen each day The below code returns an array filled with the number of users last seen for each day for $dayCount number of days that is specified via a parameter into the function.
It does this by executing a database query for each day from the previous day going back $dayCount amount of days, and another function produces from this data to provide an output in CSV-style formatting for a graph.

This essentially means that if, say, I wanted the number of users last seen on each day for the last thirty days, a total of 30 database queries will be run.

To me this seems incredibly inefficient, but I can't think of a way around this myself. Can anything be done to make this more efficient?

function GetUserCountFromDays($dayCount) {
    $arrCnt = array();
    $baseQuery;
    $result;
    for ($day = $dayCount; $day >= 1; $day--) {
        $baseQuery = "SELECT * FROM example.servers WHERE last_seen >
                      UNIX_TIMESTAMP(DATE_ADD(CURDATE(),INTERVAL -" . $day . " DAY))";
        $result = ConnectToMySQL()->query($baseQuery);
        if ($result){
            array_push($arrCnt, $result->num_rows);
        }
    }
    return $arrCnt;
}

You're right, you should avoid querying in a loop as much as you can. If $dayCount were 30, I would run a query like:

SELECT
    DATEDIFF(CURRENT_DATE,FROM_UNIXTIME(last_seen)) as inactive_days,
    COUNT(*) AS num_rows
FROM example.servers
WHERE UNIX_TIMESTAMP(DATE_SUB(CURDATE(),INTERVAL 30 DAY)) <= last_seen
GROUP BY inactive_days
ORDER BY inactive_days ASC

This should give you an array of rows, each row would have inactive_days holding the number of days between today and last seen day, and num_rows holding the number of records that were grouped into that row. The result set is also ordered with the fewest inactive days first. Your PHP script could then use that result as needed.  If you fetch the array and call it $result, you could do

foreach($result as $row):
    echo "$row[num_rows] users where last seen $row[inactive_days] days ago";
endforeach;

Quick notes:

Be sure to test! I may have a bug in my code, but you get the gist. If you find a bug, leave a comment asking me to fix it.
Don't do anything you don't understand.  Take the time to learn what each line of code does, so you can get better, catch errors and even improve upon it.
If the order of the results doesn't matter to your logic, don't use the ORDER BY clause; sorting is computationally expensive and should only be used when needed


Generic Repository without Entity Framework Almost all of generic repository pattern tutorials were making use of EF, so I haven't got much sources to compare my code with. Here is what I have done. Could you please suggest me to fix my wrongs if I have or improve the code?

Note: I am using Dapper.Net as ORM, and I am aware of Dapper.Contrib dll, I just want to implement my own methods.

Generic Repository class

public interface IRepository<T> where T : BaseEntity
{
    IDbConnection Connection { get; set; }
    void Insert(T entity);
    void Delete(T entity);
    T GetById(int id);
    IQueryable<T> GetAll();
}

A Repository Class

public interface ICustomerRepository : IRepository<Customer>
{

}

public class CustomerRepository : ICustomerRepository
{
    public IDbConnection Connection { get; set; }

    public CustomerRepository()
    {
        Connection = new SqlConnection(ConfigurationManager.ConnectionStrings["default"].ConnectionString);
    }

    public void Delete(Customer entity)
    {
        try
        {
            using (Connection)
            {
                Connection.Execute("DELETE FROM dbo.Customer Where Id = @Id", new { entity.Id });
            }
        }
        catch (Exception ex)
        {

        }
    }

    public IQueryable<Customer> GetAll()
    {
        IQueryable<Customer> list = new List<Customer>().AsQueryable();

        try
        {
            using (Connection)
            {
                list = Connection.Query<Customer>("SELECT * FROM dbo.Customer").AsQueryable();
            }
        }
        catch (Exception ex)
        {

        }
        return list;
    }

    public Customer GetById(int id)
    {
        Customer customer = new Customer();
        try
        {
            using (Connection)
            {
                customer = Connection.Query<Customer>("SELECT * FROM dbo.Customer WHERE Id = @Id", new { Id = id }).SingleOrDefault();
            }
        }
        catch (Exception ex)
        {

        }
        return customer;
    }

    public void Insert(Customer entity)
    {
        try
        {
            using (Connection)
            {
                Connection.Execute("INSERT INTO dbo.Customer VALUES(@FullName, @TcNo, @BirthDate)",
                    new
                    {
                        entity.FullName,
                        entity.Tcno,
                        entity.BirthDate
                    });
            }
        }
        catch (Exception ex)
        {

        }
    }
}

An Api Controller

public class CustomerController : ApiController
{
    private IRepository<Customer> _customerRepo;

    public CustomerController()
    {
        _customerRepo = new CustomerRepository(new SqlConnection(ConfigurationManager.ConnectionStrings["default"].ConnectionString));
    }
    [HttpGet]
    [Route("")]
    public IHttpActionResult Get()
    {
        return Json(_customerRepo.GetAll());
    }
}

I think you are in the right direction but let me suggest some things.

Remove the try catch and let the repository throw the exception. Either that or catch and throw your own exception.

Inject IDbConnection in the constructor of the repository and remove the using on the repository methods

    private IDbConnection _connection;

    public CustomerRepository(IDbConnection connection)
    {
        _connection = connection;
    }

Probably you want to use the repository in this way in many scenarios and with your current implementation you lose the transaction scope.

    public void DoWork()
    {
        using (var connection = ...)
        {
            //begin transaction
            var repository = new CustomerRepository(connection);
            var otherRepository = new OtherRepository(connection)
            var customer = repository.GetById(2);
            //do some stuff
            otherRepository.Update()...
            repository.Delete(customer);
            //commit transaction
        }
    }

This can be combined with IoC container.


Find the smallest triangular number with more than 500 factors I wrote a program about triangular numbers, which are derived from the sum of natural numbers one after the other. For example, the third triangular number is equal to: 1 + 2 + 3 = 6.
I want to find divisors of triangular numbers. for example the third triangular number have 4 divisors which equal to : 1,2,3,6.
Some triangular number with their divisors:

1: 1
3: 1,3
6: 1,2,3,6
10: 1,2,5,10
15: 1,3,5,15
21: 1,3,7,21
28: 1,2,4,7,14,28

I want to find the smallest triangular number which has more than 500 divisors.  This program works, but is too slow to find the 500th.  How can I optimize my program?
    def triangular(n):
        a= []
        for i in range (2,n):
            if n % i == 0 :
            a.append(i)
        if len(a) > 498:
            return True
        else :
            a.clear()
            return False
    x=1
    n=x
    while x>0:
        if triangular(n):
            print(n)
            break
        else:
            n+=x+1
            x+=1

Instead of
        if len(a) > 498:
            return True
        else :
            a.clear()
            return False

you could just
        return len(a) > 498

as there's no point in clearing a.
The n-th triangle number is \$\frac{n(n+1)}{2}\$, so you could factor both n and n+1 and combine their factorizations.
Takes less than a second:
from collections import Counter
from math import prod
from functools import cache

@cache
def factorize(n):
    factorization = Counter()
    d = 2
    while d * d <= n:
        while n % d == 0:
            factorization[d] += 1
            n //= d
        d += 1
    if n > 1:
        factorization[n] += 1
    return factorization

n = triangle = 0
while True:
    n += 1
    triangle += n
    factorization = factorize(n) + factorize(n + 1) - factorize(2)
    divisors = prod(power + 1 for power in factorization.values())
    if divisors > 500:
        print(triangle, divisors)
        break


Requesting Resources Until Exhaustion When an API has a 'next' feature, I use the following pattern to obtain all the results:

/** Obtains a list of channels followed by a user */
function get_follows(username, callback) {
    var channels = [];
    function handler(data) {
        if (!data || data.follows.length == 0) {
            callback(channels);
            return;
        }
        for (var i = 0; i < data.follows.length; i++)
            channels.push(data.follows[i].channel);
        JSON.load(data._links.next, handler);
    }
    var url = "https://api.twitch.tv/kraken/users/"+username+"/follows/channels?limit=75&offset=0&on_site=1";
    JSON.load(url, handler);
}

Is there a better way to go about this?

I like your code, and would use it myself, some thoughts:

I assume you declare url next to JSON.load because you want to declare it close to where you use it. I would still recommend using 1 comma separated list of variables on the top.
lowerCamelCase is good for you, get_follows -> getFollows -> getFollowedChannels ?
Comparing to 0 should be done with ===, if you do it for the length of an array I would go with a falsey comparison: if (!data || !data.follows.length) {

From a design perspective, you are circumventing the paging system, are you sure you always need every single channel? I hope you at least cache the retrieved information.


Py3 ebay-scraper - Project I need cheap GPU I need cheap GPU's and decided to make a little ebay-scraper to make sure I can buy it at cheap prices. It works and I can search for (any) product to scrape the Title, Price and Shipping cost of that product.

But I feel like I haven't done it in a generic way, hence the need to post it here. Thus my question is, could this have been done more generic?

Code

from bs4 import BeautifulSoup
from urllib.request import urlopen

def find_next_page(soup):
    '''
    Finds next page

    Returns:
        Next page link if exists
        None if next page does not exists
    '''
    next_page = soup.find('a', {'class': 'gspr next'})
    try:
        return next_page['href']
    except:
        return None

def scrape_page(soup, idx):
    '''
    Scrape page for products and save them as a dictionary

    Returns:
        A dictionary of products
    '''
    products = {}
    prod_idx = idx

    for ultag in soup.find_all('ul', {'id': 'ListViewInner'}):

        for litag in ultag.find_all('li'):
            title = litag.find('a', {'class': 'vip'})
            if not title is None:
                products[prod_idx] = {}
                title_text = title.text
                if 'New listing' in title_text:
                    title_text = title_text.replace('New listing', '').lstrip()
                title_text.strip()
                products[prod_idx]['Title'] = title_text

            ul = litag.find('ul', {'class': 'lvprices left space-zero'})
            if not ul is None:
                for li in ul.find_all('li'):
                    if '$' in li.text and not 'shipping' in li.text.lower():
                        products[prod_idx]['Price'] = li.text.split()[0]
                    if 'shipping' in li.text.lower():
                        products[prod_idx]['Shipping'] = li.text.strip()

                prod_idx += 1
    return products, prod_idx

def table_print(products):
    '''
    Prints products in nice human-readable format
    '''
    print ("{:<8} {:<100} {:<15} {:<30}".format('Key', 'Product', 'Price', 'Shipping'))
    for k, v in products.items():
        try:
            t, p, s = v
            print ('{:<8} {:<100} {:<15} {:<30}'.format(k, products[k][t], products[k][p], products[k][s]))
        except ValueError:
            continue

def scrape_product(ebay_page):
    '''
    Main scraper
    '''
    products = {}
    idx = 0

    while not ebay_page is None:
        html_doc = urlopen(ebay_page)
        soup = BeautifulSoup(html_doc, 'html.parser')
        prod, idx = scrape_page(soup, idx)
        products.update(prod)
        ebay_page = find_next_page(soup)

    return products

def make_search_string(keywords):
    '''
    Make ebay search products string
    '''
    base = 'https://www.ebay.com/sch/i.html?_from=R40&_trksid=m570.l1313&_nkw='
    end = '&_sacat=0'
    return '{0}{1}{2}'.format(base, '+'.join(keywords), end)

if __name__ == '__main__':
    print ('Project I Need a Cheap GPU (Ebay-Scraper in Python3) \n@Ludisposed \n')
    keywords = input('What do you want to search ebay for? ').split()

    # Make search string
    ebay_page = make_search_string(keywords)

    # Find all products
    products = scrape_product(ebay_page)

    # Print all products
    table_print(products)

Example

Project I Need a Cheap GPU (Ebay-Scraper in Python3)
@Ludisposed

What do you want to search ebay for? amd 580
Key      Product                                                                                Price           Shipping
0        XFX AMD Radeon RX 580 8GB GDDR5 GTR Black Edition PCI Express 3.0 New Sealed                         $305.00         +$37.83 shipping
1        MSI AMD Radeon RX 580 GAMING X 4G GDDR5 DVI/2HDMI/2Displayport PCI-Express Video                     $336.99         +$46.19 shipping
etc....

There are multiple things to improve, let's try to categorize them.

Code Style

there is a number of PEP8 code style violations, specifically:

documentations strings need to be enclosed into triple double quotes, start with a capital letter and end with a dot (reference)
group imports properly

handling a bare exception with except: is considered a bad practice
when you check for a substring to be present in a string, you can use not in, e.g. not 'shipping' in li.text.lower() can be replaced with a more readable:

'shipping' not in li.text.lower()

I would also improve the way you format the search URL, extracting the base url as a constant and using str.format() to insert keywords into the URL:

BASE_URL_TEMPLATE = 'https://www.ebay.com/sch/i.html?_from=R40&_trksid=m570.l1313&_nkw={keywords}&_sacat=0'

def make_search_string(keywords):
    """Constructs a search products string."""
    return BASE_URL_TEMPLATE.format(keywords='+'.join(keywords))

Web-Scraping and HTML Parsing

if you would switch to requests instead of urllib, you then can seriously improve on the performance of making the requests by re-using a session:

  ..if you're making several requests to the same host, the underlying TCP connection will be reused, which can result in a significant performance increase.

lxml if used instead of html.parser can provide an HTML parsing performance boost as well
see if using a SoupStrainer to focus your HTML parser on the desired part of the HTML tree only is also applicable in your scenario. It can further speed up the HTML parsing step
using a LBYL approach inside the find_next_page would be a bit more readable and concise:

def find_next_page(soup):
    """
    Finds next page.

    Returns:
        Next page link if exists
        None if next page does not exists
    """
    next_page_link = soup.select_one('a.next')
    return next_page_link['href'] if next_page_link else None

scrape_page() function is overly complicated. For instance, there is a single element with id="ListViewInner" on the page and it does not require a loop to locate this element. And, you can do better in locating elements on the page:

def scrape_page(soup, index):
    """
    Scrapes page for products and save them as a dictionary.

    Returns:
        A dictionary of products
    """
    products = {}

    for produce_index, item in enumerate(soup.select('#ResultSetItems li'), index):
        title = item.select_one('a.vip')
        price = item.select_one(".prc")
        shipping_price = item.select_one('.ship')

        products[produce_index] = {
            'Title': title.find(text=True, recursive=False) if title else None,
            'Price': price.get_text(strip=True) if price else None,
            'Shipping': shipping_price.get_text(strip=True).split()[0] if shipping_price else None
        }

    return products, produce_index

(not tested)

Also note the use of enumerate() for better handling enumeration (thanks @graipher for the idea).


SOAP request in a while loop I am developing a service where the user need to sign the seller agreement with Mobile BankID (Swedish e-sign service). To make this work I need to make SOAP request to get the progress status of the signing request in Mobile BankID.

Is this code understandable? How can I make it more clean?

def collect_request(transaction_id):
    '''You can make a delay for 9 seconds before you start to call the Collect
    method. After 9s you begin to make CollectRequest with a 3 seconds delay
    between each collect. First answer will probably be
    OUTSTANDING_TRANSACTION, then USER_SIGN and at last COMPLETE. (After six
    OUTSTANDING_TRANSACTION you get NO_CLIENT, just to indicate that the user
    not yet has started her BankID client.

    After 180 seconds, you will at last get faultStatus EXPIRED_TRANSACTION'''
    time.sleep(9)
    seller_agreement = SellerAgreement.objects.get(id=transaction_id)
    url = '<WSDL URL HERE>'
    client = Client(url)

    val = 'OUTSTANDING_TRANSACTION'
    while True:
        try:
            response = client.service.Collect(policy='logtest001',
                                              provider='bankid',
                                              displayName='Test av Mobilt BankID',
                                              transactionId=transaction_id,
                                              orderRef=seller_agreement.order_ref)
            print response
            if response.progressStatus != val:
                trigger_new_sign_event(seller_agreement.user.id, response.progressStatus)
                val = response.progressStatus
            if (response.progressStatus == 'OUTSTANDING_TRANSACTION' or response.progressStatus == 'USER_SIGN' or response.progressStatus == 'NO_CLIENT'):
                # Try again in 3 seconds
                time.sleep(3)
                continue
            if response.progressStatus == 'COMPLETE':
                # TODO: Save to db
                print "Done!"
            break
        except WebFault, f:
            print f  # Django log?
            trigger_new_sign_event(seller_agreement.user.id, f.fault.faultstring)
            # TODO: cancelled_at?
            break

This docstring is very long and confusing. The official Python style guide recommends that you have a short one line summary of what the function does before launching into longer, detailed paragraphs.

def collect_request(transaction_id):
    '''Waits while collecting a mobile bank ID request.

    You can make a delay for 9 seconds before you start to call the Collect
    method. After 9s you begin to make CollectRequest with a 3 seconds delay
    between each collect. First answer will probably be
    OUTSTANDING_TRANSACTION, then USER_SIGN and at last COMPLETE. (After six
    OUTSTANDING_TRANSACTION you get NO_CLIENT, just to indicate that the user
    not yet has started her BankID client.

    After 180 seconds, you will at last get faultStatus EXPIRED_TRANSACTION'''

I'm not sure I got it quite right, so I'm sure you can come up with a better one. Also there's not much need for using url as a variable directly before it's used 'when you can pass it directly to Client. Though it should either be a constant defined elsewhere or a user provided parameter. There's a lot of values like that in here. Making something a constant makes it clearer what the value means, where it comes from and what it's for.

WSDL_URL = "some url"
PROVIDER = "bankid"

etc.

Your third if should actually be an elif, since the second and third conditions are mutually exclusive. You should then remove the continue and instead manually call break from inside your if response.progressStatus == 'COMPLETE' block. It makes more sense that way, that your loop breaks when this condition is fulfilled, rather than the continue being called in certain other cases. Unless you want to break even if any unhandled progressStatus value is received. ie. As it stands, if progressStatus is anything but 'OUTSTANDING_TRANSACTION', 'USER_SIGN' or 'NO_CLIENT' then the loop breaks. If that is intentional, then you should comment as such.

Also don't use the old syntax for exceptions, it's ambiguous and explicitly doesn't work in Python 3. You should use except WebFault as f. The reason it's ambiguous is that it doesn't allow you to do multiple exceptions properly, imagine you had to try do except WebFault, ValueError, f. It wouldn't make any sense, however you could do except (WebFault, ValueError) as f perfectly fine.


Basic beginner's matrix class in Python This is my first class. I've been working a while on it, some functions are future ideas and are right now just bare bones and intended to be worked on later. I'm really looking for a good review of what I have so far, I've never done this before so hopefully I'm off to a decent start. I tried to log my errors as well and handle them by skipping but annotating which values are trouble. Also first time using *args in my functions.

class Matrix():

    def __init__(self, height, width):
        self.rows = [[0]*width for i in range(height)]
        self.height = height
        self.width = width

    def __str__(self):
        s = "\n" + "\n".join([str(i) for i in [rows for rows in self.rows] ]) + "\n"
        return s

    def __repr__(self):
        return (f'{self.__class__.__name__} ({self.height!r} , {self.width!r})')

    def len(self):
        return self.height * self.width

    def __add__(self, matrix2):
        return

    def __mul__(self, matrix2):
        return

    def remove(self, item):
        return

    def fill_matrix(self, fill_list):
        index = 0
        for i in range(len(self.rows)):
            try:
                for j in range(len(self.rows[i])):
                    self.rows[i][j] = fill_list[index]
                    index += 1
            except IndexError:
                print (f"Matrix not filled \nMatrix fill stopped at: row {i}, Column {j}")
                break
        return fill_list[index:]

    def add_value(self, *args):
        log = []
        for arg in args:
           try:
               arg_size = len(arg)
           except TypeError:
               log.append(f'Parameter must be sequence, skipped: {arg}')
               continue

           try:
               if arg_size == 3:
                       self.rows[arg[0]] [arg[1]] = arg[2]
               else:
                   log.append(f'Parameter has too little or too much data, skipped: {arg}')
           except  IndexError:
               log.append(f'Location parameters are out of range, skipped: {arg}')
           except TypeError:
               log.append(f'Location indicies must contain integral types, skipped: {arg}')

        return log

myMat = Matrix(5,5)
overflow = myMat.fill_matrix([i for i in range(26)])

print(myMat)

Errors =  myMat.add_value((-1,3,500), (0,0,3),(51,5, 7), (1, 2, 667), [3,4,676], (1), (1,"a", 1), (1,1, "£"))
print(myMat)

There is no need for brackets in class Matrix(): .... It can be just class Matrix: .... See docs for class definition syntax.
In self.rows = [[0]*width for i in range(height)] you should replace i by _ that we usually use for throwaway variables.
In the same line 0 is a magic number. You could make it a default parameter:

def __init__(self, height, width, fill_value=0):
    self.rows = [[fill_value] * width for _ in range(height)]

Regarding __str__:

def __str__(self):
    s = "\n" + "\n".join([str(i) for i in [rows for rows in self.rows] ]) + "\n"
    return s

Having those "\n" on both ends seems unnecessary. Just return the middle part and let user decide if he wants more blank lines between output.
[rows for rows in self.rows] is the same as self.rows. That leaves us:

def __str__(self):
    s = "\n".join([str(i) for i in self.rows])
    return s

i is a bad name. row is better.
Removing unnecessary s:

def __str__(self):
    return "\n".join([str(row) for row in self.rows])

And making it more concise:

def __str__(self):
    return "\n".join(map(str, self.rows))

Regarding __repr__:

def __repr__(self):
    return (f'{self.__class__.__name__} ({self.height!r} , {self.width!r})')

The brackets are unnecessary here, and spacing is a bit weird. It should be like this:

return f'{self.__class__.__name__}({self.height!r}, {self.width!r})'

As you probably already know, with __repr__ you should be able to pass the returned string to Python interpreter so that it could recreate the object. But in your case information about the values in your matrix would be lost. What you return is just something like Matrix(5, 5). Would this be OK? Probably it would be better if the values would be returned as well, but then the current logic of the class won't allow you to recreate the matrix as you separated filling (fill_matrix) from initializing. Maybe it would be better to be able to do both at the same time?

len method should be called __len__.
As with current implementation its value is constant, you could use lru_cache(1) so to calculate it only once and then reuse the cached value:

from functools import lru_cache

...

@lru_cache(1)
def __len__(self) -> int:
    return self.height * self.width

You have some not implemented yet methods. Instead of returning nothing:

def __add__(self, matrix2):
    return

raise a NotImplementedError:

def __add__(self, matrix2):
    raise NotImplementedError

Regarding fill_matrix:

def fill_matrix(self, fill_list):
    index = 0
    for i in range(len(self.rows)):
        try:
            for j in range(len(self.rows[i])):
                self.rows[i][j] = fill_list[index]
                index += 1
        except IndexError:
            print (f"Matrix not filled \nMatrix fill stopped at: row {i}, Column {j}")
            break
    return fill_list[index:]

Add a docstring. It is not clear at the first sight why this method should return anything. Something like:

def fill_matrix(self, fill_list):
    """
    Fills matrix by `fill_list`
    and returns what didn't fit in it.
    """

You are catching IndexError but this case seems impossible as indices are always taken from the ranges defined by shape of the matrix. You can remove it.
Why recalculating len(self.rows) and len(self.rows[i]) if you already have self.height and self.width?
The idea of iterating the index doesn't seem Pythonic. How about something like this instead:

from itertools import product

...

def fill_matrix(self, fill_list):
    fill_values = iter(fill_list)

    indices = product(range(self.height), range(self.width))

    for (i, j), value in zip(indices, fill_values):
        self.rows[i][j] = value

    return fill_values

This can accept iterators or sequences and will return an iterator with "overflow". So if you need to use those leftovers, you get them like this:

myMat = Matrix(5, 5)
overflow = myMat.fill(range(30))
print(list(overflow))
>>> [25, 26, 27, 28, 29]

It would be better to rename fill_matrix as fill and its argument fill_list as values.

Regarding add_value. I think it's better to name it replace instead.
This method is too complicated and doesn't follow Single responsibility principle. Catch the errors outside, and use this method just for replacing one value. If you need to replace multiple values, then put the function call inside the loop. See this post for a better explanation. In the end it should be as simple as that:

def insert(self, row_index, column_index, value):
    self.rows[row_index][column_index] = value

And you could call it, and catch and save the errors like this:

import traceback

...

values = [(-1, 3, 500),
          (0, 0, 3),
          (51, 5, 7),
          (1, 2, 667),
          [3, 4, 676],
          (1),
          [3, 4, 676, 123],
          (1, "a", 1),
          (1, 1, "£")]

errors = []
bad_values = []

for value in values:
    try:
        myMat.insert(*value)
    except (IndexError, TypeError):
        errors.append(traceback.format_exc())
        bad_values.append(value)

for error, value in zip(errors, bad_values):
    print(f'Error occurred for value {value}:\n'
          f'{error}')

And the output is:

Error occurred for value (51, 5, 7):
Traceback (most recent call last):
  File "C:/Users/Georgy/SO_CR/main.py", line 82, in <module>
    myMat.insert(*value)
  File "C:/Users/Georgy/SO_CR/main.py", line 55, in insert
    self.rows[row_index][column_index] = value
IndexError: list index out of range

Error occurred for value 1:
Traceback (most recent call last):
  File "C:/Users/Georgy/SO_CR/main.py", line 82, in <module>
    myMat.insert(*value)
TypeError: insert() argument after * must be an iterable, not int

Error occurred for value [3, 4, 676, 123]:
Traceback (most recent call last):
  File "C:/Users/Georgy/SO_CR/main.py", line 82, in <module>
    myMat.insert(*value)
TypeError: insert() takes 4 positional arguments but 5 were given

Error occurred for value (1, 'a', 1):
Traceback (most recent call last):
  File "C:/Users/Georgy/SO_CR/main.py", line 82, in <module>
    myMat.insert(*value)
  File "C:/Users/Georgy/SO_CR/main.py", line 55, in insert
    self.rows[row_index][column_index] = value
TypeError: list indices must be integers or slices, not str

You might be interested in Python logging module as well.
Though, I find it strange that you decided to pay so much attention to catching the errors.

Some other notes:

Don't forget about naming conventions. myMat should be my_matrix, Errors should be errors. See PEP 8: for more.
Consider using type hints.

In the end your code could look like this:

from functools import lru_cache
from itertools import product
from typing import (Any,
                    Iterable,
                    Iterator)

class Matrix:
    def __init__(self,
                 height: int,
                 width: int,
                 fill_value: Any = 0) -> None:
        self.height = height
        self.width = width
        self.rows = [[fill_value] * width for _ in range(height)]

    def __str__(self) -> str:
        return "\n".join(map(str, self.rows))

    def __repr__(self) -> str:
        return f'{self.__class__.__name__}({self.height!r}, {self.width!r})'

    @lru_cache(1)
    def __len__(self) -> int:
        return self.height * self.width

    def __add__(self, matrix2):
        raise NotImplementedError

    def __mul__(self, matrix2):
        raise NotImplementedError

    def remove(self, item):
        raise NotImplementedError

    def fill(self, values: Iterable[Any]) -> Iterator[Any]:
        """
        Fills matrix by `fill_values`
        and returns what didn't fit in it.
        """
        values = iter(values)

        indices = product(range(self.height), range(self.width))

        for (i, j), value in zip(indices, values):
            self.rows[i][j] = value

        return values

    def replace(self,
                row_index: int,
                column_index: int,
                value: Any) -> None:
        """Replaces a value by given indices by a new one."""
        self.rows[row_index][column_index] = value


jQuery Toggle Animation When you click open - a div slides out. If you click on the new div, an additional div slides out from under it. This is working great, but I need two things that I cant figure out!

How can I condense the jQuery so I don't have to add a class to it every time I want a new slider? Is it possible to do a .sibling kind of thing, or something like this?
I've tried making a close button, but I cannot get it to work the way I want. When you click close, I want the bottom div to close first, then the slide out div to close. And this button would need to work even if only one div is open.

jsFiddle

$(document).ready(function() {
   $('.cambridge').hide();
   $("#test").click(function () {
      $(".cambridge").toggle("slide", { direction: "right" }, 1000);
   });
   $('.shopping').hide();
   $("#test2").click(function () {
      $(".shopping").toggle("slide", { direction: "up" }, 1000);
   });
});

A quick review:

JSHint.com

Your code passes all checks, well done

Naming

test and test2 are unfortunate names for elements, I am sure you can come up with something better

Counter Proposal

Looking at the code, there is definitely repetition in making those sliders, you can extract what is common in to a function, and then use that function for any future sliders:

(This is blatantly stolen/modified from the deleted answer):

$(document).ready(function() {

   function registerSlider( buttonId, sliderClass, direction){
     $(sliderClass).hide();
     $(buttonId).click(function () {
       $(sliderClass).toggle("slide", { direction: direction }, 1000);
     });
   }
   registerSlider("#test", '.cambridge', 'right' );
   registerSlider("#test2", '.shopping', 'up' );

});

Your questions

I showed in my counter proposal how you can condense the code, but I do believe you will need each time a distinct class
Finding the answer to that question is not trivial (we would need a working sample), and not something codereview does.


Applying a set of styles both with a media query and with nested rules I am working on an application which responds to different screen resolutions. At a certain minimum DPI, we want to apply a set of styles. We also want to apply that same set of styles if the class hidpi is present on the root html element. Our project uses LESS.

This is the code I inherited. I've replaced the styles with some placeholders for the sake of this question (so don't mind the colors):

@media all and (min-resolution: 96dpi) {
  .foo {
    color: "maize";
  }
  .bar {
    color: "blue";
  }
}

html.hidpi {
  .foo {
    color: "maize";
  }
  .bar {
    color: "blue";
  }
}

Attempt at reducing duplication

As you can see, the code between the media query and html.hidpi is exactly the same, so I figured I should be able to reduce duplication. I came up with a solution using a mixin:

.mixinHidpi() {
  .foo {
    color: "maize";
  }
  .bar {
    color: "blue";
  }
}

@media all and (min-resolution: 96dpi) {
  .mixinHidpi();
}

html.hidpi {
  .mixinHidpi();
}

Can any more be done to reduce duplication? Are there any problems with the way I'm doing things now? In particular, I've only ever seen very simple examples of using mixins, so I'm not sure if I'm using them in the way they are supposed to be used.

You usage of the mixin will work and although it is not pretty there is not much else that can be done as far as valid LESS / CSS output.

I've written up an example of complex mixins like below.
LESS lazy mixin for element width/height/float

As well as one that might be on track with what you are trying to accomplish.
https://stackoverflow.com/questions/39338429/less-outputs-extra-semicolon

Keep in mind that this using a JavaScript array in a hacky type mannor and the output would still need to be sanitized of the extrenious code that is not valid. But I think we are both headed in the same direction.


Deleting a Specific Node from a Linked List My go at creating a function to remove a specific node from a linked list:

void deleteSpecific(DataType N, Node *&H, Node *&T)
{
  if (H == nullptr) // 1. Check Whether List Is Empty (head == NULL)
  { // If It's Empty Then, Display The Following And Terminate The Function.
    std::cout << std::endl << std::endl;
    std::cout << "\t" << "-> LL| ERR: The List Is Empty. Deletion Not Possible.";
    std::cout << std::endl << std::endl;
    return;
  }
  Node *TMP1 = H, *TMP2; // If It's Not Empty Then, Define Two Node Pointers Temp1 And Temp2 And Put Head In Temp1 Only.
  while (TMP1->data != N) // Keep Moving Temp1 Until it reaches to exact node to delete
  {
    if (TMP1->next == nullptr) // If Node is not found in list then display the message and exit the loop and function.
    {
      std::cout << std::endl << std::endl;
      std::cout << "\t" << "-> LL| ERR: Given Node Not Found. Deletion Not Possible.";
      std::cout << std::endl << std::endl;
      return;
    }
    TMP2 = TMP1;
    TMP1 = TMP1->next;
  }
  if (H->next == nullptr)
  {
    H = nullptr;
    delete TMP1;
  }
  else if (TMP1 == H)
  {
    H = H->next;
    delete TMP1;
  }
  else if (TMP1->next == nullptr)
  {
    TMP2->next = nullptr;
    T = TMP2;
    delete TMP1;
  }
  else
  {
    TMP2->next = TMP1->next;
    delete TMP1;
  }
}

Is The logic of the code correct? I've tried it with multiple inputs and it works, however are there any special cases that you can notice that'll not work with this?

Code Review
Removing the Element:
Your four choices do seem to cover all the situations.
  if (H->next == nullptr)
  {
    H = nullptr;
    delete TMP1;
  }
  else if (TMP1 == H)
  {
    H = H->next;
    delete TMP1;
  }
  else if (TMP1->next == nullptr)
  {
    TMP2->next = nullptr;
    T = TMP2;
    delete TMP1;
  }
  else
  {
    TMP2->next = TMP1->next;
    delete TMP1;
  }

But in reality there are only two situations. You are deleting the head node. You are deleting another node. The other two are special cases of deleting the tail and we should handle that separately.
  Node* TMP1 = H;
  Node* TMP2 = nullptr;   // set a value for TMP2 so we can check it/

  // Find node to delete

  // Check if the item being removed is the tail
  // and update the tail appropriately.
  if (T == TMP1) {
      T = (T == H) ? nullptr : TMP2;
  }

  // Now update the list.
  if (TMP2 == nullptr) {       // If TMP2 is null this is the head
      H = TMP1->next;          // Node we are deleting. So reset H
  }
  else {
      TMP2->next = TMP1->next; // Otherwise TMP1 is being deleted and
                               // TMP2 is the previous node.
  }

  // Now the node is unlinked delete it.
  delete TMP1;

You can simplify this even more by using an extra helper function.
Objects
Why is this a free standing function?
void deleteSpecific(DataType N, Node *&H, Node *&T)

I would expect this to be part of a class. Where head and tail are part of the object. So you don't need to pass them around.
Separation of Concerns
Your code should either be resource management or business logic. Your code mixes the two different types of code together. Here the resource management is removing the node from the list. But you also have some business logic inside your code that is talking to the user.
std::cout << std::endl << std::endl;
std::cout << "\t" << "-> LL| ERR: The List Is Empty. Deletion Not Possible.";
std::cout << std::endl << std::endl;

Is it really an error in the first place? But why is it part of the resource management code?
The resource management code should be wrapped in another layer of code that does the business logic and informs the user. That way you can re-use the deleteSpecific() function from another piece of code that does not care if the data is in the list or not.
Style Comments
Naming
Variable names are supposed to be self documenting.
 H:   head
 T:   tail

That's no so self documenting as you think. Just spell it out. There are a couple of other conventions you need to keep in mind.

Identifiers that are all caps are usually reserved for macros.
Identifiers with an initial cap are usually user defined "Types"
Identifiers with an initial lower case letter are usually objects.

These are good conventions to follow and will help you spot things as your code gets more complicated.
Pointer & Reference
In "C" the "*" was usually placed by the object. But "C++" is much more strongly types and the "*" and "&" are considered part of the type information so you usually place these with the type.
void deleteSpecific(DataType N, Node *&H, Node *&T)

Is more usually written as:
void deleteSpecific(DataType N, Node*& H, Node*& T)

But you say "AHhhh, but what about"
Node *TMP1 = H, *TMP2;

Yes it does not work in this situation. But there is a more important rule recommendation that makes this situation obsolete. The rule is to declare one variable per line. So this should actually be written as:
Node*  TMP1 = H;
Node*  TMP2 = nullptr;

Please stop writing C++ like it was C. These are completely different languages with their own styles and idioms.
Prefer "\n" over std::endl
The difference between the two is that endl flushes the buffer. Usually this is not what you want and makes the code exceedingly inefficient (user flushing is almost always wrong). If you think you need it then add an explicit flush.


Random Forest Code Optimization I am new to Python. I have built a model with randomforest in python. But I think my code is not optimized. Please look into my code and suggest if I have deviated from best practices.

Overview about the data I have:

The data has response columns and predictor columns. Also there is a column 'TestOrTrainingDataRandom' which specifies the test and training data.(There are also columns like index, Timestamp,etc which have to be removed) The predictor columns start with '3000' and ends at '3680' with a step increase of 5 (i.e in total there are 137 predictor columns) But there are some predictor columns missing. So the missing predictor columns has been interpolated.

Here is my code. Please give the feedback.

from sklearn.ensemble import RandomForestClassifier   #Package for random forest classification
import pandas as pd
from sklearn.metrics import classification_report,confusion_matrix

data = pd.read_csv("combined_spectra_and_gas_params_3.csv") #Reading the file

#creating response column

data["response"] = None
s = pd.Series(["verylow","low","medium","high","veryhigh"], dtype="category")
n=0

for row in data["H2S"]:
    if row < 120:
        data['response'].iloc[n] = s[0] #Assign 'verylow' if H2S concentration is less than 120 and do indexing
        n=n+1
    elif row >=120 and row <500:
        data['response'].iloc[n] = s[1] #Assign 'low' if H2S concentration is between 120 and 500 and also do indexing
        n=n+1
    elif row >=500 and row <1000:
        data['response'].iloc[n] = s[2] #Assign 'medium' if H2S concentration is between 500 and 1000 and also do indexing
        n=n+1
    elif row >=1000 and row <1300:
        data['response'].iloc[n] = s[3] #Assign 'high' if H2S concentration is between 1000 and 1300 and also do indexing
        n=n+1
    else:
        data['response'].iloc[n] = s[4] #Assign 'veryhigh' if H2S concentration is greater than 1300 and do indexing
        n=n+1

#create the training & test sets
b=len(data)
a=data.TestOrTrainingDataRandom [data.TestOrTrainingDataRandom == 1].count() #Count the number of training data

new_data=data.drop(data.columns[[0,1,2,3,4,5,6,7,8,121,120,119,118]], axis=1)

colnames=list(new_data)
len_column = len(new_data.columns)
len_iteration=len_column-1
j=3000;i=0;k=0

new_col=pd.DataFrame(index=range(0,b),columns=['temp'])

# To insert missing columns
while i < len_iteration:
    if int(colnames[i])== j:
        i=i+1
        j=j+5;

    else:
        for k in range(0,b):
            new_col.iloc[k]=(new_data.iloc[k,i-1]+new_data.iloc[k,i+1])/2
        new_data.insert(i,str(j),new_col)
        colnames=list(new_data)
        j=j+5
        i=i+1
        len_iteration=len_iteration+1;

new_data.insert(1,"dataselection_col",data['TestOrTrainingDataRandom'])
new_data.insert(1,'response',data['response'])

train = pd.DataFrame(index=range(0,a),columns=list(new_data))   #Creating dataframe for training data
test = pd.DataFrame(index=range(0,abs(a-b)),columns=list(new_data)) #Creating dataframe for test data

m=0;n=0;p=0;
for value in new_data['dataselection_col']:
    if value == 1 :

        train.iloc[m]=new_data.irow(n)  #If 'TestOrTrainingDataRandom' column has 1, then append that row data to train and also do indexing
        n=n+1
        m=m+1
    else:
        test.iloc[p]=new_data.irow(n)   #If 'TestOrTrainingDataRandom' column has other than 0, then append that row data to test and also do indexing
        p=p+1
        n=n+1

trainRes = train['response']    #Response column
Actuals=test['response']         #Actuals
new_train=train.drop(train.columns[[1,2]],axis=1)
colnames=list(new_train)
trainArr = train.as_matrix(colnames) #Convert dataframe into array matrix representation

#For random forest
rf = RandomForestClassifier(n_estimators=100) #Random forest generation for Classification
rf.fit(trainArr, trainRes) #Fit the random forest model

testArr = test.as_matrix(colnames)

results = rf.predict(testArr)
print 'confusion matrix\n', confusion_matrix(Actuals, results)

I have notes on style and redundancies, but you should read the Python Style Guide: PEP0008. It has a lot of good info on how to format your code to be clear and readable for yourself and others. I'll miss pointing out some of it's recommendations so do read it too.

You have a lot of unnecessary inline comments. from package import Class makes it pretty clear that you're importing Class from package. Python is designed to be readable so that you don't need to explicitly tell people these things. I'd remove most of them. In particular, with your long if and elif chains it's be better to cut down on repetition. Both by having only one comment at the top, and by putting n=n+1 at the end. Though, you can rewrite that as n+=1, and you should put spaces around operators, makes them easier to read:

for row in data["H2S"]:
    #Assign response based on H2S concentration and do indexing
    if row < 120:
        data['response'].iloc[n] = s[0]
    elif row >= 120 and row < 500:
        data['response'].iloc[n] = s[1]
    elif row >= 500 and row < 1000:
        data['response'].iloc[n] = s[2]
    elif row >= 1000 and row < 1300:
        data['response'].iloc[n] = s[3]
    else:
        data['response'].iloc[n] = s[4]
    n += 1

But actually you could just use enumerate for your for loop instead. It allows you to do the same as what you're using n for, it contains the number iteration you're on. So you can save a line and just do this:

for n, row in enumerate(data["H2S"]):
    #Assign response based on H2S concentration and do indexing
    if row < 120:
        data['response'].iloc[n] = s[0]

    ...

    else:
        data['response'].iloc[n] = s[4]

No need for the n += 1 any more.

You never use len_column except to assign to len_iteration, so just assign directly to len_iteration:

len_iteration = len(new_data.columns) - 1

You shouldn't use the semicolon line separator, it's rarely a good idea and often just looks unPythonic. Anyway, Python lets you assign multiple values at once by comma separating them:

j, i, k = 3000, 0, 0

Having them out of alphabetical order is a bit confusing though. You'd be better giving these meaningful names since their usage isn't simple.

In you're while loop you increment i and j in both conditions, so you should just do that at the end and just run the middle block if your condition is False:

while i < len_iteration:
    if int(colnames[i]) != j:
        for k in range(0, b):
            new_col.iloc[k] = (new_data.iloc[k,i-1]+new_data.iloc[k,i+1])/2
        new_data.insert(i,str(j),new_col)
        colnames = list(new_data)
        len_iteration += 1;
    j += 5
    i += 1

That block could really do with some comments though. Especially this line:

new_col.iloc[k] = (new_data.iloc[k, i - 1] + new_data.iloc[k, i + 1]) / 2

I have no idea what that's doing.

You also use m, n and p for indexing. If you're not going to give them more meaningful names, you can at least re-use i, j and k which are more commonly used for looping over indices.


Python example to save file with same name like `somefile(n).txt` My little Python scriptie needs to save a file to the CWD, but sometimes the file already exists.

I found this short and clever example to string concatenate a time signature to the name here on SE. But I wanted to make the file name look pretty,

import os
import fnmatch
import re

somefiles = []
myfiles = []
root = "./"
onefn = False

for item in os.listdir(root):
    if os.path.isfile(os.path.join(root,item)):
        somefiles.append(item)

for fname in somefiles:
    if fnmatch.fnmatch(fname, 'somefile(*).txt'):
        myfiles.append(fname)
    elif fname == 'somefile.txt':
        onefn = True

if len(myfiles) > 0:
    maxnum = re.search(r'\(([0-9]+)\)', max(myfiles))
    with open('somefile(%s).txt' % (str(int(maxnum.group(1)) + 1),), 'w') as f:
        f.write("This is some text in a new file.")
elif onefn:
    with open('somefile(1).txt', 'w') as f:
        f.write("This is sommore text in a new file.")
else:
    with open('somefile.txt', 'w') as f:
        f.write("This is enuf text in a new file.")

This is a beast, but it will write somefile(n).txt.
Any recommendations to make it less beastly?

One aspect I'd like to mention is sorting:

The contain does a max(myfiles), which is on the filenames.

In particular, out of "somefile(9).txt" and "somefile(10).txt", the max will be "somefile(9).txt". It will then choose "somefile(10).txt" as the new filename, overwriting its original content.

This is because 9 is larger than 1.


Remove redundancies in similar properties I have a lot of redundancy here, but I am not sure how could I make it look shorter.
namespace Notan
{
    public class EquipmentSystem : ScriptableObject
    {
        public UnityEvent onUpdate = new UnityEvent();

        [SerializeField] private Weapon weapon1;
        public Weapon Weapon1
        {
            get => weapon1;
            private set
            {
                weapon1 = value;
                onUpdate?.Invoke();
            }
        }

        [SerializeField] private Weapon weapon2;
        public Weapon Weapon2
        {
            get => weapon2;
            private set
            {
                weapon2 = value;
                onUpdate?.Invoke();
            }
        }

        [SerializeField] private Accessory accessory1;
        public Accessory Accessory1
        {
            get => accessory1;
            private set
            {
                accessory1 = value;
                onUpdate?.Invoke();
            }
        }

        [SerializeField] private Accessory accessory2;
        public Accessory Accessory2
        {
            get => accessory2;
            private set
            {
                accessory2 = value;
                onUpdate?.Invoke();
            }
        }

        [SerializeField] private Shield shield;
        public Shield Shield
        {
            get => shield;
            private set
            {
                shield = value;
                onUpdate?.Invoke();
            }
        }

        [SerializeField] private Armor armor;
        public Armor Armor
        {
            get => armor;
            private set
            {
                armor = value;
                onUpdate?.Invoke();
            }
        }

    }
}
```

You could make your code less redundant through the use of a custom SourceGenerator. It could work kinda like Fody works for WPF and INotifyPropertyChanged.
Sadly I have no example to provide for the SourceGenerator's implementation because I haven't had time to toy with them yet. Though the idea is to make your SourceGenerator generate the redundant code for you at compile time whenever it detects something that you want to be autogenerated.
You have two options but both are gonna require custom attribute(s) :

Make it work like a white-list : don't autogenerate by default, autogenerate only when property is tagged with your attribute. Your property (or field) would then be reduced to
[MyAutoGenerationAttribute]
public object MyProperty { get; set; }

Make it work like a black-list (like Fody does) : always autogenerate by default, don't autogenerate when property is tagged with your attribute.

I'm not really familiar with Unity but my only concern is that this solution may not be compatible with Unity's shenanigans, especially since SourceGenerators is a fairly new feature.
PS : If optimization matters I recommend you edit your setters so that the event gets raised only when the value of the property changes for real (ex: by wrapping the setter with something like if (myBackingField != value))


compression condition permutation I have 16 permutations each with 104 variables using the standard alphabet. The 104 variable permutation is selected based off of compression complexity.
This is one example of the pattern I am able to generate it has a low compression ratio of 1.3 with overhead:

  HHHFHHFFHHFHHFFHHFHHHFHAAAAHHHFHHFFHHFHHFFHHFHHHFHAAAAHHHFHHFFHHFHHFFHHFHHHFHAAAAHHHFHHFFHHFHHFFHHFHHHFH

This is a goal pattern I would like to generate it has a better compression ratio of 1.5

  mlcllltlgvalvcgvpamdipqtkqdlelpklagtwhsmamatnnislmatlkaplrvhitsllptpednleivlhrwennscvekkvlgektenpkkfkinytvaneatlldtdydnflflclqdtttpiqsmmcqylarvlveddeimqgfirafrplprhlwylldlkqmeepcrf

This is one set of permutation code generation:

static double GetCompressionRatio(string input)
{
    if (string.IsNullOrEmpty(input))
        throw new ArgumentNullException();

    MemoryStream ms = new MemoryStream();

    GZipStream gzip2 = new GZipStream(ms, CompressionMode.Compress, true);

    byte[] raw = Encoding.UTF8.GetBytes(input);
    gzip2.Write(raw, 0, raw.Length);
    gzip2.Close();

    byte[] zipped = ms.ToArray(); // as a BLOB
    int startsize = raw.Length;

    double percent = Convert.ToDouble(zipped.Length) / Convert.ToDouble(startsize);
    return percent;
}

var query =
             from sp1 in polar
                    ...
                    //22 lines
                    ...
             from vp15 in polar
             where GetCompressionRatio(sp1+...+vp15)>1.5
             select sp1+...+vp15;

foreach (var element in query)
{
    //output
}
        string[] polar = new string[6];
        polar[0]="H";
        polar[1]="Q";
        polar[2]="N";
        polar[3]="K";
        polar[4]="D";
        polar[5]="E";

        string[] nonpolar = new string[5];
        nonpolar[0]="F";
        nonpolar[1]="L";
        nonpolar[2]="I";
        nonpolar[3]="M";
        nonpolar[4]="V";

        string[] all = new string[24];
        all[0]="A";
        all[1]="B";
        all[2]="C";
        all[3]="D";
        all[4]="E";
        all[5]="F";
        all[6]="G";
        all[7]="H";
        all[8]="I";
        all[9]="J";
        all[10]="K";
        all[11]="L";
        all[12]="M";
        all[13]="N";
        all[14]="O";
        all[15]="P";
        all[16]="Q";
        all[17]="R";
        all[18]="S";
        all[19]="T";
        all[20]="U";
        all[21]="V";
        all[22]="W";
        all[23]="Y";

I need it to go much faster like 1000x if possible.

Ok.

Lots of stuff going on here and I'm writing this as I work your example so bear with me...

In you code, polar contains six letters. And you don't care about letter repetition in your top examples so what you are calculating here are not permutations but variations. Secondly, you're example shows 22 ommitted lines so you have 24 from statements in your LINQ query... that's going to generate 6^24 variations (4738381338321616896)

That's a very large number. Even if you could paralelize this across 1000 servers, they would still need to be calculating 150million compressions ratios per second for this to complete in a year. (((6^24)/1000)/365)/86400) = ~150,253,000

As soon as you call foreach (var element in query) that query is going to execute and it's going to try and create a list of 4.7 Quintillion strings in memory. This is BAD. You're attempting a brute force calculation on an impossibly large problem set so you need to go back and rethink your approach to whatever it is you are trying to solve/accomplish

But again... lets just assume that you have enough power & bandwidth & compute resource (GPUs are shiny) And lets bring it back to more manageable numbers. lets says 6^10 =

Strings in General

Strings are immutable in .NET. Everytime you alter a string, you throw away the old one, and make a new one and the garbage collector has to come along and clean up after you. so this part here will be very expensive where you concat all those strings together, twice.

where GetCompressionRatio(sp1+...+vp15)>1.5
select sp1+...+vp15;

at the very least you would want to change this to

let concatval = sp1+...+vp15
where GetCompressionRation(concatVal) > 1.5
select concatVal

Big Numbers are Big.

Using LINQ like that to generate your variations is not optimal. As I suggested in the comments... use a library. The following code snippet.

var polar = new[] { "H", "Q", "N", "K", "D", "E" };

var sw = new Stopwatch();
sw.Start();
var results1 = from a1 in polar from a2 in polar
                from a3 in polar from a4 in polar
                from a5 in polar from a6 in polar
                from a7 in polar from a8 in polar
                from a9 in polar from a10 in polar
                select a1 + a2 + a3 + a4 + a5 + a6 + a7 + a8 + a9 + a10;
var l1 = results1.Count();
sw.Stop();
Console.WriteLine("{0} variations in {1}ms", l1, sw.ElapsedMilliseconds);

sw = new Stopwatch();
sw.Start();
var results2 = new Variations<string>(polar, 10, GenerateOption.WithRepetition);
var l2 = results2.Count();
sw.Stop();
Console.WriteLine("{0} variations in {1}ms", l2, sw.ElapsedMilliseconds);

Console.ReadLine();

RESULTS

60466176 variations in 54161ms
60466176 variations in 4624ms

The Compression Method.

On my machine this is repeatedly capable of performing 100,000 compressions on a 10 character string in 2200-2300ms. With the following modifications, that comes down to 1800-1900ms. Small improvement that's possibly related to what else my machine was doing but still.

MemoryStream & GZipStream are IDisposable & should be wrapped in using() statements
removed unnecessary int assignments
removed unnecessary if(check) at top... you know this to be impossible based on your input set.

Code

static float GetCompressionRatio2(string input)
{
    using(var ms = new MemoryStream())
    using (var gzip2 = new GZipStream(ms, CompressionMode.Compress, true))
    {
        var raw = Encoding.UTF8.GetBytes(input);
        gzip2.Write(raw, 0, raw.Length);
        var percent = ((float)ms.ToArray().Length) / raw.Length;
        return percent;
    }
}


Given a function that returns a random number 0 or 1, write a function that returns a number between 0 and 199 We are given a function f2(), that can return either 0 or 1 with equal probability. Now using this function f2() only, write a function f200() which returns a number from 0 to 199 with equal probability distribution.

In Ruby:

def f2
  return Random.rand(2)
end

def f200
  # this function makes use of the property that, if you throw away
  # evenly distributed numbers, the rest of the numbers are still
  # evenly distributed

  while true do
    max = 0
    result = 0

    while max < 199 do
      result <<= 1
      max <<= 1
      result += f2()    # f2() gives a random bit
      max += 1
    end

    return result if result >= 0 && result <= 199
  end

end

# this part is only for testing: ==========================================

def print_report(tally)
  number_of_keys = tally.keys().length
  total = tally.values().inject(:+)
  expected_count = total.to_f / number_of_keys

  tally.keys().sort().each {|k| puts "%-8i %-8i off by %6.3f %" % [k, tally[k], ((tally[k] - expected_count) / expected_count * 100).abs()] }
end

n = 1_000_000
puts "checking f200() for #{n} times:"
tallyForF200 = Hash.new(0)
n.times { r = f200(); tallyForF200[r] += 1 }
print_report(tallyForF200)

If you want to see a better result (of more even distributed numbers), increase the n, which is set to 1_000_000 near the end to 10_000_000.

You're just trying to get a certain number of random bits, so calculate that number ahead of time:

bits = 199.to_s(2).length

Then your loop becomes:

loop do
  result = 0
  bits.times do
    result <<= 1
    result += f2
  end
  return result if result >= 0 && result <= 199
end


Get height for tallest possible stack of boxes This code is meant to compute the height for the tallest stack of boxes out of a given collection of boxes. A box has width, height, and depth dimensions. The height of a stack of boxes is the sum of all of the heights of the boxes in the stack. The boxes cannot be rotated. A box can only be stacked on top of another box if its width, height, and depth are strictly smaller.

I'm doing this to improve my style and to improve my knowledge of fundamental algorithms/data structures for an upcoming coding interview.

from operator import attrgetter

class Box:
    def __init__(self, width, height, depth):
        self.width = width
        self.height = height
        self.depth = depth

    def smaller_than(self, other_box):
        return (self.width < other_box.width and
                self.height < other_box.height and
                self.depth < other_box.depth)

def tallest(boxes):
    boxes = sorted(boxes, reverse=True, key=attrgetter('height'))
    largest_height = 0
    for i in range(len(boxes)):
        bottom_box = boxes[i]
        total_height = bottom_box.height
        cur_top_box = bottom_box
        for j in range(i+1,len(boxes)):
            if boxes[j].smaller_than(cur_top_box):
                total_height += boxes[j].height
                cur_top_box = boxes[j]
        if total_height > largest_height:
            largest_height = total_height
    return largest_height

The question wasn't clear about how the boxes were represented or inputted. I just made them into a Box class and assumed that they were passed as a list to tallest(). If there are any better ways of representing the boxes or inputting them (especially in an interview environment) I would be happy to hear it.

I think this code has time complexity O(n^2) and space complexity O(n). Please correct me if I'm wrong. Any suggestions about how I can improve these complexities are welcome.

The code implements a greedy approach (to be exact, n greedy attempts): as soon as a box can be placed, it is placed. It does not necessarily give an optimal solution. For example, with the set of boxes ((height, width)):

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

the algorithm will only consider towers

    (5,5) (4,1)
    (4,1)
    (3,4) (2,3)
    (2,3)

but not an optimal tower

    (5,5), (3,4), (2,3)

You are right about the complexity of the algorithm, but I am afraid it is not relevant if the algorithm itself is not correct.


Shell binary search with sorted input I have coded binary search in shell, just for practice.

Input

Input data is already sorted set of numbers streamed to the script. $1 is the sought number.

for i in {1..1000000}; do
    echo $RANDOM
done | sort -n | ./binsearch.ksh 10

Code

I use ksh as the fastest shell against bash, zsh and other bash clones, and as an interpreter that maintains lists - against dash.

Variables: puppy is the sought number; swamp is a sorted set of numbers.

#!/bin/ksh

puppy=$1; [ -z "$puppy" ] && {
    echo "@@@ No args specified."
    exit
}
size=0

while IFS= read -r line; do
    ((++size))
    swamp[${#swamp[*]}]=$line
done

echo

left=0
right=$(($size - 1))
while   [ $left -le $right ] ; do
    mid=$((($left + $right) >> 1))
#   echo "$left $mid(${swamp[$mid]})    $right"
    if      [ $puppy -eq ${swamp[$mid]} ]; then
        echo "$puppy    $mid"
        exit
    elif    [ $puppy -lt ${swamp[$mid]} ]; then
        right=$(($mid - 1))
    else
        left=$((mid + 1))
    fi
done
echo '</not found>'

Could you please tell what you think of this code and how can I improve it?

Note

I would care about POSIX compatibility if it was a chance to have lists in dash, but as far as there's none, I would have to use namespaces with eval and get my memory filled with 1e6 swamp_43254-like variables.

It's pretty cool to be able to do this in Bash.

Input validation

There are several issues here:

puppy=$1; [ -z "$puppy" ] && {
    echo "@@@ No args specified."
    exit
}

Code is most readable when there is one statement per line,
so I suggest to separate the variable assignment and the input validation steps.

exit without arguments exits with the code of the last command,
in this case 0 because the echo most probably succeeds,
so the program exits with success.
But it shouldn't. A common practice is to use exit code 2 for invalid usage.

Lastly, instead of not-valid-and-exit I think valid-or-else-exit is a somewhat cleaner logic, and also simpler to write:

puppy=$1
[ "$puppy" ] || {
    echo "@@@ No args specified."
    exit 2
}

Unnecessary $size

The size variable is unnecessary.
You can use ${#swamp[*]} instead, for example in:

right=$((${#swamp[*]} - 1))

Unnecessary $ inside $((...))

You can simplify these:

mid=$((($left + $right) >> 1))
right=$(($mid - 1))

Like this:

mid=$(((left + right) >> 1))
right=$((mid - 1))