Exercise: Reading doubles from a file and printing mean, max and min (from Programming Principles and Practice using C++) Read a sequence of double values into  a vector. Think of each value as the distance between two cities along a given route.  Compute and print the total distance (the sum of all distances). Find and print the smallest and greatest distance  between two neighboring cities. Find and print the mean distance between two  neighboring cities.

from Programming Principles and Practice using C++ by Bjarne Stroustrup

Since always entering values seems stupid, I decided to read the values from a file named "distances.txt".

print_distances_mean_max_min is what concerns me, how could I split the computing of min,max,mean and printing it while not spreading the code too thinly? Or is it fine as it is?

main.cpp

#include <iostream>
#include <vector>
#include <fstream>
#include <stdexcept>
#include <algorithm>

using std::vector;
using std::string;
using std::fstream;
using std::ios_base;
using std::runtime_error;
using std::cout;
using std::max_element;
using std::min_element;

const string distances_file_name = "distances.txt";

vector<double> load_file_of_doubles_to_vector ( string file_name )
{
    vector<double> distances;
    fstream my_file ( file_name, ios_base::in );
    double temporary_number;
    while ( my_file >> temporary_number )
    {
        distances.push_back ( temporary_number );
    }
    if ( !my_file.is_open() )
    {
        throw runtime_error ( string ( "Failed to load " ) + file_name );
    }
    return distances;
}

void print_distances_mean_max_min ( const vector<double>& distances_vector )
{
    if ( distances_vector.empty() )
    {
        throw runtime_error ( string ( "There are no entries in " ) + distances_file_name );
    }
    double total_distance = 0;
    for ( auto& n : distances_vector )
    {
        total_distance += n;
    }
    cout << "The sum of all distances is: " << total_distance << "\n";
    double max_distance = 0;
    max_distance = *max_element ( distances_vector.begin(), distances_vector.end() );
    cout << "The maximum distance is: " << max_distance << "\n";
    double min_distance = 0;
    min_distance = *min_element ( distances_vector.begin(), distances_vector.end() );
    cout << "The minimum distance is: " << min_distance << "\n";
    double mean_distance = 0;
    mean_distance = total_distance / distances_vector.size();
    cout << "The average(mean) distance is: " << mean_distance << "\n";
}

int main()
{
    print_distances_mean_max_min ( load_file_of_doubles_to_vector ( distances_file_name ) );
    return 0;
}

distances.txt

8.7
6.3
9.0
3.142
2.7

Here are some suggestions for you to improve your code.

#include <iostream>
#include <vector>
#include <fstream>
#include <stdexcept>
#include <algorithm>

Sort the #include directives in alphabetical order.  This helps navigation.

using std::vector;
using std::string;
using std::fstream;
using std::ios_base;
using std::runtime_error;
using std::cout;
using std::max_element;
using std::min_element;

This is much better than using namespace std;, but I still suggest getting into the habit of explicitly qualifying the names with std:: as soon as possible (especially for those names which you only use once, e.g., max_element).  For small programs like this it doesn't matter, but for larger programs, qualification adds clarify — when I see string, does it mean std::string?  Jewelry::string, a string of pearls?  Or Minecraft::string, used to craft bows and fishing rods?  This problem is especially evident for common identifiers, like count, arg, or data — these are all top level names in std!

Moreover, using unqualified names sometimes cause ADL (argument dependent lookup), an additional phase for name lookup, to kick in.  As a result, an attempt to call a function in the standard library may end up inadvertently calling a function in an irrelevant namespace.  Such problems come up often in large programs, so again, it's better to get into the habit of using std::.

It is acceptable to use using declarations or derivatives in reduced scopes, to reduce clutter.  But using them globally for the whole program is likely to cause problems.  Moreover, std:: is only 5 characters — reducing clutter is more relevant for things like

boost::math::double_constants::pi

or

boost::multiprecision::number<
    boost::multiprecision::mpfr_float_backend<300>,
    boost::multiprecision::et_off
>;

const string distances_file_name = "distances.txt";

Use constexpr std::string_view (defined in header <string_view>) for string constants:

constexpr std::string_view file_name{"distances.txt"};

or

using namespace std::literals;
constexpr auto file_name = "distances.txt"sv;

vector<double> load_file_of_doubles_to_vector ( string file_name )
{
    vector<double> distances;
    fstream my_file ( file_name, ios_base::in );
    double temporary_number;
    while ( my_file >> temporary_number )
    {
        distances.push_back ( temporary_number );
    }
    if ( !my_file.is_open() )
    {
        throw runtime_error ( string ( "Failed to load " ) + file_name );
    }
    return distances;
}

The parameter type should be std::string_view to avoid redundant copies.

The file stream should be of type std::ifstream instead of std::fstream because it is used for input only.  Then, you can omit the in option, which is the default for std::ifstream.

string ( "Failed to load " ) + file_name is too verbose.  Use "Failed to load "s.

Consider using std::istream_iterator, std::back_inserter (both defined in <iterator>), and std::copy to read the values:

// requires NTBS as argument
auto load_file(std::string_view file_name)
{
    // std::string_view doesn't have c_str()
    std::ifstream in{file_name.data()};
    if (!in.is_open()) {
        // assuming 'using namespace std::literals;'
        throw std::runtime_error{"the file doesn't exist"};
    }

    std::vector<double> data;
    std::copy(std::istream_iterator<double>{in},
              std::istream_iterator<double>{},
              std::back_inserter(data));
    return data;
}

void print_distances_mean_max_min ( const vector<double>& distances_vector )
{
    if ( distances_vector.empty() )
    {
        throw runtime_error ( string ( "There are no entries in " ) + distances_file_name );
    }
    double total_distance = 0;
    for ( auto& n : distances_vector )
    {
        total_distance += n;
    }
    cout << "The sum of all distances is: " << total_distance << "\n";
    double max_distance = 0;
    max_distance = *max_element ( distances_vector.begin(), distances_vector.end() );
    cout << "The maximum distance is: " << max_distance << "\n";
    double min_distance = 0;
    min_distance = *min_element ( distances_vector.begin(), distances_vector.end() );
    cout << "The minimum distance is: " << min_distance << "\n";
    double mean_distance = 0;
    mean_distance = total_distance / distances_vector.size();
    cout << "The average(mean) distance is: " << mean_distance << "\n";
}

IMO, this function is unnecessary and can be built into main directly, because it doesn't have a coherent logic — it just shows what the program does.

Catching exceptions in the main function and printing the messages helps the end user know what kind of error happened.

Don't reinvent the wheel — use std::accumulate or std::reduce (defined in header <numeric>) to calculate sums.

You can calculate the minimum and maximum values simultaneously in
\$\lfloor 3(N-1)/2 \rfloor\$ comparisons using std::minmax_element, which returns a std::pair of iterators pointing to the minimum and maximum elements respectively.  Then, we can use a structured binding to conveniently obtain the two iterators and dereference them to get the values:

int main()
try {
    auto data = load_file(file_name);
    if (data.empty()) {
        throw std::runtime_error{"no entries"};
    }

    auto sum = std::accumulate(data.begin(), data.end(), 0.0);
    auto mean = sum / data.size();
    auto [min_it, max_it] = std::minmax_element(data.begin(), data.end());

    std::cout << "The sum of all distances is: " << sum << '\n'
              << "The maximum distance is: " << *max_it << '\n'
              << "The minimum distance is: " << *min_it << '\n'
              << "The average (mean) distance is: " << mean << '\n';
} catch (std::runtime_error& error) {
    std::cerr << "Runtime error: " << error.what() << '\n';
    return 1;
} catch (...) {
    std::cerr << "Unknown error\n";
    return 2;
}

I've made an online demo by putting everything together so that you can play with the code.


Method to re-open a connection on a datacontext if it has gone down The following method is used to keep a DataContext open if it is closed or broken and is called before most calls to access the database.

It runs in a service that needs constant access to the database, constantly opening and closing the connection would be too costly. So we only reconnect if the connection has been closed, i.e. we have been kicked off.

private void OpenConnection<T>(System.Data.Linq.DataContext dataContext, string connectionString) where T : System.Data.Linq.DataContext
{
    try
    {
        if (dataContext != null && dataContext.Connection != null)
        {
            if (dataContext.Connection.State == System.Data.ConnectionState.Closed
             || dataContext.Connection.State == System.Data.ConnectionState.Broken)
            {
                dataContext.Dispose();
                dataContext = null;
            }
        }
    }
    catch
    {
        if (dataContext != null)
        {
            dataContext.Dispose();
        }
        dataContext = null;
    }

    if (dataContext == null)
    {
        dataContext = (T)System.Activator.CreateInstance(typeof(T), connectionString);
    }
}

It is called like:

OpenConnection<MyDataContext>(instanceOfMyDataContext, connectionString);

My main concerns are if my use of generics is correct and if the as T on creating an instance of the DataContext is correct. Although all pointers about my code are welcomed.

First off, not sure I fully understand why you're doing this but taking that as a given here are some tips to make your code a bit more readable and concise.

Namespaces

Unless you have conflicts with other namespaces, I'd suggest adding using System.Data; and using System.Data.Linq; to the top of you file. If you do have a conflict and you created it yourself (not from a 3rd-party library) I'd suggest changing the name so there isn't a conflict if possible. Just makes things easier.

Method Extensions

Those if-statements you've got are kind of ugly. You could create a couple of method extensions for the DataContext class like this.

public static class DataContextExtensions
{
    /// <summary>
    /// Checks that the context and its connection are not null.
    /// </summary>
    public static bool HasContextAndConnection(this DataContext dataContext)
    {
        return dataContext != null
            && dataContext.Connection != null;
    }

    /// <summary>
    /// Checks if the context's connection is broken or closed.
    /// </summary>
    public static bool IsConnectionClosedOrBroken(this DataContext dataContext)
    {
        // NOTE: You can add the above null-checks to this one so that this
        // function doesn't throws a NullReferenceException if called before the other.
        return dataContext.Connection.State == ConnectionState.Closed
            || dataContext.Connection.State == ConnectionState.Broken;
    }
}

Try-Catch

You should be intentional with your code, having a wide-open try/catch states to whoever is reading it "The person who coded this has no idea what could possibly go wrong with it." (or more commonly, "I'm too lazy to list out what could go wrong."). So not only does it make you look bad, but it also doesn't provide any information to the reader/maintainer of what is expected to go wrong.

So then you should look into what could go wrong. Well, we're already doing null-checks so there shouldn't be any NullReferenceExceptions. That leaves the Dispose() method and the Activator.CreateInstance being the only other two possibilities.

According to the IDisposable.Dispose() MSDN page, Dispose should never throw an exception. Additional StackOverflow reference here too. So we should be good there.

Activator.CreateInstance however, could throw a handful of exceptions and you don't have a try-catch around that. Here's the MSDN page that outlines the exceptions that could be thrown.

Result

private void OpenConnection<T>(DataContext dataContext, string connectionString)
    where T : DataContext
{
    // DISPOSE: Clean up the existing context if the connection is closed or broken.
    if (dataContext.HasContextAndConnection() && !dataContext.IsConnectionClosedOrBroken())
    {
        dataContext.Dispose();
        dataContext = null;
    }

    // CREATE: Instantiate a new context given the connection string.
    if (dataContext == null)
    {
        var type = typeof(T);

        try
        {
            dataContext = Activator.CreateInstance(type, connectionString);
        }
        catch (TargetInvocationException)
        {
            // Constructor threw an exception.
        }
    }
}

Personal Touch

Personally, I like adding the header comments you see above. Where the all-caps part is a quick 1-2 word description of the section and everything after is a bit more detailed. I find it helps me scim code faster, however that is something that needs maintained when code is changed.


A reliable way to remove consecutive appearances of a substring I'm attempting to write a piece of code that is supposed to remove consecutive appearances of a string (not a single character) in a StringBuilder.
It's extremely important that the method works well and does not remove or change anything that it shouldn't. Solid performance is a secondary requirement.

for example:

Input: "xxxxABCxxxxABCABCxxxxABABCxxABCABCABC"
Remove consecutive: "ABC"

Output: "xxxxABCxxxxABCxxxxABABCxxABC"

I've written a method that does this and testing shows that it works as expected, but it will be catastrophic if I use it and it results in changing the StringBuilder in an unexpected way - that's why I want another opinion on whether my code is really 'safe' in all cases:

public static void RemoveConsecutive(this StringBuilder sb, string value)
{
    if (sb == null)
        throw new ArgumentNullException("sb");
    if (value == null)
        throw new ArgumentNullException("value");
    if (value == string.Empty)
        throw new ArgumentException("value cannot be an empty string.", "value");

    bool justRemoved = false;
    for (int i = 0; i < sb.Length - value.Length; i++)
    {
        if (justRemoved || Util.ExistsAt(sb, i, value))
        {
            if (Util.ExistsAt(sb, i + value.Length, value))
            {
                sb.Remove(i, value.Length);
                justRemoved = true;
                i--;
            }
            else
            {
                justRemoved = false;
            }
        }
    }
}

// Checks if the provided string appears in the StringBuilder at the specified index
public static bool ExistsAt(StringBuilder sb, int startIndex, string str)
{
    if (startIndex < 0 || startIndex >= sb.Length)
        throw new ArgumentOutOfRangeException("startIndex", "startIndex must be a valid index in the provided StringBuilder.");
    if (startIndex + str.Length > sb.Length)
        return false;
    for (int i = 0; i < str.Length; i++)
    {
        if (str[i] != sb[i + startIndex])
            return false;
    }
    return true;
}

While this piece of code does something rather trivial, I just want another set of eyes to look at it and possibly find faults I could not. Again, any sort of unexpected behavior might be catastrophic.

Your code looks like it does exactly what you want to me.

Some notes:

StringBuilder methods (which you're emulating by using an extension method) return the modified StringBuilder, so they can be used in a fluent manner. You should probably do the same.
I don't like changing the index of a for loop. Especially since i-- means “stay at the same index”, I think that's confusing. You should use a while loop instead.
You can shorten your code by quite a bit by using Regex:

public static string RemoveConsecutive(string text, string value)
{
    // validation omited

    string regex = string.Format(@"({0})+", Regex.Escape(value));
    return Regex.Replace(text, regex, match => value);
}

Shorter code is usually less error prone. But in this case, I'm not so sure about that, because there are some intricacies in this code:

Calling Escape() is crucial if value contains some regex special characters (e.g. *).
If the value was used directly as the replacement (instead of a delegate), it wouldn't work correctly if it contained any substitution (e.g. $0).


Bool function in PHP to check if you run the script inside docker I had to do a CLI script that does something, I also had to add a validation that checks if it runs inside the Docker. After some research I found a solution that people check some processes via cat /proc/self/cgroup and then grep to search for a docker phrase within.

So I ended-up with this method. It works pretty well. Inside docker it returns true, while on my local mac it returns false.

It's not rocket science however I would like people to take a look and tell me if this is a sufficient method or a completely different approach is better.

private function isDocker(): bool
{
    $processStack = explode(PHP_EOL, shell_exec('cat /proc/self/cgroup'));
    $processStack = array_filter($processStack); // remove empty item made by EOL

    foreach ($processStack as $process) {
        if (strpos($process, 'docker') === false) {
            return false;
        }
    }

    return true;
}

Later call to this script looks like:

if ($this->isDocker() === false) {
    throw new \Exception('This helper script can be called only inside Docker container.' . "\n");
}

For this simple method, I have a couple thoughts below. Your method seems sufficient; I don't think a completely different approach is necessary, although I question how often this method is called. If it is called more than once per page load/script run, it may be worth memoizing the value once obtained for better performance.

Good job utilizing the PHP 7 return type declaration.

Because the method utilizes no instance properties or methods, it could be declared as static, and possibly public so other code could use it, though maybe it is best to leave it as private until there is a legitimate need to make it otherwise.

While the logic would need to be reversed, substr_count() could be used instead of strpos(). The difference is likely negligible but you need to ask yourself in which case would you rather have the method return as soon as the string is or isn't found...

    foreach ($processStack as $process) {
        if (substr_count($process, 'docker')) {
            return true;
        }
    }
    return false;


Palindrome using stack I recently started learning Java, algorithms, and data structures. I am trying to check if a string is palindrome. I would like a review to see where and how I could improve.

import java.util.Stack;

class Palindromer {

Stack<Character> stack = new Stack<Character>();
int top = -1;

public boolean isPalindrome(String s) {
    for (int i = 0; i < s.length(); i++) {
        stack.push(s.charAt(i));
        top++;
    }

    System.out.println("value of top is " + top);
    String returnString = "";

    while (!stack.isEmpty()) {
        returnString += stack.pop();
    }
    System.out.println(returnString);

    return returnString.equals(s);
}

}

public class PalindromeChecker {
public static void main(String[] args) {
    Palindromer palin = new Palindromer();
    if (palin.isPalindrome("BananaB")) {
        System.out.println("is palindrome");
    } else {
        System.out.println("not a palindrome");
    }
}
}

Extract routine logic into methods.

Your evaluation process should be its own method. This is something you'd want to repeat and it would make it easy to test multiple cases.

Your main method could just be:

public static void main(String[] args) {
    System.out.println(isPalindromic("racecar"));
}

Use JUnit tests

On the topic of tests, get into the habit of writing them, a good framework for testing is JUnit. You generally want to test something for edge cases and ensure that it works before sharing.

Example test case:

@Test
public void validPalindromes() {
    assertTrue(isPalindromic("racecar"));
    assertTrue(isPalindromic("mom"));
    assertTrue(isPalindromic("avid diva"));
}

Semantic considerations of palindrome

One decision you should make is how you want to account for letter case, and punctuation. The sentence, "A car, a man, a maraca." is a palindrome by English standards, but whether it is or not will be up to your implementation.

A better way to check for palindrome.

What you want to do is just reverse a string and test it against the string, fortunately StringBuilder has a reverse method.

public static String reverse(String str) {
    return new StringBuilder(str).reverse().toString();
}

So to check if a word is a valid string, you can simply have:

public static boolean isPalindromic(String str) {
    return str.equals(reverse(str));
}

If you want to account for letter case you can use equalsIgnoreCase and for punctuation you should use a Regex.

Adding these factors in would give us:

public static boolean isPalindromic(String str) {
  String potentialPalindrome = str.replaceAll("[^a-zA-Z]", "");
  return potentialPalindrome.equalsIgnoreCase(reverse(potentialPalindrome));
}

Simplest implementation

If you don't want the overhead of using StringBuilder, you can always use a loop:

public static boolean isPalindromic(String s) {
    for (int i = 0, j = s.length() - 1; i < j; i++, j--) {
        if (s.charAt(i) != s.charAt(j)) {
            return false;
        }
    }
    return true;
}


Finding solution for pancake sort efficiently without actually flipping each time Problem

Solve correct flips to sort given input with pancake sort.

Example

input (input size and values):
8
6 7 2 5 1 4 3 8

solution (number of flips and how many elements to flip per operation):
6
2 7 4 5 3 4

    2 -> 7 6 2 5 1 4 3 8
    7 -> 3 4 1 5 2 6 7 8
    4 -> 5 1 4 3 2 6 7 8
    5 -> 2 3 4 1 5 6 7 8
    3 -> 4 3 2 1 5 6 7 8
    4 -> 1 2 3 4 5 6 7 8

alternative solution:
6
3 6 2 5 7 3

Program doesn't need to actually sort anything, just give one possible solution.

What would be faster way to calculate needed flips without resorting to sorting input with ineffective pancake sort?

#include <iostream>
#include <algorithm>
#include <vector>

int main() {
    using namespace std;
    ios::sync_with_stdio(false);

    int N;
    cin >> N;
    vector<int> v(N);
    vector<int> flips;

    for (int i=0; i<N; ++i)
        cin >> v[i];

    auto first  = v.begin();
    auto last   = v.end();
    for (; first != last; --last)
    {
        auto mid = max_element(first, last);
        if (mid == last - 1) {
            continue;
        }
        if (first != mid) {
            flips.push_back(distance(first, mid+1));
            reverse(first, mid + 1);
        }
        flips.push_back(distance(first, last));
        reverse(first, last);
    }

    cout << flips.size() << endl;
    for (auto flip : flips)
        cout << flip << " ";
    cout << endl;
}

You should drop namespace std; It is bad practice even in challenges.
Use more descriptive names even if the variable is named so in the problem. You can also use a range based loop for the initialization of the elements.

size_t vectorLength;
std::cin >> vectorLength;
std::vector<int> numbers(vectorLength);
for (auto &number : numbers) {
    std::cin >> number;
}

You declare the iterators outside of the loop, which is uneccessary, if both have the same type.

for (auto first  = v.begin(), last = v.end(); first != last; --last)

Also you iterate from the end so you might want to think about using reverse iterators aka rbegin() and rend()
When comparing or increasing iterators, you should use mid == std::next(last, 1) and or std::prev accordingly. This works with every kind of iterator.

This also brings me to the bad choice of your loop variable. the iterator is called last and you compare with std::next(last, 1), which one would expect to be out of memory access. Instead define an explicit iterator auto it = numbers.end() and use it in your loop, avoiding confusion.


To-do list with counting of checked items For practice  JavaScript I created simple To-do app.
This app can add TODOs and track the number of total TODOs as well as the number of unchecked TODOs.
For this I written 4 functions:

for add newTodo - newTodo(), where I add new todo to todos array
for render list of todo - drawTodoList(), where I also add event listener to each checkbox item
confirmCheck() for change state of todo checked property
countItem() for counting all items and unchecked items

My questions:

was it right to use innerHTML to add li items?
adding event listener to all checkboxes in my case takes 4 line (in end drawTodoList). How can I short this?
how can I improve my function countItem()?

const todos = []
let newTodoId = 0
let checkboxes
let checkboxArray

const classNames = {
  TODO_ITEM: 'todo-container',
  TODO_CHECKBOX: 'todo-checkbox',
  TODO_TEXT: 'todo-text',
  TODO_DELETE: 'todo-delete',
}

const list = document.getElementById('todo-list')
const itemCountSpan = document.getElementById('item-count')
const uncheckedCountSpan = document.getElementById('unchecked-count')

function newTodo() {
  let getTodo = prompt("Add new to-do", "")

  if(getTodo == null || getTodo == "") {
    console.log("you don't add new to-do")
  } else {
    todos.push({
      id: newTodoId ++,
      text: getTodo,
      isChecked: false,
    })
    drawTodoList()
  }
}

function drawTodoList() {
  const liItem = todos.map((todo) => {
    if (!todo.isChecked) {
      return `

    <input type='checkbox' class=${classNames.TODO_CHECKBOX}>${todo.text}
`
    } else {
      return `

    <input type='checkbox' class=${classNames.TODO_CHECKBOX} checked>${todo.text}
`
    }
  })
  list.innerHTML = liItem.join('')

  checkboxes = document.querySelectorAll('input[type=checkbox]')
  checkboxArray = Array.from(checkboxes)
  checkboxArray.forEach(function(checkbox) {
    checkbox.addEventListener('change', confirmCheck)
  })
  countItem()
}

function confirmCheck() {
  let checkedToDo = todos.find(todo => {
    return todo.id == this.parentElement.id
  })
  checkedToDo.isChecked = !checkedToDo.isChecked
  countItem()
}

function countItem() {
  let itemCount = document.querySelectorAll('input[type="checkbox"]').length
  let checkedCount = document.querySelectorAll('input[type="checkbox"]:checked').length
  let uncheckedCount = itemCount - checkedCount
  itemCountSpan.innerHTML = itemCount
  uncheckedCountSpan.innerHTML = uncheckedCount
}
.center {
  align-self: center;
}

.flow-right {
  display: flex;
  justify-content: space-around;
}
.container {
  max-width: 800px;
  margin: 0 auto;
  padding: 10px;
  display: flex;
  flex-direction: column;
  background-color: white;
  height: 100vh;
}

.title, .controls, .button {
  flex: none;
}

.button {
  padding: 10px 20px;
}
<!DOCTYPE html>

    <title>TODO App</title>
    <link rel="stylesheet" type="text/css" href="./styles.css" />

My TODO App

        Item count: 0
        Unchecked count: 0

      <button class="button center" onClick="newTodo()">New TODO</button>

Adding to the DOM
There is no one right or wrong way to add to the DOM, each has its pros and cons.
Some options

Markup direct to the page via element.innerHTML (as you have done).
Using the document API to create a node tree, assign properties and then add to the DOM using any of the various node insert functions like node.appendChild.
Cloning the content of a template element that defines the nodes you want to add. Then use the document API to make the needed change and append to the page

So which to use?
Its a rather subjective question and it would be best that you try them all.

was it right to use innerHTML to add li items?

Yes and no.
The method of adding was not wrong (element.innerHTML), but the design of the function and overall approch to procedural generated content is wrong.
You are re-creating the whole displayed todo list whenever you add to it. That's like building a wall one brick at a time, but before you add the next brick you smash down the wall and rebuild it to the point where you add the next brick.
You need only add the new todo item to the end (or top) of the list of items. The rest of them can stay where you put them before.
Event listeners

adding event listener to all checkboxes in my case takes 4 line (in end drawTodoList). How can I short this?

You are referring to the lines...

checkboxes = document.querySelectorAll('input[type=checkbox]')
checkboxArray = Array.from(checkboxes)
checkboxArray.forEach(function(checkbox) {
   checkbox.addEventListener('change', confirmCheck)
})

...which could be shortened to
document.geteElementById("#"+toDo.id).addEventListener('change', confirmCheck );

But the question can not be  directly answered as the function drawTodoList is flawed, as I described in Adding to the DOM.
The click function can be added to the un-ordered list element once, at the start of the app, thus one event covers all todo items.
The checkbox that is clicked on can be found in event.target. Adding a property to the checkbox that holds the todo item id lets you apply the changes to the todolist via the todo id.
Information, data, and the display

how can I improve my function countItem()?

The count is a behavior of the information not the display, get it by stepping over the items in the todo list.
The rewrite.
I started from your HTML and CSS.
It is a bit over kill but I got lost in the writing of it. The code contains how I have implemented the 3 questions you ask. Adding to the DOM, creating event listener for new items, and counting the number of unchecked items.

const todoDataJSON = `[{ "id" : 1, "checked" : false, "text" : "Click Something todo" },{ "id" : 2, "checked" : false, "text" : "Check Click Something todo as done" }]`;

// the todo information and behaviours
const todoList = (() => {
    var UId = 1;                // NOTE must start above 0 to be truthy true
    const items = [];
    const indexed = new Map();  // For quick lookups by id
    const vetRecord = record => true; // This stub Make sure incoming data is valid and safe
    const vetText = text => typeof text === "string" && text !== "";
    const create = (text, checked = false, id = UId++) => ({id, text, checked});
    const reset = () => (items.length = 0, indexed.clear(), UId = 1);

    const toDoList = Object.freeze({
        get length() { return items.length },
        get uncheckedCount() { return items.reduce((uc, record) => uc += record.checked ? 0 : 1, 0) },
        set data(jsonText) {
            var maxId = UId;
            try {
                const data = JSON.parse(jsonText);
                reset();
                data.forEach(item => {
                    if (!vetRecord(item)) { throw new Error(); }
                    maxId = Math.max(item.id, maxId);
                    const record = create(item.text, item.checked, Number(item.id));
                    items.push(record);
                    indexed.set(record.id, record);
                });
                UId = maxId + 1;
            } catch(e) { reset() } // make data safe again
        },
        get data() { return JSON.stringify(items) },
        each(callback) { for (const record of items) { callback({...record}) } },
        toggleCheck(id) {
            const record = indexed.get(id);
            if (record) { record.checked = !record.checked }
        },
        add(text) {
            if(vetText(text)) {
                const record = create(text);
                items.push(record);
                indexed.set(record.id, record);
                return {...record};  // returns a copy not the original
            }
        }
    });
    return toDoList;
})();

/* Display an UI */
;(()=>{
    const elements = {
        list : query('#todo-list'),
        count : query('#item-count'),
        uncheckedCount : query('#unchecked-count'),
        template : query('#todo-item'),
    };

    elements.list.addEventListener("click", listItemClick);
    query("#add-button").addEventListener("click", promptForNew);

    function query(query, el = document) { return el.querySelector(query) }
    const assign = (el, props) => Object.assign(el, props);
    const append = (node, ...items) => {
        items.forEach(el => node.appendChild(el));
        return node;
    }

    function listItemClick(event) {
        if(event.target.todoId) {
            todoList.toggleCheck(event.target.todoId)
            updatePage();
        }
    }
    function displayToDoItem(todo) {
        const listItem = document.importNode(elements.template.content, true);
        assign(query("li", listItem), {id : "todoItem" + todo.id });
        assign(query("input", listItem), {
            checked : todo.checked,
            name : "todo" + todo.id,
            todoId : todo.id,   // added but will not be vissible in markup
        });
        assign(query("label", listItem), {
            "for" : "todo" + todo.id,
            textContent : todo.text
        });
        append(elements.list, listItem);
    }
    function promptForNew() {
        const todo = todoList.add(prompt("What is it you want to do", ""));
        if (todo) {  displayToDoItem(todo) }
        updatePage();
    }
    function updatePage() {
        elements.count.textContent = todoList.length;
        elements.uncheckedCount.textContent = todoList.uncheckedCount;
    }

    // source data and give to todolist
    todoList.data = todoDataJSON;
    todoList.each(displayToDoItem); // display existing
    updatePage();

})();
.center {
  align-self: center;
}

.flow-right {
  display: flex;
  justify-content: space-around;
}
.container {
  max-width: 800px;
  margin: 0 auto;
  padding: 10px;
  display: flex;
  flex-direction: column;
  background-color: white;
  height: 100vh;
}

.title, .controls, .button {
  flex: none;
}

.button {
  padding: 10px 20px;
}
<template id="todo-item">

     <input type="checkbox" class="todo-checkbox">

</template>

Things to do

    All things done & to do: 0
    Things left to do: 0

  <button class="button center" id="add-button">I Have Something To Do</button>


C++ demo code for interaction with C I want to show how to interact with C from C++ and chose a simple and small library to demonstrate. zlib's gzopen, gzwrite and gzclose looked like a reasonably closed set of funtions.

Does anyone see anything fundamentally wrong with my presentaion? The main code I present here is in class GzWrite.

Intro section:

#include <string>
#include <vector>
#include <fstream> // ifstream
#include <stdexcept> // runtime_error
#include <iostream> // cerr
// C-Headers:
#include <zlib.h> // gzXyz; sudo aptitude install libz-dev
#include <string.h> // strerror
#include <errno.h> // errno
#pragma GCC diagnostic ignored "-Wmissing-field-initializers"
using std::ifstream; using std::ofstream; using std::string; using std::vector;
namespace {

constexpr size_t BLOCK_SZ = 256*1024;
constexpr int LEVEL = Z_DEFAULT_COMPRESSION;

Core class

struct GzWrite { // RAII-Wrapper
    gzFile gz_ ; // C-Struct aus zlib.h
    explicit GzWrite(const string& filename)
        : gz_{gzopen(filename.c_str(),"wb9")} // 'w': write, 'b':binary, '9':level
    {
        if(gz_==NULL) throw std::runtime_error(strerror(errno));
    }
    ~GzWrite() {
        gzclose(gz_);
    }
    int write(const char* data, size_t len) {
        return gzwrite(gz_, data, len);
    }
    GzWrite(const GzWrite&) = delete; // no copy
    GzWrite& operator=(const GzWrite&) = delete; // no assign
};

Helper functions

vector<char> read(const string& infn) {
    ifstream inf{ infn, ifstream::binary };
    if(!inf) throw std::runtime_error("open error");
    inf.seekg(0, inf.end); // go to end of file
    const auto laenge = inf.tellg(); // current pos is file size
    if(laenge > 1024*1024*1024) throw std::runtime_error("<1GB please");
    inf.seekg(0, inf.beg); // back to begin
    vector<char> data(laenge); // make room
    inf.read(data.data(), laenge); // read all at once
    return std::move(data); // make doubly sure data is not copied
}

void pack(const string& infn, const string& outfn) {
    vector<char> indata = read(infn); // read input
    GzWrite gz{outfn}; // init output
    auto res = gz.write(indata.data(), indata.size());
    if(res==0) throw std::runtime_error("write error");
}

} // namespace

Main function:

int main(int argc, const char* argv[]) {
    try {
        const vector<string> fnamen {argv+1, argv+argc};
        for(auto fname : fnamen) {
            std::cout << "packing " << fname << "... ";
            pack(fname, fname+".gz");
            std::cout << fname << ".gz"<< "\n";
        }
    } catch(std::runtime_error exc) {
        std::cerr << "Error: " << exc.what() << "\n";
    }
}

This example is mostly about making and using a RAII wrapper, not about calling C from C++ (which is trivial).

If you want to show how to call C-like interfaces, then use zlib directly, without wrapping it.

If you want to show the native C++ way to wrap a C interface, then the wrapper should be an iostream subclass, like gzstream.

pack should read blocks of finite size rather than reading the whole file at once. It's a compression utility, so it's likely to be used on files that won't fit in memory!

Some confusing names:

GzWrite instances aren't writes; they're streams. How about GzStream or (if it uses the iostreams interface) gzostream?
Don't call a function read; it's easy to confuse with Posix read. This is traditionally called slurp or read_file.
infn will probably be interpreted as "in-function", not "in-filename". How about infile?
laenge (Länge?): length

BLOCK_SZ and LEVEL are unused.


Add elements to a textfile using a dictionary Is there a shorter way to write this code or parts of it and where can i inculde some try and excepts for validation

   People={"Dan":22,
            "Matt":54,
            "Harry":78,
            "Bob":91}

schoice=input('Enter existing name: ')
while schoice not in People:
    schoice=input('Invalid entry name is non existant, Enter a chosen name: ')
if schoice in People:
    getscore=People[schoice]
    if getscore<50:
        presents=[]
        for x in range(2):
            present=input('Enter present ' +str(x+1))
            while present == '':
                present = input('Invalid. Try again: ')
            presents.append(present)
    if getscore>=100:
        presents=[]
        for x in range (8):
            present=input('Enter present ' +str(x+1))
            while present == '':
                present = input('Invalid. Try again: ')
            presents.append(present)
    if getscore>=50 and getscore <100:
        presents = []
        for x in range (5):
            present=input('Enter present ' +str(x+1))
            while present == '':
                present = input('Invalid. Try again: ')
            presents.append(present)
File=open("presfile.txt",'a')
File.write(schoice)
File.write(str(presents)+'\n')
File.close()

PEP8
There are quite a number of style-related "errors" in the code. Try to adhere to the Python PEP8 style guide. Here are a few key points for this particular case:

Variables and functions are written in snake_case, classes PascalCase and constants (if any) are written fully in UPPERCASE.
Use a single space around operators, assignments, comparisons, etc.
Use proper indentation (e.g. at People)
Use descriptive variable names.

Also use some whitespace here and there to make it more readable.
Some examples from the code:

people instead of People
get_score instead of getscore. Also get_score seems more like a function name rather than a variable name. Perhaps just use score as this is the actual value of the variable.
if get_score >= 100 instead of if getscore>=100
file = open('presfile.txt', 'a') instead of `File=open("presfile.txt",'a')
chosen_name instead of schoice. Perhaps person or chosen_person is even better, as this gets a value from People.

Use functions
Functions make the code more readable and splits up the responsibility of the code in smaller, reusable code. This also makes it a lot easier to test the application with for instance unittest. Once divided into functions, the code can easily be imported into another project, if need be.

You are doubly checking if schoice is in People

while schoice not in People:
    schoice=input('Invalid entry name is non existant, Enter a chosen name: ')
if schoice in People:
    getscore=People[schoice]

Once the if is reached, it is already certain that schoice is in People, otherwise the while loop would not be broken.
The code
presents = []
for x in range (5):
    present=input('Enter present ' +str(x+1))
    while present == '':
        present = input('Invalid. Try again: ')
    presents.append(present)

is repeated 3 times only with a different range. Put it in a separate function. See also: DRY
def get_presents(amount):
    """Return a list of presents equal in size to the given amount"""
    presents = []
    for x in range(amount):
        while True:
            present = input('Enter present #{}: '.format(x+1))
            if present:
                presents.append(present)
                break
            else:
                print('Invalid present, try again.')
    return presents

Now you can call this function with the amount of expected presents and it will return the list of presents: get_presents(5)
The if checks
if score < 50:
    ...
if score >= 100:
    ...
if score >= 50 and score < 100
# can also be written as 50 <= score < 100
    ...

has the code check get_score up to three times. This is not particularly a performance issue in this case, but could also be written as:
if score < 50:
    ...
elif score >= 100:
    ...
else:
    ...

Use with to open the file:
with open('presfile.txt', 'a') as presfile:
    presfile.write(chosen_person)
    presfile.write(str(presents) + '\n')

This ensures that the file is always closed. See also file.close and this answer about with

The code:
def get_presents(amount):
    """Return a list of presents equal in size to the given amount"""
    presents = []
    for x in range(amount):
        while True:
            present = input('Enter present #{}: '.format(x+1))
            if present:
                presents.append(present)
                break
            else:
                print('Invalid present, try again.')
    return presents

def get_presents_for_person():
    """Description of function."""
    people = {
        "Dan": 22,
        "Matt": 54,
        "Harry": 78,
        "Bob": 91,
    }

    chosen_person = input('Enter existing name: ')
    while chosen_person not in people:
        chosen_person = input('The chosen name does not exist. '
                              'Please enter an existing name: ')

    score = people[chosen_person]
    presents = []

    if score < 50:
        presents = get_presents(2)
    elif score >= 100:
        presents = get_presents(8)
    else:
        presents = get_presents(5)

    return chosen_person, presents

if __name__ == '__main__':
    chosen_person, presents = get_presents_for_person()

    with open('presfile.txt', 'a') as presfile:
        presfile.write(chosen_person)
        presfile.write(str(presents) + '\n')

What's next?
You could make the values of People an actual class. This allows you to further expand upon it if necessary in the future. For example something like:
class Person:

    def __init__(self, name, score):
        self.name = name
        self.score = score


Avoid the object/array check and treat the return value the same way with JavaScript spread operator? I'm trying to implement a function (a template-helper function) which provides a simple merge-like functionality for both arrays and objects. The two functions are basically looks identical:
const mergeArrays = (first, ...rest) =>
  rest.reduce((acc, curr) => ([ ...acc, ...curr]), first);

const mergeObjects = (first, ...rest) =>
  rest.reduce((acc, curr) => ({ ...acc, ...curr }), first);

One could do a type check and a local variable to write a single function like this:
const mergeArraysOrObjects1 = (first, ...rest) => {
  let reducer = null;
  if ('object' === typeof first && null !== first) {
    reducer = (acc, curr) => ({ ...acc, ...curr });
  }
  if (Array.isArray(first)) {
    reducer = (acc, curr) => [...acc, ...curr];
  }

  if (!reducer) {
    throw new Error('The "merge" filter only works with arrays and hashes.');
  }

  return rest.reduce(reducer, first);
}

... but it seems so ugly to me. Another possibility is a type check and inline conditional:
const mergeArraysOrObjects2 = (first, ...rest) => {
  if ('object' !== typeof first || null === first) {
    throw new Error('The "merge" filter only works with arrays and hashes.');
  }

  return rest.reduce(
    (acc, curr) => Array.isArray(acc) ? [...acc, ...curr] : {...acc, ...curr},
    first
  );
}

I'm still learning JavaScript so I'm guessing if there is a more elegant and concise way to treat the return value the same way.

Object.assign && Array.flat
By the first example which does not have any type checking makes the throwing of errors in the second two examples look superfluous, a little over kill.
As the function Array.isArray is the more concise statement over the always ugly object test typeof obj === "Object && obj !== null you can use a simple ternary to call the correct function
You can use Object.assign rather than the long and slow object by object reduce method to merge objects.
And for the array Array.push, then Array.flat(1) depth of 1 for the arrays. To ensure you do not effect the input array first you would also need to copy it to a new array
Examples.
const merge = (first, ...rest) => {
    const arrays = (arr) => (arr.push(...rest), arr.flat(1));
    const objects = () => Object.assign({}, first,  ...rest);
    return Array.isArray(first) ? arrays([[...first]]) : objects();
}

Or a shortened form
const merge = (first, ...rest) => {
    return Array.isArray(first) ?
         (first = [[...first]], first.push(...rest), first.flat(1)) :
         Object.assign({}, first,  ...rest);
}

Update
Fixed bug as pointed out in comments.
From the same comment by ScottyJamison in which is presented an even better solution, using the rest operator on all the arguments resulting in...
const merge = (...items) => Array.isArray(items[0]) ?
    items.flat(1) :
    Object.assign({}, ...items);

or
const merge = (...items) => Array.isArray(items[0]) ? items.flat(1) : Object.assign({}, ...items);


Show input type based on selection I have a small script that checks what the user selects, and based in his selection (1, 2 or 3) show the elements, but I'm not quite happy with the result. I believe that it could be much better the code, and repeat less times, here is my code example.

$(function() {

            //Hide elements
            $('#input').hide();
            $('#textarea').hide();
            $('#option').hide();

            //Show element based on selection
            $('#input_type').change(function(){    // use class or use $('select')

                var option_selected = $(this).val();

                switch(option_selected) {
                    case "1":
                        $('#input').show();
                        $('#textarea').hide();
                        $('#option').hide();

                        break;
                    case "2":
                        $('#textarea').show();
                        $('#input').hide();
                        $('#option').hide();
                        break;
                    case "3":
                        $('#option').show();
                        $('#textarea').hide();
                        $('#input').hide();
                        break;
                    default:
                        $('#input').hide();
                        $('#textarea').hide();
                        $('#option').hide();
                }

            });
        });

The nice thing about the code below is that if there are ever more inputs you only need to add one line for each, though as has been mentioned in the comments the way you do your html and css can decrease the size of your javascript.

$(function() {
    var elements = {
        "1" : "#input",
        "2" : "#textarea",
        "3" : "#option"
    };
    var show_selected = function(selected) {
        for (var key in elements)
            $(elements[key]).toggle(selected === key);
    };
    //Hide elements
    show_selected();

    //Show element based on selection
    $('#input_type').change(function(){    // use class or use $('select')
        show_selected($(this).val());
    });
});

below is an example where your html can reduce your javascript.

<textarea class="input_type" id="textarea"></textarea>
<input class="input_type" id="input" />
<select class="input_type" id="option">
    <option>Option 1</option>
    <option>Option 2</option>
</select>
<select id="input_type">
    <option>-</option>
    <option>input</option>
    <option>textarea</option>
    <option>option</option>
</select>
<script src="https://code.jquery.com/jquery-1.10.2.js"></script>
<script>
$(function() {
    //Hide elements
    $(".input_type").hide();

    //Show element based on selection
    $('#input_type').change(function(){    // use class or use $('select')
        $(".input_type").hide();
        $("#" + $(this).val()).show();
    });
});
</script>


Maintain the ordering of the original array, but group either negative or positive numbers into sub arrays For a code challenge I thought it'd be useful to have a helper function to group positives and negatives into their own sub arrays while maintaining the ordering of the original array. I'm sure there's a better way to do this but wanted to take a crack at it before researching.

This idea came to mind, to track the state of the numbers, either positive or negative, and group them according to state. This is just MVP and I didn't account for 0.

Despite being verbose, does the code make sense> Do you have recommendations for improvement?

(code playground)

//compare two numbers state
function negAndPosCompare(numA: number, numB: number) {
    if (numA > 0 && numB > 0) {
        // we'll call 1 our positive state
        return 1;
    }
    else if (numA < 0 && numB < 0) {
        // we'll call -1 our negative state
        return -1
    }
    else {
        // false is if two numbers are not the same state
        return false;
    }
}

//simply return the state of a single number
function getState (num : number) {
    if (num > 0){
        return 1;
    }
    else if (num < 0) {
        return -1;
    }
}

function subArray_pos_and_neg(arr: number[]) {
    // our final return
    const output: number[][] = [];

    //where we'll hold numbers of the same state, it'll be exclusive, either pos, or neg.
    // we can default it to the very first number. It's a temp because we'll keep recycling it whenever state changes.
    let temp_sameKind = [arr[0]];

    // grab the state right away, are we starting with pos or neg?
    let currentState = getState(arr[0]);

    for (let i = 0; i < arr.length; i++) {
        // check that i is gte -> 1 so we can safely check arr[i-1]
        if (i >= 1) {

            //if the current number compared to the last number are not the same state, then that means the streak of a given state is over
            if (negAndPosCompare(arr[i], arr[i - 1]) === false) {

               //grab the new state.
                currentState = getState(arr[i]);

                //push the numbers of the same state that we've accumulated so far, they belong together and we'll reset our temp holder
                //to accomodate our new state of numbers
                output.push(temp_sameKind);
                temp_sameKind = [];

                //push the current number right away, it is the first number of our new state. this operation is similiar to how we defaulted our
                // temp_sameKind array right off the bat (before the for loop). if we don't push it now it'll go missing.
                temp_sameKind.push(arr[i]);
            }

            // the current number is of the same state as our current streak, therefore we want to include it in our accumulation.
            if (negAndPosCompare(arr[i], arr[i - 1]) === currentState) {
                temp_sameKind.push(arr[i]);
            }
        }
    }
    // this check outside of the for loop is the final push of our accumulated numbers... if we don't accomodate the last state of numbers,
    // they'll be excluded by our previous logic (since the state doesn't change again in the for loop)
    if (temp_sameKind){
        output.push(temp_sameKind);
    }
    return output;
}

subArray_pos_and_neg([1,2,-4,2,5,-6,-3,3,-6,8,9]); //?

I think your code is great, but it is indeed way too verbose. It makes sense, but I got to admit, it took me some time. To me longer code isn't necessarily easier to understand. For instance your if/else really confused me at first.

Personally, I think you could have improved your code by making much shorter. Here's what I came up with :

function createSubArr(A) {
    let output = [];
    for(let i = 0; i < A.length; i++) {
        let subArr = [A[i]];
        while(A[i] > 0 === A[i + 1] > 0) {
            subArr.push(A[i + 1]);
            i++;
        }
        output.push(subArr);
    }
    return output;
}

console.log(createSubArr([-1, -2, -3, 1, -1, -2, 2, 3, 4, -9, 5, 6, 7]))


List of classes to datatable I'm looking for a function to take a list of generic classes, and map it to different layout of datatable.

That means, I can't just directly map my class property names and values to grid, I need to convert it to the new values.

The solution that I came up with works, but it feels ... sloppy. Can anyone recommend a better way of doing it?

class Program
{
    static void Main(string[] args)
    {
        var list = new List<InputClass>()
        {
            new InputClass {SomeField = "A", OtherField = 1},
            new InputClass {SomeField = "B", OtherField = 2},
            new InputClass {SomeField = "C", OtherField = 3},
            new InputClass {SomeField = "D", OtherField = 4}
        };

        var dict = new Dictionary<string, Func<InputClass, object>>()
        {
            {"ColA", (f) => $"{f.SomeField}ABCD"},
            {"ColB", (f) => DateTime.Now}
        };

        var dt = ToDataTable(list, dict);
        Console.ReadKey();
    }

    public static DataTable ToDataTable<T>(List<T> list, Dictionary<string, Func<T, object>> columns) where T : class
    {
        var dt = new DataTable();

        foreach (var key in columns.Keys)
        {
            var func = columns[key];
            Type[] funcParams = (func.GetType()).GenericTypeArguments;
            dt.Columns.Add(key, funcParams[1]);
        }

        foreach (var record in list)
        {
            var objects = new List<object>();
            foreach (var key in columns.Keys)
            {
                objects.Add(columns[key](record));
            }

            dt.Rows.Add(objects.ToArray());
        }

        return dt;
    }
}

class InputClass
{
    public string SomeField { get; set; }

    public int OtherField { get; set; }
}

Cases like this I would hide the "sloppy"

I would create a class that holds the mappings from the class to the table and create methods to build the mappings.  This way there is no need for reflection and we can save the funcs to be reused.

public class Mapper<TSource>
{
    private readonly DataTable _source = new DataTable();
    private readonly List<Func<TSource, object>> _mappings = new List<Func<TSource, object>>();

    public Mapper<TSource> Configure<TProperty, TData>(Func<TSource, TProperty> property, string colName,
        Func<TProperty, TData> mapper)
    {
        _source.Columns.Add(colName, typeof (TData));
        Func<TSource, object> map = s => mapper(property(s));
        _mappings.Add(map);
        return this;
    }

    public Mapper<TSource> Configure<TProperty>(Func<TSource, TProperty> property, string colName)
    {
        _source.Columns.Add(colName, typeof(TProperty));
        Func<TSource, object> map = s => property(s);
        _mappings.Add(map);
        return this;
    }

    public Mapper<TSource> Configure<TData>(string colName, Func<TData> mapper)
    {
        _source.Columns.Add(colName, typeof(TData));
        Func<TSource, object> map = _ => mapper();
        _mappings.Add(map);
        return this;
    }

}

With this class we now have a datatable in _source that holds the columns that we want and a list of mappings from our source to the datatable.   Now we just need to create a ToDataTable method to use these in the same class.

public DataTable ToDataTable(IEnumerable<TSource> items)
{
    // make a new datatable with the same columns as the source
    var dt = _source.Clone();

    foreach (var item in items)
    {
        dt.Rows.Add(_mappings.Select(f => f(item)).ToArray());
    }

    return dt;
}

We would use it like so.  Typically you would only do the configurations once and then just reuse it with different lists.

var inputClassMapper = new Mapper<InputClass>();
inputClassMapper.Configure(f => f.SomeField, "ColA", s => $"{s}ABCD")
                .Configure("ColB", () => DateTime.Now);

// have mapper convert input to datatables
var dt = inputClassMapper.ToDataTable(list);


Complex search program I am working on a fairly complex (at least it feels complex to me at the moment) search program and am looking to possibly increase the performance. Everything works exactly how I want it to, but I'm just wondering if there are any slight performance increases I could benefit from. In this code, even minor performance increases are essential.  This is due to the extensive amount of operations being performed.

// Split search into multiple terms and check the {x} longest terms against the cache.
string[] word = searchTerm.Split(' ');
Array.Sort(word, (x, y) => y.Length.CompareTo(x.Length));
for (int i = 0; i < word.Length; i++)
{
    string searchValue = word[i];
    if (i <= MAX_WORD_ITERATIONS && (xmlsearchResults == BLANK_SEARCH_XML_SCHEMA || xmlsearchResults == string.Empty))
    {
        xmlsearchResults = GetCachedRecord(thisSearch, searchValue);
    }
    if (xmlsearchResults != BLANK_SEARCH_XML_SCHEMA && xmlsearchResults != string.Empty)
    {
        thisSearch.Value = searchValue;
        ignoreCache = true;
        break;
    }
}

xmlString.Append(string.Format(@"<{0}>", xmlHeader));
int objcount = 0;
string prevResultName = string.Empty;
try
{
    if (searchResults.hitCount > 0)
    {
        List<SearchItem> SearchResults = new List<SearchItem>();
        foreach (Node ResultNode in Results.Nodes)
        {
            string code = ResultNode.code.ToString();
            string result = ResultNode.id.ToString();
            string name = ResultNode.name_l.ToString();
            string image = string.Empty;

            if (!code.ToLower().Contains("ccb") && code.Length <= MAX_CODE_LENGTH &&
                name != prevResultName && objcount < maxResults)
            {
                Boolean addResult = true;
                if (thisSearch.FilterDescription)
                {
                    if (!name.ToLower().Contains(thisSearch.Value.ToLower()))
                    {
                        addResult = false;
                    }
                }
                if (addResult)
                {
                    SearchItem oSearch = new SearchItem(name, code);
                    SearchResults.Add(oSearch);
                    prevResultName = name;
                }
            }
        }
        var SortedSearchResults = SearchResults.OrderByDescending(s => s.Downloads).ToList();
        for (int i = 0; i < SortedSearchResults.Count(); i++)
        {
            if ((objcount < maxResults))
            {
                string te = string.Format("<ResultName>{0}</ResultName>", SortedSearchResults[i].Description);
                if (!xmlString.ToString().Contains(te))
                {
                    xmlString.Append(SortedSearchResults[i].GetXMLString());
                    objcount += 1;
                }
            }
        }
    }
}
xmlString.Append(string.Format(@"</{0}>", xmlHeader));
return objcount;

Comparing strings case insensitively is done by using the proper overload:

string.Compare(str1, str2, StringComparison.OrdinalIgnoreCase)

By calling .ToLower() for each string to compare (4 times for each iteration), you create 4 new strings each time. If you have a lot of (long) strings, this will create a lot of garbage for no reason.

In your case you use .Contains() so you can mimic this by using .IndexOf():

str4.IndexOf("dyt2", StringComparison.OrdinalIgnoreCase) >= 0

It's prettier to use bool instead of Boolean, partly because it also keeps your code consistent.

Write your variable names in full: objectCount, previousResultName, etc.

Constants are PascalCase, not WHATEVER_THIS_IS as in Java.

The addResult variable seems a little wonky. Instead of this:

Boolean addResult = true;
if (thisSearch.FilterDescription)
{
    if (!name.ToLower().Contains(thisSearch.Value.ToLower()))
    {
        addResult = false;
    }
}
if (addResult)
{
    SearchItem oSearch = new SearchItem(name, code);
    SearchResults.Add(oSearch);
    prevResultName = name;
}

Why not this:

if (!(thisSearch.FilterDescription && !name.ToLower().Contains(thisSearch.Value.ToLower()))
{
    SearchItem oSearch = new SearchItem(name, code);
    SearchResults.Add(oSearch);
    prevResultName = name;
}

You can clean this up a bit more but I'm not in a boolean algebra mood.


Counting matrix elements that have "4" or "5" as a neighbor I have a matrix (500x500) of integers. For each entry, I need to look at its surrounding neighbours (so 8 elements) and determine what the integers are, and run a function on these integers.

Here what my function looks like right now (R code with the RCpp package).

fx2<- cppFunction('NumericMatrix getNeighbours(NumericMatrix x) {
            int nrow = x.nrow(), ncol = x.ncol();

            //create a result matrix of probabilities
            NumericMatrix outProb(nrow, ncol);

            // some counters needed to calculate probability
            int kc = 0; int kpre = 0; int ks = 0;

            // i and j loop through the elements one by one.
            //  - ignore the boundary as I dont know how to handle it for now
            for (int i = 1; i < (nrow-1); i++) {
              for (int j = 1; j < (ncol-1); j++) {

                // go through the neighbours
                for(int k = -1; k <= 1; k++){
                  for(int l = -1; l <= 1; l++){

                    // check what values the neighbours are

                    // increment the kc
                    if(x(i + k, j + l) == 4){
                      kc++;
                    }

                    // increment the ks counter
                    if(x(i + k, j + l) == 5){
                      ks++;
                    }

                    // more if statements removed for readibility

                  }
                } //end of loop for neighbours

                // calculate a probability for the resulting matrix
                outProb(i, j) = functionof(kc, ks, kpre);
              }
            }
            return outProb;
            }')

I am looking to make this code faster/more efficient.

Example of result: Suppose the functionof just doubles the value of the counter.

> health
     [,1] [,2] [,3] [,4] [,5]
[1,]    4    1    1    1    1
[2,]    1    1    1    1    1
[3,]    1    1    1    1    1
[4,]    1    1    1    1    1
[5,]    1    1    1    1    1

We see that (2, 1), (1, 2) and (2, 2) have 4 as their neighbours. So the output would be

     [,1] [,2] [,3] [,4] [,5] [,6]
[1,]    0    2    0    0    0    0
[2,]    2    2    0    0    0    0
[3,]    0    0    0    0    0    0
[4,]    0    0    0    0    0    0
[5,]    0    0    0    0    0    0
[6,]    0    0    0    0    0    0

Note that there are other elements in the matrix that have 4 as a neighbour. I've omitted that from the result matrix above.

I currently wrote the code in R but even after vectorizing/parallelizing it is still very slow. So now I've moved on to a C++ function that I can call using R's RCpp package.

This is how you could write your code efficiently in base R:

offset <- function(x, v_offset = 0L, h_offset = 0L) {
   stopifnot(h_offset %in% -1:1, v_offset %in% -1:1)
   nr <- nrow(x)
   nc <- ncol(x)
   if (h_offset == +1L) x <- cbind(x[, -1], -99L)
   if (h_offset == -1L) x <- cbind(-99L, x[, -nc])
   if (v_offset == +1L) x <- rbind(x[-1, ], -99L)
   if (v_offset == -1L) x <- rbind(-99L, x[-nr, ])
   x
}

getNeighbours <- function(x, functionof) {
   v_offsets <- c(-1, -1, -1,  0,  0, +1, +1, +1)
   h_offsets <- c(-1,  0, +1, -1, +1, -1,  0, +1)
   neigh_x   <- Map(offset, list(x), v_offsets, h_offsets)
   kc   <- Reduce(`+`, Map(function(x) x == 4L, neigh_x))
   ks   <- Reduce(`+`, Map(function(x) x == 5L, neigh_x))
   kpre <- 0
   functionof(kc, ks, kpre)
}

It builds a list (neigh_x) of eight matrices in memory, one for each neighboring position. Instead of four loops, you are down to a single loop (8 iterations) wherever you see Map or Reduce, everything else uses vectorized operations.

Testing the code:

health <- matrix(1, 5, 5)
health[1, 1] <- 4
test_fun <- function(kc, ks, kpre) 2 * (kc + ks + kpre)

getNeighbours(health, functionof = test_fun)
#      [,1] [,2] [,3] [,4] [,5]
# [1,]    0    2    0    0    0
# [2,]    2    2    0    0    0
# [3,]    0    0    0    0    0
# [4,]    0    0    0    0    0
# [5,]    0    0    0    0    0


Grouping consecutive numbers into ranges in Python 3.2 The following is a function that I wrote to display page numbers as they appear in books.

If you enter the list [1,2,3,6,7,10], for example, it would return:

1-3,6-7,10

This seemed like a nice example of how the dictionary data type can be used in Python.

Is there is an even more code-efficient way to do this?

def formatpagelist (numberlist):
    tempdic={}
    returnstring=''

    for number in numberlist:
        if number-1 in tempdic.keys():
            tempdic[number-1]=number
        elif number-1 in tempdic.values():
            for key in tempdic.keys():
                if number-1==tempdic[key]: foundkey=key
            tempdic[foundkey]=number
        else:
            tempdic[number]=0

    keylist=list(tempdic.keys())
    keylist.sort()

    for key in keylist:
        if tempdic[key]>0:
            returnstring+=(str(key)+'-'+str(tempdic[key])+',')
        else: returnstring+=str(key)+','

    return returnstring

You could use this one-liner to generate groups of consecutive integers in a list:

from itertools import groupby, count

groupby(numberlist, lambda n, c=count(): n-next(c))

Then to finish it off, generate the string from the groups.

def as_range(iterable): # not sure how to do this part elegantly
    l = list(iterable)
    if len(l) > 1:
        return '{0}-{1}'.format(l[0], l[-1])
    else:
        return '{0}'.format(l[0])

','.join(as_range(g) for _, g in groupby(numberlist, key=lambda n, c=count(): n-next(c)))
# '1-3,6-7,10'

This assumes they are in sorted order and there are no duplicates.  If not sorted, add a sorted() call on numberlist beforehand.  If there's duplicates, make it a set beforehand.


Initialize a hash with all 0 values based on two arrays Given two arrays (for instance a list of people and a list of subjects), I need to initialise a Hash with all zeros for all possible combinations of these two arrays. I can manage to do that with the code on the bottom, but it seems like there should be a less convoluted, more elegant way to do this.

people = %w(tom mary rob)
subjects = %w(math english)

Hash[people.map { |person| [person, Hash[subjects.map { |subject| [subject, 0] }]]}]

# output: => {"tom"=>{"math"=>0, "english"=>0},
# "mary"=>{"math"=>0, "english"=>0},
# "rob"=>{"math"=>0, "english"=>0}}

Conceptually, that's the way to go. I'd just propose some small changes:

Use Array#to_h instead of Hash#[]
This line is too long, split it in multiple lines.

So I'd write:

scores = people.map do |person|
  person_scores = subjects.map { |subject| [subject, 0] }.to_h
  [person, person_scores]
end.to_h


Minimal Property system in c++17 So I needed to represent by objects in terms in ini format and change their values from the ini as well, so I thought why don't implement a property system for this. I want it to be bit fast so I would really like if anyone can remove the std::function if possible but its not necessary. The target application is GUI so small performance should not be a big issue, but still, I think this can be done in a faster way but I can't think of any. no one likes lag. Also, the properties are around 100 or more

#include <functional>
#include <stdexcept>
#include <string>
#include <type_traits>
#include <unordered_map>
#include <variant>

#include <QDebug>

template <typename C> class PropertySet {
public:
    using PropertyValue = std::variant<bool, int, float, std::string>;

    void set(const std::string &prop, PropertyValue v) {
        auto p = m_properties.find(prop);
        if (p == m_properties.end()) {
            throw std::runtime_error("Invalid property key");
        }
        // p.value().setter(v);
        p->second.setter(v);
    }

    void set(const std::string &prop, const std::string &v) {
        auto p = m_properties.find(prop);
        if (p == m_properties.end()) {
            throw std::runtime_error("Invalid property key");
        }
        // p.value().setter(p.value().fromStr(v));
        p->second.setter(p->second.fromStr(v));
    }

    std::unordered_map<std::string, std::string> getAllValues() {
        std::unordered_map<std::string, std::string> r;
        qDebug() << m_properties.size();
        for (const auto &p : m_properties) {
            // r[p.key()] = p.value().toStr();
            r[p.first] = p.second.toStr();
        }
        return r;
    }

private:
    struct Payload {
        std::function<std::string()> toStr;
        std::function<PropertyValue(const std::string &)> fromStr;
        std::function<PropertyValue()> getter;
        std::function<void(PropertyValue)> setter;
    };
    std::unordered_map<std::string, Payload> m_properties;

protected:
    template <typename T> static auto fromStr(const std::string &s) {
        if constexpr (std::is_same<T, int>::value) {
            return std::stoi(s);
        } else if constexpr (std::is_same<T, bool>::value) {
            return s == "true" || s == "1";
        }
        static_assert("Error");
    }

    template <typename T, typename Getter, typename Setter>
    void add(const std::string &name, Getter getter, Setter setter) {
        auto cptr = static_cast<C *>(this);
        Payload p;
        p.toStr = [getter, cptr]() { return std::to_string(std::invoke(getter, cptr)); };
        p.fromStr = [](const std::string &s) { return PropertySet::fromStr<T>(s); };
        p.getter = [getter, cptr]() { return PropertyValue(std::invoke(getter, cptr)); };
        p.setter = [setter, cptr](PropertyValue v) { std::invoke(setter, cptr, std::get<int>(v)); };
        m_properties[name] = p;
    }
};

Also, I'm using Qt, so I was thinking of replacing std::string with QString or QByteArray and std::unordered_map with QHash.

There are several ways to approach this.

The classic object oriented way is to make Payload be a simple interface class

class IPayload {
    virtual ~IPayload() = default;
    virtual std::string   toStr() = 0;
    virtual PropertyValue fromStr(const std::string &) = 0;
    virtual PropertyValue get() = 0;
    virtual void          set(PropertyValue) = 0;
};

And just create for each property its own instance of an implementation of the interface - depending on what kind of property one uses. This can be helpful when you need variety updating mechanisms to work simultaneously. Say, one Payload that stores the field only in memory, one that stores into both in memory and in a file, and one that forwards it to GUI - or a combination of these. This can be also helpful when you require specialized treatment for some properties - like time input property - that upon receiving std::string input it processes it according to some time conversion, i.e., read double and then converts it to seconds depending on the time unit specification, e.g., "5min" converts to 300 (seconds). The only issue is that you ought to instantiate a treatment for each property - which can be a hassle and make the class unsuitable for some trivial basic usages.

I don't see any circumstances when the std::function approach would better - unless one has to runtime change setters or something - which is kinda odd honestly.

Also you can try going in the opposite direction and detach the storage class from the "customized actions". Say, make a simple multipurpose ini file parser wrapping std::unordered_map<std::string,std::string> - or use an existing one like boost::ptree that can read and write xml and json in addition. While in the GUI simply call the right functions when user sets the fields. Whether it is viable / not viable / preferred option depends a lot on your development platform for the GUI as well as other nuances.

You can in fact combine the two methods. Use the boost::ptree for storing / parsing and for internal usage whilst class PropertySet make responsible for the GUI handling and just instantiate one from each other upon loading / saving / updating.


How can I refactor this method so I don't use it in multiple places? I have the following method which I use in multiple places, but it only differs by a few different things, so I was wondering how I can refactor it so I can have it in a common class and call it from there everywhere.

public override DataTable GetRecords(QueryContext queryContext, out int totalRecords)
{
    // Build Query
    //Different
    StringBuilder query = new StringBuilder("SELECT * FROM TABLE");
    Dictionary<string, string> parameters = new Dictionary<string, string>();

    if (queryContext.OrderByColumns.Count == 0)
    {
        //Can very in length or number of parameters
        queryContext.OrderByColumns.Add("param_1"); //different
        queryContext.OrderByColumns.Add("param_2"); //different
    }

    if (queryContext.Parameters.Count > 0)
    {
        foreach (QueryParameter p in queryContext.Parameters)
        {
            dataAccess.paramAdd(parameters, query, p.ColumnName, p.Value.ToString());
        }
    }

    // Order By Clause
    query.Append(queryContext.OrderByColumns.GetSqlClause());

    // Apply Limit
    if (queryContext.ApplyLimit)
    {
        query.AppendFormat(" LIMIT {0},{1}", queryContext.Offset, queryContext.Limit);
    }

    //Execute the query.
    DataSet results = dataAccess.ExecuteQuery(query.ToString(), parameters);
    totalRecords = Convert.ToInt32(results.Tables[1].Rows[0][0]);
    return results.Tables[0];
}

The only differences in other places are the value of the query variable and the paramters added by queryContext.OrdByColumn.Add(...).  Other than that, everything is the same.

My first shot was going to be doing something like:

public override DataTable GetRecords(StringBuilder query, string[] orderByParams, QueryContext queryContext, out int totalRecords)
{

    Dictionary<string, string> parameters = new Dictionary<string, string>();

    if (queryContext.OrderByColumns.Count == 0)
    {
        foreach(var param in orderByParams)
        {
            queryContext.OrderByColumns.Add(param);
        }
    }

    if (queryContext.Parameters.Count > 0)
    {
        foreach (QueryParameter p in queryContext.Parameters)
        {
            dataAccess.paramAdd(parameters, query, p.ColumnName, p.Value.ToString());
        }
    }

    // Order By Clause
    query.Append(queryContext.OrderByColumns.GetSqlClause());

    // Apply Limit
    if (queryContext.ApplyLimit)
    {
        query.AppendFormat(" LIMIT {0},{1}", queryContext.Offset, queryContext.Limit);
    }

    //Execute the query.
    DataSet results = dataAccess.ExecuteQuery(query.ToString(), parameters);
    totalRecords = Convert.ToInt32(results.Tables[1].Rows[0][0]);
    return results.Tables[0];
}

LINQ is available to me, so if that can improve it, I am welcome to ideas using that too.

For this, I'll be addressing the second code snippet you've given, as it seems to be heading in the right track.

I'm assuming QueryContext is custom code, because I couldn't find any reference to it in the MSDN and you haven't provided any other tags. In a case like this it would probably be useful to see the code for this class to better optimize your usage, however I do still have a few points. The same goes with what the dataAccess variable is.

Naming

I don't like the name you've given to your second foreach loop's variable. 'p' is confusing and will be even more so if you ever expand the body of your loop. I recommend something like queryParameter.

Implicit Typing

This tends to be a personal thing, but I strongly prefer the use of 'var' when the RHS of the variable declaration makes it obvious what the type is. It saves you having to change the type in two places when refactoring. However you use it half of the time, go all in or not at all.

Out parameters

Always a sign that you should probably make a class and return it.

If you still want an explicit number of results and you don't fancy going to that effort of making a custom class, then you could always use Tuple but a custom class is preferable.

Separation of Concerns

Seems this one function is doing 2 things: building a query, and executing a query. This would be better if they were separate, which allows you more flexibility.

Results

With all this on board, your code becomes:

public class QueryResult
{
    public DataTable Data {get; set;}

    public int RecordCount {get; set;}

    public QueryResult(DataTable data, int recordCount)
    {
        this.Data = data;
        this.RecordCount = recordCount;
    }
}

public QueryResult GetRecords(Query query)
{
    DataSet results = dataAccess.ExecuteQuery(query.statement, query.parameters);
    return new QueryResult(results.Tables[0], results.Tables[1].Rows[0][0]);
}

public class Query
{
    public string Statement {get; set;}
    public Dictionary<string,string> Parameters {get; set;}

    public Query(string statement, Dictionary<string,string> parameters)
    {
        this.Statement = statement;
        this.Parameters = parameters;
    }
}

public Query BuildQuery(StringBuilder statement, string[] orderByParams, QueryContext queryContext)
{
    var parameters = new Dictionary<string, string>();

    if (queryContext.OrderByColumns.Count == 0)
    {
        foreach(var orderbyParameter in orderByParams)
        {
            queryContext.OrderByColumns.Add(param);
        }
    }

    if (queryContext.Parameters.Count > 0)
    {
        foreach (var queryParameter in queryContext.Parameters)
        {
            dataAccess.paramAdd(parameters, statement, queryParameter.ColumnName, queryParameter.Value.ToString());
        }
    }

    // Order By Clause
    statement.Append(queryContext.OrderByColumns.GetSqlClause());

    // Apply Limit
    if (queryContext.ApplyLimit)
    {
        query.AppendFormat(" LIMIT {0},{1}", queryContext.Offset, queryContext.Limit);
    }

    return new Query(statement.ToString(), parameters);
}

You may also want to make the properties in QueryResult readonly depending on what you're hoping to do with them later.


Python script that launches a process that it needs to clean up after and kill on receiving SIGTERM/SIGINT I'm using a Python script to run a particular program that occasionally may need to be terminated (by receiving a SIGINT or SIGTERM) in which case a clean up process needs to be invoked (before killing the original process).

To test it I start the program in question and then I send a SIGINT to terminate it.

A signal handler is defined for SIGINT and SIGTERM that will launch a clean up program that takes as input a filepath output by the original process in its STDOUT. Afterwards it'll send a SIGTERM to the original process.

That clean up may hang in which case I raise a SIGALRM where I kill both the original process and the clean up process.

The problem is I'm not sure if I've covered all corner cases and I'm not really good at handling OS facilities (especially dealing with signals). For example, If I send SIGINT several times it terminates abruptly even though technically I have a flag to prevent SIGINT from being handled more than once.

EDIT: Included changes to the original code, such as the submission of subprocesses under process groups

Here's a stripped down version of the code:

import os
import sys
import signal
import subprocess

SUBMISSION_PROCESS = None # reference to the original process
CLEANUP_PROCESS    = None # reference to the clean up process
FILE_PATH          = '' # clean up process needs this from original process
HANDLER_CALLED     = False # to prevent signal handler from being called again

def kill_everything(signum, stack): # in case both processes hang...
    print 'Timed out, will kill all processes...'
    if SUBMIT_PROCESS:
        os.killpg(SUBMIT_PROCESS.pid, signal.SIGKILL)
    if CLEANUP_PROCESS:
        os.killpg(CLEANUP_PROCESS.pid, signal.SIGKILL)

def kill_submission_process(signum, stack):
    global HANDLER_CALLED
    global CLEANUP_PROCESS
    print 'Caught signal %d, please wait...' % signum

    if HANDLER_CALLED:
        print 'Already handling %d.' % signum
        return

    HANDLER_CALLED = True

    if FILE_PATH:
        stop_session_call = ['clean_up', '-arg_1', FILE_PATH]
        try:
            signal.alarm(120) # set a timeout for clean up session in case that fails too
            CLEANUP_PROCESS = subprocess.Popen(stop_session_call, preexec_fn=os.setsid)
            print 'Clean up process has started...'
            CLEANUP_PROCESS.communicate()
        except OSError as e:
            print >> sys.stderr, 'Unable to clean up session: an OSError was raised: %s' % e
    else:
        print >> sys.stderr, 'Unable to clean up session: file path not defined.'
    if SUBMISSION_PROCESS and SUBMISSION_PROCESS.poll() is None: # clean up original process if set and still running
        print 'Will clean up original submission process...'
        SUBMISSION_PROCESS.terminate()
        print 'Done, will quit.'
    raise SystemExit

if __name__ == '__main__':
    signal.signal(signal.SIGINT,  kill_submission_process)
    signal.signal(signal.SIGTERM, kill_submission_process)
    signal.signal(signal.SIGALRM, kill_everything)

    to_run = ['command', '-arg_1', 'param_1', '-arg_2', 'param_2']

    try:
        SUBMIT_PROCESS = subprocess.Popen(to_run, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, preexec_fn=os.setsid)

        # Trying to parse process output to identify a particular file path
        while True:
            line = SUBMISSION_PROCESS.stdout.readline()
            if SUBMISSION_PROCESS.poll() != None and line == '': # if no more to read
                break
            line = line.rstrip('\n')
            print line
            if line[-4:] == '.ext': # line contains file path in question
                FILE_PATH = line # store it
                print 'File path found and saved.'
    except (OSError, IOError) as e:
        print >> sys.stderr, 'Exception raised while executing %s: %s.' % (' '.join(map(str, to_run)), e)
        raise SystemExit

UPPER_CASE is reserved for constants, and your globals assuredly aren't constants. If you wish for a cleaner solution, you could possibly make a class and pass in instance methods to signal.signal.

Your final raise SystemExit seems unneeded.

You can use line.endswith('.ext') instead of slicing.

It's hard to give more detailed analysis without a better understanding of what the code is trying to do.


Scopes chaining by OR How can I DRY these scopes?

  scope :reputed, -> {
    joins{reputations.outer}.
    where{['coalesce(rs_reputations.value, 0) > ? OR purchases.force_active = ?', LOWEST_VOTE, true]}
  }

  scope :reputed_or_mine, ->(user) {
    joins{reputations.outer}.
    where{['coalesce(rs_reputations.value, 0) > ? OR purchases.force_active = ? OR purchases.user_id = ?', LOWEST_VOTE, true, user.id]}
  }

'coalesce(rs_reputations.value, 0) > ? OR purchases.force_active = true OR (? AND purchases.user_id = ?)',
LOWEST_VOTE,
!user.id.nil?,
(if user.id.nil? then -1 else user.id end)

This is semantically equivalent to both, but the AND in the third clause prevents it from having an effect when there is no user.id.


Text tokenizer - extract words and positions from text I have Set of character delimiters (DELIMITERS), eg . ,  etc. Using this I want to split text and get words with their position in text.
String.split() works fine if you want only words. The same with StringTokenizer. Wrote some simple method to deal with this, but maybe there is a better way to achieve this result?

public List<String> extractWords(String text){
    List<String> words = new ArrayList<>();
    List<WordPos> positions = new ArrayList<>();
    int wordStart = -1;
    for(int i=0; i < text.length(); i++){
        if(DELIMITERS.contains(text.charAt(i))){
            if(wordStart >=0){ //word just ended
                String word = text.substring(wordStart, i);
                positions.add(new WordPos(wordStart, i));
                words.add(word);
            }
            wordStart = -1;
        }else{ //not delimiter == valid word
            if(wordStart < 0){ //word just started
                wordStart = i;
            }
        }
    }
    return words;
}

// inner static class for words positions
public static class WordPos{
    int start;
    int end;
    public WordPos(int start, int end){
        this.start = start;
        this.end = end;
    }
}

You shouldn't shorten class names. So WordPos should become WordPosition.

Right now you are creating WordPos items which you add to a List<WordPos> which is lost after returning from the method.

Instead of using substring() you could use a StringBuilder object, where you append if the current char is contained in DELIMITERS.

You would need to take care (which you don't do right now) about the case that the text ends without any of your delimiting chars.

public List<String> extractWords(String text) {
    List<String> words = new ArrayList<>();
    StringBuilder sb = new StringBuilder(text.length());

    for (int i = 0; i < text.length(); i++) {
        char current = text.charAt(i);

        if (DELIMITERS.contains(current)) {
            if (sb.length() > 0) {
                words.add(sb.toString());
                sb.setLength(0);
            }
        } else {
            sb.append(current);
        }
    }
    if (sb.length() > 0) {
        words.add(sb.toString());
    }
    return words;
}


Managing FutureTask and ExecutorService I have many unrelated tasks to execute and I have to get the result of the task so I use the FutureTask.

I submit all the task to the thread pool and use a map to keep track of the task and the FutureTask to get the result. And two for loops are used:

ExecutorService es = Executors.newCachedThreadPool();

Map<Task, FutureTask<Boolean>> taskResultMap = new HashMap<Task, FutureTask<Boolean>>();

for (int i = 0 ; i < callerList.size(); i++)
{
    Caller caller = callerList.get(i);
    FutureTask<Boolean> futureTask = new FutureTask<Boolean>(caller);
    es.submit(futureTask);
    taskResultMap.put(caller, futureTask);

}

for (int i = 0 ; i < callerList.size(); i++)
{
    Caller caller = callerList.get(i);
    FutureTask<Boolean> task = taskResultMap.get(caller);
    System.out.println(caller.getName() + " result is " + task.get());
}

Is this a good practice about ExecutorService and FutureTask? I'm new in multi-thread programming.

Another two questions:

How can I get the execution time for each thread? print in caller iteself or outside caller?
When I invoke the FutureTask.get() method in for loop, I think the get() method may be blocked, If a time-consuming task is running, I can't get the result of the next task but to wait. How could I do this?

The official way to support a system where you control multiple Future instances from an ExecutorService is similar to what @ferada has suggested, you use a 'decorator pattern' to wrap the task, and track the results. But, the Java library has this system built in to the ExecutorCompletionService. Using that service you are able to feed tasks to the service, and wait for tasks to complete.

Your code as it is looks functional, and you successfully wait for all tasks to complete. The problem is that you are not waiting for them in the right order (you are waiting in submit-order, not completion-order).

Since the CompletionService returns a Future on submit, and the exact same one on completion, you can use that as a key to a timing Map... using an IdentityHashMap on the Future would be appropriate because it removes any dependency on the actual implementation of Future.

Note that you can use an 'enhanced-for' loop to get the Caller instances from the list.

Consider the following code:

ExecutorService es = Executors.newCachedThreadPool();
ExecutorCompletionService<Boolean> ecs = new ExecutorCompletionService<>(es);
Map<Future,Long> times = new IdentityHashMap<>();
Map<Future,Caller> jobs = new IdentityHashMap<>();

for (Caller caller : callerList) {
    Future<Boolean> f = ecs.submit(caller);
    times.put(f, System.currentTimeMillis());
    jobs.put(f, caller);
}

while (!times.isEmpty()) {
    Future<Boolean> future = ecs.take();
    // note, remove the future from the map.
    long time = System.currentTimeMillis() - times.remove(future);
    System.out.println(jobs.get(future) + " result is "
         + future.get() + " in " + time + "ms");
}

One important note here, is that I would not actually print the output of the result in the loop, but just record the details. The println is probably slow, and that would make the timing results biased. I would accumulate the completion data in a different way, and print it all after all tasks are done, or, alternatively, in a different thread.


Extract last record inserted for each key identifier I have a table with this schema

ID, int primary key
PathKey, string not null
InsertDate, datetime not null
Value, int not null

In the table there are a medium-large number of records. 1632 different pathkey and 645627 total records
The PathKey is not unique (because I store also the old values).
You can identify a record with the ID or a PathKey with the
InsertDate.

I am developing a query to extract the last record for each path key, and this is the query I am using but is ugly and incredible slow.

SELECT *
FROM ArchiveData
WHERE ID IN (
        SELECT (
                SELECT TOP 1 ID
                FROM ArchiveData
                WHERE PathKey = AD.PathKey
                ORDER BY [InsertDate] DESC
                ) AS ArchiveDataID
        FROM ArchiveData AS AD
        GROUP BY PathKey
        )
ORDER BY PathKey

Any suggestions to improve, at least, the performance?

Using a CTE (Supported by SQL Server) would help separate the logic of the query better than the sub-select in the from clause. Using the auto-increment ID is also something that can simplify the query.

Consider the following:

with MostRecent as (
    select max(ID) as ID
    from ArchiveData
    group by PathKey
)
select *
from ArchiveData inner join MostRecent on MostRecent.ID = ArchiveData.ID
order by PathKey

The above should reduce the number of joins a lot, and use a better key for the joins that are done.


Universal string conversion Below there is a function eating all strings and convert it to Unicode. It works fine. But is it the best way to do so? Is there maybe already an existing function for it?

The reason was a German Windows version which prints German ä ö ü - error-messages which are retrieved from popen/communicate etc. There must have been someone before me with the same issue.

# python2.7
def all_eating_unicode_converter(input_string):
    test1 = 'äöü'
    test2 = b'\xe4\xf6\xfc'
    test3 = u'äöü'
    """
    Converts every input to unicode. tested with testcases abouve!
    :param input_string: may be a string or unicode or bytes.
    :return: returns unicode
    """

    l.debug("type: %s\n message:%s" % (type(input_string), input_string))
    if type(input_string) is not unicode:
        # i don't need to decode unicode. because it already is!
        output_string = input_string.decode("utf-8")  # converts bytes to unicode
    else:
        output_string = input_string
    return output_string

First, you should not be using Python 2 anymore, if at all possible. It will reach end of support at the end of the year.

In Python 3 your code would just be:

def all_eating_unicode_converter(input_string):
    """
    Converts every input to unicode.
    :param input_string: may be a string or unicode or bytes.
    :return: returns unicode

    Tested with: 'äöü', b'\xc3\xa4\xc3\xb6\xc3\xbc', u'äöü'
    """
    if isinstance(input_string, bytes):
        return input_string.decode("utf-8")
    return input_string

Note that I used isinstance instead of type, to also allow derived types of bytes to work properly.

I also shortened the logic by not insisting on having a single return from the function. Here it is clear enough what is happening, and I am in general a fan of early returns.

Docstrings only work when they are actually right after the function header, so I moved the testcases inside the docstring.

In addition, one reason the logging module is so powerful is that it allows you to write log messages which are costly to print and never print them. In other words the replacement of the placeholders is only performed if the message is actually being printed, i.e. if the log level is high enough. So you should always do this:

l.debug("type: %s\n message:%s",  type(input_string), input_string)


$.fn.do() method: Execute other code within a jQuery chain What I want to do is; without leaving the chain, execute some custom/external code (and continue) within the chain. Otherwise; in a deep chain, I will have to re-query for the DOM element to get back where I've left.

The method below allows to execute custom code inside a jQuery chain synchronously or asynchronously.

//Execute code inside a jQuery chain
$.fn.do = function (callback, async) {
    if (typeof callback === 'function') {
        if (async === true) {
            this.queue(function() {
                callback($(this));
                $(this).dequeue();
            });
        } else {
            callback(this);
        }
    }
    return this;
};

Simplified Usage Example:

var results = {};
$('#some-elem')
    .css({
        //modify some styles here
    })
    .do(function($elem) {
        //do some calculations.. tests here..
        //save them to results object
    }, false) //synchronous
    .css({
        //re-set the styles back to their initial state
    });

This could be a longer chain where you could need to enter the do() method multiple times.

So;

Do you think the approach above is legitimate?
Would you suggest an alternate? Why?

I guess this plugin could be useful, however I think you can drop the async part since it's not any harder to call queue directly.

Also, I am not a huge fan of error swallowing or ignoring invalid calls since it makes the code harder to debug. If there's a call to do without providing a callback, you should let the developer be notified.

I also like to allow defining the this value for the callback function so that you do not need to use $.proxy for that purpose.

Finally I allowed to return a value from the callback to change the target object for the rest of the chain. However I am not so sure about this feature since it could harm code comprehension but I am leaving it there as an idea.

Basically it would be as simple as:

!function($) {
    $.fn.do = function (callback, thisArg) {
        if (typeof callback !== 'function') {
            throw new TypeError("the 'callback' argument must be of type 'function'");
        }

        return callback.call(thisArg || this, this) || this;
    };
}(jQuery);

Note: I've defined the plugin within an IIFE so that it still works if $.noConflict() was used.


Printing nth bell number Bell number \$B(n)\$ is defined as the number of ways of splitting \$n\$ into any number of parts, also defined as the sum of previous \$n\$ Stirling numbers of second kind.

Here is a snippet of Python code that Wikipedia provides (slightly modified) to print bell numbers:

def bell_numbers(start, stop):

   t = [[1]]                        ## Initialize the triangle as a two-dimensional array
   c = 1                            ## Bell numbers count
   while c <= stop:
       if c >= start:
           yield t[-1][0]           ## Yield the Bell number of the previous row
       row = [t[-1][-1]]            ## Initialize a new row
       for b in t[-1]:
        row.append(row[-1] + b)  ## Populate the new row
       c += 1                       ## We have found another Bell number
       t.append(row)                ## Append the row to the triangle
   for b in bell_numbers(1, 9):
       print b

But I have to print the \$n\$th bell number, so what I did was I changed the second last line of code as follows:

for b in bell_numbers(n,n)

This does the job, but I was wondering of an even better way to print the \$n\$th bell number.

You can use the Dobinski formula for calculating Bell numbers more efficiently.

Dobinski's formula is basically something like this line, although the actual implementation is a bit more complicated (this function neither returns precise value nor is efficient, see this blogpost for more on this):

import math
ITERATIONS = 1000
def bell_number(N):
    return (1/math.e) * sum([(k**N)/(math.factorial(k)) for k in range(ITERATIONS)])

E.g. you can use the mpmath package to calculate the nth Bell number using this formula:

from mpmath import *
mp.dps = 30
print bell(100)

You can check the implementation here