ffmpeg batch script for video resizing I have a few hundred mp4 files (thanks android =/) that I need to convert. I only want to convert videos that are too large (>= 1080p) and I do want to keep the exif Information so the timeline is not broken.

I hope someone can look over the script to point me to some grave mistakes or give some improvement.

Anything I am missing? Thanks in advance.

resize() {
    echo "Filename $1"
    filename=$(basename -- "$1")
    extension="${filename##*.}"
    filename="${filename%.*}"
    new_filename=${filename}.${timestamp}.${extension}
    ffmpeg -v quiet -stats -i $1 -map_metadata 0 \
    -vf scale=-1:720 -c:v libx264 -crf 23 \
    -c:a copy $new_filename < /dev/null
    exiftool -TagsFromFile $1 "-all:all>all:all" -overwrite_original     $new_filename
}

if [[ -d $1 ]]; then
    timestamp=$(date +%s)
    echo "Finding Video Files ..."
    exiftool $1/*.mp4 -if '$ImageHeight >= 1080' -p '$Filename' > /tmp/fl_$timestamp

    echo "Processing Video Files ..."
    while IFS= read -r line; do
        resize $line
    done < /tmp/fl_$timestamp
    rm /tmp/fl_$timestamp
elif [[ -f $1 ]]; then
    resize $1
else
    echo "Argument missing"
    exit 1
fi

Consider portable shell
The only Bash feature we're using is [[ ]]  for testing file properties.  It's easy to replace [[ -d $1 ]] with [ -d "$1" ] and that allows us to stick with standard shell, which is more portable and lower overhead:
#!/bin/sh

Careful with quoting
Most of shellcheck's output is due to failure to quote parameter expansions:
236052.sh:9:31: note: Double quote to prevent globbing and word splitting. [SC2086]
236052.sh:9:50: error: Delete trailing spaces after \ to break line (or use quotes for literal space). [SC1101]
236052.sh:10:5: warning: This flag is used as a command name. Bad line break or missing [ .. ]? [SC2215]
236052.sh:11:15: note: Double quote to prevent globbing and word splitting. [SC2086]
236052.sh:12:28: note: Double quote to prevent globbing and word splitting. [SC2086]
236052.sh:12:74: note: Double quote to prevent globbing and word splitting. [SC2086]
236052.sh:18:14: note: Double quote to prevent globbing and word splitting. [SC2086]
236052.sh:18:27: note: Expressions don't expand in single quotes, use double quotes for that. [SC2016]
236052.sh:18:53: note: Expressions don't expand in single quotes, use double quotes for that. [SC2016]
236052.sh:18:75: note: Double quote to prevent globbing and word splitting. [SC2086]
236052.sh:22:16: note: Double quote to prevent globbing and word splitting. [SC2086]
236052.sh:23:20: note: Double quote to prevent globbing and word splitting. [SC2086]
236052.sh:24:16: note: Double quote to prevent globbing and word splitting. [SC2086]
236052.sh:26:12: note: Double quote to prevent globbing and word splitting. [SC2086]

Ironically, you do have quotes in some places they aren't strictly necessary, so it's unclear why you missed all these.
Errors go to standard output

echo "Argument missing"
exit 1

That should be:

echo >&2 "Argument missing"
exit 1

The test here is slightly wrong: the argument may be present, but not the name of a plain file or directory.  So I'd replace that with:
elif [ -e "$1" ]
    echo "$1: not a plain file or directory" >&2
    exit 1
elif [ "$1" ]
    echo "$1: file not found" >&2
    exit 1
else
    echo "Argument missing" >&2
    exit 1
fi

It may be worthwhile to move that testing into the resize function, because at present we assume that the contents found in directory arguments are plain files (that said, we're covering a tiny corner case with that, so I wouldn't sweat it - just let the commands there fail).
Don't assume previous commands succeeded
In resize, if ffmpeg fails, there's little point running exiftool, so connect them with &&.  Also consider removing the file if it was created with errors (so we're not fooled by a partly-written output into thinking this file doesn't need conversion).
Avoid temporary files
There's no need for the file /tmp/fl_$timestamp: we could simply use a pipeline there.
Consider accepting more arguments
Instead of only allowing a single argument (and ignoring all but the first), let the user specify as many files as needed; it's easy to loop over them using for.
Handle directories using recursion
Instead of the while loop, we could invoke our script recursively using xargs.  I'll make it a separate function for clarity:
resize_dir() {
    exiftool "$1"/*.mp4 -if '$ImageHeight >= 1080' -p '$Filename' |
        xargs -r -d '\n' -- "$0" || status=false
}

(xargs -r is a GNU extension to avoid running the command with no arguments.  If this option isn't available, we'll need to modify the script so that passing no arguments isn't an error.)

Modified code
This is Shellcheck-clean, but I'm not able to test it (lacking the requisite directory of MPEG files).
#!/bin/sh

set -eu

status=true
fail() {
    echo "$@" >&2
    status=false
}

# Resize a single file
resize() {
    echo "Filename $1"
    filename=$(basename -- "$1")
    extension=${filename##*.}
    filename=${filename%.*}
    new_filename=${filename}.${timestamp}.${extension}
    if
        ffmpeg -v quiet -stats -i "$1" -map_metadata 0 \
               -vf scale=-1:720 -c:v libx264 -crf 23 \
               -c:a copy "$new_filename" < /dev/null &&
            exiftool -TagsFromFile "$1" '-all:all>all:all' \
                     -overwrite_original "$new_filename"
    then
        # success
        true
    else
        # failed; destroy the evidence
        rm -f "$new_filename" 2>/dev/null
        fail "Failed to convert $1"
    fi
}

# Resize all *.mp4 files in a single directory
# N.B. only immediate contents; not recursive
resize_dir() {
    # shellcheck disable=SC2016
    exiftool "$1"/*.mp4 -if '$ImageHeight >= 1080' -p '$Filename' |
        xargs -r -d '\n' -- "$0" || status=false
}

[ $# -gt 0 ] || fail "Usage: $0 FILE FILE..."

timestamp=$(date +%s)

for arg
do
    if [ -d "$arg" ]
    then
        resize_dir "$arg"
    elif [ -f "$arg" ]
    then
        resize "$arg"
    elif [ -e "$arg" ]
    then
        fail "$arg: not a plain file or directory"
    else
        fail "$arg: file not found"
    fi
done

exec $status # true or false


Mapping uint to value within min/max set I have a method that transforms any input uint into a [min,max] set. I am looking for improvements on the function, specifically to get rid of the input == uint.MaxValue case.

public static uint NumberInRange(uint input, uint min, uint max)
{
    decimal divider = uint.MaxValue / (decimal)max;
    if (input == uint.MaxValue)
        return (uint)max;

    var i = (uint)(Math.Floor(input / divider) + min);
    return i;
}

Let me know if you see any ways to improve this function, or if you think that there is any subtle bug within it.

I am basically just looking into a way to map a bigger set [0,uint.MaxValue] into a smaller set [min,map].

Let's say the inputs are:

NumberInRange(0, 10, 100) - I should get 10

NumberInRange(uint.MaxValue, 10, 100) - I should get 100

For any number in between, I want them to be mapped "equally" between members of the set.

Let's say I put in a number that gives value between 0 and 1:

for (input / (uint.MaxValue/max)))

All 0.xxxx values should map to min (instead of getting min for numbers between 0 and 0.5, and min+1 for 0.5-1.5, etc) in order to see equal distribution in mapping (hence Math.Floor).

I think the function does it's job pretty well.  I was just looking to see if I can eliminate that if (input == uint.MaxValue) case since it's a weird edge case (for which result is max+1). This is also one thing that can be nitpicked from math standpoint - all members of the set will get mapped from n elements, while the last member will get mapped from n+1 elements (because of this special if case).

I would recommend considering this using more explicitly the Two-Point Equation of a Line. Your code is basically doing the same thing, but thinking more explicitly in terms of the underlying mathematics would help avoid errors or extra complication.

Here is how I would implement this:

public static uint LinearTransform(uint x, uint x1, uint y1, uint x2, uint y2) {
    ulong a = (ulong)(y2 - y1)*(x - x1);
    uint b = (x2 - x1);
    return (uint)(a/b) + (a%b*2 >= b)?1:0 + y2;
}

I don't actually know C#, so it is likely that the code is not syntactically correct, or breaks the style rules of the language, but it should get the point across.

The (a%b*2 >= b)?1:0 term is to compensate for the way integer division always truncates to zero. This will make it instead round up if the fractional part would be 1/2 or more, so we don't need to do any floating point computations.

This function can then be used to perform your original task by calling it as LinearTransform(input, 0, min, uint.MaxValue, max).


Python solution for the N-Queens challenge Here is my coded solution for the N-queens challenge

  The n-queens puzzle is the problem of placing n queens on an n×n chessboard such that no two queens attack each other.

The program outputs all valid chess boards, where the queen is represented by the digit 2.

Are there any improvements that can be made to my code?

One of the main things I want to improve is the counter. I have to make the variable global because I can't work out any other way to do it.

"""Find all possible chess-board combinations of size n*n with n queens,
where queens are represented by the digit 2"""
from collections import Counter
import numpy as np

def block_positions(chess_board, row, column, limit):
    """Fills all squares that can no longer contain a queen with the value -1
    There are a maximum of 8 directions that must be blocked from a given square"""
    # Block left
    for col in range(column - 1, -1, -1):
        chess_board[row, col] = 0
    # Block right
    for col in range(column + 1, limit):
        chess_board[row, col] = 0
    # Block up
    for rw in range(row - 1, -1, -1):
        chess_board[rw, column] = 0
    # Block down
    for rw in range(row + 1, limit):
        chess_board[rw, column] = 0
    # Block L up-diag
    rw = row
    col = column
    while rw > 0 and col > 0:
        rw -= 1
        col -= 1
        chess_board[rw, col] = 0
    # Block L down-diag
    rw = row
    col = column
    while rw < limit - 1 and col > 0:
        rw += 1
        col -= 1
        chess_board[rw, col] = 0
    # Block R up-diag
    rw = row
    col = column
    while rw > 0 and col < limit - 1:
        rw -= 1
        col += 1
        chess_board[rw, col] = 0
    # Block R down-diag
    rw = row
    col = column
    while rw < limit - 1 and col < limit - 1:
        rw += 1
        col += 1
        chess_board[rw, col] = 0
    return chess_board

def initialise_board(num):
    """Build the empty board"""
    board = np.ones(num * num).reshape(num, num)
    return board

def valid_boards(board, row, num):
    """Find all valid N-queen boards"""
    global counter
    while row < num:
        indices = [index for index in range(num) if board[row, index] == 1]
        if indices == []:
            return False
        for index in indices:
            old_board = board.copy()
            board[row, index] = 2
            board = block_positions(board, row, index, num)
            is_possible = valid_boards(board, row + 1, num)
            board = old_board
            if not is_possible and index == indices[-1]:
                return False
    flattened = Counter(board.flatten())
    if flattened[2] == num:
        print(board)
        print()
        counter += 1

if __name__ == "__main__":
    counter = 0
    num = 5
    board = initialise_board(num)
    valid_boards(board, row=0, num=num)
    print(counter, "solutions")
    print("Finished")

Separate generation and presentation

valid_boards both generates the solutions, prints and counts them. Better would be to have 1 method to generate the solutions, another to present the result. The simplest way to do this is to make a generator that yields the solutions, and another method that prints and counts them

reversed

reversed(range(row)) is a lot more clear than range(row - 1, -1, -1)

magical values

you use 0 for blocked, 1 for free and 2 for an occupied tile. defining these as global constants would be a lot clearer

BLOCKED = 0
FREE = 1
OCCUPIED = 2

and then use if for example

chess_board[row, col] = BLOCKED

board size

You use num and limit for the same value, the size of the board. Lat's just call this boardsize

DRY

In block_positions you do a lot of repetition for the different directions to block. More clear would be to use a generator that yields all the neighbour of a tile, and use the coordinates yielded to block those

To do this you need a method to check whether a coordinate is valid:

def valid_coord(coordinate, boardsize=5):
    row, column = coordinate
    return 0 <= row < boardsize and 0 <= column < boardsize

then using itertools.takewhile, functools.partial and itertools.count

def neighbours(row, column, boardsize):
    validator = partial(valid_coord, boardsize=boardsize)
    yield from ((row, i) for i in range(0, column))
    yield from ((row, i) for i in range(column + 1, boardsize))
    yield from ((i, column) for i in range(0, row))
    yield from ((i, column) for i in range(row + 1, boardsize))
    yield from takewhile(validator, ((row - i, column - i) for i in count(1)))
    yield from takewhile(validator, ((row + i, column + i) for i in count(1)))
    yield from takewhile(validator, ((row - i, column + i) for i in count(1)))
    yield from takewhile(validator, ((row + i, column - i) for i in count(1)))

list(neighbours(2, 3, boardsize=5))

[(2, 0), (2, 1), (2, 2), (2, 4), (0, 3), (1, 3), (3, 3), (4, 3), (1, 2), (0, 1), (3, 4), (1, 4), (3, 2), (4, 1)]

Then the block_positions suddenly becomes very simple

def block_positions(chess_board, row, column, boardsize):
    """Fills all squares that can no longer contain a queen with the value -1
    There are a maximum of 8 directions that must be blocked from a given square"""
    for x, y in neighbours(row, column, boardsize):
        chess_board[x, y] = BLOCKED

This method doesn't need to return the board. The standard library's convention is that if a method changes the object, it doesn't return something. If it returns a new object, that is returned.

By just changing the neighbours, you can easily change this to find the solutions for [related puzzles(https://en.wikipedia.org/wiki/Eight_queens_puzzle#Related_problems)

numpy

You hardly use any of the numpy strengts, but even then this can be simpler
You can pass a tuple to np.one, so initialise_board  can be as simple as

def initialise_board(num):
    """Build the empty board"""
    return  np.ones((num, num)) * FREE

Counting the number of places queens can be simpler too: (board == OCCUPIED).sum() == num instead of np.flatten and Counter

generator

Now the valid_boards method both generates the solutions, prints them and augments the counter. Simpler would be to have them just generate the solutions, and have something else take care of the printing and counting.

Slightly adapting your algorithm to a generator approach:

def add_queen(board, row, boardsize):
    if row == boardsize:
        yield board
        return
    free_columns = (index for index in range(boardsize) if board[row, index] == FREE)
    for column in free_columns:
        new_board = board.copy()
        new_board[row, column] = OCCUPIED
        block_positions(new_board, row, column, boardsize)
        yield from add_queen(new_board, row + 1, boardsize)

and call it like this:

if __name__ == "__main__":
    boardsize = 5
    board = initialise_board(boardsize)
    for solution_count, solution in enumerate(
        add_queen(board, row=0, boardsize=boardsize), 1
    ):
        print(solution)
        print()
    print(f"{solution_count} solutions")


Export sql data to xml and csv I have the below code that takes the sql data and exports it to xml or csv. The xml is processing at good speed but my csv sometimes takes 1 hour depending on how many data rows it exports. My question is my code effective writing the csv or it can be improved for speed? Has around 40 columns and x rows which can be over 10k.

public static void SqlExtract(this string queryStatement, string xFilePath, string fileName)
        {
            string connectionString = @"Data Source=ipaddress; Initial Catalog=name; User ID=username; Password=password";
            using (SqlConnection _con = new SqlConnection(connectionString))
            {
                using (SqlCommand _cmd = new SqlCommand(queryStatement, _con))
                {
                    using (SqlDataAdapter _dap = new SqlDataAdapter(_cmd))
                    {
                        if (fileName == "item1" || fileName == "item2" || fileName == "item3")
                        {
                            DataSet ds = new DataSet("FItem");
                            _con.Open();
                            _dap.Fill(ds);
                            _con.Close();
                            FileStream fs = new FileStream(xFilePath, FileMode.Create, FileAccess.Write, FileShare.None);
                            StreamWriter writer = new StreamWriter(fs, Encoding.UTF8);
                            ds.WriteXml(writer, XmlWriteMode.IgnoreSchema);
                            fs.Close();
                            StringWriter sw = new StringWriter();
                            ds.WriteXml(sw, XmlWriteMode.IgnoreSchema);
                            string OutputXML = sw.ToString();
                            OutputXML = OutputXML.Replace("Table", "Item");
                            System.IO.File.WriteAllText(xFilePath, OutputXML);
                        }
                        else
                        {
                            DataTable table1 = new DataTable("Table1");
                            _con.Open();
                            _dap.Fill(table1);
                            _con.Close();
                            string exportCSV = string.Empty;
                            foreach (DataRow row in table1.Rows)
                            {
                                int i = 1;
                                foreach (DataColumn column in table1.Columns)
                                {
                                    if (row[1].ToString() == "002" && i > 41 || row[1].ToString() == "END" && i > 4)
                                    {
                                        //do nothing
                                    }
                                    else
                                    {
                                        if (i > 1)
                                        {
                                            exportCSV += ";";
                                        }
                                        exportCSV += row[column.ColumnName].ToString();
                                    }
                                    i++;
                                }
                                exportCSV += "\r\n";
                            }
                            //Write CSV
                            File.WriteAllText(xFilePath, exportCSV.ToString());
                        }
                    }
                }
            }
        }

Some quick remarks:

Don't start variable names with underscores unless they're class-wide private ones.
Local variables should be camelCased. OutputXML doesn't follow that rule.
Split your code into smaller methods that don't intermix data retrieval and file writing. Your method does multiple things, depending on some (odd) logic. Even its name doesn't make sense: it's called SqlExtract yet it generates CSV or XML files depending on seemingly arbitrary conditions.
Don't hardcode your connection string. Put it in a .config file and access it via the ConfigurationManager.
Why do _con.Open(); and _con.Close();? The using takes care of this.
Don't loop through table1.Rows. Instead have a method that transforms a DataRow row into a csv line and use that to construct a IEnumerable<T>, which you then can use in combination with string.Join(). Something like var csvContents = string.Join(Environment.NewLine, table1.Rows.Cast<DataRow>().Select(x => DataRowToCsvLine(x)));. That's one line (instead of 20) which expresses far better what your code does (and yes, the DataRowToCsvLine method is also a couple of lines, but it doesn't pollute the main logic).
row[1].ToString() == "END": are you sure it is always going to be "END"? Or is it possible it can be "end"? Consider using string.Equals() instead, which allows for case insensitive comparisons. Same for fileName == "item1" etc.
Properly dispose of StringWriter with a using block. Example: https://social.msdn.microsoft.com/Forums/vstudio/en-US/7d949d5c-a41c-4dc7-bbcb-429761f851d1/is-calling-flush-and-close-necesary-for-stringwriter-and-xmltextwriter?forum=netfxbcl

None of the above might improve the speed of your code, but it will make it much more maintainable and consistent.


class - tweet - 0 - to_do This class sends "tweet" data to the client in the form of a custom markup language.  How can it be improved?

It is used in Ajax call and in previous post people have suggested not to echo the result..but this is my primary means of communicating with the server via Ajax responseText.

Also <tw_p> is used to denote a pass and is read by the client.  The markup looks like this.

field 1 | field 2 | field 3 | field 4 || field 1 | field 2 | field 3 | field 4 ||

It is called like this -

new tweet();

The client knows how render this into xhtml once it receives it.

/*tweet*/

class tweet extends post
  {
  function __construct()
    {
    parent::__construct();
    $email=$_SESSION['email'];
    $flname=$_SESSION['name'];
    $message=$this->_protected_arr['f4b'];
    $time=time();
    database::query("INSERT INTO tw VALUES ('$time','$flname','$message','$email')");
    $query_return = database::query("SELECT * FROM tw ORDER BY time DESC LIMIT 7");
    $b=0;
    $c='<tw_p>';
    while($a=mysqli_fetch_assoc($query_return))
      {
      if($b==0)
        {
        $c = $c . $a['email'] . "|" .  $a['fname'] . "|" . $a['time'] . "|" . $time . "|" . $a['message'];
        }
      else
        {
        $c = $c . "||" . $a['email'] . "|" .  $a['fname'] . "|" . $a['time'] . "|" . $time . "|" . $a['message'];
        }
      $b++;
      }
    echo $c;
    }
  }

So, this is the PHP that causes you to have your 'aml' processing here.

I think people must have confused you by saying not to echo.  You are correct, you do have to communicate by printing something out.  The best way to communicate with Javascript is by using JSON.  JSON stands for JavaScript Object Notation.

You should also check for success or failure from the database query.  Below is untested code (I have no idea what your database::query returns so my error check may not be appropriate):

$query_return = database::query("SELECT * FROM tw ORDER BY time DESC LIMIT 7");

if ($query_return === false)
{
    echo json_encode(array('Success' => false));
    return;
}

$results = array('Data'    => array(),
                 'Success' => true);

while($row = mysqli_fetch_assoc($query_return))
{
    $results['Data'][] = $row;
}

echo json_encode($results);

So now you will receive an array of rows from your database that your javascript code can deal with nicely.  It will be a valid javascript object so you can check it with:

if (response.Success)
{
    // Do stuff with response.Data here.  Probably a loop over the results.
}


Codewars: Sum Strings Sum Strings
Codewars kata.
It's supposed to be able to handle big integers.

Question

Given the string representations of two integers, return the string representation of the sum of those integers.
For example:

sumStrings('1','2') // => '3'

A string representation of an integer will contain no characters besides the ten numerals "0" to "9".

Solution
"use strict";
const reverseArr = s => s.split("").reverse();

function sumStrings(a, b) {
   [a, b] = [reverseArr(a), reverseArr(b)];
   let carry = 0;
   const arr = []
   const [mx, mn] = (a.length >= b.length) ? [a, b] : [b, a];
   mx.forEach((itm, idx) => {
      let sm = Number(itm) + (Number(mn[idx]) || 0) + carry;
      [sm, carry] = sm > 9 ? [sm%10, 1] : [sm, 0];
      arr.unshift(sm)
   })
   if (carry) arr.unshift(carry);
   return arr.join("").replace(/^(0+)/, "");
}

JavaScript style

Delimit cod blocks with {}. Eg if (foo) bar = 0; should be if (foo) { bar = 0; }

Use semicolons consistently!

Avoid Array.unshift as it is much slower than Array.push. If you know the size of the array pre-allocate the array new Array(size) and then pput items on the array via an index. (See example)

Be wary of destructuring as it can sometimes be rather slow (engines converting the right side to array (like) before assigning to the left). As destructuring becomes more main stream and JS engines stabilize new features destructuring has and will continue to improve in terms of performance.

Complexity.
Your function is too complex!
The complexity of this function is \$O(n)\$ where \$n\$ is the number of digits in the result. A well written function should thus have a performance that is related linearly to \$n\$ however your functions performance does not have this relationship, demonstrating a logarithmic performance (approx) \$O(n^2)\$ making it very slow for large numbers.
The reason is your use of Array.unshift. Each time you unshift a value to the array each item on the array needs to be moved up by one item. This is compounded every time the array size doubles as jS will then create a new array, and copy all the items to that array. As JS is a managed environment and memory is not cleaned up until the function exits or is forced by low memory your function not only has high time complexity but on theoretically infinite memory machines also has a storage complexity of \$O(n^2)\$
Rewrites
The rewrites are both \$O(n)\$ time and space complex, where \$n\$ is number of digits in the result (including leading zeros).
rather than revers the digits the code pre-allocates the result array with the required number of digits and then works from the least significant digit up.
Note that when the index into the strings a or b is less than 0 the coercion forced by the bitwise OR 0 | 0 will convert undefined to the number 0.
Ignoring leading zeros (assumes that there are no leading zeros for values over 0)
"use strict";
function sumStrings(a, b) {
    var carry = 0, i = a.length, j = b.length, k = i > j ? i : j;
    const res = new Array(k);
    while (i > 0 || j > 0) {
        const sum = (a[--i] | 0) + (b[--j] | 0) + carry;
        res[--k] = sum % 10;
        carry = (sum > 9 && 1) || 0;
    }
    return (carry ? carry : "") + res.join("");
}

The next version will accept leading zeros truncating the result string if there is no carry on the last digit. The truncation is at the last non zero digit encountered.
"use strict";
function sumStrings(a, b) {
    var carry = 0, i = a.length, j = b.length, k = i > j ? i : j, lastNonZero = k;
    const res = new Array(k);
    while (i > 0 || j > 0) {
        const sum = (a[--i] | 0) + (b[--j] | 0) + carry;
        lastNonZero = sum ? k : lastNonZero;
        res[--k] = sum % 10;
        carry = (sum > 9 && 1) || 0;
    }
    return carry ? carry + res.join("") : res.join("").slice(lastNonZero -1);
}

BigInt?
You could also have just parsed the two strings as BigInt but that is not the point of the exercise.
The summing algorithm is a basic component of computing, the algorithm will also work on any number base. Eg Binary base 2 Hex base 16.
With only minor modifications the function will handle any base and never actually add any numbers


Prime generator program SPOJ time limit exceed Problem Statement

Input
The input begins with the number t of test cases in a single line
(t<=10). In each of the next t lines there are two numbers m and n (1
<= m <= n <= 1000000000, n-m<=100000) separated by a space.
Output
For every test case print all prime numbers p such that m <= p <= n,
one number per line, test cases separated by an empty line.
Example
Input:
2
1 10
3 5

Output:
2
3
5
7

3
5

My Problem
I have tried very hard to optimize the code to my best, but still the online judge shows that I am exceeding the time limit. Is it possible to optimize the following code further or I should look for an alternative approach?
import sys
final=[]

for i in xrange(int(sys.stdin.readline())):
    start,end=[int(x) for x in sys.stdin.readline().split()]
    if start==1:
        start=2
    # sieve of eras** algorithm
    final.append(list(sorted(set(xrange(start,end+1)).difference(set((p * f) for p in xrange(2, int(end ** 0.5) + 2) for f in xrange(2, int(end/p) + 1))))))

# print final list items
for item1 in final:
    print("\n".join([str(x) for x in item1]))
    print('\n')

Re-organising the code

A quite easy thing to start with is to split the logic into a part collecting the input and a part performing computation from this input. This makes things clearer but also this might make testing easier and make some optimisations possible later on.

You can also take this chance to remove whatever is not needed :

i is not needed, the usage is to use _ for throw-away values in Python.
final is not needed : you populate it one element at a time and then you iterate on it to display the elements.
You don't need to recreate a new string to call join, you can use generator expressions.

Here's what I have at this stage :

#!/usr/bin/python

import sys

if False :
    # Test from standard input
    inputs = []
    for _ in xrange(int(sys.stdin.readline())):
        start,end=[int(x) for x in sys.stdin.readline().split()]
        if start==1:
            start=2
        inputs.append((start,end))
else:
    # Hardcoded test
    inputs = [(2,10),(5,7)]

for start,end in inputs:
    # sieve of eras** algorithm
    primes = (list(sorted(set(xrange(start,end+1)).difference(set((p * f) for p in xrange(2, int(end ** 0.5) + 2) for f in xrange(2, int(end/p) + 1))))))
    print("\n".join(str(p) for p in primes)+"\n")

Now, I can use inputs = [(2,10),(5,7),(9999985,10000000),(9999937,9999987)] for performance testing.

Computing things once

At the moment, you re-compute your sieve multiple times. You might as well juste do it once with the biggest max you can find.

def sieve_of_era(n):
    primes = [True]*(n+1)
    primes[0] = primes[1] = False
    for i in range(2, int(n**0.5)+1):
        if primes[i]:
            for j in range(i*i, n+1, i):
                primes[j] = False
    return primes

sieve = sieve_of_era(max(end for _,end in inputs))

Then, it's only a matter of collecting the information and printing it :

print "\n\n".join("\n".join(str(p) for p in xrange(start,end+1) if sieve[p]) for start,end in inputs)

A last remark

Your "sieve" considers 1 as a prime number and for that number, you added a hack to change 1 into 2 in the inputs. This is not required anymore and once it's removed, everything can be included in a single list comprehension.

The code becomes :

#!/usr/bin/python

import sys

inputs = [[int(x) for x in sys.stdin.readline().split()] for _ in xrange(int(sys.stdin.readline()))] if False else [(2,10),(5,7),(9999985,10000000),(9999937,9999987)]

def sieve_of_era(n):
    primes = [True]*(n+1)
    primes[0] = primes[1] = False
    for i in range(2, int(n**0.5)+1):
        if primes[i]:
            for j in range(i*i, n+1, i):
                primes[j] = False
    return primes

sieve = sieve_of_era(max(end for _,end in inputs))

print "\n\n".join("\n".join(str(p) for p in xrange(start,end+1) if sieve[p]) for start,end in inputs)

I haven't tried edge cases for the sieve function but it looks ok.


Removing ASCII "frame" around text I sometimes save nice fortune outputs to my Evernote. The data looks like this:

/ Abou Ben Adhem (may his tribe increase!)                    \
| Awoke one night from a deep dream of peace,                 |
| And saw, within the moonlight in his room,                  |
| Making it rich, and like a lily in bloom,                   |
| An angel writing in a book of gold.                         |
| Exceeding peace had made Ben Adhem bold,                    |
| And to the presence in the room he said,                    |
| "What writest thou?"  The vision raised its head,           |
| And with a look made of all sweet accord,                   |
| Answered, "The names of those who love the Lord."           |
| "And is mine one?" said Abou. "Nay not so,"                 |
| Replied the angel.  Abou spoke more low,                    |
| But cheerly still; and said, "I pray thee then,             |
| Write me as one that loves his fellow-men."                 |
| The angel wrote, and vanished.  The next night              |
| It came again with a great wakening light,                  |
| And showed the names whom love of God had blessed,          |
| And lo!  Ben Adhem's name led all the rest.                 |
\                 -- James Henry Leigh Hunt, "Abou Ben Adhem"

I am using the following regex:

$ cat abcd | sed -r 's/^[/|\\]\s?([-A-Za-z ()!,?".;'"'"']*)[/|\\]?/\1/'

Abou Ben Adhem (may his tribe increase!)
Awoke one night from a deep dream of peace,
...

How do I improve it?

Your regex looks complicated. What's more, it doesn't work:

$ cat fortune | sed -r 's/^[/|\\]\s?([-A-Za-z ()!,?".;'$a']*)[/|\\]?/\1/'
Abou Ben Adhem (may his tribe increase!)
Awoke one night from a deep dream of peace,
And saw, within the moonlight in his room,
Making it rich, and like a lily in bloom,
An angel writing in a book of gold.
Exceeding peace had made Ben Adhem bold,
And to the presence in the room he said,
"What writest thou?"  The vision raised its head,
And with a look made of all sweet accord,
Answered, "The names of those who love the Lord."
"And is mine one?" said Abou. "Nay not so,"
Replied the angel.  Abou spoke more low,
But cheerly still; and said, "I pray thee then,
Write me as one that loves his fellow-men."
The angel wrote, and vanished.  The next night
It came again with a great wakening light,
And showed the names whom love of God had blessed,
And lo!  Ben Adhem's name led all the rest.                 |
                -- James Henry Leigh Hunt, "Abou Ben Adhem"

See this stray | in the second line from the bottom? Honestly, I didn't analyze why your regex doesn't work because, as I said, it looks horribly complicated. But, first of all, cat is not necessary here because most of sed implementations - GNU, busybox, BSD can operate on a file directly and some of them even take i parameter which makes sed modify a file in place. That being said, here's how I would do it:

$ sed -r 's,^(/|\||\\) ,,g' fortune | sed -r 's,(\\|\|)$,,g'

I tested it with GNU sed 4.2.1 and with busybox version of sed on OpenWRT. The only thing that I don't like about it is usage of -r. There may be some basic sed implementations that don't implement extended regular expressions but they should be very rare.

EDIT:
No chaining with perl:

$ perl -pe 's,^(/|\||\\) (.+?)(\\|\|)?$,\2,' fortune


Run Length Encoding (RLE) and Move To Front (MTF) in C++ I have made two quite basic algorithms used for lossless data compression.

This is my RLE:

#include <string>

std::string rle(const std::string& input) // encode (aaaaeecd -> 4a2e1c1d)
{
    std::string output;

    int n = input.size();
    int i = 0;
    while(i < n)
    {
        int run_count = 1;
        int max_run_count = std::min(255, n - i); // condition: i + run_count < n, runcount <= 255
        while(run_count < max_run_count && input[i + run_count] == input[i])
            run_count++;
        output += run_count; // run_count is between 0 and 255 (unsigned char)
        output += input[i];
        i += run_count;
    }

    return output;
}

std::string irle(const std::string& input) // decode (4a2e1c1d -> aaaaeecd)
{
    std::string output;

    for(int i = 0; i < input.size(); i += 2)
    {
        int nb = (int)(unsigned char)input[i]; // range between 0 and 255
        for(int k = 0; k < nb; k++)
            output += input[i+1];
    }

    return output;
}

and here goes my MTF:

#include <string>
#include <deque>
#include <list>
#include <algorithm>

/*
if alphabet A = {'a', 'b', 'c'}
    then string "accbc" is encoded 02021
    (a is at index 0, alphabet remains {'a', 'b', 'c'}, outputs 0 ;
     c is at index 2, alphabet becomes {'c', 'a', 'b'}, outputs 2 ;
     c is at index 0, alphabet remains {'c', 'a', 'b'}, outputs 0 ;
     b is at index 2, alphabet becomes {'b', 'c', 'a'}, outputs 2 ;
     a is at index 1, alphabet becomes {'a', 'b', 'c'}, outputs 1)
*/
std::string mtf(const std::string& input)
{
    std::string output;

    std::list<char> alphabet; // constant erase/insert
    for(int i = 0; i <= 255; i++)
        alphabet.push_back((char)i);

    for(char c: input)
    {
        auto c_it = find(alphabet.begin(), alphabet.end(), c); // find char c in the alphabet
        output += distance(alphabet.begin(), c_it); // output index of char c in the alphabet
        // move character to the front of the alphabet
        alphabet.erase(c_it);
        alphabet.insert(alphabet.begin(), c);
    }

    return output;
}

/*
reverse operation:
    for each number, outputs the character at the corresponding index in the alphabet
        then move this character to the front of the alphabet
        and keep going
*/
std::string imtf(const std::string& input)
{
    std::string output;

    std::deque<char> alphabet; // constant erase (begin) & operator[] ; insert (middle) is O(n)
    for(int i = 0; i <= 255; i++)
        alphabet.push_back((char)i);

    for(unsigned char n: input)
    {
        char temp_c = alphabet[n];
        output += temp_c;
        alphabet.erase(alphabet.begin() + n);
        alphabet.insert(alphabet.begin(), temp_c);
    }

    return output;
}

I know there are many possible improvments possible, especially for the RLE algorithm. I would just like a review on the C++ syntax I've used, especially on the structures I have chosen.

Looks quite nice, but let's get down to ripping it apart.

First your runlength-encoding:

Try to use the same type for size-variables / indices throughout:
std::basic_string<...> uses std::size_t.
That way you would also heal the one sign-mismatch.
Your comment about the encoded form // encode (aaaaeecd -> 4a2e1c1d) is wrong.
You probably meant: // encode (aaaaeecd -> \x04a\x02e\x01c\x01d)
You are wasting \x00 and thus the chance to encode runs of length 256.
(Yes, the comment // run_count is between 0 and 255 (unsigned char) is wrong.)
You are missing the header <algorithm>.

Next your Move-To-Front:

I suggest you move from std::list to std::array.
A list is a far too heavy data-structure, jumping every-which-way in memory kills your performance quite easily.
The std::deque is a bit better, but using a std::array would probably still handily beat it due to simplicity.


Custom iterator for a linked list class I have made a LinkedList class. This is a singly-linked-list and I want to make a forward_iterator for this class without using Boost. I have made the code and I want to know whether I have implemented it correctly. The source I referred to make this code is here.

template <class T>
struct node
{
    T data;
    node *next;
};

template <class T>
class LinkedList
{
    private :
    node<T> *start;
    unsigned int numElements;
    // Assume all functions are implemented
};

Iterator Code :

class iterator : public std::iterator<std::forward_iterator_tag,node<T>*>
{
    node<T>* itr;

    public :

    iterator (node<T>* temp) : itr(temp) {}
    iterator (const iterator& myitr) : itr(myitr.itr) {}
    iterator& operator++
    {
        itr = itr->next;
        return *this;

    }
    bool operator== (const iterator& rhs)
    {
        return itr == rhs.itr;

    }
    bool operator!= (const iterator& rhs)
    {
        return itr != rhs.itr;

    }
    T& operator*()
    {
        return itr->data;
    }

};

Is the above implementation correct? If not, then what changes should I make?  Also, does anything else need to be implemented?

Your iterator is missing a few important details:

You should provide the pre- and post-increment operators (++it and it++). Currently, you only have the pre-increment version.

It might also be good to provide the -> operator, since some people prefer the it->something syntax over the (*it).something one.

The comparison and dereference operators should be const. Remember Const Correctness.

The copy constructor is just performing a memberwise copy of the underlying data, so you don't need to provide one and can let the compiler default it.

The Standard Library containers always provide two flavors of iterators, the iterator type, pointing to mutable data, and the const_iterator type, pointing to immutable data. It is easy to adapt your class to support both by providing a conversion operator and inheriting from std::iterator (see the following example).

Decide which course of action should be taken when incrementing and dereferencing an invalid iterator. E.g.: list.end()++;. Should it trigger an assertion? Throw an exception? Do nothing as it is now? I would at least assert to help the debugging process. You might find exceptions more appropriate in your context.

The above points applied to your code:
#include <cassert>      // assert
#include <cstddef>      // ptrdiff_t
#include <iterator>     // iterator
#include <type_traits>  // remove_cv
#include <utility>      // swap

template
<
    class Type,
    class UnqualifiedType = std::remove_cv_t<Type>
>
class ForwardIterator
    : public std::iterator<std::forward_iterator_tag,
                           UnqualifiedType,
                           std::ptrdiff_t,
                           Type*,
                           Type&>
{
    node<UnqualifiedType>* itr;

    explicit ForwardIterator(node<UnqualifiedType>* nd)
        : itr(nd)
    {
    }

public:

    ForwardIterator()   // Default construct gives end.
        : itr(nullptr)
    {
    }

    void swap(ForwardIterator& other) noexcept
    {
        using std::swap;
        swap(itr, other.iter);
    }

    ForwardIterator& operator++ () // Pre-increment
    {
        assert(itr != nullptr && "Out-of-bounds iterator increment!");
        itr = itr->next;
        return *this;
    }

    ForwardIterator operator++ (int) // Post-increment
    {
        assert(itr != nullptr && "Out-of-bounds iterator increment!");
        ForwardIterator tmp(*this);
        itr = itr->next;
        return tmp;
    }

    // two-way comparison: v.begin() == v.cbegin() and vice versa
    template<class OtherType>
    bool operator == (const ForwardIterator<OtherType>& rhs) const
    {
        return itr == rhs.itr;
    }

    template<class OtherType>
    bool operator != (const ForwardIterator<OtherType>& rhs) const
    {
        return itr != rhs.itr;
    }

    Type& operator* () const
    {
        assert(itr != nullptr && "Invalid iterator dereference!");
        return itr->data;
    }

    Type& operator-> () const
    {
        assert(itr != nullptr && "Invalid iterator dereference!");
        return itr->data;
    }

    // One way conversion: iterator -> const_iterator
    operator ForwardIterator<const Type>() const
    {
        return ForwardIterator<const Type>(itr);
    }
};

// `iterator` and `const_iterator` used by your class:
typedef ForwardIterator<T> iterator;
typedef ForwardIterator<const T> const_iterator;

Note: In the example, I've assumed that the end of your list or an invalid iterator are marked by a null pointer. You'll need to change that if you are using some other method, such as a dummy sentry node.


Fill upper triangular matrix from a list I am looking a good way to fill up the elements of a lower triangular matrix from a list.

I currently have this:

PadRight[#, s] & /@
 Prepend[Take[elems, # + {1, 0}] & /@
   Partition[Accumulate[Range[0, s - 1]], 2, 1], {}]

where elems is the elements-list and s is its length.

Perhaps the best way is preallocation plus a clear, procedural Do-loop.  What do you think?

You can test the above with:

s = 5
elems = Range[s(s-1)/2]

Here are a few methods for your consideration and feedback.

This one uses the core of my "Dynamic Partition" function.  It is the fastest method I know for this problem.  Also, perhaps refactoring the code like this makes it more intelligible.

dynP[l_, p_] :=
 MapThread[l[[# ;; #2]] &, {{0}~Join~Most@# + 1, #} &@Accumulate@p]

#~PadRight~s & /@ {{}} ~Join~ dynP[elems, Range[s-1]]

This one is slightly shorter, using a different way to construct the indices, but also slightly slower, and perhaps less transparent.

#~PadRight~s & /@
 Prepend[
   elems~Take~# & /@
     Array[{1 + # (# - 1)/2, # (# + 1)/2} &, s - 1],
   {}
 ]

This one is terse, but slow.  Legibility is debatable.

SparseArray[Join @@ Table[{i, j}, {i, s}, {j, i - 1}] -> elems, s] // MatrixForm

Here is one I just came up with, and I am quite pleased with it.  I think it may be more understandable than most of the ones above.

Take[FoldList[RotateLeft, elems, Range[0, s - 1]] ~LowerTriangularize~ -1, All, s + 1]


A different implementation of Heap Sort The following implementation is a bit different form the standard implementations I have read. Is it correct?

#include <malloc.h>
#include <stdio.h>
void heapify(int[],int,int);
void swap(int a[],int i,int j){
    int temp=a[i];
    a[i]=a[j];
    a[j]=temp;
}
void heap_sort(int a[],int l,int r){
    for(int i=l;i<r;i++)
        heapify(a,l,r);
}
void heapify(int a[],int l,int r){//brings largest element to position l
    int size=(r-l+1);
    int i=size-1,larger;
    if(size%2){
        if(a[l+i]<a[l+i/2])
            swap(a,l+i,l+i/2);
        i--;}
    while(i>0){
        if(a[l+i]>a[l+i-1])
            larger=l+i-1;
        else
            larger=l+i;
        if(a[l+(i-1)/2]>a[larger])
            swap(a,larger,l+(i-1)/2);
        i-=2;
    }
}
void printArray(int arr[], int n)
{
   int i;
   for (i=0; i < n; i++)
       printf("%d ", arr[i]);
   printf("\n");
}

/* Driver program to test insertion sort */
int main()
{
   int arr[] = {12, 11, 13, 5, 6, 7};
    int arr_size = sizeof(arr)/sizeof(arr[0]);
    printf("Given array is \n");
    printArray(arr, arr_size);
    heap_sort(arr,0,arr_size-1);
    printf("\nSorted array is \n");
    printArray(arr, arr_size);
    return 0;
    return 0;
}

You have no extra spacing between functions except before the main() function, which has several returns.  I would put a single return between each function to make it clear where they begin and end without even looking.

You use spaces between variables and operators inconsistently, and you should be consistent:

printArray(arr, arr_size);
heap_sort(arr,0,arr_size-1);

for (i=0; i < n; i++)

There are others as well.  I would recommend using a space for readability.

I would put spaces around variables in function definitions as well:

void heapify(int[],int,int);

You don't need two return 0;s at the end of main().

While not absolutely necessary, it might help prevent bugs if you use braces around one-line ifs and loops:

for (i=0; i < n; i++)
    printf("%d ", arr[i]);

Your brace style here is rather peculiar:

if(size%2){
    if(a[l+i]<a[l+i/2])
        swap(a,l+i,l+i/2);
    i--;}

That final brace should be written on the line below.  I would use more spaces and braces as well:

if(size % 2) {
    if(a[l + i] < a[l + i / 2]) {
        swap(a, l + i, l + i / 2);
    }
    i--;
}


Multi-merge algorithm in C++ As you know there is an algorithm in C++ standard library called std::merge, that merges two sorted ranges into another range. I try to write another merge (called multi_merge) that gets multiple sorted ranges (one or more), and merged them in one.
The code is as follows:

#include <utility>
#include <vector>
#include <algorithm>
#include <iterator>

template<typename I>
using range = std::pair<I, I>;

template<typename I>
struct is_empty_range {
    bool operator()(const range<I>& x) const {
        return x.first == x.second;
    }
};

template<typename I, typename Compare>
struct range_compare {
    range_compare(Compare comp) : comp_{ comp } {}
    bool operator()(const range<I>& x, const range<I>& y) const {
        return comp_(*x.first, *y.first);
    }

    Compare comp_;
};

template<typename I, typename O, typename Compare>
O multi_merge(std::vector<range<I>> inputs, O output, Compare comp) {
    inputs.erase(std::remove_if(std::begin(inputs), std::end(inputs), is_empty_range<I>{}),
        std::end(inputs));

    if (inputs.empty())
        return output;

    while (inputs.size() != 1) {
        auto min = std::min_element(std::begin(inputs), std::end(inputs), range_compare<I, Compare>{comp});
        if (min->first != min->second) {
            *output = *min->first;
            ++min->first;
            ++output;
        }
        else {
            inputs.erase(min);
        }
    }

    return std::copy(inputs[0].first, inputs[0].second, output);
}

template<typename I, typename O>
O multi_merge(std::vector<range<I>> inputs, O output) {
    return multi_merge(inputs, output, std::less<typename std::iterator_traits<I>::value_type>{});
}

I want to know your opinions about this code, especially the way I'm passing list of ranges (a vector of pairs of iterators) to the algorithm. Do you recommend a more general way?

Here is a simple piece of code to test the algorithm:

#include <numeric>
#include <iostream>

int main() {
    constexpr size_t count = 10000;
    std::vector<int> v(count);
    std::iota(v.begin(), v.end(), 0);
    std::random_shuffle(v.begin(), v.end());

    using it_t = std::vector<int>::iterator;
    std::vector<range<it_t>> inputs;
    for (auto part = 0u; part <= count - 1000; part += 1000)
        inputs.emplace_back(v.begin() + part, v.begin() + part + 1000);

    for (auto r : inputs)
        std::sort(r.first, r.second);

    std::vector<int> output;
    output.reserve(count);

    multi_merge(inputs, std::back_inserter(output));

    std::cout << std::boolalpha << "output size is correct: "
        << (output.size() == count) << '\n';
    std::cout << std::boolalpha << "output is sorted: "
        << std::is_sorted(output.begin(), output.end()) << '\n';
}

 Godbolt Link

Efficiency

The algorithm is suboptimal. Selecting a min is linear in the number of ranges. Arranging them in a priority queue would make it logarithmic.
Correctness

Testing for (min->first != min->second) is done too late. min->first == min->second means that the empty range participated in the comparison. Which in turn means that the end of that range was dereferenced. UB is imminent.

Since

inputs.erase(std::remove_if(std::begin(inputs), std::end(inputs), is_empty_range<I>{}),
std::end(inputs));

guarantees that there are no empty ranges at the beginning, I recommend to rewrite the loop as

while (inputs.size() != 1) {
    auto min = std::min_element(std::begin(inputs), std::end(inputs), range_compare<I, Compare>{comp});
    *output = *min->first;
    ++min->first;
    ++output;

    if (min->first != min->second) {
        inputs.erase(min);
    }
}


Find duplicated text for tasks in MS Project My code in MS project VBA, in theory, doesn't do anything major.
It compares the tasks with each other, but it runs super slow.

One run through Temp lasts 15 secs. Try that with 1500-2000 tasks and the macro takes hours.

Sub Laczona()

Application.Calculation = xlManual
Application.ScreenUpdating = False

Dim Temp, Temp2 As Integer

For Temp = 1 To ActiveProject.Tasks.Count

For Temp2 = 1 To ActiveProject.Tasks.Count

If ActiveProject.Tasks(Temp).Text30 = ActiveProject.Tasks(Temp2).Text30 Then
If Temp <> Temp2 Then
ActiveProject.Tasks(Temp).Flag5 = 1
End If
End If

Next Temp2
MsgBox "Praca: " & Temp
Next Temp

MsgBox "Prace Laczone Uzupelnione"
Application.ScreenUpdating = True
Application.Calculation = xlAutomatic

End Sub

Addidional explanation:
The code runs in MS Project 2013. Its purpose is to check whether two tasks have the same text in field Text30. If that is the case, the code sets flag5 to 1.

If I understand correctly, you are testing to see if there are any duplicate entries in Text30 and if you find a duplicate setting flag 5 to 1.

In this case you are duplicating at least half of the tests that you are conducting because Task(1).Text(30) = Task(2).Text30 is the same comparison as Tasks(2).Text30 = Task(1).Text30.

So you can cut down on the number of tests that you do by eliminating tests that you have already performed.  This is done by starting the inner loop at the current value of the outer loop.

e.g.

For Temp2 = 1 To ActiveProject.Tasks.Count

should be revised to

For Temp2 = Temp To ActiveProject.Tasks.Count

However an even quicker way would be to use a Scripting.Dictionary. I don't have project installed on my PC but even so if the code below doesn't work it should at least point you in the right direction

Sub Lacunza()

    Dim myUniqueText30 As Scripting.Dictionary
    Set myUniqueText30 = New Scripting.Dictionary

    Dim myTask As Long
    For myTask = 1 To ActiveProject.Tasks.Count

        With ActiveDocument.Tasks(myTask)

            If myUniqueText30.Exists(.Text30) Then

                .Flag5 = 1

            Else

                myUniqueText30.Add Key:=.Text30, Item:=myTask

            End If

    Next

    ' As this point you have a scripting dictionary which will return the task id
    ' where the first occurence of a particular Text30 value was found.

End Sub


POSIX-ly finding a specific Java process In this code I find a list all of the running java processes and give the below function a name to look for, it will do its best. But since I find my approach a little too ugly, could some POSIX shell script writer have a look and possibly give me some simplification recommendations?

#!/bin/sh

# here is an ugly constant vuze java process name
readonly vuze_java_process_name='org.gudy.azureus2.ui.swt.Main'

is_java_program_running()
# expected arguments:
# $1 = java program name
{
    # there will always be at least one java process:
    # jdk.jcmd/sun.tools.jps.Jps, which is actually
    # the program giving us the list of java processes
    java_process_list=$( jps -l | awk '{print $2}' )

    # this behaves strangely if there is zero processes (needs verification)
    # but since there is always at least one, no problem here
    java_process_list_count=$(( $( printf '%s\n' "${java_process_list}" | wc -l ) ))

    # set the result value as if we did not find it
    result=false

    # POSIX-ly simulate FOR loop
    i=1; while [ "${i}" -le "${java_process_list_count}" ]
    do
        # here we take one line from the list on $i position
        java_process_entry=$( echo "${java_process_list}" | sed --posix --quiet "${i}{p;q}" )
        # compare the given process entry with given java program name
        if [ "${java_process_entry}" = "${1}" ]
        then
            # set the result value
            result=true
            # end this loop
            break
        fi
        # increase iterator
        i=$(( i + 1 ))
    done

    # depending on if we found vuze process running,
    # return positive or negative result value
    if [ "${result}" = true ]
    then
        return 0
    else
        return 1
    fi
}

###
### EXAMPLE on Vuze
###

# keep Vuze alive forever, check in 5 seconds interval
while true
do
    sleep 5s

    if ! is_java_program_running "${vuze_java_process_name}"
    then
        my_date_time=$(date +%Y-%m-%d_%H:%M:%S)
        printf '%s  %s\n' "${my_date_time}" "(re-)starting Vuze"
        ( /home/vlastimil/Downloads/vuze/vuze > /dev/null 2>&1 & )
    fi
done

Instead of simulating a for (( expr1 ; expr2 ; expr3 )) loop, use a POSIX

    for java_process_entry in $java_process_list

Notice that now you don't need to compute the number of lines, neither invoke sed.

There is noting wrong with early return. return 0 as soon as the desired process is found.

There is no need to store the entire list in a variable, neither invoke awk. Process the jps output sequentially.

All that said, consider

    is_java_program_running()
    {
        jps -l | while read pid java_process_entry; do
            if [ $java_process_entry = "${1}" ]; then return 0; fi
        done
        return 1
    }


Generic Implementation for Api call in c# i have created one generic implementation for api call from all over my application.

 public class HttpClientService<TResult> : IDisposable where TResult : class
{
    private HttpClient client = new HttpClient();
    public HttpClientService(string baseUrl)
    {
        client.BaseAddress = new Uri(baseUrl);
        client.DefaultRequestHeaders.Accept.Clear();
        client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));
        client.Timeout = new TimeSpan(0, 30, 0);
    }
    public TResult GetAPI(string url, Dictionary<string,string> urlParameters = null, Dictionary<string, string> headers = null)
    {
        string parameters = string.Empty;

        if (urlParameters != null)
        {
            parameters = BuildURLParametersString(urlParameters);
            url = string.IsNullOrEmpty(parameters) ? url : url + parameters;
        }

        if (headers != null)
        {
            AddHeaders(headers);
        }

        var response = Task.Run(() => client.GetAsync(url)).Result;
        var apiResult = response.Content.ReadAsStringAsync().Result;
        var result = JsonConvert.DeserializeObject<TResult>(apiResult);
        return result;
    }
    public TResult PostAPI(string url, Dictionary<string, string> formParameters = null, string jsonString = "", Dictionary<string, string> headers = null)
    {
        HttpContent contentPost = null;
        if (headers != null)
        {
            AddHeaders(headers);
        }
        if (formParameters != null)
        {
            var formContent = new FormUrlEncodedContent(formParameters);
            contentPost = formContent;
        }
        else if (!string.IsNullOrEmpty(jsonString))
        {
            var content = new StringContent(jsonString, Encoding.UTF8, "application/json");
            contentPost = content;
        }
        var response = Task.Run(() => client.PostAsync(url, contentPost)).Result;
        var apiResult = response.Content.ReadAsStringAsync().Result;
        var result = JsonConvert.DeserializeObject<TResult>(apiResult);
        return result;
    }
    private String BuildURLParametersString(Dictionary<string, string> parameters)
    {
        UriBuilder uriBuilder = new UriBuilder();
        var query = System.Web.HttpUtility.ParseQueryString(uriBuilder.Query);
        foreach (var urlParameter in parameters)
        {
            query[urlParameter.Key] = urlParameter.Value;
        }
        uriBuilder.Query = query.ToString();
        return uriBuilder.Query;
    }
    private void AddHeaders(Dictionary<string, string> headers)
    {
        foreach (var header in headers)
        {
            if (!string.IsNullOrEmpty(header.Value))
            {
                client.DefaultRequestHeaders.Add(header.Key, header.Value);
            }
        }
    }
    public void Dispose()
    {
        client.Dispose();
    }
}

and from my application used like below

using (var clientService = new HttpClientService<GeoCodeResponse>("https://maps.googleapis.com"))
{
         var param = new Dictionary<string, string>();
         param.Add("latlng", $"{latitude},{longitude}");
         param.Add("result_type", "locality");
         param.Add("key", GoogleApiKey);

         var apiResult = clientService.GetAPI("maps/api/geocode/json", param);
}

please suggest me what i am implemented is currect or any better improvement for code any changes.

A few points that come to mind:

A lot of your code closely follows the underlying API of the HTTP client instance. It would be better to have a function that generates your client with its default settings (the ones you're setting in the constructor), then just use the appropriate methods on the HttpClient.
There arent many (any?) good use cases for calling Result on a task. You should make your method async and await the task instead.
Your use of Task.Run is redundant. You're wrapping one task inside another.
What happens if there is an error? Incorrect URI, no network etc...


Modify and import as numpy.ndarray a txt file in python To import the data contained into the file my_file.txt that have the form:

Label[0] = 0.980252
Label[1] = -nan
Label[2] = -nan
Label[3] = -nan
Label[4] = 0.664706
Label[5] = -nan
Label[6] = -nan
Label[7] = -nan
Label[8] = -nan
Label[9] = -nan
Label[10] = -nan
Label[11] = 0.800183
Label[12] = -nan
Label[13] = -nan
Label[14] = -nan
Label[15] = 0
Mean Data = 15

I wrote the following code:

import numpy as np

with open('myfile.txt', 'r') as file_txt_original:
    data = file_txt_original.read()

    data = data.replace('Mean data', '-1')
    data = data.replace('Label[', '')
    data = data.replace(']', '')
    data = data.replace(' = ', ', ')

    file_txt_original.close()

with open('new_file.txt', 'w') as file_txt_copy:

    file_txt_copy.write(data)
    file_txt_copy.close()

my_array = np.loadtxt('new_file.txt', delimiter=',')

It works but this to me seems still quite an tricky solution... Any suggestion to improve this code without doing so many replacement or without saving an additional structure?

You can concatenate the replace strings after you open the file, it will give better visibility

data = file_txt_original.read()
.replace('Mean data', '-1')
.replace('Label[', '')
.replace(']', '')
.replace(' = ', ', ')


Heap Implementation in C# I am learning fundamental data structures, and I want to know if this is a valid implementation of a heap. Can any C# features be used to improve or extend the implementation? In addition to heapsort, what are some applications of a heap?

class BinaryHeap
{

    /// <summary>
    /// A binary heap implementation used to sort an integer array.
    ///
    /// 1. The Max-Heapify and Min-Heapify methods maintain the heap properties.
    ///
    /// 2. The Build-Heap methods produce the heap from an unordered array.
    ///
    /// 3. The HeapSort methods sort the array in place and runs in O(n ln n) time.
    /// </summary>

    private int heapSize;

    public BinaryHeap()
    {
        heapSize = 0;
    }

    private int ParentIndex(int currentIndex)
    {
        return currentIndex / 2;
    }

    private int LeftIndex(int currentIndex)
    {
        return currentIndex * 2;
    }

    private int RightIndex(int currentIndex)
    {
        return currentIndex * 2 + 1;
    }

    // Building the heap
    #region
    public void BuildMaxHeap(int[] A)
    {
        heapSize = A.Length - 1;

        for(int i = A.Length/2; i >= 0; i--)
        {
            MaxHeapify(A, i);
        }

    }
    public void BuildMinHeap(int[] A)
    {
        heapSize = A.Length - 1;
        for (int i = A.Length / 2; i >= 0; i--)
        {
            MinHeapify(A, i);
        }
    }
    #endregion

    // Maintaining heap properties
    //      MaxHeapify: Ensure that parents are larger than children
    //      MinHeapify: Ensure that children are larger than parents
    #region
    public void MaxHeapify(int[] A, int i)
    {

        int leftIndex = LeftIndex(i);
        int rightIndex = RightIndex(i);

        int largestIndex = 0;

        // Check to see which node in the tree subset has the largest value
        if (leftIndex <= heapSize && A[leftIndex] > A[i])
        {
            largestIndex = leftIndex;
        }
        else
        {
            largestIndex = i;
        }
        if (rightIndex <= heapSize && A[rightIndex] > A[largestIndex])
        {
            largestIndex = rightIndex;
        }

        // Do not make any switches if the largest node is the parent
        if(largestIndex != i)
        {
            int temp = A[largestIndex];
            A[largestIndex] = A[i];
            A[i] = temp;
            MaxHeapify(A, largestIndex);
        }

    }
    public void MinHeapify(int[] A, int i)
    {

        int leftIndex = LeftIndex(i);
        int rightIndex = RightIndex(i);
        int smallest;

        if (leftIndex <= heapSize && A[leftIndex] < A[i])
        {
            smallest = leftIndex;
        }
        else
        {
            smallest = i;
        }

        if (rightIndex <= heapSize && A[rightIndex] < A[smallest])
        {
            smallest = rightIndex;
        }

        if (smallest != i)
        {
            int temp = A[i];
            A[i] = A[smallest];
            A[smallest] = temp;
            MinHeapify(A, smallest);
        }

    }
    #endregion

    // Heapsort
    #region
    public int[] AscendingHeapSort(int[] A)
    {

        // Ensure all parents are greater than their children
        BuildMaxHeap(A);

        for (int i = A.Length - 1; i >= 0; i--)
        {
            int temp = A[0];
            A[0] = A[i];
            A[i] = temp;
            heapSize--;
            MaxHeapify(A, 0);
        }

        return A;

    }

    public int[] DescendingHeapSort(int[] A)
    {

        // Ensure all parents are less than their children
        BuildMinHeap(A);

        for (int i = A.Length - 1; i >= 0; i--)
        {
            int temp = A[0];
            A[0] = A[i];
            A[i] = temp;
            heapSize--;
            MinHeapify(A, 0);
        }

        return A;
    }
    #endregion

}

C#/Design related comments

Use small-letters for passing arguments.
Put your comments (The <summary> ) about the class on top of the class definition, not the field.
The way of using # regions is not really done the way you do in C# usually. There should be a description of the region right after the region on same line.
Use C# comments styling when commenting classes, methods, their parameters.
There is not much sense in designing Heap class that stores just a single integer (heap size). You could simply have public static methods (utility methods) for that purpose, because it seems none of your methods really need to store a state
If you design this as a Heap, depends on use-cases you might want to design Push/Pop methods and store the underlying array in the class. Then having a class would make sense.
Try to use C# Generics instead of strongly defined int. This way you can make your code re-usable for other types (e.g. short) and/or for any object that implements IComparable (check out MSDN for more info on these).

Where to use Heap.

In addition to sorting you can use Heap as PriorityQueue.
If you add Push/Pop methods and slightly modify the implementation, then it can serve as a PriorityQueue, which allows you to add elements(any object that implements IComparable) in random order, and the higher/lower ones will get to the top/bottom of the queue. You can go even further by designing another method - Update(...), which can modify the priority of an element (technically you could implement it by removing the old value from queue and then adding the updated value back to the queue).

Didn't validate your code correctness, but you can/should validate it by testing on some regular and corner cases etc.


Quadratic equation solver in Python I use the module os specifically for Windows since I am familiar with Batch. Batch alone didn't allow me to execute more complicated mathematical functions, I began to use Python.

import os
import math
os.system('cls')
os.system('')
os.system('color a')
os.system('title Quadratic Equation Solver')
#This is used for presentation of the function for WINDOWS cmd
#------------------------------------------------
def QuadSolve():
    os.system('cls')
    print("         Quadratic Equation Solver 1")
    print("")
    print(" X = (-b +/- sqrtb^2 + sqrt4ac)/div/2a ")
    print("")
    print("    Quadratic equation: ax^2 + bx + c")
    print("")
    print("")
    print("")
    a = int(input(" What is the value of a? ---> "))
    b = int(input(" What is the value of b? ---> "))
    c = int(input(" What is the value of c? ---> "))
    firstsec = -1 * b #first second and third "sectors" of the expression
    secsec = math.sqrt(b ** 2 - 4 * a * c)
    thirdsec = 2 * a
    Plussl = firstsec + secsec #Pluss1 is where -b and root b^2 + 4ac are added to give a value of X
    X = Plussl / thirdsec
    Negsl = firstsec - secsec #Same purpose as Plussl but subtraction is used instead to give the other solution of X
    X2 = Negsl / thirdsec
    print("")
    print("         X=%d  OR  X=%d ") % (X, X2)
    os.system('pause >nul')
    QuadSolve()

QuadSolve()

I am very interested to know if anyone has any alternatives.

Repeating code infinitely

Right now, in order to have QuadSolver repeat forever, you're calling it recursively, by calling QuadSolver again inside of itself. Unlike functional languages, which are optimized for tail-call recursion, Python isn't. Once you've recursed in the same function 1000 times, Python will raise an error. You can technically set the recursion limit using sys.setrecursionlimit, but this is a very "hacky" way to do things.

The proper way to make any piece of code repeat for a variable amount is to use a while loop. This means that the body of your QuadSolver function should be wrapped in a while loop, like this:

def QuadSolver():
    while True:
        # Original code goes here.

By doing this your code gains a few good things:

It will properly execute forever.
It's faster.
It's more readable, e.g, it's clear to the reader that you're executing a chunk of code forever.

Integer input validation

Never trust the user to enter the correct input. For example, what if I ran your program and entered asghiopisdfiojgpfdj for the variable a? Your program would raise a ValueError and crash. You need to use a try/except block to catch a ValueError in case the user enters the wrong input. This means that this section of code here:

a = int(input(" What is the value of a? ---> "))
b = int(input(" What is the value of b? ---> "))
c = int(input(" What is the value of c? ---> "))

Should be wrapped in a try/except block, like this:

try:
    a = int(input(" What is the value of a? ---> "))
    b = int(input(" What is the value of b? ---> "))
    c = int(input(" What is the value of c? ---> "))

    # Original code below the above section goes here.
except ValueError:
    print("Please enter a valid integer.")

Now, if a user enters incorrect input, the program will alert them to this, and start over.

Proper string formatting

The % operator is deprecated as of Python 2.6. Instead, you should be using the str.format function instead. For example, this line:

print("         X=%d  OR  X=%d ") % (X, X2)

Would become this:

print("         X={x1}  OR  X={x2}".format(x1=X, x2=X2))

str.format supports the following styles:

Normal parameters:

"{} {}".format("Hello", "world")

Positional parameters:

"{1} {0}".format("world", "Hello")

Named parameters:

"{word1} {word2}".format(word1="Hello", word2="world")

You can do a great deal more with str.format, but I'll leave it up to you to figure that out, as there's too much for this one post.

Getting rid of the system calls

I find all of these lines of code to be absolutely horrid:

os.system('cls')
os.system('')
os.system('color a')
os.system('title Quadratic Equation Solver')
os.system('pause >nul')

Especially this one in particular:

os.system('pause >nul')

There are many things wrong here. For starters, the call to os.system has a long startup and "cooldown" period, e.g, it takes a fair amount of time. Secondly, all the commands you're running, like cls, color, or pause are very Windows-specific. What happens if someone wants to run this on a Linux-based system? Most of these system calls are purely stylistic, and unnecessary anyways.

On the matter of this line:

os.system('pause >nul')

I find it awful that bad C++ practices are leaking into Python. The method above is slow, insecure, and unreadable. If you want to pause until user input, just use the standard input operations of your language. In the case of Python, that would just look like this:

input()

If you're still using Python 2.x for some reason, it would look like this:

raw_input()

Proper style

Unlike many languages, Python has an official style guide, PEP8, and you're violating a fair amount of the guidelines set there.

Variable names should be in snake_case, and UPPER_SNAKE_CASE if they're constants.
Function names should only be in snake_case.
Classes should be in PascalCase.
You should have two spaces between top-level code/functions/classes.

While the following items aren't actually in PEP8, they're kind of a code smell, and should be removed.

Separator comments, like the one below this should be removed. They're useless and look bad in your code.

#------------------------------------------------

Inline comments on the end of code lines should be removed. If you're having to put comments on the end of lines of code, then you probably aren't writing the code in a clear enough way. If you feel the need to put comments at the end of lines of code, then chances are, those lines should be re-written.

It's also worth mentioning docstrings as well. Docstrings are a convenient way to describe your functions and classes in Python, much like XML documentation in Visual Studio. A typical function docstring generally looks something like this:

def my_function( ... ):
    """Brief description of the function.

    Detailed description of the function.

    argument_name -- Argument description.
    """

If we take this and apply it to your QuadSolver function, we get something like this:

def quad_solver():
    """Solve the quadratic equation.

    This function takes user input, a, b, and c, and
    runs it through the quadratic equation to "solve"
    it.
    """
    ...


PHP form to database fit for purpose This code is intended to take input from a form and append it to a database table. The same data is sent to a separate function for subsequent mailing to the respondent. I'm self-taught and it works so overall I'm quite happy.

It just starts to seem a little flabby around the binding and creation of the $mail_params. I imagine there's no real performance cost here but are there more efficient ways of setting multiple variables?

As my first outing into MySQLi functions I would be grateful for a weather eye on the prepared statement and its suitability in preventing SQL injection.

function submit_form ($db_params) {

// connect
$mysqli = new mysqli($db_params['host'], $db_params['username'],
                     $db_params['password'], $db_params['dbname']);

// check
if ($mysqli->connect_errno) {
    echo "Failed to connect to MySQL: (" . $mysqli->connect_errno . ") " . $mysqli->connect_error;
    return false;
}

// prepare
if (!($stmt = $mysqli->prepare("INSERT INTO interface_response(issue,
                                                               comment,
                                                               email,
                                                               reporting_organisation,
                                                               reporting_dept_prac,
                                                               causing_organisation,
                                                               causing_dept_prac)
                                VALUES (?, ?, ?, ?, ?, ?, ?)"))) {
    echo "Prepare failed: (" . $mysqli->errno . ") " . $mysqli->error;
    return false;
}

// bind
if (!$stmt->bind_param("sssssss", $issue, $comment, $email,
                                  $reporting_organisation, $reporting_dept_prac,
                                  $causing_organisation, $causing_dept_prac)) {
    echo "Binding parameters failed: (" . $stmt->errno . ") " . $stmt->error;
    return false;
}

$email = $_POST['email'];
$mail_params['email'] = $email;

$issue = $_POST['issue'];
$mail_params['issue'] = $issue;

$comment = $_POST['comment'];
$mail_params['comment'] = $comment;

$reporting_organisation = $_POST['reporting_organisation'];
$mail_params['reporting_organisation'] = $reporting_organisation;

$reporting_dept_prac = department_or_practice($_POST['reporting_department'],
                                              $_POST['reporting_practice']);
$mail_params['reporting_dept_prac'] = $reporting_dept_prac;

$causing_organisation = $_POST['causing_organisation'];
$mail_params['causing_organisation'] = $causing_organisation;

$causing_dept_prac = department_or_practice($_POST['causing_department'],
                                            $_POST['causing_practice']);
$mail_params['causing_dept_prac'] = $causing_dept_prac;

// execute
if (!$stmt->execute()) {
    echo "Execute failed: (" . $stmt->errno . ") " . $stmt->error;
    return false;
} else {
    return send_mail($mail_params);
}

// close
$stmt->close();

$mysqli->close();

}

First of, yes, prepared statements are the way to go to prevent SQL injection (and you are using them correctly).

Reducing Code Length

The assignment of POST values to a variable as well as an array takes a lot of space, and it makes your code hard to read. There are a lot of solutions for this, one would be to move the assignment before bind, and then use the array values for binding:

$mail_params['email'] = $_POST['email'];
$mail_params['issue'] = $_POST['issue'];
[...]
if (!$stmt->bind_param("sssssss", [...], $mail_params['email'], $mail_params['issue'], [...])) {
    [...]
}

If you create one more function, it might also be a bit better structured:

submitCommentForm() {
    [extract POST values]
    insertComment([extracted post values]);
    sendMail([same extracted post values]);
    [render result]
}

insertComment($db, $comment_values) {
    [the database stuff]
}

sendMail($comment_values) {
    [send mail]
}

Misc

Your spacing and indentation isn't ideal, which makes your code a bit hard to read. You can probably use any IDE to fix this.
Don't echo in a function (it makes your code very inflexible). Just throw an exception.
Don't create your mysqli object in each function you need it, but pass it to the function as an argument (that way, you avoid duplicate code, and you make your functions more flexible and testable).


HTTP Performance Test in C# I wrote a script to test how long it takes to make requests against a url.  It takes a list of urls, an array of concurrent requests, and a number of times to attempt the test.  How could I improve on this?

//https://www.nuget.org/packages/RestSharp

string baseUrl = "http://localhost";
void Main()
{
    var urls = new string[] {
        "/some/end/point",
    };

    var concurrentRequests = new int[]{ 1, 5, 25, 50, 100, 500, 1000};
    var numberOfTurns = 10;
    RunTests(urls, concurrentRequests, numberOfTurns);

}

private void RunTests(string[] urls, int[] concurrentRequests,  int numberOfTimes = 1)
{
    for(var i = 0; i < numberOfTimes; i++)
    {
        RunFor(urls, concurrentRequests);
    }
}
private void RunFor(string[] urls, int[] concurrentRequests)
{
    concurrentRequests.ToList().ForEach( e =>
    {
         RunTest(urls, e);
    });
}
private void RunTest(string[] sites, int iterations, string description = "")
{
    var action = GetAction();

    var watch = new Stopwatch();
   // Construct started tasks
   Task<bool>[] tasks = new Task<bool>[sites.Count()];
   watch.Start();

   for(int j = 0; j < iterations; j++)
   {
        for (int i = 0; i < sites.Count(); i++)
        {
            tasks[i] = Task<bool>.Factory.StartNew(action, sites[i]);
        }
   }
   try
   {
       Task.WaitAll(tasks);
   }
   catch (AggregateException e)
   {
       Console.WriteLine("\nThe following exceptions have been thrown by WaitAll()");
       for (int j = 0; j < e.InnerExceptions.Count; j++)
       {
           Console.WriteLine("\n-------------------------------------------------\n{0}", e.InnerExceptions[j].ToString());
       }
   }
   finally
   {
    watch.Stop();
    Console.WriteLine("\"{0}|{1}|{2}\", ",sites.Count(), iterations, watch.Elapsed.TotalSeconds);
   }
}
private Func<object, bool>  GetAction()
{
    baseUrl = baseUrl.Trim('/');
    return (object obj) =>
   {
        var str = (string)obj;
        var client = new RestClient(baseUrl);
        client.Authenticator = new NtlmAuthenticator();
        var request = new RestRequest(str, Method.GET);
        request.AddHeader("Accept", "text/html");
        var response = client.Execute(request);
        return (response != null);
   };
}

You're not always using var in what I think of as a good way. We should only use the keyword when it's obvious what type the variable is. Consider this line.

Task<bool>[] tasks = new Task<bool>[sites.Count()];

It's completely obvious what tasks is because you're newing it up right there on that line. There's no reason not to simplify by using var.

But here, you have the opposite problem.

var response = client.Execute(request);

It is not obvious what the return type is, so you shouldn't be using var.


Simple multiple producer and single consumer concurrency pattern Anyone please review and share your feedback for the following code.
Multiple producers generate random integer numbers and a single consumer receives it to display only odd numbers out of it.

package main

import (
"fmt"
"time"
"math/rand"
"sync"
)

var msgs = make(chan int)
var dummy = make(chan bool)
var wg sync.WaitGroup

func main() {
    defer close(msgs)
    defer close(dummy)
    numberOfProducers := 10
    go func() {
        for i := 0; i < numberOfProducers ; i++ {
            go produce() // Multiple producers
        }
        dummy <- true
    }()
    go consume() // Single consumer go routine
    <- dummy
}

func produce() {
     wg.Add(1)
     msgs <- getRandomIntegerNumber()
}

// Single consumer
func consume () {
    for {
      msg := <-msgs
      if ( msg % 2 != 0) {
         fmt.Printf(" The number %d is odd \n", msg);
      }
      wg.Done()
    }
}

func getRandomIntegerNumber() int {
     rand.Seed(time.Now().UnixNano())
     return rand.Intn(1000)
}

So currently the WaitGroup is not being used to wait for the producing to complete.  This means that it is possible for this programme to run without producing any results as each go produce() is done in it's own go routine so true can be sent immediately to dummy if the main go routine gets all the CPU time thus main exits with nothing being printed.

Personally I would use the WaitGroup to do the waiting and then close the msgs channel to indicate to the consume that it is done and use a further WaitGroup to then indicate to main that consume is done.

In addition to the concurrency parts, I also wouldn't use global variables but instead pass them into the functions that use them.   Putting this together then you get:

package main

import (
    "fmt"
    "math/rand"
    "sync"
    "time"
)

func main() {
    msgs := make(chan int)

    // Start the consuming go routine before producing so that we can then just wait for the producing
    // to complete on the main go routine rather than having to have a separate go routine to wait to
    // close the channel.
    var consumeWG sync.WaitGroup
    consumeWG.Add(1)
    go consume(msgs, &consumeWG)

    var produceWG sync.WaitGroup
    numberOfProducers := 10
    for i := 0; i < numberOfProducers; i++ {
        produceWG.Add(1)
        go produce(msgs, &produceWG) // Multiple producers
    }

    // Wait for producing to complete then tell the consumer by closing the channel.
    produceWG.Wait()
    close(msgs)

    consumeWG.Wait()
}

func produce(msgs chan int, wg *sync.WaitGroup) {
    defer wg.Done()
    msgs <- getRandomIntegerNumber()
}

// Single consumer
func consume(msgs chan int, wg *sync.WaitGroup) {
    // Range of msgs, this will consume all messages in the channel then exit when the channel is closed.
    // This provides communication between the go routines when completion has happened as well as not
    // leaking the consuming go routine as would happen with an forever loop.
    for msg := range msgs {
        if msg%2 != 0 {
            fmt.Printf(" The number %d is odd \n", msg)
        }
    }
    wg.Done()
}

func getRandomIntegerNumber() int {
    rand.Seed(time.Now().UnixNano())
    return rand.Intn(1000)
}


How can I condense several IBAction methods in a class with a delegate? I have the following class:

MyProtocolClass.h

#import <Foundation/Foundation.h>

@protocol MyExampleProtocol <NSObject>
@required
- (void)someRequiredMethod:(NSInteger)someValue;
@end

@interface MyProtocolClass : UIView

// interface stuff

@end

MyProtocolClass.m

#import "MyProtocolClass.h"

@interface MyProtocolClass()

@property (nonatomic,assign) id<MyExampleProtocol> delegate;

@end

@implementation MyProtocolClass

// implementation stuff

- (IBAction)fooMethod:(id)sender {
    [self.delegate someRequiredMethod:0];
}

- (IBAction)barMethod:(id)sender {
    [self.delegate someRequiredMethod:1];
}

- (IBAction)fooBarMethod:(id)sender {
    [self.delegate someRequiredMethod:2];
}

@end

And this is a fairly simple example.  The problem here is that I have a view with three buttons, and each button has it's own method, which simply calls a method on the delegate with an index to tell the delegate which button was pressed.

Surely there's a way to simplify this code and improve its readability.  A more complex view with several more interactive UI elements would quickly make this file very large if we have to add another 4 lines to the file for each interactive UI element we add to the screen.

You're right.  There is a simpler way.

First of all, instead of giving every interactive element its own method, let's give them all the same method.

- (IBAction)buttonPressed:(id)sender;

If they're not all buttons, a different method name is in line, but for this example, I'll assume them all to be buttons.

Now then, whether you're creating these UI elements programmatically or via interface builder, the next important step is to give each element its own unique tag.  In the Interface Builder, this is done under the "Attributes Inspector" (the icon that looks like a shield).  The "tag" field is the second field under the "View" heading.  It doesn't matter what tag you give the button.  It only matters that the tags are unique.

Setting the tag programmatically is as simple as this:

myButton.tag = 100;

Now, instead of a different method for each button (or other UI element), we can simply use a single button:

- (IBAction)buttonPressed:(id)sender {
    [self.delegate someRequiredMethod:sender.tag];
}

Now we've condensed your code.  But we can do even better.

Let's create an enum so that we're not just sending arbitrary numbers, but instead we send values that the delegate can make more sense of (and if using a switch, Xcode will help the delegate be sure he accounts for every case).

typedef NS_ENUM(NSInteger, MyExampleButtonEnum) {
    ButtonIndexUnknown = 0,
    ButtonIndexTopLeft = 100,
    ButtonIndexTopRight = 101,
    ButtonIndexBottomLeft = 102,
    ButtonIndexBottomRight = 103
};

This should be placed in the .h and before our @protocol.  Then we can change our protocol to look like this instead:

@protocol MyExampleProtocol <NSObject>
@required
- (void)someRequiredMethod:(MyExampleButtonEnum)button;
@end

Now we just have to set our button tags to match these enum values.  The good news is, if we're doing it programmatically, we can use the enum itself to set the values.  For example...

UIButton *topLeftButton = [[UIButton alloc] initWithFrame:someFrame];
topLeftButton.tag = ButtonIndexTopLeft;
[topLeftButton addTarget:self action:@selector(buttonPressed:)
    forControlEvents:UIControlEventTouchUpInside]

And now we slightly modify our buttonPressed: method as such:

- (IBAction)buttonPressed:(id)sender {
    if (sender) {
        [self.delegate someRequiredMethod:sender.tag];
    } else {
        [self.delegate someRequiredMethod:ButtonIndexUnknown];
    }
}

And our delegate can implement the delegate method in some fashion like this:

- (void)someRequiredMethod:(MyExampleButtonEnum)button {
    switch(button) {
        case ButtonIndexTopLeft:
            // do stuff
            break;
        case ButtonIndexTopRight:
            // do stuff
            break;
        case ButtonIndexBottomLeft:
            // do stuff
            break;
        case ButtonIndexBottomRight:
            // do stuff
            break;
        case ButtonIndexUnknown:
        default:
            // do stuff
            break;
    }
}


Selection handler for color picker At some point, I have this function which updates a color picker according the the mouse position, according to whether the user clicked a mouse button and according to its previous state:

updateColorPickerState :: ColorPicker -> (GLfloat, GLfloat) -> Bool -> ColorPicker
updateColorPickerState c (x,y) mouseClicked = if isPointWithinColorPicker c (x, y)
                                                then if mouseClicked
                                                  then if state c == Opened
                                                    then c {state = Idle, selectedColor = getColorAt (x',y')}
                                                    else c {state = Opened}
                                                  else if state c == Opened
                                                    then c
                                                    else c {state = Hovered}
                                                else if state c == Opened
                                                  then if mouseClicked
                                                    then c {state = Idle}
                                                    else c
                                                  else c {state = Idle}
                                              where (posX, posY) = pos c
                                                    (sizeX, sizeY) = openedSize c
                                                    x' = truncate ((x - posX) * 25 / sizeX)
                                                    y' = truncate (((-y) - posY) * 11 / sizeY)

How can I change the if-then-else tree to something more readable?

First, Simplify the function to a pattern match form.

updateColorPickerState :: ColorPicker -> (GLfloat, GLfloat) -> Bool -> ColorPicker
updateColorPickerState c (x,y) mouseClicked =
  case isPointWithinColorPicker c (x, y) of
    True -> case mouseClicked of
      True -> case state c of
        Opened -> c {state = Idle, selectedColor = getColorAt (x',y')}
        _      -> c {state = Opened}
      False -> case state c of
        Opened -> c
        _      -> c {state = Hovered}
    False -> case state c of
      Opened -> case mouseClicked of
        True -> c {state = Idle}
        _    -> c
      _      -> c {state = Idle}
  where
    (posX, posY) = pos c
    (sizeX, sizeY) = openedSize c
    x' = truncate ((x - posX) * 25 / sizeX)
    y' = truncate (((-y) - posY) * 11 / sizeY)

Then, convert case-based pattern matches to function-based pattern matches.

updateColorPickerState :: ColorPicker -> (GLfloat, GLfloat) -> Bool -> ColorPicker
updateColorPickerState c (x,y) mouseClicked
    | isPointWithinColorPicker c (x, y) = mouseInside (state c) mouseClicked
    | otherwise                         = mouseOutside (state c) mouseClicked
  where
    -- mouse is inside bounding box
    mouseInside Opened True  = c {state = Idle, selectedColor = getColorAt (x',y')}
    mouseInside Opened False = c
    mouseInside _      True  = c {state = Opened}
    mouseInside _      False = c {state = Hovered}
    -- mouse is outside bounding box
    mouseOutside Opened False = c
    mouseOutside _      _     = c {state = Idle}
    (posX, posY) = pos c
    (sizeX, sizeY) = openedSize c
    x' = truncate ((x - posX) * 25 / sizeX)
    y' = truncate (((-y) - posY) * 11 / sizeY)


HTML5 TAG using a .main class since in my website I have a lot of <HEADER>, <FOOTER> AND <SECTION> do you think is appropriate if i use a .main class for the main HEADER, main FOOTER AND main SECTION?

In the CSS I will never style the .main class like this:

.main { ... }

but always like this:

header.main { ... }
footer.main { ... }
section.main { ... }

Of course it will be total different styling.

As a general rule, if you want your markup to be semantic, you should be using classes, IDs and document structure to describe the content as clearly as possible, to whatever level is required to achieve the styling you need. Don't just add classes and IDs with the motivation "I need something to hook a CSS selector to", try thinking about what it is you want to style and use your tools to describe the thing.

Pragmatic design means you will have to be non-semantic to support particular browsers, particularly those that can't take advantage of the more advanced selectors, but you should only append these as required, if you can't achieve your desired design easily another way.

For your scenario, I would question whether main is a very good class name. Classes are intended to classify something, such that several instances of that thing can appear in a document (and be styled in the same way). If you intend to have several things in the page classified as main, it doesn't add a great deal of semantic value - I can't understand what that element really contains, other than that it's probably more important than non-main elements.

If you are trying to say "this is the primary content of the page", you should instead use an id. Use of an ID implies "there is only one of these per page", which gives you faster-performing scripts and the ability to reference the element with anchors (#main).

Additionally, if the header, footer and section elements are being given the main class because they're all related, you could try representing this relationship more strongly by nesting them inside a parent element and applying an ID to that element, e.g:

        ...

As an aside, the way you've listed section with header and footer makes me wonder if you're using it correctly.

The HTML5 specification says:

  The section element represents a generic section of a document or
  application. A section, in this context, is a thematic grouping of
  content, typically with a heading.

It specifically warns:

  The section element is not a generic container element. When an
  element is needed for styling purposes or as a convenience for
  scripting, authors are encouraged to use the div element instead. A
  general rule is that the section element is appropriate only if the
  element’s contents would be listed explicitly in the document’s
  outline.

So, the idea of a "main" section probably isn't right. Sections are meant to be ways partition a large article into headed-sections, in very much the way a Wikipedia article is laid out. You could easily say you have a "main" article, but a "main" section doesn't make as much sense.


Getting images and saving them to localstorage The code in this project accepts an image URL and creates an  tag dynamically, using the image URL as the src attribute value. The image *name *and URL are then saved to localStorage. The page can be viewed here.

I really want to improve this code. I can't say if it's a good approach. I can't really think of anything else on how to accomplish this but it works. What I really want to know is if I can make this simpler. I think I kind overdid it. Did I?

/*jshint esversion: 6 */

function saveImages(name, value) {
    if (typeof(Storage) !== undefined) {
        localStorage.setItem(name, value);
    }
    else {
        alert("LocalStorage is not supported on your browser");
    }
}

function loadStorage() {
    const container = $(".images-container");
    let elem;
    var keys = Object.keys(localStorage),
        i = 0,
        key;
    for (; key = keys[i]; i++) {
        elem = "";
        $(container).append(elem);
        let thisImg = document.getElementById(key);
        let source = localStorage.getItem(key);
        $(thisImg).attr('src', source);
    }
}

$(function() {
    "use strict";
    loadStorage();
    const create_btn = $("#createImage");
    const save_btn = $("#saveImages");
    const inputUrl = $("#ImageUrl");
    const inputName = $("#imageName");
    const container = $(".images-container");

    var createThisImage = function(name, url) {
        if ((name === "" && url === "") || (url === "")) {
            alert(" - expected input");
        }
        else if (name === "" || document.getElementById(name)) { //if name exists or name is empty then create a random name
            name = guid();
            name = new Image(url, name);
            name.createImage();
        }
        else {
            name = new Image(url, name);
            name.createImage();
        }

    };

    $(create_btn).on('click', function() {
        var new_image = inputName.val();
        var source = inputUrl.val();
        createThisImage(new_image, source);
    });

    $('img').on('click', function() {
        const identity = $(this).attr('id');
        const isValidDeletion = confirm("Delete " + identity + "?");
        if (isValidDeletion) {
            localStorage.removeItem(identity);
            window.location.reload(true);
        }
    });

    function Image(url, name) {
        this.img_url = url;
        this.img_name = name;
    }

    Image.prototype.createImage = function() {
        let id = this.img_name;
        let elem = "";
        $(container).append(elem);
        let thisImg = document.getElementById(id);
        thisImg.src = this.img_url;
        saveImages(this.img_name, this.img_url);
        window.location.reload(true);
    };

});

//Code from https://stackoverflow.com/questions/105034/create-guid-uuid-in-javascript
function guid() {
    return s4() + s4() + '-' + s4() + '-' + s4() + '-' +
        s4() + '-' + s4() + s4() + s4();
}

function s4() {
    return Math.floor((1 + Math.random()) * 0x10000)
        .toString(16)
        .substring(1);
}

loadStorage()

Wierd selective use of jQuery.

container is already a jQuery object. Just do container.append(elem); no need to wrap it in jQuery again.
document.getElementById(key); could be shortened to $("#"+key)[0]
$(thisImg).attr('src', source); could just be thisImg.src = source

createThisImage()

Why didn't you declare this function like the other ones?
The conditional if ((name === "" && url === "") || (url === "")) is the exact same thing as if (url === "")

Image.prototype.createImage()

Image is already a JS object, it represents an <img> object. Get rid of your Image constructor and use the native one.

Image.prototype.createImage = function() {
    $(container).append(this);
    saveImages(this.id, this.src);
    window.location.reload(true);
};

This would then be used like this...

img = new Image();
img.id = name;
img.src = url;
img.createImage();