Efficiency of Project Euler problem 35 The number, 197, is called a circular prime because all rotations of the digits: 197, 971, and 719, are themselves prime.
There are thirteen such primes below 100: 2, 3, 5, 7, 11, 13, 17, 31, 37, 71, 73, 79, and 97.
How many circular primes are there below one million?

I have written the following program in python and am new to programming in Python. I am looking for suggestions for making efficient programs and developing good programming habits.
import math
data ={}

def isPrime(n):
    global data
    if n in data:
        return data[n]

    for num in range(2,math.floor(math.sqrt(n)+1)):
        if n%num == 0:
            data[n]=False
            return False
    data[n]=True
    return True
count =0
data ={}

for num in range (2,1000000):
    q=False
    num=str(num)
    for i in range(len(num)):
        if (isPrime(int(num[i:]+num[:i]))):
            q=True
        else:
            q=False
            break
    if q:
        count+=1
print (count)

Style

In Python, there is usual style guide called PEP8 : I think you should follow it to make your code more consistent with all the Python code out in the wild out there. If you want to, you'll find various tools to help you check your Python code : pep8, pycheck, pylint, pyflakes.

Global variables

The fact that you are using a global variable is a good hint that your are probably doing something wrong. It makes your code harder to track but also harder to test and to benchmark.

Also, your should be moving your code which actually performs computations behing an if __name__ == "__main__": : this allows one to easily find where stuff are triggered in your code but also and more importantly to be able to re-use your code by importing your project without re-launching all the computations you might not care about while solving a different project euler problem.

At this point, your code looks like :

import math

def is_prime(n, dict_primes):
    if n in dict_primes:
        return dict_primes[n]

    for num in range(2, math.floor(math.sqrt(n)+1)):
        if n % num == 0:
            dict_primes[n] = False
            return False
    dict_primes[n] = True
    return True

def main():
    """Main function"""
    print("Hello, world!")
    count = 0
    dict_primes = {}

    for num in range (2, 1000000):
        q = False
        num = str(num)
        for i in range(len(num)):
            if is_prime(int(num[i:]+num[:i]), dict_primes):
                q = True
            else:
                q = False
                break
        if q:
            count += 1
    print (count)

if __name__ == "__main__":
    main()

Optimisations : different algorithm

You are using some kind of cache to remember the different numbers you've checked and this is a good idea. Also, you are using a quite efficient way to check if a single number is prime, stopping at sqrt(n) + 1.
However, the best way to do this would probably be to implement a Sieve of Eratosthenes : we build it once and for all and we don't need to compute things later on. This is especially convenient here because we know that we will not need to check for any number bigger than one million.

This is already much faster :

import math

def sieve(limit):
    primes = [True] * limit
    primes[0] = primes[1] = False
    for i in range(2, math.floor(math.sqrt(limit))):
        if primes[i]:
            for j in range(i*i, limit, i):
                primes[j] = False
    return primes

def main():
    """Main function"""
    print("Hello, world!")
    limit = 1000000
    count = 0

    primes = sieve(limit)
    for num in range (2, limit):
        q = False
        num = str(num)
        for i in range(len(num)):
            if primes[int(num[i:]+num[:i])]:
                q = True
            else:
                q = False
                break
        if q:
            count += 1
    print (count)

if __name__ == "__main__":
    main()

Details

At the moment, you are using q to store whether all permutations of num where prime numbers so far. It might be interesting to point out that in Python, you can add an else to loops signifying that that was no break in the loop. Thus, you could write something like :

    for i in range(len(num)):
        if not primes[int(num[i:]+num[:i])]:
            break
    else:
        count += 1

Alternatively, you could use the all function to write it in a clean and concise way :

    if all(primes[int(num[i:]+num[:i])] for i in range(len(num))):
        count += 1

Another optimisation

I haven't checked that this really makes the code faster but let's assume that 193939 and all its permutations are prime. Instead of checking all permutations for each permuation and adding one each time every thing is fine, we could only check that we are considering the smallest permutation and add the number of permutations directly. This would look like :

   perm = {int(num_s[i:]+num_s[:i]) for i in range(len(num_s))}
    if all(n >= num and primes[n] for n in perm):
        count += len(perm)

(Please note that I have used a set as I don't want 11 for instance to be counted multiple times just because it is its own permutation).

Another optimisation

If all permutations are supposed to be primes : we know that the first itself must be prime : we can get rid of even numbers : for num in range(3, limit, 2): (you have to initialise count with 1 to count 2).

Also, we know that no even numbers must be in the string : you can add : odd_numbers = {1, 3, 5, 7, 9} out of the loops and if {int(n) for n in num_s}.issubset(odd_numbers): in the loop.

Similarly, 5 can be excluded from the list of allowed numbers as anything ending with 5 with be a multiple of 5 : you can change odd_numbers for final_numbers = {1, 3, 7, 9} and start counting from 2.

Again, this is not benchmarked but you can give it a try if you want.

And now for something different

Instead of going through all numbers and checking conditions that will most of the time not be true, you can use the fact that you already know that for numbers bigger than 9, only {1, 3, 7, 9} are allowed. Thus, you could try to go through the carthesian product of theses numbers using itertool.product.

When considering numbers with n digits, you'll only consider 4**n numbers instead of 10**n (when n = 6 for instance, it makes the difference between 4096 and 100000).

Once this is done, you can even afford to check for much bigger numbers :

def main():
    """Main function"""
    print("Hello, world!")
    count = 4
    nb_digits = 8
    primes = sieve(10**nb_digits)
    final_numbers = {'1', '3', '7', '9'}
    for l in range(2, nb_digits+1):
        for p in itertools.product(final_numbers, repeat=l):
            p_int = int(''.join(p))
            perm = {int(''.join(p[i:]+p[:i])) for i in range(len(p))}
            if p_int == min(perm) and all(primes[n] for n in perm):
                print(p, len(perm))
                count += len(perm)
    print (count)

This code checks for numbers of 8 digits by doing 13939 prime checks. Thus, it is not worth using the Eratosthene sieves anymore.

Using your initial code with no cache, we can achieve great performances :

def is_prime(n):
    for num in range(2,math.floor(math.sqrt(n)+1)):
        if n%num == 0:
            return False
    return True

def main():
    """Main function"""
    print("Hello, world!")
    count = 4
    nb_digits = 9
    final_numbers = {'1', '3', '7', '9'}
    for l in range(2, nb_digits+1):
        for p in itertools.product(final_numbers, repeat=l):
            p_int = int(''.join(p))
            perm = {int(''.join(p[i:]+p[:i])) for i in range(len(p))}
            if p_int == min(perm) and all(is_prime(n) for n in perm):
                print(p, len(perm))
                count += len(perm)
    print (count)


Download and write ~100k images Currently I will be downloading around ~100K images. This process may run on a weekly basis, I'm considering the following factors:

HTTP Requests (Will be taken care by external LB, so same source IP won't be used)
Multi-threading (concurrent.futures.ThreadPoolExecutor library to handle multi-threading)
Writing requests (Regular open and close methods)

I'm using urllib2 to handle HTTP requests, I would like to see if this properly designed to run at scale.

Any additional comments? Suggestions?

import concurrent.futures
import urllib2
import pandas as pd

MAX_WORKERS = 5
EXECUTOR_TIMEOUT = 60
FILENAME = 'files.csv'
_IMG_EXTENSION = 'jpg'

class Image():
    def __init__(self, master_id, url):
        self.master_id = master_id
        self.url = url

def get_file(filename):
    """Get dataframe information"""
    data = pd.read_csv(filename)
    data = data.drop_duplicates(subset=['id'], keep='first')
    subset = data.head()
    return subset

# Data extraction
def extract_data(data):
    """Extract data"""
    image_list = []
    for _, url in data.iterrows():
        print url[0], url[1]
        image_list.append(Image(url[0], url[1]))
    return image_list

# Retrieve a single page and report the url and contents
def load_url(image, timeout):
    """Load URL"""
    response = urllib2.urlopen(image.url, timeout=timeout)
    return response.read()

# Save image
def save_image(image_data, filename):
    """Save Image."""
    with open(str(filename) + '.' + _IMG_EXTENSION, 'wb') as output:
        output.write(image_data)
        output.close()

def download_data(image_list):
    # We can use a with statement to ensure threads are cleaned up promptly
    with concurrent.futures.ThreadPoolExecutor(max_workers=MAX_WORKERS) as executor:
        # Start the load operations and mark each future with its URL
        future_to_url = {executor.submit(load_url, image, EXECUTOR_TIMEOUT): image for image in image_list}
        for future in concurrent.futures.as_completed(future_to_url):
            image = future_to_url[future]
            try:
                image_data = future.result()
            except Exception as exc:
                print('%r Generated an exception: %s' % (image.url, exc))
            else:
                print('%r Page is %d bytes' % (image.url, len(image_data)))
                save_image(image_data, image.master_id)

Some minor improvements:

I would switch to a NamedTuple instead of a separate class:

from collections import namedtuple

Image = namedtuple('Image', ('master_id', 'url'))

Or, if you are going to stay with classes, consider adding __slots__ - may help performance-wise - though this part is not likely to be your bottleneck, but rather can be a quick and easy win
you can also improve your extract_data function:

def extract_data(data):
    """Extract data"""
    return [Image(*url) for _, url in data.iterrows()]

I would also try asyncio from Python 3.x. Here is also a relevant snippet.


Calling a Function to Calculate Multiple Coefficients I'm working through a practice problem and am wondering
if there's a more efficient way of writing my code  and/or if the syntax for my pointers looks correct.

The practice problem:

Given the absolute temperatures Tc of a cold reservoir and Th of a hot reservoir (in degrees Kelvin), the coefficients of performance (cp) of the refrigeration cycle and heat pump are given by:

Refrig: cp = Tc / (Th - Tc)
Heat: cp = Th / (Th - Tc)

Write a program that will call a function to prompt the user for the values of  Tc and Th .  It then calls a function that calculates the two coefficients of performance.  Finally, it calls a function to print the results.

Here is what I have:

#include <stdio.h>

void temperature(float *, float *);
void performance(float,float,float *, float *);
void PrintResults(float,float,float,float);

int main()
{
    float Tc, //cold reservoir; user input
        Th, //hot reservoir; user input
        Rcp, //refrigeration cycle coefficient of performance
        Hcp; // Heat pump coefficient of performance

    temperature(&Tc,&Th); //Prompts user for cold & hot reservoir temperatures
    performance(Tc,Th,&Rcp,&Hcp); //Calculates the coefficients of performance
    PrintResults(Tc,Th,Rcp,Hcp); //Prints the coefficients of performance
    return 0;
}
void temperature(float *ptrTc, float *ptrTh)
{
    printf("Enter the absolute temperature for a cold reservoir (in Kelvin): ");
    scanf("%f",&*ptrTc);
    printf("Enter the absolute temperature for a hot reservoir (in Kelvin): ");
    scanf("%f",&*ptrTh);
}
void performance(float Tc, float Th, float *ptrRcp, float *ptrHcp)
{
    *ptrRcp = Tc/(Th-Tc);
    *ptrHcp = Th/(Th-Tc);
}
void PrintResults(float Tc, float Th, float Rcp, float Hcp)
{
    printf("\nGiven the absolute temperatures of %.2f K for the cold reservoir",Tc);
    printf(" and %.2f K for the hot reservoir,",Th);
    printf(" the coefficient of performance for the refrigeration cycle is %.2f K",Rcp);
    printf(" and the coefficient of performance for the heat pump is %.2f K\n",Hcp);
}

if the syntax for my pointers looks correct

The syntax for the pointers is correct in all the function parameters, however, the pointer syntax is incorrect or unnecessary in the two scanf statements. The two output variables passed into temperature(float *, float*) are already addresses of variables so in this case the proper code would be

    scanf("%f", ptrTc);
    scanf("%f", ptrTh);

You would only need & if this was in the main program and Tc and Th were being read directly.

    float Tc;
    scanf("%f", &Tc);

  if there's a more efficient way of writing my code

All four of the variables are related, it might be better to use a struct to contain them. That way if the program needed to be modified to loop through data it would be easier to build an array of all the values. Using a struct would also decrease the number of parameters for each function to one.

Error Checking
A best practice is to check all input data as it comes in and report any errors, especially for user input. Users can make errors such as entering characters rather than numbers. The temperature(float *, float*) function could return an integer the indicated success or failure.

Separate and Initialize in Variable Declarations
There are 2 things that can be improved in the following code:

    float Tc, //cold reservoir; user input
        Th, //hot reservoir; user input
        Rcp, //refrigeration cycle coefficient of performance
        Hcp; // Heat pump coefficient of performance

This code is hard to maintain because it is hard to add a variable to the list.

Not initializing variables can lead to hard to debug problems later, this is especially true of the C programming language because it does not automatically initialize variables to a zero or null value as some other languages do. Of the two problems this is actually the more serious one, the other might be considered style.

An possible example of an improved declaration is:

    float Tc = 0.0;  //cold reservoir; user input
    float Th = 0.0;  //hot reservoir; user input
    float Rcp = 0.0; //refrigeration cycle coefficient of performance
    float Hcp = 0.0; // Heat pump coefficient of performance

Spacing, Both Vertical and Horizontal
Within expressions is it common to put spaces between operators and operands, this makes the code more readable.

Vertical separation of one line between functions might make the code more readable as well.


Java 8 functions to avoid try-catch repetition I'm developing a Java 8 + JSF application, where all the view classes extend from an abstract class called Navegable.

All the methods called from the view and intended to modify the DB in any way, have to add an information message if everything goes OK or an error message otherwise. This involves wrapping many of my methods in a try-catch clause. In order to avoid that, I've tought I could use functional programming and prepare some generic function in the parent class instead. This way:

public abstract class Navegable implements Serializable {

    protected FacesUtils facesUtils;

    protected void execute(Function<Void, Void> function) {
        try {
            function.apply(null);
            facesUtils.addInfoMessage("PROPERLY_SAVED");
        } catch (Exception ex) {
            facesUtils.addErrorMessage("ERROR");
            throw ex;
        }
    }

    public Navegable(FacesUtils facesUtils) {
        super();
        this.facesUtils = facesUtils;
    }

}

Then I'd have to call it from each of my methods modifying the DB in children classes:

public class NavegableEquipment extends Navegable {
    public void assignWorker() {
        execute(new Function<Void, Void>() {

            @Override
            public Void apply(Void t) {
                assignWorker(selectedEquipment);
                reloadEquipmentById(selectedEquipment.get_Id());
                return null;
            }
        });
    }

    //More methods
}

However, the caller method still looks like a bit verbose in its implementation. Do I have any other choice or am I limited by the language itself?

First of all, Function<Void, Void> is not an appropriate choice of functional interface for this purpose.

Function<T,R> implementation should receive an instance of T and return an instance of R (although T and R still can be of same type).

If there is no incoming type, this looks like a Supplier.

If there is no returned type, it resembles a Consumer.

If there are no types to specify, Runnable can be used, but it'd better be kept for its initial concurrent usage.

So a dedicated functional interface can be created to solve the problem:

@FunctionalInterface
interface ExecutedWithFacesLogger {

  void executeWithFacesLogger();

}

Now, the execute method changes to the following:

protected void execute(ExecutedWithFacesLogger function) {
  try {
    function.executeWithFacesLogger();
    facesUtils.addInfoMessage("PROPERLY_SAVED");
  } catch (Exception ex) {
    facesUtils.addErrorMessage("ERROR");
    throw ex;
  }
}

And the caller is reduced to:

public void assignWorker() {
  execute(() -> {
      assignWorker(selectedEquipment);
      reloadEquipmentById(selectedEquipment.get_Id());
  });
}


Generic list heapsort in Java This is an implementation of heapsort as seen on Programming Pearls, adapted for java List collections. It is working on a copy of the list passed-in on purpose at the moment.

import java.util.ArrayList;
import java.util.List;

public final class HeapSort {

    private HeapSort() {}

    public static <T extends Comparable<T>> List<T> heapSort(List<T> unsortedList) {
        assert unsortedList != null;
        assert unsortedList.size() > 1;
        List<T> sortedList = new ArrayList<T>(unsortedList);
        for (int i = 1; i < sortedList.size(); i++) {
            sortedList = siftUp(sortedList, i);
        }
        for (int i = sortedList.size() - 1; i >= 1; i--) {
            sortedList = swap(sortedList, 0, i);
            sortedList = siftDown(sortedList, i - 1);
        }
        return sortedList;
    }

    private static <T extends Comparable<T>> List<T> siftUp(List<T> unsortedList, int n) {
        assert n > 0;
        for (int i = n;;) {
            if (i == 0) {
                break;
            }
            int p = i / 2;
            if (unsortedList.get(p).compareTo(unsortedList.get(i)) >= 0) {
                break;
            }
            unsortedList = swap(unsortedList, i, p);
            i = p;
        }
        return unsortedList;
    }

    private static <T extends Comparable<T>> List<T> siftDown(List<T> unsortedList, int n) {
        for (int i = 0;;) {
            int c = 2 * (i + 1) - 1;

            if (c > n) {
                break;
            }
            if (c + 1 <= n) {
                if (unsortedList.get(c + 1).compareTo(unsortedList.get(c)) > 0) {
                    c += 1;
                }
            }
            if (unsortedList.get(i).compareTo(unsortedList.get(c)) >= 0) {
                break;
            }
            unsortedList = swap(unsortedList, i, c);
            i = c;
        }
        return unsortedList;
    }

    private static <T> List<T> swap(List<T> list, int firstIndex, int secondIndex) {
        assert list != null;
        assert list.size() > 0;
        assert 0 <= firstIndex;
        assert firstIndex < list.size();
        assert 0 <= secondIndex;
        assert secondIndex < list.size();
        if (firstIndex == secondIndex) {
            return list;
        }
        T tmpT = list.get(secondIndex);
        list.set(secondIndex, list.get(firstIndex));
        list.set(firstIndex, tmpT);
        return list;
    }
}

Should I assert on list size in softDown and siftUp? Is the passing back and forth of list references ok?

for (int i = n;;) {
    if (i == 0) {
        break;
    }

This look very odd instead you can do

for(int i = n; i > 0;){

Asserts are kinda useless in java, they only work when the jvm is started with the -ea flag (or you do some reflection). instead just test and throw the IllegalArgumentException in the public method and remove the obvious assertions that would throw anyway.


Get files recursively as relative paths I want to get a recursive list of files in a directory but I want relative paths and not the full paths that Directory.GetDirectories() would return using the AllDirectories search option. This is because I want to move the list of files to another location and perform a custom check on each file. I could try converting each absolute path to a relative path but that's a bit messy, so I decided to create a recursive function that generates relative paths as it goes.

public static string[] GetRelativeFilesRecursive(string path)
{
    IEnumerable<string> GetRelativeFilesRecursive(string basePath, string subFolder)
    {
        IEnumerable<string> getRelativeArray(Func<string, IEnumerable<string>> func)
        {
            return func(Path.Combine(basePath, subFolder)).Select(file => Path.Combine(subFolder, Path.GetFileName(file)));
        }

        IEnumerable<string> files = getRelativeArray(Directory.GetFiles);
        IEnumerable<string> directories = getRelativeArray(Directory.GetDirectories);

        foreach (string directory in directories)
        {
            files = files.Concat(GetRelativeFilesRecursive(basePath, directory));
        }

        return files;
    }

    return GetRelativeFilesRecursive(path, "").ToArray();
}

I'm using nested functions to make it more modular, i.e. I'd rather have one function that I can copy and paste as a whole than have multiple functions that could become separated. I'm using IEnumerable<string> instead of string[] because otherwise I'd have to call ToArray() on every Concat() call and I'm worried that would be bad for performance.

What do you think? Is this a good way to do this?

Building on Henrick's answer, using Ling to return the IEnumerable:

return Directory.GetFiles(root, "*", SearchOption.AllDirectories)
              .Select(p => p.Remove(0, rootLength));


Queue Implementation First time I'm using stackexchange. I'm in 8th standard. I tried to implement Queue in Java. I wrote something like this> Right now I'm not able to test it because I don't have JVM in my machine. So I just wanted to know Is this correct implementation? Am I going in the right direction?

 import java.lang.*;
 import java.util.*;

 public class Queue
{
public static void main(string[] args)
{
    LinkedList l1=new LinkedList();
    InputstreamReaderir=new InputstreamReader(System.in);
    BufferReader bf=new BufferReader(ir);
    System.out.println("Enter the number of elements to be inserted into queue");
    string str;
    str=bf.readline();
    System.out.println("Enter your Choice: ");
    System.out.println("1->Insert 2->Delete 3->Display 3->Exit");
    string choice;
    choice.readline();
    for(;;)
    {
        switch
        {
            case 1:l1.insertrear();
                   break;
            case 2:l1.deletefront();
                break;
            case 3: system.out("Contents of Queue are:" +l1);
                break;
            default: exit;
        }
    }
}

you're on the right track with using a LinkedList, this is one of the easiest ways to implement your own Queue - I hope this is an exercise, since you should otherwise use Java's built-in Queue.

I must confess I found your question interesting and decided to write a Queue so as to demonstrate. Note that you should create a class for your custom Queue. If you implement the whole thing in main, there'll be no way to reuse the code.

So you'd create a new file, let's call it Queue.java. It would look something like this:

import java.util.*;

public class Queue<T> implements Iterable<T> {

    private LinkedList<T> elements = new LinkedList<T>();

    public void enqueue(T element) {
        elements.add(element);
    }

    public T dequeue() {
        return elements.removeFirst();
    }

    public T peek() {
        return elements.getFirst();
    }

    public void clear() {
        elements.clear();
    }

    public int size() {
        return elements.size();
    }

    public boolean isEmpty() {
        return elements.isEmpty();
    }

    @Override
    public Iterator<T> iterator() {
        return elements.iterator();
    }
}

Notice that that the code is extremely simple - no method's implementation is longer than one line. However, I don't know your level of experience so there might be two weird things in here: the use of Generics and the implements Iterable<T>.

Generics

The <T> means you can put any Java object into the Queuekeeping Type safety.
This means, when you call Queue.Dequeue(), the result will still have the same type that it had when you Enqueued
it.

Iterable<T>

Oracle says:

  Implementing this interface allows an object to be the target of the foreach statement.

Note that you don't need to know more, because we simply delegate this responsibility to LinkedList.iterator() which does the work for us.

Below is some code to test your new Queue. Play around with it. You can add other queue tests to main, for instance:

testQueue(true, false, true, true);

or

testQueue(0.5, 3.6, -5.4, 0.0);

to test putting different types of data into your Queue.
If anything remains unclear, feel free to leave a comment, and I'll update my answer.

public class Main {

    public static void main(String[] args) {
        testQueue(1, 2, 3, 4, 5);
        testQueue("first", "second", "third");
    }

    private static <T> void testQueue(T... elements) {
        outputLine("testing a Queue<%s>", getClassOf(elements));
        Queue<T> queue = new Queue<T>();
        outputSize(queue);
        enqueueElementsInto(queue, elements);
        outputSize(queue);
        loopOverAllElementsOf(queue);
        dequeueAllFrom(queue);
        outputSize(queue);
        outputLine("---");
    }

    private static <T> void outputSize(Queue<T> queue) {
        outputLine("empty: %s, size: %s elements", queue.isEmpty(), queue.size());
    }

    private static <T> void enqueueElementsInto(Queue<T> queue, T... elements) {
        outputLine("enqueueing: ");
        for (T element : elements) {
            queue.enqueue(element);
            output(element + " ");
        }
    }

    private static <T> void loopOverAllElementsOf(Queue<T> queue) {
        outputLine("Queue contains: ");
        for (T element : queue) {
            output(element + " ");
        }
    }

    private static <T> void dequeueAllFrom(Queue<T> queue) {
        outputLine("dequeueing: ");
        while (!queue.isEmpty()) {
            T next = queue.peek();
            T dequeued = queue.dequeue();
            outputLine("expected: %s ", next);
            output("dequeued: %s ", dequeued);
        }
    }

    private static void outputLine(String format, Object... params) {
        output('\n' + format, params);
    }

    private static void output(String format, Object... params) {
        System.out.print(String.format(format, params));
    }

    private static <T> String getClassOf(T... elements) {
        String nameOfArray = elements.getClass().getSimpleName();
        return nameOfArray.substring(0, nameOfArray.length() - 2);
    }
}

output:

testing a Queue<Integer>
empty: true, size: 0 elements
enqueueing: 1 2 3 4 5
empty: false, size: 5 elements
Queue contains: 1 2 3 4 5
dequeueing:
expected: 1 dequeued: 1
expected: 2 dequeued: 2
expected: 3 dequeued: 3
expected: 4 dequeued: 4
expected: 5 dequeued: 5
empty: true, size: 0 elements
---
testing a Queue<String>
empty: true, size: 0 elements
enqueueing: first second third
empty: false, size: 3 elements
Queue contains: first second third
dequeueing:
expected: first dequeued: first
expected: second dequeued: second
expected: third dequeued: third
empty: true, size: 0 elements
---


Is this the right way to space four news blocks right from each other? I am working on a website and I've created four text/image news blocks.  I am not sure if it's the right / most efficient way to do this.

See my code here at jsfiddle

HTML:

Lorem ipsum dolor sit amet, consectetur adipisicing elit

Lorem ipsum dolor sit amet, consectetur adipisicing elit

Lorem ipsum dolor sit amet, consectetur adipisicing elit

Lorem ipsum dolor sit amet, consectetur adipisicing elit

        fasdfasf

CSS:

body{
background: url("../img/bg.jpg") repeat-x;
}

#wrapper{
margin: 0 auto;
width: 980px;
}

#header{
float: left;
width: 100%;
height: 70px;
}

#logo img{
float: left;
margin-top: 20px;
}

#headNews{
float: left;
margin-top: 20px

}

.headNewsItem{
float: left;
margin-right: 4px;
width: 242px;
height: 184px;
}

#headNewsItemLast{
float: left;
width: 242px;
height: 184px;
}

.headNewsItem  img {
display: block;
}

#headNewsItemLast img{
display: block;
}

.textContainer{
font-family: Arial;
line-height: 21px;
font-size: 14px;
color: #c8cbcb;
height: 40px;
padding: 10px 15px;
background-image: linear-gradient(#262828,#1c1e1e);
}

#midSection{
float:left;
width: 100%;
height: 800px;
margin-top: 20px;
background-color: #FFF;

}

I wouldn't say that there's a right or a wrong way to do it.  The only problem with what you're doing is that it only works well when you have a device that is wide enough to display all of those items on one line (ie. it doesn't work so well for handheld devices).

My recommendation would be to use the CSS multi-column module, which is responsive by default without having to use media queries.

http://jsfiddle.net/cTM3L/1/

Lorem ipsum dolor sit amet, consectetur adipisicing elit

Lorem ipsum dolor sit amet, consectetur adipisicing elit

Lorem ipsum dolor sit amet, consectetur adipisicing elit

Lorem ipsum dolor sit amet, consectetur adipisicing elit

fasdfasf

The CSS:

@import url(reset.css);
 body {
    background: url("../img/bg.jpg") repeat-x;
}
#wrapper {
    margin: 0 auto;
}
#header {
    height: 70px;
}
#logo img {
    float: left;
    margin-top: 20px;
}
#headNews {
    margin-top: 20px;
    -webkit-columns: 242px;
    -moz-columns: 242px;
    columns: 242px;
    -webkit-column-gap: 10px;
    -moz-column-gap: 10px;
    column-gap: 10px;
    list-style: none;
    padding-left: 0;
}
.headNewsItem {
    margin-right: 4px;
    width: 242px;
    -webkit-column-break-inside: avoid;
    page-break-inside: avoid; /* Moz is weird */
    break-inside: avoid;
    margin-top: 10px;
}
.headNewsItem:first-child {
    margin-top: 0;
}
.headNewsItem a, .headNewsItem img {
    display: block;
}
.textContainer {
    font-family: Arial;
    line-height: 21px;
    font-size: 14px;
    color: #c8cbcb;
    height: 40px;
    padding: 10px 15px;
    background-image: linear-gradient(#262828, #1c1e1e);
}
#midSection {
    width: 100%;
    margin-top: 20px;
    background-color: #FFF;
}

There's quite a few other things I would change here, but most of them are not relevant to the requested reviewed.  Just make sure you're using the most specific tag that appropriately describes your content: avoid using div for everything (check out some of those HTML5 tags you seem to have missed such as header, figure/figcaption, aside, etc.).  Also, there are better ways to clear floats than to use more floats.


Beginners Java calculator I am a beginner in Java programming, and I created a calculator as my first Java project. Therefore, can you review my code and provide feedbacks/ tips?

import java.util.Scanner;

public class Calculator {
    public static void main(String[] args) {

        double firstnum, secondnum, answer1, answer2, answer3, answer4, answer5, answer6;
        String text;
        int loop = 0;

        Scanner input = new Scanner (System.in);

        while (loop == 0) {

        System.out.println("Enter your first number: ");
        firstnum = input. nextDouble();

        System.out.println("Please enter the operation you would like to execute: ");
        text = input.next();

        System.out.println("Enter your second number: ");
        secondnum = input.nextDouble();

        answer1= firstnum+secondnum;
        answer2= firstnum-secondnum;
        answer3= firstnum*secondnum;
        answer4= firstnum/secondnum;
        answer5= (100*(firstnum/secondnum));
        answer6= Math.pow(firstnum,secondnum);

        switch (text) {

        case "+" :
            System.out.println(answer1);
            break;

        case "-" :
            System.out.println(answer2);
            break;

        case "*" :
            System.out.println(answer3);
            break;

        case "x" :
            System.out.println(answer3);
            break;

        case "/" :
            System.out.println(answer4);
            break;

        case "%" :
            System.out.println(answer5);
            break;

        case "^" :
            System.out.println(answer6);
            break;

        default :
            System.out.println("Your operation was not recongized.");

            }
        }
    }
}

I'm learning Java too!

Looking at this the only thing I might suggest is that you only do the calculations that you need to. You could then remove all answer variables and print the result of the calculations directly, saving time and memory, albeit not much.

In terms of presentation, I personally would use lowerCamelCase for variable naming, e.g. firstNum, and I would also indent all of the cases as they are within the switch block. Finally, for input. nextDouble(); I'd make sure there was no space there.

Despite what I say for the presentation, that part of it is totally up to you, so do what you prefer, as long as it looks clear!


Adding qty If duplicates keys found VBA Would like to have a code review so I can improve my knowledge and expertise in VBA more specifically with Dictionaries as this is the first time I used one.

For context I have duplicate data and need to consolidate creating a unique list. To achieve this I used a Dictionary concatenating 2 fields to make a unique key. Also I have a function that uses Dictionaries to get a unique key so I can find the number of occurrence for each order number and add a suffix at the end if more than 999 rows used for the same order.

Option Explicit
Sub UniqueListWithSumQty()

Dim wsSource As Worksheet
Dim ws As Worksheet
Dim sh As Worksheet
Dim strCell As String
Dim i As Long, j As Long
Dim LRow As Long
Dim dict As Object

Set ws = ActiveSheet
Set dict = CreateObject("Scripting.Dictionary")

With ws
    LRow = FindLastRow(ActiveSheet, 3, 21)
    .Range("C17:U" & LRow).Sort key1:=.Range("C17:C" & LRow), order1:=xlAscending, Header:=xlNo
    wsSource = .Range("C17:U" & LRow).value
End With
With dict
    For i = 1 To UBound(wsSource, 1)
        strCell = Join(Array(wsSource(i, 1), wsSource(i, 3)), "|")
        If Not .Exists(strCell) Then
            .item(strCell) = .Count + 1
            For j = 1 To UBound(wsSource, 2)
                wsSource(.Count, j) = wsSource(i, j)
            Next j
        Else
            wsSource(.item(strCell), 4) = wsSource(.item(strCell), 4) + wsSource(i, 4)
        End If
    Next i
    i = .Count
End With

Set sh = ActiveSheet
sh.Range("C17:U" & LRow).Clear

With sh.Range("C17")
    .Resize(i, UBound(wsSource, 2)) = wsSource
End With
Call CheckOccurance(Range("C17:C" & LRow), LRow, sh)
End Sub

Private Function CheckOccurance(rng As Range, LRow As Long, ws As Worksheet)
    Dim maxOrdNoRow As Double
    Dim timesOccured As Double
    Dim occur As Double
    Dim uniqueList As Object
    Dim i As Long
    Dim data As Variant
    Dim x As Long
    Dim key As Variant
    Dim lastUsedRow As Long

    Set uniqueList = CreateObject("Scripting.Dictionary")
    data = rng.value

    For i = 1 To UBound(data, 1)
         uniqueList(data(i, 1)) = Empty
    Next i

    For Each key In uniqueList
        maxOrdNoRow = Application.WorksheetFunction.CountIf(Range("C17:C" & LRow), "=" & key)
        timesOccured = maxOrdNoRow / 999

        lastUsedRow = ws.Range("C:C").Find(what:=key, after:=ws.Range("C16")).Row
        If timesOccured > 1 Then
            For x = 1 To WorksheetFunction.RoundUp(timesOccured, 0)
                If timesOccured > x Then
                    occur = timesOccured - (timesOccured - x)
                Else
                    occur = timesOccured
                End If
                ws.Range(Cells(lastUsedRow, 3), Cells((occur * 999) + 16, 3)).value = key & "-" & x
                lastUsedRow = (occur * 999) + 17
            Next x
        Else
            'do nothing
        End If
    Next key
End Function

Unfortunately the exercise is a bit abstract to me, because I cannot visualize the data you are manipulating. A data sample would have been welcome to better comprehend your purpose.

A few comments anyway

I don't see the code for FindLastRow
You have multiple references to C17, so it should be defined as a constant variable. It could be named START_CELL or something. Using uppercase is common practice.
999: another magic number that also appears multiple times. It should be made a constant too, MAX_ITERATIONS or something. Or it could be passed as a parameter to CheckOccurance (you already have 3). The aim is to avoid repetition, and leave room for flexibility (because you might want to change that value at some point).
Warning: CheckOccurance is a typo
You also have more 'arbitrary' values like 3, 4... I suppose they are offsets. Again it would better to use variables instead because they are more descriptive, this will make your code more readable, easier to understand and will reduce the risk of errors, especially if you shift rows/columns later. And here, doing a search & replace on 3 or 4 is not an option.
This line has 3 hardcoded values: ws.Range(Cells(lastUsedRow, 3), Cells((occur * 999) + 16, 3)).value = key & "-" & x. And x is not the most meaningful name for a variable.
Rather than using ranges like "C17:C" & LRow with hardcoded references it would be better to use named ranges. They are more descriptive and more flexible. Like variables, you define them once, and reuse them multiple times. Why You Should Be Using Named Ranges in Excel
Using ActiveSheet is tricky, because the context could change, for example your macro could switch to another sheet or workbook, and ruin your assumptions. It would be safer to retrieve a named reference (from ActiveSheet.Name) to a variable and then use that variable. If you have no sheet selected, this property returns Nothing.
If you are concatenating just two cells maybe using an array and JOINing is a bit overblown: strCell = Join(Array(wsSource(i, 1), wsSource(i, 3)), "|"). But the best is to run a benchmark and test performance using different approaches. Don't be afraid to experiment. Good code is code that is efficient while remaining intelligible.
I don't understand why you declare your variable like this: Dim wsSource As Worksheet and then use it like this: wsSource = .Range("C17:U" & LRow).value. Did you mean Range ?
The choice of data type for your variables is not always optimal: several variables are of type Double. From the doc: "Double (double-precision floating-point) variables are stored as IEEE 64-bit (8-byte) floating-point numbers ranging in value from: -1.79769313486231E308 to -4.94065645841247E-324 for negative values and 4.94065645841247E-324 to 1.79769313486232E308 for positive values". Performance matters when you are doing arithmetic operations in loops.
Even Long is more than enough: "Long (long integer) variables are stored as signed 32-bit (4-byte) numbers ranging in value from -2,147,483,648 to 2,147,483,647."
FYI: Total number of rows and columns on a worksheet: 1,048,576 rows by 16,384 columns (source: Excel specifications and limits). So when working with columns, Integer is sufficient: "Integer variables are stored as 16-bit (2-byte) numbers ranging in value from -32,768 to 32,767".
But but but surprise: "In recent versions, however, VBA converts all integer values to type Long, even if they're declared as type Integer. So there's no longer a performance advantage to using Integer variables; in fact, Long variables may be slightly faster because VBA does not have to convert them." (Source: The Integer Data Types).

Code structure

Overall, tabulation is OK except UniqueListWithSumQty
Usage of With where appropriate
Some line spacing would be desirable, calls to external methods eg. Call CheckOccurance should be highlighted and not drowned with the rest of the code because it is important to be able to follow the processing stream.
One problem: lack of comments. There are important for you too, because in a few weeks or months, you will have lost your train of thought and you will have to re-analyze your own code. It is also very helpful to put some data sample in comments when you extract or compute data, to have an idea of what the values look like. For example, when you extract rows from a range, copy-paste a row or a small portion of your range and add it to comment.
It is important not just to document the individual steps, but also the general logic of your code, for this you add a few lines of comments at the top of your module.

The code is quite short, but unfortunately it's not as easy to understand as it should be, because there are too many hardcoded values, and the underlying data is not known.

Strategy

Obviously this is what you are interested in and this is where I am going to be the least helpful, because I lack some insight.

First of all, you did not give a clear definition of what qualifies as duplicate data. You've mentioned order numbers but that's the only thing we know. So I am assuming that you are really looking for duplicate order numbers but maybe that's not the whole story. You mention 'concatenating 2 fields' but what are they ?

Maybe all of this was not even necessary, because Excel now has built-in tools to find duplicate data: How to Find Duplicates in Excel. But then, it this was not sufficient for your purpose it should have been made clear and maybe we could propose a better approach.

A COUNTIF might suffice. You even use it in your code. Maybe there is a reason but what was it, I am wondering.
Dictionaries are certainly useful, but in the present case ?


PHP function to compare two Roman numerals This was for an assignment. I had to write a function to compare two roman numerals. This had to return true only if the second number was larger than the first and had to be done without converting the letters into integers. Also prefix subtraction didn't apply so all the letters came in descending order.

Here is what I came up with:

<?php

function x_smaller_than_y($x_num,$y_num) {

//Create array with ordered list of roman numerals.
$evaluation= array("M","D","C","L","X","V","I");

//Splits submitted numbers into arrays
$x_num_arr = str_split($x_num);
$y_num_arr = str_split($y_num);

//Loops through each roman numeral in the given list
for ($evaluate = 0; $evaluate<7;$evaluate++)
  {

     //For every letter in the numerals' list loops through
     //every letter in the given numbers looking for a match.
     $counter = 0;
     while ($counter < ((strlen($x_num)))&& $counter < ((strlen($y_num))))
       {

       //If in any specific position x=y, it simply ignores it and moves forward.
       if ($x_num_arr[$counter]==$y_num_arr[$counter])
           {$counter++;}

       //If in any given position the letter in Y is higher in the list than
       //the letter in X then Y is larger than X and the function is true.
       //"break" ends the while loop and "$evaluate=8" ends the for loop.
       elseif(($x_num_arr[$counter]!=$evaluation[$evaluate])&&($y_num_arr[$counter]==$evaluation[$evaluate]))
           {return true;
            $evaluate=8;
            break;}

       //Same as above but this time the letter in X is before the letter
       //in Y so the function is false.
       elseif(($x_num_arr[$counter]==$evaluation[$evaluate])&&( $y_num_arr[$counter]!=$evaluation[$evaluate]))
           {return false;
            $evaluate=8;
            break;}

       //If none of the above cases apply simply move to the next letter.
       else {$counter++;}
        }
    }

//Since the number of iterations in the while loop is determined by the
//shorter number there can still be a case in which 2 numbers are exactly
//the same up to a point but one is simply longer than the other,
//eg. MXVI and MXVII.
//When this happens the for loop will have run through its natural
//course, $evaluate will be equal to 7, and the code below will determine
//the larger number by looking at their lenght.
if ($evaluate<8&&(strlen($x_num) < strlen($y_num)))
{return true;}

elseif ($evaluate<8&&(strlen($x_num) > strlen($y_num)))
{return false;}

}

How could have I made it better?

It gives wrong results for 9 (IX) and 5 (V).

Some notes about the code:

In the following block the $evaluate = 8; and the break; statements are unnecessary, they never run because of the return statement.

{
    return true;
    $evaluate = 8;
    break;
}

Instead of commenting name the variable to what the comment says.

//Create array with ordered list of roman numerals.
$evaluation= array("M","D","C","L","X","V","I");

It could be $numerals or $numerals_in_order, for example.
7 and 8 are a magic numbers. They should be named constants or computed values. Do they come from the size of the array? If they do make it clear, it would improve readability.
while ($counter < ((strlen($x_num)))&& $counter < ((strlen($y_num))))

This could be simplified to the following:

$size = min(strlen($x_num), strlen($y_num));
while ($counter < $size)

Instead of big if-elseif structures you could use continue and simple ifs since some of them returns the function if the condition is true, so the remaining code won't run. The last else also would be unnecessary.

$size_min = min(strlen($x_num), strlen($y_num));
while ($counter < $size_min) {
    if ($x_num_arr[$counter] == $y_num_arr[$counter]) {
        $counter++;
        continue;
    }

    if(($x_num_arr[$counter]!=$evaluation[$evaluate])&&($y_num_arr[$counter]==$evaluation[$evaluate])) {
        return true;
    }

    if(($x_num_arr[$counter]==$evaluation[$evaluate])&&( $y_num_arr[$counter]!=$evaluation[$evaluate])) {
        return false;
    }

    $counter++;
}

Inside the for loop the $evaluation array is always indexed with $evaluate. You should create a local variable for this.
The same is true for $y_num_arr[$counter] and $x_num_arr[$counter] inside the while loop. (They could be $current_x and $current_y).
It might not be an issue (depending on the specification) but the function currently accepts invalid Roman numerals too (like IIX).
At the end of the function it returns with implicit NULL if both conditions are false in the following snippet:

if ($evaluate<8&&(strlen($x_num) < strlen($y_num)))
{return true;}

elseif ($evaluate<8&&(strlen($x_num) > strlen($y_num)))
{return false;}

You should make this explicit (add a return false as the last statement) or use an assert call if it should never happen.
You could easily write some test code to check that the comparison is fine for every possible Roman numeral.

for ($i = 1; $i <= $max; $i++) {
    for ($j = 1; $j <= $max; $j++) {
        $i_r = numberToRoman($i);
        $j_r = numberToRoman($j);
        $smaller = x_smaller_than_y($i_r, $j_r);
        if ($i >= $j && $smaller) {
            echo "error: $i, $j, $i_r, $j_r\n";
        }
    }
}

If I'm right $max should be 3888 and it finishes in a few minutes. You can find a numberToRoman function here, for example..


Validating multiple parameters by regex and length I have a set of validation functions which return a response code along with an error message if the validation fails. Requests can be of three types, and each of them contains a different type of values to be validated. My question is what are the options to avoid the repetitive code, particularly in the message assignment?

const Validation = (function() {

  const isParameterOneLengthValid = (data) => {
    return data.length >= 5 && data.length <= 10;
  };

  const isParameterOneValid = (data) => {
    return !(/[^A-Za-z\-0-9\,\.]/.test(data));
  };

  const isParameterTwoLengthValid = (data) => {
    return data.length >= 1 && data.length <= 5;
  };

  const isParameterTwoValid = (data) => {
    return !(/[^0-9]/.test(data));
  };

  function Validation() {}

  Validation.prototype.validateFirstTypeRequest= (data) => {
      if (!isParameterOneLengthValid(data.parameterOne)) {
        data.code = 0;
        data.message = 'Length of the parameter one is invalid';
        return data;
      } else if (!isParameterOneValid(data.parameterOne)) {
        data.resStatus = 1;
        data.resMessage = 'Parameter one is invalid';
        return data;
      }

      if (!isParameterTwoLengthValid(data.parameterTwo)) {
        data.code = 3;
        data.message = 'Length of the parameter two is invalid';
        return data;
      } else if (!isParameterTwoValid(data.parameterTwo)) {
        data.resStatus = 4;
        data.resMessage = 'Parameter two is invalid';
        return data;
      }

      return data;
  };

  Validation.prototype.validateSecondTypeRequest= (data) => {
      if (!isParameterOneLengthValid(data.parameterOne)) {
        data.code = 0;
        data.message = 'Length of the parameter one is invalid';
        return data;
      } else if (!isParameterOneValid(data.parameterOne)) {
        data.resStatus = 1;
        data.resMessage = 'Parameter one is invalid';
        return data;
      }

      return data;
  };

  Validation.prototype.validateThirdTypeRequest= (data) => {
      if (!isParameterTwoLengthValid(data.parameterTwo)) {
        data.code = 3;
        data.message = 'Length of the parameter two is invalid';
        return data;
      } else if (!isParameterTwoValid(data.parameterTwo)) {
        data.resStatus = 4;
        data.resMessage = 'Parameter two is invalid';
        return data;
      }

      return data;
  };

  module.exports = new Validation();

  return Validation;
})();

Mutating State

I really don't like that your validation functions modify the data object that they are given. It seems like an unnecessary mutation that's mixing data with error codes. Do you have any particular reason for returning the data object? If it's passed into the function, then wouldn't the user already has a pointer to it? My reasoning is that you could instead return an failed validation response object instead of mutating the data object. Some problems with mutating the data object are immediately clear; now you can't have a property in data called code, message, resStatus, or resMessage.

Singe Exit Point

I'm not sure if this came from sanitizing the code, but the returns in the validateResponse functions are a bit redundant. One programming platitude is to strive to give functions one exit point. Currently, your validateSecondTypeRequest function has three exit points:

Validation.prototype.validateSecondTypeRequest= (data) => {
  if (!isParameterOneLengthValid(data.parameterOne)) {
    data.code = 0;
    data.message = 'Length of the parameter one is invalid';
    return data; // these returns could be merged
  } else if (!isParameterOneValid(data.parameterOne)) {
    data.resStatus = 1;
    data.resMessage = 'Parameter one is invalid';
    return data; // these returns could be merged
  }

  return data; // these returns could be merged
};

The returns in the if else block are unnecessary and could be easily eliminated to:

Validation.prototype.validateSecondTypeRequest= (data) => {
  if (!isParameterOneLengthValid(data.parameterOne)) {
    data.code = 0;
    data.message = 'Length of the parameter one is invalid';
  } else if (!isParameterOneValid(data.parameterOne)) {
    data.resStatus = 1;
    data.resMessage = 'Parameter one is invalid';
  }

  return data;
};

Eliminating Duplicate Code

Write a function for each parameter:

function validateParameterOne(value) {
  let result;

  if (value.length >= 5 && value.length <= 10) {
    result = {
      code: 0,
      message: 'Length of the parameter one is invalid'
    };
  };
  else if (!(/[^A-Za-z\-0-9\,\.]/.test(data))) {
    result = {
      resStatus: 1,
      resMessage: 'Parameter one is invalid'
    };
  }

  return result;
}

Not only do you get rid of the duplication, but now you have fewer functions; all your code for validating parameterOne is in contained in one spot. Also, now you can write your validateTypeRequest functions like so:

Validation.prototype.validateFirstTypeRequest= (data) => {
  let result;

  if (!result) {
    result = validateParameterOne(data.parameterOne);
  }

  if (!result) {
    result = validateParameterTwo(data.parameterTwo);
  }

  // If you HAVE to return and mutate data
  //if (result) {
  //  return Object.assign(data, result);
  //}

  return result;
}


Maintaining users in a Dovecut user database I learn BASH scripting occasionally from time to time, now I am working on a simple bash script that serves to maintain Dovecot user database. The general purpose of the script is able to list, add or delete user entries in the specified file. Please add some comments about the code - common mistake and etc. I want to learn more about code simplicity and avoiding bad habits.

#!/bin/bash

USERDB="/etc/dovecot/users"
MAILBOXMAP="/etc/postfix/virtual/virtual-mailbox-maps.cf"

## Add user function

add_user () {

# Reading username

while [[ -z "$USERNAME" ]]; do
    echo -n "Please enter username: "
    read "USERNAME"
done

# Reading domain name

while [[ -z "$DOMAIN" ]]; do
    echo -n "Please enter domain name: "
    read DOMAIN
done

# Reading password and generating hash with doveadm pw

while [[ -z "$PASSWORD" ]]; do
    echo -n "Please enter user password: "
    read "PASSWORD"
done

if [[ -n "$PASSWORD" ]]; then
    USERPASS="$(doveadm pw -p $PASSWORD -s SHA512-CRYPT)"
fi

# Adding provided credentials to Dovecot userdb

echo "Adding user credentials to userdb..."

echo "$USERNAME@$DOMAIN:$USERPASS:::" >> "$USERDB"

# Adding provided user@domain to postfix mailbox map

echo "Adding user credentials to postfix map..."

echo "$USERNAME@$DOMAIN $DOMAIN/$USERNAME" >> "$MAILBOXMAP"

# Hashing the map

if [[ -f "$MAILBOXMAP" ]]; then
     postmap hash://"$MAILBOXMAP"
     echo "Hashing map is done!"
else
     echo "Postfix map not exist or have been moved to another directory!"
     exit 1
fi
exit 0
}

## List all users function

list_users () {

if [[ -f "$USERDB" ]]; then
    awk -F: '/./'' {print $1} ' "$USERDB"
else
    echo "User database not exist or have been moved to another directory!"
    exit 1
fi
}

## Delete user function

delete_user () {
if [[ -n "$DELUSER" ]]; then
    sed -i "/^$DELUSER$/d" $USERDB $MAILBOXMAP && echo "User account have been  deleted!"
else
    echo "User not found!"
    exit 1
fi
}

case "$1" in
 add|-a)
    add_user
    ;;
 list|-l)
    list_users
    ;;
 delete|-d)
    shift
    deluser="$1"
    delete_user
    ;;
 *)
    echo "Unknown command"
    ;;
esac

In this code:

while [[ -z "$USERNAME" ]]; do
    echo -n "Please enter username: "
    read "USERNAME"
done

What if the variable USERNAME is not empty when this code is reached?
For example, the environment might be dirty by misbehaving scripts,
and USERNAME might already be set to garbage, and the script will not prompt the user.

echo -n is not recommended because it's not portable. Use printf instead.

There's no need to quote USERNAME when reading into it.

The equivalent code, safer and better:

USERNAME=
while [[ -z "$USERNAME" ]]; do
    printf "Please enter username: "
    read USERNAME
done

Do similarly for the other loops too.

This will fail if PASSWORD contains spaces:

USERPASS="$(doveadm pw -p $PASSWORD -s SHA512-CRYPT)"

To make it safe, quote it:

USERPASS="$(doveadm pw -p "$PASSWORD" -s SHA512-CRYPT)"

It would be better to indent the function bodies. For example:

list_users () {
    if [[ -f "$USERDB" ]]; then
        awk -F: '/./'' {print $1} ' "$USERDB"
    else
        echo "User database not exist or have been moved to another directory!"
        exit 1
    fi
}

This is especially helpful in the case of longer functions.

The quotes are strange and confusing in this awk:

        awk -F: '/./'' {print $1} ' "$USERDB"

It would be better simply this way:

        awk -F: '/./ {print $1}' "$USERDB"


Project Euler - Problem 1 Multiples of 3 and 5 (Java) I consider myself a beginner at Java but this question was an easy one.  My only concern is if there is a more efficient way to write this program. I'm very open minded.  I don't care how advanced your answer is, I'm willing to learn.

public static void main(String[] args) {

    long limit = 1000;

    long sum = sumOfMultiples(3,limit) + sumOfMultiples(5,limit) - sumOfMultiples(15,limit);

}

public static long sumOfMultiples(long number, long limit){
    long sum = 0;
    long add = number;

    for(; add < limit; add+=number){
        sum += add;
    }

    return sum;
}

You used the inclusion-exclusion principle here…

sumOfMultiples(3,limit) + sumOfMultiples(5,limit) - sumOfMultiples(15,limit);

… which is good.  The name of the function sumOfMultiples(…) is pretty clear.  (The only uncertainty is whether it includes or excludes the upper bound.  JavaDoc would help in that regard.)

There is a way to compute sumOfMultiples() in constant time.  Since

$$ 1 + 2 + 3 + \ldots + n = \frac{n(n+1)}{2}$$

you can derive a formula for

$$ k + 2k + 3k + \ldots + nk = k \frac{n(n+1)}{2} $$


Creating a count inversion array of an unsorted array of unique integers Given an array:

arr = [2,3,1,4]

I could write a count inversion array such that counting all numbers n2 after a certain number n1 in arr such that n1 > n2 and write it like this

[1 1 0 0]

Similarly, the inversion array of:

[2, 1, 4, 3]

would be:

[1, 0, 1, 0]

For:

[20]
[1, 2, 3, 4, 5, 6]
[87, 78, 16, 94]

Output would be:

0
0 0 0 0 0 0
2 1 0 0

Constraints:

\$1 \le N \le 10^4\$
\$1 \le i \le 10^6\$

The code that I wrote works for most cases but takes >10sec for an extra large number of test cases.

from copy import copy

def merge(arr, left_lo, left_hi, right_lo, right_hi, dct):
    startL = left_lo
    startR = right_lo
    N = left_hi-left_lo + 1 + right_hi - right_lo + 1
    aux = [0] * N
    res = []
    for i in xrange(N):

        if startL > left_hi:
            aux[i] = arr[startR]
            startR += 1
        elif startR > right_hi:
            aux[i] = arr[startL]
            startL += 1
        elif arr[startL] <= arr[startR]:
            aux[i] = arr[startL]
            startL += 1
            # print aux
        else:
            aux[i] = arr[startR]
            res.append(startL)
            startR += 1
            # print aux

    for index in res:
        for x in xrange(index, left_hi+1):
            dct[arr[x]] += 1

    for i in xrange(left_lo, right_hi+1):
        arr[i] = aux[i - left_lo]
    return

def merge_sort(arr, lo, hi, dct):
    mid = (lo+hi)/2
    if lo<=mid<hi:
        merge_sort(arr, lo, mid, dct)
        merge_sort(arr, mid+1, hi, dct)
        merge(arr, lo, mid, mid+1, hi, dct)
    return

def count_inversion(arr, N):
    lo = 0
    hi = N-1
    dct = {i:0 for i in arr}
    arr2 = copy(arr)
    merge_sort(arr, lo, hi, dct)
    return ' '.join([str(dct[num]) for num in arr2])

count_inversion calls merge_sort and that's where the total number of LEFT > RIGHT inversions are incremented. All numbers are stored in a dictionary with counts such that whenever L > R occurs all numbers in the left array starting from L to end of Left array are incremented by 1.

Now I understand there could be a way to optimize this snippet:

for index in res:
            for x in xrange(index, left_hi+1):
                dct[arr[x]] += 1

Thanks for an interesting, well-posed question.  Before we get to issues of performance, let me make some other suggestions for your code.

The way you have broken down the code into three functions is reasonable and logical.  Nice!
The functions you wrote don't have docstrings, so it is hard to know how to use them.  I couldn't get your count_inversion() function to run initially, for example, because I didn't know what N was supposed to be.  Adding docstrings would make this clear.
For the specific case of the N parameter in count_inversion, why do you need it?  When I used the function I did it like this:

arr = [2, 3, 1, 4]
arr2 = [2, 1, 4, 3]
arr3 = [20]
arr4 = [1, 2, 3, 4, 5, 6]
arr5 = [87, 78, 16, 94]

arrs_to_test = [arr, arr2, arr3, arr4, arr5]

[count_inversion(test, len(test)) for test in arrs_to_test]

That to me suggests that you don't need N as a parameter, instead just do something like:

def count_inversion(arr):
    # docstring goes here
    N = len(arr)
    # <<rest of code>>

I usually dislike when people are sticklers for PEP8 variable-naming conventions in mathematically oriented code, but I think your naming could use some work.  For example I had to read dct = {i:0 for i in arr} multiple times to understand that i was not an index but the data.  So dct = {el:0 for el in arr} would have been more natural to me.  Plus dct isn't the best name either.  If I understand the code correctly, perhaps result would be better?
Possible bug: Related to the above, do you really want to create a dictionary keyed on the elements of arr?  Doing so means that the behavior when integers are repeated in the input is probably not what you want:

>>> repeated = [3, 2, 1, 0, 3, 4, 5]
>>> count_inversion(repeated, len(repeated))
 '3 2 1 0 3 0 0'

Is the fifth element in this array really supposed to be "3"?
Right now the code functions because of the mutability of dct.  I'm not 100% sure it could be done cleanly, but if possible I'd suggest re-writing merge_sort and merge to return dct instead of returing None.  That way the initialization of dct could happen in those functions too, which feels more natural to me.
You are using your (mysteriously named) dct variable in a way that looks like a Counter, so you might consider using this built-in datatype of Python.
Now, to issues of performance.  In Python, line-profiler is an easy to use package for assessing the performance of your code.  I use this package in a Jupyter notebook like this:

from random import randint
%load_ext line_profiler

big_test = [randint(0, 10000) for _ in range(1000)]
%lprun -f count_inversion -f merge_sort -f merge count_inversion(big_test, len(big_test))

The output of this operation is at the end of my answer.  It shows that you are right, and the slowest part of the operation is indeed the nested for loops in merge().
Dictionaries, unlike lists, can't be mutated using slice notation.  The only reason you need the for x in xrange(index, left_hi+1): nested loop is because you can't change whole swaths of the dictionary at once.  With lists, you can.  Thus, if you agree that the possible bug I described above is in fact a bug, you can switch to storing the output values in a list instead of a dictionary, and get rid of the nested loop.  The speedup is very small for short input arrays but grows with array size.  On my machine it led to a ~2-10× improvement at input arrays of 10,000 elements.  I'll put the output of my line profiling below too.
Using numpy for applications like this makes sense because python doesn't have a mutable fixed-type data structure.  Interesting, using numpy only to represent the dct variable in your original code, while leaving in place all other for loops, speeds up the execution another 2× or so for lists of 10,000 elements.

for index in res:
        sublist_length = left_hi+1 - index
        out[index:left_hi+1] += np.ones(sublist_length, dtype = int)

Original code timing

from random import randint
big_test = [randint(0, 100) for _ in range(10000)]
%lprun -f merge count_inversion(copy(big_test), len(big_test))

Results in:

Timer unit: 1e-06 s

Total time: 26.9413 s
File: <ipython-input-1-77a541281305>
Function: merge at line 4

Line #      Hits         Time  Per Hit   % Time  Line Contents
==============================================================
     4                                           def merge(arr, left_lo, left_hi, right_lo, right_hi, dct):
     5      9999         4407      0.4      0.0      startL = left_lo
     6      9999         4315      0.4      0.0      startR = right_lo
     7      9999         5812      0.6      0.0      N = left_hi-left_lo + 1 + right_hi - right_lo + 1
     8      9999         8330      0.8      0.0      aux = [0] * N
     9      9999         4401      0.4      0.0      res = []
    10    143615        72509      0.5      0.3      for i in range(N):
    11
    12    133616        60250      0.5      0.2          if startL > left_hi:
    13      5778         2956      0.5      0.0              aux[i] = arr[startR]
    14      5778         2700      0.5      0.0              startR += 1
    15    127838        57380      0.4      0.2          elif startR > right_hi:
    16      7503         3822      0.5      0.0              aux[i] = arr[startL]
    17      7503         3507      0.5      0.0              startL += 1
    18    120335        64095      0.5      0.2          elif arr[startL] <= arr[startR]:
    19     61505        31935      0.5      0.1              aux[i] = arr[startL]
    20     61505        28853      0.5      0.1              startL += 1
    21                                                       # print aux
    22                                                   else:
    23     58830        30780      0.5      0.1              aux[i] = arr[startR]
    24     58830        34221      0.6      0.1              res.append(startL)
    25     58830        28077      0.5      0.1              startR += 1
    26                                                       # print aux
    27
    28     68829        35529      0.5      0.1      for index in res:
    29  24750922     11838525      0.5     43.9          for x in range(index, left_hi+1):
    30  24692092     14464132      0.6     53.7              dct[arr[x]] += 1
    31
    32    143615        76619      0.5      0.3      for i in range(left_lo, right_hi+1):
    33    133616        73794      0.6      0.3          arr[i] = aux[i - left_lo]
    34      9999         4362      0.4      0.0      return

Improved Code Timing (list slicing)

Timer unit: 1e-06 s

Total time: 3.11468 s
File: <ipython-input-2-224c772db490>
Function: new_merge at line 4

Line #      Hits         Time  Per Hit   % Time  Line Contents
==============================================================
     4                                           def new_merge(arr, left_lo, left_hi, right_lo, right_hi, out):
     5                                               # docstring goes here
     6      9999         4566      0.5      0.1      startL = left_lo
     7      9999         4491      0.4      0.1      startR = right_lo
     8      9999         6601      0.7      0.2      N = left_hi-left_lo + 1 + right_hi - right_lo + 1
     9      9999         7904      0.8      0.3      aux = [0] * N
    10      9999         4571      0.5      0.1      res = []
    11    143615        69607      0.5      2.2      for i in xrange(N):
    12
    13    133616        62294      0.5      2.0          if startL > left_hi:
    14      5778         3044      0.5      0.1              aux[i] = arr[startR]
    15      5778         2804      0.5      0.1              startR += 1
    16    127838        59349      0.5      1.9          elif startR > right_hi:
    17      7503         3945      0.5      0.1              aux[i] = arr[startL]
    18      7503         3673      0.5      0.1              startL += 1
    19    120335        67720      0.6      2.2          elif arr[startL] <= arr[startR]:
    20     61505        33280      0.5      1.1              aux[i] = arr[startL]
    21     61505        30419      0.5      1.0              startL += 1
    22                                                       # print aux
    23                                                   else:
    24     58830        31818      0.5      1.0              aux[i] = arr[startR]
    25     58830        34578      0.6      1.1              res.append(startL)
    26     58830        28988      0.5      0.9              startR += 1
    27                                                       # print aux
    28
    29     68829        37996      0.6      1.2      for index in res:
    30     58830        31690      0.5      1.0          sublist_length = left_hi+1 - index
    31     58830       164834      2.8      5.3          ones = [1]*sublist_length
    32     58830      2264227     38.5     72.7          out[index:left_hi+1] = map(add, out[index:left_hi+1], ones)
    33
    34    143615        73228      0.5      2.4      for i in xrange(left_lo, right_hi+1):
    35    133616        78612      0.6      2.5          arr[i] = aux[i - left_lo]
    36      9999         4440      0.4      0.1      return

Improved Code Timing (numpy)

Timer unit: 1e-06 s

Total time: 0.979072 s
File: <ipython-input-33-e2db83e49c93>
Function: d_merge at line 4

Line #      Hits         Time  Per Hit   % Time  Line Contents
==============================================================
     4                                           def d_merge(arr, left_lo, left_hi, right_lo, right_hi, out):
     5                                               # docstring goes here
     6      9999         4439      0.4      0.5      startL = left_lo
     7      9999         4360      0.4      0.4      startR = right_lo
     8      9999         5728      0.6      0.6      N = left_hi-left_lo + 1 + right_hi - right_lo + 1
     9      9999         8283      0.8      0.8      aux = [0] * N
    10      9999         4511      0.5      0.5      res = []
    11    143615        69717      0.5      7.1      for i in xrange(N):
    12
    13    133616        62436      0.5      6.4          if startL > left_hi:
    14      5778         2955      0.5      0.3              aux[i] = arr[startR]
    15      5778         2779      0.5      0.3              startR += 1
    16    127838        59843      0.5      6.1          elif startR > right_hi:
    17      7503         3916      0.5      0.4              aux[i] = arr[startL]
    18      7503         3645      0.5      0.4              startL += 1
    19    120335        66024      0.5      6.7          elif arr[startL] <= arr[startR]:
    20     61505        32834      0.5      3.4              aux[i] = arr[startL]
    21     61505        29695      0.5      3.0              startL += 1
    22                                                       # print aux
    23                                                   else:
    24     58830        31913      0.5      3.3              aux[i] = arr[startR]
    25     58830        34546      0.6      3.5              res.append(startL)
    26     58830        28552      0.5      2.9              startR += 1
    27                                                       # print aux
    28
    29     68829        36887      0.5      3.8      for index in res:
    30     58830        31918      0.5      3.3              sublist_length = left_hi+1 - index
    31     58830       303818      5.2     31.0              out[index:left_hi+1] += np.ones(sublist_length, dtype = int)
    32
    33    143615        72165      0.5      7.4      for i in xrange(left_lo, right_hi+1):
    34    133616        73672      0.6      7.5          arr[i] = aux[i - left_lo]
    35      9999         4436      0.4      0.5      return

Note:

All the code I used (both mine and the original) is in a Jupyter notebook hosted at GitHub.


Generic binary tree in C# This is a simple implementation of a generic binary tree that holds elements of type T. I was wondering if there was something that could be done better (especially in the EnumerateNodes methods).

using System.Collections.Generic;

namespace DataStructures.Node
{
    /// <summary>
    /// Implements a generic binary tree.
    /// </summary>
    /// <typeparam name="T">The type of the nodes</typeparam>
    public class Node<T>
    {
        #region Private attributes
        private T _node = default(T);
        private Node<T> _leftChild = default(Node<T>);
        private Node<T> _rightChild = default(Node<T>);
        #endregion

        #region Constructor
        public Node()
        {
            _node = default(T);
            _leftChild = null;
            _rightChild = null;
        }

        public Node(T node)
        {
            _node = node;
            _leftChild = null;
            _rightChild = null;
        }
        #endregion

        #region Accessors
        public T Value
        {
            get { return _node; }
            set { _node = value; }
        }

        public Node<T> LeftChild
        {
            get { return _leftChild; }
            set { _leftChild = value; }
        }

        public Node<T> RightChild
        {
            get { return _rightChild; }
            set { _rightChild = value; }
        }

        /// <summary>
        /// Returns the total number of elements in the tree.
        /// </summary>
        public int Count
        {
            get { return count(); }
        }
        #endregion

        #region Private methods

        private int count()
        {
            if (_leftChild == null && _rightChild == null)
                return 1;
            if (_leftChild == null)
                return 1 + _rightChild.count();
            if (_rightChild == null)
                return 1 + _leftChild.count();
            else
                return 1 + _leftChild.count() + _rightChild.count();
        }

        #endregion

        #region Public methods

        /// <summary>
        /// Adds the element on the right branch of the node.
        /// </summary>
        /// <param name="rightBranch">Node to add on the right side of the node.</param>
        public void AddRight(Node<T> rightBranch)
        {
            _rightChild = rightBranch;
        }

        /// <summary>
        /// Adds the element on the left branch of the node.
        /// </summary>
        /// <param name="leftBranch">Node to add on the left side of the node.</param>
        public void AddLeft(Node<T> leftBranch)
        {
            _leftChild = leftBranch;
        }

        public IEnumerable<T> EnumerateNodes()
        {
            yield return _node;
            if (_leftChild != null)
                foreach (T child in _leftChild.EnumerateNodes())
                    yield return child;
            if (_rightChild != null)
                foreach (T child in _rightChild.EnumerateNodes())
                    yield return child;
        }

        #endregion
    }
}

Here's an alternative way to write count().  It's shorter and uses expressions instead of statements.  You can't do as much in an expression, so I use them instead of statements when possible because there's a lower chance of things going wrong.  Readability is more important than those things, though, so I recommend using what is most readable to you.

private int count()
{
    return 1 +
        (_leftChild != null ? _leftChild.count() : 0) +
        (_rightChild != null ? _rightChild.count() : 0);
}

Or if you want to use C# 6 null checking:

private int count()
{
    return 1 + (_leftChild?.count() ?? 0) + (_rightChild?.count() ?? 0);
}

Method names should be UpperCamelCase, so count() should be Count().  I'm guessing you want to access a child's Count property instead of the method because the method is private.


Poker hand evaluator using NumPy I'm trying to write a Poker hand evaluator using NumPy, because I find pure Python is pretty slow.

def eval_(*args):

    if len(args) == 5:
        hand = np.asarray([*args], dtype=np.int8)
    else:
        hand = np.asarray(args[0], dtype=np.int8)

    ranks = np.bincount(hand // 4, minlength = 13)
    count = np.array([np.where(ranks == count)[0] for count in range(5)])

    if len(count[4]):
        return 8, count[4][0], count[1][0]
    if len(count[3]) and len(count[2]):
        return 7, count[3][0], count[2][0]
    if len(count[3]):
        return 4, count[3][0], count[1][1], count[1][0]
    if len(count[2]) == 2:
        return 3, count[2][1], count[2][0], count[1][0]
    if len(count[2]):
        return 2, count[2][0], count[1][2], count[1][1], count[1][0]

    is_straight = count[1][0] + 4 == count[1][4]
    is_acestraight = count[1][4] - 9 == count[1][3]
    is_flush = 5 in np.bincount(hand % 4)
    if is_straight: return 5 + 4 * is_flush, count[1][4]
    if is_acestraight: return 5 + 4 * is_flush, 3

    return 1 + 5 * is_flush, count[1][4], count[1][3], count[1][2], count[1][1], count[1][0]

I'm not satisfied with my code. I don't like all kinds of count[1][2] calls, I should be able to just attach the whole count[1] array to my return value but I don't know how to do it. And I have trouble with np.where(). I don't think I should use list comprehension for different count values on this:

count = np.array([np.where(ranks == count)[0] for count in range(5)])

but I have no idea.

I find [interpreted] pure python is pretty slow.

Yes, that's true. And numpy operations can offer speed advantages, since its loops are coded in compiled C. But that's only significant when manipulating large arrays, and 5 seems like a pretty small number, too small to realize gains. (I think eval_() only evaluates a single hand at a time, it's not like you're passing in thousands of hands in a single call.)

Yes, you could return count[1] instead of count[1][2], but that's quite a different thing, presenting callers with a different API. Would you please state your concern in terms of what values the caller should receive, similar to a unit test?

The list comprehension you use seems pretty reasonable to me.

You have lots and lots of magic numbers that you might define identifiers for.

It wouldn't hurt to mention https://en.wikipedia.org/wiki/List_of_poker_hands in a comment, if that matches the rankings you're returning.


Manchester encoder/decoder I have written a Manchester Encoding encoder / decoder based on my misconception of how it works (whoops). My encoder accepts an array of ones and zeroes, and returns the 'manchester encoded' data (pretending there is a constant clock to overlay onto the data). I am reasonably new to C++, and would like to advance my knowledge and coding skill in it, hence why I coded this small application. I am looking for ways to improve my code, as well as ways to increase its efficiency.

Main.cpp

#include <iostream>
#include "Manchester.h"

int main()
{
    int data[] = { 1, 1, 0, 0 }; // Some unencoded data
    int* encoded = Manchester::encode(data, 4);
    int* decoded = Manchester::decode(encoded, 8);
    return 0;
}

Manchester.cpp

#include "Manchester.h"
#include <stdexcept>
#include <sstream>
#include <cstring>
#include <cstdlib>

#ifdef DEBUG
    #include <iostream>
#endif

int *Manchester::encode(int *data, int length)
{
    int *output = new int[length * 2];

    for (int i = 0; i < length; i++)
    {

        // Work out the array indexes to use
        int bid = i * 2;
        int nbid = bid + 1;

        // Get the data
        int real = data[i];

        int bit = 0;
        int nbit = 0;

        // Work out what it is
        switch (real)
        {
        case 0:
            bit = MANCHESTER_ZERO[0] - '0'; // Subtract 48 to work out the real value
            nbit = MANCHESTER_ZERO[1] - '0';
            break;
        case 1:
            bit = MANCHESTER_ONE[0] - '0'; // Subtract 48 to work out the real value
            nbit = MANCHESTER_ONE[1] - '0';
            break;
        }

        #ifdef DEBUG
            std::cout << "[encode] " << real << " [" << bit << nbit << "]" << std::endl;
        #endif

        output[bid] = bit;
        output[nbid] = nbit;
    }

    return output;
}

int *Manchester::decode(int *data, int length)
{
    if ((length % 2) != 0)
    {
        throw std::range_error("length is not a multiple of 2");
    }

    int *output = new int[length / 2];

    for (int i = 0; i < (length / 2); i++)
    {
        // Work out the array indexes to use
        int bid = i * 2;
        int nbid = bid + 1;

        // Get the data
        int bit = data[bid];
        int nbit = data[nbid];

        // Put the data into a stringstream for comparison
        std::stringstream con;
        con << bit << nbit;
        const char* sbit = con.str().c_str();

        int real = 0;

        // Compare the data and work out the value
        if (strcmp(sbit, MANCHESTER_ONE) == 0)
        {
            real = 1;
        } else if (strcmp(sbit, MANCHESTER_ZERO) == 0) {
            real = 0;
        }

        #ifdef DEBUG
            std::cout << "[decode] bit: " << bit << nbit << " [" << real << "]" << std::endl;
        #endif

        output[i] = real;
    }

    return output;
}

Manchester.h

#ifndef MANCHESTER_H
#define MANCHESTER_H

#define DEBUG

#define MANCHESTER_ONE "01"
#define MANCHESTER_ZERO "10"

class Manchester
{
public:
    static int *encode(int *data, int length);
    static int *decode(int *data, int length);
};

#endif

Manipulating bits

One of the areas where your code may be improved is the way it handles and compares bits. You are using char* strings to represent bits while you could have used unsigned values and bitwise operations to speed up most of your operations. Actually, you should even replace every int representing bits by unsigned since the representation of unsigned values is known and they are the tool to use when representing bits.

You can replace the MANCHESTER_ONE and MANCHESTER_ZERO definitions by:

#define MANCHESTER_ONE 1u
#define MANCHESTER_ZERO 2u

1u is 0b01 in binary and 2u is 0b10.
Then you can change the following bit of code in encode:

switch (real)
{
case 0:
    bit = MANCHESTER_ZERO[0] - '0'; // Subtract 48 to work out the real value
    nbit = MANCHESTER_ZERO[1] - '0';
    break;
case 1:
    bit = MANCHESTER_ONE[0] - '0'; // Subtract 48 to work out the real value
    nbit = MANCHESTER_ONE[1] - '0';
    break;
}

By the way, the switch is useless since your are only checking for two values. You can replace it by a regular if:

if (real == 0)
{
    bit = (MANCHESTER_ZERO >> 1) & 1;
    nbit = MANCHESTER_ZERO & 1;
}
else // real == 1
{
    bit = (MANCHESTER_ONE >> 1) & 1;
    nbit = MANCHESTER_ONE & 1;
}

Since we replaced the strings MANCHESTER_* by unsigned values, we use bitwise ANDs instead of array subtracts.
A gain in efficiency may be obtained by refactoring this piece of code:

// Put the data into a stringstream for comparison
std::stringstream con;
con << bit << nbit;
const char* sbit = con.str().c_str();

int real = 0;

// Compare the data and work out the value
if (strcmp(sbit, MANCHESTER_ONE) == 0)
{
    real = 1;
} else if (strcmp(sbit, MANCHESTER_ZERO) == 0) {
    real = 0;
}

std::stringstream is known to be slow since you have to convert your integer values into strings first before performing operations on them. In a for loop, it may be a big performance loss. Here is how you can refactor this bit of code:

// Put the bits in an unsigned
unsigned sbit = (bit << 1) | nbit;

// Check only for MANCHESTER_ONE,
// assuming that it will be equal
// to MANCHESTER_ZERO if not
int real = int(sbit == MANCHESTER_ONE);

As said in the comments, I make the assumption that once computed, sbit can only be equal to MANCHESTER_ONE or MANCHESTER_ZERO and nothing else.

General advice about C++

I cannot see any hint that you are using C++11, therefore I will review your code as C++03 code:

As said in the comments, allocating memory in a function and not deallocating it is not a good idea. You should simply use a std::vector to represent your arrays of numbers.
#define is to be avoided whenever possible in C++. Consider using const variables instead.
The standard way to check whether you are debugging is #ifndef NDEBUG (the double negative is kind of ugly, but it's the standard way).
In C++, it is useless to write return 0; at the end of main. If no return statement is given, the compiler will add an implicit return 0;.
If the MANCHESTER_* constants are not used anywhere else, you can define them as private static const variables of the class Machester.

Here is your code once refactored:

#include <iostream>
#include <stdexcept>
#include <vector>

class Manchester
{
public:

    static std::vector<unsigned> encode(const std::vector<unsigned>& data, unsigned length)
    {
        std::vector<unsigned> output(length * 2);

        for (unsigned i = 0; i < length; i++)
        {

            // Work out the array indexes to use
            unsigned bid = i * 2;
            unsigned nbid = bid + 1;

            // Get the data
            unsigned real = data[i];

            unsigned bit = 0;
            unsigned nbit = 0;

            // Work out what it is
            if (real == 0)
            {
                bit = (ZERO >> 1) & 1;
                nbit = ZERO & 1;
            }
            else // real == 1
            {
                bit = (ONE >> 1) & 1;
                nbit = ONE & 1;
            }

            #ifndef NDEBUG
                std::cout << "[encode] " << real << " [" << bit << nbit << "]" << std::endl;
            #endif

            output[bid] = bit;
            output[nbid] = nbit;
        }

        return output;
    }

    static std::vector<unsigned> decode(const std::vector<unsigned>& data, unsigned length)
    {
        if ((length % 2) != 0)
        {
            throw std::range_error("length is not a multiple of 2");
        }

        std::vector<unsigned> output(length / 2);

        for (unsigned i = 0; i < (length / 2); i++)
        {
            // Work out the array indexes to use
            unsigned bid = i * 2;
            unsigned nbid = bid + 1;

            // Get the data
            unsigned bit = data[bid];
            unsigned nbit = data[nbid];

            // Put the data into an unsigned int
            unsigned sbit = (bit << 1) | nbit;

            // Check only for MANCHESTER_ONE,
            // assuming that it will be equal
            // to MANCHESTER_ZERO if not
            unsigned real = unsigned(sbit == ONE);

            #ifndef NDEBUG
                std::cout << "[decode] bit: " << bit << nbit << " [" << real << "]" << std::endl;
            #endif

            output[i] = real;
        }
        return output;
    }

private:

    static const unsigned ONE;
    static const unsigned ZERO;
};

const unsigned Manchester::ONE = 1u;
const unsigned Manchester::ZERO = 2u;

int main()
{
    typedef std::vector<unsigned> vec_t;

     // Some unencoded data
    unsigned data[] = { 1u, 1u, 0u, 0u };
    // Initialize vector with array
    vec_t vec(data, data+sizeof(data) / sizeof(unsigned));

    vec_t encoded = Manchester::encode(vec, 4);
    vec_t decoded = Manchester::decode(encoded, 8);
}

You can find a live working version here at Coliru.


Ranking lines of a file according to number of occurrences I recently dived into Ruby, so I want to improve this code. Is this good enough?

def ranksystem
    logfile = IO.readlines("some_logfile.log")
    logfile.each do |value|
      value.gsub!(/[\n]+/, "")
    end
    logfile.delete("")

    logcount = Hash.new(0)

    logfile.each do |v|
      logcount[v] += 1
    end

    logarray = logcount.sort_by {|k,v| v}.reverse

    finalreturn = ""

    logarray.each_with_index do |value, index|
      finalreturn = finalreturn + "Rank #{index+1} : #{value[1]} (#{value[0]})\n"
    end

    finalreturn

end

This code reads each line in log file like this:

apple
orange
apple
apple
apple
apple
orange
orange
banana
banana

and converts like this:

Rank 1 : 4 (apple)
Rank 2 : 3 (orange)
Rank 3 : 2 (banana)

"Cool" enough? It seems fine, if procedural. Depends what you mean by cool.

I might combine a few steps. I'd also refactor and not put everything into one method, since each separate "chunk" does very distinct, and different, things. Separation makes testing easier.

The below isn't necessarily any better (and the chunk that creates the report may actually be worse--I'm not sure; it'd be less efficient), but it provides some alternate avenues to explore further.

def get_log_lines name
  logfile = IO.readlines(name)
  logfile.collect { |l| l.gsub(/\n+/, "")}.reject { |l| l == "" }
end

def ranksystem
  loglines = get_log_lines "some_logfile.log"
  logcount = Hash[loglines.group_by {|l| l}.collect {|k, v| [v.size, k]}]
  i = 0
  logcount.keys.sort.reverse.collect { |n| i += 1; "Rank #{i}: #{n} (#{logcount[n]})" }.join("\n")
end