Find perfect squares between two numbers The following problem is from codechef.com:

  Recently Chef become very much interested in perfect squares. We all
  know Chef and his weird interests. Anyways Chef will be soon writing
  his masters thesis on perfect squares revealing what-not-known
  properties of perfect squares. While doing his research, he happened
  to be confronted with some interesting perfect squares. These prefect
  squares consists only of digits which are themselves perfect squares.
  0, 1, 4 and 9 are such digits. These are called perfect digits.

  As we all know Chef also has habit of asking too many questions, he is
  asking- given two numbers a and b, how many perfect squares exists
  between these two numbers inclusive, that contains only perfect
  digits.

  Input:

  First line of input will contains T, number of test cases. Then T
  lines follows, each containing two positive integers a and b.

  Constraints:

  \$T \le 500\$
  \$1 \le a \le b \le 10000000000\$

  Output:

  For each input, output number of perfect digit squares between given
  numbers.

  Sample:

  Input:

2
1 10
100 10000
 Output:

3
9

How can I decrease the running time of my solution?

#include<stdio.h>
int main()
{
    int test;
    long long int num1,num2,start,stop,i,j,square,rem;
    scanf("%d",&test);
    while(test--)
    {
        long long int count=0;
        scanf("%lld%lld",&num1,&num2);
        start=sqrt(num1);
        stop=sqrt(num2);
        for(i=start;i<=stop;i++)
        {
            square = i*i;

            if(square<num1)
            {
                continue;
            }
            else
            {
                while(1)
                {

                    rem=square%10;
                    if(rem!=1 && rem!= 4 && rem!=9 && rem!=0 )
                    {
                        break;
                    }
                    if(square>=10)
                    {
                        square=square/10;

                    }
                    else
                    {
                        if(square!=1 && square!= 4 && square!=9 &&square!=0 )
                        {
                                break;
                        }
                        else
                        {
                            count+=1;
                        }
                        break;
                    }
                }
            }
        }
        printf("%lld\n",count);
    }
    return 0;
}

I didn't have a very close look at your solution, but a simple (algorithm-agnostic way) to improve efficiency for this kind of task, is to cache (think about which data structure would suit this task) previously found solutions. For instance consider test 1 being a=1, b=10000 and test 2 exactly (or almost) the same, like for instance a=1 and b=10000 or a=2 and b=9999. You are going to check recompute the whole thing, although you just did that.

p.s.: I'll have a closer look at it if you are still struggling with it when I get home.


Finding the longest palindrome from the given string Recently I came across this problem which instructs me to find the longest substring which is a palindrome:

  As we all know, a palindrome is a word that equals its reverse. Here
  are some examples of palindromes: malayalam, gag, appa, amma.

  We consider any sequence consisting of the letters of the English
  alphabet to be a word. So axxb,abbba and bbbccddx are words for our
  purpose. And aaabbaaa, abbba and bbb are examples of palindromes.

  By a subword of a word, we mean a contiguous subsequence of the word.
  For example the subwords of the word abbba are a, b, ab, bb, ba, abb,
  bbb, bba, abbb, bbba and abbba.

  In this task you will given a word and you must find the longest
  subword of this word that is also a palindrome.

  For example if the given word is abbba then the answer is abbba. If
  the given word is abcbcabbacba then the answer is bcabbacb.

I got this with dynamic programming, but for larger test cases it seems I am breaking the memory barrier. When the length is about 4500, it gives me a segmentation fault. That's probably because 4500 x 4500 = 20250000, which is about \$2*10^7\$, and the size of my boolean array is too much in front of the imposed memory limit of 64 MB.

#include <iostream>
#include <cstring>
#include <string>

void longestpalin(std::string str,int n){
    bool table[n][n];                   //boolean table for keeping records of palindromes
    std::memset(table,0,sizeof(table));

    int maxLength = 0;
    for(int i=0;i<n;i++){
        table[i][i] = true; // obviously every letter is palindrome of itself
    }

    int start = 0;

    for(int i=0;i<n-1;i++){
        if(str[i] == str[i+1]){
            table[i][i+1] = true; //two letters repeated is also a palindrome
            maxLength = 2;
            start = i;
        }
    }

    for(int k=3;k <= n;k++){
        for(int i=0;i<n-k+1;i++){
            int j = i + k - 1;
            if(table[i+1][j-1]&&str[i] == str[j]){//checking whether the middle letters are in a pallindrome or not
                table[i][j] = true;
                if(k > maxLength){
                    maxLength = k;
                    start = i;
                }
            }
        }
    }
    std::cout << maxLength << std::endl;
    std::cout << str.substr(start,maxLength) << std::endl;
}

int main (int argc, char const* argv[])
{
    int n;
    std::cin >> n;
    std::string word;
    std::cin >> word;
    longestpalin(word,n);
    return 0;
}

Is there a better approach to avoid a segmentation fault?

You will hit a 64MB memory restriction regardless of how you allocate an array 20+ million entries strong. So you need to revisit an algorithm.

The problem doesn't look like a DP to begin with. Notice that the mere initialization of the array drives the time complexity to be quadratic. If you are happy with the quadratic time, notice that a naive approach requires constant space.

And since the quadratic time complexity will TLE on any decent OJ, you shall check out Manacher's algorithm.


Formatting options for a long IF statement? I have a somewhat unwieldy IF statement and am wondering if anyone has any suggestions on how to make it more readable.

One one line it was all but impossible to understand, so I broke it up into multiple lines:

if (
        empty($row)                                                            // IF the row doesn't exist
    || (!empty($row)                                                           // OR it does exist
        && (     date('j', time()) != date('j', strtotime($row['date_added'])) // AND hasn't been update this day
             && (   empty($row['ignore'])                                      // AND has never been marked to ignore
                 || time()-strtotime($row['ignore']) > 3600*24*7               // OR was last marked to ignore over a week ago
                )
           )
       )
) {
    return $this->updateRow($id);
}
else {
    return 0;
}

However, it's still kind of hard to decipher. I thought about breaking up it into IF/ELSE statements, which makes it a lot more readable, but it also means having redundant code:

if (empty($row)) { // if the row doesn't exist
    return $this->updateRow($id);
}
else { // the row does exist
    if (date('j', time()) != date('j', strtotime($row['date_added']))) {             // if the row hasn't been updated this day
        if (empty($row['ignore']) || time()-strtotime($row['ignore']) > 3600*24*7) { // if the row has never been marked to ignore OR was last marked to ignore over a week ago
            return $this->updateRow($id);
        }
        else { // the row was marked to ignore within the past week
            return 0;
        }
    } // the row has been updated this day
    else {
        return 0;
    }
}

What is the most readable way to format this IF statement?

The first method as it's more concise and has no redundant code
The second method since it's a little more readable (even though it has a lot of redundant code)
A different formatting method

I've used the second snippet since I've found that much easier/faster to understand:

Here you could omit the else keyword:

if (empty($row)) { // if the row doesn't exist
    return $this->updateRow($id);
}
else { // the row does exist
...
}

The following is the same:

if (empty($row)) { // if the row doesn't exist
    return $this->updateRow($id);
}
// the row does exist
...

Then, you could create an explanatory variable to make the comments unnecessary:

$rowExists = !empty($row)
if (!rowExists) {
    return $this->updateRow($id);
}
...

Another explanatory variable with an inverted condition:

$rowExists = !empty($row)
if (!rowExists) {
    return $this->updateRow($id);
}

$updatedToday = (date('j', time()) == date('j', strtotime($row['date_added'])));
if ($updatedToday) {
    return 0;
}

Explanatory variables give names to the expression and reveal the developer's intent, so maintainers will know what's the purpose of (date('j', time()) == date('j', strtotime($row['date_added']))) faster (it should mean that we have an update today) (he or she doesn't have to figure it out or reverse engineer).

It also makes bug fixing faster since a maintainer easily can validate whether the purpose is wrong (we need to check whether we have an update this week instead of this day!) or the implementation (to check that we have an update today or not we should use created field instead of date_added, for example).
Yet another explanatory variables and flattened conditonals:

$neverIgnored = empty($row['ignore']);
if ($neverIgnored) {
    return $this->updateRow($id);
}

$ignoredLastWeek = (time() - strtotime($row['ignore']) < 3600 * 24 * 7);
if ($ignoredLastWeek) {
    return 0;
}

return $this->updateRow($id);

Final code:

$rowExists = !empty($row)
if (!rowExists) {
    return $this->updateRow($id);
}

$updatedToday = (date('j', time()) == date('j', strtotime($row['date_added'])));
if ($updatedToday) {
    return 0;
}

$neverIgnored = empty($row['ignore']);
if ($neverIgnored) {
    return $this->updateRow($id);
}

$ignoredLastWeek = (time() - strtotime($row['ignore']) < 3600 * 24 * 7);
if ($ignoredLastWeek) {
    return 0;
}

return $this->updateRow($id);

You could also create a boolean isUpdateNeeded() function:

function isUpdateNeeded(...) {
    $rowExists = !empty($row)
    if (!rowExists) {
        return true;
    }

    $updatedToday = (date('j', time()) == date('j', strtotime($row['date_added'])));
    if ($updatedToday) {
        return false;
    }

    $neverIgnored = empty($row['ignore']);
    if ($neverIgnored) {
        return true;
    }

    $ignoredLastWeek = (time() - strtotime($row['ignore']) < 3600 * 24 * 7);
    if ($ignoredLastWeek) {
        return false;
    }

    return true;
}

if (isUpdateNeeded(...)) {
    return $this->updateRow($id);
}
return 0;

References:

Clean Code by Robert C. Martin, G19: Use Explanatory Variables
Clean Code by Robert C. Martin, Bad Comments, p67: Don’t Use a Comment When You Can Use a Function or a Variable
Refactoring: Improving the Design of Existing Code by Martin Fowler, Introduce Explaining Variable
Flattening Arrow Code


Imaging with getPixel() and loops I wrote perfectly working code that reads an image, and then does a kind of threshold on it by zone. If the zone has too much white then it turns all the zone into white, otherwise it turns into black.

The problem is that the code was originally from a C++ source with pointers and all instead of GetPixel() etc. Speed was just instant, nothing more. Now that I rewrote "safely" (no pointers thingy), it take like 20 seconds per images, so instant versus 20 seconds = insane difference.

Could any of you help me improve that small code, even if you use back unsafe pointers code etc? I don't really care about that. The part I absolutely need is speed, and it has to be quick as possible because I will deal with huge images library. Instead of taking 30 minutes, it will literally takes days.

    private Bitmap doti(Bitmap imag)
    {
        int threshold = 7;
        int distance = 9;
        int rows, cols, row, col, x, y, count;
        int dhalf = (distance / 2) + 1;
        int Sqdistance = SQ(distance);
        rows = imag.Height;
        cols = imag.Width;
        Bitmap bitmap0 = (Bitmap)imag.Clone();
        Bitmap bitmap = new Bitmap(cols, rows);
        Bitmap outmap = new Bitmap(cols, rows);

        //convert to grayscale of a single byte
        for (row = 0; row < rows; row++)
            for (col = 0; col < cols; col++)
            {
                Color pixel = bitmap0.GetPixel(col, row);
                byte grayish = (byte)Math.Floor((decimal)(pixel.R + pixel.G + pixel.B) / 3);
                bitmap.SetPixel(col, row, Color.FromArgb(grayish, grayish, grayish));
            }

        //check our threshold to set black or white by checking each pixels in a square defined by distance.
        for (row = 0; row < rows; row++)
            for (col = 0; col < cols; col++)
            {
                count = 0;
                //could optimize here heavily maybe, by only checking inside a circle rather than square+dist.
                for (x = Math.Max(col - dhalf, 0); x < Math.Min(col + dhalf, cols); x++)
                    for (y = Math.Max(row - dhalf, 0); y < Math.Min(row + dhalf, rows); y++)
                    {
                      //if inside the square and pixel color is not black count one more not black pixel
                        if ((Sqdistance > DISTANCE(col, row, x, y)) && ((bitmap.GetPixel(x, y).R) > 0)) //this second condition is killing a lot the speed to begin.
                            count++;
                    }

                //if too much count of not black pixel, set it white.
                if (count >= threshold)
                    outmap.SetPixel(col, row, Color.FromArgb(255, 255, 255));
                else
                    outmap.SetPixel(col, row, Color.FromArgb(0, 0, 0));

            }

        return outmap;
    }

SQ() and DISTANCE():

private int SQ(int a) { return ((a) * (a)); }
private int DISTANCE(int a, int b, int c, int d) { return (SQ(a - c) + SQ(b - d)); }

The ´var´ keyword:

From the C# Programming Guide:

  The var keyword can also be useful when the specific type of the variable is tedious to type on the keyboard, or is obvious, or does not add to the readability of the code.

So lines like:

int threshold = 7;
Bitmap bitmap0 = (Bitmap)imag.Clone();

would become:

var threshold = 7;
var bitmap0 = (Bitmap)imag.Clone();

Variable names:

Names like bitmap, bitmap0, Sqdistance don't have a useful meaning, not for you, nor for others reading/reviewing your code. Use meaningful names for your variables, better for readability and maintainability.

Curly braces:

Although you're not writing "wrong" code by omitting the braces on the outer loops, it seriously decreases readability. Following is only 2 lines longer and is much easier to understand:

for (row = 0; row < rows; row++)
{
    for (col = 0; col < cols; col++)
    {
        //Code...
    }
}

Other:

if (count >= threshold)
    outmap.SetPixel(col, row, Color.FromArgb(255, 255, 255));
else
    outmap.SetPixel(col, row, Color.FromArgb(0, 0, 0));

can be rewritten to:

var colorBit = count >= treshold ? 255 : 0;
outmap.SetPixel(col, row, Color.FromArgb(colorBit, colorBit, colorBit));

Following code contains too much braces in my opinion:

private int SQ(int a) { return ((a) * (a)); }

Rewrite it as follows:

private int Square(int a)
{
    return a * a;
}

or a different approach:

private int Square(int a)
{
    return (int)Math.Pow(a, 2);
}

Also for those two last methods, don't make them upper case. Method names in .NET are Pascal case (Capitalization Conventions).

Performance:

Regarding the performance. I didn't take a deep dive in the code or test it, but is there a reason why you perform the loops twice? Isn't there a possibility to execute all the code inse the double for-loop?

I did a performance analysis on your method and it is indeed the GetPixel method that is slowing things done. I did a Google search and apparently a lot of people are dealing with this. Here's an interesting question on StackOverflow that has a possible solution: C# - Faster Alternatives to SetPixel and GetPixel for Bitmaps for Windows Forms App.

Seems that an important part of this approach is the LockBits() method call. More reading about it on MSDN: Bitmap.LockBits Method (Rectangle, ImageLockMode, PixelFormat)


Command for inequality of char I want to ask the user about whether he wants to continue playing or not. For this I have used,

char choice;
do
{
     System.out.println("Would you like to continue?(y/n)");
     choice=sc.nextLine().charAt(0);
     if (choice!='y' && choice!='n')
     {
           System.out.println("Invalid Entry");
     }
}while (choice!='y' && choice!='n');

This is working fine and I didn't face any issues, but is that the right way?

There are some minor issues with this:

The condition to check the value of choice is duplicated
The program will crash if the input is empty
The variable choice is declared outside the loop body
The formatting is a bit unusual

I suggest this way instead:

while (true) {
    System.out.println("Would you like to continue?(y/n)");
    String line = sc.nextLine();
    if (!line.isEmpty()) {
        char choice = line.charAt(0);
        if (choice == 'y' || choice == 'n') {
            return choice;
        }
    }
    System.out.println("Invalid Entry");
}


Bubble sort a list of integers for a number of iterations I wrote a program for this problem, but it stopped because it took too much time. Any tips?

The Problem:
Bubble sort is the simplest algorithm for elements sorting. At each iteration we sequentially compare values of subsequent elements and swap them if necessary.

Your job is to write a program which finds a state of a given list of positive integer numbers after applying a given count of bubble sort iterations

Input Sample:
Each line contains a space-separated list of positive integers and ends with a number of iterations, separated by vertical line ‘|’. Example:

48 51 5 61 18 | 2

Output Sample:

5 48 18 51 61

Explanation:

Iteration 1: 48 5 51 18 61

Iteration 2: 5 48 18 51 61

My code:

def bubsort(num, iters):
    itered = 1
    while itered != iters + 1:
        nums = sorting(num)
        itered += 1
    print ' '.join(num)

def sorting(num):
    for i in xrange(len(num) - 1):
        challenger = int(num[i])
        opponent = int(num[i + 1])
        if challenger > opponent:
            num[i], num[i + 1] = num[i + 1], num[i]
    return num

test = raw_input()
whole = test.strip().split(' | ')
num = whole[0].split()
iters = int(whole[1])
bubsort(num, iters)

Three improvements:

Use the for loop idiom instead of a counter and use _ for variables you don't use.
return the result, don't print it, you should separate processing and output.
Use nums, not num because you should sort multiple numbers, a plural is a more sensible choice.

def bubsort(nums, iters):
    for _ in xrange(iters):
        nums = sorting(nums)
    return nums


Matrix transposition of a string Per a Reddit Daily Programming Challenge, I am performing a matrix transposition on a string using JavaScript.

Example input: 'The quick brown fox jumped over the lazy dog'

Output:

Tqbfjotld
hurouvhao
eioxmeezg
 cw pr y
 kn e
    d

While my code works, I wanted to know:

From a readability standpoint, would it be better to break the code up into smaller functions?
If there's anything in the code that can be simplified.

transposeString('The quick brown fox jumped over the lazy dog');

function transposeString(str) {

  if (str.indexOf(' ') === -1) {
    var map = Array.prototype.map;

    var output = map.call(str, function (char) {
        return char + '\n';
    });
    return output.join('');
  } else {
    var master = [],
        longestWord = '';

    words = str.split(' ');

    words.forEach(function(word) {
      if (word.length >= longestWord.length) longestWord = word;
    });

    longestWord.split('').forEach(function() {
      master.push([]);
    });

    var treatedWords = words.map(function (word) {
      if (word.length < longestWord.length) {
        var spaces = Array(longestWord.length - word.length + 1).join(' ');
        return word = word + spaces;
      } else {
        return word;
      }
    });

    treatedWords.forEach(function (word) {
      var n = 0;
      word.split('').forEach(function (char) {
        master[n].push(char);
        n++;
      });
    });

    master.forEach(function (arr) {
      console.log(arr.join(''));
    });

    return;
  }
}

Here you go, with comments.

Obviously, in production I would have only a handful of comments and not the exhaustive ones I put here. However this is the sort of points I might make during a code review.

When I write code I optimize for it for communicating to a human reader what I intend the code to do. In other words - I want it to read like an essay. Therefore, I extract anything detail-heavy into small helper methods at the bottom of the script. The idea is that someone should open my script and the first thing they see should give them a top-level overview of the general flow of logic.

I didn't comment at all on your solution itself, instead just cleaning up the algorithms that were already there.

transposeString('The quick brown fox jumped over the lazy dog');

//Always comment public apis with usage and your intent when writing it.

//Transpose a string of words into a vertical matrix
//from http://www.reddit.com/r/dailyprogrammer/comments/1m1jam/081313_challenge_137_easy_string_transposition/
function transposeString(str) {
  // the ~ (ones complement) operator makes indexOf work like 'contains'
  // ~(-1) == 0, ~(x != -1) != 0
  // not sure why you need this condition at all however in terms of the problem - seems like everything
  //would just work
  if (!~str.indexOf(' '))
    //definitely don't have two large branches of conditional logic in the same function, put them
    //in separate well-named functions
    //Also always prefer to return early to get the simple paths out of the way in conditional logic.
    //The whole "single point of exit" advice is from back in the day of monolithic functions and gotos all over the place
    return makeVerticalString(str);

  //My preference is to use a single var block with everything aligned and commas preceeding
  //the terms. I have good reasons for this but not anything that I care to get into without
  //being super-pedantic. It's ok to have a different style here.
  var  words            = str.split(' ')         //you were misssing var here
      ,longestWord      = getLongestWord(words)  //give algorithms their own function

      //word matrix - I recommend commenting this so that you know it's a matrix and not a flat array
      //also note that it is much cleaner to use a map than a forEach and explicit push
      //When you see array.push() anywhere it can almost always be cleaned up with a map
      ,transposedMatrix = longestWord.split('').map(function() { return [] })

      //give a name to the thing that you're doing here and extract the algorithm to its own function
      //also rename 'treated' to 'padded' to be more explicit
      ,paddedWords = words.map(padWordsShorterThan(longestWord.length));
      ;
  //again, move the actual algorithm to its own labeled function
  paddedWords.forEach(addEachLetterToARow(transposedMatrix));

  transposedMatrix.forEach(function (arr) {
    console.log(arr.join(''));
  });
  //no need for return if you're not returning anything

  //A line to deliniate private helper functions
  //////////////////////////////////////////////////////////////////////////////////////////////
  function makeVerticalString(str) {
    //You're reducing an array to a single value so really reduce is what you want here
    Array.prototype.reduce.call(str, function(m, c) { return m+c+'\n'}, '')
  }

  function getLongestWord(words) {
    //again going from an array to a single value - we can use reduce here
    return words.reduce(function(longest, thisWord){ return thisWord.length >= longest.length ? thisWord : longest}, '');
  }

  //a function that returns a function can be a good tool for controlling scope while keeping things readable
  //note that the inner function still has a name so that it shows up properly in callstack traces  (rather than "anonymous")
  function padWordsShorterThan(threshold) { return function padWordsShorterThan(word) {
    if (word.length >= threshold)
      return word;
    //no need for else clauses when you return early
    return word + repeatStr(" ", longestWord.length - word.length + 1);
  }}
  function repeatStr(str, times) {
    //you have a neat little way of implementing a character repeat but its difficult to read - drop it in its own method
    return Array(times).join(str)
  }

 //Naming anonymous functions not only helps make clear what they're doing
 //but makes them show up named in a debugging callstack rather than "anonymous"
 //Rule of thumb - name anonymous functions that are doing significant logic
 function addEachLetterToARow(matrix) { return function addEachLetterToARow(word) {
    var n = 0;
    word.split('').forEach(function addToRow(char) {
      matrix[n].push(char); //note that you have a possible error condition here where matrix[n] does not exist. Do you want to check for that explicitly?
      n+=1; //I agree with Doug Crockford that ++ is wierd and error-prone. += reads much better in my opinion
    });
  } }
}


Translator-ish Library In this library:

text_data(val) translates val to computer data types
colorcode() returns the selected color in hex

Can you please tell me how I can make it better?

from tkinter.colorchooser import askcolor
def text_data(text):
    texti = []
    binary = []
    ordv = []
    hexadecimal = []
    octal = []
    i = 0
    while i < len(text):
        j = ord(text[i])
        k = bin(ord(text[i]))
        l = hex(ord(text[i]))
        m = oct(ord(text[i]))
        k = k[2:]
        l = l[2:]
        m = m[2:]
        print(text[i], " ", j, " ", k, "", l, "", m)
        binary.append(k)
        ordv.append(j)
        hexadecimal.append(l)
        texti.append(text[i])
        octal.append(m)
        i += 1
    print("\n")
    print(binary)
    print("\n")
    print(ordv)
    print("\n")
    print(hexadecimal)
    print("\n")
    print(octal)
def colorcode():
    return askcolor()[1]
print(colorcode())

Instead of

i = 0
while i < len(text):
    j = ord(text[i])
    ...
    ...
    ...
    i += 1

and using text[i] for accessing individual elements (which is NOT a Pythonic way).
Get rid of i and write simply

for c in text:
    j = ord(c)
    ...
    ...
    ...

    j = ord(text[i])
    k = bin(ord(text[i]))
    l = hex(ord(text[i]))
    m = oct(ord(text[i]))

After performing the first of those commands is ord(text[i]) already in the variable j, isn't it? So why not reuse it in the rest of them:

for c in text:
    j = ord(c)
    k = bin(j)
    l = hex(j)
    m = oct(j)

Instead of

print(text[i], " ", j, " ", k, "", l, "", m)

use the format() method and replacement fields {}:

print("{}   {}   {}  {}  {}".format(text[i], j, k, l, m))


String Swap-case one-liners (6 ways) I did this small project where there are six one-liner anonymous methods that can swap case of a given string of any length. For example "heLLo, woRLd" ==> "HEllO, WOrlD". I used this string as the example to demonstrate how each of the function works. This string "heLLo, woRLd" can simply be replaced with Console.ReadLine() to get user anticipated inputs. I understand one-liners are not readable and maybe not considered good practice, and used variable names are not proper as well. This whole project was for fun. I believe that way I can learn more. My expectation is that someone can tell me if there are more ways to do this case-swapping (of course, has to be one-liner)?. I think I used 3 (kinda) algorithms to do the swapping. Moreover, all of the one-liners are void functions and all the succeeding functions are shorter than the preceding ones.

using System;
using System.Linq;

class x
{
    static void Main(string[] args)
    {
        // one-liner string swapcase
        (new Action<string>(delegate (string x) { foreach (char i in x) { char b = i; b^= char.IsLetter(b) ? (char)32 : (char)0; Console.Write(b); } }))("heLLo, woRLd");

        Console.WriteLine();

        //one-liner string swapcase2
        (new Action<string>(delegate (string x) { foreach (char i in x) { Console.Write(char.IsUpper(i) ? char.ToLower(i) : char.ToUpper(i)); } }))("heLLo, woRLd");

        Console.WriteLine();

        // one-liner string swapcase3
        ((Action<string>)(delegate (string x) { foreach (char i in x) { Console.Write(char.IsUpper(i) ? char.ToLower(i) : char.ToUpper(i)); } }))("heLLo, woRLd");

        Console.WriteLine();

        //one-liner string swapcase4
        ((Action<string>)((x) => { foreach (char i in x) { Console.Write(char.IsUpper(i) ? char.ToLower(i) : char.ToUpper(i)); } }))("heLLo, woRLd");

        Console.WriteLine();

        //one-liner string swapcase5
        "heLLo, woRLd".ToList().ForEach(i => Console.Write(char.IsUpper(i) ? char.ToLower(i) : char.ToUpper(i)));

        Console.WriteLine();
        //one-liner string swapcase6
        "heLLo, woRLd".ToList().ForEach(i => Console.Write(i^= char.IsLetter(i) ? (char)(1 << 5) : (char)0));
        Console.ReadLine();
    }
}

I find your code amusing but since this is not Code Golf but Code Review I'll review it from our perspective this is, as clean-code first.

Even if you try to write compact code you should not use magic exressions and sacrifice readability. Instead encapsulate them in (local) function or use proper constants. You also should not use unrelated variable names like here i for char where c would be more appropriate. Alternatively you can also use the Aggregate to loop over the string and collect the results in a StringBuilder. The (char)0 can be replaced with the much prettier default.

const char caseOffset = (char)(1 << 5);

char SwapCase(char c) => c ^= (char.IsLetter(c) ? caseOffset : default);

var result =
    "heLLo, woRLd"
        .Aggregate(
            new StringBuilder(),
            (b, c) => b.Append(SwapCase(c))
        ).ToString();

One more option would be to use Select and new string and feed it with the new result.

var result = new string("heLLo, woRLd".Select(SwapCase).ToArray())

The advantage of Select is that you don't need the => lambda anymore.


Non-Sieve Prime Number generator #include <iostream>
#include <vector>
#include <cmath>
using namespace std;
int main()
{
int l=0;
bool isprime;
vector<int>primes;
primes.push_back(2);
for (int nur = 3;true;nur+=2)
{

     isprime=true;
     double ilimit= sqrt(nur);
    for (int primecount=0 ;primes[primecount]<=ilimit;primecount++)
    {
        if (nur % primes[primecount] == 0)
        {
            isprime = false;
            break;
        }

    }

     if (isprime)
     {
             cout<<nur<<endl;
          primes.push_back(nur);
             // l++;
     }

     if (nur<0)
     break;

}
cout<<"Finished"<<endl;
//cout<<primes[l];

  return 0;
 }

Is there anyway i can optimize this further without using a sieve?

Your code includes the optimization of evaluating only odd-valued candidates (by iterating with a step size of 2) because the even-valued candidates are known to contain a factor of 2, and are therefore always composite.

This candidate-skipping optimization can be generalized to include prime factors greater than 2, by varying the step size in a certain way.

I made a version of your code which skips candidates (values of nur) containing factors of 2 and/or 3.  This is done fairly cheaply by alternating step size between 2 and 4.

Extending this optimization to even greater primes gives diminishing returns for two reasons: The step size adjustment calculation becomes more complex (and slower), eventually costing more time than what is saved by skipping candidates; and higher prime factors exist more sparsely than low ones in the candidate population.  Skipping 2 alone catches 1/2 of the composite candidates, and skipping 3 alone would catch 1/3.  But if used together, half of the composites 3 would skip were already skipped by the 2 skip, so skipping 3 just catches an additional 1/6 of the composite candidates.  Including a 5 skip would only eliminate an additional 1/30 of the composite candidates, and a 7 skip 1/210.

I experimented with replacing the floating-point sqrt call with an integer approximation, but it turns out that sqrt is very fast (at least on my machine).  And then I gave up entirely after seeing Vedran Šego's optimization, which is brilliant, especially because as nur gets bigger, the gaps between calls to sqrt become larger, to the point where it doesn't really matter much how slow your sqrt calculation is.  So, I had to add this to my example, thank you Vedran Šego.

One other thing, the nur loop must terminate.  If nur is signed (like int), it will overflow and wrap around to a negative value. This is a completely pathological situation -- calling sqrt with a negative number, plus other things, depedning on optimizations employed.

Even for unsigned nur, duplicate primes will be added to the primes vector until the process crashes when it runs out of memory.

So my example terminates before nur overflows.  I also defined a prime_t type, to make it easy to switch the integer type used for calculating and storing primes.  I use unsigned int in my example, because it gives results a larger maximum value at no extra cost compared to int.  Try unsigned long long if you really want to see the program continue for much longer than you will be willing to wait.

#include <iostream>
#include <vector>
#include <cmath>

typedef unsigned int prime_t;
// max_prime_t is the maximum value which may be held in a variable of type prime_t
// this assumes prime_t is an integer type (signed or unsigned)
// It dupicates information available from std::numeric_limits
const prime_t max_prime_t =
     prime_t(0)-prime_t(1) > prime_t(0) ?
         ~prime_t(0) :
         ~prime_t(0) ^ (prime_t(1) << (sizeof(prime_t)*8-1));

int main() {
    std::vector<prime_t> primes = { 2, 3 };
    prime_t ilimit = 0;
    prime_t ilimit_limit = 0;

    static const prime_t max_nur = max_prime_t - 4;

    // "step" alternates between 2 and 4,
    // to skip values containing factors of 2 and/or 3
    for(prime_t nur=5, step=4; nur<=max_nur; nur += (step ^= 6)) {
        if(nur >= ilimit_limit) {
            // Vedran Sego's optimization
            ilimit = static_cast<prime_t>(std::sqrt(nur));
            ilimit_limit = (ilimit + 1) * (ilimit + 1);
        }
        for(auto one_prime : primes) {
            if(one_prime > ilimit) {
                std::cout << nur << '\n';
                primes.push_back(nur);
                break;
            }
            if(nur % one_prime == 0) {
                break;
            }
        }
    }
}


Imperative Sieve of Eratosthenes using destructive updates in Haskell I've implemented the prime sieve of Eratosthenes in Haskell using a mutable array of unboxed Bools instead of the usual pure and immutable datastructures.

My main concern with this code is speed. For large upper bounds, it runs approximately 70% slower than C code implementing the same algorithm.
Memory is already optimal (1 bit per number plus some small overhead).

I would also like to know if the code is reasonably idiomatic, considering the fact that is impure, and basically non-functional (meaning imperative).
Any nitpicks are appreciated.

import Prelude
import System.Environment (getArgs)
import Data.Array.Unboxed (UArray, (!))
import Data.Array.Storable (newArray, readArray, writeArray, getBounds)
import Data.Array.ST (STUArray, runSTUArray)
import Control.Monad (when)
import Control.Monad.ST (ST)

markMultiplesAsNotPrime :: STUArray s Int Bool -> Int -> ST s ()
markMultiplesAsNotPrime isPrime k = do
  (_, n) <- getBounds isPrime
  kIsPrime <- readArray isPrime k
  when kIsPrime $ do
    let multiples = takeWhile (<=n) $ iterate (+k) (k+k)
    mapM_ (\i -> writeArray isPrime i False) multiples

sieve :: Int -> UArray Int Bool
sieve n | n < 0 = undefined
sieve n = runSTUArray $ do
  isPrime <- newArray (2, n) True :: ST s (STUArray s Int Bool)
  let kMax = isqrt' n
  let ks = takeWhile (<=kMax) . iterate (+1) $ 2
  mapM_ (markMultiplesAsNotPrime isPrime) ks
  return isPrime

isqrt' :: Int -> Int
isqrt' = ceiling . (sqrt :: Double -> Double) . fromIntegral

primesSmallerThanOrEqualTo :: Int -> [Int]
primesSmallerThanOrEqualTo n | n < 2 = []
primesSmallerThanOrEqualTo n = let
    isPrime = sieve n
  in
    [i | i <- [2 .. n], isPrime ! i]

main :: IO ()
main = do
  args <- getArgs
  mapM_ (print . last . primesSmallerThanOrEqualTo . read) args

In case it is relevant:
I compiled and executed the code on an amd64 CPU using "The Glorious Glasgow Haskell Compilation System, version 7.10.3" with the optimization switch "-O2".

Some timings (as measured with the runtime system switch "-s", best of five runs): $$ \begin{array}{4c} n & π(n) & t [s] & \log\left(\frac{t_2}{t_1}\right) \bigg/ \log\left(\frac{π(n_2)}{π(n_1)}\right) \\ 10^4 & 1.23 \cdot 10^3 & 2 \cdot 10^{-3} & - \\ 10^5 & 9.59 \cdot 10^3 & 4 \cdot 10^{-3} & 0.3 \\ 10^6 & 7.85 \cdot 10^4 & 1.0 \cdot 10^{-2} & 0.4 \\ 10^7 & 6.65 \cdot 10^5 & 9.3 \cdot 10^{-2} & 1.04 \\ 10^8 & 5.76 \cdot 10^6 & 1.25 \cdot 10^0 & 1.20 \\ 10^9 & 5.08 \cdot 10^7 & 1.43 \cdot 10^1 & 1.12 \\ 10^{10} & 4.55 \cdot 10^8 & 1.61 \cdot 10^2 & 1.10 \\ 10^{11} & 4.12 \cdot 10^9 & 1.88 \cdot 10^3 & 1.12 \end{array} $$

takeWhile (<=n) $ iterate (+k) (k+k) can be written as [2*k, 3*k..n] which, here, can be replaced with [k*k, k*k+k..n].

Similarly, takeWhile (<= kMax) . iterate (+1) $ 2 can be written as [2..kMax].

Algorithmically, we can work with odds only, and switch to [k*k, k*k+2*k..n] and [3,5..kMax] (special-casing the 2). Taking it further to pre-ignoring the multiples of 3 (5,7...) also, this is known as wheel factorization optimization.

mapM and mapM_, though very idiomatic, can be faster if coded manually as loops (recursive functions). This would also enumerate the numbers manually, instead of creating the interim lists and hoping the compiler won't actually  create them.

Using unsafeRead and unsafeWrite should be faster than the safe variants.

Lastly, last . primesSmallerThanOrEqualTo can be replaced with lastPrime,

lastPrime n = let arr = sieve n in
              take 1 [ i | i <- [n, n-1..2], arr ! i]

Even faster is to move the last prime-finding code into the sieve function itself, and return that prime from it, instead of the array.


How to download tgz file from a url and decompress it efficiently? I have a .tgz file that I need to download given a url inside a Testing folder and then decompress it. I am able to download the .tgz file successfully from the url and also able to successfully decompress it without any issues. My .tgz file will be approx 100 MB max. I am using SharpZipLib library here to decompress.
There are two parts to this:

Download .tgz file from a url in a particular folder.
If file is downloaded successfully then only decompress .tgz file in that same folder othewise not.

Below is my code:
private void DownloadTGZFile(string url, string fileToDownload)
{
    using (var client = new WebClient())
    {
        client.DownloadFile(url + fileToDownload, "Testing/configs.tgz");
    }
    Stream inStream = File.OpenRead("Testing/configs.tgz");
    Stream gzipStream = new GZipInputStream(inStream);

    TarArchive tarArchive = TarArchive.CreateInputTarArchive(gzipStream);
    tarArchive.ExtractContents("Testing/");
    tarArchive.Close();

    gzipStream.Close();
    inStream.Close();
}

Question:
I wanted to check on below things:

Is using WebClient efficient here? If not, then what can I use to download the file efficiently for me considering file size is 100 MB max?
Also can I add a timeout here so that if I am not able to download the file in that time period, then it just times out. But it should retry for 3 times (or with some retry policy) and skip it if not able to by logging an error for the last try.
And if it is able to download the .tgz file successfully then decompress it in the same folder.

I am a newbie who started working with C# so still confuse on usage of WebClient and not sure if it is the right way to do it also. And this will be a production code so wanted to see what's the best way to re-write this whole code if there is any.

DownloadTGZFile does two things, which is one thing too many.

It does what it name says it does (download a .TGZ file),
but it also unpacks the TGZ file, and that is not in the method's name.

Unpacking the file should be a different method. You should then have a new method that calls both methods.

Also, please follow the Microsoft guidelines:

The PascalCasing convention, used for all identifiers except parameter
names, capitalizes the first character of each word (including
acronyms over two letters in length), as shown in the following
examples:
PropertyDescriptor HtmlTag
A special case is made for two-letter acronyms in which both letters
are capitalized, as shown in the following identifier:
IOStream

Thus the method name should be DownloadTgzFile.


Perfomance and Other Improvements I am trying to wrap my arms around all the digital files we have, so I thought I would organize all of our pictures and videos into folders named after dates.  I'm learning F#, and this felt like a good chance for me to exercise that muscle.

Please review the code.  I'm open to any criticisms, but I also have some specific areas of interest:

I feel like the filtering out of the thumbs.db file should be inside the sequence expression.  What's a good way to do that?
In getNewFilename, that function and its internal incrementFilename function each have an expression checking for the existence of the file.  I think that check could be cleanly put into a single check, but I can't think quite how.
It took 1 hour and 10 minutes to copy 120GB in over 28,000 files.  This is on a quad-core laptop with a 7,200 RPM drive.  Realizing that the drive is the limiting factor for this work, and there could be clashes in filenames, what might be some good concurrent processing techniques to possibly have this code finish sooner?

open System
open System.Drawing
open System.IO
open System.Text
open System.Text.RegularExpressions

let rec getAllFiles baseDir =
    seq {
        yield! Directory.EnumerateFiles(baseDir)

        for dir in Directory.EnumerateDirectories(baseDir) do
            yield! getAllFiles dir
    }

let dateTakenFromExif file =
    let r = new Regex(":")
    use fs = new FileStream(file, FileMode.Open, FileAccess.Read)
    use myImage = Image.FromStream(fs, false, false)
    let propItem = myImage.GetPropertyItem(36867)
    let dateTaken = r.Replace(Encoding.UTF8.GetString(propItem.Value), "-", 2)
    DateTime.Parse(dateTaken)

let getDateTaken file =
    try
        dateTakenFromExif file
    with
    | :? Exception ->
        File.GetLastWriteTime(file)   //Use the last write time in the event that the file was moved/copied

let addToFile file n =
  match n with
  | 2 ->
    let fileDir = Path.GetDirectoryName(file)
    let nextFile = Path.GetFileNameWithoutExtension(file) + "-" + n.ToString() + Path.GetExtension(file)
    Path.Combine(fileDir, nextFile)
  | _ ->
    let prev = n-1
    file.Replace("-" + prev.ToString(), "-" + n.ToString())

let getNewFilename newFilePath =
    let rec incrementFilename file n =
        let filenameIncremented = addToFile file n
        match File.Exists(filenameIncremented) with
        | false -> filenameIncremented
        | true -> incrementFilename filenameIncremented (n+1)

    match File.Exists(newFilePath) with
    | false -> newFilePath
    | true  -> incrementFilename newFilePath 2

let move destinationRoot files =
    let moveHelper file =
        let dateTaken = getDateTaken file
        let finalPath = Path.Combine(destinationRoot, dateTaken.Year.ToString(), dateTaken.ToString("yyyy-MM-dd"))
        if not(Directory.Exists(finalPath)) then Directory.CreateDirectory(finalPath) |> ignore

        let newFile = getNewFilename (Path.Combine(finalPath, Path.GetFileName(file)))

        try
          File.Copy(file, newFile)
        with
        | :? Exception as e ->
          failwith (sprintf "error renaming %s to %s\n%s" file newFile e.Message)

    files |> Seq.iter moveHelper

let moveFrom source =
  getAllFiles source
    |> Seq.filter (fun f -> Path.GetExtension(f).ToLower() <> ".db")  //exlcude the thumbs.db files
    |> move """C:\_EXTERNAL_DRIVE\_Camera"""
  printfn "Done"

#time
moveFrom """C:\Users\Mike\Pictures\To Network"""
moveFrom """C:\_EXTERNAL_DRIVE\Camera"""

A few things:

There's already a recursive overload of EnumerateFiles so you don't need getAllFiles. Your top level function would be:

let moveFrom source =
  Directory.EnumerateFiles(source, "*", SearchOption.AllDirectories)
    |> Seq.filter (fun f -> Path.GetExtension(f).ToLower() <> ".db")  //exlcude the thumbs.db files
    |> move """C:\_EXTERNAL_DRIVE\_Camera"""
  printfn "Done"

:? Exception is a type test pattern match, which is unnecessary in a try/with because with only fires for exceptions anyway. You could do this:

let getDateTaken file =
  try dateTakenFromExif file
  with _ -> File.GetLastWriteTime(file)

and this:

try File.Copy(file, newFile)
with e -> failwith (sprintf "error renaming %s to %s\n%s" file newFile e.Message)

instead.

addToFile could be merged into getNewFileName to form a simpler function without repetition:

let getNewFilename newFilePath =
  let rec loop file n =
    if File.Exists(file) then
      let fileDir = Path.GetDirectoryName(file)
      let nextFile = Path.GetFileNameWithoutExtension(file) + "-" + n.ToString() + Path.GetExtension(file)
      loop (Path.Combine(fileDir, nextFile)) (n+1)
    else file
  loop newFilePath 2

For easy parallelization you might try using PSeq ("parallel sequence") from the F# PowerPack. You might see some gains from simply doing this:

files |> PSeq.iter moveHelper


Acquiring indices for anagrams of an input string I am working on an interview question from Amazon Software:

  Design an algorithm to take a list of strings as well as a single input string, and return the indices of the list which are anagrams of the input string, disregarding special characters.

I was able to design the algorithm fine. What I did in psuedocode was:

Create an array character count of the single input string
For each string the list, construct the an array character count
Compare the character count of each string in list to single output string
If same, add it to a list that holds all the indexes of anagrams
Return that list of indices

Here is my implementation in Java (it works; I've tested it):

 public static List<Integer> indicesOfAnag(List<String> li, String comp){
    List<Integer> allAnas = new ArrayList<Integer>();
    int[] charCounts = generateCharCounts(comp);
    int listLength = li.size();
    for(int c=0;c<listLength; c++ ){
        int[] charCountComp = generateCharCounts(li.get(c));
        if(isEqualCounts(charCounts, charCountComp))
            allAnas.add(c);
    }
    return allAnas;
}
private static boolean isEqualCounts(int[] counts1, int[] counts2){
    for(int c=0;c<counts1.length;c++) {
        if(counts1[c]!=counts2[c])
            return false;
    }
    return true;
}
private static int[] generateCharCounts(String comp) {
    int[] charCounts = new int[26];
    int length = comp.length();
    for(int c=0;c<length;c++) {
        charCounts[Character.toLowerCase(comp.charAt(c)) - 'a'] ++;
    }
    return charCounts;
}

I think that this solution runs in \$O(n)\$ where n is the number of strings in the list (I'm not sure, so can someone verify?). What I am concerned about is the space complexity.  I think the space complexity is \$O(n)\$ because I am creating \$n\$ copies of the 26 length array. Is there anything that I can do to reduce the amount of space I am using? I think this space will cumulate and add up.

You're making an assumption that these strings contain just letters.  If there are spaces, digits, or letters with diacritics, then you'll end up with an ArrayIndexOutOfBoundsException.

isEqualCounts() is just a reimplementation of Arrays.equals(int[], int[]).  If you choose not to use Arrays.equals(), then I suggest starting with assert counts1.length == counts2.length;.

A more robust strategy, which doesn't rely on the assumption that the text is purely alphabetic, would be to check whether each sorted array of characters matches the sorted array of characters of the reference string.

You could optionally add a quick-but-crude check by calculating a "summary" or "fingerprint" of each string that consists of its length and, say, the sum of all its characters.  Any string whose fingerprint is different from the fingerprint of the reference could be immediately rejected.  The strings with matching fingerprints could then be checked more thoroughly.


A thread-safe blocking FIFO queue A legacy project was upgraded from legacy code that used either SystemV FIFO device (or similar queue on Windows platform) which was deeply ingrained into every thread's implementation. Following implementation of locking queue with a wait was meant to emulate original FIFO use:
#include <queue>
// target platform doesn't have  std::mutex or support >C++14
#include <QWaitCondition>
#include <QMutex>

template < typename _Msg, typename _Alloc = std::allocator<_Msg> >
class WaitQue
{
public:
    typedef _Alloc  Allocator;
    typedef _Msg    DataType;

    // a thread calls this to place a copy of msg
    void post(const DataType& msg)
    {
        QMutexLocker lock_(&mx);
        que.push(msg);
        cv.wakeOne();
    }

    // a thread calls this to to wait for a message
    void wait(DataType& msg)
    {
        /// wait if que empty
        QMutexLocker cvlock_(&mx);
        while(que.empty())  // spurious exit from wait()
            cv.wait(&mx);

        msg = que.front();
        que.pop();
    }

    unsigned long size()
    {
        QMutexLocker lock_(&mx);
        return que.size();
    }

private:
    typedef std::deque<DataType, Allocator> Container;
    std::queue<DataType, Container> que;

    QWaitCondition cv;
    QMutex mx;
};

A simplest use of such queue without any fluff:
#include <QCoreApplication>
#include <QtConcurrent>
#include <QFuture>

struct Message {
    int value;
};

using Queue = WaitQue<Message>;

Queue east; // messages for east-side events
Queue west; // messages for west-side events

void westSide()
{
   Message msg = {};

   for(int i = 5; i-->0; )
   {
       qDebug() << "West side is working." << endl;
       east.post(Message{i}); //

       west.wait(msg); //
   }
}

void eastSide()
{
   Message msg = {};
   do
   {
       east.wait(msg);
       qDebug() << "East side received " << msg.value << endl;
       west.post(msg);
   }while(msg.value);
}

int main(int argc, char *argv[])
{
   QCoreApplication a(argc,argv);
   QFuture<void> future1 = QtConcurrent::run(westSide);
   QFuture<void> future2 = QtConcurrent::run(eastSide);

   future1.waitForFinished();
   future2.waitForFinished();
   return 0;
}

In reality most of workers are infinite state-machine loops operating only after receiving messages or generating such messages for other workers while some are actually doing ping-pong exchange as above.
Target platform got partial C++11 support and Qt < 5.4 support. std::queue was used instead of Qt container for sake of custom allocator support and condition variable QWaitCondition is used to lock queue. Custom allocator allows to avoid heap allocation. Is this correct implementation and if its not, what flaws are there? Is there a Qt analog for this maybe?
PS. Indefinite wait was a known problem that was solved by design of any scenario was ending with posting "stop thread" messages to all queues in past implementation of project.

template < typename _Msg, typename _Alloc = std::allocator<_Msg> >

Those names are dangerous - underscore followed by capital is reserved to the implementation for any purpose.  That means they could be macros, which would give you some tricky parse errors to diagnose!  Stick to names you're allowed to use (in this case, I'd just drop the underscores).
post()

{
    QMutexLocker lock_(&mx);
    que.push(msg);
    cv.wakeOne();
}

There's an inefficiency here, because waking another thread while we still hold the mutex will cause that thread to immediately block.  Better to release the mutex before waking:
{
    {
        QMutexLocker lock_(&mx);
        que.push(msg);
    }
    cv.wakeOne();
}

A design choice here: should a queue be allowed to expand until it throws std::bad_alloc?  Or do we want a maximum capacity at which the write end blocks?
If you have enough C++11 support, prefer to accept the argument by value, then std::move() it to the container.  That avoids copying rvalue arguments.
wait()

void wait(DataType& msg)

That's a clunky interface.  Prefer to just return the DataType.
I'm not a fan of the truncated "Que" name, and seem to stutter on it every time I read it.  What's wrong with the full word?


Delete the characters of one string from another string I've got an interview coming up for which the following is a prospective question:

  Given 2 strings, for instance:

  John Lennon
  Paul ON

  delete every character from string 1 which corresponds with any
  character in string 2 (case exclusive).

  So following the anticipated manipulations, the output in our case
  would be:

    Jhe

I've devised the following program for the same:

 BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
    System.out.printf("Enter 2 strings%n");
    String s1,s2,s3;
    s1=br.readLine();
    s2=br.readLine();
    s3="[";
    for(int j=0;j<s2.length();j++)
    {
       s3+=Character.toLowerCase(s2.charAt(j));
       s3+=Character.toUpperCase(s2.charAt(j));
    }
    s3+="]";
    Pattern p = Pattern.compile(s3);
    Matcher m = p.matcher(s1);
    int[] a = new int[10];
    int k=0;
    while(m.find())
    {
        a[k]=m.start();
        k++;
    }
    int j=0;
    for(int i=0;i<k;i++)
    {
        System.out.print(s1.substring(j, a[i]));
        j=a[i]+1;
    }
    System.out.print(s1.substring(j));

This works seamlessly, but my predicament is that these folks don't want you to just get a bloody veracious output, they want a highly optimized output i.e. spatial and temporal requirements as to your program should to be as minimum as can be. And mine is nowhere near the epitome of the optimal solution

If anybody feels that they can alter my code or come up with something better that'd be optimal, I'd be much obliged!

I'd use an object to hold the compiled version  of the process for maximum efficiency. This would allow user to create a stripping tool and re-use it for maximum throughput.

Also you have the opportunity to create Strippers. :)

public class Test {

    /**
     * Use a Stripper to encode the stripping process.
     */
    private static class Stripper {

        /**
         * Boolean lookup for maximimum performance.
         *
         * Could use BitSet to save a little space.
         *
         * Length 256 - assumes ASCII.
         */
        private final boolean[] strip = new boolean[256];

        // Compiles the strip string.
        public Stripper(CharSequence strip) {
            for (int i = 0; i < strip.length(); i++) {
                char ch = strip.charAt(i);
                // Both lowercase and uppercase.
                this.strip[Character.toLowerCase(ch)] = true;
                this.strip[Character.toUpperCase(ch)] = true;
            }
        }

        /**
         * Strips all characters from s that were in the `strip` parameter of
         * the constructor.
         *
         * Note the use of `CharSequence` to avoid premature `toString`.
         *
         * @param s - The source of the data to strip.
         * @return The stripped version of s
         */
        public CharSequence strip(CharSequence s) {
            StringBuilder stripped = new StringBuilder();
            for (int i = 0; i < s.length(); i++) {
                char ch = s.charAt(i);
                if (!strip[ch]) {
                    stripped.append(ch);
                }
            }
            return stripped;
        }
    }

    public static final String strip(String from, String strip) {
        return new Stripper(strip).strip(from).toString();
    }

    public void test() {
        System.out.println("Stripped: " + strip("John Lennon", "Paul ON"));

    }

    public static void main(String args[]) {
        try {
            new Test().test();
        } catch (Throwable ex) {
            ex.printStackTrace(System.err);
        }
    }
}

Correctly prints:

  Jhe

This algorithm should have an \$O(n)\$ construction complexity and an \$O(n)\$ processing complexity.

Please forgive the I'd do it a completely different way - this answer was posted before it was moved to Code Review.

Added

It seems a bit of narrative would be useful.

Obviously the desired process comes in two parts, first the setup and second the stripping of characters from the original string. This code performs the setup during the construction of the object by building an array of 256 booleans, one for each possible 8-bit character code. Each boolean holds a true value if the character at this position should be stripped from the string.

This ensures that the absolute minimum needs to be done at process time. All that is required is to take one pass through the string, looking up each character in the boolean array. If we find true then the character is discarded, a false means keep it.


JSON data serialization I'm parsing a JSON data send from server to new JSON Object using JavaScript.

JSON data from server will be in this format

SERVER JSON:

{
  'serverFromDate': '15/07/1988',
  'serverToDate': 123,
  'hdnUserName' : 'User',
  'hdnRole' :'Admin',
  'hdncmbRoles':[{'text':'Admin', 'hdnvalue':'admin'},
           {'text':'Client', 'hdnvalue':'client'}]
}

and I'm converting this JSON to below format

CLIENT JSON:

{
      'clientFromDate': '15/07/1988',
      'clientToDate': 123,
      'txtUserName' : 'User',
      'txtRole' :'Admin',
      'cmbRoles':[{'text':'Admin', 'value':'admin'},
               {'text':'Client', 'value':'client'}]
    }

My Conversion Util is like this...I will check for every server key is present in the Server JSON, if it there then i will added it to the client JSON.

var j_Utils = {
  getValueOf: function(json) {
    return function(key) {
      return json[key];
    }
  },

  hasKeyOf: function(json) {
    return function(key) {
      return json.hasOwnProperty(key);
    }
  }
};

function getMappedData(data) {
  var jObj = {},
    getValueOf = j_Utils.getValueOf(data).bind(j_Utils),
    hasKeyOf = j_Utils.hasKeyOf(data).bind(j_Utils);

  if (hasKeyOf('serverFromDate'))
    jObj['clientFromDate'] = getValueOf('serverFromDate');
  if (hasKeyOf('serverToDate'))
    jObj['clientToDate'] = getValueOf('serverToDate');

  //..........................................
  // TOO MANY DATA OR MAY NOT HAVE ALL TIME
  //..........................................

  return jObj;
};

var clientData = getMappedData({
  'serverFromDate': '15/07/1988',
  'serverToDate': 123
});
console.table(clientData);

Some time these data will be huge, so I'm supposed to write a lengthy javascript code which does the same operation from mapping one value to other. So I want to simplify this JavaScript code in a more efficient and simpler way. The keys from server and client are different (yeah there may be some prefix and suffix or some keys are entirely different), is there any possible ways to simplify these steps in ES6/ES5?

You are right, this can be simplified, quite a lot actually!

First, I'm not sure I see the value of the j_utils object. The methods are simple enough that there isn't much value in creating a function for them, and I'm not sure I see the value of currying here.

That said, it cleans up nicely with arrow functions.

const j_Utils = {
  getValueOf: json => key => json[key],
  hasKeyOf: json => key => json.hasOwnProperty(key)
}

Now the getMappedData function, if I'm not mistaken you currently have a lot more if statements that essentially follow this pattern:

if (hasKeyOf(serverKey)) {
  result[clientKey] = getValueOf(serverKey)
}

This is a great opportunity to use some array methods to move your code into configuration. Here's one way to do it.

function getMappedData(data) {
  var clientServerMapping = [
    { client: 'clientFromDate', server: 'serverFromDate' },
    { client: 'clientToDate', server: 'serverToDate' },
  ];

  const getValueOf = j_Utils.getValueOf(data);
  const hasKeyOf = j_Utils.hasKeyOf(data);

  return clientServerMapping
    .filter(item => hasKeyOf(item.server))
    .reduce((carry, item) => {
        carry[item.client] = getValueOf(item.server);
        return item;
    }, {});
};

And here's the method without requiring the j_Utils helpers.

function getMappedData(data) {
  const clientServerMapping = [
    { client: 'clientFromDate', server: 'serverFromDate' },
    { client: 'clientToDate', server: 'serverToDate' },
  ];

  return clientServerMapping
    .filter(item => data.hasOwnProperty(item.server))
    .reduce((carry, item) => {
      carry[item.client] = data[item.server];
      return carry;
    }, {});
}

let clientData = getMappedData({
  'serverFromDate': '15/07/1988',
  'serverToDate': 123
});
console.log(clientData);

Lastly, I didn't take this approach as your keys might differ, but if all that changes between your object keys input and your object keys output is the word 'server' to 'client', you could just loop through, replacing each key, something like the following.

function getMappedData(data) {
  const result = {};

  Object.keys(data).forEach(key => {
    result[key.replace('server', 'client')] = data[key];
  });

  return result;
}

let clientData = getMappedData({
  'serverFromDate': '15/07/1988',
  'serverToDate': 123
});
console.log(clientData);


Difference between dates in Months and Days I've written a function that returns how many Months and Days are between two dates. I want to respect calendar month boundaries, but I'm ignoring time as it's not relevant for my needs.

I've got the following, but I feel like I'm missing thing I can do to optimize it/refactor it. So I'm looking for additional eyes to help me review this.

//As opposed to TimeSpan ^_^
public readonly struct CalendarSpan
{
    public CalendarSpan(int months, int days)
    {
        Months = months;
        Days = days;
    }
    public readonly int Months;
    public readonly int Days;
}

public static CalendarSpan DifferenceInMonthsAndDays(DateTime startDate, DateTime endDate)
{
    //Ensuring that the larger of two dates is the latter argument
    if (startDate > endDate)
        return DifferenceInMonthsAndDays(endDate, startDate);

    //Start with getting the difference in months
    var months = 12 * (endDate.Year - startDate.Year) + endDate.Month - startDate.Month;

    int days;
    //Add the difference in months to the smaller of the two dates
    var addedMonths = startDate.AddMonths(months);
    if (addedMonths <= endDate)
        //So long as we can avoid negative numbers, just do the simple math
        days = endDate.Day - addedMonths.Day;
    else
    {
        //Otherwise, backoff by 1 months, and use the built in Subtract logic to get the number of days between the two dates
        months--;
        addedMonths = startDate.AddMonths(months);
        days = endDate.Subtract(addedMonths).Days;
    }

    return new CalendarSpan(months, days);
}

UPDATE
I have been trying to get unit tests for this, and I honestly don't know why I ever attempted this. It is NOT an easy or straight forward thing to do at all.
Insert obligatory meme of Homer disappearing into the hedges

Last answer deleted (instead of substantially revising).

Notes (see comments in code)

CalendarSpan signed (date1 > date2 => negative span)
Extension overload of DateTime.Add() taking CalendarSpan argument
Unit test approach (see Main() which iterates positive and negative differences, using DateTime functions to check integrity of results)

`

using System;

namespace DateDiff
{
  static class Program
  {
    public readonly struct CalendarSpan
    {
      public CalendarSpan(int months, int days)
      {
        Months = months;
        Days = days;
      }
      public readonly int Months;
      public readonly int Days;
    }

    //  Maintains sign-consistency, i.e. if l > r returned value components are positive and if l < r negative
    public static CalendarSpan MonthDaysDifference(this DateTime l, DateTime r)
    {
      var mDiff = l.Month - r.Month + 12 * (l.Year - r.Year);

      var dDiff = (l - r.AddMonths(mDiff)).Days;
      if (dDiff == 0 //  no day, the span is a whole number of months
          || l > r && dDiff > 0 //  day and month differences ...
          || l < r && dDiff < 0) //  ... have the same sign
        return new CalendarSpan(mDiff, dDiff);
      //  Signs differ
      var dSign = Math.Sign(dDiff);
      var mSign = Math.Sign(mDiff);
      //  It doesn't matter which sign is which, we adjust months in the correct direction
      mDiff += dSign;
      //  and recalculate days
      return new CalendarSpan(mDiff, (l - r.AddMonths(mDiff)).Days);
    }

    public static DateTime Add(this DateTime dt, CalendarSpan span)
    {
      return dt.AddMonths(span.Months).AddDays(span.Days);
    }

    //  Included to suggest how to test
    static void Main(string[] args)
    {
      for (int i = 0; i < 1000; i++)
      {
        var date1 = new DateTime(2017, 12, 25).AddDays(i);
        for (int j = 0; j < 1000; j++)
        {
          var date2 = new DateTime(2019, 01, 01).AddDays(j);
          var dateDiff = date1.MonthDaysDifference(date2);
          //var dateDiff2 = MonthDaysDifference(date1,date2);//just checking
          if (date2.Add(dateDiff) != date1)
          {
            Console.WriteLine($"ERROR: {date1:yyyy-MM-dd} - {date2:yyyy-MM-dd} = {dateDiff.Months} months + {dateDiff.Days} days");
            Console.WriteLine("Any key to exit");
            Console.ReadKey();
            return;
          }
          Console.WriteLine($"{date1:yyyy-MM-dd} - {date2:yyyy-MM-dd} = {dateDiff.Months} months + {dateDiff.Days} days");
        }
      }
    }
  }
}


Is there a better way of implementing this control structure? I have the following control structure that I would like to improve:

if(number >= 100)    then doSetInc(id, 8, 20)
elseif(number >= 91) then doSetInc(id, 8, 30)
elseif(number >= 81) then doSetInc(id, 8, 50)
elseif(number >= 7)  then doSetInc(id, 8, 70)
elseif(number >= 1)  then doSetInc(id, 8, 100)
end

Kinda hard to know what you mean with better since you don't specify in what regard.

In regards to cleaner code:

FunctionName doSetInc is a bit weird, what does it do?
You are sending in 3 variables into the function, it's common practice to use as few as possible (rarely more than 2, 0 or 1 prefered).
You are always sending in "id" and "8" into the function, thus this function might not need those attributes and could be set in another way.

Other than that, since I find no easy mathematical similarities between these checks and what to do, I would say no, you can't improve it given this small piece of code and no other context.


Speed up file format conversion I wrote this quick script to convert a genotype file into a BED file. Running it seems to be taking a very long time.

#BED file format
#http://genome.ucsc.edu/FAQ/FAQformat.html#format1
#1.chrom - The name of the chromosome (e.g. chr3, chrY, chr2_random) or scaffold (e.g. scaffold10671).
#2.chromStart - The starting position of the feature in the chromosome or scaffold. The first base in a chromosome is numbered 0.
#3.chromEnd - The ending position of the feature in the chromosome or scaffold. The chromEnd base is not included in the display of the feature. For example, the first 100 bases of a chromosome are defined as chromStart=0, chromEnd=100, and span the bases numbered 0-99.

from collections import defaultdict
from operator import itemgetter
from itertools import groupby
from __future__ import print_function

genotype = open("<file>", "rU")

def listToRange(list):
    ranges = []
     for k, g in groupby(enumerate(list), lambda (i,x):i-x):
        group = map(itemgetter(1), g)
        ranges.append((group[0], group[-1]))
    return ranges

#Create Dictionary with all positions for each chrom
chromDict = defaultdict(list)

genotypeLines = list(genotype.readlines()[1:])

for item in genotypeLines:
    objects = item.split()

    chromDict[objects[0]].append(int(objects[1]))

#print() implemented in python 3 for use in python 2 need to: from __future__ import     print_function
for key, value in chromDict.iteritems():
    if len(listToRange(value)) > 1:
        for i in range(0,(len(listToRange(value)))):
            print(key, listToRange(value)[i][0],listToRange(value)[i][1], sep="\t")
    else:
        print(key, listToRange(value)[0][0], listToRange(value)[0][1], sep="\t")

#need to capture output to terminal i.e. python script.py > myfile.bed

genotype.close()

Input file example:

Chr             Position 1      2      3
HE669507        9752    C       C       C
HE669507        9753    T       T       T
HE669507        9755    A       A       A

This is what the output looks like. It turns positions into intervals

HE669507        9752    9753
HE669507        9755    9755

I am not sure what is causing it to be so inefficient, but I suspect it might be something to do with the way I print the output.

I do not know python, so I should probably restrain from answering. But my initial observation by looking at your code is the repeated call to listToRange in the last for loop.

We can also skip the if len() > 1.

Rewrite:

for key, value in chromDict.iteritems():
    chrom_range = listToRange(value)
    chrom_len = len(chrom_range)
    for i in range(0, chrom_len):
        print(key, chrom_range[i][0], chrom_range[i][1], sep="\t")

Some other notes:

There is also the question about variable names. In your
listToRange(list), list is a python keyword.
import __future__  statements should be served before
anything else.
Comments and code should be within a width limit. I stay with the 80
rule.
Space after comma.

As you tag question with beginner I would also recommend PEP 8 – Style Guide for Python Code. It is one of those things that when one accumulate experience one better see where and when it is appropriate to deviate from. Start out strict and lax later if appropriate.

There is also a pep8 command line tool for easy validation.

As such I can add the bullet point:

Function names should be lowercase, with words separated
by underscores as necessary to improve readability.

That is: if this is not part of a bigger code-base where the norm is to use camel-case.

For file read with can be nice to use. I simply quote python.org by:

  It is good practice to use the with keyword when dealing with file objects. This has the advantage that the file is properly closed after its suite finishes, even if an exception is raised on the way. It is also much shorter than writing equivalent try-finally blocks:

As noted, I do not know python, but add a simple writeup anyway, you might find some of it useful:

""" GENO TO BED """
# BED file format
# http://genome.ucsc.edu/FAQ/FAQformat.html#format1
# 1.chrom      - The name of the chromosome (e.g. chr3, chrY, chr2_random) or
#                scaffold (e.g. scaffold10671).
# 2.chromStart - The starting position of the feature in the chromosome or
#                scaffold. The first base in a chromosome is numbered 0.
# 3.chromEnd   - The ending position of the feature in the chromosome or
#                scaffold. The chromEnd base is not included in the display of
#                the feature. For example:
#    The first 100 bases of a chromosome are defined as
#    chromStart=0, chromEnd=100, and span the bases numbered 0-99.

from __future__ import print_function
from collections import defaultdict
from operator import itemgetter
from itertools import groupby
import sys

def list_to_range(list_):
    """ List to range """
    ranges = []
    for k, groups in groupby(enumerate(list_), lambda (i, x): i - x):
        group = map(itemgetter(1), groups)
        ranges.append((group[0], group[-1]))
    return ranges

def dump_bed(chrom_dict):
    """ Print BED to stdout """
    for key, value in chrom_dict.iteritems():
        chrom_range = list_to_range(value)
        chrom_len = len(chrom_range)
        for i in range(0, chrom_len):
            print(key, chrom_range[i][0], chrom_range[i][6], sep="\t")

def dict_from_file(filename):
    """ Generate dictionary from file """
    chrom_dict = defaultdict(list)
    with open(filename, "rU") as handle:
        next(handle) # Skip first line
        for line in handle:
            if line == "\n":
                continue
            try:
                key, val = line.split()[0:2]
                chrom_dict[key].append(int(val))
            except ValueError:
                # Here you likely want to print some error.
                # Use except ValueError, err: to get the
                # error description.
                pass

    return chrom_dict

def main():
    """ Main routine """
    chrom_dict = dict_from_file(sys.argv[1])
    dump_bed(chrom_dict)
    return 0

if __name__ == '__main__':
    if len(sys.argv) == 1:
        print("Missing filename.", file=sys.stderr)
        sys.exit(1)
    sys.exit(main())


WordPress plugin for obfuscating a file URL I'm making a WordPress plugin that obfuscates the real file URL and forces the download of one of this 3 types of URL:

Local: http://example.com/wp-content/custom/filename.zip
Dropbox: https://www.dropbox.com/s/DBOX-KEY/filename.zip?dl=1
Google Drive: https://docs.google.com/uc?export=download&id=GDOCS-KEY

The final URL that's provided to the user has a code which is checked against the database (http://example.com/private/?download_file=SITE-KEY). If correct, there are other checks in place and at the end the file is served through the script below with a read_file to the real URL:

$download_url = "LOCAL-GOOGLE-DROPBOX-URL";

@ob_end_clean(); //turn off output buffering to decrease cpu usage

// required for IE, otherwise Content-Disposition may be ignored
if ( ini_get( 'zlib.output_compression' ) ) ini_set( 'zlib.output_compression', 'Off' );

header( 'Content-Type: ' . $mime);
header( "Content-Disposition: attachment; filename=\"".$final_name."\";" );
header( "Content-Transfer-Encoding: binary" );
header( 'Accept-Ranges: bytes' );
header( "Cache-control: private" );
header( 'Pragma: private' );
header( "Expires: Mon, 26 Jul 1997 05:00:00 GMT" );

flush();
readfile( $download_url );

I'm inspecting the headers and response and there's no trace of the real URL.

Doubts:

Is this really a "secure" method?
Can the real URL be sniffed by other means?

For completeness, here are the inspection results for a Google Drive file:

Request URL:http://example.com/private/?download_file=1310819c43
Request Method:GET
Status Code:200 OK

Request Headers
  Accept:text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
  Accept-Charset:ISO-8859-1,utf-8;q=0.7,*;q=0.3
  Accept-Encoding:gzip,deflate,sdch
  Accept-Language:en-US,en;q=0.8
  Connection:keep-alive
  Host:example.com
  User-Agent:Mozilla/5.0 (Macintosh; Intel Mac OS X 10_7_4) AppleWebKit/537.22 (KHTML, like Gecko) Chrome/25.0.1366.0

  Safari/537.22 FirePHP/4Chrome
        X-FirePHP-Version:0.0.6

Query String Parameters
  pvdn_file:1310819c43

Response Headers
  Accept-Ranges:bytes
  Access-Control-Allow-Origin:http://example.com
  Cache-control:private
  Connection:Keep-Alive
  Content-Disposition:attachment; filename="Test File";
  Content-Transfer-Encoding:binary
  Content-Type:image/png
  Date:Sun, 19 May 2013 17:50:00 GMT
  Expires:Mon, 26 Jul 1997 05:00:00 GMT
  Keep-Alive:timeout=5, max=100
  Pragma:private
  Server:Apache
  Transfer-Encoding:chunked
  X-Pingback:http://example.com/xmlrpc.php
  X-Powered-By:PHP/5.3.6

The server is fetching the contents of the URL and then sending it to the user. Unless you accidentally output the URL, via an error or similar, there is no way the user could see it. The server is effectively sitting between them and the remote server.

If you can, it would probably be better to host the files locally otherwise you will be using twice the bandwidth and also have the risk that one of the remote servers might go down.

As long as the user can't specify the remote URL or edit the contents of it, it should be secure.


Google foobar level 2 challenge I am a beginner in Python-3. I was solving a level 2 challenge in Google Foobar. Given an initial amount of items (LAMBs) I have to distribute it among individuals, who are ranked according to seniority, while adhering to the following rules:

The junior most individual gets 1 Lamb
An immediately senior individual can't get more than twice their juniors lambs, otherwise the junior will rebel
A senior individual can't get less lambs than the sum of their 2 previous juniors, otherwise the senior will rebel
In case lambs are leftover and adding a senior does not violate any previous rules, they must be added.

The goal is to find the difference between how many individuals can be paid if we pay the minimum amount to each (stingy) and the maximum amount to each (generous). I noticed that stingy condition resembles a Fibonacci sequence and the generous condition is sum of powers of 2. My code is as follows:
from math import log2

def solution(total_lambs)
    return (stingy(total_lambs) - generous(total_lambs))

def stingy(total_lambs)
    if (total_lambs == 1): # from condition 1 at least 1 individual gets paid
        return 1
    elif (total_lambs == 2): # from condition 2 and 4 the most each can is 1
        return 2
    else:
        ind, total, x0, x1 = 2, 2, 1, 1 # no. individuals, sum of Fib seq, intial Fib nos.
        while (total <= total_lambs):
            x0, x1 = x1, x0 + x1 # advancing the Fib seq
            total = total + x1 # total is the sum of lambs paid
            if (total > total_lambs):
                return ind
            else:
                ind = ind + 1

def generous(total_lambs):
    if (total_lambs == 1):
        return 1
    if (total_lambs == 2):
        return 2
    else:
        ind = int(log2(capital + 1)) # typecasting to int removes decimals
        if (total_lambs - 2**n + 1 >= 2**(n - 1) + 2**(n - 2)): # condition 2, 3 and 4
            return ind + 1
        else:
            return ind

Please suggest improvements wherever possible.

Early-returns
else-after-return is redundant, so your stingy is equivalent to
    if (total_lambs == 1): # from condition 1 at least 1 individual gets paid
        return 1
    if (total_lambs == 2): # from condition 2 and 4 the most each can is 1
        return 2

    ind, total, x0, x1 = 2, 2, 1, 1 # no. individuals, sum of Fib seq, intial Fib nos.
    while (total <= total_lambs):
        x0, x1 = x1, x0 + x1 # advancing the Fib seq
        total = total + x1 # total is the sum of lambs paid
        if (total > total_lambs):
            return ind
        ind = ind + 1

Return parens
You have a mix of this style:
return 1

and this style:
return (stingy(total_lambs) - generous(total_lambs))

the latter not needing outer parens. Probably best to go with the non-parenthetical style. The same is true of
while (total <= total_lambs):

In-place addition
total = total + x1
ind = ind + 1

can be
total += x1
ind += 1


Printing results in XML I have been writing PHP programs in the MVC pattern for quite some time. But I am not sure if I am doing it right.

For instance, I have this PHP file that prints results in XML, and I put this file in 'model':

header("Content-Type: text/xml");
echo '<?xml version="1.0" encoding="utf-8" standalone="yes" ?>
<response>
';

# Set object property/ variable.
$post = $connection->arrayToObject($_POST,$property_overloading = true);
//print_r($post);

# Assuming no error.
$error = false;

# Get the required field and put them in an array.
$array_required = is_array($post->required)? $post->required : explode(' ',$post->required);

# Get title
if (!$post->title)
{
    if(in_array('title', $array_required))
    {
        $error = true;
        echo '<error elementid="title" message="TITLE - Please entitle this album." />';
    }
}

# Get description
if ($post->description)
{
    if(in_array('description', $array_required))
    {
        $error = true;
        echo '<error elementid="description" message="DESCRIPTION - Please enter something." />';
    }
}
else
{
    # Ceck if there is a line break, if the match is found, replace it with an empty space.
    if (preg_match("/\n/", $post->description))
    {
        $post->description = preg_replace("/\n/is", " ", $post->description);
    }
}

# Get hide
if(!$post->hide) $post->hide = 0;
    else $post->hide = 1;

# Print general error message.
if ($error) echo '<errorgeneral message="Please amend the error above." />';

# check if no more errors
if (!$error)
{
    $sql = "
    UPDATE album
    SET
        title = ?,
        description = ?,
        hide = ?
    WHERE album_id = ?
    ";

    # use the instantiated db connection object from the initiator.php, to process the query.
    $result = $connection->executeSQL($sql,array(
        $post->title,
        $post->description,
        $post->hide,
        $post->album_id
    ));

    # Must convert all special characters to XML.
    # http://en.wikipedia.org/wiki/List_of_XML_and_HTML_character_entity_references
    echo '<result message="Updated." itemid="'.$post->album_id.'" itemtitle="'.htmlspecialchars($post->title).'" itemdescription="'.htmlspecialchars($post->description).'" itemcategory="album"/>';
}

echo '</response>';

So, which part of it should be in the controller, and which should be in the model, helper, or view?

Ok, this might come across as blunt and hurtful, but it's meant to be helpful (code-review IMO should be harsh, here's why):

"I have been writing PHP programs in the MVC pattern for quite some time"
No, you haven't. Really. M-V-C stands for, as you well know: Model-View-Controller. The code you pasted just strings all three together in one single script. That's spaghetti-code (at best), not framework-like MVC code. Not even close.

"I have this PHP file that prints a result in XML, and I put this file in 'model'"
Again: any file that generates output belongs in the view. As the name suggests, anything that is sent to the client (ie the client gets to "see"), is part of the clients view of your application. You really should read up on MVC, to find out what belongs where. (there's a link at bottom of my answer)

At the top, you set your header, then you process the request (without checking if there has been a POST request). Setting the headers as soon as possible (before anything has been sent to the client) is understandable. But in the context of the MVC pattern, it shouldn't really matter. In fact, it's one of the great things about the real MVC pattern: inside your controller and model layer, you're absolutely sure the headers haven't been sent yet, because the view is only generated (and sent to the client) at the very end of the request.
So, as your app sets about processing the request, and you hit a point where the client should be redirected, you can do so, almost blindly. Safe in the knowledge that the headers aren't sent yet. That's why MVC is so successful, and why redirects are mostly found in the controller. To clarify, look at this very simple graph (from cakePHP site, but it applies to MVC in general):

(source: cakephp.org)

Here you see the flow of an MVC webapp:

The client interacts with the view, and a request is sent to the server.
The dispatcher receives teh request and does some initial processing. Based on this request, the dispatcher deceides which controller needs to be constructed, and which method needs to be called to set things in motion
The controller can do some more checks, to make sure the request is valid. If this is not the case the Model layer is left alone, and the client is redirected. Back over to the dispatcher, (2). This step should be uncommon (as it only occurs when something isn't quite right.
Normally, the inital processing on the request data consists mainly of pouring the data into objects, and deciding what the Model layer needs to do with them.
The model layer receives data from the controller and takes over. Here's where you'll find the actual logic. DB connections, computation, ... it's all contained here (which is why this is also referred to as the business layer). The data provided by the controller can be broken down more (checked/processed), used in queries etc... The result is returned to the controller. Note that the model layer does not communicate with the view in any way!
The controller checks what the Model-layer returned. This can either be data, needed to build a new view, or an object/string saying the data turned out to be invalid/insufficient or nothing at all. Whatever it is, the controller can decide to redirect (back to step 1), call the model again (back to 3) or moving on: build the view, passing the data it received from the Model-layer.
Finally, we can start building the view. The controller passes the data it received from the model layer and the View takes over. It isn't untill we've reached this step that the headers are set. During steps 1 through 4, the headers are nowhere near being set, but at this point, it's too late to go back.
That's why the view is a simple script, containing markup and simple PHP (some loops, some if's and the occasional switch). A view contains no logic at all!.
Finally, the resulting page (view) is sent to the client. Job done.

I hope this makes a few things clear, but now
Moving on:

$connection->arrayToObject($_POST,$property_overloading = true);

$property_overloading = true? why not pass true? now you have a global variable that is just sitting there. $connection, as I gather from the query at the bottom of your code, is a DB class. What is it doing processing your POST parameters? That's not it's job. That's a gross violation of the Single Responsability Principle. Don't. Just Don't!
SRP states that an object/class should have one and only one task (reason to change): processing POST params and connecting to the DB are 2 distinct tasks.

Following that, you check a series of parameters that may or may not be set, and sanitize them one by one. I mean: really! you're using an object, clearly that object should take care of sanitizing the data, and, using getters, you could just do something like this

$post->get('hide', false);
//with getter in Post class:
public function get($param, $defaultValue = null)
{
    if (!isset($this->data[$param]))
    {//not set, return default
        return $defaultValue;
    }
    return $this->data[$param];
}

The same applies to setters, only the setter sanitizes the data.
Though at this point, you have bigger fish to fry: don't do things like

if (preg_match("/\n/", $post->description))
{
    $post->description = preg_replace("/\n/is", " ", $post->description);
}

In the if experession, you're using a regular expression, only to then use another regex to remove the unwanted chars. Either do:

public function setDescription($desc)
{//in object, of course
    $this->description = preg_replace('/\n/',' ',$desc);
    return $this;
}
//or:
$this->description = implode(' ', explode(PHP_EOL, $desc));

If the string doesn't contain any new-lines, it's not going to be changed! Why bother checking?

Next, you're echo-ing all over the place, to which I can only say: don't. as Dave Jarvis says use an API like simplexml.

Stringing a query at the end and executing it, depending on what $error hold (is it a global??) is just terrible By the time you're generating output, you should have all your data ready set, good to go. Building the response/view should be a formality. That's why a view should not contain any logic. All data has been fetched and structured accordingly.

To answer your question (on which part should go where):

processing the request is usually done in the Routing objects, and the processed request is then used to create/and pass to the (front)controller. The controller passes the bits and pieces of the request to the model-LAYER (services, helpers, data-models), which is where all the logic is kept. The controller then receives the results from the model-layer, and passes it on to the view, which (through a series of loops and if's) generates the output.

If any of this is still a bit merky, refer to the MVC wiki or ask for more details.


Receive an array input and output a string I have some code which is working, but I would be interested if anyone could review it and tell me if there is a more efficient way of achieving my goal.

I receive input from a data source as an array such as ["Y","Y",,"Y","Y",,"Y"].

The values relate to investment fund names and I am outputting a string such as "Balanced, Cash, High Growth, Moderate and Growth options".

If there is only one value populated with "Y" the string could be "Conservative option".

If there are two values, the string could be "Conservative and Cash options".

For more than two values, the string could be "Conservative, Cash and Moderate options".

I know that the order of the values will always be the same, eg the first value will always be the Balanced option. Here is the code:

// balanced, cash, high growth, moderate and growth
var params = ["Y","Y",,"Y","Y",,"Y"];

getString(params)

function getString(values) {

// map for the values
var fundMap = {
    0: "Balanced",
    1: "Cash",
    2: "Conservative",
    3: "High Growth",
    4: "Moderate",
    5: "Shares",
    6: "Growth"
    }

var fundArray = [];

// get fund names from map and push to array
for (var i = 0; i < values.length; i++) {
    if (values[i] == "Y") {
        fundArray.push(fundMap[i]);
    }
}
console.log(fundArray);

var fundString = "";

if (fundArray.length == 1) {

    fundString = fundArray + " option";

}
 else if (fundArray.length == 2) {

    fundString = fundArray[0] + " and " + fundArray[1] + " options";
}

 else  {

    for (var i = 0; i < fundArray.length -2; i++) {
        fundString = fundString + fundArray[i] +  ", ";
    }

    fundString = fundString + fundArray[fundArray.length -2] + " and ";
    fundString = fundString + fundArray[fundArray.length -1] + " options";
}
console.log(fundString);
}

I would love to know if there is a more efficient or just a neater way of writing this code please.

Thank you!

A few stylistic nitpicks: the function body should be indented and there should be a space following the - signs.

getString should be changed to take an array of boolean values, instead of a sparse array of "Y" strings. You can write a separate function to convert your original format to a bool array.

function optionsToBoolArray(array){
    var converted = [];
    for(var i = 0; i < array.length; i++){
        converted[i] = array[i] === "Y";
    }
    return converted;
}

getString is also a undescriptive name for your function. You should change it to something along the lines of optionsToReadableString.

fundMap doesn't need to be an object. Since you are currently using like an array, explicitly make it an array for clarity.

fundString = fundArray + " option"; should be fundString = fundArray[0] + " option"; to avoid implicit conversions for readability.

The code for multiple options can also be simplified by using array methods:

else {
    fundString = fundArray.slice(0,fundArray.length - 1).join(", ") + " and " + fundArray[fundArray.length - 1] + " options";
}

This also lets you eliminate the else if branch.

You should also extract the console.log out of the function to separate the logic. After doing this, you can eliminate fundString completely by just returning the value from each branch of the if/else.

You should also look into learning ES6 features such as using let instead of var.