Optimize this jQuery tab controller further How do I optimize this code further?  Goal is to edit "js-tab" in one place.  This is what I tried to follow.

Full code here.

var ToggleTab = {

init: function() {
    this.tab = $(".js-tab");
    this.bindEvents();
},

bindEvents: function() {
    this.tab.on("click", this.toggleTab);
    //this.tab.on("click", $.proxy(this.toggleTab, this));
},

toggleTab: function(e) {
    var showTab = $(this).data("tab-content");
    e.preventDefault();

    $(".js-tab").each(function() {
        if ($(this).data("tab-content") === showTab) {
           $(".js-tab-" + showTab).removeClass("hide");
        } else {
            $(".js-tab-" + $(this).data("tab-content")).addClass("hide");
        }
    });
}

};

$(function() {

    ToggleTab.init();

});

I've always been partial to writing JavaScript classes that take a root element:

function TabController() {
    // Maintain reference to "this" in event handlers
    this.toggleTab = this.toggleTab.bind(this);
}

TabController.prototype = {

    $element: null,

    constructor: TabController,

    init: function(element) {
        this.$element = $(element).on("click", ".js-tab", this.toggleTab);

        return this;
    },

    toggleTab: function(event) {
        var that = this,
            showTab = event.target.getAttribute("data-tab-content");
        event.preventDefault();

        this.$element.find(".js-tab").each(function() {
            var tabContent = this.getAttribute("data-tab-content");

            if (tabContent === showTab) {
                that.$element.find(".js-tab-" + showTab).removeClass("hide");
            } else {
                that.$element.find(".js-tab-" + tabContent).addClass("hide");
            }
        });
    }
};

Plus, I like to replace jQuery API calls that wrap cross browser native DOM methods  -- e.g. $(foo).data("bar") most times can be replaced with element.getAttribute("data-bar").

And to use:

// using document.documentElement means you don't need a jQuery dom-ready event handler
var tabs = new TabController().init(document.documentElement);

While you technically only need one tabber per page, making this an instantiable class allows you to unit test your JavaScript.

describe("TabController", function() {

    var element, tabs, event;

    function FauxEvent(type, target) {
        this.type = type;
        this.target = target;
    }

    FauxEvent.prototype.preventDefault = function() {};

    describe("toggleTab", function() {

        beforeEach(function() {
            element = document.createElement("div");
            tabs = new TabController().init(element);
        });

        it("shows a tab", function() {
            element.innerHTML = [
                'Tab 1',
                '

'
            ].join("");

            event = new FauxEvent("click", element.firstChild);

            tabs.toggleTab(event);

            expect($(element.lastChild).hasClass("hide")).toBe(false);
        });

    });

});

This makes your code truly modular. You could go a step further and make the CSS class names and data attributes customizable:

function TabController() {
    // Maintain reference to "this" in event handlers
    this.toggleTab = this.toggleTab.bind(this);

    this.options = {
        classPrefix: "js-tab",
        hiddenClass: "hide",
        dataAttribute: "data-tab-content"
    };
}

TabController.prototype = {

    $element: null,

    options: null,

    constructor: TabController,

    init: function(element, options) {
        this.$element = $(element).on("click", "." + this.options.classPrefix, this.toggleTab);
        jQuery.extend(this.options, options || {});

        return this;
    },

    toggleTab: function(event) {
        var that = this,
            showTab = event.currentTarget.getAttribute(this.options.dataAttribute);
        event.preventDefault();

        this.$element.find("." + this.options.classPrefix).each(function() {
            var tabContent = this.getAttribute(that.options.dataAttribute);

            if (tabContent === showTab) {
                that.$element.find("." + that.options.classPrefix + "-"
                    + showTab).removeClass(that.options.hiddenClass);
            } else {
                that.$element.find("." + that.options.classPrefix + "-"
                    + tabContent).addClass(that.options.hiddenClass);
            }
        });
    }
};

Some sample HTML

Tab 1

Tab 2

    Content A

    Content B

JSFiddle: http://jsfiddle.net/CL8Ns/


Handling a text field and submit button with Ajax There's this Ajax-heavy, jQuery-based site which I'm about to work on. There hasn't been much structure in the JavaScript code so far (lots of functions at global scope, hard-coded in the HTML onclick="something(args);".  How can I improve upon that and introduce some structure?

Here's what I've tried. This part is supposed to handle a single element which consists of a text field (span) which is replaced by an input on focus and runs an AJAX update call when a submit button is clicked or enter is pressed.

$(function() {
    var box = $('.foobar-info'),
        input = box.find('input'),
        label = box.find('span#name'),
        button = box.find('button');

    label.click(start_edit);
    button.click(submit);
    input.keypress2(KEYS.enter, submit);

    function start_edit() {
        box.addClass('is-edit');
    }
    function submit() {
        MyAjaxModule.edit_foobar(save_success,
            {'id': GLOBAL_FOO_ID, 'value': input.val()});
    }
    function save_success(data) {
        label.text(input.val());
        box.removeClass('is-edit');
    }
});

Showing and hiding of parts is driven by the CSS:

.foobar-info .state-display {display: inline;}
.foobar-info .state-edit {display: none;}
.foobar-info.is-edit .state-display {display: none;}
.foobar-info.is-edit .state-edit {display: inline;}

So far this achieves:

keeps the behaviour away from the markup
keeps relevant behaviour together in one block

On the other hand, I don't think it's testable or particularly easy to re-use with different configurations.

I've attempted to rewrite this to use objects and this. lookups instead of the big closure, but didn't end up in a code that would please me. For example, this part

label.click(start_edit);

would look like

this.label.click(this.start_edit.bind(this));

which looks really verbose plus it's super-easy to forget the unwieldy .bind(this) part.

This article served some inspiration, but felt rather hacky to me, mostly because relying on global name lookups.

Questions:

Any particular problems with my approach except what I've mentioned so far? How can I solve it?
Is there a canonical way to structure the code which I'll be happy to incorporate?

There are two schools of thought

Programmatic
Declarative

Your example here is programmatic, where JavaScript is binding events after the DOM load. The problem I have with programmatic is, using your words, although it "keeps the behavior away from the markup" it moves the markup into the behavior. The interface that is needed to reuse the same JavaScript is now tied to what is defined on the page. Using declarative, your example being onclick="something(args);", I can more easily follow a Model View Presenter (MVP) pattern where the view simply forwards all events to the Presenter to perform the business logic.

Here the events can be organized into a class object to remove them from the global namespace. Scope can be controlled by passing in the model as a parameter.

presenter.js

function Presenter(parameter) {

  // private variables
  var myVar;

  // public method
  this.submit = function() {
  };

  // private method
  function doSomething() {
  }
}

page.html

...
<script type="text/javascript" src=".../presenter.js"></script>
<script type="text/javascript">
  var presenter = new Presenter(...);
</script>
...
<button onclick="presenter.onButtonClick();">Click me</button>


Computing the n-th prime I've written some Python 3 code that computes the \$n\$-th prime. I implemented first a naive isprime function that looks for divisors of \$m\$ between \$2\$ and \$\lfloor \sqrt m \rfloor+1\$. Then a loop looks for primes and stops when the \$n\$th one is found.

from math import sqrt
def isprime(n):
    for i in range(2,int(sqrt(n))+1):
        if n%i==0:
            return False
    return True

def prime(n):
    m=3
    i=2
    ans=3
    while i<=n:
        if isprime(m):
            i=i+1

            ans=m
            m=m+2
        else:
            m=m+2
    return ans

It occured to me that prime performs a lot of unnecessary computations: for a given \$m\$, it checks if composite numbers (like 14,16) divide \$m\$. That is useless, and it would be more efficient to look only for prime divisors of \$m\$. This led me to some "storage" approach, where I maintain a list of all the primes I've found, and use them to test for divisors of the next numbers.

from math import sqrt
def prime(n):
    list=[2]
    i=1
    m=3
    while i<n:
        flag=0
        for p in list:
            if m%p==0:
                flag=1
                break
            else:
                continue
        if flag==0:

            list.append(m)
            m=m+2
            i=i+1
        else:
            m=m+2
    return list

The \$n\$th prime is given by prime(n)[-1]

I have an issue with the performance of the second code: it's really slow.
On my computer, according to the Unix command time python code.py, computing the \$6000\$-th prime with the first code takes \$0.231\$ seconds, and \$2.799\$ seconds with the other approach!

Why is the clever way slower than the naive one?

First, you should read the Python style guide it has a lot of info on good practices for code readability.

But as for your code, part of the reason the smarter way is slower is that you're creating the whole list every time. It's not stored at all when you create it and this means that instead you're just taking extra time because it has to go through the effort of building the list every time even though you only need the last value. Also, you're not limiting what primes you test for division, so you've lost the root optimisation you used before.

If you cached this list as part of the function then you actually would have time saved on repeated calls, though it would still be slower initially due to building the initial list. There is a way to set up a cache pretty easily, using default parameters. If you're unfamiliar with them, leave a comment and I can explain more about how they're generally used, though I'm using them in a different way than normal here.

If you change def prime(n) to def prime(n, cache=[2]) then you'll have a list called cache created containing 2, the first prime. Each time you call prime it refers back to the same list, so if you append your primes to it, it will be retained and save you all that work.

First of all, this means you might not need to calculate anything when you start. You can just try to return your prime value immediately:

def prime(n, cache=[2]):
    try:
        return cache[n - 1]
    except IndexError:
        pass

To explain, try except is used to attempt a piece of code that may generate an error. So it tries to return the nth prime (use n-1 because lists are indexed from 0) but if it can't find that value in the list it will throw an IndexError. Our except will catch this and just pass, meaning it goes on to run the rest of the function.

The other benefit is you can start your while loop from the last point in that list. So m (which I'd rename as num) can just be the last prime cached + 1 and i can be set as len(cache), since that's the amount of previous primes we're skipping.

i = len(cache)
num = cache[-1] + 1

Now you can begin your while loop, skipping the previously found values but still having them as your divisor check. Note also, you can collapse this with some other syntax and smarter logic. Python has else syntax for a for loop. If you never call break from a for loop then the else will be run, but otherwise it will be skipped. With this in mind you can remove multiple lines where you had to use flag before. Also if you're calling m = m + 2 in both cases, it should just be at the end for both conditions. And lastly, you can shorten var = var + 1 to var += 1 in Python, so I did that too.

while i < n:
    for prime in cache:
        if num % prime == 0:
            break
    else:
        cache.append(num)
        i += 1
    num += 2

Here's how the fully rewritten code would look:

def prime(n, cache=[2]):

    try:
        return cache[n - 1]
    except IndexError:
        pass

    i = len(cache)
    num = cache[-1] + 1

    while i < n:
        for prime in cache:
            if num % prime == 0:
                break
        else:
            cache.append(num)
            i += 1
        num += 2

    return cache[-1]


Parsing scraped data from html table I've written a simple python web scraper that parses text from an html table and stores the scraped data in List of dictionaries. The code works and doesn't seem to have any glaring issues performance-wise, but I only used the bare bones modules of lxml and requests.

Is there a more efficient/elegant way to condense the script or to improve the runtime?

The code is below:

import requests
from lxml.html import fromstring
import pprint
import re

url = "https://jobs.mo.gov/content/missouri-warn-notices-py-2017"
response = requests.get(url)
root = fromstring(response.content)

table = root.xpath('.//*[@summary="Missouri WARN Notices PY 2016"]')[0]
tableRes = []
columnHeaders = table.xpath(".//tr//th/span/text()")

for row in table.xpath(".//tr")[1:]:
    i = 0
    rowDict = {}
    for col in row.xpath(".//td"):
        if i != 1:
            rowDict[columnHeaders[i]] = re.sub(r"[\n\t]*", "","".join(col.xpath(".//text()")).replace(u'\xa0', u' '))
        else:
            rowDict[columnHeaders[i]] = re.sub(r"[\n\t]*", "","".join(col.xpath(".//a/text()")).replace(u'\xa0', u' '))
        i += 1
    tableRes.append(rowDict)

pp = pprint.PrettyPrinter(indent=4)
pp.pprint(tableRes)

Looked at your script and found a few things that can be improved in terms of code quality as well as performance. On my local machine i was able to cut the runtime in half with the code changes.

The default separator value of str.split() is whitespace character(s) (space, tab, newline, return, formfeed). Combining it with a ' '.join() will return a proper string. The unicode chars ('xa0') in your example are also handled by this approach. In general a good way to deal with unicode is to use the normalize function from the unicodedata package.
Instead of define and manipulate an index for your loop you can simply use enumerate in your for-loops.
I prefer to store the values per row in a simple list and create the dict after the loop is done via the zip function. The performance improvement is minor but i think it improves readability of the code quite a bit.
In general you should maybe have a look at the PEP-8 guide - your variable names are not PEP-8 conform.

Improved version:

    table_res = []
    column_headers = table.xpath(".//tr//th/span/text()")
    for row in table.xpath(".//tr")[1:]:
        cells = []
        for i, cell in enumerate(row.xpath(".//td")):
            if i != 1:
                cells.append(' '.join(cell.text_content().split()))
            else:
                cells.append(' '.join(''.join(cell.xpath('.//a/text()')).split()))

        table_res.append({k: v for k, v in zip(column_headers, cells)})

When you compare the outputs of both versions you will find some small differences. This is mainly due to the fact that your strings sometimes have a trailing whitespace or the date is missing some whitespace.


Sorting an Integer array The program sorts the array from lowest to highest and outputs the index of the searched value:

public static void main(String[] args) {

    Integer[] random = {6, -4, 12, 0, -10};
    List<Integer> list = new ArrayList<Integer>(Arrays.asList(random));
    Collections.sort(list);
    Integer[] array = list.toArray(new Integer[list.size()]);

    int y = Arrays.binarySearch(array, 6);
    System.out.println(y);

}

This works as expected, but you can see that I had to use 3 lines just to sort the array, before being able to do a binarySearch on it.

Can this code be improved?

As you say, this is wrong. This

Integer[] random = {6, -4, 12, 0, -10};
List<Integer> list = new ArrayList<Integer>(Arrays.asList(random));
Collections.sort(list);
Integer[] array = list.toArray(new Integer[list.size()]);

could be replaced by

Integer[] random = {6, -4, 12, 0, -10};
List<Integer> list = Arrays.asList(random);
Collections.sort(random);

where Arrays.asList just wraps the array without making a copy. Even better would be

Integer[] random = {6, -4, 12, 0, -10};
Arrays.sort(random);

Also note that sorting something just in order to be able to use binary search once makes no sense.

Also note that

Integer[] random = {6, -4, 12, 0, -10};

uses objects wrapping the int, while

int[] random = {6, -4, 12, 0, -10};

would be much more efficient.


Wrap array operation result string inside another string I'm trying to loop through an array, add " before, ", after each member but ommit the , at the very last member in order to generate a JSON string:

const generate_list_data_attribute = (attribute_name, from_array) => {
    let string = '';

    from_array.forEach((element, key, array) => {
        if(Object.is(array.length - 1, key)) {
            string += `"${element}"`
        } else {
            string += `"${element}",`
        }
    });

    return `data-${attribute_name}="[${string}]"`;
}

I don't like the fact that I need to Object.is(array.length-1, key) in order to dictate what the code should do with the last member. It should be automatically handled.

To note: I understand that I can just return to skip the usage of else but I'm looking for a way to automatize that "last member" check more or less.

Input (array): my_array = ["food", "gourmet", "foodie"]

Operation: generate_list_data_attribute('random', my_array);

Output (string): data-random="["food", "gourmet", "foodie"]"

Use JavaScript's array methods to your advantage and solve you problem the functional way:

const generate_list_data_attribute = (attribute_name, from_array) => {
  const string = from_array.map(element => `"${element}"`).join(",")
  return `data-${attribute_name}="[${string}]"`
}


Deleting/sending to trash files with Python I made this program to automate/simplify the erasing of a lot of files, I have some ideas to improve it but that's not why I'm putting it here, it's because it's a very dangerous program(it deletes stuff permanently) and since I'm a beginner(ish) I want some advice from better programmers to improve it.

import os
import send2trash
import shutil
file = input("Type the file path")
while os.path.exists(file) is False:  # validation
    file = input("Path not found, try again:")
for i, j, k in os.walk(file):  # files check
    print("The folder is in this path:", i)
    print("The subfolders are named:", j)
    print("The files are named:", k)
while True:
    try:
        command = int(input("1 - Delete permanently the file\n2 - Send the file to the bin"))
    except ValueError:
        print("Just the numbers 1 or 2 are accepted, follow the instructions")
        continue
    if command not in range(1, 3):
        print("Just the numbers 1 or 2 are accepted, follow the instructions")
        continue
    else:
        break
if command == 1:
    try:
        os.rmdir(file)  # deletes if there is no inside folder
    except OSError:
        shutil.rmtree(file)  # deletes if there is inside folders
else:
    send2trash.send2trash(file)  # send to the bin

There are some things that can be improved here

Divide your code into functions, don't put everything in the global namespace!
Use a if __name__ == '__main__' guard.
It's not bad to give your code a little more space in the sake of readability, a few blank lines would help make this code look better.

Read PEP8#blank-lines for more details.
Don't use == or is for False, True comparison

See PEP8#programming-recommendations.

while os.path.exists(file) is False:

You can rewrite to a more idomatic while not os.path.exists(file):
Naming is important

file = input("Type the file path")

This is a path, so rename it to path = ...

for i, j, k in os.walk(file):

You could name those to what they represent:

for folder, subfolder, files in os.walk(path):


Series sum calculator This is my program currently to calculate sum of following series:

$$\frac{x^2}{2!} + \frac{x^4}{4!} + \frac{x^6}{6!} \dots$$

#include <iostream>

using namespace std;
int main() {
    int fac = 1;
    float x, sum = 0, p = 1;

    cout << "\nENTER x: ";
    cin >> x;
    p =  x;
    for(int i = 2; i <= 10; i++) {
        fac *= i;
        p *= x;
        cout << p << " ";
        if(i%2 == 0) {
            sum += p/fac;
        }
    }
    cout << " >> " << sum;
    return 0;
}

Is it well written? Can it be used without any optimisation for larger n values (currently only tried with small values)? How can I optimize these calculations?

Here is a rewritten version of your sum which incorporates some techniques which improve performance, numeric stability and the accuracy of the final result.

The two multiplicative terms (the power of x and the factorial) are combined into a single accumulated value (the variable named "term").  Obviously, for larger x, the two separate values grow rapidly, quickly surpassing the capacity of their types.
But the combined term (power / factorial) is well-behaved for a much broader range of input values, usually going to zero as iterations accumulate.
Each iteration of the loop corresponds to one term in the summation.  There is no fundamental reason for each iteration to correspond to each integer power of x. For this particular series, the term's power of x goes up by 2, so I precompute the square of x in order to do this with a single multiplication.  This also gets rid of that awkward conditional inside the loop (if(i%2==0)) (probably optimized away by the compiler, but still best to be rid of it).
The last trick automatically determines the iteration count by comparing this iteration's updated sum (the variable next_sum) with the last iteration's sum (the running sum variable).  If the sum hasn't changed, it stops iterating and returns that value.  This technique causes problems in several situations, however and cannot be universally relied upon.  For example, some series don't sum monotonically to a limit -- the sum will bounce around it, and the summation's state can end up oscillatory forever. Also, this relies on a fixed-precision data type (like float or double). In general, more work should probably be put into deciding when the sum's precision is acceptable and no further iterations need to be calculated. But, this trick is so super-cheap and easy to implement and it is usually so good at detecting the best final result that I include it here, but use it at your peril.

#include <iostream>
#include <iomanip>

int main() {
    float x;
    std::cout << "\nENTER x: ";
    std::cin >> x;

    std::cout << std::setprecision(9);

    const float x_squared = x * x;
    float sum = 0;
    float term = 1;
    for(int i = 2;; i += 2) {
        term *= x_squared / (i * (i - 1));
        const float next_sum = sum + term;
        if(next_sum == sum) break;
        sum = next_sum;
        std::cout << std::setw(2) << i << ": " << sum << "\n";
    }
    // std::cout << ">>  " << sum << "\n";
}

Here are some results.  I changed the code to dump the term's x exponent and the running sum at each iteration, rather than dumping the exponentiated x.

The original version and my version dump the same figures when given an input value of 1.2:

ENTER x: 1.2
 2: 0.720000029
 4: 0.806400061
 6: 0.810547233
 8: 0.810653865
10: 0.810655594

For an input value of 10, Wolfram says we should get about 11012.232920...
But with 10, the original code has problems (I went ahead and set the max iterated power to 30, just to see where things end up).

ENTER x: 10
 2: 50
 4: 466.666656
 6: 1855.55554
 8: 4335.71436
10: 7091.44629
12: 9179.12207
14: 87368.5547
16: 5076917.5
18: -1.1079721e+009
20: -4.86787072e+010
22: -1.91795581e+013
24: -1.30792867e+015
26: -5.52487843e+016
28: -7.32410864e+018
30: 7.02255016e+020

My version (For an input value of 10, it stops at a max x power of 30):

ENTER x: 10
 2: 50
 4: 466.666656
 6: 1855.55542
 8: 4335.71387
10: 7091.44531
12: 9179.12109
14: 10326.1953
16: 10804.1426
18: 10960.335
20: 11001.4385
22: 11010.335
24: 11011.9463
26: 11012.1943
28: 11012.2275
30: 11012.2314

And this is very close to Wolfram's cosh(10)-1, given the precision of a 32-bit float.


Counting sort implementation in C I've just implemented counting sort in C and would like to hear your opinions on it. As I'm usually being rather picky when rating other people's code, please go ahead and be picky, too -- I will try and justify my decisions but I'm thankful for any hints :)

#include <stdlib.h>
#include <stdio.h>
#include <assert.h>

/**
 * parses "[ a , b , c ]" into an array.
 * requirements:
 *  * first character must be a '['
 *  * subsequent characters until closing ']' must be `unsigned integer`,
 *    seperated by ','. Leading and trailing whitespace ignored.
 *
 * @return:
 *  * 0 on success
 *  * 1 if there was found an element > max
 *  * 2 if not len elements were found
 */
int parse_uint_array(size_t len, unsigned int maxval, unsigned int array[len],
        char *input)
{
    /* start at 1 to skip leading '[' */
    unsigned int pos = 1;
    for (size_t i = 0; i < len; i++)
    {
        int ccount;
        int n = sscanf(&input[pos], " %u , %n", &array[i],
                &ccount);
        if (n != 1 && i+1 < len) return 2;
        if (array[i] > maxval) return 1;
        pos += ccount;
    }
    return 0;
}

void print_uint_array(size_t len, unsigned int array[len])
{
    printf("[");
    for (size_t i = 0; i+1 < len; i++) {
        printf("%u, ", array[i]);
    }
    printf("%u]\n", array[len-1]);
}

/**
 * sorts an array of len size with maximum value maxval (can be less) which has
 * to be < UINT_MAX
 *
 * @returns
 *  * 1 if maxval+1 = 0, ie. usually maxval = UINT_MAX.
 *    This limitation is due to how counting sort works
 *  * 2 if malloc fails
 *  * 3 if an element is bigger than specified maxval
 */
int countsort_uint(size_t len, unsigned int maxval, unsigned int array[len])
{
    if (maxval+1 == 0) return 1;

    unsigned int *counts = malloc(sizeof(int)*(maxval+1));
    if (counts == NULL) {
        perror("malloc");
        return 2;
    }

    for (size_t i = 0; i < len; i++) {
        if (array[i] > maxval) return 3;
        counts[array[i]] += 1;
    }

    size_t pos = 0;
    for (size_t i = 0; i < (maxval+1); i++) {
        for (size_t j = pos; j < counts[i]+pos; j++) {
            assert(counts[i]+pos <= len);
            array[j] = i;
        }
        pos += counts[i];
    }
    assert(pos == len);

    free(counts);

    return 0;
}

int main(int argc, char *argv[])
{
    if (argc < 4) {
        fprintf(stderr, "not enough parameters supplied\n");
        return 1;
    }

    size_t len; if (sscanf(argv[1], "%zu", &len) != 1) return 2;
    if (len == 0) {
        fprintf(stderr, "length shouldn't be 0\n");
        return 4;
    }
    unsigned int max; if (sscanf(argv[2], "%u", &max) != 1) return 3;
    if (max+1 == 0) {
        fprintf(stderr, "maximum value < UINT_MAX\n");
        return 5;
    }

    unsigned int *array = malloc(sizeof(int)*len);
    if (array == NULL) {
        perror("malloc");
        return 6;
    }

    {
        int err;
        if ( (err = parse_uint_array(len, max, array, argv[3])) ) {
            fprintf(stderr, "parse_uint_array: returned %d\n", err);

            switch (err) {
            case 1:
                fprintf(stderr, "not all elements were less or equal than specified maximum\n");
                break;
            case 2:
                fprintf(stderr, "malformed array or less elements than specified\n");
                break;
            default:
                fprintf(stderr, "unknown error\n");
                break;
            }

            return 7;
        }
    }

    print_uint_array(len, array);

    {
        int err;
        if ( (err = countsort_uint(len, max, array)) ) {
            fprintf(stderr, "countsort_uint: returned %d\n", err);
            return 8;
        }
    }

    print_uint_array(len, array);

}

Input format

There are a few things I don't like about the way input is handled:

Having all of your input done via the command line is awkward.  Suppose you wanted to try a test case of 10 million integers.  How would you even do that?  I propose that you read your input from stdin instead.  That way you could just create a test input file and run your program like countsort < input.txt to test it.
I don't feel like the user should have to specify the maximum value as the second input value.  Your program could just compute the maximum value as it reads in all the numbers.  This would also allow you to remove some of the error checking you have where you test the array values against the maximum value.
I don't think there is a need for the [ and ] parts of the input, and also the comma separators.  It would be simpler to just have the input be: the count n followed by n numbers without any extra brackets or commas.

Overflow in size computation

It looks like you took care of the special case where the maximum value was UINT_MAX, because in your call to malloc() you used the value max+1 and didn't want that to overflow to zero.  However, there are many other ways to overflow in that same line:

unsigned int *counts = malloc(sizeof(int)*(maxval+1));

Suppose you were on a 32-bit platform with size_t being 32-bits wide, and suppose maxval were 0x40000000.  The expression sizeof(int) * (maxval+1) would evaluate to 4 * 0x40000001 or just 4 because the result would be truncated to 32 bits.  Note that 0x40000000 is just one example.  Any value greater than or equal to 0x3fffffff would cause problems for this platform.

Bug: using uninitialized memory

You allocate your counts array using malloc(), but you never clear your counts array to 0 before you start to use it.  So you could end up with garbage values for your counts.  I would suggest using calloc() so that your allocated array is cleared to zero automatically.  A side benefit to using calloc() is that it avoids your overflow problem because the arguments are split into a size and a count:

unsigned int *counts = calloc(maxval+1, sizeof(int));

Overflow in counts

If you were on a 64-bit platform with 64-bit size_t and 32-bit unsigned int, you could run into another type of overflow problem.  Since your counts array is of type unsigned int (32-bit), your counts could overflow because your array could have more than 2^32 elements. For example, suppose your array had 5 billion instances of the number 5.  Your program would overflow counts[5] and get the wrong count in the end.


Making Tic-Tac-Toe winning check more efficient My code works perfectly fine and I would just like to clean it up a little. From the research I've done, I cannot find anyone that used a multidimensional list to a game of TTT. I'm just looking for a way to make this code prettier than it is right now.

grid = [['-','-','-'],
       ['-','-','-'],
       ['-','-','-']]
turn = 'x'
stl = True
print("Please use this format: row/col")
rounds = 0

def printGrid(grid):
    for i,j,k in grid:
        b = print(i,j,k)

def place(grid,turn,row,col):
    if grid[row][col] == '-':
        grid[row][col] = turn
        return True

    else:
        print("Please select an untaken postion\n")
        return False

while stl:
    rounds += 1
    print("Round %i!" % rounds)

    try:

        printGrid(grid)
        print("\nIt is %s's turn.\n" % turn)
        inp = input("What position do you want to place your symbol? (row/col 1-3/1-3) ")
        row,col = inp.split('/')
        row = int(row)-1
        col = int(col)-1
        print()

        a = place(grid,turn,row,col)

        if a == True:
            if turn == 'x':
                turn = 'o'
            else:turn = 'x'

    except(IndexError,ValueError):print("\nPlease select a viable position.\n")

    print('----------------------------------\n')

    if ['x','x','x'] in grid:
        print("X Victory! You won in %i rounds!\n" % rounds)
        printGrid(grid)
        stl = False
    elif ['o','o','o'] in grid:
        print("O Victory! You won in %i rounds!\n" % rounds)
        printGrid(grid)
        stl = False

    if grid[0][0] == 'x' and grid[1][0] == 'x' and grid[2][0] == 'x':
        print("X Victory! You won in %i rounds!" % rounds)
        printGrid(grid)
        stl = False
    elif grid[0][1] == 'x' and grid[1][1] == 'x' and grid[2][1] == 'x':
        print("X Victory! You won in %i rounds!" % rounds)
        printGrid(grid)
        stl = False
    elif grid[0][2] == 'x' and grid[1][2] == 'x' and grid[2][2] == 'x':
        print("X Victory! You won in %i rounds!" % rounds)
        printGrid(grid)
        stl = False
    elif grid[0][0] == 'x' and grid[1][1] == 'x' and grid[2][2] == 'x':
        print("X Victory! You won in %i rounds!" % rounds)
        printGrid(grid)
        stl = False
    elif grid[0][2] == 'x' and grid[1][1] == 'x' and grid[2][0] == 'x':
        print("X Victory! You won in %i rounds!" % rounds)
        printGrid(grid)
        stl = False

    elif grid[0][0] == 'o' and grid[1][0] == 'o' and grid[2][0] == 'o':
        print("O Victory! You won in %i rounds!" % rounds)
        printGrid(grid)
        stl = False
    elif grid[0][1] == 'o' and grid[1][1] == 'o' and grid[2][1] == 'o':
        print("O Victory! You won in %i rounds!" % rounds)
        printGrid(grid)
        stl = False
    elif grid[0][2] == 'o' and grid[1][2] == 'o' and grid[2][2] == 'o':
        print("O Victory! You won in %i rounds!" % rounds)
        printGrid(grid)
        stl = False
    elif grid[0][0] == 'o' and grid[1][1] == 'o' and grid[2][2] == 'o':
        print("O Victory! You won in %i rounds!" % rounds)
        printGrid(grid)
        stl = False
    elif grid[0][2] == 'o' and grid[1][1] == 'o' and grid[2][0] == 'o':
        print("O Victory! You won in %i rounds!" % rounds)
        printGrid(grid)
        stl = False

As you can see, the last portion of the code is gross. I would just like to see if there is a better way of dealing with this.

You could store the winning positions as follows:

winning = [[(0, 0), (1, 0), (2, 0)],
           [(0, 1), (1, 1), (2, 1)],
           [(0, 2), (1, 2), (2, 2)],
           ...]

Now you can much more easily loop through the positions and test them, using all:

for positions in winning:
    for symbol in 'xy':
        if all(grid[r][c] == symbol for r, c in positions):
            print("{} Victory! You won in {} rounds!".format(symbol.upper(),
                                                             rounds)
            stl = False
            break

I would wrap all of this in a class, to make life a bit easier:

class TicTacToe:

    BLANK = '-'

    WINNING = [[(0, 0), (1, 0), (2, 0)],
               [(0, 1), (1, 1), (2, 1)],
               [(0, 2), (1, 2), (2, 2)],
               ...]

    def __init__(self):
        self.grid = [[self.BLANK for _ in range(3)] for _ in range(3)]

    def __str__(self):
        return '\n'.join([' '.join(row) for row in self.grid])

    def won(self):
        """Return the winning symbol, or None if not yet won."""
        for positions in self.WINNING:
            for symbol in 'xy':
                if all(self.grid[r][c] == symbol for r, c in positions):
                    return symbol

   def make_move(self, symbol, row, col):
       if self.grid[row][col] != self.BLANK:
           raise ValueError("Position already occupied.")
       self.grid[row][col] = symbol

   ...

In terms of general code review, note that:

You should not have multiple statements on the same line (e.g. else:turn = 'x');
The style guide includes guidance on whitespace (e.g. see the spaces after commas in the code I have posted);
There's no need to assign the result of print - it is None, just print(i,j,k);
You should not have code just running within the script, write a main function and guard a call to it behind if __name__ == '__main__':; and
You should generally have more, smaller blocks of functionality (e.g. you could have a function whose job is to get the user to input a valid row/col selection).


Click handlers for two to ten buttons I have click event handlers for two buttons. The code is working well, but I want to apply the DRY principle. How should I rewrite the following code in the button_click event handler so that I don't have to repeat in all my 10 buttons?

private void CreateWindows(UserControl ux, Form rpt, object sender, ItemClickEventArgs e)
{
    ux.Show();

    rpt.SuspendLayout();
    rpt.ControlBox = false;
    rpt.TopLevel = false;
    rpt.WindowState = FormWindowState.Maximized;
    rpt.MdiParent = this;
    rpt.Show();
    rpt.ResumeLayout();

    dock1.SuspendLayout();
    dock1.Controls.Clear();
    dock1.Controls.Add(ux);
    dock1.ResumeLayout();
}

private BaseDocument FindDocument<T>() where T : Form
{
    foreach (BaseDocument document in dxDocumentManager.View.Documents) {
        if (document.Form is T) {
            return document;
        }
    }
    return null;
}

private void button1_ItemClick(object sender, ItemClickEventArgs e)
{
    dxRpt12MonthsItemPurchaseAnalysisByAmount filter = new formFilter1();

    BaseDocument document = FindDocument<formReport1>();

    if(document != null)
    {
        tabbedView1.Controller.Activate(document);
        if (this.ActiveMdiChild is formReport1)
        {
            filter.Focus();
        }
    }
    else
    {
        CreateWindows(new formFilter1(), new formReport1(), sender, e);
    }
}

private void button2_ItemClick(object sender, ItemClickEventArgs e)
{
    fmSalesInvoiceSearch filter = new formFilter2();

    BaseDocument document = FindDocument<formReport2>();

    if (document != null)
    {
        tabbedView1.Controller.Activate(document);
        if (this.ActiveMdiChild is formReport2)
        {
            filter.Focus();
        }
    }
    else
    {
        CreateWindows(new formFilter2(), new formReport2(), sender, e);
    }
}

Naming:

Like the previous answer stated, your classnames are far from following conventions. The should be PascalCase and not camelCase. Also a name like dxRpt12MonthsItemPurchaseAnalysisByAmount is definitly not meaningful, not for you, not for others who read your code. Try names like FirstReportForm, FirstFilterForm or FilterForm1, ReportForm1 and so on.

Tip: Names of Classes, Structs, and Interfaces

Braces:

Microsoft doesn't have any conventions on the formatting of your braces. This means you can chose between following:

if (isValid) {
    DoSomething();
}

and

if (isValid)
{
    DoSomething();
}

But it's best practice to stick with one and not to mix the two up in your code. (I prefer the second use of braces.)

Logical use of variables:

In both click event handlers you instantiate a new filter. Whether or not document is null, you call CreateWindows with again a new instance of that filter-class. Just re-use the instance you created before. You didn't manipulate it in any way.

Generic method:

First of all, the following code is written out of my head and not tested as I have little information what all your classes are/mean. But here's an attempt to make it generic:

private void HandleButtonClick<T>(Form filterForm) where T : Form, new()
{
    var document = FindDocument<T>();

    if(document == null)
    {
        CreateWindows(filterForm, new T(), sender, e);
    }
    else
    {
        tabbedView1.Controller.Activate(document);
        if (this.ActiveMdiChild is T)
        {
            filterForm.Focus();
        }
    }
}

And the usage of the method:

private void button1_ItemClick(object sender, ItemClickEventArgs e)
{
    HandleButtonClick<ReportForm1>(new FilterForm1());
}

private void button2_ItemClick(object sender, ItemClickEventArgs e)
{
    HandleButtonClick<ReportForm2>(new FilterForm2());
}

Notes:

T is the type of the report form
The new() constraint is set on the method, check tip for more info
I reversed the null check for readability, this might be personal choice

Tip: new Constraint (C# Reference)

I cannot reproduce your code and therefore cannot test the method. Please let me know if you got it to work using this method. Hope this helps!


A class for solving a system of linear equations using Gaussian Elimination class LinearEquationSolver
{
    List<LinearEquation> rows = new List<LinearEquation>();
    decimal[] solution;

    public void AddLinearEquation(decimal result, params decimal[] coefficients)
    {
        rows.Add(new LinearEquation(result, coefficients));
    }

    public IList<decimal> Solve()       //Returns a list of coefficients for the variables in the same order they were entered
    {
        solution = new decimal[rows[0].Coefficients.Count()];

        for (int pivotM = 0; pivotM < rows.Count() - 1; pivotM++)
        {
            int pivotN = rows[pivotM].IndexOfFirstNonZero;

            for (int i = pivotN + 1; i < rows.Count(); i++)
            {
                LinearEquation rowToReduce = rows[i];
                decimal pivotFactor = rowToReduce[pivotN] / -rows[pivotM][pivotN];
                rowToReduce.AddCoefficients(rows[pivotM], pivotFactor);
            }
        }

        while (rows.Any(r => r.Result != 0))
        {
            LinearEquation row = rows.FirstOrDefault(r => r.NonZeroCount == 1);
            if (row == null)
            {
                break;
            }

            int solvedIndex = row.IndexOfFirstNonZero;
            decimal newSolution = row.Result / row[solvedIndex];

            AddToSolution(solvedIndex, newSolution);
        }

        return solution;
    }

    private void AddToSolution(int index, decimal value)
    {
        foreach (LinearEquation row in rows)
        {
            decimal coefficient = row[index];
            row[index] -= coefficient;
            row.Result -= coefficient * value;
        }

        solution[index] = value;
    }

    private class LinearEquation
    {
        public decimal[] Coefficients;
        public decimal Result;

        public LinearEquation(decimal result, params decimal[] coefficients)
        {
            this.Coefficients = coefficients;
            this.Result = result;
        }

        public decimal this[int i]
        {
            get { return Coefficients[i]; }
            set { Coefficients[i] = value; }
        }

        public void AddCoefficients(LinearEquation pivotEquation, decimal factor)
        {
            for (int i = 0; i < this.Coefficients.Count(); i++)
            {
                this[i] += pivotEquation[i] * factor;
                if (Math.Abs(this[i]) < 0.000000001M)    //Because sometimes rounding errors mean it's not quite zero, and it needs to be
                {
                    this[i] = 0;
                }
            }

            this.Result += pivotEquation.Result * factor;
        }

        public int IndexOfFirstNonZero
        {
            get
            {
                for (int i = 0; i < Coefficients.Count(); i++)
                {
                    if (this[i] != 0) return i;
                }
                return -1;
            }
        }

        public int NonZeroCount
        {
            get
            {
                int count = 0;
                for (int i = 0; i < Coefficients.Count(); i++)
                {
                    if (this[i] != 0) count++;
                }
                return count;
            }
        }
    }
}

Are there any edge-cases I've missed? Is there a better way to handle potential rounding errors than just checking if a value is close to zero and zeroing it if it is? Is it overkill to use Decimals like this?

Here's a test program:

class Program
{
    static void Main(string[] args)
    {
        LinearEquationSolver test = new LinearEquationSolver();

        test.AddLinearEquation(5, 1, 2, 1, -1);
        test.AddLinearEquation(16, 3, 2, 4, 4);
        test.AddLinearEquation(22, 4, 4, 3, 4);
        test.AddLinearEquation(15, 2, 0, 1, 5);
        var result = test.Solve();

        foreach (var asdf in result)
        {
            Console.Write(asdf);
        }

        Console.ReadKey();
    }
}

Enumerable.Count() vs count access

One problem that slows your program's performance is that you call

Coefficients.Count()

In a few places while you are using array instead of an IEnumerable<T> where this is the only way to count the elements and as you can see this is a method rather than being a property. Instead you should simply call Coefficients.Length.

Edit

As @t3chb0t pointed in the comments and discussed here C# will simply return the Count property of the collection even when you call the method .Count() which means Coefficients.Count() & Coefficients.Length are almost equivalent in performance since the method has to do a type checking.

Nevertheless when possible, you should always prefer using your collection's specific functions and properties, rather than the generic ones. In your case Coefficients.Length.

Properties vs fields

Ideally you should always use properties instead of fields

public decimal[] Coefficients;
public decimal Result;

Can become

public decimal[] Coefficients { get; }
public decimal Result { get; set; }

You properties should be as restrictive as possible the less your class exposes the better.

Indexer with public backing collection ?

This doesn't makes much sense. Usually when a class has a indexer it's used internally with a private backing collection, but you have an iterator + a public collection.

In your case I don't see any reason why would you use an indexer here the public array Coefficients is more than enough and it's a lot more readable, so you should remove that indexer and just access the array like this :

public void AddCoefficients(LinearEquation pivotEquation, decimal factor)
{
    for (int i = 0; i < this.Coefficients.Length; i++)
    {
        Coefficients[i] += pivotEquation.Coefficients[i]*factor;
        if (Math.Abs(Coefficients[i]) < 0.000000001M)
            //Because sometimes rounding errors mean it's not quite zero, and it needs to be
        {
            Coefficients[i] = 0;
        }
    }
    this.Result += pivotEquation.Result*factor;
}

Redundant this qualifier

While I do agree that this is up to a personal preference I don't like it. You don't need a single this qualifier in your code it would be better to remove it it's just extra text to read..

Expression bodied properties + LINQ

public int IndexOfFirstNonZero
{
    get
    {
        for (int i = 0; i < Coefficients.Length; i++)
        {
            if (Coefficients[i] != 0)
            {
                return i;
            }
        }
        return -1;
    }
}

public int NonZeroCount
{
    get
    {
        int count = 0;
        for (int i = 0; i < Coefficients.Length; i++)
        {
            if (Coefficients[i] != 0) count++;
        }
        return count;
    }
}

You can really shorten those get only properties using LINQ :

public int IndexOfFirstNonZero
{
    get
    {
        int value = (int) Coefficients.FirstOrDefault(x => x != 0);
        return value == default(int) ? -1 : value;
    }
}

public int NonZeroCount => Coefficients.Count(t => t != 0);


Threads listening on TCP and rendering in a loop I have two threads, where one listens on TCP and the other renders in a loop:

  private void checkBox1_CheckedChanged(object sender, EventArgs e)
        {
            try
            {
                if (ReceiveCheck.Checked)
                {
                    tcplisten.Start();
                    ListenThread = new Thread(new ThreadStart(Listen));
                    ListenThread.Start();
                    RenderThread = new Thread(new ThreadStart(Render));
                    RenderThread.Start();

                }
                else
                {
                    tcplisten.Stop();
                    RenderThread.Abort();
                    ListenThread.Abort();
                }
            }
            catch (Exception ex)
            {
                MessageBox.Show(ex.Message, "Checkbox");
            }

        }

Is this a good way of handling the threads, to just start them and later kill them when I want to?

No, this is absolutely not a good way. Thread.Abort() should never be used, because it is very hard to write correct code when an exception can happen at almost any point in your code.

Instead, you should implement cooperative cancellation either by using a volatile bool flag, or, even better, CancellationToken.

With that your code could look like this:

Thread ListenThread;
Thread RenderThread;
CancellationTokenSource CTS;

private void checkBox1_CheckedChanged(object sender, EventArgs e)
{
    try
    {
        if (ReceiveCheck.Checked)
        {
            tcplisten.Start();
            CTS = new CancellationTokenSource();
            ListenThread = new Thread(() => Listen(CTS.Token)));
            ListenThread.Start();
            RenderThread = new Thread(() => Render(CTS.Token)));
            RenderThread.Start();
        }
        else
        {
            tcplisten.Stop();
            CTS.Cancel();
        }
    }
    catch (Exception ex)
    {
        MessageBox.Show(ex.Message, "Checkbox");
    }
}

Your Listen() and Render() methods would then periodically check IsCancellationRequested of the passed in token and return if it's true.


Putting 3 different on click functions into one reusable function I have image slider with thumbnails, simple one where I account for left and right arrow clicks that show respective images and thumbnail clicks that load related image. Functions have a lot in common and I'm trying to figure out how to put them into one and use it for all 3 different events, but am having trouble as thumbnail one is slightly different.

//Handle next-image click on product single page
    $('.images').on('click', '#next-image', function(e) {
        e.preventDefault();
        var current = $('.thumbnail-link.first');

        //Check if thumbnail is last, if it is make first thumbnail next
        if(current.next().hasClass('thumbnail-link')) {
            next = current.next('.thumbnail-link');
        } else {
            next = $('.thumbnail-link').first();
        }

        //Grab image src from data-image attribute
        var image = next.data('image');

        $('.woocommerce-main-image')
        .find('img')
        .animate({
            opacity: 0
        }, 300, function() {
            $(this).attr('src', image);
            $(this).animate({opacity: 1}, 300);

        })
        current.removeClass('first');
        next.addClass('first');
    });

    //Handle prev-image click on product single page
    $('.images').on('click', '#prev-image', function(e) {
        e.preventDefault();
        var current = $('.thumbnails .first');

        //Check if thumbnail is last, if it is make first thumbnail next
        if(current.prev().hasClass('thumbnail-link')) {
            prev = current.prev('.thumbnail-link');
        } else {
            prev = $('.thumbnail-link').last();
        }

        //Grab image src from data-image attribute
        var image = prev.data('image');

        $('.woocommerce-main-image')
        .find('img')
        .animate({
            opacity: 0
        }, 300, function() {
            $(this).attr('src', image);
            $(this).animate({opacity: 1}, 300);
        })
        current.removeClass('first');
        prev.addClass('first');
    });

    //Handle click on thumbnail
    $('.thumbnails').on('click', '.thumbnail-link', function(e) {
        e.preventDefault();
        var current = $('.thumbnail-link.first'),
            image = $(this).data('image');

        $('.woocommerce-main-image')
        .find('img')
        .animate({
            opacity: 0
        }, 300, function() {
            $(this).attr('src', image);
            $(this).animate({opacity: 1}, 300);
        })

        current.removeClass('first');
        $(this).addClass('first');
    });

A lot of present code can be made reusable.

I have made the necessary changes and created a single reusable function handleEvent

// Handle next-image click on product single page
var handleEvent = function(e, currentClass, linkClass, method) {
    e.preventDefault();

    var current = $('.' + currentClass + '.first');
    var next;

    // handles click on thumbnail
    if (typeof method == "undefined") {
        next = $(this);
    } else {
        // Handles next-image and prev-image click on product single page
        // Check if thumbnail is last, if it is make first thumbnail next
        if (current.next().hasClass(linkClass)) {
            next = current.next('.' + linkClass);
        } else {
            next = $('.' + linkClass)[method]();
        }
    }

    // Grab image src from data-image attribute
    var image = next.data('image');

    $('.woocommerce-main-image')
        .find('img')
        .animate({
            opacity: 0
        }, 300, function() {
            $(this).attr('src', image);
            $(this).animate({
                opacity: 1
            }, 300);
        });

    current.removeClass('first');
    next.addClass('first');
};

Example of calling this reusable function from three different events is as follows

// Handle next-image click on product single page
$('.images').on('click', '#next-image', function(e) {
    handleEvent(e, 'thumbnail-link', 'thumbnail-link', 'first');
});

// Handle prev-image click on product single page
$('.images').on('click', '#prev-image', function(e) {
    handleEvent(e, 'thumbnails', 'thumbnail-link', 'last');
});

// Handle click on thumbnail
$('.thumbnails').on('click', '.thumbnail-link', function(e) {
    handleEvent(e, 'thumbnail-link');
});


Simple object pool on Android Assume the server is sending data to our app at regular intervals, at least once per second. The received data is parsed and stored in memory as a POJO. A blocking queue is processing data in a worker thread.

Suppose I want to use a very simple object pool, so I don’t have to create a new POJO for each server response. I don’t want to use e.g. Apache Commons Pool.

Would the following implementation suffice?

public abstract class ObjectPool<T> {

    private final Stack<T> mPool;

    public ObjectPool() {
        mPool = new Stack<T>();
    }

    public T obtain() {
        if (mPool.isEmpty()) {
            return create();
        } else {
            return mPool.pop();
        }
    }

    public void recycle(T object) {
        if (null != object) {
            destroy(object);
            mPool.push(object);
        }
    }

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

    public int getSize() {
        return mPool.size();
    }

    protected abstract T create();

    protected abstract void destroy(T object);

}

Can I shoot myself in the foot using this? Any caveats I’m not seeing?

Thanks!

I have to question whether this makes any sense in your application:

Assume the server is sending data to our app at regular intervals, at least once per second

Have you done any testing/metrics that indicate that the current system you have is actually a problem? I would be very surprised if it is.
If the server is sending data as infrequently as that, there does not seem to be any reason to 'pool' the objects at all. The overhead of 'cleaning' an instance, storing it, recycling it, repopulating it, and repeating with it is probably just as overwhelming as simply creating a new instance and GC'ing the old one.
Unless you can already identify 'specifically' that object creation is a significant part of an existing performance problem, I would suggest that you are looking in the wrong places for performance improvements.
EDIT - hypothetically...
Answering hypothetically: no, I don't believe your solution is enough...

by the time you get to the frequency where object creation/reuse is a problem, you will be needing multiple concurrent threads to do the work anyway, and your solution is not synchronized (by the way, new and GarbageCollection are thread-safe)
You have created an artificial latency in your program flow - see Ahmdahl's law .. by adding to the critical path instead of taking work off the path (parallelizing things). As your system gets busier, you are making the problem worse and not better.

Bottom line, is that, at low volumes your current solution is not going to make a difference, and that at high frequencies, your solution is only going to make things worse.
One other thing to consider, and Android performance is not my strength, but on 'full' computers (I often work with systems with more than 128 cores in highly-threaded systems), the cost of having to bring the stack in to your system cache, and then replace it with the cache of your object is probably going to be more than just creating a new object would... Again, the more busy your system, the more this sort of issue 'hurts'.


LeetCode: Transpose Matrix C# Please review performance
https://leetcode.com/problems/transpose-matrix/

Given a matrix A, return the transpose of A.
The transpose of a matrix is the matrix flipped over it's main
diagonal, switching the row and column indices of the matrix.
Example 1:
Input: [[1,2,3],[4,5,6],[7,8,9]] Output: [[1,4,7],[2,5,8],[3,6,9]]
Example 2:
Input: [[1,2,3],[4,5,6]] Output: [[1,4],[2,5],[3,6]]
Note:
1 <= A.length <= 1000 1 <= A[0].length <= 1000

using System;
using System.Linq;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace ArrayQuestions
{
    [TestClass]
    public class TransposeMatrix
    {

        /// <summary>
        /// 1 2 3       1 4 7
        /// 4 5 6       2 5 8
        /// 7 8 9       3 6 9
        /// </summary>
        [TestMethod]
        public void Transpose3X3()
        {
            int[][] matrix = { new[] { 1, 2, 3 }, new[] { 4, 5, 6 }, new[] { 7, 8, 9 } };
            int[][] expected = { new[] { 1, 4, 7 }, new[] { 2, 5, 8 }, new[] { 3, 6, 9 } };
            int[][] output = Transpose(matrix);
            for (int i = 0; i < output.GetLength(0); i++)
            {
                CollectionAssert.AreEqual(expected[i], output[i]);
            }
        }

        [TestMethod]
        public void Transpose2X3()
        {
            int[][] matrix = { new[] { 1, 2, 3 }, new[] { 4, 5, 6 } };
            int[][] expected = { new[] { 1, 4 }, new[] { 2, 5 }, new[] { 3, 6} };
            int[][] output = Transpose(matrix);
            for (int i = 0; i < output.GetLength(0); i++)
            {
                CollectionAssert.AreEqual(expected[i], output[i]);
            }
        }
        public int[][] Transpose(int[][] A)
        {
            int h = A.GetLength(0);
            int w = A[0].GetLength(0);

            //we need to allocate the matrix as transposed already
            //for example 2x3 should allocate 3x2
            int[][] res = new int[w][];
            for (int i = 0; i < w; i++)
            {
                res[i] = new int[h];
            }

            for (int r = 0; r < h; r++)
            {
                for (int c = 0; c < w; c++)
                {
                    res[c][r] = A[r][c];
                }
            }
            return res;
        }
    }
}

You iterate through the same array twice (once while initializing and once while assigning values), you can do both inside the same loop.

for (int i = 0; i < w; i++)
{
    res[i] = new int[h];
    for (int j = 0; j < h; j++)
    {
        res[i][j] = A[j][i];
    }
}


User-defined class for a box We have just learned about classes. Please help me improve this program. I created a class called Box with members that represent its dimensions.  Then I calculate the volume and surface area and print it.

#include <iostream>

using std::cout;
using std::endl;

class Box
{
public:
    Box(){};
    double m_Length;
    double m_Width;
    double m_Height;
private:
};

int main()
{
    Box box1;
    Box box2;

    double boxVolume(0.0);
    double surfaceArea(0.0);

    box1.m_Height = 18.0;
    box1.m_Length = 78.0;
    box1.m_Width = 24.0;

    boxVolume = box1.m_Height * box1.m_Length * box1.m_Width;
    surfaceArea = 2.0 * ((box1.m_Height * box1.m_Width) + (box1.m_Length * box1.m_Height) + (box1.m_Length * box1.m_Width));

    cout << "Volume of box1 = " << boxVolume << endl;
    cout << "Surface Area of box1 = " << surfaceArea << endl;
}

The Box class has all public members and doesn't have any behavior (no methods). As such, it's a missed opportunity and could have been a simple struct.

Even worse, what's the point of a Box if any of length or width or height is undefined? These should all be required to construct a meaningful representation of a box.

The calculation of the surface and volume are inside the main method. This is not convenient, because you would have to re-type the formula every time you want to calculate this for every single box.

To remedy the above:

make the length, width, height variables private: code outside of the class should not modify them
make the length, width, height required at construction time: a box without these parameters doesn't make sense, and is not good modelization
I would go as far as making these variables constant: in real life, the width / height / length of a physical box is usually not editable
add surface and volume calculation logic as methods on the box, so that they can be reused for any Box object

A better interface for a Box would be:

class Box
{
public:
    Box(double length, double width, double height);
    inline double volume();
    inline double surface();
private:
    const double m_Length;
    const double m_Width;
    const double m_Height;
};

With implementation:

Box::Box(double length, double width, double height) :
    m_Length(length), m_Width(width), m_Height(height) {}

double Box::volume()
{
    return m_Length * m_Width * m_Height;
}

double Box::surface()
{
    return 2 * (m_Length * m_Width + m_Width * m_Height + m_Height * m_Length);
}

With the above, the main method can be rewritten as:

int main()
{
    Box box1(78, 24, 18);

    cout << "Volume of box1 = " << box1.volume() << endl;
    cout << "Surface Area of box1 = " << box1.surface() << endl;
}

Note that since the implementation of the constructor and the surface and volume methods are so simple,
it might be tempting to write them inside the declaration of the class, like this:

class Box
{
public:
    Box(double length, double width, double height) : m_Length(length), m_Width(width), m_Height(height) {}
    double volume() { return m_Length * m_Width * m_Height; }
    double surface() { return 2 * (m_Length * m_Width + m_Width * m_Height + m_Height * m_Length); }
private:
    double m_Length;
    double m_Width;
    double m_Height;
};

But this is a not a good practice.
The good practice is to cleanly separate the interface declaration from the implementation.
However,
while you're learning,
if this last version is easier for you,
then it's ok, you can do it this way, for now.
Just remember later as your understanding evolves,
to try to separate interface from implementation.

Finally, some other minor issues:

No need to write 18.0 when assigning to a double variable,
you could write simply 18.

No need to put parentheses around multiplication in (a * b) + (c * d),
a * b + c * d is equivalent.


Bash script that creates a local environment for CHICKEN scheme (like virtualenv for Python) Usage

To create a localenv called sicp, do $ localenv sicp
Activate the localenv: $ . sicp/bin/activate
Check installed eggs in the local repository: $ chicken-status
Install a new egg called 'sicp' in the local repository: $ chicken-install sicp
Deactivate the localenv: $ deactivate

This is the first time I have used bash for any purpose at all. Is there any other way to get the grandparent directory of the current directory?

#!/bin/bash

if [ -z "${1}" ]; then
    echo 'Usage: localenv DIR'
    echo
    echo 'localenv: You must provide the name of the directory to install the chicken environment.'
    exit 1
elif [ -e "$(pwd)/${1}" ]; then
    echo 'localenv: file or directory already exists.'
    exit 1
fi

# check if chicken is installed and save the location of csi and chicken-install.
CSI=$(command -v csi)

if [ -z "${CSI}" ]; then
    echo 'CHICKEN is not installed!'
    exit 1
else
    echo "CHICKEN interpreter is at ${CSI}"
fi

CHICKEN_INSTALL=$(command -v chicken-install)

# obtain global CHICKEN_HOME, CHICKEN_PREFIX, CHICKEN_REPOSITORY paths
CHICKEN_HOME=$(${CSI} -p "(chicken-home)")
CHICKEN_PREFIX="$(dirname $(dirname ${CHICKEN_HOME}))"
CHICKEN_REPOSITORY=$(${CSI} -p "(repository-path)")
# chicken binary version
BINARY_VERSION=$(basename "${CHICKEN_REPOSITORY}")

# local copy of chicken files are kept here
LOCAL_CHICKEN_PREFIX=$(realpath "${1}")

# remember where we were before jumping directories
OLDPWD=$(pwd)

# create all the necessary directories
mkdir -p "${LOCAL_CHICKEN_PREFIX}" && cd "$_" || exit 1
mkdir -p bin lib/chicken share/chicken include/chicken

# populate ./bin
for file in ${CHICKEN_PREFIX}/bin/{chicken,chicken-bug,chicken-install,chicken-profile,chicken-status,chicken-uninstall,csi,csc,feathers}; do
    [ -f "${file}" ] && cp "${file}" ./bin
done

# initialize empty alternative repository
${CHICKEN_INSTALL} -init "./lib/chicken/${BINARY_VERSION}"

# include files are essential
cp -R "${CHICKEN_PREFIX}"/include/chicken/{chicken.h,chicken-config.h} ./include/chicken

# populate ./share
cp -R "${CHICKEN_HOME}/setup.defaults" ./share/chicken

# create init file for the localenv
# init file acts like .csirc for the localenv
touch "${LOCAL_CHICKEN_PREFIX}/.init"

# write activate file
# Backslash escapes '$' and prevents expansion of the shell variables
cat << EOF > "${LOCAL_CHICKEN_PREFIX}/bin/activate"
export OLDPATH="\${PATH}"
export PATH="${LOCAL_CHICKEN_PREFIX}/bin:\${PATH}"

export OLDPS1="\${PS1}"
export PS1="($1)\${PS1}"

export CHICKEN_PREFIX="${LOCAL_CHICKEN_PREFIX}"
export CHICKEN_HOME="${LOCAL_CHICKEN_PREFIX}/share/chicken"
export CHICKEN_REPOSITORY="${LOCAL_CHICKEN_PREFIX}/lib/chicken/${BINARY_VERSION}"
export LOCAL_CSIRC="${LOCAL_CHICKEN_PREFIX}/.init"

# register a feature identifier 'localenv'
# this is required to override commands in the .csirc file
alias csi="csi -D localenv"

# deactivation function
deactivate()
{
    export PATH="\${OLDPATH}"
    export PS1="\${OLDPS1}"
    unset OLDPATH
    unset OLDPS1
    unset CHICKEN_PREFIX
    unset CHICKEN_HOME
    unset CHICKEN_REPOSITORY
    unset LOCAL_CSIRC
    unalias csi
    unset -f deactivate
}

EOF

cd "${OLDPWD}"

I don't understand why the $(pwd)/ prefix is needed in this condition:

elif [ -e "$(pwd)/${1}" ]; then

If ${1} is an absolute path, the condition may not match the directory, even if it exists. Why not write simply [ -e "${1}" ], which should work well with both relative and absolute paths.

OLDPWD is automatically set by Bash when you change directories,
no need to set it manually.
In any case, since the script is intended to be executed instead of sourced,
there's no need to cd back to $OLDPWD.

As for $(dirname $(dirname ${CHICKEN_HOME})), I think that's nicely readable. For the record, you could use ${CHICKEN_HOME%/*} twice to strip suffixes until the grandparent (+ an extra check for the special case of /). Since parameter expansion is a built-in, this would be faster than executing dirname twice. The performance would make a difference if you called dirname a thousand times in a loop, but that's not the case here. And since using dirname is simpler and more readable, I would stick with that.


Building a Map of Set using Java Streams I wrote a collector that would take a stream of Map.Entry<Integer, MyObject> which I want to make into Map<Integer, Set<MyObject>>

import java.util.*;
import java.util.concurrent.*;
import java.util.stream.*;
class MyObject {

}
class Main {
  public static void main(String[] args) {
    Map<Integer, Set<MyObject>> collected = Stream.of(
       Map.entry(123, new MyObject()),
       Map.entry(123, new MyObject()),
       Map.entry(234, new MyObject()),
       Map.entry(345, new MyObject())
    )
    .collect(
      ConcurrentHashMap<Integer, Set<MyObject>>::new,
      (map, entry) -> {
        map.computeIfAbsent(
          entry.getKey(),
          k -> new HashSet<MyObject>()
        )
        .add(entry.getValue());
      },
      (map1, map2) -> {
        map2.forEach((map2Key, map2Object) -> {
          final Set<MyObject> objects =
            map1.computeIfAbsent(map2Key, k -> new HashSet<MyObject>());
          objects.addAll(map2Object);
        });
      }
    );
    System.out.println(collected);
  }
}

I was wondering if there's a built in way of doing this, as it seems to be a common thing to do.

One way I can refactor this is to move the collector out to a different class like so:

class EntryToMapCollector<K, V> implements Collector<Map.Entry<K, V>, Map<K, Collection<V>>, Map<K, Collection<V>>> {
    @Override
    public Supplier<Map<K, Collection<V>>> supplier() {
        return ConcurrentHashMap<K, Collection<V>>::new;
    }

    @Override
    public BiConsumer<Map<K, Collection<V>>, Map.Entry<K, V>> accumulator() {
        return (map, entry) -> {
            map.computeIfAbsent(entry.getKey(), k -> new HashSet<V>())
                .add(entry.getValue());
        };
    }

    @Override
    public BinaryOperator<Map<K, Collection<V>>> combiner() {
        return (map1, map2) -> {
            final Map<K, Collection<V>> combined = new ConcurrentHashMap<>(map1);
            map2.forEach((map2Key, map2Observations) -> {
                combined.computeIfAbsent(map2Key, k -> new HashSet<V>())
                    .addAll(map2Observations);
            });
            return combined;
        };
    }

    @Override
    public Function<Map<K, Collection<V>>, Map<K, Collection<V>>> finisher() {
        return m -> m;
    }

    @Override
    public Set<Characteristics> characteristics() {
        return Set.of(Characteristics.CONCURRENT, Characteristics.UNORDERED);
    }
}

and replace the call with .collect(new EntryToMapCollector<Integer, MyObject>());

https://repl.it/@trajano/ForkedSpryRobot

Have you looked at .groupingBy() and .toSet()?

Map<Integer, Set<MyObject>> collected = Stream.of(...)
    .collect(Collectors.groupingBy(Map.Entry::getKey,
                                   Collectors.mapping(Map.Entry::getValue, Collectors.toSet())));

You possibly want .groupingByConcurrent() instead, though it is not exactly clear why.

Not a repl.it, but a JShell log:

U:\>jshell
|  Welcome to JShell -- Version 13
|  For an introduction type: /help intro

jshell> class MyObject { }
|  created class MyObject

jshell> Map<Integer, Set<MyObject>> result = Stream.of(
   ...>    Map.entry(123, new MyObject()),
   ...>    Map.entry(123, new MyObject()),
   ...>    Map.entry(234, new MyObject())
   ...> ).collect(
   ...>    Collectors.groupingByConcurrent(
   ...>       Map.Entry::getKey,
   ...>       Collectors.mapping(
   ...>          Map.Entry::getValue,
   ...>          Collectors.toSet()
   ...>       )
   ...>    )
   ...> );
result ==> {234=[MyObject@78c03f1f], 123=[MyObject@5ce81285, MyObject@52a86356]}

jshell>


Simple administrator authentication page in PHP I made a simple administrator page lock in PHP and I was wondering how secure it was. I want to use this script to secure administrator pages for my portfolio site but I want it to be fairly secure.

<?php

/*
 * Array of authenticated users
 */

$users = [
  // Username => Password
  'admin' => 'password'
];

/*
 * Expiration time in seconds
 */

$expire = 30 * 60;

/*
 * We take care of the rest
 */

// Start the session
session_start();

// Last session activity time is greater than the set expiration time
if (isset($_SESSION['LAST_ACTIVITY']) && (time() - $_SESSION['LAST_ACTIVITY'] > $expire)) {
  // Free all session variables
  session_unset();
  // Destroy all session variables
  session_destroy();
}

// Set last session activity time
$_SESSION['LAST_ACTIVITY'] = time();

// On form submission
if (isset($_POST['username'], $_POST['password'])) {
  // Reference username and password
  $username = $_POST['username'];
  $password = $_POST['password'];
  // Provided username exists
  if (array_key_exists($username, $users)) {
    // Provided password is verified
    if ($password == $users[$username]) {
      // Set the session user identifier
      $_SESSION['USER_ID'] = $username;
    }
  }
}

// User identifier is not present or is valid
if (!isset($_SESSION['USER_ID']) || !array_key_exists($_SESSION['USER_ID'], $users)) {
  // Render the form for authentication attempts
  echo '<form method="post">
<input type="text" name="username" placeholder="Username">

<input type="password" name="password" placeholder="Password">

<input type="submit">
</form>';
  // Don't allow any action until authentication is finished
  exit();
}

?>

I plan on adding things like halting the script for a few seconds depending on how many attempts were made from the current session.

Any suggestions or insights on if this is secure or how to make it more secure would be very helpful.

You should not store your password in plaintext, as this means that read access (eg via LFI) automatically leads to more (probably some sort of write access via the admin backend). Use bcrypt for this (this also solves the next point automatically).

Ideally, you should also use some sort of timing safe string comparison.

Ideally, I would move the user credentials into an extra config file, which can be placed outside the web root (in case the connection breaks up while a user edits the file, which may lead to a backup file like login.php~ or similar being created, which may be severed by a webserver).

  I plan on adding things like halting the script for a few seconds depending on how many attempts were made from the current session.

I guess that is one solution, but I would just deny attempts completely for x seconds if y attempts where made from that IP in the last z seconds.

Misc

You are overdoing it a bit with comments, which leads people to ignore your comments. Comments should not rephrase what the code already told us, but add additional information in case the code is unclear (Username => Password and possibly Expiration time in seconds/Array of authenticated users are the only comments I would actually keep).


Tic-Tac-Toe in Java with GUI I've wrote a little program to play Tic-Tac-Toe:

public class Control {
    public static void main(String args[]) {
        Gui gui = new Gui();

    }
}

The following classed is used for applying the rules of tic-tac-toe:

import javax.swing.*;

public class TicTacToe {
    public static int count = 0;
    public static String[][] board = new String[3][3];
    public boolean test = true;

    public void buttonClicked(JButton button) {
        if(test) {
            if(button.getText().equals("")) {
                count++;
                if(count % 2 == 1) {
                    button.setText("X");
                }
                if(count % 2 == 0) {
                    button.setText("O");
                }
            }
        }

    }

    public void gameRules(JButton button) {
        if(test) {

            //"X" or "O"?
            String string = button.getText();

            //Gives coordinates of the button
            int x = Character.getNumericValue(button.getName().charAt(0));
            int y = Character.getNumericValue(button.getName().charAt(1));
            board[x][y] = string;

            if(board[0][0] != null && board[0][0].equals(board[1][1]) && board[1][1].equals(board[2][2])) {
                JOptionPane.showMessageDialog(null,string + " won.");
                test = false;
            }

            else if(board[0][2] != null && board[0][2].equals(board[1][1]) && board[1][1].equals(board[2][0])) {
                JOptionPane.showMessageDialog(null,string + " won.");
                test = false;
            }
            else if(count == 9) {
                JOptionPane.showMessageDialog(null, "draw.");
                test = false;
            }
            else {
                for (int i = 0; i < 3; i++) {
                    if (board[i][0] != null && board[i][0].equals(board[i][1]) && board[i][0].equals(board[i][2])) {
                        JOptionPane.showMessageDialog(null, string + " won.");
                        test = false;
                        break;
                    }
                    if (board[0][i] != null && board[0][i].equals(board[1][i]) && board[0][i].equals(board[2][i])) {
                        JOptionPane.showMessageDialog(null, string + " won.");
                        test = false;
                        break;
                    }
                }
            }
        }
    }
}

And finally the class for the GUI:

import javax.swing.*;
import java.awt.*;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;

public class Gui {
    public Gui() {
        JFrame frame = new JFrame();
        JPanel panel = new JPanel();
        TicTacToe ticTacToe = new TicTacToe();
        panel.setLayout(new java.awt.GridLayout(3, 3));
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

        for (int i = 0; i < 3; i++){
            for(int j = 0; j < 3; j++) {
                final JButton button = new JButton();
                String string = i +  "" + j;
                button.setText("");
                button.setName(string);
                button.addActionListener(
                        new ActionListener() {
                            public void actionPerformed(ActionEvent e) {
                                ticTacToe.buttonClicked(button);
                                ticTacToe.gameRules(button);
                            }
                        });
                button.setFont(new Font("Arial", Font.PLAIN, 40));
                button.setBorder(BorderFactory.createLineBorder(Color.BLACK));
                panel.add(button);
            }

        }

        frame.add(panel);
        frame.setSize(400,400);
        frame.setVisible(true);

    }
}

I would appreciate any suggestions to improve the code.

Event Dispatching Thread

Since Swing is not thread safe, all Swing UI component creation and modification should be done on the Event Dispatching Thread (EDT).  It isn’t a problem with this program, but you could run into it if you also created (say) a javax.swing.Timer in your initiation.

It is an easy change to create the GUI on the EDT.  Instead of Gui gui = new Gui(); use:

SwingUtilities.invokeLater(Gui::new);

But I Won???

Play a game, with these moves:

 O | X | O
---+---+---
 X |   | X
---+---+---
 O | X | O

And then put the final X in the center.  You win in two directions at once, but because count == 9, the game is a draw?

Separate GUI / Logic

It looks like you tried to separate the GUI from the game logic, but:

TicTacToe still accesses/manipulates JButton objects
TicTacToe shows JOptionPane dialogs

So, it is not separate from the Swing GUI; you could not reuse it in an SWT or JavaFX application.

The TicTacToe class should have functions something like:

bool isValidMove(int x, int y)
void makeMove(int x, int y)
bool isGameOver()
Player getWinner()

and the GUI should convert buttons into x, y locations, display messages, etc

static

Can you play a second game?  No.  There is no way to reset count for a new game.  It would continue to count above 9 with additional moves!

Can you play two games at once?  No!  There is only one board object.  Two simultaneous games would corrupt each other’s board and count variables!

Why are these variables static?  It prevents multiple TicTacToe objects from being created.  If you removed static from the variables, you could start a new game by creating a new TicTacToe object (and resetting the GUI).

Advanced: Also, you could allow the computer to experiment with different moves, and look several moves in the future, ... but only if you could create these extra TicTacToe boards which aren’t the ones being displayed in the UI.

Simplify Logic

            if(count % 2 == 1) {
                button.setText("X");
            }
            if(count % 2 == 0) {
                button.setText("O");
            }

If count % 2 is not 1, it must be 0.  Use an else clause:

            if(count % 2 == 1) {
                button.setText("X");
            } else {
                button.setText("O");
            }

This test is very verbose:

        if(board[0][0] != null && board[0][0].equals(board[1][1]) && board[1][1].equals(board[2][2])) {

Who can win on any turn?  The player that just made a move, of course.  And their symbol is stored in string (a terrible variable name, by the way).

So if the above test was to pass, all the symbols would equal string, so you could write it a little more concisely as:

        if(string.equals(board[0][0]) && string.equals(board[1][1]) && string.equals(board[2][2]) {

If you defined constants (or better, used enum if you are familiar with them):

    public final static String PLAYER_X = "X";
    public final static String PLAYER_O = "O";

and explicitly set the contents of board[][] to either one of those objects, then you could safely test for object identity, instead of equality:

        if(board[0][0] == player && board[1][1] == player && board[2][2] == player) {


PMX crossover implementation in Clojure I'm starting to learn Clojure and as an exercise I've implemented the following functions (written in Java) in Clojure:

private int[] SwapValues(int val1, int val2, int[] array) {
    int[] result = new int[array.length];
    for (int i = 0; i < array.length; i++) {
        if (array[i] == val1) {
            result[i] = val2;
        } else if(array[i] == val2) {
            result[i] = val1;
        } else {
            result[i] = array[i];
        }
    }

    return result;
}

public int[][] PartialCross(int[] subject1, int[] subject2, final int begin, final int end) {
    int[][] result = new int[2][subject1.length];
    int[] child1 = subject1.clone();
    int[] child2 = subject2.clone();

    for (int i = begin; i < end; i++) {
        child1 = SwapValues(subject1[i], subject2[i], child1);
        child2 = SwapValues(subject1[i], subject2[i], child2);
    }

    result[0] = child1;
    result[1] = child2;

    return result;
}

My result is:

(def chromosone1 [5 8 1 7 6 4 9 3 2])
(def chromosone2 [5 3 2 6 1 8 7 4 9])

(defn extract [input i j]
  (filter #(not (nil? %1))
          (map-indexed #(if (and (>= %1 i) (< %1 j)) %2) input)))

(defn chrossover-swap [input i]
  (let [i1 (first i)
        i2 (last i)]
    (map #(cond
           (= %1 i1) i2
           (= %1 i2) i1
           :else %1) input)))

(defn crossover [input1 input2]
  (let [i1 (extract input1 3 5)
        i2 (extract input2 3 5)
        i3 (partition 2 (interleave i1 i2))]
    (loop [to-swap i3
           o1 input1
           o2 input2]
      (if (zero? (count to-swap))
        [o1 o2]
        (do
          (def i (first to-swap))
          (recur (rest to-swap) (chrossover-swap o1 i) (chrossover-swap o2 i)))))))

(print (crossover chromosone1 chromosone2))

The code works, but I may have made it more complicated than it needs to be. I'm a beginner in functional programming and would like to know if this could be written in a simpler or more efficient way.

looks great!

(filter #(not (nil? %1)) ...)

Prefer % instead of %1 for single arguments

Simpler to write as (remove nil? ...)

(map #(cond
       (= %1 i1) i2
       (= %1 i2) i1
       :else %1) input))

When you have map + function definition in one statement, consider creating a named function with a defn, or using a for expression:

(for [i input]
  (cond ...))

Never ever def inside a loop:

(do
  (def i (first to-swap))

def creates a global var http://clojure.org/vars

You want:

(let [i (first to-swap)]
  ...)

Consider using destructuring:

(loop [[i & more] i3
  ...
 (if i
   (recur more (swap o1 i) (swap o2 i))
   [o1 o2]))


Sorting polymorphic classes I'm learning from "Jumping into C++" by Alex Allain (sample chapter and TOC) and solved the first problem in Chapter 26 "Inheritance and Polymorphism".

I'm especially concerned about my use of dynamic_cast, as that isn't covered yet and I suspect there's a better way.

  Implement a sort function that takes a vector of pointers to an interface class, Comparable, that defines a method, compare(Comparable& other), and returns 0 if the objects are the same, 1 if the object is greater than other, and -1 if the object is less than other. Create a class that implements this interface, create several instances, and sort them. If you're looking for some inspiration for what to create-try a HighScoreElement class that has a name and a score, and sorts so that the top scores are first, but if two scores are the same, they are sorted next by name.

#include <iostream>
#include <vector>
#include <string>

using namespace std;

class Comparable
{
public:
    virtual ~Comparable() { }
    virtual int compare(const Comparable& other) const = 0;
};

class HighScoreElement : public Comparable
{
public:
    HighScoreElement(string name, int score)
        : _name(name), _score(score)
    {
    }
    string getName() const {
        return _name;
    }
    int getScore() const {
        return _score;
    }
    virtual int compare(const Comparable& other) const {
        const HighScoreElement *other_hse = dynamic_cast<const HighScoreElement*>(&other);
        if (other_hse) {
            int this_score = getScore(),
                other_score = other_hse->getScore();
            if (other_score > this_score) {
                return -1;
            } else if (other_score < this_score) {
                return 1;
            } else {
                string this_name = getName(),
                       other_name = other_hse->getName();
                if (other_name > this_name) {
                    return -1;
                } else if (other_name < this_name) {
                    return 1;
                } else {
                    return 0;
                }
            }
        }
        // an exception should be thrown here
        return 0;
    }
private:
    string _name;
    int _score;
};

void sortComparableVector(vector<Comparable*>& v) {
    for (int i = 0, e = v.size() - 1; i < e; i++) {
        int sorted_i = i;
        for (int j = i + 1; j < e + 1; j++) {
            if (v[sorted_i]->compare(*v[j]) < 0) {
                sorted_i = j;
            }
        }
        if (sorted_i != i) {
            Comparable *replacement = v[sorted_i];
            v[sorted_i] = v[i];
            v[i] = replacement;
        }
    }
}

int main()
{
    vector<Comparable*> v;

    v.push_back(new HighScoreElement("Kate Bush", 10));
    v.push_back(new HighScoreElement("Peter Gabriel", 20));
    v.push_back(new HighScoreElement("Robert Fripp", 5));
    v.push_back(new HighScoreElement("Phil Collins", 25));
    v.push_back(new HighScoreElement("Roger Waters", 25));
    v.push_back(new HighScoreElement("David Gilmour", 15));

    sortComparableVector(v);

    for (unsigned i = 0; i < v.size(); i++) {
        const HighScoreElement *el = dynamic_cast<const HighScoreElement*>(v[i]);
        cout << i << ": " << el->getScore() << '\t' << el->getName() << '\n';
    }

    return 0;
}

Avoid using namespace std. You just don't know what-all symbols that spews into your global namespace, causing lots of grief when you least suspect it.
Your interface should contain protected default-constructor, copy-constructor, and copy-assignment-operator.
You should use =default when you are explicitly defaulting a special-member-function, so it can stay trivial.
If you use a struct, default-access is public instead of private, which would allow you to dispense with some access-specifiers.
Though that can be controversial for classes which have non-public members, or generally any feature not supported by C.

struct Comparable {
    virtual ~Comparable() = default;
    virtual int compare(const Comparable& other) const = 0;
protected:
    Comparable() = default;
    Comparable(const Comparable&) = default;
    Comparable& operator=(const Comparable&) = default;
};

HighScoreElement should be marked final, prohibiting further derivation and thus allowing some optimizations, unless you redesign it for inheritance. Which you actually shouldn't do.
Don't needlessly create copies of non-trivial types on call / return, use constant references.
Only HighScoreElement::getName fails that now.
Consider marking overrides with override, which in contrast to repeating virtual actually is meaningful.
You should bail out immediately if your dynamic_cast fails.
Consider just changing it to a reference-cast so that's automated.
Avoid useless blocks and concomitant further indentation.
After a return, processing leaves the function immediately.

int compare(const Comparable& other) const override {
    auto&& x = dynamic_cast<const HighScoreElement&>(other);
    if(getScore() < x.getScore())
        return -1;
    else if(getScore() > x.getScore())
        return 1;
    if(getName() < x.getName())
        return -1;
    else if(getName() > x.getName())
        return 1;
    return 0;
}

There's std::swap in <utility> for swapping two items.
Insertion-sort isn't the best algorithm there is, but it will get the job done, as long as the vector isn't too big.
You should use a smart-pointer to manage your elements, best-suited is probably std::unique_ptr from <memory>.
You could use list-initialization to initialize your vector instead of adding the elements afterwards one-by-one:

vector<Comparable*> v {
    new HighScoreElement("Kate Bush", 10),
    new HighScoreElement("Peter Gabriel", 20),
    ...
};

return 0; is implicit in C++ and C99+ for main.


React table with JSON I'm pretty new to React and I was working on some JSON data. Now I just to understand basic concepts, so I wrote this little component, but in my eyes it's not efficient.

import React from 'react';
import {getJson} from './getJson';
import AddRow from './AddRow';

class TableComponent extends React.Component {
    constructor(props) {
      super(props)
      this.state = {
        checked: true,
        rows:[],
        json: []
      }
    }

    checkboxHandler() {}

    componentDidMount() {
      this.setState((prevState) => {
        return {
          json: getJson(),
        }
      })
    }

    checkAll = () => {
        this.setState({
            checked: !this.state.checked
        })

        this.setState(prevState => {
          const json = prevState.json.map(obj => ({
            ...obj,
            items: obj.items.map(item => ({
              ...item,
              value: this.state.checked,
            }))
          }));
          return { json };
        });
      };

    render() {
      return (

              {this.state.json.map((obj, i) => {
                return (

                    {obj.items.map((data, i) => {
                        return(

{data.label}

                                    <input
                                        type="checkbox"
                                        checked={data.value}
                                        onChange={this.checkboxHandler}
                                    />

                        )
                    })}

                )
              })}

          <button onClick={this.checkAll}>check\uncheck</button>
          <AddRow actualJson={this.state.json}/>

      )
    }
  }

  export default TableComponent;

I'm not proud of it because I feel that I'm not respecting the heart of React - use module and components. Is there a way to have cleaner code?

There isn't much code to break this down to smaller components but based on the post :

this.setState() passing an Object or a callback function :

If you want to update the state with a value that's not dependent on the previous one you can do :

this.setState({ key : newValue });

If you have to update the state based on the previous one,

this.setState(prevState => ({key: prevState.key + value }));

see state updates may be asynchronous for more details.

the reason why i mention this is because in componentDidMount() you're passing a callback function to setState when you don't need the the prevState.

componentDidMount() {
  this.setState({ json: getJson() })
}

In checkAll() you can do one setState() instead of two:

checkAll = () => {
   this.setState(prevState => ({
     checked: !prevState.checked,
     json: prevState.json.map(obj => ({
       ...obj,
       items: obj.items.map(item => ({
         ...item,
         value: prevState.checked,
       }))
     }))
   }));
};

Use implicit return  in the render method :

the render method can use implict returns inthe .map() and you don't really need the i, it can be refactored to :

render() {
  return (

          {this.state.json.map(obj => (

              {obj.items.map(data => (

{data.label}

                    <input
                      type="checkbox"
                      checked={data.value}
                      onChange={this.checkboxHandler}
                    />
                	)
              )}

)
          )}

      <button onClick={this.checkAll}>check\uncheck</button>
      <AddRow actualJson={this.state.json}/>

  )
}


bad GNU libc argp code In helpcovid git commit e27dd3b123c18300665 we have a stupid bug related to Linux GNU glibc argp program argument parsing.

void
hcv_parse_program_arguments(int &argc, char**argv)
{
  struct argp_state argstate;
  memset (&argstate, 0, sizeof(argstate));
  argstate.input = &hcv_progargs;
  hcv_progargs.hcv_progmagic = HCV_PROGARG_MAGIC;
  static struct argp argparser;
  argparser.options = hcv_progoptions;
  argparser.parser = hcv_parse1opt;
  argparser.args_doc = "*no-positional-arguments*";
  argparser.doc = "github.com/bstarynk/helpcovid"
                  " - a GPLv3+ free software to help organizing"
                  " against Covid19 - NO WARRANTY";
  argparser.children = nullptr;
  argparser.help_filter = nullptr;
  argparser.argp_domain = nullptr;
  if (argp_parse(&argparser, argc, argv, 0, nullptr, nullptr))
    HCV_FATALOUT("failed to parse program arguments to " << argv[0]);
#warning TODO: complete hcv_parse_program_arguments
} // end hcv_parse_program_arguments

The bug is obvious, but I cannot find it.

At runtime, with the main  function being:

int
main(int argc, char**argv)
{
  hcv_early_initialize(argv[0]);
  hcv_parse_program_arguments(argc, argv);
  HCV_SYSLOGOUT(LOG_NOTICE, "start of " << argv[0] << std::endl
                <<  " version:" << hcv_versionmsg);
  HCV_SYSLOGOUT(LOG_INFO, "normal end of " << argv[0]);
  return 0;
} // end of main

we observe immediately:

rimski.x86_64 ~/helpcovid 19:12 .0 % ./helpcovid
** HELPCOVID FATAL! hcv_main.cc:84:: corrupted program arguments
**** FATAL ERROR hcv_main.cc:84
./helpcovid[3684197]: FATAL STOP hcv_main.cc:84 (Success)
zsh: abort (core dumped)  ./helpcovid

happening in

// parse a single program option
static error_t
hcv_parse1opt (int key, char *arg, struct argp_state *state)
{
  /* Get the input argument from argp_parse, which we
     know is a pointer to our arguments structure. */
  struct hcv_progarguments *progargs
    = reinterpret_cast<hcv_progarguments *>(state->input);
  if (!progargs || progargs->hcv_progmagic != HCV_PROGARG_MAGIC)
    // this should never happen
    HCV_FATALOUT("corrupted program arguments"); //@@@@@@@@@@@@@@@@@@@
  switch (key)
    {
    default:
      return ARGP_ERR_UNKNOWN;
    }
} // end hcv_parse1opt

The error being at line with //@@@@@@@@@@@@@@@@@@@, file hcv_main.cc line 84. The gdb debugger tells that progargs is a null pointer (and should not be).

I think that the call to argp_parse() in hcv_parse_program_arguments() is missing a pointer to the arguments struct. Looking at the main() function in the canonical Argp Example 3, we see that the argp_parse() function is passed a pointer to struct arguments arguments.

int
main (int argc, char **argv)
{
  struct arguments arguments;

  /* Default values. */
  arguments.silent = 0;
  arguments.verbose = 0;
  arguments.output_file = "-";

  /* Parse our arguments; every option seen by parse_opt will
     be reflected in arguments. */
  argp_parse (&argp, argc, argv, 0, 0, &arguments);

  printf ("ARG1 = %s\nARG2 = %s\nOUTPUT_FILE = %s\n"
          "VERBOSE = %s\nSILENT = %s\n",
          arguments.args[0], arguments.args[1],
          arguments.output_file,
          arguments.verbose ? "yes" : "no",
          arguments.silent ? "yes" : "no");

  exit (0);
}