A simple phonebook in React After learning Javascript for quite some time, I began learning ReactJS and... I am confused with the proper design patterns.

import React, { useState } from 'react';
import ReactDOM from 'react-dom';

const Search = ({persons, filter}) =>
{
    const [name, setName] = useState(void 0);

    function search(event){
        event.preventDefault();
        filter(event.target.value.toLowerCase());
    }

    return (

Search

            <form onSubmit={search}>
                <input onChange={(event) => search(event)}/>
                <button type="submit">Search</button>
            </form>

    )
}

const Persons = ({persons}) =>
{
    function renderPersons(){
        const elements = [];

        persons.forEach(person => {
            if(person.show){
                elements.push(
{person.name} -- his/her telephone number: {person.phone}
);
            }
        });

        return elements;
    }

    return(

            {renderPersons()}

    )
}

const App = () =>
{
    const [ persons, setPersons ] = useState([
        { name: 'Arto Hellas', phone: '040-123456', show: true },
        { name: 'Ada Lovelace', phone: '39-44-5323523', show: true },
        { name: 'Dan Abramov', phone: '12-43-234345', show: true },
        { name: 'Mary Poppendieck', phone: '39-23-6423122', show:true }
    ]);

    const [ formInfo, setNewFormInfo ] = useState({name: '', phone: ''});

    function addPerson(event){
        event.preventDefault();

        let able = true;

        persons.forEach(person => { // checks if the person's name is already added
            if(person.name == formInfo.name)
                able = false;
        });

        if(able)
            setPersons(persons.concat({name:formInfo.name, phone:formInfo.phone, show:true}));
        else
            alert(`${formInfo.name} is already added to the list!`);
     }

    function filter(nameToFilter){
        if(nameToFilter === ''){ // if the search form is empty, then show everyone
            setPersons((prev) => prev.map(person => ({...person, show:true})));
        }
        else{
            const filtered = [];
            persons.forEach(person => {
                if(!person.name.toLowerCase().includes(nameToFilter))
                    filtered.push({...person, show:false});
                else
                    filtered.push({...person});
            });
            setPersons(filtered);
        }
    }

    return (

            <Search persons={persons} filter={filter} />

Phonebook

            <form onSubmit={addPerson}>

                    name: <input onChange={(event) => setNewFormInfo({name: event.target.value, phone: formInfo.phone})}/>

                    number: <input onChange={(event) => setNewFormInfo({name: formInfo.name, phone: event.target.value})}/>

                    <button type="submit">add</button>

            </form>

Persons

            <Persons persons={persons} />

    )
}

ReactDOM.render(<App />, document.getElementById("root"))

It works, but I have a feeling the code really is poorly designed; the course I am taking doesn't focus on details and motivates you to self-learn a lot.

I do have a specific question, though. Can I make persons a class? I just feel like in larger applications, each person would have methods and more variables; can I hold classes inside of React states instead of dicts?

There are a bunch of small improvements you can make.

Since you aren't using name or setName, you can remove them and their useState call (consider a linter to warn on unused variables).

You have <form onSubmit={search}>, but the search function does filter(event.target.value.toLowerCase()). Forms don't have .values, so this will throw an error when the button is pressed. Also, the search function could be confused with the Search component - the difference is only in capitalization, which isn't enough. The button doesn't do anything otherwise, because searching occurs when the input's change event fires.  You might consider removing the button completely - this way you can remove the search function and just call filter:

<form>
    <input onChange={event => filter(event.target.value)} />
</form>

In your code, eveywhere you return JSX at the bottom of a function, there's no trailing ; after the final ). This can result in bugs if you ever decided to put the fragment into a variable instead of returning immediately, due to automatic semicolon insertion. Since you're using semicolons everywhere else, consider using them everywhere they're appropriate as well. (again, linting will help with this)

Similarly to the search vs Search above, you also have a persons array and a Persons component. Maybe call the component PersonsList instead, since it creates a
.

Rather than using forEach and conditionally pushing to an array outside, it may be a better idea to construct the array directly by filtering for elements which match, then .mapping to the individual
s. Higher-order functions are great:

const PersonsList = ({ persons }) => (

        {(
            persons
                .filter(person => person.show)
                .map(person =>
{person.name} -- his/her telephone number: {person.phone}
)
        )}

);

In addPerson, if you want to check whether any items in an array match a condition, rather than setting a flag outside and conditionally reassigning it while iterating over an array, it's more appropriate to use Array.prototype.some:

const alreadyExists = persons.some(person => person.name === name);

If you were to use your original code, at least use strict equality with === instead of loose equality ==.

Also in addPerson, using window.alert is almost never a good choice. It's user-unfriendly, since it completely blocks them from using the rest of the page. It also prevents any other Javascript from executing until the alert is dismissed, which would be a problem if there were anything on the page other than this phone number component. Inform the user of the problem some other way. A very simple tweak would be to add errorMessage to the form's state:

if (!alreadyExists)
    setPersons(persons.concat({ name, phone, show: true }));
else
    setNewFormInfo({ name, phone, errorMessage: `${name} is already added to the list!` });

and add to the form's JSX:

{(errorMessage &&
{errorMessage}
)}

In the filter function, there's no need to check if the input is empty - if it is, everything will be rendered anyway. Feel free to remove that part and make the code simpler.

Also in filter, since you're creating a filtered array by iterating over each element of persons and pushing an element to filtered, using .map would be more appropriate than forEach / if / push:

function filter(nameToFilter) {
    const nameToFilterLower = nameToFilter.toLowerCase();
    setPersons(persons.map(person =>
        ({
            ...person,
            show: person.name.toLowerCase().includes(nameToFilterLower),
        })
    ));
}

If you run the development version, you'll see the warning:

  Warning: Each child in a list should have a unique "key" prop.

Without such a prop, each element will have to be re-created each time. See here for an in-depth explanation. Since you're only adding items to the list, easiest fix would be to add
 {
    return (

Search

            <form>
                <input onChange={event => filter(event.target.value)} />
            </form>

    );
};

const PersonsList = ({ persons }) => (

        {(
            persons
                .filter(person => person.show)
                .map(({ name, phone }) =>
{name} -- his/her telephone number: {phone}
)
        )}

);

const App = () => {
    const [persons, setPersons] = useState([
        { name: 'Arto Hellas', phone: '040-123456', show: true },
        { name: 'Ada Lovelace', phone: '39-44-5323523', show: true },
        { name: 'Dan Abramov', phone: '12-43-234345', show: true },
        { name: 'Mary Poppendieck', phone: '39-23-6423122', show: true }
    ]);

    const [{ name, phone, errorMessage = '' }, setNewFormInfo] = useState({ name: '', phone: '' });
    function addPerson(event) {
        event.preventDefault();

        const alreadyExists = persons.some(person => person.name === name);
        if (!alreadyExists)
            setPersons(persons.concat({ name, phone, show: true }));
        else
            setNewFormInfo({ name, phone, errorMessage: `${name} is already added to the list!` });
    }

    function filter(nameToFilter) {
        const nameToFilterLower = nameToFilter.toLowerCase();
        setPersons(persons.map(person =>
            ({
                ...person,
                show: person.name.toLowerCase().includes(nameToFilterLower),
            })
        ));
    }

    return (

            <Search filter={filter} />

Phonebook

            <form onSubmit={addPerson}>

                    name: <input onChange={event => setNewFormInfo({ name: event.target.value, phone })} />

                    number: <input onChange={event => setNewFormInfo({ name, phone: event.target.value })} />

                    <button type="submit">add</button>

                {(errorMessage &&
{errorMessage}
)}
            </form>

Persons

            <PersonsList persons={persons} />

    );
};
ReactDOM.render(<App />, document.getElementById("root"))
.error {
  color: red;
}

  Can I make persons a class?

You can, but React highly recommends using hooks (and stateless functional components) when possible. They're easier to make sense of too, IMO.

  in larger applications, each person would have methods and more variables

If you want to encapsulate common functionality for Persons, you can easily do so by putting it into its own file and defining those functions you need inside that file (or by importing them), eg:

// example helper function
const transformName = name => name.toUpperCase();
export const PersonsList = ({persons}) =>
  // ...

{transformName(name)} -- his/her telephone number: {phone}

  // ...

There are a few rare cases for which classes must still be used, but none of those circumstances are relevant here.


Manipulating a .csv file to look for two common values to create a key, then summing up values My code reads a .csv file, looks for a couple values to create its key, and then wraps up the data based on a few business rules I have. I'm trying to learn "the right way", so I'd really appreciate some tips and hints.

import csv
import sys

# add DictReader
with open(sys.argv[1], 'rbU') as csvinfile:
    myreader = csv.reader(csvinfile, delimiter=',', quotechar='"')

    # initialize variables and dictionaries
    line_cntr = 0
    dict = {}
    cntr = {}

    for row in myreader:
        # constant values
        nd_amt = row[14]
        deduct_amt = row[15]
        nondeduct_ytd = row[16]
        deduct_ytd = row[17]
        pid = row[18]
        don_date = row[19]
        amount = row[23]
        anon = row[25]
        int_0003 = row[38]
        int_0006 = row[39]
        int_0028 = row[40]

        # create a composite key for our dictionary
        key = pid + ":" + don_date
        # check to see if key exists in dictionary, if not then add
        # as per BR-0010 every group of up to 6 entries with same P_ID and Don_Date should print on their own line (i.e. different entry in Dict).
        if key in dict:
            if cntr[key] % 6 != 0:
                dict[key][14] += row[14]
                dict[key][15] += row[15]
                dict[key][16] += row[16]
                dict[key][17] += row[17]
                dict[key][23] += row[23]
                cntr[key] += 1
            else:
                key = pid + ":" + don_date + ":" + str(cntr[key]//6)
                dict[key] = row
        else:
            dict[key] = row
            cntr[key] = 1

        # debugging
        for key in cntr:
            if cntr[key] > 6:
                print(key, cntr[key])

        # keep track of lines processed for recon
        line_cntr += 1

# add DictWriter
with open(sys.argv[2], 'wb') as csvoutfile:
    mywriter = csv.writer(csvoutfile, delimiter=',', quotechar='"')

    for key in dict:
        outline = (key, dict[key])
        mywriter.writerow(outline)

    # print recon totals at bottom
    csvoutfile.write("Total lines processed: " + str(line_cntr))

I have some comments in there about wanting to try out DictReader/DictWriter in my next draft of code, so if anyone has tips on that it would be much appreciated as well.

Overall, your code is pretty clean.

Here are some points for improvement:

Use the if __name__ == '__main__' idiom if this file is to be executed directly. https://docs.python.org/2/library/main.html
Structure your code so that no code is executed before the name clause or upon import. In other words, don't have any code blocks which start at the left-hand margin. Instead, place code within a few functions which are called by a main run() function.
Use an enum or constants to represent the index values for things like nd_amount.

EDIT

ND_AMT = 14
...

nd_amt = row[ND_AMT]


Set of jQuery .onclick functions I am hiding and fading in different content on the same page using jQuery to hide() and fadeIn() the content depending on which link is clicked.

It works how I want it to, but the way I've written the jQuery looks like it could be simplified.

$("#navItem1").on('click', function(){
    $('#content-wrap1').hide();
    $('#content-wrap2').hide();
    $('#content-wrap3').hide();
    $('#content-wrap4').hide();
    $('#content-wrap5').hide();
    $('#content-wrap1').fadeIn(1000);
});
$("#navItem2").on('click', function(){
    $('#content-wrap1').hide();
    $('#content-wrap2').hide();
    $('#content-wrap3').hide();
    $('#content-wrap4').hide();
    $('#content-wrap5').hide();
    $('#content-wrap2').fadeIn(1000);
});
$("#navItem3").on('click', function(){
    $('#content-wrap1').hide();
    $('#content-wrap2').hide();
    $('#content-wrap3').hide();
    $('#content-wrap4').hide();
    $('#content-wrap5').hide();
    $('#content-wrap3').fadeIn(1000);
});
$("#navItem4").on('click', function(){
    $('#content-wrap1').hide();
    $('#content-wrap2').hide();
    $('#content-wrap3').hide();
    $('#content-wrap4').hide();
    $('#content-wrap5').hide();
    $('#content-wrap4').fadeIn(1000);
});
$("#navItem5").on('click', function(){
    $('#content-wrap1').hide();
    $('#content-wrap2').hide();
    $('#content-wrap3').hide();
    $('#content-wrap4').hide();
    $('#content-wrap5').hide();
    $('#content-wrap5').fadeIn(1000);
});

function fadeInFirstContent(){
    $('#content-wrap1').fadeIn(1000);

}
fadeInFirstContent();

Here's a jfiddle

How can I rewrite the jQuery so it does exactly the same thing but in much less code?

Perhaps change the HTML:

Navitem:

The content

jQuery

$('.navItem').on('click',function(){
   $('.content-wrap').hide();
   $('#content-wrap'+$(this).data('content-id')).fadeIn(1000);
});


Generating 3 combinations in Python I have been experimenting around with generating all the n-combinations of an array. This code quickly generates all \$\text{k-combinations}\$ of a given array. I am testing my own implementation because of some restrictions in coding competitions (also, knowing the basics never hurts).

"""
Generates 3-combinations of an array.

A B C D E F G H
^ ^ ^
i j k

A B C D E F G H
^ ^   ^
i j   k

A B C D E F G H
^   ^   ^
i   j   k
"""
from itertools import combinations
import time

def combinations_3(ary):
    l = len(ary)
    for i in xrange(l-2):
        for j in xrange(i+1, l-1):
            for k in xrange(j+1, l):
                yield (ary[i], ary[j], ary[k])

start = time.time()
for combination in combinations_3([1, 2, 3, 4]):
    print combination
print time.time() - start

print("===========Library Implementation============")
start = time.time()
for combination in combinations([1, 2, 3, 4], 3):
    print combination
print time.time() -start

Output

(1, 2, 3)
(1, 2, 4)
(1, 3, 4)
(2, 3, 4)
0.00043797492981
===========Library Implementation============
(1, 2, 3)
(1, 2, 4)
(1, 3, 4)
(2, 3, 4)
6.19888305664e-05

It seems that my implementation is a lot slower than the library code. How can I improve performance of my code.

You probably want to use timeit module with similar setup:

run.py

from itertools import combinations

def combinations_3(ary):
    l = len(ary)
    for i in xrange(l-2):
        for j in xrange(i+1, l-1):
            for k in xrange(j+1, l):
                yield (ary[i], ary[j], ary[k])

def _test(generator):
    for combination in generator:
        #: To meger time of sequence generation we should ommit IO operations.
        pass

def test_lib(lenght=10000):
    _test(combinations(list(range(1, lenght)), 3))

def test_self_written(lenght=10000):
    _test(combinations_3(list(range(1, lenght))))

test.py

import timeit

#: Print title
print('                library time   self written time')

#: Counter of sequence number.
for i in list(range(10, 100, 10)):
    #: Meger execution time for self written combinations implementation.
    self_written = timeit.Timer(
        #: Tested expression.
        'test_lib({})'.format(i),
        #: Test setup, here we just import tested function.
        'from run import test_self_written as test_lib'
    ).timeit(1000)  #: And here we just set number of testing iterations.

    lib = timeit.Timer(
        'test_lib({})'.format(i),
        'from run import test_lib'
    ).timeit(1000)

    #: Print output in format "<number of elements>: library time, self written time"
    print('{:03} elements: {:10.6f}     {:10.6f}'.format(i, lib, self_written))

And here is my output:

$ python2 test.py
                library time   self written time
010 elements:   0.007060       0.030495
020 elements:   0.058317       0.239376
030 elements:   0.211884       0.817189
040 elements:   0.522477       1.903142
050 elements:   1.047018       3.803112
060 elements:   1.969069       6.590189
070 elements:   2.960615      10.500072
080 elements:   4.468489      15.176092
090 elements:   6.402755      21.669669
$ python3 test.py
                library time   self written time
010 elements:   0.009168       0.038298
020 elements:   0.058049       0.285885
030 elements:   0.208938       1.031432
040 elements:   0.523407       2.450311
050 elements:   1.048463       4.796328
060 elements:   1.828277       7.832434
070 elements:   2.970929      13.067557
080 elements:   4.481792      18.496334
090 elements:   6.353836      26.277946
$ pypy test.py
                library time   self written time
010 elements:   0.011924       0.040272
020 elements:   0.056972       0.046638
030 elements:   0.185858       0.083793
040 elements:   0.456202       0.188640
050 elements:   0.913935       0.338211
060 elements:   1.588666       0.542124
070 elements:   2.537772       0.846942
080 elements:   3.816353       1.232982
090 elements:   5.471375       1.707760

And python versions

$ python2 -V && python3 -V && pypy -V
Python 2.7.6
Python 3.4.3
[PyPy 2.2.1 with GCC 4.8.2]

As you can see, pypy is much faster than cpython implementation.

Actually, if you are using cpython you do not really want to reinvent standard library functions because they are pretty nice optimized by c compiler and you code will be interpreted instead of compiling. In case of pypy you probably want to make some research, because JIT interpreter have a lot of different corner cases.


Try every option for a command and make anagrams out of the successful ones What I have: I am trying to find what words you can make out of Linux command options like ls -cat or ps -elf. To do that I am iterating through an array of 50 commands, trying to run each with flags -[a-z] and then making anagrams out of the successful options. It is working and here is what I have:

#!/bin/bash

# Pull in a list of common Linux commands
commandList=( $(
  curl 'http://www.thegeekstuff.com/2010/11/50-linux-commands/' 2> /dev/null \
    | grep -o '<h3>\([0-9].*[a-z][a-z][a-z].*\)</h3>' \
    | awk '{ print $(NF-2) }' \
    | sed -nr '/rm|wget|less|shutdown|<h3>/!p'
) );

# Pipe successful "$command -$option" pairs to 'an' to generate anagrams
for command in ${commandList[@]} ; do
    (for option in {a..z} ; do
        timeout -k 5 5 "$command" -"$option" > /dev/null 2>&1;
            if (($? == 0)); then
                printf "$option" | tr -d '\n'
            else
                    :
            fi
    done) | xargs an -w -d saneWordlist -m 3 2> /dev/null \
          | sed 's/^/\ '$command' -/' >> commandOptions.log
done

Self Criticism:
I know it is unreliable to rely on exit codes that are not at all consistent and just hoping the devs used 0 to indicate success. I also know I abused sed and awk a lot. I also didn't try to write this in a very POSIX-y way.

What I want help on:
I am not too worried about the web scraping part, I know it is cheap and messy. But, how can I improve the core section where I misuse sed and awk so much? I know there cleaner ways to accomplish what I want, but can't think of anything.

Some of the commands in the web scraping part can be simplified.

Instead of awk '{ print $(NF-2) }', why not simply awk '{ print $2 }'?

Instead of sed -nr '/rm|wget|less|shutdown|<h3>/!p', it would be simpler and more portable to use grep -vE 'rm|wget|less|shutdown|<h3>'.

Using a Bash array is a bit overkill,
because the output of the pipeline is guaranteed to be one word per line,
no other white space.
So using a simple variable and word splitting should do the job, and simpler:

commandList=$(
  curl 'http://www.thegeekstuff.com/2010/11/50-linux-commands/' 2> /dev/null \
    | grep -o '<h3>\([0-9].*[a-z][a-z][a-z].*\)</h3>' \
    | awk '{ print $2 }' \
    | grep -vE 'rm|wget|less|shutdown|<h3>'
)

for command in $commandList; do

Many improvements are possible here:

timeout -k 5 5 "$command" -"$option" > /dev/null 2>&1;
    if (($? == 0)); then
        printf "$option" | tr -d '\n'
    else
            :
    fi

In Bash, you can use &>/dev/null to redirect both stdout and stderr.

Since $option only contains a single letter and printf doesn't print a trailing newline like echo, the tr command there is pointless.
The double-quotes around $option are also unnecessary.
The empty else branch should be removed.

Instead of using a condition on $?, you could use the command itself directly:

if timeout -k 5 5 "$command" -$option &> /dev/null; then
    printf $option
fi

You could rewrite the final sed simpler using double-quotes:

sed "s/^/ $command -/" >> commandOptions.log

I also removed a pointless \ from in front of a space within the quoted expression.


Factory to instantiate and configure Selenium WebDriver I, also, need to know whether this is a valid implementation of the factory pattern.

The amount of effort which goes into configuring DriverFactory to simply get a WebDriver instance seems disproportionate to the point where I ask:  "is there not a better way?"

The main class code:

package dur.bounceme.net.SeleniumBase;

import java.io.IOException;
import java.util.Properties;
import java.util.logging.Logger;
import org.openqa.selenium.WebDriver;

public class App {

    private static final Logger LOG = Logger.getLogger(App.class.getName());
    private Properties properties = new Properties();

    public static void main(String[] args) throws IOException {
        new App().initSelenium();
    }

    private void initSelenium() throws IOException {
        properties.loadFromXML(App.class.getResourceAsStream("/selenium.xml"));
        WebDriver webDriver = DriverFactory.getWebDriver(properties);  //is this factory and usage correct?
        HomePage homePage = new HomePage(webDriver);
        homePage.populateCatalogue();
    }
}

And my factory:

package dur.bounceme.net.SeleniumBase;

import java.net.MalformedURLException;
import java.net.URL;
import java.util.ArrayList;
import java.util.List;
import java.util.Properties;
import java.util.logging.Logger;
import org.openqa.selenium.WebDriver;
import org.openqa.selenium.firefox.FirefoxBinary;
import org.openqa.selenium.firefox.FirefoxDriver;
import org.openqa.selenium.firefox.FirefoxOptions;

public class DriverFactory {

    private static final Logger LOG = Logger.getLogger(DriverFactory.class.getName());

    private Properties properties = new Properties();
    private FirefoxBinary firefoxBinary = null;
    private FirefoxOptions firefoxOptions = null;
    private WebDriver webDriver = null;
    private URL url = null;
    private String driver = null;
    private String gecko = null;
    private final List<String> commandLineOptions = new ArrayList<>();

    private DriverFactory() {
    }

    private DriverFactory(Properties properties) throws MalformedURLException {
        this.properties = properties;
        loadProps();
        init();
    }

     static WebDriver getWebDriver(Properties properties) throws MalformedURLException {
        DriverFactory driverFactory = new DriverFactory(properties);
        return driverFactory.webDriver;
    }

    private void loadProps() throws MalformedURLException {
        LOG.fine(properties.toString());
        url = new URL(properties.getProperty("url"));
        driver = properties.getProperty("driver");
        gecko = properties.getProperty("gecko");
        String commandLineOption = properties.getProperty("option01");
        commandLineOptions.add(commandLineOption);
    }

    private void init() throws MalformedURLException {
        firefoxBinary = new FirefoxBinary();
        commandLineOptions.forEach((commandLineOption) -> {
            LOG.fine(commandLineOption.toString());
            firefoxBinary.addCommandLineOptions(commandLineOption);
        });
        System.setProperty(driver, gecko);
        firefoxOptions = new FirefoxOptions();
        firefoxOptions.setBinary(firefoxBinary);
        webDriver = new FirefoxDriver(firefoxOptions);
        webDriver.get(url.toString());
        LOG.fine(webDriver.getTitle());
        LOG.fine(webDriver.getCurrentUrl().toLowerCase());
    }

    void close() {
        webDriver.close();
    }

}

There seems no nice way to get around loading configuration from a file, which is then loaded.  But, it strikes me that I'm re-inventing the wheel of loading a props file for the purpose of configuring a Selenium WebDriver when I expect there's a tried-and-true approach.

Or, at least, more re-usable.

Although, what possible use there would be for multiple WebDriver instances is beyond me.  It would seem Selenium Grid would handle that question quite differently.

Quick Simplifications

The 0-args constructor is not used. Remove it.
close() can not be called in a meaningful way. Nobody can obtain an instance of DriverFactory, so making that an instance method is not useful in any way.
You're misusing fields as variables. Everything you're doing can be put into the factory method itself. There's no need for any instance data, you're not accessing it anywhere.

Let's consider instead:

static WebDriver getWebDriver(Properties properties) throws MalformedURLException {
    LOG.fine(properties.toString);
    final URL url = new URL(properties.getProperty("url"));
    final String driver = properties.getProperty("driver");
    final String gecko = properties.getProperty("gecko");
    final List<String> commandLineOptions = new ArrayList<>();
    commandLineOptions.add(properties.getProperty("option01"));

    final FirefoxBinary ffBinary = new FirefoxBinary();
    commandLineOptions.forEach((commandLineOption) -> {
        LOG.fine(commandLineOption);
        ffBinary.addCommandLineOptions(commandLineOption);
    });
    System.setProperty(driver, gecko);

    final FirefoxOptions ffOptions = new FirefoxOptions();
    ffOptions.setBinary(ffBinary);

    WebDriver result = new FirefoxDriver(ffOptions);
    result.get(url.toString());
    LOG.fine(webDriver.getTitle());
    LOG.fine(webDriver.getCurrentUrl().toLowerCase());
    return result;
}

This does mostly the same as the code you presented. We can see here that opening yourself up to MalformedURLException is not necessary. You're reading the String from properties into a URL and then call .toString() on it.

What we can see pretty easily now is that you're reading the contents of the passed Properties into variables to just use them once. It might be easier to reimagine the code as something like the following:

public static WebDriver getWebDriver(Properties properties) {
    final FirefoxBinary ffBinary = discoverBinary(properties);
    setSystemProperties(properties);
    final FirefoxOptions ffOptions = buildOptions(ffBinary);
    return createWebDriver(ffOptions, properties);
}

This makes the code comparatively abstract and easy to follow. You can grasp what happens without needing a deeper understanding of Selenium itself (not that the original code required a lot of it).

This abstraction just makes it easier to understand what's going on. The methods themselves are pretty trivial :)


Running tasks in parallel I've just started writing asynchronous methods for the first time having watched some tutorials.

I have a method where I run two tasks in parallel (notifySales, insertDownload):

Task insertDownload = _downloadService.InsertDownloadRequestAsync(model, Request.UserAgent);

if (product.NotifySalesByEmail)
{
    Task notifySales = _downloadService.EmailDownloadRequestAsync(model);
    await Task.WhenAll(insertDownload, notifySales);
}
else
{
    await insertDownload;
}

As you can see, the task notifySales only gets run if product.NotifySalesByEmail is true. This task, as well as task insertDownload can be run in parallel.

I'm not sure I like the way in which I have said 'if true await both else just await one'.

Would a better way to be to create a Task[] array, add the tasks that are required to run and then await the array?

The code as it stands doesn't read well. And your text is also slightly confusing. I do believe I would rework the code to something along these lines:

Initialize an array of tasks
Add each task to array, with necessary conditions if needed
Await execution of all tasks

This would make for your code to handle extra tasks better, it would clear up the confusion on which tasks actually gets executed, and finally all tasks can be executed in parallell however the compiler feels like doing that.


Implementation of Non-Generic Stack in Java I'm starting out to work on DS in Java for the first time and thought that some inputs will get me going to more data structures. I implemented a stack which is non-generic. I followed some of the posts and realized that the top pointer should not be at -1. I know this is a very basic implementation (probably the worst). The LIFO part is working well. I am looking for suggestions on what are some ways to improve this code other than making this to be Generic Stack.

Code:

import java.util.*;

//******************************* Stack Demo Class **********//

class StackDemo{
  int stackarray[] , top;

  // Constructor

  StackDemo(int size){
    stackarray = new int[size+1];
    top = 0;
    System.out.println("The size of stack array is "+ (stackarray.length-1));
  }

  // *********************PUSH******************************//

  void push(int ele){
    if(top == stackarray.length - 1){
      System.out.println("Stack Overflow");
    }
    else{
      top++;
      stackarray[top] = ele;
    }
  } // Push Method

  // *********************POP******************************//

  int pop(){
    int lastElement = 0;
    if(top == 0){
      System.out.println("Stack is Underflow");
    }
    else{
      lastElement = stackarray[top];
      top--;
    }
    return lastElement;
  }  // Pop Method

  // *********************DISPLAY******************************//

  void display(){
    if(top == 0){
      System.out.println("Nothing to Display");
    }
    for (int i = 1; i<=top ; i++){
      System.out.print(stackarray[i] +"\t");
    }
    System.out.println("");
  } // Display Method

}

// *****************************Main Class*******************//

public class Stacks{
  public static void main(String args[]){
    StackDemo sd = new StackDemo(10);
    Scanner s = new Scanner(System.in);
    while(true){
      System.out.println("Enter your choice : \n 1.Push \n 2.Pop \n 3.Display \n 4.Exit");
      int choice = s.nextInt();
      switch(choice){
        case 1:
          System.out.println("Enter an Element to push");
          int pushElement = s.nextInt();
          sd.push(pushElement);
          break;
        case 2:
          int popElement = sd.pop();
          System.out.println("Popped Element is "+popElement);
          break;
        case 3:
          sd.display();
          break;
        case 4:
          System.exit(0);
          break;
        default:
          System.out.println("Enter valid choice");
      }  // Switch Closing
    } // While Closing
  } // Main method
} // Main class

Thanks for sharing your code,

Avoid Wild Imports

You import

import java.util.*;

but the only thing you use from the util package is Scanner,
so why not just

import java.util.Scanner;

Now it's obvious to anyone reading what you'll be using from util.

Counting Starts at 0

stackarray = new int[size+1];

in your constructor, If I make a stack of size 10, I'm actually making one of size 11!

You're also accommodating for your +1 size in your constructor with

if(top == stackarray.length - 1){
   ...
}

if you just initialised your array as new int[size];

then this could look like

if(top == stackarray.length){
   ...
}

At the moment you're offsetting in 2 places, when in fact you don't need to do this anywhere! This is making the code more difficult for other people to follow.

In your display method

for (int i = 1; i<=top ; i++){
    ...
}

you can just start this at i = 0, starting at 1 is very unnatural.

this should be re-written like

for (int i = 0; i < top ; i++){
     ...
}

Unusual Behaviour

right now, if I pop() an empty stack, it will return 0. This would be extremely unusual behaviour for an empty data structure. It wouldn't really be empty anymore would it?

  int pop(){
    int lastElement = 0;
    if(top == 0){
      System.out.println("Stack is Underflow");
    }
    else{
      lastElement = stackarray[top];
      top--;
    }
    return lastElement;
  }  // Pop Method

instead of printing "Stack is Underflow", why not throw a new EmptyStackException(msg). That's what it's for after all.

In your push method, you print a message if the stack is full. Again I would suggest throwing an appropriate exception here, or even better, you could resize the array to accommodate the new element. (This would be great practice for making an ArrayList implementation.)

What's Missing?

Your stack is missing an isEmpty() or empty() method, this should just return true/false for if the stack is empty or not. This is extremely simple to implement so you shouldn't have any problems with this!

Your stack is missing a peek() method, this should show the next element, without removing it.

Why Stop There?

Some other methods you might like to add for practice could be,

public boolean contains(int element) // if the lement is present in the stack

public int numberOf(int element) // occurrences of a given element

public int smallest() // the smallest element in the stack

public int largest()

public int totalSum()

public int average()

Hopefully this review was useful for you, keep it up!


Using an interface for decoupling I am studying about how to use an interface for decoupling, and I wrote a program.  Does it actually implemented decoupling?  Are there any possible modifications to make it better?

public partial class MainPage
{
    // Constructor
    public MainPage()
    {
        InitializeComponent();

        IDBInterface idbInterface = new SQL();
        CommunicationChannel com = new CommunicationChannel();
        com.Update(idbInterface);
    }
}

interface IDBInterface
{
    void Update();
}

class SQL:IDBInterface
{
    public void Update()
    {
        MessageBox.Show("Please wait.. We are updating SQL Server...");
    }
}

class Oracle : IDBInterface
{
    public void Update()
    {
        MessageBox.Show("Please wait.. We are updating Oracle Server...");
    }
}

class CommunicationChannel
{
    public void Update(IDBInterface idb)
    {
        idb.Update();
    }
}
}

Yes that's it, that is decoupled.

However, the benefits of this are not visible in your example because the look of SQL and IDBInteraface are the same.

It would become obvious if your example was of that kind:

interface IDBInterface
{
    void Update(string sqlQuery);
    void Insert(string sqlQuery);
    void Select(string sqlQuery);
}

class SQL:IDBInterface
{
    public void Update(string sqlQuery){/*implementation*/};
    public void Insert(string sqlQuery){/*implementation*/};
    public void Select(string sqlQuery){/*implementation*/};

    public void connect(){/*implementation*/};
    public void disconnect(){/*implementation*/};
    public void optimize(){/*implementation*/};;
    public void backup(){/*implementation*/};;
}

In such cases, when using the IDBInterface, you would not even mind about all the administrative work to be done ( connect, disconnect, optimize, backup).


File system manipulation helper What do you think about this file system manipulation helper? There is an utility class Folder which I can use to define directory structure of my app:

static class Folders
{
    public static Folder Bin =>
        new Folder(Assembly.GetExecutingAssembly());

    public static Folder App => Bin.Up();
    public static Folder Docs => App.Down("Docs");
    public static Folder Temp => App.Down("Temp");
}

Then I can do some manipulations in an easy way:

    static void Main(string[] args)
    {
        Folders.Temp.Create();
        Folders.Bin.Run("scan.exe.", "-no-ui");
        Folders.Temp.CopyTo(Folders.Docs, f => f.EndsWith(".pdf"));
        Folders.Temp.Empty();
    }

Here is the library class Folder used above:

public class Folder
{
    readonly string _path;

    public Folder(Assembly assembly)
        : this(Path.GetDirectoryName(assembly.Location))
    {
    }

    public Folder(string path)
    {
        _path = path;
    }

    public override string ToString() => _path;

    public static implicit operator string(Folder folder) =>
        folder.ToString();

    public IEnumerable<Folder> Folders =>
        Directory.GetDirectories(this, "*", SearchOption.TopDirectoryOnly)
            .Select(p => new Folder(p));

    public IEnumerable<Folder> AllFolders =>
        Directory.GetDirectories(this, "*", SearchOption.AllDirectories)
            .Select(p => new Folder(p));

    public IEnumerable<string> Files =>
        Directory.GetFiles(this, "*.*", SearchOption.TopDirectoryOnly);

    public IEnumerable<string> AllFiles =>
        Directory.GetFiles(this, "*.*", SearchOption.AllDirectories);

    public Folder Up() =>
        new Folder(
            new DirectoryInfo(this)
                .Parent.FullName);

    public Folder Down(string folderName) =>
        new Folder(
            Path.Combine(
                this,
                folderName));

    public void Create() =>
        Directory.CreateDirectory(this);

    public void Empty()
    {
        var directoryInfo = new DirectoryInfo(this);
        if (!directoryInfo.Exists)
            return;

        foreach (var file in AllFiles)
            File.Delete(file);

        foreach (var folder in Folders)
            Directory.Delete(folder, true);
    }

    public void CopyTo(Folder destination) =>
        CopyTo(destination, file => true);

    public void CopyTo(Folder destination, Func<string, bool> filter)
    {
        //Create directories
        foreach (string directoryPath in AllFolders)
            Directory.CreateDirectory(directoryPath.Replace(this, destination));

        //Copy all the files & replaces any files with the same name
        foreach (string filePath in AllFiles.Where(filter))
            File.Copy(filePath, filePath.Replace(this, destination), true);
    }

    public void Run(string exe, string args = "")
    {
        string defaultCurrentDirectory = Environment.CurrentDirectory;
        Environment.CurrentDirectory = this;
        try
        {
            var process = new Process();
            process.StartInfo.FileName = exe;
            process.StartInfo.Arguments = args;
            process.Start();
            process.WaitForExit();
            if (process.ExitCode != 0)
                throw new InvalidOperationException(
                    $"{exe} process failed with exit code {process.ExitCode}.");
        }
        finally
        {
            Environment.CurrentDirectory = defaultCurrentDirectory;
        }
    }
}

I'm always a fan of using interfaces wherever possible so that unit tests can mock out my component easily. So let's create a couple of interfaces:

public interface IFolder
{
    IEnumerable<IFolder> Folders { get; }

    IEnumerable<IFolder> AllFolders { get; }

    IEnumerable<string> Files { get; }

    IEnumerable<string> AllFiles { get; }

    IFolder Up();

    IFolder Down(string folderName);

    void Create();

    void Empty();

    void CopyTo(IFolder destination);

    void CopyTo(IFolder destination, Func<string, bool> filter);

    void Run(string exe, string args = "");
}

and

internal interface IFolders
{
    IFolder Bin { get; }

    IFolder App { get; }

    IFolder Docs { get; }

    IFolder Temp { get; }
}

I also like to re-use constants. And I added another implicit operator to go with your new constructor:

public class Folder : IFolder
{
    private const string DirectoryWildcard = "*";

    private const string FileWildcard = "*.*";

    private readonly string _Path;

    public Folder(Assembly assembly)
        : this(Path.GetDirectoryName(assembly.Location))
    {
    }

    public Folder(string path)
    {
        this._Path = path;
    }

    public override string ToString() => this._Path;

    public static implicit operator string(Folder folder) => folder.ToString();

    public static implicit operator Folder(string path) => new Folder(path);

    public static implicit operator Folder(Assembly assembly) => new Folder(assembly);

    public IEnumerable<IFolder> Folders => Directory
        .GetDirectories(this, DirectoryWildcard, SearchOption.TopDirectoryOnly)
        .Select(path => new Folder(path));

    public IEnumerable<IFolder> AllFolders => Directory
        .GetDirectories(this, DirectoryWildcard, SearchOption.AllDirectories)
        .Select(path => new Folder(path));

    public IEnumerable<string> Files => Directory.GetFiles(this, FileWildcard, SearchOption.TopDirectoryOnly);

    public IEnumerable<string> AllFiles => Directory.GetFiles(this, FileWildcard, SearchOption.AllDirectories);

    public IFolder Up() => new Folder(new DirectoryInfo(this).Parent.FullName);

    public IFolder Down(string folderName) => new Folder(Path.Combine(this, folderName));

    public void Create() => Directory.CreateDirectory(this);

    public void Empty()
    {
        var directoryInfo = new DirectoryInfo(this);

        if (!directoryInfo.Exists)
        {
            return;
        }

        foreach (var file in AllFiles)
        {
            File.Delete(file);
        }

        foreach (var folder in Folders)
        {
            Directory.Delete(folder.ToString(), true);
        }
    }

    public void CopyTo(IFolder destination) => this.CopyTo(destination, file => true);

    public void CopyTo(IFolder destination, Func<string, bool> filter)
    {
        // Create directories
        foreach (var folder in this.AllFolders)
        {
            Directory.CreateDirectory(folder.ToString().Replace(this, destination.ToString()));
        }

        // Copy all the files & replaces any files with the same name
        foreach (var filePath in this.AllFiles.Where(filter))
        {
            File.Copy(filePath, filePath.Replace(this, destination.ToString()), true);
        }
    }

    public void Run(string exe, string args = "")
    {
        var defaultCurrentDirectory = Environment.CurrentDirectory;

        Environment.CurrentDirectory = this;
        try
        {
            var process = new Process { StartInfo = { FileName = exe, Arguments = args } };

            process.Start();
            process.WaitForExit();
            if (process.ExitCode != 0)
            {
                throw new InvalidOperationException(
                    $"{exe} process failed with exit code {process.ExitCode}.");
            }
        }
        finally
        {
            Environment.CurrentDirectory = defaultCurrentDirectory;
        }
    }
}

And finally, the Folders.cs implementation:

internal class Folders : IFolders
{
    private readonly Assembly _Assembly;

    public Folders(Assembly assembly = null)
    {
        this._Assembly = assembly ?? Assembly.GetExecutingAssembly();
    }

    public static IFolders Default => new Folders();

    public IFolder Bin => new Folder(this._Assembly);

    public IFolder App => this.Bin.Up();

    public IFolder Docs => this.App.Down("Docs");

    public IFolder Temp => this.App.Down("Temp");
}

Now that it's not static, you'll either have to create a new one with a particular assembly, or Folders.Default will have the current assembly as per original design.


Suspend and resume a thread using signals The other day I ran into this question on stackoverflow.
Just for curiosity I wanted to implement suspending and resuming the thread, so I used signals and used pause() in the signal handler.

Is it OK to use this way? This question has many answers how to suspend/resume a thread, but I want to know what is wrong with my approach.

As far as I know signal handlers should be treated as interrupts i.e. minimum code and immediate return from it.

According to this, pause() is async-signal safe system call.

#include <stdio.h>
#include <unistd.h>
#include <pthread.h>
#include <signal.h>

// Since I have only 2 threads so using two variables,
// array of bools will be more useful for `n` number of threads.
static int is_th1_ready = 0;
static int is_th2_ready = 0;

static void cb_sig(int signal)
{
        switch(signal) {
        case SIGUSR1:
                pause();
                break;
        case SIGUSR2:
                break;
        }
}

static void *thread_job(void *t_id)
{
        int i = 0;
        struct sigaction act;

        pthread_detach(pthread_self());
        sigemptyset(&act.sa_mask);
        act.sa_flags = 0;
        act.sa_handler = cb_sig;

        if (sigaction(SIGUSR1, &act, NULL) == -1)
                printf("unable to handle siguser1\n");
        if (sigaction(SIGUSR2, &act, NULL) == -1)
                printf("unable to handle siguser2\n");

        if (t_id == (void *)1)
            is_th1_ready = 1;
        if (t_id == (void *)2)
            is_th2_ready = 1;

        while (1) {
                printf("thread id: %p, counter: %d\n", t_id, i++);
                sleep(1);
        }

        return NULL;
}

int main()
{
        int terminate = 0;
        int user_input;
        pthread_t thread1, thread2;

        pthread_create(&thread1, NULL, thread_job, (void *)1);
        // Spawned thread2 just to make sure it isn't suspended/paused
        // when thread1 received SIGUSR1/SIGUSR2 signal
        pthread_create(&thread2, NULL, thread_job, (void *)2);

        while (!is_th1_ready && !is_th2_ready);

        while (!terminate) {
                // to test, I am sensing signals depending on input from STDIN
                printf("0: pause thread1, 1: resume thread1, -1: exit\n");
                scanf("%d", &user_input);

                switch(user_input) {
                case -1:
                        printf("terminating\n");
                        terminate = 1;
                        break;
                case 0:
                        printf("raising SIGUSR1 to thread1\n");
                        pthread_kill(thread1, SIGUSR1);
                        break;
                case 1:
                        printf("raising SIGUSR2 to thread1\n");
                        pthread_kill(thread1, SIGUSR2);
                        break;
                }
        }

        pthread_kill(thread1, SIGKILL);
        pthread_kill(thread2, SIGKILL);

        return 0;
}

This is a useful question because it illustrates how a seemingly working program using pthreads can be loaded with pitfalls.  I hope that you take this answer in the spirit in which it is intended, which is to help you improve your programming.  I didn't see any major problems with the way you're using signals.

Consider using standard threads

You may already know this, but since C11, threads are now part of the standard.  Using them instead of POSIX threads (pthreads) could make the program, in theory anyway, somewhat more portable.  I'll make no further mention of that, however, and the rest will deal with pthreads, although most of the suggestions still apply.

Think carefully about using non-standard calls

The sigemptyset() call is neither a C nor a POSIX standard.  While it's widely supported, it isn't really portable, strictly speaking.  That may be fine for your purposes, but it's important to know which calls are standard and which are not to improve the portability of your program. (I was actually thinking of sigisemptyset here — my mistake!)

Use a mutex to access shared resources

Both threads are attempting to use printf but that's a problem because that requires access to a shared resource, namely stdout.  Access printf within POSIX threads are guaranteed to be thread safe, but the output could be scrambled (that is, the letters from two different threads could be intermixed).  One could use a mutex to assure that only one thread at a time is using that resource.  Here's one way to do that:

#include <stdarg.h>

static pthread_mutex_t stdout_mutex = PTHREAD_MUTEX_INITIALIZER;

static int locking_printf(const char *format, ...) {
    va_list args;
    va_start(args, format);
    pthread_mutex_lock(&stdout_mutex);
    int retval = vprintf(format, args);
    pthread_mutex_unlock(&stdout_mutex);
    va_end(args);
    return retval;
}

Then replace every instance of printf with locking_printf, including the ones in main since that's a thread, too.

Match return types with function

There's no need to write return NULL; at the end of thread_job because it's a void function.  Also, it's my view that one should omit return 0; from the end of main.  It's always contentious because others disagree (often very passionately!), but here's the rationale.

Use bool types where appropriate

The is_th1_ready and is_th2_ready variables are more appropriately described as bool rather than int variables.  For that reason, I'd recommend using <stdbool.h> and writing their declaration and initialization like this:

static bool is_th1_ready = false;
static bool is_th2_ready = false;

Think about casting

Creating the threads uses this kind of call:

pthread_create(&thread2, NULL, thread_job, (void *)2);

That's fine and appropriate.  However, the later use of that data within the thread is a bit unusual:

if (t_id == (void *)1)
    is_th1_ready = 1;

It's perfectly technically valid, but the more conventional approach is to cast the passed void * into something intelligible to the local thread -- often this is a pointer to a struct containing multiple values.  In this case, I'd probably write it like this instead:

if ((int)t_id == 1)
    is_th1_ready = true;

On my machine this causes a compiler warning because int and void * are not the same size (on this machine, with this compiler), but that's not a problem because we know with certainty that this is, in fact, an int.  Another approach would be to do the cast once in the creation of a local int variable.

Rethink the use of scanf

If the user enters a letter or some other non-numeric input, the scanf input fails and the user loses control of the application.  It's well known that scanf has problems.  For that reason, I'd recommend instead using fgets and then strtod.

char buf[5];  // we don't need a big buffer
fgets(buf, sizeof buf, stdin);
switch(strtol(buf, NULL, 10)) {

This will interpret any "garbage" input as 0, but that's probably OK for this program.  Note also that for POSIX systems, non-numeric input can be detected by looking at the errno value.

Prefer for to while where appropriate

The only use of terminate is within the loop, so I'd suggest changing from while to for like this:

for (bool running = true; running; ) {

Note that I've inverted the sense to eliminate the need to negate.  The compiler would probably have done something like that anyway, but I think it's more clear for human readers this way.

Don't bother killing threads at exit

The return from main or a call to exit() will both cause the threads to be cleaned up and resources returned, so I would probably omit the two pthread_kill calls at the end of main.

Consider more robust error handling

It's nice that the code is checking the return value of sigaction, but should the thread even launch if we can't handle the signals?  It's a design choice, but one worth considering, in my opinion.


Leaflet map distance from the onclick marker Here is the part of working code:

  function SelectPoints(lat,lon){
var dist = document.getElementById("miles").value;

xy = [lat,lon];  //center point of circle

var theRadius = parseInt(dist) * 1609.34  //1609.34 meters in a mile
//dist is a string so it's convered to an Interger.

selPts.length =0;  //Reset the array if selecting new points

  job.eachLayer(function (layer) {
    // Lat, long of current point as it loops through.
    layer_lat_long = layer.getLatLng();

    // Distance from our circle marker To current point in meters
    distance_from_centerPoint = layer_lat_long.distanceTo(xy);

    // See if meters is within radius, add the to array
    if (distance_from_centerPoint <= theRadius && $('#cf').is(":checked")) {
     selPts.push(layer.feature);
}
})

    job2.eachLayer(function (layer) {
    // Lat, long of current point as it loops through.
    layer_lat_long = layer.getLatLng();

    // Distance from our circle marker To current point in meters
    distance_from_centerPoint = layer_lat_long.distanceTo(xy);

    // See if meters is within radius, add the to array
    if (distance_from_centerPoint <= theRadius && $('#vm').is(":checked")) {
     selPts.push(layer.feature);
}
})

job3.eachLayer(function (layer) {
    // Lat, long of current point as it loops through.
    layer_lat_long = layer.getLatLng();

    // Distance from our circle marker To current point in meters
    distance_from_centerPoint = layer_lat_long.distanceTo(xy);

    // See if meters is within radius, add the to array
    if (distance_from_centerPoint <= theRadius && $('#bt').is(":checked")) {
     selPts.push(layer.feature);
}
})

but it looks like it's going to be repeatable over next few layers. So far is only 3 layers job, job2 and job3, what doesn't look bad, but I am worried about the future.

Does anyone knows how to make this code a bit shorter?

Thanks & Regards,

A short review;

selPts.length = 0; //Reset the array if selecting new points always resets the array, even if no points will be added, also selPts = [] is more idiomatic
Seems like xy is not declared there, and it probably should be. Nor is selPts
I feel center describes more accurately the purpose of xy
Idiomatic JavaScript uses lowerCamelCase, so layer_lat_long -> layerLatLong etc.
In the end I went with layerLatLng since you get it from a function called getLatLng
I only knew selPts was selectedPoints due to comments, I think you should spell it out completely
You can extract the common logic into a function and pass that function to .eachLayer()

Obligatory rewrite:

function SelectPoints(latitude, longitude) {
  var dist = document.getElementById("miles").value,
    theRadius = parseInt(dist, 10) * 1609.34; //1609.34 meters in a mile

  center = [latitude, longitude]; //center point of circle
  selectedPoints.length = []; //Reset the array if selecting new points

  function updateSelectedPoints(layer){
    // Lat, long of current point as it loops through.
    layerLatLng = layer.getLatLng();
    // Distance from our circle marker To current point in meters
    distanceFromCenter = layerLatLng.distanceTo(center);
    // See if meters is within radius, add the to array
    if (distanceFromCenter <= theRadius){
      selectedPoints.push(layer.feature);
    }
  }

  if($('#cf').is(":checked")){
    job.eachLayer(updateSelectedPoints);
  }
  if($('#vm').is(":checked")){
    job2.eachLayer(updateSelectedPoints);
  }
  if($('#bt').is(":checked")){
    job3.eachLayer(updateSelectedPoints);
  }
}


XOR two files together I am trying to XOR two files each 1Mb which contains only 0 and 1. It works, but I think it is very slow for the C++ program - approximately 4 sec.

Could someone, please, suggest the fastest XOR method on large files or at least tell what you think about XORing 1MB files running time? May 0.5sec or 1sec? or even less...
What should be my goal related to time?

PS
Yes, I did not read files by chunks or mapping them.

#include <iostream>
#include <fstream>
#include <vector>
#include <cstring>
#include <algorithm>
#include <iterator>
using namespace std;

inline std::vector<int> read_vector_from_disk(std::string file_path)
{
    std::ifstream instream(file_path, std::ios::in | std::ios::binary);
    std::vector<int> data((std::istreambuf_iterator<char>(instream)), std::istreambuf_iterator<char>());
    return data;
}

int main()
{
    // Open result file.
    fstream new_file;
    new_file.open("output.txt", ios::out | ios::app);
    if (!new_file)
    {
        cout << "File creation failed";
    }

    // Here we will store the result
    std::vector<int> xored_file;

    std::vector<int> in_data = read_vector_from_disk("first_arg.txt");
    std::vector<int> out_data = read_vector_from_disk("second_arg.txt");

    //creating iterator
    vector<int>::iterator first_iter = in_data.begin();
    vector<int>::iterator second_iter = out_data.begin();

    //printing all elements
    cout << "XOR-ed elements are: ";

    for (; first_iter != in_data.end() && second_iter != out_data.end(); first_iter++, second_iter++)
    {
        int xored = *first_iter ^ *second_iter;
        xored_file.push_back(xored);
    }

    // Print result.
    vector<int>::iterator out_iter = xored_file.begin();
    for (; out_iter != xored_file.end(); out_iter++)
    {
        new_file << *out_iter;
    }

    cout << "Complete";

    return 0;
}

#include <cstring>

seems unused.

using namespace std;

Not a good plan - just use the std:: qualifier, or import just the names you need, in the smallest reasonable scope.

inline std::vector<int> read_vector_from_disk(std::string file_path)
{
    std::ifstream instream(file_path, std::ios::in | std::ios::binary);
    std::vector<int> data((std::istreambuf_iterator<char>(instream)), std::istreambuf_iterator<char>());
    return data;
}

This is a very inefficient way to copy a file into memory - the vector will likely have to reallocate (copying its contents) several times, as the input iterators can't be used to predict the eventual size.

Two faster approaches would be

memory-map the file contents, probably using Boost::interprocess
work in a streaming fashion with character-by-character input (e.g. using std::transform())

Also, there's no check whether any of this reading succeeded at all.  That's bad.

std::vector<int> xored_file;

Again, we have a vector we'll append to without first reserving capacity.  That's reducing your efficiency.  I don't see why we need to store a copy of output, instead of immediately writing it to the output stream.

When we've finished writing, we should close the file and confirm that it was successfully written:

new_file.close();
if (!new_file) {
    std::cerr << "Failed to write output file\n";
    return EXIT_FAILURE;
}


Counting the number of words in a string using C I wrote a simple program to count the number of words in a string using C. Would you mind reading it and telling me if the method used is acceptable? Or if there are style errors or ways I can improve it?

 /*Procedure that counts the number of words in a string
    A word is any set of characters (or a single character)  separated
    from the word after it and before it by a space*

    For example, "a" is one word, "!" is one word, "peut-etre" is one word, "Builder Jack" are two words*/

    #include <stdio.h>

    int word_count(char *array){
        int count = 0; /*Number of words*/

        while(*array != '\0'){
            if(*array == ' '){
                count++;
            }
            array++;
        }
        return (count+1);
    }

    int main(void){
        char line[1000];

        puts("Enter text");

        fgets(line, sizeof(line), stdin);

        printf("Word count is %i", word_count(line));

        return 0;
    }

As has already been said, your code is not very robust and whilst it will work for some happy path scenarios, there is a lot of scope for error.

Variable initialisation

You don't initialise line.  Consequently it has an undefined value.  In local testing this isn't going to be a huge problem because you call fgets and read from stdin into the variable.  However, as indicated by @πάντα-ῥεῖ's example this isn't always the case.  It's very possible for calls to fgets to fail in which case line will contain whatever it is initialised to.

Unless you're really sure you don't need it, err on the side of caution and initialise your variables:

char line[1000]="";

Error Checking

As I've said, fgets can fail.  When it does, it returns NULL.  You're not currently checking the fgets call's return value.  In most cases when you're dealing with stdin, you shouldn't encounter errors so you can avoid the need to check the return value through correct variable initialisation.  However, if you're not going to initialise your variables it's a good idea to check if fgets returns a value to indicate that it has successfully read something.

End of line

fgets will read until the end of either the buffer size supplied or the end of line, whichever comes first.  If it reaches the end of the buffer (sizeof(line)), then the buffer will be NULL terminated.  If on the other hand
it reads to the end of line, the buffer will include an end of line character '\n' before the NULL termination.  At the moment, you're treating this end of line character as part of a word which is probably incorrect.

What's a word?

Currently your method is actually returning the number of spaces in the input string, plus 1.  This works for simple cases where you assume that you start with a word and any space will be followed by a new word.  However, there are lots of edge cases where this isn't going to be true.  I put a double space after a full stop, should that really count as two words?  If I have nothing but spaces in the string I would expect 0, that's not what I get etc.

It may be that for your usage, your definition of a word is enough, however it doesn't feel like the right definition.  It seems like groups of whitespace should be treated as a single word boundary, rather than always being treated as the start of a new word.  You have decided that punctuation on its own is a word, does that really make sense?

Whitespace Vs ' '

Currently, you're only recognising actual space characters as word separators.  This may be correct for you, however as suggested by @Incomputable you could use isspace to check for a wider range of whitespace characters.  This would make your code more robust and able to deal with other whitespace characters like tabs.

Output considerately

puts automatically appends a newline to the end of the string it writes to stdout.  This isn't the case for printf, you need to append it yourself.  You don't currently append a new line, so it's messy for the user.

Naming & Comments

Comments should tell you add useful insight into what's going on in your code.  The comment at the top of the file is good, it contains a description of your specification for a word.  This is useful, although it's in the wrong place.  It's a description of what the word_count method does, why isn't it directly above the method?

You also have this comment:

int count = 0; /*Number of words*/

If you give count a proper name, then you don't need the comment:

int wordCount = 0;

Indentation

Your file indentation seems a bit off.  It may be a copy/paste error, however I'd expect method signatures to start at the extreme left of the line, not floating one indentation in.  If it is a copy/paste error, in future, try to make sure your code in the question looks like it does in your editor.


Simple Python Hangman Prog I just started learning programming in Python and decided to write a game of hangman to try and learn more about how certain things worked. Any constructive criticism would be greatly appreciated.

import random
print('let\'s play hangman')
#set words
first = ['t','e','s','t']
second = ['t', 'e', 's', 't', 'e', 'd']
third = ['t', 'e', 's', 't', 'i', 'n', 'g']
fourth = ['r', 'a', 'n', 'd', 'o', 'm']
fifth = ['g',  'e',  'n',  'e',  'r',  'a',  't',  'e']
#set list of possible answers
alphabet = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm',
'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z']
#set a couple of varibles
answer = []
end = 0
right = 0
wrong = 0
endCheck = 0
alreadyGuessed = 0
#randomly pick a word
random=random.randint(1, 5)
if random == 1: words = first
if random == 2: words = second
if random == 3: words = third
if random == 4: words = fourth
if random == 5: words = fifth
#set varible length to the number of letters in the word
length = len(words)
#figure out what number must be reached in order to declare winner
for loop in range(length):
    end = end + loop
#set answer to as many asterisks as there are letters in the word
for marker in range(length):
    answer.append('*')
#start the main loop and end if win or lose
while wrong < 5 and endCheck != end:
    #print the answer withe asterisks in place of unknown letters
    print ('The word is ', end = '')
    for word in range(length):
        print(answer[word], '', end='')
    print('')
    #start loop to get answer and check if already guessed
    while alreadyGuessed < 1:
        #get guess
        letter = str(input('What is your guess? '))
        #make sure guess is in lowercase
        letter = str.lower(letter)
        #convert guess into ascii number and set to 0 for a, 1 for b...
        guess = ord(letter) - 97
        #check if letter already guessed
        if alphabet[guess] == ['*']:
            print('You already guessed that letter, try again')
            alreadyGuessed = 0
        #otherwise set letter to guessed and continue
        else:
            alphabet[guess] = ['*']
            alreadyGuessed = 1
    #reset guessed tracker
    alreadyGuessed = 0
    #if guess is right enter it into answer at correct spot and mark this
    turn as a right guess
    for marker2 in range(length):
        if words[marker2] == letter:
            answer[marker2] = letter
            endCheck = endCheck + marker2
            right = right + 1
    #check if entire word has been guessed
    if endCheck == end:
        print('You guessed it right before getting a hangman! You win!')
        print ('The word was ', end = '')
        for word in range(length):
            print(answer[word], end='')
    #check if a right letter was guessed this turn
    if right > 0 and endCheck != end:
            print('You got one right! no hangman for you this turn')
    #check if a wrong letter was guessed this turn
    if right == 0:
        print('You missed it this turn!')
        wrong = wrong + 1
        print ('number missed =',  wrong)
    #check if to many wrong guesses were made
    if wrong == 5:
        print('You got a hangman!')
    #reset right guesses to 0 before next turn
    right = 0
    #skip a space between turns
    print ('')

First, some comments on the UI:

The word is t e s t * n *
What is your guess? g
You got one right! no hangman for you this turn

The word is t e s t * n g
What is your guess? i
You guessed it right before getting a hangman! You win!
The word was testing

You need to do a better job separating the "game board" from the verbiage. Also, maybe put some quotes and other punctuation around the printed answer:

The word was, "testing"!

Now for the code. I'm not going to do a complete review because ... it makes my eyes burn. Instead, I'm going to assume that:

You're coming to python from C or C# or Java - some language with array support and lots of curly braces.
You're in a course, and being deliberately held back.

So let's look at your coding style:

import random
print('let\'s play hangman')
#set words
first = ['t','e','s','t']
second = ['t', 'e', 's', 't', 'e', 'd']
third = ['t', 'e', 's', 't', 'i', 'n', 'g']
fourth = ['r', 'a', 'n', 'd', 'o', 'm']
fifth = ['g',  'e',  'n',  'e',  'r',  'a',  't',  'e']
#set list of possible answers
alphabet = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm',
'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z']
#set a couple of varibles

Use vertical space to separate unrelated things

You didn't separate things using vertical space. This is perhaps the greatest sin in programming. Always, always use vertical space (blank lines, paragraphs, etc.) to separate parts of your code. If you switch from doing one thing to another, insert a blank line. If you end a paragraph and start a new paragraph, insert a blank line. If you come to the end of a function or a loop, insert a blank line.

Even if you are the most secretive hacker ever, and you never intend to share your code with a single other living human being, at some point you will have to go back and read stuff that you, yourself wrote six months earlier! Make it easy on yourself, or on whatever other humans have to read your code: try to make it easy to read! Use vertical space to separate unrelated things.

Keep related things together

Next, consider the line:

print('let\'s play hangman')

Ignoring the lack of a leading capital letter, and no ending punctuation, why is this line where it is?

This is clearly the "start of the game." That's fine. But at line #2, you aren't ready to start the game. There's a bunch of variable initialization and other things that need to happen before the game starts. This line should be moved down to where all the rest of the user-facing code is located. Keep related things together.

Let your code speak for itself

Your comments suck.

There are six basic questions you can answer: Who, What, Where, When, Why, and How?

Of those, three are only used in intro-to-programming courses: Who, Where, When. If you have to put a big header block at the top of your program with your student id, name, course number, professor's name, etc., that's okay. Just do it once, and we will all skip right over it!

The other three- What, Why, and How- should only be used if you can't actually answer the question with the code itself!

Specifically, if you can't choose your variable, type, and subroutine names in such a way as to make the answer obvious.

If you can make the answer obvious, then don't add a comment. It just wastes my time, and annoys the compiler.

To wit:

#set varible length to the number of letters in the word
length = len(words)

First, you misspelled "variable." But second: how could you possibly think this comment adds value to your source code?

There are some occasions when you need to answer the question "What?" But those occasions are few and far between. You're actually better off never answering that question unless somebody calls you on it. Until and unless someone asks you "what is this code doing?" I would suggest you simply never use a comment to explain what you are doing - because at the "learning" stage, it is almost always obvious.

You might explain "why" you're computing some value, or "how" you're doing a computation. But, again, for the most part these things are obvious.

Now consider your variable, words:

first = ['t','e','s','t']

# ...
random=random.randint(1, 5)
if random == 1: words = first

Apparently, words is actually the secret word the player is trying to guess. Which is fine, except the whole program is about words, and words is plural - suggesting perhaps a list of words or a dictionary of words or a disk file containing more than one actual word!

This variable is the most important piece of data in your program, and you named it wrong. Maybe secret_word instead? (Because answer is taken.)

There are very few beginner programs that need comments. But there are often programs that need better variable or function names. Let your code speak for itself.

K.I.S.S.

Consider this:

for loop in range(length):
    end = end + loop

# ...

while wrong < 5 and endCheck != end:

    # ...

    for marker2 in range(length):
        if words[marker2] == letter:
            # ...
            endCheck = endCheck + marker2

This is "interesting" because it appears that you determine that the player has won the game by adding up the index of all the correctly guessed letters, 0 + 1 + 2 + 3 + 4 ... and matching the sum against a pre-computed value.

This is so bizarre that I wonder if you copied your solution from a different language. Perhaps in C, or Pascal, or assembly, or just any language with bad string support? Also, of course, it won't work: you start with zero, and you use 0 as the index of the first character. This is a bug.

The word is * e n e r a * e
What is your guess? t
You guessed it right before getting a hangman! You win!
The word was *enerate

Ignoring the whole "it doesn't work" thing, this solution is vulnerable to all sorts of programmer errors, and it is not at all obvious what you're doing.

A far better test for "done" would be simple equality: is A equal to B? It turns out that Python implements list equality correctly. You can simply say,

if listA == listB:

and get the correct result. Alternatively, you could store your words as strings rather than lists. This would make it somewhat awkward to manipulate them, since Python strings are immutable. But it would again let you simply say,

if wordA == wordB:

or even,

while wordA != wordB:

and have a simple, obvious operation for checking if the player has won.
As the saying goes, "keep it simple stupid."

A place for everything...

Most of your code is inside a while loop. But much of your code doesn't get executed more than once!

Consider this:

#check if entire word has been guessed
if endCheck == end:
    print('You guessed it right before getting a hangman! You win!')
    print ('The word was ', end = '')
    for word in range(length):
        print(answer[word], end='')
#check if a right letter was guessed this turn
if right > 0 and endCheck != end:
        print('You got one right! no hangman for you this turn')
#check if a wrong letter was guessed this turn
if right == 0:
    print('You missed it this turn!')
    wrong = wrong + 1
    print ('number missed =',  wrong)
#check if to many wrong guesses were made
if wrong == 5:
    print('You got a hangman!')

That code is a mixture of "things to do at the end of the game" and "things to do in a loop."

Mixing things like that is a mistake. First, because it requires you to check your conditions over and over (which is inefficient). And second because you are flipping back and forth: he's in! he's out! he's in! he's out! This is unnecessary complexity (violating the KISS principle), and it doesn't keep related things together.

Instead of jumbling all these conditions together, ask yourself: what things go inside the loop, and what things go outside the loop?

Obviously, winning or losing the game is "outside the loop" because in either case you won't be asking for any more guesses.

Similarly, reporting if a single guess was right or wrong is inside the loop, because then you will ask for more guesses.

Consider using the break or the continue statement to short-circuit the rest of the loop body. Like this:

while True:
    # guess, etc.
    if answer == secret_word or wrong_guesses == 5:
        break

if answer == secret_word:
    # you win
else:
    # you lose

Identify the various conditions, and then have "A place for everything, and everything in its place!"

I'm not going to address the places where your code is not "Pythonic". You've said you're learning, and I believe you. Plus, I expect some other people will chime in for that. There are a lot of things you can do to clean this code up, and to make better use of language features. But I think you should address the "bigger" issues first, because they will still be true even if you aren't programming in Python. Bad comments in C# are still bad comments. Poor variable names in Scheme are still poor variable names.


Listing all directories, sub directories and files starting from the root I have to deploy this code that will list all the directories, sub directories and files in it starting from the root. The code works but I am not sure if this is the correct way to list. It should not fail, though.

Also, in despite not allowing directories with the name . and .., they get printed. Why is that?

<?php

# Snippet that lists all the directories,sub directories and files under that directory
# recursive function

function directory_f_lister($root) {
    $dir_list = scandir($root);
    for($var=0;$var<count($dir_list);$var++) {
        if(is_readable($root.$dir_list[$var])) {
            if(is_dir($root.$dir_list[$var])) {
                if($dir_list[$var] === "." || $dir_list[$var] === "..") continue;
                echo "
Name of directory $dir_list[$var]
";
                echo "
";
                $dh = opendir($root.$dir_list[$var]);
                while(($name = readdir($dh)) !== false) {
                    if(is_dir($root.$dir_list[$var].$name)) {
                        if($dir_list[$var] === "." || $dir_list[$var] === "..") continue;
                        echo "Name of directory :  $name ";
                        echo "
";
                        directory_f_lister($root.$dir_list[$var].$name);
                    }else {
                        echo $name;
                        echo "
";
                    }
                }
            }
        }
    }
}

directory_f_lister(DIRECTORY_SEPARATOR);
#end

well . and  .. are not directories, thats are navigationpoints of directories, . means go inside a root directory, .. means go one folder back,

you might noticed this already in includes like include ../../../foo/bar.php

also instead of scandir and readdir php has classes for this purpose DirectoryIterator or FileSystemIterator

And if you wish to create reusable methods, dont do output there, instead return an array of folders/files something like

$directories = scanDirectories($rootDirectory); //create output array
foreach($directories as $directory){
    echo $directory.'
';
}

this allowes you to modify the output without editing the sourcecode, you could also wrap the output within a function e.g. viewDirectories and reuse it on different places.

here is a small Example

    ini_set('html_errors', 'On');

function scanDirectory($path) {
    $path = realpath($path);

     return  new RecursiveIteratorIterator(new RecursiveDirectoryIterator($path), RecursiveIteratorIterator::SELF_FIRST);

}

foreach(scanDirectory('./') as $name=> $object){

    echo $name.'
';
}

About your question:

$root.$dir_list[$var].$name //this is your directory name

but youre checking if this name is . and ..

$dir_list[$var]

so its never becomes true


Python Socket Receiver I put this together and am having some doubts to the number of connections I can make. I also doubt if I can brute force the server to serve up connections and then hang.

Please take a look and tell me what I can do better.

'''
    Simple socket server using threads
'''

import socket
import sys
from thread import *

HOST = '192.168.0.102'   # Symbolic name meaning all available interfaces
PORT = 8887 # Arbitrary non-privileged port

s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
print 'Socket created'

#Bind socket to local host and port

try:
    s.bind((HOST, PORT))
except socket.error as msg:
    print 'Bind failed. Error Code : ' + str(msg[0]) + ' Message ' + msg[1]
    sys.exit()

print 'Socket bind complete'

#Start listening on socket
s.listen(10)

print 'Socket now listening'

#Function for handling connections. This will be used to create threads
def clientthread(conn):

    #infinite loop so that function do not terminate and thread do not end.
    while True:

        #Receiving from client
        data = conn.recv(1024)
        print data

        reply = 'OK...' + data + str(addr)
        if not data:
            break

        conn.sendall(reply)

    #came out of loop
    conn.close()

#now keep talking with the client
while 1:
    #wait to accept a connection - blocking call
    conn, addr = s.accept()
    print 'Connected with ' + addr[0] + ':' + str(addr[1])

    #start new thread takes 1st argument as a function name to be run, second is the tuple of arguments to the function.
    start_new_thread(clientthread ,(conn,))

s.close()

This code is very nice and easy to follow along with.

You should check the return value of socket.socket to make sure that the socket was created successfully

Usually, the name given to an error in a try/catch(or except) statement is something along the lines of "err", not "msg".

You should pass an error code to sys.exit so that external programs will know if there were any errors during the execution of the program.

In the while loop of clientthread, you use True to specify an infinite loop.

However, in the while loop just below that function, you use 1. You should stay consistent and use True for that one.

Instead of just printing the data, add a short message before it so the purpose of the number is more easily understood in output.

Example:

print 'Client data: ' + data

I don't believe that the s.close call at the end of your code will actually run.

This may not be necessary, as the socket may close automatically, but I would set up a signal handler to close the socket when someone tries to exit the program. You can use the signal library.

I came up with something like this:

def close_socket():
    s.close()
    sys.exit(0) # success

Then, right before your final while loop:

signal.signal(signal.SIGINT, close_socket)


Finding the maximum of an array to solve a rope-burning puzzle I am trying to get the maximum value out of this array.

There is a long rope of length totalLength at its each unit length some ropes are tied pointing in upper and lower directions with their length given. I am saving them in upper and lower arrays. Question is if we light up the front end how much time it will take to burn the whole rope. The fire spreads at 1m/s.

These two arrays upper and lower they have some value and the I am adding the max of them in each iteration and at the end I am finding the max value.

My idea is the total time taken will be equal to burning the longest part of the rope. I am finding the longest part and displaying it. It seems like this code is not good enough and is not clearing the time limits.

import java.util.*;

class TestClass {
    public static void main(String args[] ) throws Exception {

        Scanner scan = new Scanner(System.in);
        int N = scan.nextInt();//total test cases;
        int totalLength = 0;
        int maxSum = 0, maxUL = 0;
        int[] upper;
        int[] lower;
        int[] result;
        while(N-- > 0) {

              maxSum = 0;

              totalLength = scan.nextInt();// total length of a base graph
              upper = new int[totalLength - 1];
              lower = new int[totalLength - 1];
              result = new int[totalLength - 1];
              for(int i = 0; i < totalLength - 1; i++){
                upper[i] = scan.nextInt();
              }

             for(int i = 0; i < totalLength - 1; i++){
                lower[i] = scan.nextInt();
              }

              for(int i = 0; i <= totalLength - 2; i++){
                   maxUL = (upper[i] > lower[i]) ? upper[i]: lower[i];
                   maxSum = i+1 + maxUL;
                   result[i] = maxSum;
              }

              int maxVal = 0;
              for(int i = 0; i < result.length; i++){
                if(maxVal < result[i]){
                    maxVal = result[i];
                }
              }
              System.out.println(maxVal);
        }
    }
}

Sample Input

1 // for the test cases
5 // the totallength of the main rope
1 0 2 2
1 1 1 0

Output

6

Space complexity

In terms of space, you can solve this in \$O(1)\$ space.
In other words, you don't need upper, lower, and result arrays.
You can calculate the rolling max while parsing the values of upper and lower without storing them.

Time complexity

You are performing \$N\$ operations several times, where \$N\$ is the total length - 1:

scan the elements of upper
scan the elements of lower
find the max of upper - lower pairs
find the max of the max values

That looks like \$O(4N)\$. If you rewrite as I suggested, without extra storage and calculating the rolling max as you go,
this can be reduced to \$O(2N)\$, since only the first two traversals remain, which is inevitable to read the input.

That being said,
since \$O(4N)\$ and \$O(2N)\$ are of the same magnitude (linear),
I'd be surprised if this made a difference for the time limit exceeded result.
Usually TLE happens when there is a magnitude of difference between the expected solution and the actual (for example quadratic instead of linear).

Extra tips

Use an enhanced for loop when you don't need the value of the index.
For example instead of:

          for(int i = 0; i < result.length; i++){
            if(maxVal < result[i]){
                maxVal = result[i];
            }
          }

Write as:

          for(int val : result) {
            if(maxVal < val){
                maxVal = val;
            }
          }

These loop conditions are identical, but written differently:

         for(int i = 0; i < totalLength - 1; i++){
         // ...
         for(int i = 0; i <= totalLength - 2; i++){
         // ...

It's easier to read if you adopt a more consistent writing style.

Instead of doing all the work in the main method,
it would be better to decompose this to multiple methods.
In the decomposed solution you should have a method that takes some input (possibly even a Scanner),
and return some output instead of printing it directly.
When written that way,
unit testing becomes possible.
And then of course, add some unit tests.


Language switch based on parameters and session state I use this to set language session for navigation. Someone said that this switch is too cumbersome. Why? Is there an easier way?

$defaultLang = 'it';

if (!empty($_GET["lang"])) {
    switch (strtolower($_GET["lang"])) {
        case "en":

            $_SESSION['lang'] = 'gb';
            break;
        case "tr":

            $_SESSION['lang'] = 'tr';
            break;
        default:

            $_SESSION['lang'] = $defaultLang;
            break;
    }
}

if (empty($_SESSION["lang"])) {
        $_SESSION["lang"] = $defaultLang;
}

Yes.  Definitely.  You can use a lookup array.

$langs = [
    'en' => 'gb',
    'tr' => 'tr',
];

$defaultLang = 'it';

if (isset($_GET['lang'])) {
    $lang = strtolower($_GET['lang']);
    $_SESSION['lang'] = $langs[$lang] ?? $defaultLang;
} elseif (!isset($_SESSION['lang'])) {  // meaning, no $_GET['lang'] and no $_SESSION['lang']
    $_SESSION['lang'] = $defaultLang;
}
// if there is no $_GET['lang'] and there is a $_SESSION['lang'], then nothing to update

A lookup array is concise and a breeze to maintain.  You only need to update the lookup or the fallback value; never the processing block.

?? is the null coalescing operator, so if the lang value is not found in the lookup, the fallback value will be used.

If your php version is not over 7 (then I urge you to upgrade) then you will need a longer condition syntax.

$_SESSION['lang'] = isset($langs[$lang]) ? $langs[$lang] : $defaultLang;


Rhythmic task controller The purpose of a rhythmic task controller is to accept a task and execute all the queued tasks in a cycle or "rhythm" defined by the client code. A task returns true or false depending on whether or not it is "finished". A finished task should be removed from the controller's control and won't be executed again while an unfinished task should be executed on the next cycle.

I don't think my solution is as well optimized as it could be, and this is a solution that needs to be as fast as possible as it serves as the core task controller in my game framework!

import static java.util.concurrent.Executors.newSingleThreadScheduledExecutor;

import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;

public final class RhythmicTaskController implements TaskController, Runnable {

    private final ExecutorService service;
    private final ConcurrentLinkedQueue<Task> queue = new ConcurrentLinkedQueue<>();

    public RhythmicTaskController(int threads, int delay, TimeUnit unit) {
        service = Executors.newFixedThreadPool(threads);

        newSingleThreadScheduledExecutor().scheduleWithFixedDelay(this, delay, delay, unit);
    }

    @Override
    public void offer(Task task) {
        queue.offer(task);
    }

    @Override
    public void run() {
        int initial = queue.size();
        ConcurrentMap<Future<Boolean>, Task> futures = new ConcurrentHashMap<>(initial);
        queue.forEach(task -> futures.put(service.submit(task::finish), task));
        futures.forEach((r, task) -> {
            try {
                if (r.get())
                    queue.remove(task);
            } catch (Exception e) {
                e.printStackTrace();
            }
        });
    }

}

You could remove all elements from the queue and then offer them back if necessary.

public void run() {
    ArrayList<Task> tasks = new ArrayList<>();
    {
      Task task;
      while((task = queue.poll()) != null) {
        tasks.add(task);
      }
    }
    for(Task task: tasks) {
      service.submit(() -> {
        if(!task.finish()) offer(task);
      });
    }
}


Table component for scrolling through lines of data I'm working on a table component that is able to scroll through millions of lines of data. Not bad for the front end. The way I'm doing this is to only render a small table with 10 rows and then, when the user scrolls, I change the data in that table.

The table itself is not in a scrollable div because the entire html element fits on the page. This is working OK, but it means that the scroll on the page is not disabled the way it would be if the table were in a scrollable div.

I really don't like how I have to hide the overflow on the body when the mouse enters the component to stop the scroll event from moving the whole page instead of just the data in the component.

import Ember from 'ember';
import layout from '../templates/components/table';

let TableComponent = Ember.Component.extend(
  {
    tagName: 'table',
    layout: layout,
    firstRow: 0,
    lastRow: 10,
    isScrollEnabled: false,

    maxRow: Ember.computed('content',
      function () {
        return Ember.get(this, 'content.length');
      }
    ),

    setup: function () {
      this.$().on('mousewheel', Ember.run.bind(this, this._didScroll));
      this.$(window).on('focus', Ember.run.bind(this, this._focus));
      this.$(window).on('blur', Ember.run.bind(this, this._blur));
    }.on('didInsertElement'),

    tearDown: function () {
      this.$().off('mousewheel', Ember.run.bind(this, this._didScroll));
      this.$(window).off('focus', Ember.run.bind(this, this._focus));
      this.$(window).off('blur', Ember.run.bind(this, this._blur));
    }.on('willDestroyElement'),

    visibleContent: Ember.computed('content', 'firstRow', 'lastRow',
      function () {
        let content = Ember.get(this, 'content');
        let firstRow = Ember.get(this, 'firstRow');
        let lastRow = Ember.get(this, 'lastRow');

        return content.slice(firstRow, lastRow);
      }
    ),

    _didScroll(event) {
      if (!Ember.get(this, 'isScrollEnabled')) {
        return;
      }
      if (event.originalEvent.wheelDelta >= 0) {
        // scroll up but not past first row
        if (Ember.get(this, 'firstRow') > 0) {
          this.decrementProperty('firstRow');
          this.decrementProperty('lastRow');
        }
      } else {
        // scroll down but not past last row
        if (Ember.get(this, 'lastRow') < Ember.get(this, 'maxRow')) {
          this.incrementProperty('firstRow');
          this.incrementProperty('lastRow');
        }
      }
    },

    _focus() {
      Ember.set(this, 'isScrollEnabled', true);
    },

    _blur() {
      Ember.set(this, 'isScrollEnabled', false);
    },

    mouseEnter() {
      Ember.$('body').css('overflow', 'hidden');
    },

    mouseLeave() {
      Ember.$('body').css('overflow', 'auto');
    }
  }
);

export default TableComponent;

The mouseEnter and mouseLeave functions are my big concerns.

hide the overflow on the body when the mouse enters the component to stop the scroll event from moving the whole page instead of just the data in the component

Isn't that already the default behavior of scrollable divs? Well, except the page doesn't set overflow:hidden on itself. Once your cursor is in a scrollable area, scroll will transfer there. When it reaches the end, the page scroll will pick up the rest of your scroll (really apparent when answering in SE with a really tiny textarea).

So I believe you're preventing that "page picking up after the end" thing. If so, you don't need to disable scrolling for the component. Once you hover over it, just disable the page scroll and enable when you move out.

Now for your code, you're inconsistently doing the "egyptian" (collapsed, K&R) brackets. Well, the problem is the inconsistency, not the fact that you're using one or the other. Just choose one style and stick with it.

maxRow: Ember.computed('content', function () {
  return Ember.get(this, 'content.length');
}),

let TableComponent = Ember.Component.extend({
  tagName: 'table',
  layout: layout,
  ...

To make these checks easier, try configuring your IDE to tell you. I believe most IDEs/text editors have plugins for eslint, jshint, and jslint.


Getting death row inmates' last statement Initial version of the code appeared as an answer to an SO question. I refactored the code a bit and it works pretty well, IMHO. I get a solid .csv file with all the data from Texas Department of Criminal Justice's Death Row page.
What I was especially interested in was getting all offenders' last statement, if there was any, which the codes accomplishes.
What I'd want to get here is some feedback on utilizing pandas, as I'm relatively new to it. Also, some memory efficiency suggestions would be nice too, I guess.
For example, should I save the initial version of the .csv file and then read it so I can append the last statements? Or keeping everything in memory is fine?
If you find any other holes, do point them out!
The code:
import random
import time

import pandas as pd
import requests
from lxml import html

base_url = "https://www.tdcj.texas.gov/death_row"
statement_xpath = '//*[@id="content_right"]/p[6]/text()'

def get_page() -> str:
    return requests.get(f"{base_url}/dr_executed_offenders.html").text

def clean(first_and_last_name: list) -> str:
    name = "".join(first_and_last_name).replace(" ", "").lower()
    return name.replace(", Jr.", "").replace(", Sr.", "").replace("'", "")

def get_offender_data(page: str) -> pd.DataFrame:
    df = pd.read_html(page, flavor="bs4")
    df = pd.concat(df)
    df.rename(
        columns={'Link': "Offender Information", "Link.1": "Last Statement URL"},
        inplace=True,
    )

    df["Offender Information"] = df[
        ["Last Name", 'First Name']
    ].apply(lambda x: f"{base_url}/dr_info/{clean(x)}.html", axis=1)

    df["Last Statement URL"] = df[
        ["Last Name", 'First Name']
    ].apply(lambda x: f"{base_url}/dr_info/{clean(x)}last.html", axis=1)
    return df

def get_last_statement(statement_url: str) -> str:
    page = requests.get(statement_url).text
    statement = html.fromstring(page).xpath(statement_xpath)
    text = next(iter(statement), "")
    return " ".join(text.split())

def get_last_statements(offenders_data: list) -> list:
    statements = []
    for item in offenders_data:
        *names, url = item
        print(f"Fetching statement for {' '.join(names)}...")
        statements.append(get_last_statement(statement_url=url))
        time.sleep(random.randint(1, 4))
    return statements

if __name__ == "__main__":
    offenders_df = get_offender_data(get_page())
    names_and_urls = list(
        zip(
            offenders_df["First Name"],
            offenders_df["Last Name"],
            offenders_df["Last Statement URL"],
        )
    )
    offenders_df["Last Statement"] = get_last_statements(names_and_urls)
    offenders_df.to_csv("offenders_data.csv", index=False)

The scraping part is intentionally slow, as I don't want to abuse the server, but I do want to get the job done. So, if you don't have a couple of minutes to spare, you can fetch the offenders_data.csv file from here.

Types
Here:
def clean(first_and_last_name: list) -> str:

list should more specifically be List[str].
Fluent syntax
Consider using fluent syntax for your clean, which - in this case - is basically just a reformat:
    return (
        "".join(first_and_last_name)
        .replace(" ", "")
        .replace(", Jr.", "")
        .replace(", Sr.", "")
        .replace("'", "")
        .lower()
    )


Password string generation I'm trying to create passwords using the following snippet to match a custom password policy. I've added three gtr statements, at the end of the string, to guarantee my password policy is met.

foo=$(gtr -dc 'a-zA-Z0-9' < /dev/urandom | head -c 96;
gtr -dc 'A-Z' < /dev/urandom | head -c 1;
gtr -dc 'a-z' < /dev/urandom | head -c 1;
gtr -dc '0-9' < /dev/urandom | head -c 1);
echo $foo

Could this code be simplified?

I don't understand why you need GNU tr for this task. The macOs tr should work equally well.

For passwords, you should use /dev/random instead of urandom.

It's a funny password policy if it really requires the password to be 99 characters long. At that length, it doesn't matter anymore whether the password contains a digit or not because such a long password, if generated by a strong random generator, cannot be guessed.

The code in itself is fine. I would indent it differently, though, lining up all four calls to tr. Writing a function to eliminate the code duplication is probably overkill.