ResourceManager Singleton As I understand creating lots of resource managers in C# may be a bad idea so I thought that the best thing to do would be to create a singleton for this.

However, I'm not 100% if this is a good idea, or if this is a good way to implement it.

The main thing about the implementation I am unsure about is making the "Instance" private and creating number of static functions to expose the functionality. This means I can call Local.GetString instead of Local.Instance.GetString which is less clunky. I have not seen any singleton implementation which does this so I wonder if there is some problem with it.

I am also using locking but I don't see how to get around that so that is probably fine.

public class Locale
{
    // using System.Lazy<T> -- see http://msdn.microsoft.com/en-us/library/dd642331.aspx
    static readonly Lazy<Locale> lazy = new Lazy<Locale>(() => new Locale(), true);
    static readonly object _locker = new object();

    Dictionary<Type, ResourceManager> _managers;

    public Locale()
    {
        _managers = new Dictionary<Type, ResourceManager>();
    }

    static Locale Instance { get { return lazy.Value; } }

    public static ResourceManager GetManager(Type resourceType)
    {
        return Instance.GetResourceManager(resourceType);
    }

    public static string GetString(Type resourceType, string name)
    {
        var manager = Instance.GetResourceManager(resourceType);
        return manager.GetString(name);
    }

    public static string GetString(Type resourceType, string name, CultureInfo culture)
    {
        var manager = Instance.GetResourceManager(resourceType);
        return manager.GetString(name, culture);
    }

    public static object GetObject(Type resourceType, string name)
    {
        var manager = Instance.GetResourceManager(resourceType);
        return manager.GetObject(name);
    }

    public static object GetObject(Type resourceType, string name, CultureInfo culture)
    {
        var manager = Instance.GetResourceManager(resourceType);
        return manager.GetObject(name, culture);
    }

    ResourceManager GetResourceManager(Type resourceType)
    {
        lock (_locker)
        {
            ResourceManager manager;
            if (!_managers.TryGetValue(resourceType, out manager))
            {
                manager = new ResourceManager(resourceType);
                _managers.Add(resourceType, manager);
            }
            return manager;
        }
    }
}

I am using it like so

public class LocalizedDescriptionAttribute : DescriptionAttribute
{
    readonly Type _resourceType;
    readonly string _resourceName;
    readonly CultureInfo _culture;

    public LocalizedDescriptionAttribute(Type resourceType, string resourceName)
        : this(resourceType, resourceName, null)
    {
    }

    public LocalizedDescriptionAttribute(Type resourceType, string resourceName, CultureInfo cultureInfo)
        : base()
    {
        _resourceType = resourceType;
        _resourceName = resourceName;
        _culture = cultureInfo;
    }

    public override string Description
    {
        get
        {
            if (_culture == null)
                return Locale.GetString(_resourceType, _resourceName);
            return
                Locale.GetString(_resourceType, _resourceName, _culture);
        }
    }
}

I have of course read Jon Skeet's article on singletons.

The main thing about the implementation I am unsure about is making the "Instance" private and creating number of static functions to expose the functionality. This means I can call Local.GetString instead of Local.Instance.GetString which is less clunky. I have not seen any singleton implementation which does this so I wonder if there is some problem with it.

I don't think there's a problem with that. Your static methods are only doing things which otherwise the calling code would do.

  I am also using locking but I don't see how to get around that so that is probably fine.

If your code is single-threaded then you don't need locking at all.

If your code is multi-threaded, perhaps you can construct the singleton before the multi-threading starts: for example by calling a static Locale.CreateInstance method in your program's Main (or from Application_Start if your code is ASP.NET).

If your code is multi-threaded then the non-static Locale methods must also be thread-safe: including your Locale._managers dictionary accessors, and your ResourceManager methods. ResourceManager is already documented as a thread-safe class.

An alternative to the explicit lock you implemented is to use the ConcurrentDictionary class instead of Dictionary, specifically its GetOrAdd method, something like:

ResourceManager GetResourceManager(Type resourceType)
{
    Func<Type, ResourceManager> valueFactory =
        resourceType => new ResourceManager(resourceType);
    return managers.GetOrAdd(resourceType, valueFactory);
}

Stylistically, some people recommend auto-declaring variables types as var for example ...

    var valueFactory = resourceType => new ResourceManager(resourceType);

... or simply ...

ResourceManager GetResourceManager(Type resourceType)
{
    return managers.GetOrAdd(resourceType, resourceType => new ResourceManager(resourceType));
}


C program to "monitor" NGINX status with AF_INET I'm trying to implement a simple "monitoring" tool in C to be used on our servers to check NGINX status, executed periodically with a cron task.

This is my very first attempt to use C instead of bash or (lovely) Python - just for some practice.

So I googled code for "socket open" and used some code in the socket_get() function which does exactly NGINX's check.

The second function send_alarm() is just send email with system() (had no time to take libcurl closer).

#include <stdio.h>
#include <sys/socket.h>
#include <arpa/inet.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>

#define HOST_ADDR "127.0.0.1"
#define HOST_PORT 80

int socket_get(int i_addr, int i_port) {

    int socket_desc;
    struct sockaddr_in server;

    socket_desc = socket(AF_INET , SOCK_STREAM , 0);

    if (socket_desc == -1) {
        return 1;
    }

    server.sin_addr.s_addr = inet_addr(HOST_ADDR);
    server.sin_family = AF_INET;
    server.sin_port = htons(HOST_PORT);

    if (connect(socket_desc , (struct sockaddr *)&server , sizeof(server)) < 0) {
        return -1;
    }

    return 0;
}

void send_alarm() {

    char cmd[100];
    char to[20] = "user@domain.kiev.ua";

    char hostname[1024], from_host[24];
    gethostname(hostname, 1024);

    // we have two NGINX hosts - the "master" and the "secondary"
    // here set from_host value depending on the $HOSTNAME
    if (strcmp(hostname, "lj3hwzghi6ibg000000") ==0) {
         strncpy(from_host, "Master", 24);
    } else if (strcmp(hostname, "lj3hwzghi6ibg000001") ==0) {
        strncpy(from_host, "Secondary", 24);
    } else {
        exit(1);
    }

    // concatanate all to the $cmd variable
    sprintf(cmd, "echo NGINX is in DOWN state on the host %s! | /usr/bin/mailx -s \"ALARM from %s\" %s", from_host, from_host, to);
    system(cmd);

}

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

    if (socket_get(atoi(HOST_ADDR), HOST_PORT) == 0) {
        printf("NGINX up and running\n");
    } else {
        printf("Can't connect to local NGINX service!\n");
        send_alarm();
    }

}

Starting with main(): the signature implies that we accept command-line arguments, but we ignore argc and argv, using the hard-coded HOST_ADDR and HOST_PORT instead.  This makes the program less flexible.

An improvement would be to use those values as defaults, but allow command-line arguments to override them:

 const char *server = HOST_ADDR;
 if (argc > 1)
     server = argc[1];
 uint16_t port = 0;
 if (argc > 2)
     port = atoi(argc[2]);
 if (port == 0)
     port = HOST_PORT;

Argument processing can get much more sophisticated than this, but there's the bare minimum for the job.

Note in particular that atoi(HOST_ADDR), after expansion, is atoi("127.0.0.1") which always evaluates to 127.  What you really want is to change the signature of socket_get() to accept a string hostname:

int socket_get(const char *i_addr, uint16_t i_port)
{
    struct sockaddr_in server;
    server.sin_addr.s_addr = inet_addr(HOST_ADDR);
    if (server.sin_addr.s_addr == INADDR_NONE)
        return -1;

Notice that inet_addr may fail, so handle that gracefully.

There are severe problems in send_alarm(), apart from immediately failing if the compiled-in host names ever change.

Although we're using strncpy() to avoid overrunning from_host, that's slightly dangerous because it can leave an unterminated string in the output (and I'd change that hard-coded 24 to sizeof from_host, too).  That's minor, as the fixed strings we're using won't overflow the storage.

A more serious problem is that to is only 20 chars long, and we're initialising it with a 20-char string.  That means it won't have a null terminator, which will cause undefined behaviour when it's accessed by any string functions (such as the printf()).  A safer version is

    char to[] = "user@example.com";

Finally, the sprintf() is really dangerous.  For some reason, we haven't used snprintf() here, so it's possible to overrun the output cmd, and in fact, it's actually guaranteed to do so, given that cmd is only 100 chars in size, and just adding the lengths of the format string and to will exceed that.

Thankfully, all the issues in this function are easily fixed, because I'm going to recommend removing it completely.  Instead of invoking mailx directly from this program, it's more flexible to provide the necessary output and leave it to the caller to decide what to do with it (this is in keeping with the "Unix Philosophy" - do one thing well, and support composition of tools).

My recommendation is to delete send_alarm() and change the calling code to

    if (socket_get(server, port)) {
        fprintf(stderr, "Can't connect to NGINX service on %s:%u!\n", server, (unsigned int)port);
        return 1;
    }

By writing output and returning non-zero when (and only when) we fail, we make it easy to use in shell scripts or with cron (cron will send you email if the command produces output, and this is a perfect example of how to cooperate with that).


Search in a nested list of a dictionary I have a dictionary structured as {a_1: [((b_11,c_11),d_11), ((b_12, c_12), d_12), ...], a_2: [...], ...}.

I want to extract the maximum abs(c) among all the possible c values. Currently I'm doing this throught two for loops. One for each a, and other for each of set {b,c,d}.

elements = {
            0: [((0, -4), 1)],
            1: [((1, -5), 1), ((0, -4), 1)],
            2: [((2, -3), 1)],
            3: [((3, -2), 1), ((0, -5), 1)],
            }

max_c = 0

for a in list(elements):
    for (b, c), d in elements[a]:
        max_c = max(max_c, abs(c))

Is there any way to do this without the for loops? and if it is, is it pythonic? or I should keep this way because it's more understandable?

Thanks in advance.

When you are looping over the keys a and values elements[a] of a dictionary, use the items method to loop over both at the same time:

for a, value in elements.items():
    for (b, c), d in value:

(If I knew what these values represented, I would pick a better name than value.)
The code here doesn't use the keys, it only uses the values. So use the values method instead:

for value in elements.values():
    for (b, c), d in value:

The code doesn't use b or d either. It is conventional to use the name _ for unused variables:

for (_, c), _ in value:

A double iteration over some lists and then over the elements of those lists can be combined into a single iteration using itertools.chain.from_iterable:

from itertools import chain
for (_, c), _ in chain.from_iterable(elements.values()):

The repeated calls to max can become a single call taking a generator expression:

values = chain.from_iterable(elements.values())
max_abs_c = max(abs(c) for (_, c), _ in values)

It's common to worry whether code is "Pythonic" but it is better to think in terms of general principles (clarity, simplicity, maintainability, testability, usability, efficiency, etc.) that apply to code in any language.


Checking if a number is a palindrome and is incrementing Edit: Ok, this was a disaster of a question. I'll make sure to use timeit in the future

I solved a Codewars problem that had the user check if a number was a palindrome, was incrementing, and etc. After I solved it, I noted how the top solutions were much more simple, intuitive, and elegant than mine, although they were less efficient. I figured this was because code golf is more important in Codewars than efficiency, but after Googling around it seemed to me that perhaps a logically intuitive and elegant solution was more important in Python than an efficient but less elegant solution. Is there any truth to this?

To make this post more appropriate for the scope of Code Review, I will include two solutions for two problems, is_palindrome and is_incrementing. The first solution is the more elegant and logically intuitive one, and the second solution is my attempt at a more efficient one. I would like to know which solution is more appropriate for Python, and also what improvements could be made to my solution.

In Python, it is regularly suggested to use the following to determine if a string is a case-sensitive palindrome:

def is_palindrome(word):
    return word == word[::-1]

Here is my attempt at a more efficient way to accomplish this:

def is_palindrome(word):
    low = 0
    high = len(word) - 1
    while low < high:
        if word[low] != word[high]:
            return False
        low += 1
        high -= 1
    return True

As another example, to determine if a number is incrementing, such as 234 but not 231 it is suggested to use:

def is_incrementing(number):
    return str(number) in '0123456789'

Instead of something like:

def is_incrementing(number)
    number = str(number)
    prev = number[0]
    for char in number[1:]:
        if int(char) != int(prev) + 1:
            return False
        prev = str(int(prev) + 1)
    return True

If we look at your four different functions, you should be able to see that two are simple and easy to read, this goes in line with Pythons ideas, "simple is better than complex" - zen of Python. This is good as it means that others can read and understand your code with ease. This is a benift to both you and others. It also means if you need to change the functionality slightly, it's simple and easy. With your optimised function they take time to read and understand. This is a trade off, either you have fast code that is hard to understand, or slow but easy to understand code. And so from a human point of view, we prefer slower code.

I guess you, like I, prefer fast code. This leads to problems. Say we are tasked to make a program, from day 1 we'll be using hard to read code to make something simple. This at first is ok, but then it effects the overall structure of the project. And slowly we've made a monster, a hard to read monster. This is why premature optimisations are bad.

Instead if you don't worry about speed from the beginning, then you don't fall into this problem. And when you need to increase the performance of your code you can then swap out the old slow code for new fast code, only when it's needed. Sure you have to write a function twice, but at least we don't need to write the entire program again.

There is also another benefit to using simple code. When you're implementing the optimisation you have a base function to time against. This is good as then you can easily tell if your code is an optimisation, or just added complexity. Your function for example show this, if you time them both against their 'simple' version, you'll find, you haven't optimised the code, you've only obfuscated the code.


Wrapping every expression with % 2 ** 32 to get a C-like behavior I'm trying to port the following C loop to Python:

for (int32_t i = 32; --i >= 0;) {
    v0 += ((v1 << 4 ^ v1 >> 5) + v1) ^ (sum + k[sum & 3]);
    sum -= delta;
    v1 += ((v0 << 4 ^ v0 >> 5) + v0) ^ (sum + k[sum >> 11 & 3]);
}

The problem is that the these integers are 32-bit unsigned and the code above relies on integer wrapping. My first attempt was to introduce the U32 class (see here), but after porting this code to Python 3, the solution got so ugly (see here) that I started to looking for other ones. My best idea at the moment is to wrap every expression with % 2 ** 32, so that it wraps around. Here's how this looks:

for _ in range(32):
    v0 = (
        v0
        +
        (
            (
                (v1 << 4) % 2**32 ^ (v1 >> 5) % 2**32
            ) % 2**32
            +
            v1 ^ (sum_ + k[sum_ & 3]) % 2**32
        ) % 2**32
    ) % 2**32

    sum_ = (sum_ - delta) % 2 ** 32

    v1 = (
        v1
        +
        (
            (
                (
                    (v0 << 4) % 2**32 ^ (v0 >> 5) % 2**32
                ) % 2**32 + v0
            ) % 2**32
            ^
            (sum_ + k[(sum_ >> 11) % 2**32 & 3]) % 2**32
        ) % 2**32
    ) % 2**32

It looks strange and I'm not sure it's the best idea. What do you think of it?  Is it something that needs improving?  If so, how?

Firstly, the expensive modulo operation % 2**32 can be replaced by a cheap bitwise and: & 2**32-1. Make sure to familiarize yourself with operator precedence, because it's not intuitive when it comes to bitwise operators (if you haven't done that yet).

As a next step, you could use the following properties to optimize the equations:

(a % m + b % m) % m = (a + b) % m
(a & n ^ b & n) & n = (a ^ b) & n

That allows you get rid of many of the & 2**32-1 operations at the cost of bigger intermediate results.  If you want to make sure to get the highest speed, do some perfs. In any case, it will make the code much more readable.


Householder transformation I'm trying to make this Householder method run as quickly as possible. It runs fine now, but I'm still a bit new to java, is there anything you guys see to make this run faster?

public static ArrayList<double[][]> householder(double[][] A) {
    int m = A.length; // rows
    int n = A[0].length; // cols
    byte sign;
    double sum, A0,tmp;
    ArrayList<double[][]> QR = new ArrayList<>();
    double[] v = new double[m];
    double[][] Q = identity(m);
    for(int k = 0; k < n; k++) {
        sum = 0;
        sign = 1;
        A0 = A[k][k];
        if(A0 < 0){
            sign = -1;
        }
        for(int i = k; i < m; i++) {
            sum += A[i][k]*A[i][k];
        }
        tmp = sign*Math.sqrt(sum);
        v[k] = tmp + A0;
        tmp = Math.sqrt(2*(sum + A0*tmp));
        v[k] /= tmp;
        for(int i = k+1; i < m; i++) {
            v[i] = A[i][k]/tmp;
        }
        for(int j = 0; j < n; j++) {
            sum = 0;
            for(int i = k; i < m; i++) {
                sum += v[i]*A[i][j];
            }
            for(int i = k; i < m; i++) {
                A[i][j] -= 2*v[i]*sum;
            }
        }
        for(int j = 0; j < m; j++) {
            sum = 0;
            for(int i = k; i < m; i++) {
                sum += v[i]*Q[i][j];
            }
            for(int i = k; i < m; i++) {
                Q[i][j] -= 2*v[i]*sum;
            }
        }
    }
    QR.add(transpose(Q));
    QR.add(A);
    return QR;
}

I'm mostly concerned with the portion of the code that multiplies the vectors to get the Q and A matrices.

Here are some ideas I had but wasn't sure would help much.

if there's a better way to return the values than arraylist, like if double[][][] would be faster. I have had issues with recasting double[][][] arrays to double[][] however.
changing all the ints to shorts, i was concerned with casting
I only need sign to be a bit
should I write sum = 0.0?
should I initialize the for loop int i's and j's outside of the loop to save memory?

byte sign;
double sum, A0,tmp;

Keep variables to the most inner scope where they can be defined.

sign = 1;
A0 = A[k][k];
if(A0 < 0){
    sign = -1;
}

sign can be assigned to a ternary expression

sign = A0 < 0 ? -1 : 1;

tmp = sign*Math.sqrt(sum);
v[k] = tmp + A0;
tmp = Math.sqrt(2*(sum + A0*tmp));
v[k] /= tmp;

EDIT Oops screwed up, the maths on v[k] evaluation... too many times...

EDIT 2 changing tmp to have the right value for the inner loop.

I would reduce the calculation to this:

    double sqrSum = sign*Math.sqrt(sum);
    double tmp = Math.sqrt(2*(sum + A0*sqrSum));
    v[k] = (sqrSum  + A0) / tmp;

This is trying to store the value in v[k] just once, instead of accessing it twice.

  if there's a better way to return the values than arraylist, like if
  double[][][] would be faster. I have had issues with recasting
  double[][][] arrays to double[][] however.

Well in the end you know that you are returning an Array with two entries, the first being transpose(Q), and the second being A. This is pretty simple to achieve both on the implementation and the caller as long the method is properly documented.

Return on implementation

return new double[][][]{
    transpose(Q),
    A
}

Use on caller

var result = householder(...);
var transpose = result[0]; //documentation says entry 0 has the transpose
var a = result[1]; //documnentation says entry 1 has A

Resulting code

public static double[][] householder(double[][] A) {
    int m = A.length; // rows
    int n = A[0].length; // cols
    double[] v = new double[m];
    double[][] Q = identity(m);
    for(int k = 0; k < n; k++) {
        double sum = 0;
        byte sign = A0 < 0 ? -1 : 1;
        double A0 = A[k][k];
        for(int i = k; i < m; i++) {
            sum += A[i][k]*A[i][k];
        }
        double sqrSum = sign*Math.sqrt(sum);
        double tmp = Math.sqrt(2*(sum + A0*sqrSum));
        v[k] = (sqrSum  + A0) / tmp;
        for(int i = k+1; i < m; i++) {
            v[i] = A[i][k]/tmp;
        }
        for(int j = 0; j < n; j++) {
            sum = 0;
            for(int i = k; i < m; i++) {
                sum += v[i]*A[i][j];
            }
            for(int i = k; i < m; i++) {
                A[i][j] -= 2*v[i]*sum;
            }
        }
        for(int j = 0; j < m; j++) {
            sum = 0;
            for(int i = k; i < m; i++) {
                sum += v[i]*Q[i][j];
            }
            for(int i = k; i < m; i++) {
                Q[i][j] -= 2*v[i]*sum;
            }
        }
    }
    return new double[][][]{
        transpose(Q),
        A
    }
}

  Changing all the ints to shorts, i was concerned with casting

Don't bother with it. In the end all of it will be running mostly on cpu registers which can hold 32 bits, incrementing 16 or 32 bits costs mostly the same and you shouldn't be bothered by that.

  I only need sign to be a bit

This is almost effectively the same as evaluating sign to be +1 or -1 once. keeping this on a variable makes you code cleaner. Pre-optimization is the rule of all evil, first you need to make sure your code is maintainable, understandable, follows design principles, and optimally that you chose the best data structures and kept the algorithm running time to a minimum, although that also falls in the optimization hole is not as aggressive as going into the last details like you referring to.

  should I write sum = 0.0?

Meh floating point is not my area of expertise but what I think is even if there is a difference on the representation between 0 and 0.0 which there isn't, you end up with slightly errors of accuracy.

  should I initialize the for loop int i's and j's outside of the loop
  to save memory?

No. Compilers do a great job optimizing trivial things for you, such as this and also the ones mentioned before. In the end the cpu will need to increment a variable, hopefully that variable will be on the cpu register or in cache, if not will be on the stack (ram). The variations between this can be different depending on how you write your code but it is the sort of things that you if you are truly worried about then you go into onto other languages like C and ultimately assembly.

On variable declaration and assignment.

This seems to be a topic a little bit out of your knowledge, let's get some of those doubts sorted.

Historically, in languages like C, variable names were declared at the beginning of a method, or scope. The reason being that if you didn't do that way the compiler would give you a warning. I suspect that this warning could be due to possible inefficiency on the usage of the stack, but I'm not totally positive for this (at least this seem to be a plausible reason for those warnings).

Thus, like I told you, things on the internals can change a little bit depending how/where you declare you variables. Compilers can use registers differently, stack can be managed differently, cache can be used differently and so on. However in Java this is thing that is mostly out of your control, unless you go to take a peek to the bytecode and how that code would translate into assembly you really can't do much about it. That's why you move onto C or assembly if you are concerned about those things.

Some algorithm pseudo-codes also follow the same strategy.

This is primarily why you will often see declaration of variables at the beginning of a method or scope.

However things work differently on Java and some other languages (I would say most of them but I think that would push the boundaries). In Java the compiler does not give you a warning for where you are declaring a variable. However for readability variables tend to be declared at the beginning the scope, notice that I said scope and not method this time. Variables can also be declared in the first moment when you have everything needed to assign them.

Like I told you tmp in this algorithm can be defined in the first for body, however it could also be defined on the first inner for body. There are two reasons however to why you should declare on the outer for.

tmp is needed to assign a value to v[k].
The evaluation of Math.sqrt(2*(sum + A0*sign*Math.sqrt(sum))) is expensive and only needs to be evaluated once. To make sure that this is only evaluated once for each k, this variable needs to be assigned out of the inner for body.


Determine the longest segment of integers within an inclusive range that doesn't contain a bad number using Python I am new to python.
Got a question in Hackerrank.
Given an Array of bad numbers and a range of integers, how can I determine the longest segment of integers within that inclusive range that doesn't contain a bad number?
For example, you are given the lower limit l = 3 and the upper limit r = 48, The array badNumbers = [37,7,22,15,49,60]. Segments without bad numbers are [3,6], [8,14], [16,21], [23,36] and [38,48]. The longest segment is [23,36] and it is 14 elements long.
Problem : Function Description Complete the function goodStatement in the editor below. The function must return an integer denoting the length of the longest contiguous range of natural number in the range l to r, inclusive, which does not include any bad numbers.
goodSegment has the following parameter(s): badNumbers[badNumbers[0],...badNumbers[n-1]]: an array of integers l: an integer, the lower bound, inclusive r: an integer, the upper bound, inclusive
Constraints \$1 \le n \le 10^5\$, \$1 \le badNumbers[i] \le 10^9\$, badNumbers contains distinct elements
def goodSegment(badNumbers, l, r):
    ranges = []
    subrange = []
    large=0
    for i in range(l,r+1):
        if badNumbers.count(i)==0:
            if len(subrange)==0:
                subrange.append(i)
            else:
                if i==r:
                    subrange.append(i)
                    ranges.append(subrange)
                    if len(range(subrange[0],subrange[1]+1))>large:
                        large=len(range(subrange[0],subrange[1]+1))
        else:
            if len(subrange)==1:
                subrange.append(i-1)
                ranges.append(subrange)
                if len(range(subrange[0],subrange[1]+1))>large:
                    large=len(range(subrange[0],subrange[1]+1))
                subrange=[]
            else:
                subrange=[]

    return large

badNumbers=[5,4,2,15]
l=1
r=10

result = goodSegment(badNumbers, l, r)
print(result)

The code is working fine. But this is not getting executed in 10 seconds as per the hackerrank test cases. How can I optimize this for a faster execution?

PEP 8
The Style Guide for Python Code lists many recommendations:

variables & functions should be snake_case
commas should be followed by a space
binary operators should be surrounded by a space (eg, l == r instead of l==r, and r + 1 instead of r+1)

Some of the function/variable names may be dictated to you by the coding challenge, but for all others you should follow the PEP 8 conventions.
Optimizations
Loop range
for i in range(l, r + 1):
Assuming your range is 1 to \$10^9\$, this loop will take more than 10 seconds if the code inside the loop takes more than 10 nanoseconds per iteration.  That's asking a lot for an interpreted language.  You would do better if that loop can be removed.
More on this in a moment.
Counting -vs- existence
if badNumbers.count(i)==0:
You are searching the list of bad numbers counting the number occurrences of the value i in the list.

badNumbers contains distinct elements

Because badNumbers contains unique elements, the count will be either 0 or 1.  More over, as soon as we find the first match, we can stop counting even if uniqueness was guarenteed, since we are testing the count against zero.  What you really want is simply to ask if the value i is in badNumbers.  You write that exactly how we said it:
if i in badNumbers:
This is still an \$O(n)\$ operation, but would be faster.  As mentioned in another answer, turning badNumbers into a set would speed that up into an \$O(1)\$ operation.  But let's investigate another option.
Reducing data
Your example badNumbers = [37, 7, 22, 15, 49, 60] with limits l=3 and r=48 reveals more inefficiency.  The badNumbers contains numbers outside of the limits.  Looking at those numbers over and over again (such as in badNumbers.count(i) == 0 or i in badNumbers) is wasted time.
We can filter out the "bad" bad numbers which end up just wasting time.
badNumbers = [number for number in badNumbers if l <= number <= r]
Organizing your data
You are looping over your candidate numbers, and then effectively searching to see if the candidate number is in the bad numbers list.  If that list was sorted, you could just maintain an index to the next unmatched bad number, and increment it each time a match was found.
Combining that with the previous step:
badNumbers = sorted(x for x in badNumbers if l <= x <= r)
Now in your example, the badNumbers would be [7, 15, 22, 37]
Now, it = iter(badNumbers) would create an iterator which could walk over the list, in order, bad_number = next(it) could extract the next bad number from the list, and we've gotten rid of that pesky index I mentioned above, too.
But I'd really like to get rid of that first loop, so lets take a different approach.
Gap Length and End Posts
[7, 15, 22, 37]
In this array, we can immediately see there are 15-7-1 good numbers between the first two bad numbers, 22-15-1 between the next pair, and 37-22-1 between the last pair.  This is almost all we need to get the longest run of good numbers!
What is missing is the good number runs at the beginning and end of the list.  We can fix that by adding some "end posts", extra bad numbers one beyond the good range end points:
badNumbers = [l - 1] + sorted(x for x in badNumbers if l <= x <= r) + [r + 1]
With limits of l=3 and r=48, this creates badNumbers = [2, 7, 15, 22, 37, 49].
Now, every pair of numbers can be used to determine the length of a run of good numbers.
pairwise
Since we want to take the numbers in pairs, it makes sense to consult the Python itertools library for an appropriate function.  It isn't built-in, but the pairwise recipe  can be installed from more-itertools.
Take the numbers as pairs, computing the number of good values between the pair of bad values, and remember the maximum value.  The "minus 1" from the internal calculation can be delayed till the end, for efficiency, since \$\max{(x_i-1)} == \max{(x_i)} - 1\$
from more_itertools import pairwise

def goodSegment(bad_numbers, l, r):
    bad_numbers = [l - 1] + sorted(x for x in bad_numbers if l <= x <= r) + [r + 1]
    gap_lengths = (b - a for a, b in pairwise(bad_numbers))
    return max(gap_lengths) - 1

badNumbers = [5, 4, 2, 15]
l = 1
r = 10

result = goodSegment(badNumbers, l, r)
print(result)

If more-itertools is not available, you can make something almost equivalent
def pairwise(numbers):
    return zip(numbers[:-1], numbers[1:])

It may even be faster, although it will use up to three times more memory.


Refactoring a bunch of and statements Trying to refactor this code and can't seem to think of a way to make it cleaner. Type is  a property in my Person class.

foreach (var item in list)
{
    if (person.Type != PersonType.Employee && person.Type != PersonType.Manager && person.Type != PersonType.Contractor && person.Type != PersonType.Executive)
    {
        DoSomething();
    }
}

public enum PersonType : int {
   Employee = 0,
   Manager = 1,
   Contractor = 2,
   President = 3,
   Executive = 4
}

I should note that there are other types in the PersonType class which I don't want to show.

This is such a perfect match for the Replace Conditional with Polymorphism Refactoring that it even looks as if it was specifically designed to demonstrate the Replace Conditional with Polymorphism Refactoring. I mean, the field it is basing its behavior on is even called Type!

interface Person { void DoSomething(); }

class PrivilegedPerson : Person {
    public void DoSomething() {
        // do something
    }
}

class UnprivilegedPerson : Person {
    public void DoSomething() {} // literally do nothing
}

class Employee   : UnprivilegedPerson {}
class Manager    : UnprivilegedPerson {}
class Contractor : UnprivilegedPerson {}
class President  : UnprivilegedPerson {}
class Executive  : UnprivilegedPerson {}


FizzBuzz in Javascript I'm sure everyone here knows what FizzBuzz is. I would like constructive criticism for my solution.

I'm a beginner to programming as a whole and this isn't my first solution, but it's what I think is my best solution. I'd like to see if this is a reasonable solution, or is just plain terrible.

for(var i=1;i<=100;i++) {
var output = "";

    if(i % 15 == 0) {output = "FizzBuzz"}
    else if(i % 3 == 0) {output = "Fizz"}
    else if(i % 5 == 0) {output = "Buzz"}
    else {output = i;}

console.log(output);
}

The output is correct. It's your standard FizzBuzz output.

I'd like to see if this is a reasonable solution, or is just plain terrible.

I wouldn't say it is "terrible" - mostly because it works and doesn't appear to be very inefficient. However, there are some improvements that can be made.

use strict equality comparison - i.e. === when comparing values. That way it won't need to convert the types.

Style Guide Consider following a style guide. Many common style guides advise separating keywords with a space - e.g. if ( instead of if(.

Use consistent indentation The first and last lines within the for loop are not indented, while the other lines between them are, though maybe it was the case that your code was indented properly but when you pasted here it was thrown off because of the markdown formatting...

Abstract logic into a function As Paul's answer suggests: you can put the core logic into a function that returns the output but doesn't handle outputting the value (e.g. to the console). This allows such code to be atomic and testable - congruent with the Single Responsibility Principle. Also, the return statement can eliminate the need for else keywords within a function. One drawback is that calling a function on each iteration may slow down operations but should be negligible for a set of numbers 1 through 100.

Updated Code
Consider the modified code below, utilizing the feedback above. I also used this code in the code contained in my recent question: Fizzbuzz with dynamic height container
Note: the inline console in the snippets is truncated to ~50 lines, but the complete console log should be visible in your browser console.

function fizzBuzz(value) {
    if (value % 15 === 0) { return "FizzBuzz"; }
    if (value % 3 === 0) { return "Fizz"; }
    if (value % 5 === 0) { return "Buzz"; }
    return value;
}
for (let i = 1; i <= 100; i++) {
    console.log(fizzBuzz(i));
}

One option I did consider is to minimize the number of modulo operations, append to the output string instead of outputting it. If you are trying to optimize for speed, this might not be the approach to take because appending to the string may be much slower than doing an extra couple modulo operations. Try a comparison in this jsPerf test.

function fizzBuzz(value) {
    let output = "";
    if (value % 3 === 0) { output += "Fizz"; }
    if (value % 5 === 0) { output += "Buzz"; }
    return output || value;
}
for (let i = 1; i <= 100; i++) {
    console.log(fizzBuzz(i));
}


Modern C++ compliant selection sort I am writing, as review, Modern C++ compliant versions of sorting algorithms I previously have seen. This post is about selection sort, and selection sort only.

Whilst selection sort is rather simple, straightforward, and not the best in terms of performance, I wanted to know if there were any ways to improve upon my implementation, especially in terms of performance:

// Orders a list of values by repeatedly putting the smallest
// or largest unplaced value into its final position.
template<typename T>
void sort_selection(std::vector<T> & arr)
{
    std::vector<T>::iterator min, beg, end = arr.end();
    for (beg = arr.begin(); beg < end; ++beg)
    {
        min = std::min_element(beg, end);
        if (*min < *beg)
            std::iter_swap(min, beg);
    }
}

I'm using local variables min, beg, end outside the loop to lower the amount of dereferencing and stack allocations and take advantage of temporal locality. Also I am using iterators and min_element, iter_swap to be modern c++ compliant.

There's not really much code here to review, and for what it is, all I really have are some subjective stylistic concerns (I'm not a fan of omitted braces or multiple declarations per line). Instead of reviewing it as a stand alone snippet, I'm going to review it as what I would expect a fully idiomatic and generic sorting function to be in C++.

The biggest issue I see with this is that it's not quite up to snuff with regards to C++ idioms. It's very rare to write a method that actually takes a container (and isn't just a convenience method for the iterator version). Instead, it's customary to take two iterators since that allows you to operate on any container. In certain situations, you can even make decisions based on the iterator type. For example, you could have your sort fall back to std::sort if the iterator is a random access iterator.

Anyway, what I'd expect to see is this:

template<typename FwdIterator>
void sort_selection(FwdIterator beg, FwdIterator end)
{
    ...
}

Some people prefer just Iter or something similar, but I like to specify the lowest iterator requirement necessary to use the function. In this case, you just require a forward iterator.

Speaking of iterator types, you should use != over < when dealing with iterator loops if all you care about is whether or not the second iterator has been reached by the first.

operator< might not be present on iterators in situations where operator!= is. This is a perfect example actually. Forward iterators are not required to define operator< and in fact usually won't. Other than the std::vector hard coding, the beg < end is the only thing stopping you from generalizing to a forward iterator.

As Ben Voigt noted, it would also be expected to generalize the comparator:

template<typename FwdIterator, typename Comparator>
void sort_selection(FwdIterator beg, FwdIterator end, Comparator cmp)

You would then either have the comparator default to std::less or you would have two functions: one that takes a comparator and one that just calls the comparator taking version with a std::less instance.

All in all, without going completely overboard, I would expect a sorting method to look something like this:

template<typename FwdIterator, typename Comparator>
void sort_selection(FwdIterator beg, FwdIterator end, Comparator cmp)
{
    for (; beg != end; ++beg)
    {
        FwdIterator min = std::min_element(beg, end, cmp); // auto if >= C++11
        if (cmp(*min, *beg))
            std::iter_swap(min, beg);
    }
}

template<typename FwdIterator>
void sort_selection(FwdIterator beg, FwdIterator end) {
    // using instead of typedef if >= C++11
    typedef typename std::iterator_traits<FwdIterator>::value_type value_type;
    sort_selection(beg, end, std::less<value_type>());
}

It might be worth noting that you don't actually need two methods. I just think it's a little cleaner than this:

template<typename FwdIterator, typename Comparator = std::less<typename std::iterator_traits<FwdIterator>::value_type>>
void sort_selection(FwdIterator beg, FwdIterator end, Comparator cmp = Comparator())


OpenSSL PBKDF2-HMAC-SHAx proof of concept I have regrettably been away from C programming for a very long time, so I'd like to get a quick code review of a first proof of concept, before I get too far into adding capabilities, using the getopt library for arguments, alternate output and input character sets, a PolarSSL implementation, and so on and so forth.

Target platforms: gcc 4.5+ for MinGW for Windows, gcc 4.7+ for Linux.

My ranking of concerns:

Security holes
Incorrect use of OpenSSL
Memory leaks
Performance
Suboptimal or incorrect compilation commands
Inefficient code
deviations from ANSI C/portability
Unclean code
Suggestions for improvement
other

As a purely secondary interest, if anyone has advice on the getopt library, C code for Base32/Base64/etc output formats, or PolarSSL use, I'd appreciate it.

Linux compilation:

gcc -O3 -lssl pbkdf2.c -o pbkdf2

Windows compilation:

gcc -O3 -I OpenSSLLibraryPath\include -L OpenSSLBinariesPath -leay32 pbkdf2.c -o pbkdf2

The code:

#include <string.h>
#include <stdio.h>
#include <stdint.h>
#include <openssl/evp.h>
#include <openssl/sha.h>

// Originally from http://stackoverflow.com/questions/10067729/fast-sha-2-authentication-with-apache-is-it-even-possible

void PBKDF2_HMAC_SHA_256(const char* pass, const unsigned char* salt, int32_t iterations, uint32_t outputbytes, char* HexResult)
{
    unsigned int i;
    unsigned char digest[outputbytes];
    PKCS5_PBKDF2_HMAC(pass, strlen(pass), salt, strlen(salt), iterations, EVP_sha256(), outputbytes, digest);
    for (i = 0; i < sizeof(digest); i++)
        sprintf(HexResult + (i * 2), "%02x", 255 & digest[i]);
}

void PBKDF2_HMAC_SHA_512(const char* pass, const unsigned char* salt, int32_t iterations, uint32_t outputbytes, char* HexResult)
{
    unsigned int i;
    unsigned char digest[outputbytes];
    PKCS5_PBKDF2_HMAC(pass, strlen(pass), salt, strlen(salt), iterations, EVP_sha512(), outputbytes, digest);
    for (i = 0; i < sizeof(digest); i++)
        sprintf(HexResult + (i * 2), "%02x", 255 & digest[i]);
}

int main(int argc, char *argv[])
{
  printf("Usage: %s SHA-512|SHA-256 password salt iterations outputlengthinbytes\nSHA-512 is recommended (for password hashing, outputlengthinbytes <= 64)\nSHA-256 is not as good, (for password hashing, outputlengthinbytes <= 32)\n",argv[0]);
  if(argc != 6){printf("Incorrect number of parameters.\n");return 2;};
  char pass[strlen(argv[2])];
  unsigned char salt[strlen(argv[3])];
  int32_t iterations;
  uint32_t outputbytes;

  strcpy(pass,argv[2]);
  strcpy(salt,argv[3]);
  iterations = atoi(argv[4]);
  outputbytes = atoi(argv[5]);
  printf("\n%s\n",argv[1]);

  char HexResult[2*outputbytes-1];

//    printf("Computing PBKDF2(HMAC-SHA512, '%s', '%s', %d, %d) ...\n", pass, salt, iterations, outputbytes);
  memset(HexResult, 0, outputbytes);
  if (strcmp(argv[1],"SHA-512")==0)
    {
    PBKDF2_HMAC_SHA_512(pass, salt, iterations, outputbytes, HexResult);
    }
  else if (strcmp(argv[1],"SHA-256")==0)
    {
    PBKDF2_HMAC_SHA_256(pass, salt, iterations, outputbytes, HexResult);
    }
  else
    {
    printf("Invalid value for hash - choose one of:\nSHA-512\nSHA-256\n");
    return 1;
    }

  printf("Result: in Base16 (hex)\n%s\n", HexResult);
  return 0;
}

Things you did well on:

Looks like you weren't too rusty based on this code.  Looks very nice organizationally.
You return different values for different error conditions.

Things you could improve on:

Standards

Initialize i in your for loops.(C99)

for (unsigned int i = 0; i < sizeof(digest); i++)

Syntax

Don't over-compact your code.

if(argc != 6){printf("Incorrect number of parameters.\n");return 2;};

Let it breathe a bit.

if(argc != 6)
{
    printf("Incorrect number of parameters.\n");
    return 2;
}

Use puts() instead of printf() when you aren't formatting a string.

puts("Incorrect number of parameters.");

You don't follow proper C naming conventions.

 char HexResult[2*outputbytes-1];

Either use camelCase, or snake_case with variable names.
Your indentation is a bit odd.

if (strcmp(argv[1],"SHA-512")==0)
  {
  PBKDF2_HMAC_SHA_512(pass, salt, iterations, outputbytes, HexResult);
  }

But that could be a copy-paste issue.

Commenting:

The only comments I see are a link to a Stack Overflow post, and a commented-out bit of old code.  Use more comments to explain what your code does, and how it accomplishes that/why it works.

Look at it as if a new developer was going to look at your code.  You should try to explain it so that he could understand it's basic function and how it works right away.  You might thank yourself when you return back to this project in the future, forgetting completely how it works. ;)


Node.js Nested Callbacks, multiple requests I'm making requests to Parse.com to 3 different objects that are needed to render a View.
I'd like to know what's the general/best approach in cases like these in which there are multiple requests before actually handling the data.

Code:

    var Parse = require('parse-api').Parse;
    var parse = new Parse();

    exports.single = function() {
        parse.find('Class_1',params1,function(error,response1){
            parse.find('Class_2',params2,function(error, response2){
                parse.find('Class_3',params3,function(error3, response3){

                    // handle responses....
                    // render view....
                })
            })
        })
    }

Thanks!

Use Step.js or async.js to make this look cleaner.  Step.js is simpler, but async.js gives you more flexibility.

Also, your function single needs to have a callback, and cannot return values since the functions it's invoking are not returning values.

Additionally, generally the data retrieval methods should be separate from the rendering methods.

With Step.js, this could look like:

 var Parse = require('parse-api').Parse,
    Step = require('step');
    var parse = new Parse();
exports.getAndRenderData = function(res, params1, params2, params3){
   getData(params1, params2, params3, function(err, data){
      if(err)
        throw err:
      else {
        res.render('viewname', data);
      }
   });
}

var getData = function(params1, params2, params3, callack) {
    var data = {};
    Step()(
        function getClass1(){
            parse.find('Class_1',params1, this);
        },
        function getClass2(err, class1data){
           if (err) throw err;
           data.class1 = class1data;
           parse.find('Class_2',params2, this);
        },
        function getClass3(err, class2data){
           if (err) throw err;
           data.class2 = class2data;
           parse.find('Class_3',params3, this);
        },
        function callbackWithAll(err, class3data){
           if (err) callback(err);
           else {
             data.class3 = class3data;
             callback(null, data);
           }
        }
    );
}


Sum of an integer's digits The question is that in a given integer n you have to sum its digits. For example, the sum for 111 would be 3.

Though I managed to solve the problem, I have a good feeling that this algorithm can still be improved.

public static int digit(int n){
    String numbers = n+"";
    int sum = 0;
    for(int x = 0; x < numbers.length(); x++){
        sum += Integer.parseInt(numbers.charAt(x)+"");
    }

    return sum;
}

There is an arithmetic solution to this problem, without converting to a string. It's probably what the string conversion does internally and will therefore likely be much more efficient.

Quoting Wikipedia:

  The digit sum of a number \$x\$ in base \$b\$ is given by
  $$
\sum_{n=0}^{\lfloor \log_b x\rfloor} \frac{1}{b^n}(x \bmod b^{n + 1} - x \bmod b^n).
$$

This StackOverflow answer has an implementation in C# (which is very similar to Java):

sum = 0;
while (n != 0) {
    sum += n % 10;
    n /= 10;
}

In addition, I'd like to comment on the naming of your method. In my opinion, digit doesn't describe its function very well. Instead I'd recommend something similar to digitSum.

Note that this approach doesn't handle negative numbers. They cause the function to go into an infinite loop. To handle that case, you can something like the following:

public static int digitSum(int n) {
    int sum = 0;
    bool isNegative = false;

    if (n < 0) {
        isNegative = true;
        n *= -1;
    }

    while (n != 0) {
        sum += n % 10;
        n /= 10;
    }

    if (isNegative) {
        return -sum;
    } else {
        return sum;
    }
}


User authentication, management I have recently started studying JavaScript, nodeJS, and reactJS on the front-end, and I have some questions about best practices to apply in this JavaScript world. I would like to ask for opinions regarding how I could improve my code.
Starting with an example: I have 2 functions, with different ways of handling promises
Here I have a route to add a user:
 addUsers(req,res,next) {
    try {
        const {name,email,login} = req.body;
        User.existLogin(req.body.login)
        .then(async result => {
            if(!result){
                const password = await bcrypt.hashSync(req.body.password, 10);
                User.create({
                    name, email, login, password }).then(result => {
                    res.status(201).json({Results: result.dataValues})
                })
            }else{
                return res.status(409).json({message: 'Login already exists'});
            }
        })
    } catch (error) {
        res.status(500).json({error: error})
    }
}

And here I have another function to login:
async login(req,res){
    const  user  = await User.existLogin(req.body.login);
    if (!user) { return res.status(400).json({result: 'Login is wrong '});}
    const isPassword = await User.isPassword(user.dataValues.password, req.body.password);
    if (!isPassword) { return res.status(400).json({result: 'Password is wrong '}); }
    const jwt = auth.signjwt(auth.payload(user));
    console.log(jwt);
    res.status(200);
    res.json(jwt);
}

Looking at this the login function seems to me to be cleaner, but I have doubts if really yours I could improve in one of two (I follow different logics to do both).
I have an auth folder, where I export functions, such as my payload, my sign, my middlware to validate my jwt, but I don't know if this is a correct decision.
const jwt = require('jsonwebtoken');
const User = require('../models/User')
const config= require('../config/dbconfig');
const moment = require('moment');

module.exports = {
    signjwt (payload) {
        return jwt.sign(payload,
            config.secretToken
                   )
    },
    payload (usuario) {
        return {
            sub: usuario.id,
            name: usuario.nome,
            email: usuario.email,
            login: usuario.username,
            admin: true,
            iat: Math.floor(moment.now()/1000), // Timestamp de hoje
            exp: moment().add(10, 'minutes').unix() // Validade de 2 dias
        }
    },
    async auth(req,res,next){
        const token = req.header('Authorization');
        console.log(token);
        if(!token) return res.status(401).json('Unauthorized');
        try{
            const decoded = jwt.verify(token,config.secretToken);
            const user = await User.findByPk(decoded.sub);
            console.log(user);
            if(!user){
                return res.status(404).json('User not Found');
            }
            res.json(user);
            next();
        }catch(error){
            console.error(error);
            res.status(400).json('Invalid Token');
        }
    }
}

Initial thoughts:
This code looks very ... compressed without strictly needing to. Another thing that I notice is the level of nesting that shows in here.
Generally speaking it doesn't particularly make a difference how you decide to handle Promises, in the vast majority of cases it's just a matter of personal preference.
One thing you do not need to worry about is specially handling a promise rejection, since express automatically converts thrown errors into an appropriate response. That is what login takes advantage of to look so clean.
One final initial thing I notice is the inconsistency that shows in the code.
Inconsistencies:
Compare the use of spaces in the following example:
const  user   = await User.existLogin(req.body.login);
const isPassword = await User.isPassword(user.dataValues.password, req.body.password);

Or compare the use of braces as well as whitespace for these examples:
if(!token) return res.status(401).json('Unauthorized');
if (!user) { return res.status(400).json({result: 'Login is wrong '});}
if(!user){
    return res.status(404).json('User not Found');
}

Compare also the use of newlines and indentation levels in the following examples:
User.create({
    name, email, login, password }).then(result => {
    res.status(201).json({Results: result.dataValues})
})
return jwt.sign(payload,
    config.secretToken
           )
return {
    sub: usuario.id,
    // ....
    exp: moment().add(10, 'minutes').unix()
}

All this is to say: Pick one style and stick to it. Whichever style you decide on, it makes it easier to read if the style is consistent.
Personally I prefer a style with more space than you've shown here for most cases.
Indentation:
You're using a strange mix of indentation styles and you're making lots of use of early returns, except in some small cases where I don't understand it. I personally prefer to almost always return early inside the request handler and if there's more complex logic I'd consider extracting that into a separate function. That yields code which will look a lot like this:
function addUsers(req, res) {
    const name = req.body.name;
    const email = req.body.email;
    const login = req.body.login;

    const exists = await User.existLogin(login);
    if (exists) {
        return res.status(409).json({ message: 'Login already exists' });
    }

    const password = bcrypt.hashSync(req.body.password, 10);
    const created = User.create({ name, email, login, password });
    res.status(201).json({ results: created.dataValue });
}

Note that I removed the use of await with hashSync (should be clear why).
Also note that I lower-cased the member results in the returned JSON result.
Obviously this doesn't quite have the same error behaviour as the code presented in the question, but it nicely shows how to avoid deeply nesting blocks.
HTTP / web standard authorization patterns
As a minor point I'd like to add that the auth middleware shown here is not quite following the common header authorization pattern. It's missing a response header indicating the expected authorization type. Since you're working with a token here, usually the header would be 'WWW-Authenticate: "Bearer"', but that would imply the expected Authorization header should start with "Bearer", which is not the case here.
To be more ... standard-conform I'd expect a proper bearer-token Authentication scheme to specifically define the authorization type used in the scheme. An option could be to explicitly specify you expect a jwt-token as type resulting in the following headers:
'WWW-Authenticate: "jwt-token"'
---
'Authorization: "jwt-token <jwt_token_goes_here>"'

Note that this suggestion of course would require an adjustment to the way the token is obtained, adding another way the request can be malformed :)
Conclusion:
All in all this is a solid start, to get this code to the next level just requires a few tweaks, mostly around being consistent in how the code is structured in itself.
Well done :)


Converting timedate timezones with Joda-Time I've been struggling to get to grips with Joda-Time, but feel that I've finnaly managed to get a grip on some of the functionality that I need.

I have written a function that will convert a DateTime from one timezone to another.  I've not done any unit testing on it (but have done my own testing), but would appreciate comments on if it could be improved.  The code is in native Java.

import org.joda.time.*;
import org.joda.time.format.*;
public class testing {

    /**
     * @param args
     */
    public static void main(String[] args) {

public string ConvertTimeZones(String sFromTimeZone, String sToTimeZone, String sFromDateTime){
        DateTimeZone oFromZone       = DateTimeZone.forID(sFromTimeZone);
        DateTimeZone oToZone         = DateTimeZone.forID(sToTimeZone);

        DateTime oDateTime           = new DateTime(sFromDateTime);
        DateTime oFromDateTime       = oDateTime.withZoneRetainFields(oFromZone);

        DateTime oToDateTime         = new DateTime(oFromDateTime).withZone(oToZone);

        DateTimeFormatter oFormatter = new DateTimeFormat.forPattern("yyyy-MM-dd'T'H:mm:ss.SSSZ");
        DateTimeFormatter2           = new DateTimeFormat.forPattern("yyyy-MM-dd H:mm:ss");

        DateTime oNewDate            = oFormatter.withOffsetParsed().parseDateTime(oToDateTime.toString());

        return oFormatter2.withZone(oToZone).print(oNewDate.getMillis());
   }
}
}

The arguments:

sFromTimeZone = UTC
sToTimeZone = Europe/London
sFromDateTime = 2012-05-08 18:00:00

should produce a return of

2012-05-08 19:00:00

First of all, currently it does not compile. You should fix that, non-working codes are off-topic here.
According to the Code Conventions for the Java Programming Language, method names should start with lowercase letters and class names should start with uppercase letters. ConvertTimeZones should be convertTimeZones.

Code Conventions for the Java Programming Language, 9 - Naming Conventions
Effective Java, 2nd edition, Item 56: Adhere to generally accepted naming conventions

The o prefix is unnecessary for every variable.
You could omit lots of intermediate objects. Here is a simplified version:

public static String convertTimeZones(final String fromTimeZoneString,
        final String toTimeZoneString, final String fromDateTime) {
    final DateTimeZone fromTimeZone = DateTimeZone.forID(fromTimeZoneString);
    final DateTimeZone toTimeZone = DateTimeZone.forID(toTimeZoneString);
    final DateTime dateTime = new DateTime(fromDateTime, fromTimeZone);

    final DateTimeFormatter outputFormatter
        = DateTimeFormat.forPattern("yyyy-MM-dd H:mm:ss").withZone(toTimeZone);
    return outputFormatter.print(dateTime);
}

Here are a few unit tests:

import static ...TimeZoneConverter.convertTimeZones;
import static org.junit.Assert.assertEquals;

import java.util.Arrays;
import java.util.Collection;

import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameters;

@RunWith(value = Parameterized.class)
public class TimeZoneConverterTest {

    private final String expectedDatetime;
    private final String fromTimezone;
    private final String toTimezone;
    private final String inputDatetime;

    public TimeZoneConverterTest(final String expectedDatetime,
            final String fromTimezone, final String toTimezone,
            final String inputDatetime) {
        this.expectedDatetime = expectedDatetime;
        this.fromTimezone = fromTimezone;
        this.toTimezone = toTimezone;
        this.inputDatetime = inputDatetime;
    }

    @Parameters
    public static Collection<Object[]> data() {
        final Object[][] data = new Object[][] {
            { "2012-05-08 19:00:00",
                "UTC", "Europe/London", "2012-05-08T18:00:00" },
            { "2012-05-08 17:00:00",
                "Europe/London", "UTC", "2012-05-08T18:00:00" },
            { "2012-05-08 20:00:00",
                "UTC", "CET", "2012-05-08T18:00:00" },
            { "2012-05-08 19:00:00",
                "America/Tijuana", "CET", "2012-05-08T10:00:00" },
            { "2012-11-08 21:00:00",
                "Europe/London", "Asia/Dubai", "2012-11-08T17:00:00" },
            { "2012-11-19 2:00:00",
                "Europe/London", "Asia/Tokyo", "2012-11-18T17:00:00" } };
        return Arrays.asList(data);
    }

    @Test
    public void test() {
        assertEquals(expectedDatetime,
            convertTimeZones(fromTimezone, toTimezone, inputDatetime));
    }
}


Hover effects using HTML and SASS I recently created one of my first hover effects using HTML and SASS.  Since it is my first hover effect, I'm almost positive that is it horribly inefficient.  I'm sure there's a way to do this effect using half the number of lines of code, but I can't figure out any ways to do it.  The hover effect in question is for the "envelop" logo at the top left.

Here's a codepen of my page so far.

HTML

(

envelop

)

finish navbar, mobile version formatting is still fucked up....

CSS (SASS)

$green: #4ede96;
$logoAnimationTime: 0.2s;

@mixin mLogoEffect_pre
{
    color: white;
    font-size: 48px;
    display: inline-block;
    transition: $logoAnimationTime 0 ease;
    opacity: 0;
}

header
{
    height: 100px;
    width: 100%;
    background: $green;
    position: fixed;
    top: 0;
    @media only screen and (min-device-width : 320px) and (max-device-width : 480px)
    {
        height: 200px;
    }

    .logocontainer
    {
        @media only screen and (min-device-width : 320px) and (max-device-width : 480px)
        {
            margin-left: 35%;
            display: block;
        }
        padding-top: 15px;
        display: inline-block;

        > a
        {
            text-decoration: none;

            .logo
            {
                margin-top: 0;
                font-family: 'Merriweather', serif;
                font-size: 48px;
                padding: 0 50px;
                color: white;
                display: inline-block;

            }

            .logoeffect-left
            {
                @include mLogoEffect_pre;
                margin-left: 70px;
                margin-right: -80px;
                margin-top: 0;
                margin-bottom: 0;
            }

            .logoeffect-right
            {
                @include mLogoEffect_pre;
                margin-left: -80px;
                margin-top: 0;
                margin-bottom: 0;
            }

            &:hover
            {
                .logoeffect-left
                {
                    transition: $logoAnimationTime 0 ease;
                    margin-right: -50px;
                    margin-left: 40px;
                    opacity: 1;
                    @media only screen and (min-device-width : 320px) and (max-device-width : 480px)
                    {
                        //dont change anything (hide animation)
                        @include mLogoEffect_pre;
                        margin-left: 70px;
                        margin-right: -80px;
                        margin-top: 0;
                        margin-bottom: 0;
                    }
                }
                .logoeffect-right
                {
                    transition: $logoAnimationTime 0 ease;
                    margin-left: -50px;
                    opacity: 1;
                    @media only screen and (min-device-width : 320px) and (max-device-width : 480px)
                    {
                        //dont change anything (hide animation)
                        @include mLogoEffect_pre;
                        margin-left: -80px;
                        margin-top: 0;
                        margin-bottom: 0;
                    }
                }
            }
        }

    }

}

HTML

The headline elements (h1-h6) are meant to be used only for headlines.  You have 3 of them, 2 of which only contain punctuation.  That's a misuse of the element.  If you need markup that's purely for styling purposes, use a span.

(
    envelop
    )

CSS

There's almost no reason to use min-device-width over min-width.  Ever.  There's also no good reason to specify a minimum bound on your "handheld" media query.  The following media query is just as effective, but significantly smaller:

@media only screen and (max-width : 480px) {
    // stuff
}

In a vast majority of cases, going mobile first with your media queries, rather than desktop first is going to get you smaller CSS.

// stuff all devices share
@media only screen and (min-width: 480px) {
    // stuff handheld devices don't need
}

You've got a significant amount of code duplication in your CSS because you're failing to take advantage of the cascade.  For instance, you're including your mLogoEffect_pre mixin 4 times, when you only need it once:

.logoeffect-left, .logoeffect-right {
    @include mLogoEffect_pre;
}

Your entire hover section has way more code than it needs because you're setting/unsetting/resetting properties.  It can be simply expressed like this:

&:hover
{
    .logoeffect-left
    {
        margin-right: -50px;
        margin-left: 40px;
        opacity: 1;
    }
    .logoeffect-right
    {
        margin-left: -50px;
        opacity: 1;
    }
}

I really must caution against splitting up your media queries like that.  It really increases the size of your compiled CSS.

@media (min-width: 480px) {
    .one-selector { }
}

@media (min-width: 480px) {
    .another-selector { }
}

@media (min-width: 480px) {
    .yet-another-selector { }
}

Seems a little silly, don't you think?  Media query bubbling is a fine feature of Sass, but it should be used conservatively.  Much better:

@media (min-width: 480px) {
    .one-selector { }
    .another-selector { }
    .yet-another-selector { }
}

Learn to love shorthand:

.foo {
    margin-left: 70px;
    margin-right: -80px;
    margin-top: 0;
    margin-bottom: 0;
}

Compared to:

.foo {
    margin: 0 -80px 0 70px;
}

On your .logocontainer, you have margin-left: 35%; for handhelds.  Why not just use text-align: right instead?  This will allow you to avoid wrapping that occurs on very narrow devices.

http://codepen.io/cimmanon/pen/JbHAC


Create a dictionary from a list of key and multi list of values in Python I have a list of key:
list_date = ["MON", "TUE", "WED", "THU","FRI"]

I have many lists of values that created by codes below:
list_value = list()
for i in list(range(5, 70, 14)):
    list_value.append(list(range(i, i+10, 3)))

Rules created that:

first number is 5, a list contains 4 items has value equal x = x + 3, and so on [5, 8, 11, 14]

the first number of the second list equal: x = 5 + 14, and value inside still as above x = x +3
[[5, 8, 11, 14], [19, 22, 25, 28], [33, 36, 39, 42], [47, 50, 53, 56], [61, 64, 67, 70]]

I expect to obtain a dict like this:
collections = {"MON":[5, 8, 11, 14], "TUE" :[19, 22, 25, 28], "WED":[33, 36, 39, 42], "THU":[47, 50, 53, 56], "FRI":[61, 64, 67, 70]}

Then, I used:
zip_iterator = zip(list_date, list_value)
collections = dict(zip_iterator)

To get my expected result.
I tried another way, like using the lambda function.
for i in list(range(5, 70, 14)):
    list_value.append(list(range(i,i+10,3)))
    couple_start_end[lambda x: x in list_date] = list(range(i, i + 10, 3))

And the output is:
{<function <lambda> at 0x000001BF7F0711F0>: [5, 8, 11, 14], <function <lambda> at 0x000001BF7F071310>: [19, 22, 25, 28], <function <lambda> at 0x000001BF7F071280>: [33, 36, 39, 42], <function <lambda> at 0x000001BF7F0710D0>: [47, 50, 53, 56], <function <lambda> at 0x000001BF7F0890D0>: [61, 64, 67, 70]}

I want to ask there is any better solution to create lists of values with the rules above? and create the dictionary collections without using the zip method?

One alternative approach is to take advantage of enumerate() -- which allows
you to iterate over what could be thought of as a zip() of a list's indexes and
values. To my eye, this version seems a bit clearer and simpler.
days = ['MON', 'TUE', 'WED', 'THU', 'FRI']

d = {
    day : [
        (5 + i * 14) + (j * 3)
        for j in range(4)
    ]
    for i, day in enumerate(days)
}

print(d)


automating to push changes to github for multiple repos using python This Python script does following activities:

clone repo, as provided from the list
creating a new branch "jenkinsMigrate"
rename Jenkinsfile to Jenkinsfile.migrate
push code to GitHub in a new branch.

NOTE: credentials were configured already.

import git
import os
import subprocess

gitUrl = "https://*****"
cwd = "*****"

with open("migrateList.txt", "r") as file:
    for line in file:
        #configure gitUri for each repository
        gitUri = gitUrl + line.strip() + ".git"
        try:
            global repo
            repo = git.Repo.clone_from(gitUri, cwd + line.strip())
        except:
            print(" directory already available")
        os.chdir(cwd + line.strip())

        #checkout new branch for migration
        repo.git.checkout('-b', "jenkinsMigrate")

        subprocess.call(["git", "mv", "Jenkinsfile", "Jenkinsfile.migrate"])
        repo.git.add(update=True)
        repo.index.commit("jenkins migration")
        origin = repo.remote(name='origin')

        #push new branch to github
        subprocess.call(["git","push", "--set-upstream", "origin", "jenkinsMigrate"])
        subprocess.call(["cd", ".."])

Sample text file:

$ cat migrateList.txt
repo1
repo2
repo3
repo4

This is a working code, but all I am looking for is to maintain consistency in the commands that I use. eg: For some of the git commands that I have used are from gitpython module where as others are invoked with shell commands. Apart from these any other suggestions are welcome.

Thanks!

UPDATE 1: python - 2.7.10

gitPython module

Review

Stick to the PEP8 style guide

Functions and variables should be snake_case
Constants like gitUrl should be UPPER_SNAKE_CASE

There is a function for getting the current working directory

os.getcwd() Depending on your use case this might be useful
If you are using Python3.5+ you should change the older subprocces.call with the newer subprocess.run
Don't catch bare Exceptions

If you now what exceptions are going to be caught it is better to define them


Generating a collection of controls This question I asked previously mentions a function named BuildControlCollection, which I didn't go into the details of since it wasn't relevant. However, because the implementation contains some funky code I'm not 100% on (it completely works, I'm just unsure if it's the best way to do it), I decided to put this up for review too.

Public Sub BuildControlCollection(ByRef ipForm As Form,
                                  ByRef mpCollection As Collection,
                                  ByVal ipControlType As ControlTypes)

The function takes the form that we're building a control collection from, an unset collection object (which will be created and filled), and an enum value to indicate the type(s) of controls to fill the collection with.

Enum ControlTypes
    eTextBox = &H1
    eComboBox = &H2
    eLabel = &H4
    eButton = &H8
    eFrame = &H10
    eRadioButton = &H20
    eListBox = &H40
    eLine = &H80
    eRectangle = &H100
    eCheckbox = &H200
    eChart = &H400
    eAll = &H800
End Enum

Public Sub BuildControlCollection(ByRef ipForm As Form, _
                                  ByRef mpCollection As Collection, _
                                  ByVal ipControlType As ControlTypes)

    If Not mpCollection Is Nothing Then
        Err.Raise 5000, "Collection has previously been set. This operation would delete the collection."
    End If

    Set mpCollection = New Collection

    Dim lControl As Control
    For Each lControl In ipForm.Controls

        If ipControlType And eAll Then
            mpCollection.Add lControl

        ElseIf (ipControlType And ControlTypes.eButton) And TypeName(lControl) = "CommandButton" Then
            mpCollection.Add lControl

        ElseIf (ipControlType And ControlTypes.eChart) And TypeName(lControl) = "ObjectFrame" Then
            mpCollection.Add lControl

        ElseIf (ipControlType And ControlTypes.eCheckbox) And TypeName(lControl) = "CheckBox" Then
            mpCollection.Add lControl

        ElseIf (ipControlType And ControlTypes.eComboBox) And TypeName(lControl) = "ComboBox" Then
            mpCollection.Add lControl

        ElseIf (ipControlType And ControlTypes.eFrame) And TypeName(lControl) = "Frame" Then
            mpCollection.Add lControl

        ElseIf (ipControlType And ControlTypes.eLabel) And TypeName(lControl) = "Label" Then
            mpCollection.Add lControl

        ElseIf (ipControlType And ControlTypes.eLine) And TypeName(lControl) = "Line" Then
            mpCollection.Add lControl

        ElseIf (ipControlType And ControlTypes.eListBox) And TypeName(lControl) = "ListBox" Then
            mpCollection.Add lControl

        ElseIf (ipControlType And ControlTypes.eRadioButton) And TypeName(lControl) = "RadioButton" Then
            mpCollection.Add lControl

        ElseIf (ipControlType And ControlTypes.eRectangle) And TypeName(lControl) = "Rectangle" Then
            mpCollection.Add lControl

        ElseIf (ipControlType And ControlTypes.eTextBox) And TypeName(lControl) = "TextBox" Then
            mpCollection.Add lControl

        End If

    Next lControl

End Sub

The thinking behind the last argument of BuildControlCollection is to allow multiple options to be passed (eg eTextBox Or eButton) - I've seen this used in the built-in functions such as MsgBox - the second argument (somewhat inaccurately named Buttons) of which allows you to specify for eg vbOKOnly Or vbExclamation to get a messagebox with both an OK button and a warning triangle. I don't know what this is called, so I haven't been able to Google a real implementation, so I've had to make my best guess at it.

I understand that it works by comparing bits - for eg, vbOKOnly may be 0000 0001, whilst vbExclamation may be 0001 0000, so passing vbOkOnly Or vbExclamation (0001 0001) matches on both bits.

Whilst my implementation above definitely works, that enormous If/ElseIf smells funny. If anyone can tell me what the bit flagging thing used in MsgBox is called, that would be really useful too.

I don't know what this is called, so I haven't been able to Google a real implementation, so I've had to make my best guess at it. [...] If anyone can tell me what the bit flagging thing used in MsgBox is called, that would be really useful too.

They're called Flag enums in .NET (see this SO question), and apparently the naming is also appropriate for VB6 enums.

That If block definitely smells, because all branches result in lControl being added to mpCollection. Hence, it's not really an If...Else If logic you need here, rather something like:

If CanAddThisControl(ipControlType, lControl) Then mpCollection.Add lControl

This effectively eliminates/replaces the entire If block, but leaves you with a CanAddThisControl method to implement. Let's see...

Private Function CanAddThisControl(ipControlType As ControlTypes, lControl As Control) As Boolean

    'return true if the enum value matches the control's type

End Function

This is where VB6/VBA's lack of structures really hurts. What you need is really some kind of KeyValuePair that associates an enum value with a control type. What if we created a class to do just that?

Private Type tKeyValuePair
    key As Variant
    value As Variant
End Type

Private this As tKeyValuePair
Option Explicit

Public Property Get key() As Variant
    If IsObject(this.key) Then
        Set key = this.key
    Else
        key = this.key
    End If
End Property

Public Property Let key(k As Variant)
    If IsEmpty(k) Then Err.Raise 5
    this.key = k
End Property

Public Property Set key(k As Variant)
    If IsEmpty(k) Then Err.Raise 5
    Set this.key = k
End Property

Public Property Get value() As Variant
    If IsObject(this.value) Then
        Set value = this.value
    Else
        value = this.value
    End If
End Property

Public Property Let value(v As Variant)
    this.value = v
End Property

Public Property Set value(v As Variant)
    Set this.value = v
End Property

Public Function ToString() As String
    ToString = TypeName(Me) & "<" & TypeName(this.key) & "," & TypeName(this.value) & ">"
End Function

(damn VB6 case insensitivity!)

So now we have a way of associating enum values with a string:

Private Function CreateKeyValuePair(key As ControlTypes, value As String) As KeyValuePair
    Dim result As New KeyValuePair
    result.key = key
    result.value = value
    Set CreateKeyValuePair = result
End Function

Private Function GetControlTypesAsKeyValuePairs As Collection
    Dim result As New Collection
    result.Add CreateKeyValuePair(ControlTypes.eButton, "Button")
    result.Add CreateKeyValuePair(ControlTypes.eChart, "ObjectFrame")
    result.Add CreateKeyValuePair(ControlTypes.eCheckBox, "CheckBox")
    result.Add CreateKeyValuePair(ControlTypes.eComboBox, "ComboBox")
    result.Add CreateKeyValuePair(ControlTypes.eFrame, "Frame")
    result.Add CreateKeyValuePair(ControlTypes.eLabel, "Label")
    result.Add CreateKeyValuePair(ControlTypes.eLine, "Line")
    result.Add CreateKeyValuePair(ControlTypes.eListBox, "ListBox")
    result.Add CreateKeyValuePair(ControlTypes.eRadioButton, "RadioButton")
    result.Add CreateKeyValuePair(ControlTypes.eRectangle, "Rectangle")
    result.Add CreateKeyValuePair(ControlTypes.eTextBox, "TextBox")
    Set GetControlTypesAsKeyValuePairs = result
End Function

The above code could be simplified to a one-liner if you implemented a List to wrap the poorly tooled Collection class; see this CR post:

  Private Function GetControlTypesAsKeyValuePairs() As List
      Dim result As New List
      result.Add CreateKeyValuePair(ControlTypes.eButton, "Button"), _
                 CreateKeyValuePair(ControlTypes.eChart, "ObjectFrame"), _
                 CreateKeyValuePair(ControlTypes.eCheckBox, "CheckBox"), _
                 CreateKeyValuePair(ControlTypes.eComboBox, "ComboBox"), _
                 CreateKeyValuePair(ControlTypes.eFrame, "Frame"), _
                 CreateKeyValuePair(ControlTypes.eLabel, "Label"), _
                 CreateKeyValuePair(ControlTypes.eLine, "Line"), _
                 CreateKeyValuePair(ControlTypes.eListBox, "ListBox"), _
                 CreateKeyValuePair(ControlTypes.eRadioButton, "RadioButton"), _
                 CreateKeyValuePair(ControlTypes.eRectangle, "Rectangle"), _
                 CreateKeyValuePair(ControlTypes.eTextBox, "TextBox")
      Set GetControlTypesAsKeyValuePairs = result
  End Function

Now that we have a way of associating each enum value with a specific string, we're equipped to implement CanAddThisControl - I'll assume you went with a Collection, but the code would be pretty much identical if you used the List class I've mentioned above (just swap Collection for List):

Private Function CanAddThisControl(ipControlType As ControlTypes, lControl As Control) As Boolean

    Dim enums As Collection
    Set enums = GetControlTypesAsKeyValuePairs

    Dim kvp As KeyValuePair
    For Each kvp In enums
        If (ipControlType And kvp.Key) And TypeName(lControl) = kvp.Value Then
           CanAddThisControl = True
           Exit For
        End If
    Next

End Function

This should enable you to make the loop in BuildControlCollection as simple as this:

For Each lControl In ipForm.Controls

    If (ipControlType And eAll) Or CanAddThisControl(lControl) Then
        mpCollection.Add lControl
    End If

Next

Now this is a little inefficient, because CanAddThisControl is rebuilding the KeyValuePair collection every time it's called. But it's a fair start I think.


Converting prefix expressions to infix Here is my new working code. Man, working with strings in C is tedious  and annoying. I had built the algorithm in short time, took hours figuring how to implement it properly in C. Tell me if there are more bugs or if some improvement is possible.

#include<stdio.h>
#include<stdlib.h>
#include<string.h>
typedef struct node{
  char data[100];
  struct node *next;
}SNode;

typedef struct{
 int count;
 SNode *top;
}stack;

int isEmpty(stack *s){
  return (s->count==0);
 }

void push(stack *s,char x[]){
  SNode *temp = (SNode *)malloc(sizeof(SNode));
  strcpy(temp->data,x);
  temp->next = s->top;
  s->top=temp;
  s->count++;
  }

char * pop(stack *s){
     if(isEmpty(s)){
      return "-1";

     }
    SNode *temp = s->top;
    s->top = temp->next;
    static char a[10];
    strcpy(a,temp->data);
    free(temp);
    s->count--;
    return a;
  }

 int main(){
    stack *s = (stack *)malloc(sizeof(stack));
    s->count = 0;
    char a[100];
    printf("Enter prefix expression: ");
    gets(a);
    int i;
    for(i=strlen(a)-1;i>=0;i--){
    if(a[i]=='+'||a[i]=='-'||a[i]=='*'||a[i]=='/'){
       char s1[10], s2[10];
       strcpy(s1,pop(s));
       strcpy(s2, pop(s));
       char b1[10] ="(",b2[10]=")";
       strcat(b1,s1);
       char t[2]="\0";
       t[0]=a[i];
       strcat(b1,t);
       strcat(b1,s2);
       strcat(b1,b2);
       push(s,b1);
      }
     else if(isalpha(a[i])){
         char t[2] = "\0";
          t[0] = a[i];
          push(s,t);
     }
    }
    char *infix = pop(s);
    printf("Infix expression: %s",infix);
    return 0;
     }

malloc()

In general, malloc may fail, and will return NULL when that happens, you should deal with that.

The result of malloc does not have to be cast to the target type, additionally it is usually safer to use the target variable in the sizeof expression.

stack *s = malloc(sizeof *s);
if (s == NULL) { // deal with it}

gets()

You have a buffer that is 100 chars wide, the long-deprecated gets() will just write over the end of the buffer, you should try and use fgets() e.g fgets(buffer, 100, stdin) or alternatively gets_s() if your compiler supports C11's optional Annex K.

Editing the string in place

Meaning changing the characters in the string without extra memory allocation. You're changing the string in place, this probably makes things much more complicated than they need to be. Using an input and an output string would clear things up. Using the pointer into the string rather than the string index might make things a little bit more readable as well. I don't think I would choose to edit the string in place unless I was forced to do it. Not shifting the string also makes it easier to visualize what's going on as you only have to move the index forward.

Accessing the string via an index

With char* buffer, buffer[i] and *buffer are equivalent; I'd probably trend more towards using the string pointer rather than the index, that might be a matter of taste.

Naming consistency

You have the node struct typedefed as SNode and the stack just typedefed as stack. It's easy to spot when someone is inconsistent, no matter if I agree with the style or not. Here there are two ways of how you define the struct and two ways (one capitalized, and the other not) of writing the typedef; stick to one way.

pop()

Popping an empty stack is a bug in most cases, if not you probably should treat it like a bug and assert on the empty stack. Returning \0 hides the fact that you tried to call pop on the empty stack making it harder to track down the bug

Warnings

isalpha() is defined in <ctype.h> which is not included

Bug

*+ab+cd is parsed as a + b * c + d and given operator precedence that is incorrect - the result should be (a + b) * (c + d).


Repeating use of "v." annoys me. Do you see a nicer way? This code smells to me, but I don't see the way to clean it up. I see v. mentioned over and over. Any hints would be appreciated!

  #
  # Create a new Value if it doesn't already exist, and initialize the attributes per
  # the parameters of the call.
  #
  def self.find_or_create_value(prog_id, part_id, round_id, quest_id, new_value=nil)
    prog = Program.find(prog_id)
    attrs = {participant_id: part_id, round_id: round_id, question_id: quest_id}
    v = prog.values.where(attrs).first
    if v.nil?
      v = prog.values.build
      v.assign_attributes(attrs, without_protection: true)
    end
    v.value = new_value
    v.save
    v
  end
end

You can use rails find_or_initialize_by method. Here is example:

  def self.find_or_create_value(prog_id, part_id, round_id, quest_id, new_value=nil)
    prog = Program.find(prog_id)
    prog.values.find_or_initialize_by_participant_id_and_round_id_and_question_id(part_id, round_id, quest_id) do |v|
      v.value = new_value
      v.save
    end
  end


Counting the number of words in a string I have written this program to count the number of words in a string. I have checked my program for the worst-case scenario. If any of you can find any cases for which this program doesn't work, please let me know so that I can work and improve it.

And just to be clear, we don't want symbols like, "," , "!" , "?", "." , "\n" to be counted as words. But obviously, "I" should be counted as word, as we consider it in the language. I have made sure of all this by replacing them with spaces. Let me know if I have missed something.

Is this a good solution?  How can I improve the code?

#include <iostream>
#include <string>

void replace(std::string& str, char x, char y);
int countWords(std::string x);

int main(){
std::cout<<countWords(" \n \t Hello, world ! ");
}

void replace(std::string& const str, char x, char y){
for(unsigned int i=0;i<str.size();i++){
    if(str[i]==x) str[i]=y;
}
}

int countWords(std::string x){

replace(x,',',' ');
replace(x,'.',' ');
replace(x,'!',' ');
replace(x,'?',' ');
replace(x,'(',' ');
replace(x,')',' ');
replace(x,'\n',' ');
replace(x,'\t',' ');
replace(x,'"',' ');

if(x.empty()) return 0;
int Num=1;

for(unsigned int i=1;i<x.size();i++){
    if(x[i]==' ' && x[i-1]!=' ') Num++;
}
if(x.back() == ' ') Num--;
return Num;
}

I personally would use the built-in way of reading words.

std::stringstream wordStream("A list of words");

std::string       word;
int               count = 0;
while(wordStream >> word)
{
    ++count;   // We read one space separated words
}              // from the stream.

Of course the default does not handle punctuation correctly (as the words are divided by space. But we can handle that.

So how do we make things look like a space.
All you need to do is tell the stream what characters are spaces. To do this you need to create a ctype facet and implant it into the stream.

#include <locale>
#include <string>
#include <sstream>
#include <iostream>

// This is my facet that will treat the ,.- as space characters and thus ignore them.
class WordSplitterFacet: public std::ctype<char>
{
    public:
        typedef std::ctype<char>    base;
        typedef base::char_type     char_type;

        WordSplitterFacet(std::locale const& l)
            : base(table)
        {
            std::ctype<char> const&  defaultCType  = std::use_facet<std::ctype<char> >(l);

            // Copy the default value from the provided locale
            static  char data[256];
            for(int loop = 0;loop < 256;++loop) { data[loop] = loop;}
            defaultCType.is(data, data+256, table);

            // Modifications to default to include extra space types.
            table[',']  |= base::space;
            table['.']  |= base::space;
            table['!']  |= base::space;
            table['?']  |= base::space;
            table['(']  |= base::space;
            table[')']  |= base::space;
        }
    private:
        base::mask  table[256];
};

Now you just have to add it to a stream.

int main()
{
    std::stringstream   wordsAndPunt("word1 ,,, word2");
    wordsAndPunt.imbue(std::locale(std::locale(), new WordSplitterFacet(std::locale()));

    // Now apply the count.
    std::string word;
    int         count = 0;
    while(wordsAndPunt >> word)
    {
        ++count;   // We read one space separated words
    }              // from the stream.
    std::cout << count << "\n";
}

Now we have the basics in place you don't even need the loop. You can replace it with standard algorithms.

int main()
{
    std::stringstream   wordsAndPunt("word1 ,,, word2");
    wordsAndPunt.imbue(std::locale(std::locale(), new WordSplitterFacet(std::locale()));

    // Now apply the count.
    int  count = std::distance(std::istream_iterator<std::string>(wordsAndPunt),
                               std::istream_iterator<std::string>());
    std::cout << count << "\n";

}


Setup ordering in priority blocking queue I have a PriorityBlockingQueue in which I am adding SocketHolder. And let's say current datacenter where this code is running is abc, we have three datacenters abc, def and pqr.

private static final PriorityBlockingQueue<SocketHolder> liveQueue = new PriorityBlockingQueue<>();

for (SocketHolder socket : listOfSockets) {
    liveQueue.add(socket);
}

Now what I need to do is:

I have to setup some sort of ordering on liveQueue so that all SocketHolder whose datacenter is abc (which is current datacenter where the code is running), they should be at the top so which means when we retrieve it from this liveQueue all SocketHolder with abc datacenter should come out first and then later on all other SocketHolder with other dc's. Similarly for cases where current datacenter is either pqr or def.

This is my SocketHolder class that implements Comparable interface and what I need to do in my compareTo method so that at the top of liveQueue (after adding elements to it) we have all SocketHolder whose datacenter are current datacenter and then followed by other datacenter in which I have no priority.

Here is how I did: The live datacenter will be assigned priority 1, and the other datacenters priority 2. The queue will process the lowest priority first.

public final class SocketHolder implements Comparable<SocketHolder> {
   private final Socket socket;
   private final Datacenter dc;
   private final Context context;
   private final int priority;

   public SocketHolder(Socket socket, Datacenter dc, Context context) {
      super();
      this.socket = socket;
      this.dc = dc;
      this.context = context;
      this.priority = this.dc.name().equalsIgnoreCase(Utils.CURRENT_DATACENTER.get().name()) ? 1 : 2;
   }

   @Override
   public int compareTo(SocketHolder o) {
       return Integer.compare(this.priority, o.priority);
   }
}

I wanted to see if this is the right way to do it or is there is any better way.

You currently couple your SocketHolder class with the environment it exists in, as you have the comparable interface implemented in SocketHolder which in turn uses Utils.CURRENT_DATACENTER.get().name(). I don't think that this is a good idea, as this dependency seems unnecessary and makes isolated testing difficult. (Or in other words: bad gut-feeling ;-))

Suggestion: as you set up your priority queue in a specific environment, and at that place it is probably OK to know the environment. So, use the constructor for priority queue where you pass a Comparator explicitly, and write a comparator which basically does the same as the compareTo-method you have in your SocketHolder right now.


Creating a join table entry by looking up name I wrote a stored procedure that allows a join entity to be created by name for my fictional characters database. I would like some feedback on it with regard to exception handling, and conditions I didn't think of. Also I would like to know if this procedure is good as far as performance goes.

The tables it involves are characters, medias, actors, character_media_actor. The schemas are as follows.

CREATE TABLE medias (
  med_unv_id       INTEGER NOT NULL REFERENCES universes (unv_id),
  med_title        VARCHAR(255) NOT NULL,
  med_release_date DATE,
  med_type         INTEGER REFERENCES media_types (mt_id),
  med_id           SERIAL PRIMARY KEY,
  med_attribs      JSON
);

CREATE OR REPLACE VIEW v_medias AS
SELECT
  med_unv_id,
  med_title,
  med_release_date,
  mt_name as type,
  med_id,
  med_attribs,
  mt_uses_actors
FROM medias m
  JOIN media_types mt ON mt.mt_id = m.med_type;

CREATE TABLE actors (
  act_id    SERIAL PRIMARY KEY,
  act_name  VARCHAR(255) NOT NULL,
  act_birth DATE,
  act_death DATE,
  act_imdb  VARCHAR(4096) UNIQUE,
  act_bio   TEXT,
  is_locked  BOOLEAN DEFAULT FALSE,
  CHECK (act_birth < act_death)
);
DROP TABLE IF EXISTS characters CASCADE;
CREATE TABLE characters (
  chr_unv_id INTEGER NOT NULL REFERENCES universes (unv_id),
  chr_name   VARCHAR(255) NOT NULL,
  chr_id     SERIAL PRIMARY KEY,
  chr_bio    TEXT,
  UNIQUE(chr_name,chr_unv_id)
);
CREATE TABLE character_media_actors (
  cma_act_id INTEGER REFERENCES actors (act_id),
  cma_chr_id INTEGER NOT NULL REFERENCES characters (chr_id),
  cma_med_id INTEGER NOT NULL REFERENCES medias (med_id),
  cma_id     SERIAL PRIMARY KEY,
  UNIQUE(cma_act_id,cma_chr_id,cma_med_id)
);

The stored procedure is as follows:

CREATE OR REPLACE FUNCTION add_cma_by_names(
  IN p_med_title VARCHAR(255),
  IN p_act_name  VARCHAR(255),
  IN p_chr_name  VARCHAR(255)
)
  RETURNS VOID AS $$
DECLARE
  lk_act_id INTEGER := (SELECT act_id
                        FROM actors
                        WHERE LOWER(act_name) = LOWER(p_act_name));
  lk_chr_id INTEGER := NULL;
  lk_med_id INTEGER := NULL;
  md_unv_id INTEGER := NULL;
  cr_unv_id INTEGER := NULL;
  mt_actors BOOLEAN;
BEGIN
  -- perform validation first
  IF p_med_title IS NULL OR LENGTH(p_med_title) = 0
  THEN
    RAISE EXCEPTION 'A media title must be given';
  END IF;
  IF p_chr_name IS NULL OR LENGTH(p_chr_name) = 0
  THEN
    RAISE EXCEPTION 'A character name must be given';
  END IF;

  -- load the media and universe ids in one go
  SELECT
    med_unv_id,
    med_id,
    mt_uses_actors
  INTO md_unv_id, lk_med_id, mt_actors
  FROM v_medias
  WHERE LOWER(p_med_title) = LOWER(med_title);
  -- we require the universe id before we can get the character id
  SELECT
    chr_id,
    chr_unv_id
  INTO lk_chr_id, cr_unv_id
  FROM characters
  WHERE LOWER(p_chr_name) = LOWER(chr_name);

  IF mt_actors IS TRUE AND lk_act_id IS NULL
  THEN
    RAISE EXCEPTION
    'Selected media has actors must not be null: actor = %', p_act_name;
  END IF;

  IF lk_med_id IS NULL
  THEN
    RAISE EXCEPTION 'Media Not Found, name = %', p_med_title;
  END IF;

  IF lk_chr_id IS NULL
  THEN
    RAISE EXCEPTION 'Character Not Found, name = %', p_chr_name;
  END IF;

  IF md_unv_id <> cr_unv_id
  THEN
    RAISE EXCEPTION 'Character and Media must be in same universe';
  END IF;

  INSERT INTO character_media_actors (cma_act_id, cma_chr_id, cma_med_id) VALUES
    (lk_act_id, lk_chr_id, lk_med_id);

  RETURN;
END $$
LANGUAGE PLPGSQL;

I'm not a postgres expert by any means, so take this as just an opinion:

DECLARE
  lk_act_id INTEGER := (SELECT act_id
                        FROM actors
                        WHERE LOWER(act_name) = LOWER(p_act_name));

Several issues with this.

I don't think the DECLARE section should be used for querying a table. I think it should only perform literal assignments. (I don't know if there's any accepted guidance around this, but it seems wrong to me.)

LOWER(act_name) will ensure any index on that column isn't used. Granted it's not an issue in your code because you don't have an index, but in the real world you probably would (or you'd suffer the performance impact of a sequential scan on that table every time the function is called, which would become an issue with a large table).

act_name has no guarantee of uniqueness - and it shouldn't, as there can certainly be more than one actor with the same name. So the above would throw an exception if it found more than one result. What then?

The query runs before the validation. If validation subsequently fails, you've done a sequential scan for nothing.

lk_chr_id INTEGER := NULL;
lk_med_id INTEGER := NULL;
md_unv_id INTEGER := NULL;
cr_unv_id INTEGER := NULL;

None of these variables need to be explicitly set to NULL, as all variables not explicitly initialised are NULL by default. (I'm not sure if this holds for all types e.g. enums, but certainly for all "native" and custom types I've used, it holds.)

  IF p_med_title IS NULL OR LENGTH(p_med_title) = 0
  THEN
    RAISE EXCEPTION 'A media title must be given';
  END IF;
  IF p_chr_name IS NULL OR LENGTH(p_chr_name) = 0
  THEN
    RAISE EXCEPTION 'A character name must be given';
  END IF;

What if it's whitespace? I'd do something like this instead:

IF NULLIF(TRIM(p_med_title), '') IS NULL THEN...

Although, I might cache the trimmed inputs so I don't have to trim them again when using them in the subsequent queries.

Also you could use ELSIF rather than having two separate IF statements.

You should probably also validate p_act_name here before looking it up in the table, or if it's valid for this parameter to be null or empty, then check it's not null/empty before querying the table for the id.

As to the rest, I can't quite understand what exactly it's trying to do and what all the conditions are. I would prefer seeing the desired outcome and constraints written in plain English rather than working it out from the code.