Multithread resource producer and consumer algorithm I have made a little example of a code that stores frames of a camera in a buffer that runs in a separate thread and another thread can consume it. Is this model efficient?

//isEmpty_buffer is an atomic<bool>
//Mat is a opencv Matrix of pixels
//Max buffersize is 1000
//Camera mutex is a shared_timed_mutex

Mat Camera::get_last()
{
    while (isEmpty_buffer.load()) {

    }

    shared_lock<shared_timed_mutex> lock(camera_mutex);

    Mat ret = buffer.back().clone();
    buffer.front().release();
    buffer.pop();

    if (buffer.empty()) {
        isEmpty_buffer.store(true);
    }

    return ret;
}

void Camera::add_frame(Mat &frame)
{
    lock_guard<shared_timed_mutex> lock(camera_mutex);

    if(buffer.size() < MAX_BUFFER_SIZE){
        buffer.push(frame.clone());
        isEmpty_buffer.store(false);
    } else{
        cout << "Buffer at limit!" << endl;
    }

}

I'll look first at some general programming principles as would be applicable to a single threaded queue, and then I'll look at the concurrency concerns.

First, if something can fail, it must be able to indicate to calling code that it has failed. In some cases, if failure means that something has gone really quite badly wrong, the way to indicate that would be with exceptions. In other cases some cases it would be by returning some sort of success code. In other cases you should rewrite the code so that it cannot fail, perhaps in this case by waiting for the buffer to get some space. However, your add_frame doesn't handle this. If the buffer is full then it prints to standard output (which in most programs is not even being monitored) but as far as the producer is concerned its image has been sent into the system and will be processed in due course. Instead, it's just gone nowhere.

As a matter of style, it is generally not advised to use using namespace, and even more so doing it twice in the same file. It's better to be explicit: use std::cout and cv::Mat. In this case in particular, there is an annoying amount of overlap between OpenCV function and object names and the standard library ones. OpenCV also supplies such things as abs and Vector (although not vector) and confusion about which you're using is like a trail of honey for bugs to find your code!

Also on style,

MAX_BUFFER_SIZE

  Do not use ALL_CAPS for constants just because constants used to be macros.

Thus sayeth the style guide.

Unless MAX_BUFFER_SIZE is actually a preprocessor macro, don't use capitals. If MAX_BUFFER_SIZE is a preprocessor macro, change it to be a constexpr.

In terms of program architecture, a thread safe queue is quite a general utility method, and one that is conceptually distinct from a camera. It would probably be better to move all this code (together with the buffer, mutexes, etc) to its own class and its own file.

Because it is such a general utility, it is perhaps worthwhile templating it so that you could use a similar queue for datatypes other than cv::Mat. The only thing that currently interferes with that is the clone() call.

Because cv::Mat is an inherently costly data type, it is worth thinking quite hard about whether each clone() call is needed. By default I would not expect a queue to be deep copying its input. In a producer/consumer context, I would expect that the producer pushes to the queue once it is done creating the object, and has no further use. Therefore I would suggest that the clone() can probably be removed. It is definitely unnecessary to have a clone taken on entering and on leaving the queue! (If in fact the producer does need to hold onto the image, and further needs to ensure that the consumer doesn't write to or otherwise mess with that image, it would be more explicit to do the clone before it gets into add_frame.)

Mat ret = buffer.back().clone();
buffer.front().release();
buffer.pop();

This just looks wrong to me. If the image at the back of the buffer is the one that you're returning, why is it the image at the front of the buffer that you're releasing? Further, why are you releasing it at all? The cv::Mat destructor calls release(), so as soon as the reference counter decides you're done with it. Combined with not wanting to clone if possible, I'd expect something more like

cv::Mat ret = buffer.front();
buffer.pop;

Edit: I have noticed that the second and third lines of this got caught up in your edit to switch from back to front, and the first was missed. So it's probably just an editing bug rather than a conceptual one, and a very easy fix. I would still question the clone() and release() though.

Now for concurrency. Please note that I don't have enough context to run this code, much less extensively test it. Concurrency of course needs very extensive testing, because the bugs it produces tend to be intermittent.

I am inherently suspicious of this atomic isEmpty_buffer flag. If you can't directly check buffer.empty() because it may not be updated perfectly in sync with buffer, you also cannot rely on this external atomic variable which will also not be updated in sync with buffer.

I do not believe that a shared_timed_mutex is suitable for this use case. It is generally used when some jobs are guaranteed not to mutate the data structure. If threads aren't going to change anything, you can have as many of them swarming over it at once as you want, and hence shared becomes relevant. The mutex only needs to make sure that threads which do change things get exclusive access, so that a thread which is only reading doesn't read half the state from before an update and half after. (And also so that two updates don't clash) However this is not what's happening here: both the producer and the consumer change the data structure by pushing and popping.

It might in fact work out okay if you guarantee that there is never more than one consumer, in which case you're not really using the shared aspect of the mutex at all. If this is the case you should be very explicit about your assumptions.

Presuming that you selected instead a more conventional mutex, your

while (isEmpty_buffer.load()) {

}

is still not enough to prevent things from breaking if there are multiple consumers. It is outside the lock, which means that when the queue has one element in it, two consumers could both get past that check and queue up in front of the mutex. The first consumer would go through, pop the element, and unlock the mutex on its way out. Then the second consumer would go through, pop against the empty buffer, and crash and burn.


Two versions of Word Break II - exhaustive combinations I am solving Word Break II from leetcode:

  Given a non-empty string s and a dictionary wordDict containing a list of non-empty words, add spaces in s to construct a sentence where each word is a valid dictionary word. Return all such possible sentences.

  Note:

  The same word in the dictionary may be reused multiple times in the segmentation.
  You may assume the dictionary does not contain duplicate words.

  Input:

s = "pineapplepenapple"
wordDict = ["apple", "pen", "applepen", "pine", "pineapple"] Output:

[
"pine apple pen apple",
"pineapple pen apple",
"pine applepen apple"
]

  Explanation: Note that you are allowed to reuse a dictionary word.

I think my Python has improved since I posted island problem last. Your input is appreciated.

Here are the two solutions:

def inverse_iterative(s, wdict):
    st = []
    ans = []

    for w in wdict:
        if s.startswith(w):
            if len(s) == len(w):
                ans.append(w)
                return ans
            st.append((w, 0, [w]))

    while st:
        cur_w, cur_idx, cur_list = st.pop()
        cur_start = cur_idx + len(cur_w)

        for w in wdict:
            if not s[cur_start:].startswith(w):
                continue
            if len(w) == len(s[cur_start:]):
                ans.append(' '.join(cur_list+[w]))
            else:
                st.append((w, cur_start, cur_list+[w]))
    return ans

def inverse_helper(s, wdict, memo):
    if s in memo:
        return memo[s]
    if not s:
        return []

    res = []
    for word in wdict:
        if not s.startswith(word):
            continue
        if len(word) == len(s):
            res.append(word)
        else:
            result_of_rest = helper(s[len(word):], wordDict, memo)
            for r in result_of_rest:
                r = word + ' ' + r
                res.append(r)

    memo[s] = res
    return res

ans_iterative = inverse_iterative(s, wdict)
ans_recursive = inverse_helper(s, wdict, {})

The first solution

There's a bug in the first loop:

    for w in wdict:
        if s.startswith(w):
            if len(s) == len(w):
                ans.append(w)
                return ans
                ^^^^^^^^^^ should not do this!
            st.append((w, 0, [w]))

It's not correct to return when one of the words is the same as the sentence,
because there might be other words to compose that sentence.
At it is, the program produces incorrect result for the input:

"foobar"
["foobar", "foo", "bar"]

The first loop duplicates some of the logic of the second loop.
You can remove the first loop completely if you initialize st like this:

st = [('', 0, [])]

This solution is much slower than the second,
because unlike the second,
it doesn't do anything to avoid repeated computation of sub-problems.
For example, if the input string is "foobar" + some_very_long_string,
and the word list has "foo", "bar", and "foobar", then the sentences for some_very_long_string will be computed twice at least, for the "foo bar" and "foobar" prefixes.

The second solution

The posted code is broken, probably you copy-pasted a work-in-progress version:

result_of_rest = helper(s[len(word):], wordDict, memo)

This line references the nonexistent function helper,
and the nonexistent variable wordDict.
The fix is easy enough:

result_of_rest = inverse_helper(s[len(word):], wdict, memo)

Next time please make sure to post working, verified code!

This solution implements memoization manually.
Python has a cool technique to make this easier,
using lru_cache from the functools package.
In a nutshell,
all you need is a function with hashable arguments,
and then you can simply annotate it with @lru_cache to benefit from easy memoization.

from functools import lru_cache

def word_break_helper(text, wdict):
    @lru_cache(maxsize=128)
    def helper(s):
        res = []
        for word in wdict:
            if not s.startswith(word):
                continue
            if len(word) == len(s):
                res.append(word)
            else:
                for sentence in helper(s[len(word):]):
                    res.append(word + ' ' + sentence)
        return res

    return helper(text)

Use better names

The names used in both solutions are quite poor.

I don't know what's "inverse" about these solutions.
Sure, the first one uses a stack, to which it appends and pops values,
but it's not really important anyway.
So I don't get "inverse" in these names.

res, and ans are poor as well. Why not naturally sentences?

wordDict doesn't follow Python naming conventions (should be snake_case).

Further improving performance

Although it doesn't seem to be important in this puzzle,
looping over the list of supported words to find possible prefixes would be inefficient with a large number of words.
You could use a more efficient data structure to retrieve potential prefix words without iterating over all the supported words.


Scaling decimal numbers up within bounds of limit This code is part of a simulation and inputs will be positive values less than one and inputs are constrained elsewhere. The goal of the algorithm is to scale the inputs up to much larger numbers, although the total sum is bounded. These represent the number of entities in the simulation.

Conceptually it is best to remember that the value 0.001 can also be expressed as 10^-3 and 4.2*10^-3 is also written as 0.0042. What the algorithm is trying to do is determine the exponent needed such that all of the numbers are positive integer values. Once all of the inputs are integers is then ensures that they are scaled within the bound of the maximum entities.

For example, if the simulation has a limit of MAX_ENTITY_COUNT = 1000000 entities and the user enters input[0].raw = 0.01, and input[1] = 0.1 I would expect to see input[0].count = 90900 and input[1].count = 909000 for a total of 999,900 entities.

private void findEntityCount(List<Inputs> input) {
    // Find the smallest exponent based upon the natural log
    int smallest = Integer.MAX_VALUE;
    for (Inputs entry : input) {
        int exp = (int)Math.log(entry.raw);
        if (exp < smallest) {
            smallest = exp;
        }
    }

    // Calculate the scaling, note that this is closely related to finding the
    // mantissa of the input value, but subtracting one from the value helps
    // pack things a bit better
    long total = 0;
    double scaling = Math.pow(10, Math.abs(smallest) - 1);
    for (Inputs entry : input) {
        entry.count = (long)Math.ceil(entry.raw * scaling);
        total += entry.count;
    }

    // Use the initial total to find a multiplier that we can use to adjust
    // the initial counts to the final count for the simulation
    double multiplier = MAX_ENTITY_COUNT / total;
    for (Inputs entry : input) {
        entry.count *= multiplier;
    }
}

So far this code appears to be working well and resulting in quantities that are scaled correctly and pack the space fairly well. However, since it is playing around with how floating points are stored I'm looking for feedback that:

The approach makes sense (i.e., there isn't a better way to do it that is clearer).
What are some pitfalls that I should be aware of.

I haven't seen any issues yet, but generally I've also only seen a limited range of inputs.

double smallest = Double.MAX_VALUE;

This is even larger:

double smallest = Double.POSITIVE_INFINITY;

    if (exp < smallest) {
        smallest = exp;
    }

Unless you need to do something else in the body, like remembering an index, this can better be replaced with:

    smallest = Math.min(smallest, exp);

Anti-pattern: The -1 you describe is a 'magic value', which in this case, after a bit of reverse engineering, just divides scaling by 10, like so:

double scaling = Math.pow(10, Math.abs(smallest)) / 10;

This is clearer to casual readers of your code, but still a magic number unless rationalized in the comments.

Pitfall: There is no check if entry.raw is actually not larger than one. If it is, your approach breaks.

Performance / Precision / Confusing: don't calculate base-e log for every number and then base-10 exp (that combination alone is already confusing enough). Just find the very smallest number (not the smallest exponent) and set double scaling = 1.0 / smallest;. To get a number-correct result to your current implementation, because of the base confusion, it would be:

double scaling = 40.287487705590465 / smallest;

Note that this just moves the 'magic number' problem. You need to rationally explain the value here, and not just state that it 'packs things better'.


Parabola calculations - follow-up Previous question

After applying the suggestions, I wanted to revise the question with the new code because I don't think this is good enough.

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

/* global loop counters */
int i=0, j, k;

/* to find a, b, and c from the input */
float A(const char* in){
    char c[5];
    if(in[0] != 'x'){ /* if first letter isn't x
                      ** eg there is sth in front of x */
        while(in[i] != 'x'){ /* until x in found */
            i++; /* increment i */
        }
        strncpy(c, in, i); /* copy first i characters of in to c */
        return atoi(c); /* make c an int and return */
    }

    if(in[0] == '-' && in[1] == 'x') /* if first two letters are "x" */
        return -1; /* -x = -1 * x */

    return 1; /* if nothing is in front of x, a is 1 */
}

float B(const char* in){
    char c[5];
    int b=1;

    j = i += 3; /* three characters after x (^2+ or ^2-) */
    while(in[j] != 'x') { /* until x is found */
        j++; /*increment j */
    }

    if(in[j-1] != '+' && in[j-1] != '-'){ /* if not -x or +x
                                          ** eg if there is b */
        strncpy(c, in+i, j); /* copy j characters starting from i */
        b = atoi(c); /* make c an int and assign this to b */
    }

    if(in[j-1] == '-') /* if there is a minus sign */
        b = -b; /* negate b */

    return b; /* return b */
}

float C(const char* in){
    char c[5];
    k = j += 1; /* one character after x (+ or -) */
    while(in[k] != '=') { /* until we find a = */
        k++; /* increment k */
    }
    strncpy(c, in+j, k); /* copy k characters starting from j */
    return atoi(c); /* make c an int and return */
}

int main(int argc, char const *argv[]) {
    if(argc == 1){ //if no arguments given
        printf("Usage: %s <quadratic equation>\n", argv[0]);
        return EXIT_FAILURE;
    }

    /* variables for main function */
    float discriminant;
    float a, b, c; /* coefficients */
    float vx, vy; /* vertex */
    char sign; /*positive or negative or zero */

    a=A(argv[1]);
    b=B(argv[1]);
    c=C(argv[1]);

    /*                   b^2 - 4ac     */
    discriminant = pow(b, 2) - 4 * a * c;

    if(discriminant < 0) {
        sign = 'n';
        printf("Parabola doesn't have real roots.\n");
    } else if(discriminant == 0.00f) {
        sign = 'z';
        printf("Parabola has one real root.\n");
    } else if(discriminant > 0) {
        sign = 'p';
        printf("Parabola has two real roots.\n");
    }

    /* vertex points */
    vx = -( b / (2*a) );
    vy = -( discriminant / (4*a) );

    printf("Parabola looks %s\n", a < 0 ? "downwards" : "upwards");
    printf("Vertex: V(%.2f,%.2f)\n", vx, vy);
    printf("Axis of symmetry: %.2f\n", vx);
    printf("y-intercept: %.2f\n", c);

    if(sign=='n'); /* if negative, doesn't have roots */
    else if(sign=='z') /* if zero, root is vertex' abcsissa is root */
        printf("x-intercept: %.2f\n", vx);
    else if(sign=='p'){ /*if positive, roots can be calculated... */
        float x1, x2;
        x1=(-b + sqrt(discriminant)) / (2*a);
        x2=(-b - sqrt(discriminant)) / (2*a);
        printf("x-intercepts: %.2f and %.2f\n", x1, x2);
    }
}

General: Do not use int for float values.
Either use float and float functions like sqrtf() or use double.
When fabs(b) is close to sqrt(discriminant), OP's approach suffers severe loss of precision due to the + or -.  More numerically stable code posted.  Still needs some divide by 0.0 checks.

//x1=(-b + sqrt(discriminant)) / (2*a);
//x2=(-b - sqrt(discriminant)) / (2*a);
float s = sqrtf(discriminant);
if (b < 0) {
  x1 = (-b + s)/(2*a);
  x2 = c/x1;
} else if (b > 0) {
  x2 = (-b - s)/(2*a);
  x1 = c/x2;
} else {
  x1 = s/(2*a);
  x2 = -x1;
}

pow(b, 2) - 4 * a * c; also suffers from severe loss of precession when b*b is about 4*a*c.  If OP is interested there are method to better calculate that too.  It comes down to how quick you want to write/execute the code versus the need for correctness on edge conditions.
Unclear format usage.  About 45% of all float numbers will print as 0.00 with "%.2f".  Suggest a more meaningful format

   // printf("Vertex: V(%.2f,%.2f)\n", vx, vy);
   printf("Vertex: V(%e,%e)\n", vx, vy);

Dangerous code.  Nothing stop code from incrementing into la-la land.  If this A() and B() are only used in this controlled environment, make them static, else they should robustly handle all sorts of string inputs.

float A(const char* in) {
  ...
  while(in[i] != 'x'){ /* until x in found */
    i++; /* increment i */
  }

More weak code.  Nothing insures that c will be a string after the strncpy call.  Just use strtol() or strtof().

strncpy(c, in, i); /* copy first i characters of in to c */
return atoi(c);

Unclear why code is using atoi(), yet returning float.
Corner case: With b = atoi(c);, should c have the textual value of -INT_MIN, then assignment to b is UB.
Consider printing something when sign=='n'
Make a clean separation of parsing form quadratic calculation.  (2 functions)


Recording a customer's shopping cart as an order 2.0 Some days ago, I put up a question here for review about the best way to handle JDBC transactions in the context of an e-commerce Java web application.

After a kind suggestion from user @gardenhead, I decided to use the try-with-resources construct to automatically close my resources, namely the statement and the connection objects.

I also found this SO answer, which told me how to properly use try-with-resources for this kind of operation.

This is my improved code:

public long insertOrder(Customer customer, Hashtable<String, CartItem> shoppingCart)
{
    try (Connection connection = MySqlConnection.getConnection()) {
         try (Statement statement = connection.createStatement()) {
            connection.setAutoCommit(false);
            long orderId = System.currentTimeMillis();
            OrderPeer.insertOrder(statement, orderId, customer);
            OrderDetailsPeer.insertOrderDetails(statement, orderId, customer);
            connection.commit();
            return orderId;
        } catch (SQLException e) {
            connection.rollback();
            System.err.println("Could not insert order: " + e.getMessage());
            return 0L;
        }
    } catch (SQLException e) {
        return 0L;
    }
}

The rollback is triggered when either there is a problem with the transaction itself, or when there is a problem with the automatic closing of the statement. The later due to the fact that it would be indicative of a database access error that left the transaction in an unknown state.

If there is anything else that could be improved, please let me know.

Maps

(copying my answer from another question):

A HashMap or ConcurrentHashMap (if you require the thread-safety) is preferred over the legacy Hashtable, as mentioned in the Javadoc:

  As of the Java 2 platform v1.2, this class was retrofitted to implement the Map interface, making it a member of the Java Collections Framework. Unlike the new collection implementations, Hashtable is synchronized. If a thread-safe implementation is not needed, it is recommended to use HashMap in place of Hashtable. If a thread-safe highly-concurrent implementation is desired, then it is recommended to use ConcurrentHashMap in place of Hashtable.

Typo?

OrderDetailsPeer.insertOrderDetails(statement, orderId, customer);

Compared to your first version, it seems that shoppingCart should be inserted here instead of customer, unless you have changed your APIs. If so, then shoppingCart is unused in this method...


Extracting command data from a REST API I have a method in C# which communicates with REST API.It has a several small task like splitting of a string ,checking a commandText is valid or not,POST and GET JSON payload to/from REST API.

  public  string PostToAPI([FromBody]string value)
        {
            string input = value;
            //splitting of string into substrings
               var str = input.Split(new[] { ' ' }, 2);
                var payload = new testModel
               {
                   Command = str[0],
                   CommandText = str[1]
               };
            //list of commands
              List<string> cmdText = new List<string>(){"/calc", "/test" };
            //check CommandText is valid or not
              if(cmdText.Contains(payload.CommandText))
              {
               //return valid
              }

            // Serialize our concrete class into a JSON String
            var stringPayload = JsonConvert.SerializeObject(payload);
             try
             {
                 // Wrap our JSON inside a StringContent which then can be used by the HttpClient class
                 var httpContent = new StringContent(stringPayload, Encoding.UTF8, "application/json");

                  using (var httpClient = new HttpClient())

                 {
                     HttpResponseMessage message = httpClient.PostAsync("https://testapi.com", httpContent).Result;
                     if (message.IsSuccessStatusCode)
                     {
                         string result = message.Content.ReadAsStringAsync().Result;
                         return result;
                     }
                     else
                     return message.StatusCode+"kindly contact to administrator";

                 }
             }
             catch (Exception ex)
             {
                 return ex.InnerException.ToString();
             }

            }

splitting of a string splits the string into two substrings.

checking a commandtext, checks commandtext is valid or not.

they all are in one method.From architecture/design point of view is this right or should i split the task into different module or in different classes to add more flexibility, readability,As there may be a situation where i just need to add commands string and don't want to touch the other task.

How to organize such c# tasks into different modules/class to make them more flexible, readable?
Thanks!

What's the point of string input = value;? Why not work with value directly?

Comments should tell me why,  not what. If you suspect I cannot detect //splitting of string into substrings from your code, then your code is bad.

testModel is a class and thus should be PascalCase;.

cmdText is a collection of commands? That variable name doesn't suggest so at all. No wonder you need to preface it with //list of commands. If it is a list of commands, call it commands. Or considering the context, validCommands.

What's the point of this:

if(cmdText.Contains(payload.CommandText))
{
    //return valid
}

This doesn't do anything.

You start out using var, yet later on you get more explicit: HttpResponseMessage message, string result. Be consistent.

Same for your if...else:

if (message.IsSuccessStatusCode)
{
    string result = message.Content.ReadAsStringAsync().Result;
    return message.Content.ReadAsStringAsync().Result;
}
else
return message.StatusCode+"kindly contact to administrator";

The bad indentation hide that return message.StatusCode+"kindly contact to administrator"; is actually your else. Always use brackets!

What's the point of string result = message.Content.ReadAsStringAsync().Result;? Why not simply return message.Content.ReadAsStringAsync().Result;?

Quite frankly this code looks very unfinished. Your style is inconsistent, indentation is all over the place, there are superfluous blank lines (after using (var httpClient = new HttpClient()) for instance), the comments are superfluous, there's code in there that doesn't do anything,...

This also looks like a method in a Controller, and that's worrying. Because even if it's a mere 50 lines, if you have several methods like this pretty soon your controller class will be very bloated. Moreover this one method does three things:

convert the incoming data into a payload
checking the payload against a list of values
posting the payload to another API

Each of these should be their own method, and thus the body of this controller should be its own class, split into multiple methods. Look for instance at MediatR to "un-bloat" your controllers.


Counting all substrings with exactly k distinct characters Problem Statement : Given a string of lowercase alphabets, count all possible substrings (not necessarily distinct) that has exactly k distinct characters. Example:

Input: abc, k = 2 Output: 2 Possible substrings are {"ab", "bc"}

I have written the solution with a two pointer approach. I am not sure how to calculate the time complexity of the program?

According to me complexity should be O(n*k)

public static void main(String[] args)
    {
    String s = "abacuusttlvbnc";
    int k=3;
    char[] sArr = s.toCharArray();
    int strLen = sArr.length;
    Set<Character> set = new LinkedHashSet<Character>();
    int l=0;
    int r=0;

    while(l<=strLen-k){  // will run (arrayLength - k) times
        for(int i=0;i<k;i++){   // will run k times for every while iteration
            set.add(sArr[l]);
            l++;
        }
        if(set.size()==k){
            System.out.println("substring : " + set);
        }
        set.clear(); // O(k) for every while iteration
        r++;
        l= r;
     }
  }
}

Output :

substring : [b, a, c]
substring : [a, c, u]
substring : [u, s, t]
substring : [t, l, v]
substring : [l, v, b]
substring : [v, b, n]
substring : [b, n, c]

Consider

public static void findSubstringsWithKDistinctCharacters(String s, int k) {
    char[] letters = s.toCharArray();

    for (int i = 0, n = letters.length - k; i <= n; i++) {
        Set<Character> uniques = new LinkedHashSet<Character>();

        for (int j = i, m = i + k; j < m; j++) {
            uniques.add(letters[j]);
        }

        if (uniques.size() == k) {
            System.out.println("substring : " + uniques);
        }
    }
}

This is a simpler version of what you wrote.  It gets rid of your r variable entirely, as it is unnecessary.

I also changed the names of sArr and set to things that I find more descriptive.

I moved the code into its own method so that it can be called multiple times.

There are two reasons to move the declaration of uniques into the loop.  One, this is less code.  Two, if you changed this code to produce a list of results rather than print the results, the other version doesn't work.  You'd have multiple copies of whatever the last set was rather than unique copies.  I'd only use the clear version if I knew that performance of this method was a bottleneck.

Bug

Unfortunately, both this version and your original do not match the linked problem statement:

  Input: aba, k = 2
  Output: 3
  Possible substrings are {"ab", "ba", "aba"}

  Input: aa, k = 1
  Output: 3
  Possible substrings are {"a", "a", "aa"}

They only find two solutions for each of these, as they stop counting once there are k characters (distinct or not) in the substring.  They should keep going until they've verified that the next character isn't a duplicate of a character already in the substring.

Complexity

Calling the time complexity \$\mathcal{O}(n\cdot k)\$ is reasonable.  It's slightly more accurate to say that it is \$\mathcal{O}(n\cdot k - k^2)\$, but \$k\$ is never larger than \$n\$ so it's reasonable to view this as \$\mathcal{O}(n\cdot k)\$.  Note that if you fix the algorithm, it would be \$\mathcal{O}(n^2)\$, as the worst case is an input of all the same character.  That gives substrings up to length \$n\$ where this is limited to length \$k\$.


Beginner C fgetline() implementation I am doing C coding practice by reinventing a getline function that works with any streams. Any comments on the overall code quality, including correctness, style and performance, are greatly appreciated.
fgetln.h:
#ifndef FGETLN_H
#define FGETLN_H
#include <stdio.h>

char *fgetln(FILE *fp);
/*
 * Read a line from fp, dynamically allocate memory and store it as a string.
 * The '\n' at the end of the line and any '\0' before are discarded. Return
 * a pointer to the string when successful; return NULL if the line is empty
 * or an error occurs.
 *
 * This function implements the "callee allocates, caller frees" model. You
 * should pass the returned pointer to free() when it is no longer needed.
 */

#endif  // FGETLN_H

fgetln.c:
#include <stdio.h>
#include <stdbool.h>
#include <stdlib.h>
#include <stdint.h>

#define EXPAND_RATIO 2
// Aggressiveness of memory expansion, must be an integer ≥ 2.

char *fgetln(FILE *fp)
{
    signed char c;

    // Look for hints of an empty line and discard any '\0' at the start.
    while (true) {
        c = getc(fp);
        if (c == '\0') continue;
        if (c == '\n' || c == EOF) return NULL;
        break;
    }

    // If we have reached here, the line has content in it.
    ungetc(c, fp);

    char *p_tmp, *line = NULL;
    size_t arr_size = sizeof(char);
    bool end_of_line = false;
    unsigned long long i = 0;

    p_tmp = realloc(line, arr_size);
    if (p_tmp == NULL) goto err_cleanup;
    line = p_tmp;

    do {
        c = getc(fp);
        if (c == '\0') continue;
        if (c == '\n' || c == EOF) {
            end_of_line = true;
            c = '\0';
        }

        if ((i * sizeof(char)) == arr_size) {
            if (arr_size == SIZE_MAX) {
                if (c != '\0') goto err_cleanup;  // Too long
            } else {
                // Assuming SIZE_MAX % sizeof(char) == 0.
                arr_size = arr_size > (SIZE_MAX / EXPAND_RATIO)
                         ? SIZE_MAX
                         : arr_size * EXPAND_RATIO;
                p_tmp = realloc(line, arr_size);
                if (p_tmp == NULL) goto err_cleanup;
                line = p_tmp;
            }
        }

        line[i++] = c;
    } while (!end_of_line);

    // Shorten the string to its logical size if available.
    size_t logic_size = i * sizeof(char);

    if (logic_size < arr_size) {
        p_tmp = realloc(line, logic_size);
        if (p_tmp == NULL) goto err_cleanup;
        line = p_tmp;
    }
    return line;

err_cleanup:
    free(line);
    return NULL;
}

#undef EXPAND_RATIO

Excellent comment in the header.  It's very important to be clear who owns the returned memory and how to release it.  It ought to mention that both newline and \0 are considered as end-of-line characters.
Good realloc() usage - we don't leak when allocation fails.
Some small improvements:

#define EXPAND_RATIO 2

Why do we need a preprocessor macro for this?  I would use a simple static const size_t.

size_t arr_size = sizeof(char);

sizeof (char) is 1 by definition (since sizeof yields the number of char required for its argument).  I wouldn't start with such a small buffer anyway, since almost every line will need several realloc() calls.  Start with something sensible, such as 40 (half the width of a standard terminal).

p_tmp = realloc(line, arr_size);
if (p_tmp == NULL) goto err_cleanup;
line = p_tmp;

Since we know that line is null at this point, we should just use malloc() to be clearer, and eliminate a window where *line is invalid:
char *line = malloc(arr_size);
if (!line) return line;

    if ((i * sizeof(char)) == arr_size) {

We can remove the multiplication by 1 from this:
    if (i == arr_size) {

        if (arr_size == SIZE_MAX) {

Ooh, that's very thorough.  We're unlikely to have managed to allocate that much memory in a single chunk!

            // Assuming SIZE_MAX % sizeof(char) == 0.

Again, any number modulo 1 is zero, so that's a truism, not a mere assumption.

#undef EXPAND_RATIO

Arguably overkill, given that this is the very end of the translation unit anyway.


improve the design of class “accuracy” in Python I am learning about the class and methods in Python.

The class Accuracy is a class of several (13 in total) statistic values between a reference polygon and one or more segmented polygons based on shapely module.

from numpy import average

#some stat

def ra_or(ref, seg):
    return average([(ref.intersection(s).area/ref.area) for s in seg])

def ra_os(ref, seg):
    return average([(ref.intersection(s).area/s.area) for s in seg])

def sim_size(ref, seg):
return average([(min(ref.area, s.area)/max(ref.area,s.area)) for s in seg])

def AFI(ref,seg):
   return (ref.area - max([s.area for s in seg]))/ref.area

where ref.intersection(s).area is the area of intersection between reference and segmented polygon-i

my class design (really basic and probably to improve) is:

class Accuracy(object):
    def __init__ (self,ref,seg = None, noData = -9999):
        if seg == None:
            self.area = ref.area
            self.perimeter = ref.length
            self.centroidX = ref.centroid.x
            self.centroidY = ref.centroid.y
            self.data =  [self.centroidX,
                        self.centroidY,
                        self.area,
                        self.perimeter,
                        noData,
                        noData,
                        noData,
                        noData,
                        noData]
        else:
            self.area = ref.area
            self.perimeter = ref.length
            self.centroidX = ref.centroid.x
            self.centroidY = ref.centroid.y
            self.segments = len(seg)
            self.RAor = ra_or(ref,seg)
            self.RAos = ra_os(ref,seg)
            self.SimSize = sim_size(ref,seg)
            self.AFI = AFI(ref,seg)
            self.data = [self.centroidX,
                        self.centroidY,
                        self.area,
                        self.perimeter,
                        self.segments,
                        self.RAor,
                        self.RAos,
                        self.SimSize,
                        self.AFI]

from shapely.geometry import Polygon

p1 = Polygon([(2, 4), (4, 4), (4, 2), (2, 2), (2, 4)])
p2 = Polygon([(0, 3), (3, 3), (3, 0), (0, 0), (0, 3)])

accp1 = Accuracy(p1,[p2])
accp1.data
[3.0, 3.0, 4.0, 8.0, 1, 0.25, 0.1111111111111111, 0.44444444444444442, -1.25]

accp1 = Accuracy(p1)
accp1.data
[3.0, 3.0, 4.0, 8.0, -9999, -9999, -9999, -9999, -9999]

If you plan on calling the four functions ra_or, ra_os, sim_size and AFI outside of Accuracy then it is good to keep them as functions. If they never get called except through Accuracy, then they should be made methods.

Classes can help organize complex code, but they generally do not make your code faster. Do not use a class unless there is a clear advantage to be had -- through inheritance, or polymorphism, etc.

If you want faster code which uses less memory, avoid using a class here. Just define functions for each attribute.

If you want "luxurious" syntax -- the ability to reference each statistic via an attribute, then a class is fine.

If you plan on instantiating instances of Accuracy but not always accessing all the attributes, you don't need to compute them all in __init__. You can delay their computation by using properties.

 @property
 def area(self):
    return self.ref.area

Note that when you write accp1.area, the area method above will be called. Notice there are no parentheses after accp1.area.

To be clear, the advantage to using properties is that each instance of Accuracy will not compute all its statistical attributes until they are needed. The downside of using a property is that they are recomputed everytime the attribute is accessed. That may not be a downside if self.ref or self.seg ever change.

Moreover, you can cache the result using Denis Otkidach's CachedAttribute decorator. Then the attribute is only computed once, and simply looked up every time thereafter.

Don't use an arbitrary value for noData like noData = -9999. Use noData = np.nan, or simply skip noData and use np.nan directly.

import numpy as np
from shapely.geometry import Polygon
nan = np.nan

class Accuracy(object):
    def __init__(self, ref, seg=None):
        self.ref = ref
        self.seg = seg

    @property
    def area(self):
        return self.ref.area

    @property
    def perimeter(self):
        return self.ref.length

    @property
    def centroidX(self):
        return self.ref.centroid.x

    @property
    def centroidY(self):
        return self.ref.centroid.y

    @property
    def data(self):
        return [self.centroidX,
                self.centroidY,
                self.area,
                self.perimeter,
                self.segments,
                self.RAor,
                self.RAos,
                self.SimSize,
                self.AFI]

    @property
    def segments(self):
        if self.seg:
            return len(self.seg)
        else:
            return nan

    @property
    def RAor(self):
        if self.seg:
            return np.average(
                [(self.ref.intersection(s).area / self.ref.area) for s in self.seg])
        else:
            return nan

    @property
    def RAos(self):
        if self.seg:
            return np.average(
                [(self.ref.intersection(s).area / s.area) for s in self.seg])
        else:
            return nan

    @property
    def SimSize(self):
        if self.seg:
            return np.average(
                [(min(self.ref.area, s.area) / max(self.ref.area, s.area))
                 for s in self.seg])
        else:
            return nan

    @property
    def AFI(self):
        if self.seg:
            return (self.ref.area - max([s.area for s in self.seg])) / self.ref.area
        else:
            return nan

p1 = Polygon([(2, 4), (4, 4), (4, 2), (2, 2), (2, 4)])
p2 = Polygon([(0, 3), (3, 3), (3, 0), (0, 0), (0, 3)])

accp1 = Accuracy(p1, [p2])
print(accp1.data)
# [3.0, 3.0, 4.0, 8.0, 1, 0.25, 0.1111111111111111, 0.44444444444444442, -1.25]

accp1 = Accuracy(p1)
print(accp1.data)
# [3.0, 3.0, 4.0, 8.0, nan, nan, nan, nan, nan]

Here is how you could save your data (as a numpy array) to a CSV file:

np.savetxt('/tmp/mytest.txt', np.atleast_2d(accp1.data), delimiter=',')

And here is how you could read it back:

data = np.genfromtxt('/tmp/mytest.txt', dtype=None)
print(data)
# [  3.   3.   4.   8.  nan  nan  nan  nan  nan]


Generate short database identifier names from an integer I wrote the following to generate short database identifier names (e.g. parameters). This isn't a terribly practical requirement and could be fulfilled nearly as well by using base-26, but what seemed like a simple problem became an interesting challenge.

My result doesn't feel very clean, and it has some collisions. I would appreciate performance or elegance suggestions.

Rules: First character must be alphabetic, subsequent must be alphanumeric.

public string intToDatabaseIdentifier(int number)
{
    if(number < 0 || number > 1000000)
        throw new ArgumentOutOfRangeException("number");
    if(number > 25 && number <= 25 + 10) // Skip 0-9 (modified base 36)
        number += 10;
    if(number > 971 && number <= 971 + 360) // Skip 0a-09 (modified base 36)
        number += 360;
    if(number > 35027 && number <= 35027 + 12960) // Skip 0aa-099 (modified base 36)
        number += 12960;
    var stack = new Stack<char>();
    // Base 36, but starting with letters rather than numbers
    const string characters = "abcdefghijklmnopqrstuvwxyz0123456789";
    while(number >= 0) {
        stack.Push(characters[number % 36]);
        number = number / 36 - 1;
    }
    return new string(stack.ToArray());
}

Additional characters (e.g. _, #) can be easily added, but they yield little and make the method less database-agnostic, so I'll stick with alphanumerics.

Results:

a b c d e f g h i j k l m n o p q r s t u v w x y z
aa ab ac ad ae af ag ah ai aj aa ab ac ad ae af ag ah ai aj ak al am an ao
ap aq ar as at au av aw ax ay az a0 a1...

Ideally the output will be as above (without the collisions). The accepted answer in SO works but produces output in an unusual order, e.g. aa ba ca

The trick is to find a formula for where a new place has to be added. In this case the formula is:

$$ d_k = 26*(36^k-1)/35 $$

If n >= dk then the name of n has at least k+1 digits.

The turnover points are 26, 962, 34658, 1247714, ...

name 24          = y
name 25          = z
name 26          = aa
name 27          = ab

name 960         = z8
name 961         = z9
name 962         = aaa
name 963         = aab

name 34656       = z98
name 34657       = z99
name 34658       = aaaa
name 34659       = aaab

name 1247712     = z998
name 1247713     = z999
name 1247714     = aaaaa
name 1247715     = aaaab

name 44917728    = z9998
name 44917729    = z9999
name 44917730    = aaaaaa
name 44917731    = aaaaab

After finding k, just represent n - dk as a k+1 digit number base 36.

Some Python code:

theChars = "abcdefghijklmnopqrstuvwxyz0123456789"

# the magic formula:
def dk(k):
  return (26 * (36**k-1)) / 35

def name(n):
  # find the largest k s.t. dk(k) <= n
  k = 0
  while dk(k) <= n:
    k += 1
  k -= 1
  # take k+1 digits mod 36
  q = n - dk(k)
  digits = ""
  while k >= 0:
    r = q % 36
    q = q / 36
    digits = digits + theChars[r]
    k -= 1
  return digits[::-1]  # shortcut to reverse a string

def testForDuplicates(a,b):
  seen = set()
  for n in xrange(a,b):
    m = name(n)
    if m in seen:
      print "duplicate found at n =", n, ", name:", m
    seen.add(m)

def printRange(a,b):
  for n in xrange(a,b+1):
    print n, " ->", name(n)

printRange(20,30)
printRange(955,965)
printRange(34650,34662)
printRange(1247710,1247720)
printRange(44917727,4491773)

testForDuplicates(0,1000000) # takes about 6 secs


Rocks, Paper Scissors game I wrote a Rock Paper Scissors game and was hoping to get some feedback on issues like:

Am I utilizing object oriented techniques correctly?  What can I do better?
Am I using main() correctly?  My understanding is that it should be a main controller for the rest of the program.  Should I be utilizing a counter in a different method, instead of in main?
Am I using my instance variables correctly?
I realize that the computer choice is skewed.  How should I do it differently?
The way I check for wins seems a little convoluted.  Should I be using a different system to check?
What did I do well, so that I can remember to use those techniques?  What did I do poorly?

Main

import java.util.*;
public class driver {

public static void main(String[] args) {

    int userWins = 0, computerWins = 0;

    System.out.println("Welcome to Rocks, Paper, Scissors. How many games would you like to play?");
    Scanner in = new Scanner(System.in);
    int numberOfGames = in.nextInt();
    int userCounter = 0;
    int computerCounter = 0;
    for (int i = 0; i < numberOfGames; i++) {
        RPS Game = new RPS();
        String user = Game.startGame();
        String computer = Game.computerChoice();
        boolean wins = Game.evaluateGame(user, computer);
        if (wins == true){
            userWins++;
        }
        else {
            computerWins++;
        }
        System.out.println("Number of wins:");
        System.out.println("Computer: " + computerWins);
        System.out.println("User: " + userWins);
        System.out.println();
    }
  }
}

RPS

import java.util.Scanner;

public class RPS {
    String comparableChoice;
    String userComparableChoice;

        public String startGame() {
        Scanner in = new Scanner(System.in);
        System.out.println("Choose, (1)Rock, (2)Paper or (3)Scissors");
        System.out.println("enter number: 1,2 or 3");
        int userChoice = in.nextInt();

        if (userChoice == 1){
            System.out.println("Your choice: Rock");
            userComparableChoice = "Rock";
        }
        else if (userChoice == 2){
            System.out.println("Your choice: Paper");
            userComparableChoice = "Paper";
        }
        else{
            System.out.println("Your choice: Scissors");
            userComparableChoice = "Scissors";
        }
        return userComparableChoice;
    }

    public String computerChoice(){
        int computerChoiceNumber = (int)(Math.random() * 10);
        if (computerChoiceNumber <= 3){
            System.out.println("Computer choice: Rock");
            comparableChoice = "Rock";
        }
        else if (computerChoiceNumber <= 6){
            System.out.println("Computer choice: Paper");
            comparableChoice = "Paper";
        }
        else{
            System.out.println("Computer choice: Scissors");
            comparableChoice = "Scissors";
        }
        return comparableChoice;
    }

    public boolean evaluateGame(String userFormal, String computerFormal){

        if (userFormal.equals(computerFormal)){
            System.out.println("It's a tie");

        }
        if (userFormal.equalsIgnoreCase("Rock") && computerFormal.equalsIgnoreCase("Scissors")){
            System.out.println("You won!");
            return true;
        }
        if (userFormal.equalsIgnoreCase("Paper") && computerFormal.equalsIgnoreCase("Rock")){
            System.out.println("You won");
            return true;
        }
        if (userFormal.equalsIgnoreCase("Scissors") && computerFormal.equalsIgnoreCase("Paper")){
            System.out.println("You won");
            return true;
        }
        else {
            System.out.println("Computer won");
            return false;
        }

    }

}

I would only call the method startGame() inside the main function and handle from there the entire logic of the game. The main method shouldn't be aware of the exact procedure of a game. It just wants to start a game and wants to know who won. As an output from that method you could return true if the user wins and false if the computer wins. If you want to handle a tie separately you can return an int value instead for indicating the outcome (-1 computer wins, 0 draw, 1 user wins).

Inside the startGame() method you can call the game logic functions/your algorithm. In pseudo code it would look sth like this:

public int startGame()
{
    userChoice();
    computerChoice();
    return evaluateWinner(); // Evaluate winner and returns -1, 0, 1 depending on the user and computer choice (i.e. who the winner is or if its a tie)
}

You can make the other methods of the RPS class private to have a clean interface for this class, allowing other classes only to start a game and prevent them interfering the game algorithm itself.


Need help optimizing PHP/MySQL code snippet I am sorry for the simple request, but I'm trying to figure out the best way to optimizing the following bit of code.  I hate using 20 lines of code when 12-15 will suffice.  I also don't like having to use intermediate variables if I don't need to.

$total_query_raw = "SELECT SUM( total ) AS total
                        FROM  `tblinvoices`
                        WHERE  `datepaid`
                            BETWEEN  '$start_date 00:00:00' AND  '$end_date 23:59:59'";

$total_query = mysqli_query($link, $total_query_raw);
    if (!$total_query) {
        echo "DB Error, could not query the database\n";
        echo 'MySQL Error: ' . mysqli_error();
        exit;
    }
$total_result = mysqli_fetch_array($total_query);
$grand_total = $total_result['total'];
echo "Grand Total: $" . number_format($grand_total,2) . "<br>";
mysqli_free_result($total_query);

you might want to write it like this

$total_query_raw = "SELECT SUM( total ) AS total
                        FROM  `tblinvoices`
                        WHERE  `datepaid`
                            BETWEEN  '$start_date 00:00:00' AND  '$end_date 23:59:59'";

$total_query = mysqli_query($link, $total_query_raw);

if (!$total_query) {
    echo "DB Error, could not query the database\n";
    echo 'MySQL Error: ' . mysqli_error();
    exit;
} else {
    $total_result = mysqli_fetch_array($total_query);
    $grand_total = $total_result['total'];
    echo "Grand Total: $" . number_format($grand_total,2) . "
";
    mysqli_free_result($total_query);
}

otherwise you are going to try and perform action on something that you know will give you an error, that is the whole reason for the if statement,  so put the rest of that stuff in the else statement.

and I am not sure but I think you can write the last part of it like this

I think that it will do the assignment inside the if statement and return a 1 or 0 (which relates to true or false)
but I may be thinking of another language that allows you to do this

if ($total_query = mysqli_query($link, $total_query_raw))
{
    $total_result = mysqli_fetch_array($total_query);
    $grand_total = $total_result['total'];
    echo "Grand Total: $" . number_format($grand_total,2) . "
";
    mysqli_free_result($total_query);
}
else
{
    echo "DB Error, could not query the database\n";
    echo 'MySQL Error: ' . mysqli_error();
    exit;
}

UPDATE

ok so I did a little research on the Declaration & Assignment inside the condition, and based of THIS ANSWER you can perform the assignment inside the If Condition.

it does sound like you have to assign a "truthy" value?  not sure exactly what that means. I assume that if the assignment is good, it will return True but if the assignment is bad it will return False.


Count the number of leaders in both sides (slices) of an array in C#. Codility EquiLeaders task EquiLeader

  A non-empty array A consisting of N integers is given.
  The leader of this array is the value that occurs in more than half of
  the elements of A.

  An equi leader is an index S such that 0 ≤ S < N − 1 and two sequences
  A[0], A[1], ..., A[S] and A[S + 1], A[S + 2], ..., A[N − 1] have
  leaders of the same value.

  For example, given array A such that:

A[0] = 4
A[1] = 3
A[2] = 4
A[3] = 4
A[4] = 4
A[5] = 2 we can find two equi leaders:

  0, because sequences: (4) and (3, 4, 4, 4, 2) have the same leader,
  whose value is 4. 2, because sequences: (4, 3, 4) and (4, 4, 2) have
  the same leader, whose value is 4. The goal is to count the number of
  equi leaders.

  Write a function:

  class Solution { public int solution(int[] A); }

  that, given a non-empty array A consisting of N integers, returns the
  number of equi leaders.

  For example, given:

A[0] = 4
A[1] = 3
A[2] = 4
A[3] = 4
A[4] = 4
A[5] = 2 the function should return 2, as explained above.

  Write an efficient algorithm for the following assumptions:

  N is an integer within the range [1..100,000]; each element of array A
  is an integer within the range [−1,000,000,000..1,000,000,000].

This is the code I wrote, final score is a 100%. It took me a couple of tries (and days) like most of the solutions I'm posting here. Is it good code? What can be improved? I saw a question about SOLID a couple of hours ago, and that led me to start reading about the subject. This exercises are very specific and won't be extended, so (I know I'm not applying this principles here) should I still apply those principles to this kind of exercises (to practice and get used to them) or just leave it to real life extensible things?

using System;
using System.Collections.Generic;

class Solution {
    public int solution(int[] A) {
        Dictionary<int, int> leftPartition = new Dictionary<int, int>();
        Dictionary<int, int> rightPartition = new Dictionary<int, int>();

        int leadersCounter = 0;
        int leftPartitionSize = 1, rightPartitionSize = A.Length;
        int candidate, leader;

        for (int index = A.Length - 1; index >= 0; index--)
        {
            candidate = A[index];
            if (rightPartition.ContainsKey(candidate))
            {
                rightPartition[candidate]++;
            }
            else
            {
                rightPartition.Add(candidate, 1);
            }
        }
        leader = A[0];
        for (int index = 0; index < A.Length; index++)
        {
            candidate = A[index];
            if (leftPartition.ContainsKey(candidate))
            {
                leftPartition[candidate]++;
            }
            else
            {
                leftPartition.Add(candidate, 1);
            }
            rightPartition[candidate]--;
            rightPartitionSize--;
            if (leftPartition[candidate] > leftPartitionSize / 2)
            {
                leader = candidate;
            }
            if (leftPartition[leader] > leftPartitionSize / 2 && rightPartition[leader] > rightPartitionSize / 2 )
            {
                leadersCounter++;
            }
            leftPartitionSize++;
        }
        return leadersCounter;
    }
}

If you need to get a value of a Dictionary<TKey, TValue> you shouldn't us ContainsKey() together with the Item property getter but TryGetValue(), because by using ContainsKey() in combination with the Item getter you are doing the check if the key exists twice.
From the refernce source

public bool ContainsKey(TKey key)
{
    if (key == null) throw new ArgumentNullException("key");

    TValue throwAwayValue;
    return TryGetValue(key, out throwAwayValue);
}

public TValue this[TKey key]
{
    get
    {
        TValue value;
        if (!TryGetValue(key, out value))
        {
            throw new KeyNotFoundException();
        }
        return value;
    }
    set
    {
        if (key == null) throw new ArgumentNullException("key");
        TValue dummy;
        TryAddInternal(key, value, true, true, out dummy);
    }
}

Multiple declarations of variables on a single line should be avoided because it reduces the readability of the code.
If the type is clear from the right-hand-side of an assignment one should use var instead of the concrete type.
You should declare the variables as near to their usage as possible. This makes reading the code easier as well.
Althought this is from a programming excercise please note that methods should be named using PascalCase casing and method parameters should be named using camelCasecasing. Meaning solution should be Solution and A should be a if we talk about casing.

Implementing the mentioned points (without casing stuff) will look like this

public int solution(int[] A)
{
    var leftPartition = new Dictionary<int, int>();
    var rightPartition = new Dictionary<int, int>();

    for (var index = A.Length - 1; index >= 0; index--)
    {
        var candidate = A[index];
        rightPartition.TryGetValue(candidate, out int value);
        rightPartition[candidate] = value + 1;
    }

    var leadersCounter = 0;
    var leftPartitionSize = 1;
    var rightPartitionSize = A.Length;
    var leader = A[0];

    for (var index = 0; index < A.Length; index++)
    {
        var candidate = A[index];

        leftPartition.TryGetValue(candidate, out int value);
        leftPartition[candidate] = value + 1;

        rightPartition[candidate]--;
        rightPartitionSize--;

        if (leftPartition[candidate] > leftPartitionSize / 2)
        {
            leader = candidate;
        }
        if (leftPartition[leader] > leftPartitionSize / 2 && rightPartition[leader] > rightPartitionSize / 2)
        {
            leadersCounter++;
        }
        leftPartitionSize++;
    }
    return leadersCounter;
}


MSSQL query with join and where clause I've the following tables

Candidates
Id  Name  PicId
 1  Tom   NULL
 2  James 1
 3  Bob   NULL
 4  Manny 2

Attachments
Id  Name
 1  One.jpg
 2  Two.doc
 3  Three.rtf

The join table AttachmentCandidate
CandidateId  AttachmentId
    3             1
    3             2
    4             3

So from the above tables it shows that a candidate's 'PicId' is nullable & one candidate can have 0 or many attachments.

Now the query what I wish to create is the one which should return me Candidate's Name, PicId, & Attachment Name, but I don't want the candidates who neither have a PicId nor any Attachments.

I was able to achieve it with a subquery. Is there any better way it can be done??

SELECT r.cname,r.picId,r.attName FROM
(SELECT Candidates.Name as cname,
Candidates.PicId as picId,
Attachments.Name as attName
FROM Candidates LEFT JOIN AttachmentCandidate
ON Candidates.Id = AttachmentCandidate.CandidateId
LEFT JOIN Attachments
ON AttachmentCandidate.AttachmentId = Attachments.Id) as r
WHERE (r.picId IS NOT NULL OR r.attName IS NOT NULL)

Result
cname   picId  attName
James   1      NULL
Bob     NULL   One.jpg
Bob     NULL   Two.doc
Manny   3      Three.rtf

And the candidate 'Tom' not is present in the result since he doesn't have a picId nor any attachments

My only real comment to make is that the subquery is irrelevant. You can achieve the same result without it.

When specifying your column aliases in the outer query. There is no need for you to use AS picid as the column is already called picid.

You could also use table aliases as well to shorten some of the statements. I personally prefer to see aliases than fully qualified table names. Assuming everyone sticks to a naming convention of course. My normal rule of thumb is that each capital letter of the table becomes the alias.

Also, as we can remove the subquery, we can change the WHERE clause so it is no longer filtering out the attachment name but instead the PK (WHERE a.Id IS NOT NULL).

For the columns being returned, can we make them more explicit? i.e. CandidateName instead of CName, etc.

Here is the modified version of your query using the changes above:

SELECT
     c.Name     AS CandidateName
    ,c.picid    AS PicId
    ,a.Name     AS AttachmentName
FROM Candidates c
LEFT JOIN AttachmentCandidate ac ON c.Id = ac.CandidateId
LEFT JOIN Attachments a ON ac.AttachmentId = a.Id
WHERE c.picid IS NOT NULL
OR a.Id IS NOT NULL

I'm not sure whether the formatting is just bad when you created your question or whether your SQL query is like that? If so, it's badly formatted and difficult to read. I try and break up each part of the query into separate segments so it's easy to read and maintain by others:

SELECT
     r.cname
    ,r.PicId
    ,r.attName
FROM
(
    SELECT
        Candidates.Name     AS cname
        ,Candidates.PicId   AS picid
        ,Attachments.Name   AS attname
    FROM Candidates
    LEFT JOIN AttachmentCandidate ON Candidates.Id = AttachmentCandidate.CandidateId
    LEFT JOIN Attachments ON AttachmentCandidate.AttachmentId = Attachments.Id
) AS r
WHERE (r.PicId IS NOT NULL OR r.attName IS NOT NULL)

As you requested an alternative, i'll give you one. Essentially it's pretty much the reverse of what you're doing. I'm gathering all of the candidates with attachments first and then I RIGHT JOIN back onto the candidates table and filter out the ones without a PicId.

SELECT
    c.Name      AS CandidateName
    ,c.PicId    AS PicId
    ,a.Name     AS AttachmentName
FROM AttachmentCandidate ac
INNER JOIN Attachments a ON a.Id = ac.AttachmentId
RIGHT JOIN Candidates c ON c.Id = ac.CandidateId
WHERE c.picid IS NOT NULL
OR a.Id IS NOT NULL


Searching the same values in an array If I am not mistaken, this method has a complexity \$O(N^2)\$:

int[] exampleArray = { 4, 6, 2, 2, 6, 6, 1, 55, 55, 2, 5, 6, 90, 8, 8, 8, 10, 70 };
int k = GetMaximumCycle(exampleArray);

public static int GetMaximumCycle(int[] anArray)
{
   int length = anArray.Length;
   int result = 0;
   for (int i = 0; i < length; i++)
   {
      for (int j = 0; j < length; j++)
      {
          if (anArray[i] == anArray[j])
          {
             if (!(result > Math.Abs(i - j)))
                 result = Math.Abs(i - j);
          }
      }
   }
   return result;
}

I compare values and gets the maximum difference between items of array.
The result variable is 10.

And if an input array is very large, this code works slowly. Is it possible to simplify the for loops or remove the second for loop to have a complexity of \$O(N)\$?

You can use a Dictionary<int, int> of positions of the first occurrence of each element value.
Next, iterating over all elements of the input array, you do:

If the positions[value] is empty, store the current index i in it: positions[value] = i.
Else, calculate the distance (or cycle length).
If this distance is greater than maximum, save it as a new maximum.

This algorithm has time complexity \$O(n)\$.

The code:

public static class CycleSearch
{
    public static int GetMaximumCycle(int[] array)
    {
        Dictionary<int, int> positions = new Dictionary<int, int>();

        int result = 0;
        for (int i = 0; i < array.Length; i++)
        {
            int value = array[i];
            int position;
            if (!positions.TryGetValue(value, out position))
            {
                positions[value] = i;
            }
            else
            {
                int cycleLength = i - position;
                if (cycleLength > result)
                {
                    result = cycleLength;
                }
            }
        }
        return result;
    }
}

Usage:

int[] exampleArray = { 4, 6, 2, 2, 6, 6, 1, 55, 55, 2, 5, 6, 90, 8, 8, 8, 10, 70 };
Console.WriteLine(CycleSearch.GetMaximumCycle(exampleArray));

Output:

  10


Generate all possible combinations of letters in a word This is a JavaScript function that returns ALL the possible combinations of whatever word the user enters. I am looking to clean this code up a bit...any and all suggestions are welcome!

  function allAnagrams (word) {
  if (word.length < 2) {
    return [word];
  } else {
      var allAnswers = [];
      for (var i = 0; i < word.length; i++) {
        var letter = word[i];
        var shorterWord = word.substr(0, i) + word.substr(i + 1, word.length - 1);
        var shortwordArray = allAnagrams(shorterWord);
        for (var j = 0; j < shortwordArray.length; j++) {
          allAnswers.push(letter + shortwordArray[j]);
        }
      }
      return allAnswers;
  }
}

allAnagrams("")

I'm going to throw my hat in the ring with the iterative version of Heap's method of generating permutations. It took me a while to get right, as my reference source has a typo in the algorithm -_-.

function swap(chars, i, j) {
    var tmp = chars[i];
    chars[i] = chars[j];
    chars[j] = tmp;
}

function getAnagrams(input) {
    var counter = [],
        anagrams = [],
        chars = input.split(''),
        length = chars.length,
        i;

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

    anagrams.push(input);
    i = 0;
    while (i < length) {
        if (counter[i] < i) {
            swap(chars, i % 2 === 1 ? counter[i] : 0, i);
            counter[i]++;
            i = 0;
            anagrams.push(chars.join(''));
        } else {
            counter[i] = 0;
            i++;
        }
    }

    return anagrams;
}

With some sample timings to count the anagrams of "codereview":

$ time node anagrams-heap-iterative.js
3628800

real    0m0.855s
user    0m0.000s
sys     0m0.015s

$ time node anagrams-megawac.js
3628800

real    0m2.286s
user    0m0.000s
sys     0m0.000s


Parallel Calls to External WCF Service - ASP.NET Web Api I have come across a situation where I feel running some code in parallel will greatly improve performance, but I am concerned about the implementation and am looking for some confirmation. Take the following controller resources
[HttpGet]
[Route("{id}")]
public IHttpActionResult Get(int id)
{
     return Ok(_context.Get(id));
}

[HttpGet]
[Route("async/{id}")]
public async Task<IHttpActionResult> Get(int id)
{
     return Ok(await _context.GetAsync(id));
}

The contents of Get(...) function are fairly straight forward. A WCF call is made using the passed id to retrieve document a object. That document object contains a list of associated material numbers for which I need to make a separate WCF call for each to gather additional data
public DocumentModel Get(int id)
{
     //Call to WCF service
     DocumentModel model = _documents.Get(id);

     //For each material, make a call to WCF service
     //Potential for bottle neck due to possibility of large number of associated materials
     foreach (Material material in model.Materials)
         model.MaterialModels.Add(_materials.Get(material.Number));

     return model;
}

It is worth noting that I am using a library developed and maintained by a colleague to call WCF services that are hosted on the same VM, so lets just say that I don't have access to modify this library for sake of the question. Retrieving a material looks like the following
public MaterialModel Get(string number)
{
     using(var client = new MaterialClient(out WcfResult result))
    {
         client.Get(number, out Material material, out Result result);
         return new MaterialModel(material);
    }
}

Everything works as expected, but as previously noted its possible for a significant number of materials to be returned (~20-60). Typically, I wouldn't encounter a problem like this since I would be utilizing HATEOAS and supplying the resource for each material in the DocumentModel object, however; this is an API for internal business use and the business requires all the data be in the response so I am left with little choice. So I made a separate implementation that will retrieve that material data in parallel
public async Task<DocumentModel> GetAsync(int id)
{
     //Call to WCF service
     DocumentModel model = _documents.Get(id);
     IEnumerable<Task<MaterialModel>> tasks = model.Materials.Select(material =>
          _materials.GetAsync(material.Number));

     MaterialModel[] result = await Task.WhenAll(tasks);
     model.MaterialModels.AddRange(result);
     return model;
}

Here is where my concern is. There are no Async implementations in the library, so my initial thought was to just do the following
public Task<MaterialModel> GetAsync(string number)
{
     //This concerns me
     return Task.Run(() =>
     {
         using(var client = new MaterialClient(out WcfResult result))
         {
              client.Get(number, out Material material, out Result result);
              return new MaterialModel(material);
         }
     });
}

Both implementations work, but when I run them both side by side the one making calls in parallel is significantly faster and diagnostics shows the flow is what I expected as well

Is this an acceptable implementation or are there some pitfalls to this I may not be aware about?

I think instead of creating an unknown amount of task using PLinq would make it simpler and have control over the number of task inflight/created
something like
var result = model.Materials
                .AsParallel()
                .WithDegreeOfParallelism(8) // whatever you want or leave it out
                .Select(material => _materials.Get(material.Number));

model.MaterialModels.AddRange(result);

This way it's not trying to make synchronous code async and can limit the amount of task inflight vs unlimited amount.


Doubly linked list in Rust I have just started learning Rust, and in order to try to get the hang of references, ownership and mutability, I have attempted to make a doubly linked list. It now compiles, and the add function seems to be working as intended.

There is one implementation detail that I am curious about, and in general I'm wondering whether my code violates any good practice. Also, related to this, have I painted myself into any kind of corner if I want to extend this code? (Making add() keep the list ordered, using generics, implementing Iterator, and so on.)

use std::rc::{Rc, Weak};
use std::cell::RefCell;

#[derive(Debug)]
struct Node {
    value: i32,
    next: RefCell<Option<Rc<Node>>>,
    prev: RefCell<Weak<Node>>,
}

impl Node {
    pub fn add(&self, this: &Rc<Node>, i: i32) {
        if let Some(ref r) = *self.next.borrow() {
            r.add(r, i);
            return;
        };
        *self.next.borrow_mut() = Some(Rc::new(Node {
            value: i,
            next: RefCell::new(None),
            prev: RefCell::new(Rc::downgrade(&this)),
        }));
    }
}

#[derive(Debug)]
struct List {
    head: RefCell<Option<Rc<Node>>>,
}

impl List {
    pub fn add(&self, i: i32) {
        if let Some(ref r) = *self.head.borrow() {
            r.add(r, i);
            return;
        };
        *self.head.borrow_mut() = Some(Rc::new(Node {
            value: i,
            next: RefCell::new(None),
            prev: RefCell::new(Weak::new()),
        }));
    }
}

fn main() {
    let list = List {
        head: RefCell::new(None),
    };
    println!("{:?}", list);
    list.add(1);
    list.add(2);
    println!("{:?}", list);
}

The detail I'm curious about is whether the if let parts can be done with a match. Or, more specifically, can it be done without the extra return call. No matter what I tried (both match and if let with else), the borrow() got in the way of the borrow_mut() in the None arm, and I couldn't get the pattern matching to work without the borrow().

Before looking at your code, I want to give a general notice: don't use linked lists. They are almost always the wrong choice of data structure. They are useful primarily pedagogically.

Furthermore, I should warn you that implementing a doubly-linked list will not look like typical Rust. You can't implement a doubly-linked list in Rust without using escape hatches like Rc, RefCell, or raw pointers. These escape hatches are necessary when doing low level things like implementing a data structure, but normal Rust code avoids them as much as possible. If you want to get used to "thinking in Rust" you should pick a different exercise.

The first thing about your code I note is that Node::add function. It takes a pointer to itself twice. self and this. Having two different pointers to the same thing is unhelpful. Just drop the &self parameter and refer to this consistently in the function. Then you can call the function as Node::add.

The second thing is that you have RefCells containing Rc's. However, the standard way is to have it the other way around. It should be Rc<RefCell<?>> not RefCell<Rc<?>>. You'll find that your current design does not let you modify the value of each node.

A third thing is the lack of mutable references. Your add method allows adding to your list without the list being mutable. That doesn't really make sense, your add method really should take a mutable reference to the list. You get away with this because of the previous point.

Finally, you would be better off not using recursion to add the item to the end of list. Recursion is much slower than iteration. It would make more sense to scan through the nodes looking for the end of the list in a loop than in a recursive function call.


Two click functions doing the same thing with difference the selector I have the following jQuery click functions:

$childCheckbox.click(function(){
    if(!$(this).hasClass('checked') && !$parentCheckbox.hasClass('checked')){
        $(this).attr('checked', 'checked').addClass('checked');
        $parentCheckbox.prop('indeterminate',true);
    }
    else if($(this).hasClass('checked')){
        $(this).removeAttr('checked', 'checked').removeClass('checked');
        $parentCheckbox.prop('indeterminate',false);
    }
});

$subparentCheckbox.click(function(){
    if(!$(this).hasClass('checked') && !$parentCheckbox.hasClass('checked')){
        $(this).attr('checked', 'checked').addClass('checked');
        $parentCheckbox.prop('indeterminate',true);
    }
    else if($(this).hasClass('checked')){
        $(this).removeAttr('checked', 'checked').removeClass('checked');
        $parentCheckbox.prop('indeterminate',false);
    }
});

As you can see both are exactly the same with the exception of the "selector". Is there a good way to combine these to minimize code?

Here are 3 - there are more, no doubt

Give both checkboxes a class and use that as the selector

$(".someClass").click( ... )

Declare the event handler elsewhere, and use it anywhere:

function checkboxClick(event) {...};

$subparentCheckbox.click(checkboxClick);
$childCheckbox.click(checkboxClick);

Use .add to combine the two checkboxes into one collection before calling .click like so

$subparentCheckbox.add($childCheckbox).click(...)`


Map/Reduce multi level JSON object I have a list of rescues for ambulance drivers in different states that are displayed on a kanban board. My JSON object is divided by the status of the rescues, and then grouped by driver. The structure is like this:
--Status (unassigned/assigned/arrived/rescued)
----Driver group
-------Rescue list
The rescues have a datetime and lat/long associated with them, so I can use map/reduce to find the rescue that the drivers attended most recently, and show that location on a map.
One issue is that a driver group can exist in one swimlane and none of the other.
I want to: filter the list down to the arrived/rescued states only, (although logically this may not be necessary as it's not possible to have an arrival time without a driver associated, but this is out of scope) and for each driver group return their latest rescue by arrival time or rescue time. This will leave me with a distinct list of driver groups, with only the most recent rescue for that group
Here's the working code:
let groups = cases
                .filter(swimlane => parseInt(swimlane.rescueStatus) >= 3)
                .map(groups => groups.rescuerGroups)
                .map(rescuerGroups => rescuerGroups)
                .reduce((result, current) => {

                  if(result.length === 0){
                    return current;
                  }
                  else{

                    current.forEach(currentRescueGroup => {

                      let index = result.findIndex(parentRescueGroup => {

                        return parseInt(parentRescueGroup.rescuer1) == parseInt(currentRescueGroup.rescuer1) &&
                            parseInt(parentRescueGroup.rescuer2) == parseInt(currentRescueGroup.rescuer2)

                      })

                      index > -1 ?
                        result[index].rescues = result[index].rescues.concat(currentRescueGroup.rescues)
                        :
                        result.push(currentRescueGroup);

                    })

                    return result;

                  }}, [])
                .map(rescueGroup => {

                  let maxRescue = rescueGroup.rescues.reduce((current, previous) => {

                  let currentTime = new Date(current.ambulanceArrivalTime) > (new Date(current.rescueTime) || new Date(1901, 01, 01)) ? current.ambulanceArrivalTime : current.rescueTime;
                  let previousTime = new Date(previous.ambulanceArrivalTime) > (new Date(previous.rescueTime) || new Date(1901, 01, 01)) ? previous.ambulanceArrivalTime : previous.rescueTime;

              return previousTime > currentTime ? current : previous;

            })

             return {
                       rescuer1Abbreviation: rescueGroup.rescuer1Abbreviation,
                       rescuer2Abbreviation: rescueGroup.rescuer2Abbreviation,
                       latestLocation: maxRescue.latLngLiteral,
                       rescues: rescueGroup.rescues
                     }
           })

And here's a JSBin including the working code:
https://jsbin.com/hocasob/edit?js,console
Any help to make the reduce sections simpler would be much appreciated

Turning a comment into a review (I provide an alternative implementation, but the basic ideas can be applied to the OP implementation).
First, some general thoughts.
As I mention in the comments regarding the parseInt function, a linter can help spot any problems. A popular one is ESLint. But perhaps more to the point, the data seems to already be a number, so there shouldn't be any need to parse it.
It can help to clean up the code quite a bit by extracting the filters, reducers, mappers, sorters, etc. outside as a variable and give them a descriptive name, for example:
// filter
const arrivedOrRescued = ({rescueStatus}) => rescueStatus >= 3

// sorter
const byArrivalOrRescueTime = (a, b) =>
  Date.parse(a.ambulanceArrivalTime || a.rescueTime)
    - Date.parse(b.ambulanceArrivalTime || b.rescueTime)

// reducer
const byRescuers = (acc, {rescuerGroups}) => {
  rescuerGroups.forEach(({
    rescues
    , rescuer1Abbreviation: r1
    , rescuer2Abbreviation: r2
  }) => {
    let group = `${r1}-${r2}`
    acc.has(group) ? acc.get(group).push(...rescues) : acc.set(group, rescues)
  })

  return acc
}

so getting a Map (just an example) by rescuers out of the swimlanes could then become
let rescuers = cases.filter(arrivedOrRescued).reduce(byRescuer, new Map())

and sorting their rescues could become
rescuers.forEach((rescues, abbrevs) => rescues.sort(byArrivalOrRescueTime))

after which the latest rescue is the last element of the array now that they have all been collected into a single array and sorted.
One thing I would like to promote here when there can be somewhat complex nested objects in the data, is to use JSDoc comments to document the data shape.
For example:
/*
 * @typedef {Object} Swimlane
 * @property {Array.<RescuerGroup>} rescuerGroups
 * @property {Number} rescueStatus
 * @property {String} rescueStatusName
 *
 * @typedef {Object} RescuerGroup
 * @property {Array.<Rescue>} rescues
 * @property {?Number} rescuer1
 * @property {?Number} rescuer2
 * @property {?String} rescuer1Abbreviation
 * @property {?String} rescuer2Abbreviation
 *
 * @typedef {Object} Rescue
 * @property {?String} ambulanceArrivalTime
 * @property {?String} callDateTime
 * @property {?String} callerName
 * @property {String} callerNumber
 * @property {?Number} callOutcomeId
 * @property {Number} emergencyCaseId
 * @property {Number} emergencyCodeId
 * @property {Number} emergencyNumber
 * @property {Number} isLargeAnimal
 * @property {Number} latitude
 * @property {Number} longitude
 * @property {{lat: Number, lng: Number}} latLngLiteral
 * @property {String} location
 * @property {Array.<String>} patients
 * @property {Number} rescuer1
 * @property {Number} rescuer2
 * @property {?String} rescueTime
 * @property {Number} RescueStatus
 */

This makes it easier for others especially in data transformations.
Just a note that I haven't tested any of this, so take the above code with a grain of salt. Also, many will probably tell me to use more braces and semicolons.


Level system in a gamification platform Currently, I'm working on gamification platform and I have the following rules for achieving a new level:

Level 0: When user registers
Level 1: When user confirms account
Level 2: When user completes a quiz
Level 3: When user completes a mission and publish more than 5 comments in the blog
Level 4: When user completes more than two missions and completes a campaign (campaign means donating money for something)
Level 5: Raised at least $50 in campaigns and complete at least two campaigns

Now, I have the following database where I'll keep a track of each user action (e.g. complete_quiz, complete_mission, etc.):

Achievements

id (int)
event (varchar)
amount (int) # used when event has specific value, e.g. money for complete_campaign event
user_id (int)
date_created (int)

When the user make any action (e.g. complete_mission), I'll insert the action in the table above and will call the following method to check if user has new level unlocked:

public function checkIfAchievmentUnlocksNewLevel($userObj)
{
    $currentLevel = $userObj->level;

    $nextLevel = ++$currentLevel

    $isLevelUnlocked = false;

    switch($nextLevel)
    {
        case 0:
        case 1:
            // Each registered user will have these levels by default.
        break;
        case 2:
            $completedQuiz = $this->db->select("SELECT " . $this->fieldsList . " FROM " . $this->table . " WHERE event = :event AND user_id = :user_id ", array(':event' => 'complete_quiz', ':user_id' => $userObj->id));
            $isLevelUnlocked = count($completedQuiz) ? true : false;
        break;
        case 3:
            $completedMissions = $this->db->select("SELECT " . $this->fieldsList . " FROM " . $this->table . " WHERE event = :event AND user_id = :user_id", array(':event' => 'complete_mission', ':user_id' => $userObj->id));
            $wildwireComments = $this->db->select("SELECT " . $this->fieldsList ." FROM " . $this->table . " WHERE event = :event AND user_id = :user_id", array(':event' => 'wildwire_comment', ':user_id' => $userObj->id));
            $isLevelUnlocked = (count($completedMissions) && count($wildwireComments) >= 5) ? true : false;
        break;
        case 4:
            $completedMissions = $this->db->select("SELECT " . $this->fieldsList . " FROM " . $this->table . " WHERE event = :event AND user_id = :user_id", array(':event' => 'complete_mission', ':user_id' => $userObj->id));
            $completedCampaigns = $this->db->select("SELECT " . $this->fieldsList ." FROM " . $this->table . " WHERE event = :event AND user_id = :user_id", array(':event' => 'complete_campaign', ':user_id' => $userObj->id));
            $isLevelUnlocked = (count($completedMissions) >= 2 && count($completedCampaigns)) ? true : false;
        break;
        case 5:
            $campaignRaisedMoney = $this->db->select("SELECT SUM(amount) FROM " . $this->table . " WHERE event = :event AND user_id = :user_id", array(':event' => 'complete_campaign', ':user_id' => $userObj->id));
            $completedCampaigns = $this->db->select("SELECT " . $this->fieldsList ." FROM " . $this->table . " WHERE event = :event AND user_id = :user_id", array(':event' => 'complete_campaign', ':user_id' => $userObj->id));
            $isLevelUnlocked = (count($compaignRaisedMoney) >= 50.00 && count($completedCampaigns) >= 2) ? true : false;
        break;
    }

    if($isLevelUnlocked)
    {
        $userHelper->setLevel($nextLevel);
    }

}

This code is working now, but I want to refactor it and am looking for any suggestions on how to improve it.

I would agree with @ardaozkal that an XP based system would be better (more flexible, etc).

But assuming that your approach is good enough for your situation, I also have some suggestions:

You have quite a bit of duplication, for example the completed mission queries or the completed campaign queries. You should extract those to their own function and reuse that.
Your code could generally benefit from more functions. Other candidates would be hasCompletexXMissions($db, $amount) or getCompletedMissionCount($db).
If this project is a bit more complex, you definitely want to add classes, such as Mission, which may then contain the functions mentioned above.
Your lines are too long, which seriously impacts readability. 226 characters don't even fit on many modern screens. I would aim for about 80 characters.
You have a bit too much vertical whitespace for my taste. Do the first three statements really all need their own paragraph?
You can get rid of $currentLevel, it's only used in one location, and the name doesn't really add any value. Just use $userObj->level directly.


Various different box shadows The following code will be used for educational purposes. It is to demonstrate various way that the box-shadow property can be used. Any feedback on areas where I have not used best practices would be great.

Working example

HTML:

<!DOCTYPE HTML>

    <title>Box Shadow</title>
    <link rel="stylesheet" type="text/css" href="boxshadow.css">

CSS:

body{
        background: #ccc;
        margin: 0;
}
.container{
    padding: 20px 50px;
    overflow: hidden;
}
.box{
    background-color: #fff;
    width: 500px;
    height: 200px;
}
.left{
    float: left;
}
.right{
    float: right;
}
/* Basic Shadow */
.shadow1{
    box-shadow: 5px 5px 0 rgba(150,150,150,0.5);
}
/* Inset Shadow and blur radius Variety */
.shadow2{
    box-shadow: inset 0 0 10px black;
}
/* Inset Shadow and blur radius */
.shadow3{
  background: #F2F2F2;
  box-shadow: inset 3px 3px 3px #BFBFBF, inset -3px -3px 3px #8C8C8C;
  border-radius: 10px;
}
/* Spread radius Shadow */
.shadow4{
    box-shadow: 0 20px 10px -10px rgba(150,150,150,0.8);
}
/* Curved Shadows */
.shadow5{
    position: relative;
    overflow:hidden;
}
.shadow5:before{
    content: "";
    position: absolute;
    z-index: 1;
    width: 96%;
    top: -15px;
    height: 15px;
    left: 2%;
    border-radius: 100px / 5px;
    box-shadow: 0 0 39px rgba(0,0,0,0.6);
}
.shadow5:after{
    content: "";
    position: absolute;
    z-index: 1;
    width: 96%;
    bottom: -15px;
    height: 15px;
    left: 2%;
    border-radius: 100px / 5px;
    box-shadow: 0 0 39px rgba(0,0,0,0.6);
}
.shadow6{
    position: relative;
    overflow:hidden;
}
.shadow6:before{
  content: "";
  position:absolute;
  z-index: 1;
  width:10px;
  top: 5%;
  height: 90%;
  left: -10px;
  border-radius: 5px / 100px;
  box-shadow:0 0 13px rgba(0,0,0,0.6);
}
.shadow6:after{
  content: "";
  position:absolute;
  z-index: 1;
  width:15px;
  top: 5%;
  height: 96%;
  right: -15px;
  border-radius: 5px / 100px;
  box-shadow:0 0 13px rgba(0,0,0,0.6);
}

they are all .box class.

I am thinking that you should get rid of the container divs and get rid of the .box class and give it to all divs because after you get rid of the container divs all the divs will be the boxes.

you should do one of those fancy HTML5 Tags for the content section instead of having a container classed div.

check this out, I have removed some tags and some CSS classes to make the code a little cleaner.  the output is slightly different but I think this is a little bit better.  if you need a width on the content you can add that as well.

what happens when the browser window is resized is that the boxes are now hugging the left and right sides of the content,  if you want to keep it to two rows at the max you would just add a width to the content tag

UPDATE

I changed some more things to make this so that you could add more examples with ease, and so that it would be more view-able on mobile devices.

This is the New Version that I came up with

I removed the Left and Right Classes, and moved the float left into the Styling for the Div tags, this way the boxes will wrap the viewport is resized.

I also changed the size of the boxes.

I removed the container classes as they weren't needed.

I added the styling from the box class to the div's themselves.

I am apparently wrong about sizing the content tag that I created, maybe it's not proper HTML5 or I was doing something wrong.

Here is the code that I have after the changes that I made

<!DOCTYPE HTML>

    <title>Box Shadow</title>
    <link rel="stylesheet" type="text/css" href="boxshadow.css">

  <content>

  </content

CSS

body{
    background: #ccc;
    margin: 0;
}
content{
    width:1000px;
}
div {
    background-color: #fff;
    width: 500px;
    height: 200px;
    padding:15px;
    margin:15px;
}
.left{
    float: left;
}
.right{
    float: right;
}
/* Basic Shadow */
.shadow1{
    box-shadow: 5px 5px 0 rgba(150,150,150,0.5);
}
/* Inset Shadow and blur radius Variety */
.shadow2{
    box-shadow: inset 0 0 10px black;
}
/* Inset Shadow and blur radius */
.shadow3{
    background: #F2F2F2;
    box-shadow: inset 3px 3px 3px #BFBFBF, inset -3px -3px 3px #8C8C8C;
    border-radius: 10px;
}
/* Spread radius Shadow */
.shadow4{
    box-shadow: 0 20px 10px -10px rgba(150,150,150,0.8);
}
/* Curved Shadows */
.shadow5{
    position: relative;
    overflow:hidden;
}
.shadow5:before{
    content: "";
    position: absolute;
    z-index: 1;
    width: 96%;
    top: -15px;
    height: 15px;
    left: 2%;
    border-radius: 100px / 5px;
    box-shadow: 0 0 39px rgba(0,0,0,0.6);
}
.shadow5:after{
    content: "";
    position: absolute;
    z-index: 1;
    width: 96%;
    bottom: -15px;
    height: 15px;
    left: 2%;
    border-radius: 100px / 5px;
    box-shadow: 0 0 39px rgba(0,0,0,0.6);
}
.shadow6{
    position: relative;
    overflow:hidden;
}
.shadow6:before{
    content: "";
    position:absolute;
    z-index: 1;
    width:10px;
    top: 5%;
    height: 90%;
    left: -10px;
    border-radius: 5px / 100px;
    box-shadow:0 0 13px rgba(0,0,0,0.6);
}
.shadow6:after{
    content: "";
    position:absolute;
    z-index: 1;
    width:15px;
    top: 5%;
    height: 96%;
    right: -15px;
    border-radius: 5px / 100px;
    box-shadow:0 0 13px rgba(0,0,0,0.6);
}

Some other things that I noticed while pasting this new code is that you assign a z-index of 1 to some of your shadow styles and not to the others, I don't think that this is necessary for what you are doing here.  the z-index defaults to 1 or 0 if I remember right, and that should be fine for what you are doing here so you don't even need to add that in there.


Longest common subsequence (LCS) for multiple strings How would you improve this code? Particularly the check and check_all functions?

The time complexity of the algorithm of mlcs is \$O(|\Sigma|MN)\$, where \$\Sigma\$ is the alphabet, M is the number of strings and N is the length of the strings.
Is that right?

My analysis: candidates() performs \$O(|\Sigma|M)\$ operations and is called \$O(N)\$ times.

Based on the reviewed code posted before at
Multiple longest common subsequence (another algorithm)

def check(string, seq):
    i = 0
    j = 0
    while i < len(string) and j < len(seq):
        if string[i] == seq[j]:
            j += 1
        i += 1
    return len(seq) - j

def checkall(strings, seq):
    for x in strings:
        a = check(x, seq)
        if not a == 0:
            print(x, seq, a)
            return False
    return True

def mlcs(strings):
    """Return a long common subsequence of the strings.
    """
    if not strings:
        raise ValueError("mlcs() argument is an empty sequence")
    strings = list(set(strings)) # deduplicate
    alphabet = set.intersection(*(set(s) for s in strings))

    # indexes[letter][i] is list of indexes of letter in strings[i].
    indexes = {letter:[[] for _ in strings] for letter in alphabet}
    for i, s in enumerate(strings):
        for j, letter in enumerate(s):
            if letter in alphabet:
                indexes[letter][i].append(j)

    # Generate candidate positions for next step in search.
    def candidates():
        for letter, letter_indexes in indexes.items():
            candidate = []
            for ind in letter_indexes:
                if len(ind) < 1:
                    break
                q = ind[0]
                candidate.append(q)
            else:
                yield candidate

    result = []
    while True:
        try:
            # Choose the closest candidate position, if any.
            pos = None
            for c in candidates():
                if not pos or sum(c) < sum(pos):
                    pos = c
            letter = strings[0][pos[0]]
        except TypeError:
            return ''.join(result)
        for let, letter_indexes in indexes.items():
            for k, ind in enumerate(letter_indexes):
                ind = [i for i in ind if i > pos[k]]
                letter_indexes[k] = ind
        result.append(letter)

strings = []
# Alphabet for the strings.
sigma = ["a", "b", "c", "d"]
# Alphabet for the LCS.
sigmax = ["e", "f", "g"]
import random
Nx = 67 # Length of LCS.
N = 128 # Length of strings. N >= Nx.
M = 128 # Number of strings.
x = ""
for _ in range(Nx):
    x += random.choice(sigmax)
for _ in range(M):
    string = ""
    for _ in range(N):
        string += random.choice(sigma)
    indexes = list(range(N))
    random.shuffle(indexes)
    indexes = sorted(indexes[:len(x)])
    for j in range(len(x)):
        string = string[:indexes[j]]+x[j]+string[indexes[j]+1:]
    strings += [string]

#strings = ["abbab", "ababa", "abbba"]
#strings = ["abab", "baba", "abaa"]
#strings = ["bacda", "abcde", "decac"]
#strings = ["babbabbb", "bbbaabaa", "abbbabab", "abbababa"]
#strings = ["ab", "aba"]
#print("Strings:")
#print(strings)
l = mlcs(strings)
print("LCS:")
print(l, len(l), checkall(strings, l))

Conventions

It's highly recommended to have all imports at the top of your file.
It's not nice for readers to have a different environment than expected.

It's also better to be safe than sorry, so you may want to wrap your global code in a if __name__ == '__main__':.

One-line docstrings by convention have both """ on the same line.
You must be thinking of multi-line docstrings, where the last """ goes on a new line.

Code

checkall

Python has some strong readability conventions.
For example, in checkall you use the not operator after the equals operator.
This is strange, as you could just use the not equals operator.
But in Python if a != 0: is always true and false every time if a: is.
And so it's better to just use if a.

# Using two operators rather than one
if not a == 0:
    ...

# Comparing to value (twice).
if a != 0:
    ...

# What you want to be doing:
if a:
    ...

I like the idea of writing declarative statements from functional programming,
which can really help readability in checkall.

If we were to say what the algorithm is:
Go through the list of strings, if any of them after going through check are true return false, otherwise return true.

Which can be converted to:

def checkall(strings, seq):
    return not any(check(x, seq) for x in strings)

check

A bit of a 'out there' one, even though it's harder to think backward in algorithms, or at least for me.
Changing check to run backwards can improve readability.
Rather than doing n < len(...) you can just do n >= 0.

It's not that big a difference, so you may not want to think about doing it.
But it will save a few cycles on all the lens.

mlcs

poor variable names such as s, make reading a little harder, s is short for string, but could be super, special or soup.
It's much easier to comprehend enumerate(string). In changing it it will also not be hidden in the other one letter variables.

this lead to me not quickly understanding what you are doing in the bits after candidates.

There are other problems, candidates can be put in the for loop, rather than as a function. And you should try to reduce the 8 for-loops that the function has.