Randomly Permute Elements in a List I want to randomly permute a finite list in the most effective and efficient way in C#. My attempt is as follows.

/*===================================*
 * Compile it to produce Shuffle.exe *
 * ==================================*/
using System;
using System.Collections.Generic;
using System.IO;

namespace Shuffle
{
    class Program
    {
        static void Main(string[] args)
        {
            int Columns = int.Parse(args[0]);
            int Rows = int.Parse(args[1]);
            int Seed = int.Parse(args[2]);
            string OutputFilename = args[3];

            List<string> OrderedList = new List<string>();
            for (int x = 0; x < Columns; x++)
                for (int y = 0; y < Rows; y++)
                    OrderedList.Add(string.Format("{{{0},{1}}}", x, y));

            Random rnd = new Random(Seed);
            List<string> ShuffledList = new List<string>();

            for (int i = 0; i < Columns * Rows; i++)
            {
                int x = rnd.Next(OrderedList.Count);
                ShuffledList.Add(OrderedList[x]);
                OrderedList.RemoveAt(x);
            }

            using (StreamWriter sw = new StreamWriter(OutputFilename))
            {
                foreach (string s in ShuffledList)
                    sw.WriteLine(s);
            }

        }
    }
}

This program will be used in my production. Could you review whether or not my code is already the most efficient and effective in C#?

Don't waste time on micro-optimizations. Use the simplest possible techniques that are known to perform well. In your specific case, it means replacing your multiple lists with one single array and using the standard fisher-yates-shuffle.
Naming conventions
In good C# code, local variables are camelCase beginning with a lower case letter. I renamed most of your variables to better express intent.

Generating the strings
You'll get a performance increase of about 30%-40% by using an array instead of a list (not much to be gained here).
Don't bother with string concatenation instead of string.Format, there is hardly any measurable difference in speed, but a huge difference in readability and maintainability.
string[] elements = new string[columns * rows];
for (int column = 0; column < columns; column++)
    for (int row = 0; row < rows; row++)
        elements[columns * row + row] = string.Format("{{{0},{1}}}", column, row);

Shuffling them
Whenever you remove from a list, all the remaining elements have to be shifted to fill the gap. Use your array instead and use the fisher-yates-shuffle; don't attempt inventing your own.
for (int i = elements.Length - 1; i > 0; i--)
{
    int swapIndex = random.Next(i + 1);
    string tmp = elements[i];
    elements[i] = elements[swapIndex];
    elements[swapIndex] = tmp;
}

This runs about 160 times as fast as your shuffling implementation.

Don't reinvent the wheel
When saving the results to disk, use File.WriteAllLines which does exactly what you were doing with the StreamWriter - in a single line.
File.WriteAllLines(outputFilename, elements);

Complete code (argument parsing omitted)
Once you've realized that a few milliseconds more or less don't matter, it would be good to split this up into several methods that each do exactly one thing. I'll leave that as an exercise to you.
string[] elements = new string[columns * rows];
for (int column = 0; column < columns; column++)
    for (int row = 0; row < rows; row++)
        elements[column*rows + row] = string.Format("{{{0},{1}}}", column, row);

Random random = new Random(seed);
for (int i = elements.Length - 1; i > 0; i--)
{
    int swapIndex = random.Next(i + 1);
    string tmp = elements[i];
    elements[i] = elements[swapIndex];
    elements[swapIndex] = tmp;
}

File.WriteAllLines(outputFilename, elements);

Here's what I came up with, it needs to be cleaned up and documented some more, but here's the gist.

Since the random accesses into Lists<> turn out to be slow (especially removing items), we can get rid of that.

So, make an array of numbers.

Assign those numbers randomly to the elements in the List if they haven't been used before.

Now you have a Dictionary where the string is the original elements, and the
int is the random position.

Now sort the Dictionary by value (the random location).

Now return it as a List again.

    private static void randomizeList3(List<string> OrderedList,
                                       List<string> ShuffledList, Random rnd)
    {
        int count = OrderedList.Count;
        int twoCount = 2 * count;

        bool[] usedNums = new bool[twoCount];

        Dictionary<String, int> dicto = new Dictionary<string, int>(count);

        int nextPos;

        foreach (string s in OrderedList)
        {
            nextPos = rnd.Next(twoCount);
            while (usedNums[nextPos] == true)
            {
                nextPos = rnd.Next(twoCount);
            }
            usedNums[nextPos] = true;

            dicto.Add(s, nextPos);
        }

        List<KeyValuePair<string, int>> myList = dicto.ToList();

        myList.Sort(
            delegate(KeyValuePair<string, int> firstPair,
            KeyValuePair<string, int> nextPair)
            {
                return firstPair.Value.CompareTo(nextPair.Value);
            }
        );

        foreach (KeyValuePair<string, int> p in myList)
        {
            ShuffledList.Add(p.Key);
        }

    }

I use a usedNums array which is twice as big, so that the chance of randomly choosing an already used element is only 50%.

Anyway, after timing on an instance of 200 cols, 200 rows:

Original Approach takes 12,805 milliseconds
   randomizeList3 takes 214 milliseconds

on a grid 2000 by 2000,

   randomizeList3 takes 4,425 milliseconds.

I'm sure this could be improved.


Function to test if exactly one of three parameters is truthy, without using equality operators Given the following instructions I have created a function that achieves the requirements. I feel my function is a bit too complex, though it does what it's supposed to. The difficult part for myself was avoiding the equality operators. The only way I could think to solve this is with a bit of math and a comparison operator. How can I simplify this function and save some editing time?

  Write a function onlyOne that accepts three arguments of any type.

  onlyOne should return true only if exactly one of the three arguments are
  truthy. Otherwise, it should return false.

  Do not use the equality operators (== and ===) in your solution.

My function:

function onlyOne(input1, input2, input3){
output = false;
let truthy = 0;
let falsey = 0;
if (!!input1) {
    truthy++;
} else falsey++;
if  (!!input2) {
    truthy++;
} else falsey++;
if (!!input3) {
    truthy++;
} else falsey++;
if (falsey > truthy) {
    output = true;
}
if (falsey > 2) {
    output = false;
}
return output;
}

(originally asked on StackOverflow and was directed here.)

You don't need to count the falsy arguments.  The challenge only asks you to count the truthy ones.  Since true is treated as 1 by the + operator, you can use that to perform the count.

As for the prohibition on using equality operators, you can easily work around that using two inequalities.

You forgot to declare output, so that was treated as a global variable.

function onlyOne(a, b, c) {
    let truthyCount = !!a + !!b + !!c;
    // Workaround for the prohibited test truthyCount == 1
    return 0 < truthyCount && truthyCount < 2;
}

console.log("These should be true:");
console.log(onlyOne(true, null, undefined));
console.log(onlyOne(false, '', '0'));
console.log(onlyOne(false, 0, '0'));
console.log(onlyOne(false, [], null));
console.log(onlyOne(1/0 /* Infinity */));

console.log("These should be false:");
console.log(onlyOne(1, 'a', Infinity));
console.log(onlyOne(false, null, undefined));
console.log(onlyOne(false, NaN, null));
console.log(onlyOne(0));

Based on @Blindman67's suggestion, here's another solution, which involves fewer operations.  How it works is less obvious from inspection, though.  Basically, it tests whether the number of falsy parameters is 2.

function onlyOne(a, b, c) {
    return !((!a + !b + !c) - 2);
}

console.log("These should be true:");
console.log(onlyOne(true, null, undefined));
console.log(onlyOne(false, '', '0'));
console.log(onlyOne(false, 0, '0'));
console.log(onlyOne(false, [], null));
console.log(onlyOne(1/0 /* Infinity */));

console.log("These should be false:");
console.log(onlyOne(1, 'a', Infinity));
console.log(onlyOne(false, null, undefined));
console.log(onlyOne(false, NaN, null));
console.log(onlyOne(0));

This is similar to the the Array.filter solution already posted, but can expand to any number of arguments which I thought made it unique enough to add.

function onlyOne(/*a, b, b -- works for any # of args*/) {
    return !(Array.prototype.filter.call(arguments, i => i).length-1);
}

// test
const T = [true, 1, false, null, '', undefined];
T.forEach(t1 => {
 T.forEach(t2 => {
  T.forEach(t3 => {
    console.log([t1, t2, t3]);
    console.log(onlyOne(t1, t2, t3));
    console.log();
  });
 });
});


Building categories block based on JSON I'm not even sure if this is how you're supposed to do it, but this is what I have so far:

$.getJSON("/api/get_categories", function(data){
    var items = [];
   $.each(data["categories"], function(key, value){

    items.push('
<div class="card waves-effect waves-block waves-light"

'+value+'

');
   });

   $("#categories").html(items.join(""));

});

The issue is that the string within my push looks really awkward and long. What's the better way of doing this?

I have a few issues with your code before I get into your actual question:

Your indentation is inconsistent and wrong, you should be using two or four space indentation, not three-space indentation.
Your usage of quotation marks is inconsistent, use either " or ', but stay consistent.
BUG: you're missing a close html tag character in here: <div class="card waves-effect waves-block waves-light"
.
Instead of pushing it into the block as one big lump, consider pushing it through at each category, that way it'll actually look like it's loading, and you can avoid storing a massive html string. You can use jQuery's .append to do this

As for your specific question, I would style it better by splitting it into parts, and joining each part like so:

$.getJSON('/api/get_categories', function(data){
    $.each(data.categories, function(key, value){
        $('#categories').append(
            '
' +
            '

' +
            '
' +
            '' +
            value + '

'
        );
    });
});

I would use a templating engine like underscore to keep all your markup separate.


Highlight the active link in a navigation menu I wonder if my script is all right. Please review my code:

<!DOCTYPE html>

<meta charset="UTF-8">
<title>Menu</title>

Home

News

Contact

About

It should highlight the current page link in the navigation bar.

querySelectorAll

Here's another approach, if you don't mind older browsers. You can use querySelectorAll instead of document.links. That way, you end up with a smaller result set rather than all links, and save you CPU cycles for running the loop.

In this case, we get only those links that have the same href value as the current document url:

var links = document.querySelectorAll('a[href="'+document.URL+'"]');

However, note the browser compatibility and there are quirks across implementations.

Not all links need active

Now if you think about it, not all links with the document.URL need to have active. Say you have active set the font to 2em. If you get all links that point to the same page, they'd be 2em in size, regardless of where they are in the page!

Most likely, you only need it for the primary navigation. Consider adding a class to further refine the result set.

Home

News

Contact

About

// Getting only the links that are in .navigation
var links = document.querySelectorAll('.navigation a[href="'+document.URL+'"]');

// More specific CSS
.navigation a.active {background:red;}

className

Now you have to note that setting className replaces it with the value, like this example. If the links you have happen to have existing classes (and styles that come with them), your script will unintentionally remove their styles due to the replaced className.

What you can do is get the existing className value, split them by spaces (multiple class names are separated by spaces), append your class name, join them back and then change the value, like I did here:

var element = document.getElementById('change');
var classNames = element.className.split(' ');
classNames.push('huge');
element.className = classNames.join(' ');

you can use these for <a> tag

a:link {color:#FF0000;}      /* unvisited link */
a:visited {color:#00FF00;}  /* visited link */
a:hover {color:#FF00FF;}  /* mouse over link */
a:active {color:#0000FF;}  /* selected link */

See this DEMO


Drawing a table for data from file I wrote a function for drawing a table for data that comes from file:

import data_manager
import os

table = data_manager.get_table_from_file('games.csv')

len_of_longest_name = 0
i = 0
while i < len(table):
    if len_of_longest_name < len(table[i][1]):
        len_of_longest_name = len(table[i][1])
    i += 1

i = 0
len_of_longest_studio = 0
while i < len(table):
    if len_of_longest_studio < len(table[i][2]):
        len_of_longest_studio = len(table[i][2])
    i += 1
print("/", 6 * "-", "|", (len_of_longest_name - 2) * "-", "|", (len_of_longest_studio - 2) * "-", "|", "---", "\\")

# constructing string

i = 0
while i < len(table):
    string = '|'
    string += table[i][0] + '|'
    if len(table[i][1]) < len_of_longest_name:
        spaces = (len_of_longest_name - len(table[i][1])) * ' '
        string += table[i][1] + spaces + '|'
    else:
        string += table[i][1] + '|'
    if len(table[i][2]) < len_of_longest_studio:
        spaces = (len_of_longest_studio - len(table[i][2])) * ' '
        string += table[i][2] + spaces + '|'
    else:
        string += table[i][2] + '|'
    string += table[i][3] + '|'
    if len(table[i][4]) < 2:
        string += table[i][4] + ' |'
    else:
        string += table[i][4] + '|'
    if i < len(table) - 1:
        print(string)
        print("|", 6 * "-", "|", (len_of_longest_name - 2) * "-", "|", (len_of_longest_studio - 2) * "-", "|", "---", "|")
    else:
        print(string)
    i += 1
print("\\", 6 * "-", "|", (len_of_longest_name - 2) * "-", "|", (len_of_longest_studio - 2) * "-", "|", "---", "/")

The code works as I want but I think there's a better (shorter) method to do the same. Could you show me the method?

The output looks like this:

Python has many great ways to iterate.  In general if you find yourself looping on an integer, like in the following code, there is a good chance you are doing it wrong.  It is generally better to iterate on the data structure itself.

Sample code:

I will use this loop as an example:

len_of_longest_name = 0
i = 0
while i < len(table):
    if len_of_longest_name < len(table[i][1]):
        len_of_longest_name = len(table[i][1])
    i += 1

Don't iterate on the length of a data structure:

This is probably not what you want:

while i < len(table):

It would likely be better as something like:

for row in table:

Or, if for some reason you really need the index, then:

for i, row in enumerate(table):

Example with looping on length removed:

So removing i, and looping on table, we get:

len_of_longest_name = 0
for row in table:
    if len_of_longest_name < len(row[1]):
        len_of_longest_name = len(row[1])

Use builtin functions.

Python have a max() function.  Using max(), the example becomes:

len_of_longest_name = 0
for row in table:
    len_of_longest_name = max(len_of_longest_name, len(row[1]))

Comprehensions:

Since max() can take a comprehension the example loop can be reduced to:

len_of_longest_name = max(len(row[1]) for row in table)

That is six lines of code reduced to one.

Thanks for your answers. I wrote this:

import data_manager

def table_lines():
    string = ""
    for indices in range(0, len(max_len)):
        value = max_len[indices]
        string += "|" + (value * "-")
    string += "|"
    return string

def data_lines(table):
    data_string = "|"
    for row in table:
        for indices in range(0, len(row)):
            if len(row[indices]) < max_len[indices]:
                data_string += row[indices] + ((max_len[indices] - len(row[indices] )) * " ") + "|"
            else:
                data_string += row[indices] + "|"
        print(table_lines())
        print(data_string)
        data_string = "|"

def get_longest(table):
    for row in table:
        value_len = []
        for element in row:
            value_len.append(len(element))
        # for debug print(row)
        values_len.append(value_len)
    # for debug print(values_len)

    for row in values_len:
        for indices in range(0, len(row)):
            try:
                if row[indices] > max_len[indices]:
                    max_len[indices] = row[indices]
            except IndexError:
                max_len.append(row[indices])

The output looks like this:

Now I will work on fixing a corners of table.
Thanks once more!


Parsing double from string I'm trying to parse Double from String in Java. The string input is not necessarily a number, any non number input should be treated as 0.

Given below is a solution using try-catch, where a Array of string is converted into a Array of Double:

public static void main(String[] args) {
    // Array of strings to be parsed as double.
    // In case of NaN I have to assign it a value 0.
    ArrayList<String> dirtyList = new ArrayList();
    dirtyList.add("Null");
    dirtyList.add("1.24");
    dirtyList.add("0");
    dirtyList.add("0asd");
    ArrayList<Double> doubleList = new ArrayList();
    for ( String item : dirtyList) {
        doubleList.add(parseDouble(item));
    }
    System.out.println(doublelist);
}

// Function to clean data
static Double parseDouble(String x) {
    Double a;
    try {
        a = Double.parseDouble(x);
    } catch (Exception ParseException) {
        a = (double) 0;
    }
    return a;
}

Output:

[0.0, 1.24, 0.0, 0.0]

Is it okay to exploit try-catch in situations like this? Is there a better approach?

In Python, the best practice is to use exceptions for many more situations than is best practice in Java. In Python, you shoot first, ask questions later .... you seek forgiveness, not permission.... you ... {add cheesy analogy here....}

In Java, Exceptions are expensive. When an exception happens, the virtual machine needs to backtrack the call stack, often synchronizing on critical aspects of the runtime, perhaps bringing the entire application in to a locked state, while the exception is built. The Java compiled code is often inlined, and optimized, and the call stack for the actual optimized code has to be translated back to the source code, etc.

If you expect your input data to have invalid double input, then it is not an exceptional case to have invalid data, is it. In Java, you seek permission before having to seek forgiveness... so, you validate the data before doing the hard work.

The documentation for Double.valueOf(String) even goes so far as to show you exactly how to validate an input String before parsing it... using a regular expression, and shows how to use it too.

So, let's compare two ways of parsing the values.... the forgiveness way:

private static final double parseForgiveness(String value) {
    try {
        return Double.parseDouble(value);
    } catch (NumberFormatException nfe) {
        return 0.0;
    }
}

(Note, there's something important in there, I use double and not Double. Using primitives in Java when you can is almost always a better idea for performance than using the full Object version, like Double).

So, the above system begs forgiveness when the data is broken.... what about prevalidating the number...? The code is basically copied from the Javadoc:

private static final double parsePermission(String value) {
    if (DOUBLE_RE.matcher(value).matches()) {
        return Double.parseDouble(value);
    }
    return 0.0;
}

where the DOUBLE_RE is defined as:

private static final String Digits     = "(\\p{Digit}+)";
private static final String HexDigits  = "(\\p{XDigit}+)";
// an exponent is 'e' or 'E' followed by an optionally
// signed decimal integer.
private static final String Exp        = "[eE][+-]?"+Digits;
private static final String fpRegex    =
    ("[\\x00-\\x20]*"+  // Optional leading "whitespace"
     "[+-]?(" + // Optional sign character
     "NaN|" +           // "NaN" string
     "Infinity|" +      // "Infinity" string

     // A decimal floating-point string representing a finite positive
     // number without a leading sign has at most five basic pieces:
     // Digits . Digits ExponentPart FloatTypeSuffix
     //
     // Since this method allows integer-only strings as input
     // in addition to strings of floating-point literals, the
     // two sub-patterns below are simplifications of the grammar
     // productions from section 3.10.2 of
     // The Java Language Specification.

     // Digits ._opt Digits_opt ExponentPart_opt FloatTypeSuffix_opt
     "((("+Digits+"(\\.)?("+Digits+"?)("+Exp+")?)|"+

     // . Digits ExponentPart_opt FloatTypeSuffix_opt
     "(\\.("+Digits+")("+Exp+")?)|"+

     // Hexadecimal strings
     "((" +
      // 0[xX] HexDigits ._opt BinaryExponent FloatTypeSuffix_opt
      "(0[xX]" + HexDigits + "(\\.)?)|" +

      // 0[xX] HexDigits_opt . HexDigits BinaryExponent FloatTypeSuffix_opt
      "(0[xX]" + HexDigits + "?(\\.)" + HexDigits + ")" +

      ")[pP][+-]?" + Digits + "))" +
     "[fFdD]?))" +
     "[\\x00-\\x20]*");// Optional trailing "whitespace"

private static final Pattern DOUBLE_RE = Pattern.compile(fpRegex);

Note that a Pattern is compiled as a single static, and then used to match the String before actually parsing it. That seems like a lot of work, but consider a situation when about half the values are invalid in a String sequence.... how does it affect the performance?

I wrote a UBench tool a little while ago to test performance of these sorts of exercises. The first thing I do is write a function to generate an input array of 50% valid values, and 50% invalid. Here's that code:

private static final char[] alpha = " `1234567890-=~!@#$%^&*()_+qwertyuiop[]\\QWERTYUIOP{}|asdfghjkl;'ASDFGHJKL:\"zxcvbnm,./ZXCVBNM<>?".toCharArray();

private static final String wordRand(Random rand) {
    int len = rand.nextInt(10) + 3;
    char[] chars = new char[len];
    IntStream.range(0, len).forEach(i -> chars[i] = alpha[rand.nextInt(alpha.length)]);
    return new String(chars);
}

private static String[] buildInput(int size) {
    List<String> values = new ArrayList<>();
    Random rand = new Random(size);
    for (int i = 0; i < size; i++) {
        values.add(Double.toString(rand.nextDouble() * i));
        values.add(wordRand(rand));
    }
    return values.toArray(new String[values.size()]);
}

That buildInput method will return an array of Strings where half the values are guaranteed valid float Strings, and the other half are almost certainly not valid (thought it's possible they may be). Then, I run it through a comparative benchmark using the following code:

private static final double[] parseAll(String[] values, ToDoubleFunction<String> parser) {
    return Stream.of(values).mapToDouble(parser).toArray();
}

public static void main(String[] args) {
    UUtils.setStandaloneLogging(Level.INFO);
    String[] values = buildInput(1000);
    UBench bench = new UBench("Parsing");
    bench.addTask("Permission", () -> parseAll(values, DoubleParser::parsePermission));
    bench.addTask("Forgiveness", () -> parseAll(values, DoubleParser::parseForgiveness));
    bench.press(1000).report();
}

That's complicated, I guess, but really it sets up two tests, one which parses 2000 input values using the forgiveness method, the other test uses the permission method. It runs each test 1000 times, and then takes some measurements of each run, and reports on it. The report, on my computer, looks like:

INFO   2015-08-07 12:13:03.058 net.tuis.ubench.UBench(<init>): Creating UBench for suite Parsing
INFO   2015-08-07 12:13:06.860 net.tuis.ubench.UBench(press): UBench suite Parsing: completed benchmarking all tasks using mode INTERLEAVED in 3785.000ms
Task Parsing -> Permission: (Unit: MILLISECONDS)
  Count    :     1000      Average  :   1.1980
  Fastest  :   1.0339      Slowest  :  26.8094
  95Pctile :   1.3693      99Pctile :   2.9096
  TimeBlock : 1.702 1.265 1.143 1.176 1.073 1.086 1.168 1.194 1.076 1.096
  Histogram :   977    17     5     0     1

Task Parsing -> Forgiveness: (Unit: MILLISECONDS)
  Count    :     1000      Average  :   2.5568
  Fastest  :   2.3117      Slowest  :  12.5496
  95Pctile :   2.8740      99Pctile :   5.5604
  TimeBlock : 3.054 2.539 2.522 2.645 2.411 2.471 2.470 2.591 2.419 2.446
  Histogram :   978    20     2

What that means, is that the average time to parse 2000 values using the regex is 1.20 milliseconds, and using the exception system is 2.56 milliseconds - more than twice as slow.

Additionally the fastest parse is 1.03 milliseconds, vs. 2.31 - again more than twice as slow.

Bottom line is that in Java (unlike Python), you should not use exceptions to handle data validation when you expect data to regularly be invalid....

This approach is totally fine. :) Just a pointer for parseDouble(): you should return the results immediately, instead of a single return at the end. The benefits of this are more compact code and short-circuiting of the logic.

private static Double parseDouble(String x) {
    try {
        // use valueOf to return a wrapper object instead
        return Double.valueOf(x);
    } catch (NumberFormatException e) {
        return Double.valueOf(0);
    }
}

You should be catching the stricter NumberFormatException rather than the general Exception, and use camelCase for the variable name (furthermore, ParseException sounds like the text-formatting-related exception class name).

If you are on Java 8, then you can alternatively treat this as a mapping of Stream<String> to DoubleStream using the same method as the ToDoubleFunction lambda implementation, e.g.:

double[] result = Stream.of("Null", "1.24", "0", "0asd")
                        .mapToDouble(YourClass::parseDouble).toArray();


Beginner's Sudoku solver in C I'm beginner programmer and I've challenged myself to write sudoku solver in C. I have finished it in few days and now I want to make it faster.

With this sample input execution time is 1.781 seconds (which includes loading of program, I'm using code:blocks in which time is automatically measured)

0 0 0 0 5 0 0 0 1
1 0 0 8 0 0 0 0 0
0 0 8 0 4 6 0 0 2
5 7 0 0 0 0 0 0 0
0 6 0 3 0 0 2 0 0
0 9 0 4 2 0 0 7 0
0 0 0 0 0 0 0 0 4
0 0 0 0 9 8 5 0 0
0 3 0 0 0 0 7 2 0

Here's the code:

 #include<stdio.h>

int sudoku[9][9]=
{
    0,0,0,0,0,4,0,0,0,
    0,6,8,0,0,0,5,0,0,
    0,2,0,0,0,0,0,7,6,
    6,0,0,0,0,0,8,9,0,
    0,0,5,2,6,0,0,0,0,
    0,0,0,9,0,0,1,0,0,
    0,0,0,0,0,7,0,5,0,
    0,4,0,0,0,0,0,0,1,
    0,0,0,0,5,1,4,0,0
};
int a[9][9]=
{
    0,0,0,0,0,4,0,0,0,
    0,6,8,0,0,0,5,0,0,
    0,2,0,0,0,0,0,7,6,
    6,0,0,0,0,0,8,9,0,
    0,0,5,2,6,0,0,0,0,
    0,0,0,9,0,0,1,0,0,
    0,0,0,0,0,7,0,5,0,
    0,4,0,0,0,0,0,0,1,
    0,0,0,0,5,1,4,0,0
};
//PRINT SUDOKU
void printSudoku()
{
    int i,j;
    for(i = 0; i < 9; i++) { if(i==3 || i==6) printf("\n");
        for(j = 0; j < 9; j++)    {
            printf("%d",sudoku[i][j]);
            printf(" ");
            if(j>7) printf("\n");
            if(j==2 || j==5) printf("  ");

        }
    }
    printf("\n");
}
//CHECK ROW
int checkRow(int num, int i)    {
    int j,x=1;
    for(j = 0; j < 9; j++)
        if(sudoku[i][j] == num)    return 0;

    return x;
}
//CHECK COLUMN
int checkCol(int num, int j) {
    int i,x=1;
    for(i = 0; i < 9; i++)
        if(sudoku[i][j]==num)    return 0;

    return x;
}

//CHECK SQUARE
int checkSquare(int num, int xj, int yi)
{
    int x=1;
    int i,j;
    for(i = yi; i < yi+3; i++)
        for(j = xj; j < xj+3; j++)
            if(sudoku[i][j]==num)    return 0;

    return x;
}
//MAIN PROGRAM
main()
{
    int i,j,num,row,col;
    int pi,pj,xj,yi,square;

    printSudoku();

    for(i = 0; i < 9; i++)
        for(j = 0; j < 9; j++) {
            if(sudoku[i][j]==0)    {
                num=1;

                do    {
                    if(num>9)    {
                        pi=i;
                        if(j == 0)  {
                            if(i!=0)    {
                                pj=8;
                                pi=i-1;
                            }
                        }
                        else    {
                            pj=j-1;
                        }
                        for(i = pi; i >= 0; i--)
                            for(j = pj; j >= 0; j--)    {
                                pj=8;

                                if(a[i][j]==0 && sudoku[i][j]==9)    {
                                    sudoku[i][j]=0;
                                }
                                else if(a[i][j]==0 && sudoku[i][j]<9)    {

                                    num=sudoku[i][j]+1;
                                    sudoku[i][j]=0;
                                    goto check;
                                }
                            }
                    }
                check:row=checkRow(num,i);
                    col=checkCol(num,j);
                    if(j<3)    xj=0;
                    if(j>2 && j<6) xj=3;
                    if(j>5) xj=6;
                    if(i<3)    yi=0;
                    if(i>2 && i<6) yi=3;
                    if(i>5) yi=6;
                    square=checkSquare(num,xj,yi);
                    if(row==1 && col==1 && square==1)
                        sudoku[i][j]=num;
                    else
                        num++;

                }
                while(row==0 || col==0 || square==0);
            }
        }

    printf("\n");
    printSudoku();

}

As I've said I'm a beginner programmer so sorry that my code is long and messy.

Does anyone know how can I make this faster?
Would it be faster if I used recursion?
And also how can I measure real execution time?

Advice 1

Instead of having checkRow, checkCol and checkSquare, I suggest you allocate a simple char array for each row, column and minisquare:

char visited[10];

You initialize each visited[0], ..., visited[9] to zero. Next, when you put a number in an empty cell, you mark it in three such "sets": current column, current row and current minisquare. This arrangement will bring the complexity of checking each row, column and minisquare from \$\Theta(d)\$ to \$\Theta(1)\$, where \$d\$ is the width/height of the input sudoku board.

Advice 2

What comes to the actual algorithm, I suggest you use recursion/backtracking. That will clean your code quite a bit, and will allow you better adapting it to, say, \$4 \times 4\$ or \$16 \times 16\$-sudokus.

The idea is as follows. You march through the board rows, each row from left to write. You leave the cells that have a predefined value as is, but you put 1 to the first empty cell, and recur to the next cell. If 1 does not belong, you try 2, 3 and so on. Once you have found a valid value, move to the next empty cell. At some point it might happen than none of the numbers fit. In such a case you go one step backwards (backtracking) and try increment the previous value. If you can translate from Java, see this, starting from line 163.

The boolean [9][9][9] is initially set all on true.

You have 2 in one cell so you put false into the line and column and block of [][][2]

When you check all the sudoku spots you fill the other table counting the true.

To traverse the sudoku array I would prefer 2 variables: one for the column and one for the block so the code will be more readable.


A NOT macro for negative conditions Motivation

I typically try to avoid using ! in if-statements, because (to me) it seems easy to overlook. So, I'll usually just spell it out instead, ex: if (done == false) instead of if (!done). While that makes the intention more obvious, it doesn't read very fluidly.

Macro code

For those cases I wrote the following macro:

#define NOT(x) ((x) == false)

The outer parentheses make it possible to use NOT(cond) without additional parentheses in if and make it less error prone:

if NOT(condition) {
    ...
}

Complete example of macro usage

#include <iostream>

#define NOT(x) ((x) == false)

bool successful() {return false;}

int main(int argc, char **argv)
{
    if NOT(successful()) {
        std::cout << "Warning! Self-destruction activated...\n";
    }

    const bool this_is_true = true;

    if NOT(this_is_true) {
        std::cout << "True is false.\n";
    }

    return EXIT_SUCCESS;
}

Concerns

While this works, I have some concerns:

From a best-practices or clean code perspective, is this frowned upon?
How are macros generally viewed, or reported on, in code analysis tools? We are considering employing a system called CAST-AIP for code analysis and, while I am strongly in favor of doing so, I am wondering how many volumes of practice violations my reports are going to be.

Something like if (foo == false) will always be frowned upon. If you find the negation hard to spot you might consider using not instead (see this for more info).
E.g. if (not foo)

From what I've seen macros are generally discouraged as they tend to obfuscate code and are not typesafe.

why not just

#define NOT !

It would be simpler to use.


Sorting large 1GB file with 100 millions numbers using merge sort My task is to sort 1GB file with 100 million numbers using merge sort without recursion. My idea is to split file into 4 pieces, then to 2 and at the end to one file.

#include<iostream>
#include<fstream>
#include <cstdlib>
#include <vector>
#include <algorithm>
using namespace std;
std::vector <long> vec;

int i=0,numberOfpieces=0;
string line,filename;

//sorting and merge 2 files
void sort_merge(string result, string file1, string file2){

fstream wynik((result+".txt"), std::ios::out);

    string line, line2;
    long s1,s2;
    fstream sorted_1,sorted_2;
    sorted_1.open(file1+".txt");
    sorted_2.open(file2+".txt");

    getline(sorted_1,line);
    s1=atol(line.c_str());
    getline(sorted_2,line2);
    s2=atol(line2.c_str());

    do
    {
        if(s1==s2)
        {
            wynik<<s1<<endl;
            getline(sorted_1,line);
            s1=atol(line.c_str());
            getline(sorted_2,line2);
            s2=atol(line2.c_str());
        }
        if(s1<s2)
        {
            wynik<<s1<<endl;
            getline(sorted_1,line);
            s1=atol(line.c_str());
        }
        if(s1>s2)
        {
            wynik<<s2<<endl;
            getline(sorted_2,line2);
            s2=atol(line2.c_str());
        }
        if(sorted_1.eof())
        {
            do
            {
                wynik<<s2<<endl;
                getline(sorted_2,line2);
                s2=atol(line2.c_str());

            }
            while(!sorted_2.eof());
            break;
        }

        if(sorted_2.eof())
        {
            do
            {
                wynik<<s1<<endl;
                getline(sorted_1,line);
                s1=atol(line.c_str());
            }
            while(!sorted_1.eof());
        }

    }
    while (!sorted_1.eof() ||!sorted_2.eof());}

int main()
{
    fstream file;
    file.open("file.txt");
    if (file.good() == true )
    {
        do
        {
            for(i=1;i<=25000000;i++){ //splittining file into 4 pieces
            getline(file,line);
            vec.push_back(atol(line.c_str()));
            if(file.eof()){ //in case of last piece when quantity of numbers is less than 50
                break;
            }
            }
            sort(vec.begin(), vec.end() );//sorting vector
            std::string nm = std::to_string(numberOfpieces); //creating indyvidual name for piece
            filename="sorted_piece_"+nm+".txt";
            fstream sorted_piece(filename, std::ios::out);

            for ( auto &i : vec ) { //saving into file
                sorted_piece<< i <<endl;
            }

            numberOfpieces++;
            vec.clear();
        }
        while ( !file.eof() );
    }
string outFile;
string sorted1;
string sorted2;
int j=0,counti=0;

for(j=0;j<numberOfpieces;j+=2){//merging into 2 pieces
    std::string j1 = std::to_string(j);
    std::string j2 = std::to_string(j+1);
    std::string c = std::to_string(counti);
    outFile=c+"_sorted_piece";
    sorted1="sorted_piece_"+j1;
    sorted2="sorted_piece_"+j2;
    sort_merge(outFile,sorted1,sorted2);
    counti++;
}
outFile=numbersSorted;
sorted1=1_sorted_piece;
sorted2=2_sorted_piece;
sort_merge(outFile,sorted1,sorted2);//merging into 1 piece

return 0;
}

Unfortunately it is slow and I don't know exactly what is the best way to improve it, because vector sorting has \$O(nlog(n))\$ complexity. Also, my algorithm with merging 2 files have \$O(n)\$ complexity. Maybe splitting into files takes too much time, but I can't hold vectors that are too long in RAM.

Don't do this

using namespace std;

This is a bad habit that will get you into a lot of trouble. Please break the habit before it becomes ingrained.

see: Why is “using namespace std” considered bad practice?

Note: The reason the "standard" library uses the namespace std:: is so that it is not a large burden to use as a prefix.

Prefer '\n' over std::endl

Others have mentioned it.
The problem is that std::endl performs a flush of the stream (in addition to adding '\n') as a result this makes using the stream very inefficient as you need to wait to write every value to disk before continuing.

Let the system flush its own internal buffer at the optimum time. It will do this automatically without any of your help.

Reading Value

I see that you are not using the standard stream to integer conversion.

long value;
stream >> value;

but rather using the line reader than using atol() to convert the string into a number.

I assume you have done the appropriate speed tests and found this to be faster on the scale that you are using and you don't need all the facilities that the stream operator>> provides.

If this is true then I would wrap up that call so that you can use atol() and operator>> to make your code more readable.

struct Number
{
    long value;
    static std::string  reusableBuffer;
    operator long() const {return value;}
    friend std::istream& operator>>(std::istream& str, Number& data) {
        std::getline(str, data.reusableBuffer);
        data.value = std::atol(data.reusableBuffer);
        return str;
    }
};

Now you can read numbers using operator>> but still have the speed you have verified with atol().

Never test for eof in a loop

while (!sorted_1.eof() ||!sorted_2.eof());}

The problem becomes that if there is an error on the stream then you end up going into an infinite loop (if there is an error reading from the stream it goes into a bad state until you  reset it. While in a bad state it will refuse to read any more values and thus never reach the end of the file.

Rather than testing for eof() you should check to make sure the stream is good() (if it reaches eof() or error() then it will no longer be good() so your loop will still exit correctly).

The easy way to test for good() is to simply use the stream in a boolean context (an expression that is expected to be bool) and the compiler will convert the stream to bool which results in a call to good().

while(sorted_1 || sorted_2);

Merge I think can be simplified.

I think we can simplify that merge algorithm a lot.

Number number;
if (sorted1 >> number) {
    sorted1Value = number;
}
if (sorted2 >> number) {
    sorted2Value = number;
}
while (sorted1 && sorted2) {
    // Don't need to explicitly test if they are equal.
    // If they are equal always use the sorted1 file.
    // that way you get a stable sort (an important property for
    // sorted values in some situations).
    //
    // It also makes the code simpler.
    if (sorted1Value <= sorted2Value) {
        result << sorted1Value << "\n";
        sorted1 >> number;
        sorted1Value = number;
    }
    else {
        result << sorted2Value << "\n";
        sorted2 >> number;
        sorted2Value = number;
    }
}
// At this point one of the streams is empty.

Copy one file to another.

When you have merged all the values out of one file. Then the remaining numbers from the other file are simply copied to the destination. But there is no need to spend extra time converting those numbers from text to an integer then back into text. Simply copy the raw data from one file to the other.

// At this point one of the streams is empty.
if (stream1) {
    result << sorted1Value;
    result << stream1.rdbuf()
}
if (stream2) {
    result << sorted2Value;
    result << stream2.rdbuf()
}

Intermediate format

So the intermediate files don't need to be in human readable form. You spend a lot of time converting integers to and from text. Rather than doing this you could write the binary representation of the number to an intermediate file and then read it directly back into the number when merging.

// writing
result.write(reinterpret_cast<char*>(&sorted1Value), sizeof(sorted1Value));

// reading
stream1.read(reinterpret_cast<char*>(&sorted1Value), sizeof(sorted1Value));

Merging more than one file.

Currently you perform the merge operation 3 times. Once to merge parts 1 and 2 into a temp file. Then you merge 3 and 4 into a second temp file and finally you merge the temp files into the output.

Merging from different streams is not that memory intensive. So you should read from all the temporary files at the same time (within reason as long as you have less than a thousand part file).

So you should open all 4 temp files and read the values from them. Each iteration find the largest value from each of the 4 files write that to the output and read the next value.

Global State.

Global state

std::vector <long> vec;

int i=0,numberOfpieces=0;
string line,filename;

Is always considered a bad thing. Never use it. You can still get the same efficiencies by using local values in the appropriate places.

One thing you way want to do to increase the efficiency of std::vector is to reserve its size. That way it never needs to do any reallocation (which is expensive).

vec.reserve(25000000);

Note your use of <= in the for loop is very unusual.

for(i=1;i<=25000000;i++)

Normally this is written as:

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

This is because indexes into arrays (vectors) always start at zero.

Z-Tree is a new data structure bases on bits splitting and branching. With Z-Tree, you can sort a file of 1GB in memory with the time complexity of O(n). Much faster than merge sort.


Converting between std::wstring and std::string While researching ways to convert back and forth between std::wstring and std::string, I found this conversation on the MSDN forums.

There were two functions that, to me, looked good. Specifically, these:

std::wstring s2ws(const std::string& s)
{
    int len;
    int slength = (int)s.length() + 1;
    len = MultiByteToWideChar(CP_ACP, 0, s.c_str(), slength, 0, 0);
    wchar_t* buf = new wchar_t[len];
    MultiByteToWideChar(CP_ACP, 0, s.c_str(), slength, buf, len);
    std::wstring r(buf);
    delete[] buf;
    return r;
}

std::string ws2s(const std::wstring& s)
{
    int len;
    int slength = (int)s.length() + 1;
    len = WideCharToMultiByte(CP_ACP, 0, s.c_str(), slength, 0, 0, 0, 0);
    char* buf = new char[len];
    WideCharToMultiByte(CP_ACP, 0, s.c_str(), slength, buf, len, 0, 0);
    std::string r(buf);
    delete[] buf;
    return r;
}

However, the double allocation and the need to delete the buffer concern me (performance and exception safety) so I modified them to be like this:

std::wstring s2ws(const std::string& s)
{
    int len;
    int slength = (int)s.length() + 1;
    len = MultiByteToWideChar(CP_ACP, 0, s.c_str(), slength, 0, 0);
    std::wstring r(len, L'\0');
    MultiByteToWideChar(CP_ACP, 0, s.c_str(), slength, &r[0], len);
    return r;
}

std::string ws2s(const std::wstring& s)
{
    int len;
    int slength = (int)s.length() + 1;
    len = WideCharToMultiByte(CP_ACP, 0, s.c_str(), slength, 0, 0, 0, 0);
    std::string r(len, '\0');
    WideCharToMultiByte(CP_ACP, 0, s.c_str(), slength, &r[0], len, 0, 0);
    return r;
}

Unit testing indicates that this works in a nice, controlled environment but will this be OK in the vicious and unpredictable world that is my client's computer?

I would, and have, redesign your set of functions to resemble casts:

std::wstring x;
std::string y = string_cast<std::string>(x);

This can have a lot of benefits later when you start having to deal with some 3rd party library's idea of what strings should look like.

No, this is dangerous! The characters in a std::string may not be stored in a contiguous memory block and you must not use the pointer &r[0] to write to any characters other than that character! This is why the c_str() function returns a const pointer.

It might work with MSVC, but it will probably break if you switch to a different compiler or STL library.


Use counter for ajax loader I was using a Boolean var for indication of loading so my code looked like this:
(this example is in angularjs but i think it can be relevant for any ajax call handlers)

//HTML:

...LOADING...

//JS:
//Init
$scope.isLoading = false;

//Executing http request:
$scope.isLoading = true;
mainService.post(alteredItems).then(function (response) {
init();
}).catch(function (err) {
  $scope.callBack(err);
}).finally(function () {
  $scope.isLoading = false;
});

function init() {
  $scope.isLoading = true; //another loading
  mainService.get().then(function (response) {
  if (response) {

  }
  $scope.isLoading = false;
  });
}

The problem I had was that if 1 ajax method will complete before the other one so the "isLoading" will be false, and the loader will not be displayed although the request has not yet ended.

I converted the $scope.isLoading to int. So now code looks like this:

//Init
$scope.isLoading = 0;

//Start any ajax call
$scope.isLoading++;

//Finish
$scope.isLoading--;

Which works great, also no need to change the html because of the casting.
I can't find a drawback in this approach and it seems better than the Boolean option.

What do you think?

You SHOULD consider migrating to Angular (aka Angular2, aka Angular5) from ANgularJS (version 1), as it's not developing and soon won't be supported anymore.
I guess nested promises would be much useful here (more details below).

Rewrite init() as follow

function init() {
  return mainService.get().then(function (response) {
    if (response) {

    }
    $scope.isLoading = false;
  });
}

And in main block instead of

mainService.post(alteredItems).then(function (response) {
  init();
})

Write:

mainService.post(alteredItems).then(function (response) {
  return init();
})

In this case your finally would be called AFTER bot async calls completed.

P.S. Consider using library cg-buisy https://github.com/cgross/angular-busy
It automates displaying of loading indicator and accepts promises. Used it a lot in production.

Store a couple : function name + boolean so you can even provide information on which request is actually processing on a multi request process.

The benefit of this will be to Identify which one of the request is upon a time slow or not returning result.

Ex:

{[ req1:0,
req2:1
req3:1
]}

You'll know that req1 hasn't returned any result yet.
With that simple int counter you loose that information, and won't be able to debug easily.


Highlighting partitions in distinct color Given:

G is an arbitrary graph
partitions is a list of its edges divided into (in this case) 3 sets.

Color generation:

Its purpose is to generate one distinct color for each partition.
The function turns a number between 0 and 1 into an RGB color.
For the given example, with 3 partitions, it would be called with 1/3, 2/3, and 1, and return a list of codes for green, blue, and red.
For other numbers it would be used to spread the different colors as widely as possible.
Visually, think of a circle with R, G, and B equally spaced around it.
The number is the fraction of the way around the circle from R to pick the color, so if the number is .4, the color would be between G and B, closer to the G.

Color assignment:

For each set in the partition, a different color is generated and assigned to a list of colors.
In this case edge_color_list would be 9 greens, 4 blues, and 2 reds.
The actual list is:
[(0, 1.0, 0.0), (0.0, 0, 1.0), (0.0, 0, 1.0), (0, 1.0, 0.0), (0, 1.0, 0.0), (0, 1.0, 0.0), (0, 1.0, 0.0), (0, 1.0, 0.0), (0, 1.0, 0.0), (0, 1.0, 0.0), (0.0, 0, 1.0), (0, 1.0, 0.0), (1.0, 0, 0.0), (1.0, 0, 0.0), (0.0, 0, 1.0)]

Problem:
The top and bottom sections can't really be changed, but the def_color() function and edge_color_list sections each look like they were written in C, and I think they could be done more elegantly.
It's obvious how I was thinking while writing it (i.e. how I would write it in C), but I'd like to know how python coders think.
I'm having trouble writing python without a heavy C accent (I wrote C for decades).
I understand the python language fairly well (or know how to recognize what I don't know and how to look it up).
And I've reached the stage where what I've written feels wrong.
But I still don't seem to think in native python:
#!/usr/bin/env python3
import matplotlib.pyplot as plot
import networkx as nx

G = nx.petersen_graph()
partitions = [{0, 3, 4, 5, 6, 7, 8, 9, 11}, {1, 2, 10, 14}, {12, 13}]

def get_color(fraction):
    if fraction < 1/3:
        color = (1-3*fraction, 3*fraction, 0)
    elif fraction < 2/3:
        fraction -= 1/3
        color = (0, 1-3*fraction, 3*fraction)
    else:
        fraction -= 2/3
        color = (3*fraction, 0, 1-3*fraction)
    return color

edge_color_list = [None] * len(G.edges())
for i in range(len(partitions)):
    items = list(partitions[i])   # convert from set
    for j in range(len(items)):
        edge_color_list[items[j]] = get_color((i+1)/len(partitions))

nx.draw(G, with_labels=True, pos=nx.circular_layout(G), width=2,
        node_color='pink', edge_color=edge_color_list)
plot.show()

Any suggestions about how I could have thought differently while writing the above would be appreciated.

I would change this
for i in range(len(partitions)):
    items = list(partitions[i])   # convert from set
    for j in range(len(items)):
        edge_color_list[items[j]] = get_color((i+1)/len(partitions)

to:
for idx, partition in enumerate(partitions): # I personally do it your way
    items = list(partition)   # convert from set
    for item in items:
        edge_color_list[items] = get_color((idx+1)/len(partitions)
        # idx is the `i` from before

I referenced this.
There might be a linting plugin to help align your code towards being pythonic


Porting a web-based POS system to Android I'm currently porting a web-based POS system to Android. The Android version uses NavigationDrawer to navigate between menus.

What I need a code review on is my current approach in loading fragments upon selected an item from navigation drawer. I'm using reflection to achieve my case.

MainActivity.java

@Override
    public void onNavigationDrawerItemSelected(String menuItemText) {
        FragmentManager fragmentManager = getSupportFragmentManager();
        BaseFragment fragmentToOpen = null;
        Method newInstanceMethod = null;
        try {
            newInstanceMethod = AvailableMenuList.MENU_ITEM_TABLE.get(menuItemText).getMethod("newInstance", null);
            fragmentToOpen = (BaseFragment) newInstanceMethod.invoke(fragmentToOpen, null);
        } catch (Exception e) {
            e.printStackTrace();
        }

        fragmentManager.beginTransaction()
                .replace(R.id.container, fragmentToOpen, menuItemText)
                .addToBackStack(menuItemText)
                .commit();
    }

HashMap implementation of available POS functions in Android version:

    public class AvailableMenuList {
        private AvailableMenuList() {} //Disable object instantiation

            public static final HashMap<String, Class<BaseFragment>> MENU_ITEM_TABLE = new HashMap() {{
                put("Announcement", AnnouncementListFragment.class);
                put("Board", BoardListFragment.class);
                put("SalesRegistration", SalesRegFragment.class);
                put("SalesRecords", SalesRtvFragment.class);
                put("OrderRegistration", OrderRegFragment.class);
                put("OrderRecords", OrderRtvFragment.class);
            }};
        }
    }

Is there a better approach of instantiating a new Fragment (by calling newInstance) instead of using reflection? Currently, there's only 6 functionalities but there's going to be at least 30 more in the future.

You can use fragmentManager.findFragmentByTag() instead.

If the fragment is not found, initalize a new one.

fragment = new YourFragment();


Helper function to make `setTimeout` synchronous I'm making a helper function to make setTimeout synchronous. Currently, it is asynchronous so other code runs before the timeout is done.

function syncSetTimeout(func, ms, callback) {
    (function sync(done) {
            if (!done) {
            setTimeout(function() {
                eval("(" + func + ")();");
                sync(true);
            }, ms);
            return;
        }
        eval("(" + callback + ")();");
    })();
}

func is a function that should be run after the specified timeout.
ms specifies how many milliseconds to wait before running func.
callback is the function that runs after the func is done running.

Is there a way to get rid of those eval statements? I've read that eval is evil. I'm also wondering if I can do this without a nested function. If there are any other ways to improve this code then please say so.

The Eval

Eval is indeed, the monster. There are plenty of reasons why you should not use it, although there are some situations where you are forced to use it. For sure, this is not the requirement in yours situation. If func is a function, you can call it using .apply.

function syncSetTimeout(func, ms, callback) {
    (function sync(done) {
        if (!done) {
            setTimeout(function() {
                func.apply(func);
                sync(true);
            }, ms);
            return;
        }
        callback.apply(callback);
    })();
}

Reference: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/apply

Or event without apply:

function syncSetTimeout(func, ms, callback) {
    (function sync(done) {
        if (!done) {
            setTimeout(function() {
                func();
                sync(true);
            }, ms);
            return;
        }
        callback();
    })();
}

The synchronous

You need to know, that trying to make setTimeout() sync means that you have some very bad design idea behind. You should not do it and you will not be able to make it really sync without killing performance of the browser and UX of your app.

I believe that you would like to create sync delay in your application. You are not the first who wants to do it (most of beginners or people with background from different language). You can take a look at answers to this question from stackoverflow: https://stackoverflow.com/questions/6921895/synchronous-delay-in-code-execution

From the answer to linked question (by OverZealous):

  JavaScript is a single-threaded language. You cannot combine
  setTimeout and synchronous processing. What will happen is, the timer
  will lapse, but then the JS engine will wait to process the results
  until the current script completes.

One reason people are trying to take advantage of sync delays is returning result from async code. This is not good strategy in JS. If you would like to return async results just use callbacks or more likely - Promises or Observables.

There's a simpler solution if you are using nodejs

await new Promise(done => setTimeout(() => done(), 180000));


C# Hash Code for triangle class I have a custom class to handle some triangle calculations that I am doing. For the most part I am happy with it, however, there is one aspect that I don't really trust. That is my implementation of HashCode.

Some quick information about my triangle- it has 3 Vector2 structs that represent points of two floats. This is an inbuilt class with the Unity engine but the simple version of it is that it is a struct with two floats and a couple math functions.

public class Triangle2D {
    public Vector2 point1;
    public Vector2 point2;
    public Vector2 point3;

    //... Some other methods that don't matter

    public override int GetHashCode() {
        var hashCode = -305700999;
        hashCode = hashCode * -1521134295 + (EqualityComparer<Vector2>.Default.GetHashCode (point1)
            + EqualityComparer<Vector2>.Default.GetHashCode (point2)
            + EqualityComparer<Vector2>.Default.GetHashCode (point3));
        return hashCode;
    }
}

The specific thing about this class that is special is that, it shouldn't matter what order points 1, 2 and 3 are in. If the points are shared, then the Triangle2D objects are the same. My equals function reflects that, but I worry that my hashCode function is not specific enough. Due to the additive nature of things, I suspect there could be a lot of collisions.

Is there some way that I could make the hashCode function have less collisions? Is it good enough as it is?

I use this object in a lot of hashSets to prevent duplicate triangles from getting listed.

Hash code

  it shouldn't matter what order points 1, 2 and 3 are in

There is a way to ensure that: Sorting. Sort your points locally within your hashcode method, first by their x value, then by their y value.

The algorithm for the hashcode could be this one, as also commented above by radarbob. I will leave it for you to adapt it to your own class.

General

Your class has public fields. It is better to make these private and grant public access to them (if needed) via properties. In this case, for something like a triangle, making it a struct instead of a class might be also suitable.

The following piece of code does not make much sense:

var hashCode = -305700999;
hashCode = hashCode * -1521134295 + [...]

I already added the link to a good implementation of the hash code method above, so I won't go into detail about the algorithm. But what you are doing in the above two lines before the addition is just calculating what could simply be a constant. The compiler will probably even precompute the -305700999 * -1521134295 part. Multiplying the hash code with a prime number (like 23 or 31) should happen before every addition of another value.

Overall this looks good, but I have one question, why don't you write this as

public override int GetHashCode() {
    var hashCode = -305700999 * -1521134295;
    hashCode += (EqualityComparer<Vector2>.Default.GetHashCode (point1)
        + EqualityComparer<Vector2>.Default.GetHashCode (point2)
        + EqualityComparer<Vector2>.Default.GetHashCode (point3));
    return hashCode;
}

This should be the same, and it seems cleaner. Also, what are those random constants?


Finding the minimum and maximum sums For this HackerRank warmup challenge, I was wondering if there was a way I could improve the selection of my indices for the minSum:

Here is the problem:

  Print two space-separated long integers denoting the respective minimum and maximum values that can be calculated by summing exactly four of the five integers. (The output can be greater than 32 bit integer.)

  Sample Input

1 2 3 4 5

  Sample Output

10 14

Here is the solution:

function miniMaxSum(inArr) {
    // Complete this function

    let maxSum = 0,
        minSum = 0;
   inArr.sort();

    for (var i = arr.length - 1, o = 0; i >= arr.length - 4, o < 4; i--, o++){
       maxSum += arr[i];
       minSum += arr[o];
    }
    console.log(minSum, maxSum)
}

Is there a way I could have avoided the o counter in my for loop and instead selected the array in a different way? Like minSum += arr[some math computation].

Also, any performance optimizations are welcome too.

Instead of calculating two sums, you could just calculate one: the sum of all elements. Then, the min sum is the sum minus the maximum element, and max sum is the sum minus the minimum element.
Note that sorting just to find the minimum and maximum elements is inefficient, because sorting is generally an \$O(n \log n)\$ operation,
and finding the minimum and maximum (and the sum) is possible in a single pass, which would be \$O(n)\$.

Hard-coding the number 4 makes the solution very rigid, applicable to only this specific problem. In your implementation you could use the length of the array instead in a way to make that number 4 unnecessary, and easily make your implementation applicable to a more general set of problems. The number 4 is an artificial restriction; it's wise to get rid of it.

I wouldn't bother sorting the array. Instead I would calculate the sum, and find minimum an maximum values in the list. The answers can then be calculated as sum - maxval and sum - minval.


Finding the largest averaged array Sometimes the best way to teach is show what you feel is a shiny approach instead of letting a student muddle through coding and potentially loose a future brilliant coder.

This is what I would show for 'Determine the largest averaged array`. Please review any and all aspects.

'use strict';
//Return the average of 1 array
function arrayAverage(array) {
  var length = array.length,
      sum = 0,
      i;
  for (i = 0; i < length; i++) {
    sum += array[i];
  }
  return sum / length;
}

//Compare 2 arrays, return the array with the largest average
//If the averages are equal, return array2
function largestAveragedArray(array1, array2) {

  return arrayAverage(array1) > arrayAverage(array2) ? array1 : array2;
}

//Figure out which array is largest, return `first` or `second` accordingly
function compare(array1, array2) {
  var largest = largestAveragedArray(array1, array2);
  return largest == array1 ? 'first' : 'second';
}

var smaller = [100, 80],
    larger = [100, 100];

document.write(compare(larger, smaller) + '<br>' + compare(smaller, larger));

First of all, a minor complaint, but be careful when using 'use strict'; at a file level—when concatenating JavaScript files for minification, this can have adverse affects. Whenever possible, use it at the top of a function instead.

Anyway, on to the actual code. Your arrayAverage function is fine, but it could be improved and made more explicit by using a more functional style. The .reduce function is perfect for this case, turning the function into a simple one-liner:

function arrayAverage(array) {
  return array.reduce(function (a, b) { return a + b; }) / array.length;
}

Your compare function is, frankly, rather odd. I see three main problems with it:

It does not handle arrays that have equal averages very well. Simply preferring the second array is not a good compromise.
Returning strings for the result is a little strange and unnatural. While there are no enums in a language like JavaScript, using string literals is not a good replacement.
The name compare is very generic and doesn't explain what the function does at all.

To fix all these problems, I propose the following function:

function arrayCompare(arrayA, arrayB) {
  var averageA = arrayAverage(arrayA),
      averageB = arrayAverage(arrayB);
  return averageA < averageB ? -1
       : averageA > averageB ?  1
                             :  0;
}

This is clearer, and it handles the equality case more elegantly. If you really hate nested conditional expressions with a passion, you could use a set of if statements instead, but I think this is perfectly readable when properly formatted.

This change eliminates the need for the largestAveragedArray function entirely, simplifying the code somewhat. As a final note, document.write makes me cringe, but I understand that it's useful in code snippets, so I'll let it slide here. Just don't use it in a real project.

(function () {
  'use strict';

  // returns the average of a single array
  function arrayAverage(array) {
    return array.reduce(function (a, b) { return a + b; }) / array.length;
  }

  // compares two arrays;
  // returns -1 if avg(a) < avg(b), 1 if avg(a) > avg(b), or 0 if avg(a) = avg(b)
  function arrayCompare(arrayA, arrayB) {
    var averageA = arrayAverage(arrayA),
        averageB = arrayAverage(arrayB);
    return averageA < averageB ? -1
         : averageA > averageB ?  1
                               :  0;
  }

  var smaller = [100, 80],
      larger = [100, 100];

  document.write(arrayCompare(smaller, larger) + '
' + arrayCompare(larger, smaller) + '
' + arrayCompare(smaller, smaller));
})();

Actully thats a best algorithm for it

var smaller = [100, 80],
    same = [ 100,100],
      larger = [100, 100];

function largestAveragedArray( a, b ){
    var c = a.length > b.length ? a : b
        , i = 0
        , sumA = 0
        , sumB = 0
        , avgA = 0
        , avgB = 0;

    for( ; i < c.length; i++ ){
        sumA += a[ i ] ? a[ i ] : 0;
        sumB += b[ i ] ? b[ i ] : 0;
    }

    avgA = sumA / ( a.length > 0 ? a.length : 1 );
    avgB = sumB / ( b.length > 0 ? b.length : 1 );

    return avgA == avgB ? 0 : ( avgA > avgB ? 1 : -1 );
}

console.log( largestAveragedArray( smaller, larger ) );
console.log( largestAveragedArray( larger, smaller ) );
console.log( largestAveragedArray( same, larger ) );

It has O(n) complexity, being n the length of the biggest array. So instead of looping through both arrays it finds the biggest loop through it, sum a and b, acessing their indexes directly, and then divide the sum by it respectivo length to find the averages and then compare it.

Perfomance Test:
http://jsperf.com/averageaarray


Max path sum algorithm This is for Project Euler question 18 and I'm looking for feedback on whether my algorithm is incorrect or whether I've just implemented it wrong (which I don't think is the case after some testing).

The question is to traverse the maximum path in a triangle from top to bottom.

                            75
                          95  64
                        17  47  82
                      18  35  87  10
                    20  04  82  47  65
                  19  01  23  75  03  34
                88  02  77  73  07  63  67
              99  65  04  28  06  16  70  92
            41  41  26  56  83  40  80  70  33
          41  48  72  33  47  32  37  16  94  29
        53  71  44  65  25  43  91  52  97  51  14
      70  11  33  28  77  73  17  78  39  68  17  57
    91  71  52  38  17  14  91  43  58  50  27  29  48
  63  66  04  68  89  53  67  30  73  16  69  87  40  31
04  62  98  27  23  09  70  98  73  93  38  53  60  04  23

Starting from the second bottom row, each value is its current value + max(left child, right child), which transforms each position in the tree to be its max path sum from that point.

#include <iostream>
#include <fstream>
#include <iterator>
#include <sstream>
#include <vector>

using namespace std;
using Tree = vector<vector<int>>;

void transform_mps(Tree& tree) {
    // start from bottom up, each node's value is node.val + max(left child, right child)
    // transforms tree such that each node is that node's max path sum
    for (int layer = tree.size() - 2; layer >= 0; --layer) {
        for (size_t pos = 0; pos < tree[layer].size(); ++pos) {
            tree[layer][pos] = tree[layer][pos] + max(tree[layer+1][pos], tree[layer+1][pos+1]);
        }
    }
}

int main(int argc, char* argv[]) {
    if (argc < 2) { cout << "Need to pass in file to read from\n"; return 1; }

    ifstream f {argv[1]};
    if (!f.is_open()) { cout << "Could not open file\n"; return 1; }

    Tree tree;
    string line;
    while (getline(f, line)) {
        istringstream is {line};
        tree.push_back(vector<int>(istream_iterator<int>(is), istream_iterator<int>()));
    }
    transform_mps(tree);
    int mps {tree[0][0]};
    cout << "Max path sum: " << mps << endl;
}

/********** leftovers from previous implementation, ignore
constexpr bool LEFT {false};
constexpr bool RIGHT {true};

int pathsum(Tree& tree, size_t layer, size_t pos, bool right) {
    int sum {};
    ++layer;    // next level
    for (; layer < tree.size(); ++layer)
        if (right) sum += tree[layer][++pos];
        else sum += tree[layer][pos];
    return sum;
}
***********/

Looks good to me, algorithmically and syntactically. What makes you think it's wrong?

Stylistically, there are a few things you could improve:

Don't using namespace std. It's more idiomatic and readable to explicitly qualify std::max, std::cout, etc., and it's not much more typing.
Don't comment out dead code; delete it instead. ;)
tree[layer][pos] = tree[layer][pos] + ...; should be simply
tree[layer][pos] += ...;

And algorithmically, notice that if you start at the top of the tree and work downward, you don't need to store O(Rows²) numbers; you just need to store the previous row's sums, for a total space cost of O(Rows). (And then at the end you have Rows possible answers, and you pick the std::max_element of all of them.)

This might be a Project Euler constraint; but in the Unix/Linux world, it's more traditional to get your input from stdin rather than reading a filename off the command line. (Most utility programs support both, of course. The only one I know of that doesn't is tr.) Combine that with the top-down approach, and you get a nice Unixy stream filter that makes just one single pass over the data.

That the tree is modified is not an issue for me. Your function takes the tree by non-const reference, and if you want to keep the original you can always "store" the original.

That you read from a file is not an issue either. The algorithm has been separated from the way that data was read in.

I would probably have gone around creating some nice OO DAG and then use depth-first or breadth-first search. But then mine would be more generic for DAGs in general.

Either way, in my solution I would return the path itself as well as the result, something yours does not do.


Fisher-Yates modern shuffle algorithm I have implemented the shuffling algorithm of Fisher-Yates in C++, but I've stumbled across the modulo bias.  Is this random number generation by rand() correct for Fisher-Yates?

srand(time(NULL));
vector<int> elements;
// push numbers from 1 to 9 into the vector
for (int i = 1; i < 9; ++i)
{
    elements.push_back(i);
}

// the counter to be used to generate the random number
int currentIndexCounter = elements.size();
for (auto currentIndex = elements.rbegin(); currentIndex != elements.rend() - 1;
     currentIndex++ , --currentIndexCounter)
    {
        int randomIndex = rand() % currentIndexCounter;
        // if its not pointing to the same index
        if (*currentIndex != elements.at(randomIndex))
        {
            //then swap the elements
            swap(elements.at(randomIndex), *currentIndex);
        }
    }

@Loki Astari is correct in his comment:

  rand() is well know for its bad distribution. But you are using it correctly. Note: rand() % currentIndexCounter does not give you a perfect distribution unless currentIndexCounter is an exact divisor of RAND_MAX. You may want to look at C++11 random header and the generators provided inside.

Since you're using C++11, you should instead utilize the <random> library.  It includes things such as pseudo-random number generators and random number distributions.

Beyond that, I'll just point out some additional things:

With C++11, you should now be using nullptr instead of NULL.

This should be changed in srand():

std::srand(std::time(nullptr));

However, as mentioned above, you should not be using this with C++11.  But in any situation where you must stay with rand, the above would be recommended.
With C++11, you should also have access to initializer lists.  This will allow you to initialize the vector instead of calling push_back() multiple times:

std::vector<int> elements { 1, 2, 3, 4, 5, 6, 7, 8, 9 };

Also note that your push_back() was only pushing numbers 1-8 into the vector, despite what your comment said.  You've used < instead of <= in the loop statement, so the 9 was excluded.
currentIndexCounter should be of type std::vector<int>::size_type, which is the return type of size() (or you can just use auto).  This should've given you compiler warnings as it's a type-mismatch and a possible loss of data.  Make sure your compiler warnings are turned up high.
currentIndex is a misleading name because it's being used with iterators, not indices.

Here, auto gives currentIndex the type std::reverse_iterator from std::rbegin().  You could simply name this something like iter.  It's okay for iterator names to be short.
In the second for loop, you just need elements.rend(), without the - 1.  The iterator will already go through the entire vector and stop at the last reverse element.
The iterator increment should be prefix instead in order to improve performance a bit by avoiding a copy.  It's best to do this with all nontrivial types such as iterators.
I've tested this by giving randomIndex a value greater than elements.size(), causing an exception to be thrown.  Even if std::rand() or another RNG is guaranteed to give you an intended value, it may be best to handle this properly.

Here's the final version with my own given changes, which I've also ran here.  I've also included a concise way of displaying the vector before and after the shuffling.

#include <algorithm>
#include <iostream>
#include <iterator>
#include <random>
#include <vector>

int main()
{
    // seed the RNG
    std::random_device rd;
    std::mt19937 mt(rd());

    std::vector<int> elements { 1, 2, 3, 4, 5, 6, 7, 8, 9 };

    std::cout << "Before: ";
    std::copy(elements.cbegin(), elements.cend(),
        std::ostream_iterator<int>(std::cout, " "));

    auto currentIndexCounter = elements.size();

    for (auto iter = elements.rbegin(); iter != elements.rend();
        ++iter, --currentIndexCounter)
    {
        // get int distribution with new range
        std::uniform_int_distribution<> dis(0, currentIndexCounter);
        const int randomIndex = dis(mt);

        if (*iter != elements.at(randomIndex))
        {
            std::swap(elements.at(randomIndex), *iter);
        }
    }

    std::cout << "\nAfter: ";
    std::copy(elements.cbegin(), elements.cend(),
        std::ostream_iterator<int>(std::cout, " "));
}

Output:

Before: 1 2 3 4 5 6 7 8 9
After: 7 9 3 2 4 5 6 1 8

Here is my take on it with the modern <random>:

static const void fisher_yates_shuffle(std::deque<card>& cards)
{
  std::random_device                          random                    ;
  std::mt19937                                mersenne_twister(random());
  std::uniform_int_distribution<std::uint8_t> distribution              ;
  for (auto i = cards.size() - 1; i > 0; --i)
  {
    distribution.param(std::uniform_int_distribution<std::uint8_t>::param_type(0, i));
    std::swap(cards[i], cards[distribution(mersenne_twister)]);
  }
};


Verifying a person's age When coding I noticed I started to get into a habit of not using if-else statements when the else block only has one line of code. For example, if I have code that can be solved like this:

public Person(int initialAge) {
    if(initialAge < 0){
        System.out.println("Age is not valid, setting age to 0.");
        age = 0;
    }
    else{
        age = initialAge;
    }
   }

I will instead remove the else statement entirely to cut down on a few lines of code. This results in my code looking more like this:

public Person(int initialAge) {
    age = initialAge;
    if(initialAge < 0){
        System.out.println("Age is not valid, setting age to 0.");
        age = 0;
    }
   }

I'm wondering if this is a bad coding habit to get into and if I should break out of this coding habit or if it'll be fine to continue doing this. Would doing this be a bigger issue down the line when I get onto more complex programs?

Edit: Just wanted to give some more information for why part of the code is written the way it is. The snippet provided is part of a coding challenge where one of the requirements is to set any negative number inputted to 0.

The biggest problem I see here is that you are printing a warning message to System.out and then using a default value for something which sounds like an Exception.

Why allow negative values at all?

if (initialAge < 0) {
    throw new IllegalArgumentException("Initial age cannot be negative, was specified as " + initialAge);
}
age = initialAge;

This way it is always up to the caller to pass a valid age instead of the method defaulting to zero which will bring other problems. It is better to have your program break early because of a bug than to have a warning message be printed out which may indicate a bug.

An example where this can go terribly wrong: you have multiple threads which may have access to a variable foo. Imagine a situation where you first set foo to the else-value in thread 1. Then the CPU gets handed to thread 2 which reads the wrong value from foo before thread 1 is able to set the correct one.

Of course there are a lot of ways to avoid this, but let's be honest - everyone makes mistakes. And when the mistake is using a slightly wrong value in a complex calculation, well, good luck debugging.

Bonus information: both C++ and Java (and other languages?) allow you to just omit the curly brackets around the else for a single statement, eg:

if (foo)
    bar = 1;
else
    bar = 2;


How can I prettify blocks with more than 2 of each block? Actually, I just have no ideas.

How can I make breaklines to make it more readable?

Can anyone suggest ways on improving this?

module TestingDataLoad
  def get_files_from(search_target)

    Dir[search_target].each do | d |
      testing_files={}
      if File.directory? d
        Dir.glob("#{d}/*json") do |file|
          if File.basename(file,".json") =~ /(\w+)(_\d+-\d+)/
            event_type = $1.to_sym
            if testing_files.has_key? event_type
              testing_files[event_type] << file
            else
              testing_files[event_type] = [file]
            end
          elsif File.basename(file,".json") =~ /(\w+)/
            event_type = $1.to_sym
            testing_files[event_type] = [file]
          end
        end
      end

      return testing_files
    end
  end

A couple of things I noticed:

Dir[...] and Dir.glob(...) are synonymous, but you're using both forms?
Speaking of, glob accepts fairly complex patterns, which could help narrow the search
Skip the testing_files.has_key? check; just use testing_files[event_type] ||= []
The explicit return makes little sense. You're looping through the globbed dir paths, but your explicit return means that only the first will ever be checked. Maybe there's only supposed to be one path, but that depends on what search_target is, which you don't know.
Your method will return nil, an empty hash or a populated hash. Limit the number of possible return values.

Mostly though, it seems like the filename-branching is unnecessary. In both cases you add grab the \w+ part of the name, and use it as a hash key for an array. The only difference is that
if the filename has numbers in it, it'll add the file to the array, otherwise it'll replace the array with a new 1-element array.

I'd say treat all files the same, and append them to the array. Don't ever replace the array. As far as I can figure, your method is entirely dependent on the platform/filesystem you're running it on, since glob doesn't dictate the order in which files are returned.

So given 3 files in the following order:

foobar.json
foobar_1-1.json
foobar_1-2.json

your method will return all three. But if the order's reversed, it'll only return "foobar.json". Or, if it's somehow mixed, it might return "foobar.json" and "foobar_1-1.json", but not the 3rd file.

So instead of risking inconsistent behavior, I'd suggest sticking to 1 way of treating the files, regardless of whether they're numbered or not.

def get_files_from(search_target)
  files = {}

  Dir[search_target].each do | d |
    next unless File.directory?(d) # skip if d isn't a dir

    Dir["#{d}/*.json"] do |file|
      event = File.basename(file)[/^\w+/]
      testing_files[event] ||= []
      testing_files[event] << file
    end
  end

  files
end

If you want to be more exacting, you can do this instead

Dir["#{d}/*.json"].select { |file| File.basename(file, '.json') =~ /^\w+(_\d+-\d+)?$/ }.each do |file|
  ...
end

I have no idea what your code does--never used ruby or json--but if you're going for readability, space your if statements:

// Pseudo Code(!!):
if expression
    if expression
        result
    end

    if expression
        result
    end
end

That works, or you can use the following method:

if expression // -- 1 --
    if expression // -- 2 --
        result
    end // -- 2 --

    if expression // -- 3 --
        result
    end // -- 3 --
end // -- 1 --

Doing this allows you to recognize the beginning and end of each statement
as well as the result within each statement.


What changes should I implement in my templated sorter class so that it performs efficiently? I have a college assignment to implement a class called sorter, whose object is a sorted vector (the user is allowed to sort it by the method he/she desires). I have been able to implement it as following. Please review it and suggest me the changes I should make to make it efficient.

template <typename T>
class Sorter {
    std::vector<T> vector;
    int count(T* array) {
        register int i = 0;
        while (*(array + i)) {
            i++;
        }
        i--;
        return i;
    }
    void swap(T& t1, T& t2) {
        T temp = t1;
        t1 = t2;
        t2 = temp;
    }
    void bubbleSort() {
        for (int i = 0; i < vector.size(); i++) {
            for (int j = 0; j < vector.size() - 1; j++) {
                if (vector[j] > vector[j + 1]) {
                    swap(vector[j], vector[j + 1]);
                }
            }
        }
    }
    std::vector<T> mergeSort(std::vector<T> vec) {
        int begin = 0;
        int end = vec.size() - 1;
        auto combine = [=](std::vector<T> lv, std::vector<T> rv) {
            std::vector<T> retVec;
            int i = 0;
            int j = 0;
            while (i < lv.capacity() && j < rv.capacity()) {
                if (lv[i] < rv[j]) {
                    retVec.push_back(lv[i]);
                    i++;
                }
                else {
                    retVec.push_back(rv[j]);
                    j++;
                }
            }
            while (i < lv.capacity()) {
                retVec.push_back(lv[i]);
                i++;
            }
            while (j < rv.capacity()) {
                retVec.push_back(rv[j]);
                j++;
            }
            return retVec;
        };
        if (vec.size() == 1) {
            return vec;
        }
        int mid = (begin + end) / 2;
        auto it = vec.begin();
        std::vector<T> left = mergeSort(std::vector<T>(it, it + mid + 1));
        std::vector<T> right = mergeSort(std::vector<T>(it + mid + 1, vec.end()));
        return combine(left, right);
    }
    void sort() {
        std::cout << "Choose your option:" << "\n";
        std::cout << "1. bubblesort" << "\n";
        std::cout << "2. mergesort" << "\n";
        int op = 2;
        //std::cin >> op;
        switch (op) {
        case 1:
            bubbleSort();
            break;
        case 2:
            vector = mergeSort(vector);
            break;
        }
    }
public:
    Sorter() = delete;
    Sorter(std::vector<T> vec) : vector(vec) {
        sort();
    }
    Sorter(T* array) {
        int num = count(array);
        for (int i = 0; i < num; i++) {
            this->vector.push_back(*(array + i));
        }
        sort();
    }
    std::vector<T> getSorted() {
        return vector;
    }
};

The updated version is always available at this link

In the sort() function its currently hardcoded to merge sort which I assume is not intentional. However, if you do allow the user to give an input you should do some error checking. You'll currently get unexpected behaviour if the user enters anything other than 1 or 2.
You should use std::swap rather than implementing your own swap function. Some types can make optimisations over what you've done.
From a design point of view I would argue that c++ is not an inherently object oriented language and that bubblesort and mergesort should be free functions taking a pair of iterators, to allow them to be more easily reused. You've pretty much done this for merge sort already, it uses none of the members of the Sorter class so could just be made a free function.
Your bubble sort implementation is inefficient. It should check if the vector is sorted after each pass, rather than doing the outer loops you could do while(!isSorted(vector)) or similar.
Merge sort has a very big lambda in it for combine. From a readability  point of view I would pull that out into a separate function.
Both mergesort and bubblesort use indices with operator[] to get the elements from the vector. The preferred way to do this in c++ is using iterators, see https://stackoverflow.com/questions/131241/why-use-iterators-instead-of-array-indices
In count() register is deprecated (and strictly is removed since c++17). You should probably avoid using it to improve compatibility.
I would need to think about it more, but I question if while (*(array + i)) in count will always behave as expected. You assume that the array has an end sentinel value that will dereference to null. I don't that's guaranteed. As far as I know there is not an easy way to get the size from a c style array. The normal way to handle this is to require the user to pass in the size as well.
Update on capacity(). Your use of capacity() in the merge sort is not correct. You should use size() instead. size() tells you the number of elements currently in the vector whereas capacity() tells you how many elements the vector reserved space for internally. i.e. if you insert more than this number of things into the vector it will internally reserve more space and do reallocate all the elements. It's important to note that the capacity can be greater than the number of elements in the vector. In your case it works ok as the constructor with a pair of iterators initialises the capacity equal to the size. However, I don't think this is guaranteed and is complier dependant. To see the difference try the following code:

    std::vector<int> v;
    std::cout << 0 << " " << v.size() << " " << v.capacity() <<"\n";
    for( int i = 0; i < 10; ++i)
    {
        v.push_back(i);
        std::cout << i << " " << v.size() << " " << v.capacity() <<"\n";
    }

0 0 0
0 1 1
1 2 2
2 3 4
3 4 4
4 5 8
5 6 8
6 7 8
7 8 8
8 9 16
9 10 16

READABILITY

Function: int count(T* array)

I don't prefer this way of acessing a (T) while(*(array+i)), because of readability reasons. Better way: while(array[i])

PERFORMANCE

Function: void swap(T& t1, T& t2)

Faster way is to use XOR (^) swap algorithm. it is faster and small amout of memory is used.

if (t1 != t2)
  {
    *t1 ^= *t2;
    *t2 ^= *t1;
    *t1 ^= *t2;
  }

and function declaration should be inline void swap(T& t1, T& t2)

Function:  std::vector<T> getSorted()

Should be inline std::vector<T> getSorted()


Handling missing elements on login form in Selenium I try to improve my code so that I don't have to program a try - catch block all the time.

e.g. at the moment I have to write many codeblocks like this:

try {
    $login_feld = $SeleniumObj->driver->findElement(WebDriverBy::id("login_feld"));
} catch(NoSuchElementException $exception) {
    $SeleniumObj->setErrorMessage("1 ID login_feld does not exist.", $exception,  true, "login_field");
    break;
}
try {
    $password_field = $SeleniumObj->driver->findElement(WebDriverBy::id("password_field"));
} catch(NoSuchElementException $exception) {
    $SeleniumObj->setErrorMessage("1 ID password_field does not exist.", $exception,  true, "login_field");
    break;
}
try {
    $field3 = $SeleniumObj->driver->findElement(WebDriverBy::id("field3"));
} catch(NoSuchElementException $exception) {
    $SeleniumObj->setErrorMessage("1 ID field3 does not exist.", $exception,  true, "login_field");
    break;
}

My goal is to reduce this to one line.

Therefore I outsourced the whole driver->findElement(WebDriverBy::id("xy")); to a new function from the class Selenium called ById

/**
 *
 * @param id $id
 * @param bool $takeScreenshot
 * @return void
 */
public function ById($id, $takeScreenshot=false)
{
    try {
        return $this->driver->findElement(WebDriverBy::id($id));

    } catch(NoSuchElementException $exception) {

        $this->setErrorMessage("ID '$id' does not exist.", $exception,  $takeScreenshot, $id);
        //break;  <- does not work in this context
    }
}

Now theoretically I can code the whole block like this:

$login_feld = $SeleniumObj->ById('login_feld', true);
$password_field = $SeleniumObj->ById('password_field', true);
$field3 = $SeleniumObj->ById('field3', true);

However, one problem still persist. I can't call break in the exception block in the function ById, otherwise I get

  break' not in the 'loop' or 'switch' context

Now to solve this I could instead return a status, e.g. false if the code reached in the exception block and then check this before proceeding

/**
 *
 * @param id $id
 * @param bool $takeScreenshot
 * @return void
 */
public function ById($id, $takeScreenshot=false)
{
    try {
        return $this->driver->findElement(WebDriverBy::id($id));

    } catch(NoSuchElementException $exception) {

        $this->setErrorMessage("ID '$id' does not exist.", $exception,  $takeScreenshot, $id);
        return false;
    }
}

-

$login_field = $SeleniumObj->ById('login_field', true);
if ($login_field == false) { break; }
$pw_field = $SeleniumObj->ById('pw_field', true);
if ($pw_field == false) { break; }
$field3 = $SeleniumObj->ById('field3', true);
if ($field3 == false) { break; }

But now I need two lines per block instead of one.
Is it possible to improve this even further?

I don't understand what you are intending to do with break in your catch blocks to begin with.  Even in your first code example, you are not within a loop construct to where this makes sense.

In your new code, you should have method name begin with lower case (i.e. byId) as this is pretty much standard in PHP.  I don't really see what value this method is bringing though, as I would tend to agree with the post by @YourCommonSense that you might simplify your operation by using a looping construct.  However I might suggest you put the try-catch within the loop if your intent is to be able to engage in the try-catch for each element rather than for the collection of elements as a whole.

For example:

$elements = [
    'login_feld' => null,
    'password_field' => null,
    'field3' => null
];
foreach($elements as $id => $notUsed) {
    try {
        $elements[$id] = $SeleniumObj->driver->findElement(WebDriverBy::id($id));
    } catch (NoSuchElementException $e) {
        $SeleniumObj->setErrorMessage(
            '1 ID ' . $id . 'does not exist.',
            $e,
            true,
            $id
        );
        // if you want to break here, you now have proper loop context
        // break;
    }
}

// if successful, you can now access the elements from $elements array

This approach is scalable as you add more elements into your test cases, as you could just add new id keys into the element array and not worry about polluting your global namespace with a bunch of $[elementId] variables. You also now have the ability to simply look at the value stored at each key to determine if the element was found.  So for example, if you are going to perform a test action against a certain element, you can test for it like:

if(!is_null($elements['login_field'])) {
    // execute tests against element
} else {
    // log test fail
}

This approach might totally alleviate your need to break altogether, meaning you could now continue to execute your test suite even if one of the elements you expect to be present is missing.  You could just fail those particular tests relevant to the missing element.

Every time you see repeated lines of code, they can be reduced to a single line run in a loop, so you can do:

try {
    foreach(["login_field","password_field","field3"] as $field)
    {
        $$field = $SeleniumObj->driver->findElement(WebDriverBy::id($field));
    }
} catch(NoSuchElementException $exception) {
    $SeleniumObj->setErrorMessage("1 ID $field does not exist.", $exception, true, $field);
    break;
}