Dictionary-based dispatch in Python with multiple parameters

You could change it to just store the functions and the arguments.  Then use the argument unpacking to fill in the correct arguments to the function call.

action = 'op2'
functions = {
    'op1': (printf, ('op1 called',)),
    'op2': (printf2, ('op2 called', 'param 2')),
}

fn, args = functions.get(action, (printn, ('default op',)))
fn(*args) # call the function unpacking the provided arguments

A better solution in this particular case IMHO, rather than defining multiple functions with varying number of arguments, you could just define one function accepting variable arguments to achieve the same thing.

def printf(*args):
    if args:
        for arg in args:
            print arg
    else:
        print 'default op'

Then it's just a matter of storing only the arguments to the call rather than the functions themselves:

action = 'op2'
arguments = {
    'op1': ('op1 called',),
    'op2': ('op2 called', 'param 2'),
}

printf(*arguments.get(action, ()))


First, Welcome to code review, and a very nice question for a beginning self learner in C++.

user1118321 is correct about a everything he says, I'm going to emphasis one or two items and provide some references before I mention a few other things.

Use Functions:
Functions allow one to break up their code into smaller, easier to understand logical blocks. When
designing software it is often easier to break a complex problem into smaller problems that are
easier to program. Some people use flow charts to map out their program, and high level flow
charts will only contain the functions. These will be followed by lower level flow charts that
map out the logic of each function.

Using functions allows one to decrease the amount of code written in some cases.

The main() function should be primarily used to set up the environment for the rest of the program,
call a function to execute the rest of the program and catch any errors.

If you find yourself repeating code move that code to a function so it only needs to be written and
debugged once. This is actually a programming principle generally referred to as Don't Repeat Yourself
and you may see it referred to here on code review as DRY.

Both functions and classes should follow the Single Responsibility Principle (SRP). The Single Responsibility
Principle is that a function should do one thing, and one thing only. This makes code easier to write,
debug and maintain. It also makes the function easier to reuse.
The Single Responsibility Principle may also be a reference to Demeter's Law (I may be wrong about them being the same principle).

Another programming principle sometimes mentioned is Keep It Simple (KISS), although this isn't just for
programming.

You may want to look into SOLID programming as well.

A more comprehensive list of programming principles can be found here.

Use Existing Classes
A long time ago when I started using C++ the Standard Template Library (STL), sometimes called the
Standard Library, had not yet been defined or implemented. We had to create our own strings, queues, vectors,
stacks and other container classes. Using the C++ container classes and the standard library requires
A Lot Less Code and makes it much simpler to write correct software. Two great website to become familar
with if you are going to write C++ code are CPlusPlus and the C++ reference.

These websites are better
than using a book as a reference because it remains up to date, and documents the C++11, C++14 and C++17
standards all in one place.

The string container class provides one with much more functionality then cstrings.

The follow is new content not previously mentioned.

Magic Numbers
The term magic number is sometimes used for numeric constants in code that are not obvious. By Not Obvious I mean that it's not obvious what the number represents. Non-symbolic references to numbers
should be replaced either by named constants, or enums (enumerator types). An example of a named
constant is:

constexpr double PI = 3.1415926;

Now in your code below, it is somewhat obvious what 1 and 2 are, but generally switch statements use enums or
symbolic constants.

    std::cout << "\nPress 1 to display the data:";
    std::cout << "\nPress 2 to search for a company";
    cin >> option;

    switch(option)
    {
        case 1:
           for(count = 0; count < num; count++)
           {
               company com1[count];
               com1[count].displaydata();
           }
           break;

        case 2:
            for(count = 0; count < num; count++)
            {
                char input[10];
                company com1[count];

                std::cout << "\nPlease enter the company type:";
                cin.getline(input, 10);

                if(strcmp(input,com1[count].companytype) == 0)
                {
                    com1[count].displaydata();
                }

            }
            break;

        default:
            std::cout<<"\nError: terminating program...";
    };

Please note that the default case should probably have a break; statement as well, especially if is
embedded within another control statement such as a loop.

Readability and Maintainability
Making the code more readable makes it more maintainable by you or anyone else that needs to add functionality.

The code would be much more readable if it was consistently indented as I show above in the included section
of the code. Blocks of code within { and } should be indented.

Operators in control statements such as for() loops, while loops and if statement should be separated by
spaces.

Use vertical spacing to make the code easier to read as well.

In C++ and more modern languages it is more common to create variables as you need them. I would change the above
code to

    std::cout << "\nPress 1 to display the data:";
    std::cout << "\nPress 2 to search for a company";

    int option;
    cin >> option;

This allows the maintainer to know the type of variable without jumping all over the code.

BUGS
This code may work due to the specific compiler you are using, but the fact is that you create the
company array at least 3 times. Once in every loop that I see. The variable com1 should be created
once at the top of main where count, num and option are created.

company com1[count];

The variable com1 should probably be created as
    vector com1;

This would provide a variable sized array of companies. The code for inputing the companies wouldn't change,
but the code for displaying a company or searching for a company can change:

for (auto thiscompany: com1)
{
    thiscompany.displaydata();
}

You could use the find_if() function instead of the current search.


Your code is not Pythonic as it does not adhere to the standard Python style guide.

divisibleSumPairs should be snake case divisible_sum_pairs.
You should have one space both sides of most operators. i=0 i+=1, ar[i]+ar[j] are all harder to read than they need to be.
Most of your variable names don't describe what they contain.
You print inside a function that should return.

def divisible_sum_pairs(length, divisor, items):
    i = 0       #pointer 1
    j = 1       #pointer 2 is faster than p1
    count = 0   #pointer 3 will only increment 1 once j = n-1 (second last element)
    pairs = 0
    while count < length - 1:
        if (items[i] + items[j]) % divisor == 0:
            pairs += 1
        j += 1
        count += 1
        if count == length - 1:
            i += 1
            j = i + 1
            count = i
    return pairs

You don't need pointer 3 as count = j + 1:

j += 1
count += 1

j = i + 1
count = i

We can just replace count with j - 1.
We can simplify all of the places where count was being used.
This is as you have - 1s  on both sides of the operators.
Rather than while j < length it would be clearer if you instead used while i < length - 1.
This comes in two parts:

It is confusing to see while j < length with if j == length in the body.
You're not actually bound by j you're bound by i.

Rather than using a while loop we can see that there's an opportunity to use two for loops to make things easier to read.

Note: These for loops have the exact same time complexity as your while.

def divisible_sum_pairs(length, divisor, items):
    pairs = 0
    for i in range(length - 1):
        for j in range(i + 1, length):
            if (items[i] + items[j]) % divisor == 0:
                pairs += 1
    return pairs

We can simplify the code by using using a comprehension.

For example we can extract getting the combinations of items.

combinations = (
    (items[i], items[j])
    for i in range(length - 1)
    for j in range(i + 1, length)
)
for a, b in combinations:
    if (a + b) % divisor == 0:

We can instead sum with a comprehension that generates numbers.

pairs = sum(
    1
    for a, b in combinations
    if (a + b) % divisor == 0
)

We can exploit the fact that bools are integers and move the if's expression as the comprehension's expression.
We can use itertools.combinations to remove the need to manually get the combinations.

import itertools

def divisible_sum_pairs(_, divisor, items):
    return sum(
        (a + b) % divisor == 0
        for a, b in itertools.combinations(items, 2)
    )

Every change has been a 1 to 1 replacement. The time complexity is still \$O(\binom{n}{2})\$ (\$O(n^2)\$) and memory complexity is still \$O(1)\$.

However now readability is much better.


Calculate SHA1 hash from binary and verify with a provided hash

I'm going to tell you the things in your code that made me wonder.

# Script that verifies the correctness of a toolbar offers

import urllib, bencode , hashlib

Extra space after bencode suggests lack of attention to detail.

from hashlib import sha1

url = "http://update.utorrent.com/installoffer.php?offer=conduit"

filename = "content.txt"

f= urllib.urlretrieve(url, filename)

Use of single-letter variables suggests lack of attention to clean code.

#Bdecoding the response

with open (str(filename), 'rb') as myfile:

filename is already a string.  Converting it to a string again suggests confusion as to what is going on.

           data=myfile.read()
           decoded = bencode.bdecode(data)

Massive indentation suggests disregard for consistent indentation style.

Downloading the file to disk then reading it into memory is poor use of available APIs. You should download the file directly into memory.

# Returning the list for the key 'offer_urls'

Excess whitespace suggests someone is enter-happy.

list = decoded['offer_urls']

"Returning" is technically incorrect here. You aren't in a function, so you aren't returning anything. Use of list is a bad name for a variable since it's a built-in Python type.

#print list

# Returning the URL of the binary that is prefixed with "http://download3.utorrent.com/"

length = len (list)

Putting spaces after function calls is odd, and you don't do it consistently. There's also rarely a good reason to store the length of a list.

prefix = "http://download3.utorrent.com/"

i= -1

No space before the =.

while i < length:
     i = i + 1
     if list[i].startswith(prefix) :
        break
binary= list[i]

You are using a while loop when you should be using a for loop. It is also going to cause an IndexError, rather than fail gracefully, if the prefix isn't found.

print "The URL of the the binary is: " , binary

# Returning the sha1 hash contained in the value of the key 'offer_hash'

encrypted_hash = decoded['offer_hash']

Is that hash really encrypted?

sha1_hash1 = encrypted_hash.encode('hex')
print "The sha1 hash contained in the value of the key 'offer_hash' is: " , sha1_hash1

# Downloading the binary and calculating its hash

urllib.urlretrieve ( binary , "utct2-en-conduit-20130523.exe")
file = "C:\Python27\utct2-en-conduit-20130523.exe"

Hard-coded filename will be easy to break. You shouldn't even need to do it. The string also contains "\" which should be escaped, or the whole string should be raw. i.e.

"C:\\Python27\\utct2-en-conduit-20130425.exe"

or

r"C:\Python27\utct2-en-conduit-20130425.exe"

or in most cases you can use the other slashes, even on Windows.

"C:/Python27/utct2-en-conduit-20130425.exe"

You got away with what you did, but that's mostly because you are lucky.

with open (file, 'rb') as myfile:
           downloaded=myfile.read()

This last piece of code did pretty much the same thing as a previous bit of code, but you didn't use a function.

k = hashlib.sha1()
k.update(downloaded)
sha1file = k.hexdigest()
print "The sha1 hash of the downloaded binary is: " , sha1file

# Verify that the calculated sha1 hash is identical to the one provided in the offer details

if (sha1file == sha1_hash1) :

You've put parentheses, which are not necessary.

    print "Test result = Pass"
else :
    print "Test result = Fail"

Your basic problems:

Your style is poor and inconsistent
A few things suggest you don't quite understand what's going on
You haven't taken care to make the code readable

Here's my take on the problem:

# Script that verifies the correctness of a toolbar offers

import urllib
import bencode
import hashlib
import contextlib

# it's common practice to put input details like this
# as global constants at the start of your script
URL = "http://update.utorrent.com/installoffer.php?offer=conduit"
PREFIX = "http://download3.utorrent.com/"

# by breaking down your task into functions
# you can make the code easier to follow
def read_bencoded_url(url):
    with contextlib.closing(urllib.urlopen(url)) as offer_file:
        return bencode.bdecode(offer_file.read())

def find_string_with_prefix(strings, prefix):
    for string in strings:
        if string.startswith(prefix):
            return string
    else:
        raise Exception("Did not find prefix: {}".format(prefix))

# this function is a bit more complicated
# but it avoids saving the file to disk or loading it entirely into memory.
# instead it hashes it 4096 bytes at a time
def hash_of_url(url):
    hasher = hashlib.sha1()
    with contextlib.closing(urllib.urlopen(url)) as binary_file:
        while True:
            data = binary_file.read(4096)
            if not data:
                break
            hasher.update(data)
    return hasher.hexdigest()

# the actual high level logic just calls the functions
# this avoid obscuring the logic with lower level details
def main():
    decoded = read_bencoded_url(URL)
    binary = find_string_with_prefix(decoded['offer_urls'], PREFIX)
    reported_hash = decoded['offer_hash'].encode('hex')
    actual_hash = hash_of_url(binary)
    print "The sha1 hash contained in the value of the key 'offer_hash' is: ", reported_hash
    print "The sha1 hash of the downloaded binary is: " , actual_hash

    if reported_hash == actual_hash:
        print "Test result = Pass"
    else:
        print "Test result = Fail"

if __name__ == '__main__':
     main()


Some points:

g.then(function() { … g.done ("two done");}) I (personally) don't like that signature, I'm more used to callbacks that get passed as parameters (g.then(function(done) { … done ("two done");})). They tend make the chained functions better reusable - in your case it might not matter since you're using dedicated function expressions anyway which are bound to g. And your signature simplifies argument handling in the Manager :-)
this.allDone = false; seems to be a misnomer for me. The way you're using it, it should rather be called startedAndNotRunning. Technically if you're initialising the empty list they are all done already :-)
This is highly confusing:

if ( this.where <= this.functionList.length-2 ) {
    this.where++;
    …

What happens to the last two functions? Nothing special. It better is written

this.where++;
if (this.where < this.functionList.length) {
    …

Then the iterator variable (where) will become the length of the array after the "loop", which is the common behaviour. Yet, you cannot use done any more to [re]start the chain - you'd need to decrement where before. Therefore I'd recommend to restructure the methods to

function Manager () {
    …
    this.where = 0;
}
Manager.prototype.start = function () {
    // you might reset allDone and where
    this.next();
};
Manager.prototype.done = function (prevRes) {
    this.previousResult = prevRes;
    this.where++;
    this.next();
};
Manager.prototype.next = function() {
    if (this.where < this.functionList.length)
        this.functionList[this.where](this.previousResult);
    else {
        this.allDone = true;
        console.log ( "done(): stopping at where=" + this.where + " with length = " + this.functionList.length);
    }
};
Manager.prototype.then = function (fun) {
    this.functionList.push (fun);
    if (this.allDone) {
        this.allDone = false;
        this.start();
        console.log ( "then(): continuing");
    }
};


Data type detection for a CSV file parser

Yes, you can make that code a lot more readable by noticing a simple fact. When you have the code

String decimalPattern = detectDecimal(valueAsString);
if(decimalPattern != null){
    return stringToBigDecimal(valueAsString, decimalPattern);
}else{
    // ...
}

The else part is actually unnecessary. If the test is true then we return with a value. This means that you exit early from the method, without going further down.

So you can rewrite it like this:

String decimalPattern = detectDecimal(valueAsString);
if(decimalPattern != null){
    return stringToBigDecimal(valueAsString, decimalPattern);
}
// ...

As you found out, this has the advantage that the subsequent code is shifted by one indentation to the left, making it easier to read. So only with this change, the code now looks like:

public static Object stringToDataType(String valueAsString) throws ParseException {
    // detections ordered by probability of occurrence in Buffer_Bank.
    String decimalPattern = detectDecimal(valueAsString);
    if (decimalPattern != null) {
        return stringToBigDecimal(valueAsString, decimalPattern);
    }
    String integerPattern = detectInteger(valueAsString);
    if (integerPattern != null) {
        return stringToBigInteger(valueAsString);
    }
    String datePattern = detectDate(valueAsString);
    if (datePattern != null) {
        return stringToDate(valueAsString, datePattern);
    }
    return valueAsString;
}

You posted an example of your detect* and stringTo* methods. In fact, you don't need two methods. You only need a single one that is specific to the current type you want to test. What you want is:

Try to interpret the value as a decimal and return it;
If not, try to interpret the value as an integer and return it;
If not, try to interpret the value as a date and return it.

Said like this, it is possibe to realize that you only need 3 methods: tryDate, tryInteger and tryDecimal. Each of those will do the necessary to try to interpret the String and return null if they can't. Basically, the code you currently have inside stringTo* should be merged into the part where you found the correct pattern in detect*. As an example:

private static Object tryDate(String dateString) {
    for (Map.Entry<String, String> entry : DATE_FORMAT_REGEXPS.entrySet()) {
        if (dateString.toLowerCase().matches(entry.getKey())) {
            SimpleDateFormat dateFormat = new SimpleDateFormat(entry.getValue());
            try {
                return dateFormat.parse(dateString);
            } catch (ParseException e) {
                // what to do in this case? Possibilites: throw an exception or return null
            }
        }
    }
    return null; // value is not a date
}

Note that I changed your code so that it loops directly around the entries instead of looping just with the keys. You should also decide what to do in case of a ParseException. You could rethrow a custom runtime exception, wrapping the original, or returning null to signal that the attempt failed.

The big issue is that there is some duplicated logic in this: each time we are detecting a pattern and, if it's not null, applying it to the value. If you were to add more detection algorithm, this method can become very quickly copy/pasted code and clumsy. We need to refactor that and make it generic.

Using Java 8, you can define that as a list of Function.

private static final List<Function<String, Object>> FUNCTIONS =
    Arrays.asList(s -> tryDecimal(s), s -> tryInteger(s), s -> tryDate(s));

The goal of those functions is to convert a String into a Object. Here, we define 3 which are the 3 methods mentioned above. Then we can have

public static Object stringToDataType(String valueAsString) {
    return FUNCTIONS.stream()
                    .map(f -> f.apply(valueAsString))
                    .filter(Objects::nonNull)
                    .findFirst()
                    .orElse(valueAsString);
}

This creates a Stream pipeline over the functions. Each of them is applied to the given String, non-null elements are filtered and only the first one is kept. Effectively, this will select the first non-null element, so it will select the first mapping that matched. If all are null then we return a default value with orElse which is the given value as String.

If somehow you want to keep your current approach with two different methods (one for detecting the pattern and one for parsins), then what you want comes down to creating a custom class that will hold:

A function that returns the pattern detected given the value as String;
A function that returns the object given the value as String and a pattern.

Simply, it could be:

public final class Conversion {

    private final UnaryOperator<String> patternDetector;
    private final BiFunction<String, String, Object> toObjectFunction;

    public Conversion(UnaryOperator<String> patternDetector, BiFunction<String, String, Object> toObjectFunction) {
        this.patternDetector = patternDetector;
        this.toObjectFunction = toObjectFunction;
    }

    public UnaryOperator<String> getPatternDetector() {
        return patternDetector;
    }

    public BiFunction<String, String, Object> getToObjectFunction() {
        return toObjectFunction;
    }
}

Then we can make a static utility that applies a given Conversion to a given value:

private static Object stringToDataType(String valueAsString, Conversion conversion) {
    String pattern = conversion.getPatternDetector().apply(valueAsString);
    if (pattern != null) {
        return conversion.getToObjectFunction().apply(valueAsString, pattern);
    }
    return null;
}

and finally, we can refactor the main method:

private static final List<Conversion> CONVERSIONS = Arrays.asList(
    new Conversion(v -> detectDecimal(v), (v, p) -> stringToBigDecimal(v, p)),
    new Conversion(v -> detectInteger(v), (v, p) -> stringToBigInteger(v)),
    new Conversion(v -> detectDate(v), (v, p) -> stringToDate(v, p))
);

public static Object stringToDataType(String valueAsString) throws ParseException {
    return CONVERSIONS.stream()
                      .map(c -> stringToDataType(valueAsString, c))
                      .filter(Objects::nonNull)
                      .findFirst()
                      .orElse(valueAsString);
}

Now, if we want to add more conversion in the future, we just need to update this list that, arguably, should be made a constant instead of being initialized each time. The Stream pipeline will return the first non-null element and will short-circuit once it has been found. If all are null, there we return the default value which is the given value as string.


Remove all occurrences of a digit from a number

Yes, the code can be simplified quite a bit, and in the process the static/global variable eliminated.

At least to me, an obvious implementation would be something like this:

unsigned rem(unsigned num, unsigned dig)
{
    if (num == 0)
        return num;

    unsigned digit = num % 10;

    unsigned temp = rem(num / 10, dig);

    if (digit != dig)
        temp = temp * 10 + digit;

    return temp;
}

So we basically split the number into the least significant digit, and everything else. We do the recursive call on the "everything else" part. Then we look at the least significant digit. If it's to be removed, we just leave it off the result. If it's not to be removed, we multiply the return value by 10, then add our digit on as the least significant again.

Of course, variations in syntax are possible. For example, you could use something like:

temp = rem(num / 10, dig);

return dig != digit ? temp * 10 + digit : temp;

That makes it shorter in terms of the number of lines of code, but it's essentially identical in terms of the real work being done.


Generally speaking, view should bind to viewmodel and viewmodel should notify view through injected adapter. I read about this approach in Mark Seemann's "Dependency injection in .NET" book.

You are injecting, to your viewmodels, interface like this:

public interface IWindow
    {
        void Close();

        IWindow CreateChild(object viewModel);

        void Show();

        bool? ShowDialog();
    }

Download source code to see usage: http://www.manning.com/seemann/

Another approach, through static Messager, is proposed by mvvm light framework.
Here are some tutorials and videos: http://www.galasoft.ch/mvvm/#tutorials

Personally I prefer injection, because only stateless classes should be considered as static (otherwise there are not testable). Besides, static Messager hides class dependency.


Implementation of the Stack data structure using a linked list in C++

template <typename V> using ptr = std::shared_ptr<V>;

Class specific details like aliases should be kept in the class or wrapped in a namespace to prevent global namespace pollution.

Why std::shared_ptr? std::shared_ptr is typically reserved for shared-ownership resource management and that has a cost associated to it.  They are twice the size of a raw pointer as the std::shared_ptr object stores both the raw pointer to the resource and a raw pointer to the reference count.  Reference counting must also be atomic to handle multiple reader/writers, resulting in slower increments and decrements.  If you want to use a smart pointer, C++ provides std::unique_ptr for exclusive-resource management and should typically be the default smart pointer unless you need std::shared_ptr.

    class Node {
    // ...
    }

Node contains some payload data.  We can read data through getData().  How do we write to data?  Why return a copy of data?  Since your Node class is an internal implementation detail that isn't used outside of your Stack, consider making Node an aggregate.

    struct Node {
        T data;
        ptr<Node> next;
    }

    Stack() : top{} {}

If your constructors are initializing data members with constant initialization values, explicitly initialize your data members using in-class initializers.

    ptr<Node> top{};        // default initializes top to nullptr
public:
    Stack() = default;      // don't suppress compiler generated def-ctor
    Stack(const std::shared_ptr<std::vector<T>>& vec) {
        // ...
    }

    ptr<Node> top;
public:
    // omitted destructor

On destruction, smart pointers are great because they clean themselves up.  Unfortunately, recursive destructions are not optimized.  If you had a deep linear hierarchy of smart pointers, there is going to come a point where you overflow the stack due to too many destruction calls.  If you used a smart pointer for a stack, you still need to implement a destructor which requires the rule of five.  The rule of five says that if you provide any of the copy assignment/constructor, move assignment/constructor, or destructor special member functions, you must provide the others.

T pop() {
    T forSafety{}; //how can I avoid this?

The problem with returning from pop is what do you return?  Returning by value is inefficient for expensive types.  Your other option is to return by reference, but pop is a destructive operation that would result in returning a dangling pointer.  A third option is to separate the concerns.  Make pop a non-value returning function that simply removes the top element.  peek already provides access to the top element.

T peek(){
    T forSafety{}; //how can I avoid this?
    if(!isEmpty()){
        return top->getData();
    } else return forSafety;
}

Member functions that inspect but do not mutate the object should be marked with a const suffix.  Mutations can happen internally through direct access and externally through references, so marking functions as const (read-only) prevents mutation.  Strive for const-correctness.

Return by value is inefficient and doesn't allow for external mutation of the value.  As stated in the review of pop, you don't want to return a dangling pointer.  The simplest solution is to delegate construction of a static variable to be referenced.

   T& peek() {
        if (top) {
            return top->getData();
        }
        return failed_range_check();
    }

    const T& peek() const {
        if (top) {
            return top->getData();
        }
        return failed_range_check();
    }

private:
    T& failed_range_check() {
        static T placeholder{};
        return placeholder;
    }

It works but it's still terrible.  The callee has no idea an error occurred and the function returns a value that looks to be valid.  A better solution is to use one of the many error handling techniques that already exist.  The simplest is to throw an exception.

    T& peek() {
        return do_peek();
    }

    const T& peek() const {
        return do_peek();
    }

private:
    T& do_peek() {
        if (!top) {
            throw std::range_error("Stack<>::peek(): Cannot peek empty stack.");
        }
        return top->getData();
    }

Other options include using typesafe std::error_condition with containers like Alexandrescu's Expected<T>, variant<T>, or classic struct/std::tuple result types.  If you don't care about the error type, you could use an optional<T>.

    if(top==nullptr) return true;
    else return false;

You don't have to explicitly compare to nullptr as the bool conversion for nullptr results in false.

  4.12 Boolean conversions [conv.bool]

  \$^1\$ A prvalue of arithmetic, unscoped enumeration, pointer, or pointer to member type can be converted to a prvalue of type bool. A zero value, null pointer value, or null member pointer value is converted to false; any other value is converted to true. For direct-initialization (8.5), a prvalue of type std::nullptr_t can be converted to a prvalue of type bool; the resulting value is false.

Continue to strive for const-correctness.

    bool isEmpty() const {
        return !top;
    }

void display(){
    auto _tmp = top;
    while(_tmp!=nullptr){
        std::cout << _tmp->getData() << std::endl;
        _tmp = _tmp->getNext();
    }
}

Stacks only provide access to the last element inserted, so a function that displays the content of the stack is unnecessary.  If you need to verify data is being entered and stored correctly, use testing and a debugger.

Be aware that <iostream> statically initializes the standard streams (std::cin, std::cout, std::cerr).  Rather than directly including <iostream> in headers and incurring the initialization costs on users.  Consider including <iosfwd> and pass streams through referencing parameters.

Be aware that

std::cout << std::endl;

is equivalent to

std::cout << '\n' << std::flush;

The call to std::flush may have an effect on stream efficiency.  Rather than researching which streams behave poorly with flushing, explicitly state your intent in code that you want a new line.

std::cout << '\n';                 // Explicit EOL char.
std::cout << std::flush;           // Explicit flush the stream.
std::cout << '\n' << std::flush;   // Explicit both

When there is an obvious loop variable, prefer for loops over while loops.

void display(std::ostream& out) {
    for( auto& curr = top; curr; curr = curr->next) {
        out << curr->getData() << '\n';
    }
}

int main(int argc, char** argv) {

main is a special function.  The standard allows main to take 2 forms.

  3.6.1 Main function [basic.start.main]

  An implementation shall allow both

  \$^{(2.1)}\$ — a function of () returning int and

  \$^{(2.2)}\$ — a function of (int, pointer to pointer to char) returning int

If you do not plan to use the parameters, explicitly convey your intent by omitting them.

int main() {

std::shared_ptr<std::vector<std::string>> names{new std::vector<std::string>{"Julia","Bob","Angela","fSociety"}};
auto s = std::make_shared<Stack<std::string>>(names);

Don't use smart pointers or dynamic allocation when you do not need it.

auto names = std::vector<std::string>{"Julia","Bob","Angela","fSociety"};
auto s = Stack<std::string>(names);        // conv ctor needs to be updated
s.display();
// ...

  How is this designed so far?

You are missing some basic methods.

std::size_t size() const;
T& emplace(Args...); (requires Node emplacement support)
void push(T&&);
void swap(T&);

As for design, applying the adaptor pattern would have been a much better choice.  A stack provides specialized access (last-in, first-out) that can be mapped to an underlying container.  push does an insert on one of the ends. top provides direct access to the same end.  pop removes the element at that end.  std::stack is designed this way.  The underlying container could be any sequence container that provides the required interface (push_back, pop_back, back).

The best part of composing with an existing container class is that it naturally provides the rule of zero.  Your adaptor class has nothing to do with ownership as it simply maps its interface on the underlying container.


LINQ (convenient)

You could rewrite the loops with LINQ so this

var completed = new List<string>();
foreach (var file in Directory.EnumerateFiles("C:\\Completed", "*.json"))
{
  completed.AddRange(
      JsonConvert.DeserializeObject<List<RootObject>>(File.ReadAllText(file)).Select(o => o.id));
}

could become this

var completd =
    Directory.EnumerateFiles("C:\\Completed", "*.json")
    .Select(File.ReadAllText)
    .Select(json => JsonConvert.DeserializeObject<List<RootObject>>(json).Select(x => x.id))
    .SelectMany(x => x)
    .ToList();

You can do the same with the other loop.

When using LINQ you might experience some performance hit but don't be discouraged from using it. Use LINQ first because it's easy to use and read. Only if you are not satisfied with its performance rewrite it with a normal loop.

In most cases you won't even notice that LINQ is in action.

Run a profiler if you're not sure. Don't blindly rewrite everything with loops because LINQ potentialiy could slow something down unless you really, really, really need to.

var processed =
  unfinished.Where(o => !completed.Contains(o.id)) // (#1) Ignore all completed RootObjects.
      .GroupBy(o => o.id).Select(objects => objects.First()) // (#2) Remove all duplicate RootObjects.
      .ToList();

I think this is the most inefficient part in your code because the completed.Contains method is a O(n) operation.

There are two alternatives for it: either use a HashSets or the Except extension that

  [..] returns those elements in first that do not appear in second. It does not also return those elements in second that do not appear in first.

In order to make it work and make its usage easier you should get entire RootObjects from the first query instead of just the ids and you'll need a custom comparer for the RootObject that you can even reuse for the Distinct extension so you can replace the

.GroupBy(o => o.id).Select(objects => objects.First())

with

.Distinct(new RootObjectComparer())

The new query is now much easier to understand and does not requrie any comments. It also swaps the Distinct with the Except so it has less items to process.

var processed =
    unfinished
    .Distinct(new RootObjectComparer())
    .Except(completed, new RootObjectComparer())
    .ToList();

The custom comparer.

class RootObjectComparer : IEqualityComparer<RootObject>
{
    public bool Equals(RootObject left, RootObject right)
    {
        return left.id == right.id;
    }

    public int GetHashCode(RootObject obj)
    {
        return obj.id;
    }
}

LINQ free (fast)

You could try to mix LINQ and the HashSet into this where you not only provide the root-objects to the constructor but also the custom comparer.

A hash-set stores only unique objects so there's no need to filter them.

var unfinished = new HashSet<RootObject>(
    Directory
        .EnumerateFiles("C:\\Unfinished", "*.json")
        .Select(File.ReadAllText)
        .Select(json => JsonConvert.DeserializeObject<List<RootObject>>(json))
        .SelectMany(x => x),
    new RootObjectComparer()
);

The fastet version should be however this LINQ free one. Here you import the lists directly into the hash-set.

var unfinished = new HashSet<RootObject>(new RootObjectComparer());

foreach (var file in Directory.EnumerateFiles("C:\\Unfinished", "*.json"))
{
    unfinished.UnionWith(
        JsonConvert.DeserializeObject<List<RootObject>>(File.ReadAllText(file)));
}

Then you exclude the completed items with

unfinished.ExceptWith(completed);


I don't know if this would be significant, but a first suggestion would be to replace the relatively costly re operation with the basic string operationitem['name'].endswith("AuditFeesExpenses").

Another possible suggestion, based on @Dex'ter's comment would be to change the stdout redirection into a regular .write() on the output file.

But what I'd really recommend is to profile the script to figure out the hot spots. I suspect that the bottleneck is within BeautifulSoup, and if that's the case, (given that you're only searching for a substring and not parsing) perhaps you could find an alternative search method.


Well, that looks like a straightforward use of perfect forwarding in the parameter (T&&, std::forward<T>) and in the return type (decltype(auto)). So yep, you've got it.

There are plenty of places in C++ where x and identity(x) don't do the same thing, either because x is a name as well as an expression, or because x is a construct that looks like an expression but technically is not one. (Personally, I consider almost all of these places to be mistakes in the design of C++.) Here are the ones I can think of.

Replacing identity(x) with x in all of the following examples changes the behavior of the code sample.

As mentioned in the comments on the question, the name of a function template can deduce what kind of function pointer to decay to:

template<class T> void f(T) {}
void (*fp)(int) = identity(f);

The name of a local variable gets copy elision (a.k.a. Named Return Value Optimization, or NRVO):

struct S {
    S() {}
    S(const S&) { puts("copy"); }
    S(S&&) { puts("move"); }
};
S f() {
    S s;
    return identity(s);
}
int main() { f(); }

The name of a local variable is treated as an rvalue in the context of return:

struct S {
    S() {}
    S(const S&) { puts("copy"); }
    S(S&&) { puts("move"); }
};
std::any f() {
    S s;
    return identity(s);
}
int main() { f(); }

Lifetime extension applies to temporaries whose construction is "visible" in some sense that is extremely subtle when I start thinking hard about it. Interposing the call to identity disables lifetime extension.

struct S {
    S() { puts("ctor"); }
    S(const S&) = delete;
    S(S&&) = delete;
    ~S() { puts("dtor"); }
};
int main() {
    const S& x = identity(S());
    puts("done");
}

Braced initializers and string literals look like expressions, but in the context of an initializer, they're special cases:

const char x[] = identity("hello");
int y = identity({5});

Now, I can't imagine how you'd use identity in your codebase where any of these quirks might matter; but then, I don't quite see how you plan to use identity at all. Your only fleshed-out example is just using std::transform(..., identity) as a verbose way of writing std::copy(...), which doesn't seem very useful.


Three PHP database queries to manage accounts

Common functionality refactoring

public function deleteAccount($token)
{
    $sql1 =
        "DELETE FROM $this->table_user
         WHERE $this->key_token = ?;";

    $stmt1 = $this->connection->prepare($sql1);
    $stmt1->bind_param("s", $token);
    $stmt1->execute();
    $result = $stmt1->affected_rows;
    $stmt1->close();
    return ($result >= 1);
}

You could change this to something like

private function modify($sql, array $parameters = [])
{
    $stmt = $this->connection->prepare($sql);

    if (!empty($parameters))
    {
        // ensure $parameters are passed by reference
        $parameters_by_reference = [];
        for ($i = 0, $n = count($parameters); $i < $n; $i++)
        {
            $parameters_by_reference[] =& $parameters[$i];
        }

        call_user_func_array([$stmt, bind_param], $parameters_by_reference);
    }

    $stmt->execute();
    $result = $stmt->affected_rows;
    $stmt->close();

    return $result;
}

public function deleteAccount($token)
{
    $sql = <<<EOSQL
DELETE FROM $this->table_user
  WHERE $this->key_token = ?
EOSQL;

    return ($this->modify($sql, ['s', $token]) >= 1);
}

Now it only takes two or three statements to set up the SQL query.  The repeated part is all inside the modify function.  Note that modify can handle any of INSERT, UPDATE, or DELETE.

I prefer the heredoc as being better for multiline strings.

I don't like numbered variables, particularly if there's only one of them.

I prefer to use single quotes for PHP strings that do not have any variable interpolation in them.

You do not need to pass a ; when executing a single statement programmatically.

Be careful about $this->table_user and $this->key_token.  These should NOT contain any user input.  Otherwise, this would be subject to SQL injection.

You can then change

public function resetPassword($password, $hash, $email)
{
    $sql1 =
        "UPDATE $this->table_user
         SET $this->key_hash = ?
         WHERE $this->key_email = ? AND $this->key_passwordhash = ?;";

    $stmt1 = $this->connection->prepare($sql1);
    $passhash = password_hash($password, $this->algorithm);
    $stmt1->bind_param("sss", $passhash, $email, $hash);
    $stmt1->execute();
    $result = ($stmt1->affected_rows == 1);
    $stmt1->close();
    return $result;
}

to

public function resetPassword($password, $hash, $email)
{
    $sql = <<<EOSQL
UPDATE $this->table_user
  SET $this->key_hash = ?
  WHERE $this->key_email = ? AND $this->key_passwordhash = ?
EOSQL;

    $passhash = password_hash($password, $this->algorithm);

    return ($this->modify($sql, 'sss', $passhash, $email, $hash) == 1);
}

PDO

Note that if you switch to PDO, then modify could be simpler:

private function modify($sql, array $parameters = [])
{
    $stmt = $this->connection->prepare($sql);

    foreach ($parameters as $parameter => $value)
    {
        $stmt->bindParam($parameter, $value);
    }

    $stmt->execute();

    return $stmt->rowCount();
}

And you would call it like

    return ($this->modify($sql, [1 => $passhash, 2 => $email, 3 => $hash]) == 1);
    return ($this->modify($sql, [1 => $token]) >= 1);

Or you could use named parameters, but that requires modifying the SQL string as well.


Find DivTag element in list and click when innerHTML matches account name

To eliminate the need for the flag you could do it like this:

var element = _my_repo.SignInToYourAccount.UseAnotherAccount;
foreach (var item in _list)
{
    if (item.GetInnerHtml().Equals(_account_name))
    {
        element = item;
        break;
    }
}
element.Click();

However, your code can be greatly simplified with the Linq extension methods provided in the System.Linqnamespace:

var element = _list.FirstOrDefault(x => x.GetInnerHtml().Equals(_account_name)) ?? _my_repo.SignInToYourAccount.UseAnotherAccount;
element.Click();


Python - A Regex Text File Searcher by Content Project

Documentation
"""
returns the directory if the path is a directory, and it exists otherwise asks to input again
"""

OK; but that's not what this function does at all. The "asking to input again" is done elsewhere.
Type hints
Guessing for this signature; type hints would remove the guesswork:
def format_output(matched_paths, matched_dict):

could become
def format_output(
    matched_paths: Iterable[str],
    matched_dict: Dict[
        str,
        Dict[str, str]
    ],
):

If this is true, the second type is complex enough that it could benefit from external declaration, i.e.
MatchesDict = Dict[
    str,
    Dict[str, str]
]

Dictionary iteration
for path in matched_paths:
    for line, matches in matched_dict[path].items():
        for match in matched_dict[path][line]:

should be
for path in matched_paths:
    for line, matches in matched_dict[path].items():
        for match in matches:

In other words, items gets you a key and a value; when you have the value, use it.
Logic inversion
I find that this:
    if re.search(string, path.read_text()):
        matched_dict[path] = dict()
        for i, line in enumerate(text_file.readlines()): # i refers to line number
            matches = list(re.finditer(string, line))
            if matches:
                matched_dict[path][i] = []

                for m in matches:
                    matched_dict[path][i].append(m.span())
        return True
    return False

is more legible as
    if not re.search(string, path.read_text()):
        return False

    matched_dict[path] = dict()
    for i, line in enumerate(text_file.readlines()): # i refers to line number
        matches = list(re.finditer(string, line))
        if matches:
            matched_dict[path][i] = []
             for m in matches:
                matched_dict[path][i].append(m.span())
    return True


Fun with CallerLineNumberAttribute: clever hack or terrible idea?

This is definitely an interesting use of [CallerLineNumber] but I think that the risk for people using it wrong resulting in hard to find bugs is to great. If you take an example from the issue you yourself mentioned.

if (detector.DetectUp(y > 100) ||
    detector.DetectUp(x < 500))
{...}

Would not as one might think have the same behavior as

if (detector.DetectUp(y > 100) || detector.DetectUp(x < 500))
{...}

Another gotcha I found was that lambda functions does not give the line or name of where they are called but where they are defined. This makes sense but might not be what the user expects. The example below prints Main: 10 and not HelpFun: 16.

using System;
using System.Runtime.CompilerServices;

namespace CallerLineNumberTest
{
    class Program
    {
        static void Main(string[] args)
        {
            HelpFun(() => PrintLineNr());
            Console.ReadKey();
        }

        private static void HelpFun(Action a)
        {
            a();
        }

        private static void PrintLineNr([CallerMemberName] string name = "",
            [CallerLineNumber] int lineNumber = 0)
        {
            Console.WriteLine(name + ": " + lineNumber);
        }
    }
}


You can achieve the same but simpler if you reorder the validation steps.

If there are two arguments and both are test, that's invalid:

[ "$1" = test ] && [ "$2" = test ] && invalid

Where invalid is a helper function:

invalid() {
    echo not valid
    exit 1
}

If the second argument is test, then enable testing,
or else if the first argument is test then enable testing and shift the arguments:

if [ "$2" = test ]; then
    echo enable testing
elif [ "$1" = test ]; then
    echo enable testing
    shift
fi

At this point, the should be 0 or 1 arguments, not more.
That was a missing validation step in the original code:

[ $# -le 1 ] || invalid

If we reach this point,
there must be 0 or 1 arguments.
The original case statement can come here as before, without any further if statements. It can use the invalid helper function in the invalid case.


Dynamic Allocation and Reallocation of Memory

Code would have benefited with allocateList(), allocateParticles(), freeList(), freeParticles() functions, rather than trying coding it all up in main().

allocation and deallocation of memory correct?

Allocations lack error checking.  This is the big issue as it also impacts proper de-allocation code.

plist->list[k] = realloc(plist->list[k], .... leak memory should the allocation fail.

DIM*plist->counter[k])*sizeof(plist->list[k][0]) could overflow int math, yet work well with size_t math.  See re-ordering below.

Robust-ness

Use size_t instead of int for indexing. As "code snippet of a larger program", the array sizes may be very large.  The best type for array indexing is size_t, an unsigned type.

// for(int d=0; d<DIM; d++){
for(size_t d=0; d<DIM; d++){
        p[i].x[d] = frand();
        p[i].v[d] = frand();
    }

Simplification

C does not require the cast nor () with sizeof object

// Particle *p = (Particle*)malloc(N*sizeof(*p));
Particle *p = malloc(N * sizeof *p);

// I prefer `sizeof` first, especially  for math that may benefit with `size_t` math.
//                   v---------------v all size_t math
Particle *p = malloc(sizeof *p * N * N);
// vs                v---v may overflow `int` math
Particle *p = malloc(N * N * sizeof *p);

Simplify coding, review and maintenance.  Is plist.list a double *?  Why bother attempting to co-coordinate the type of member list with the malloc() call which may be in another file?

// plist.list = malloc(sizeof(double*)*numprocs);
plist.list = malloc(sizeof *(plist.list) * numprocs);

// plist.counter = calloc(numprocs, sizeof(int));
plist.counter = calloc(numprocs, sizeof *(plist.counter));
// or
plist.counter = calloc(numprocs, sizeof plist.counter[0]);

The *1 is curious and lacks benefit

// plist.list[k] = malloc(sizeof(double)*1);
plist.list[k] = malloc(sizeof *(plist.list[k]));

buildList() would benefit with re-write - IMO, is is not simple to follow.  As code lacks guiding comments, I am left to guess how to simplify it without losing meaning.  Sample idea with some style improvements.

// Style issue: Avoid wrapping presentation code and horizontal scroll bars
void buildList(Particle *p, size_t n, ParticleList *plist, size_t numprocs) {
  for (size_t np=0; np<numprocs; np++) {
    for (size_t i=0; i<n; i++){
      printf("np %zu, i %zu\n", np, i);
      if (rand()%10 < 5) {
        size_t new_count = plist->counter[k] + 1;
        void *new_ptr = realloc(plist->list[np],sizeof plist->list[np][0] * DIM*new_count);
        if (new_ptr == NULL) {
          TBD_ErrorHandler();
          // ...
        }
        plist->list[k] = new_ptr;
        plist->counter[k] = new_count;
        // I think the following is the same - I had trouble following original code.
        for(size_t dm = 0; dm < DIM; dm++) {
          plist->list[pn][new_count + j] = p[i].x[dm];
        }
      } // endif
    }
  }
}

"l" in "%lf" not required - more of a style issue

// printf("id: %d m: %lf x: %lf %lf v: %lf %lf\n",
printf("id: %d m: %f x: %f %f v: %f %f\n",
  p[i].id, p[i].m, p[i].x[0], p[i].x[1], p[i].v[0], p[i].v[1]);

Sample ParticleList_free()

void ParticleList_free(ParticleList *pl) {
  if (pl) {
    if (pl->list) {
       for(int k=0; k<numprocs; k++) {
         free(pl->list[k]);
         pl->list[k] = NULL; // ***
       }
       free(pl->list);
       pl->list = NULL; // ***
    }
    free(pl>counter);
    pl>counter = NULL; // ***
    free(pl);
  }

  // Sample usage
  ParticleList *pl = ParticleList_alloc();
  buildList(p, pl, numprocs, N);
  ....
  ParticleList_free(pl);
  pl = NULL; // ***

*** Useful for debugging, but not needed for production code.  By setting to NULL, errant uses of pointers are typically more likely to be found.  YMMV.


ActiveRecord SQL search with a filter based on parameters

Even if you don't want to load your data into ActiveRecord, you can use AR to prepare your SQL statement using to_sql:

def self.for_venue(venue, options = {})

  filter = options[:filter]

  date_conditions = case options[:date]
  when 'upcoming' then  "datetime > ?"
  when 'past'     then  "datetime < ?"
  end

  sql_query = Event
    .where(:venue_id => venue.id, :deleted => 0)
    .where(date_conditions, Time.now.beginning_of_day)
    .joins(search_joins)
    .limit(options[:limit])
    .offset(options[:offset])
    .order(options[:order])
    .to_sql

  results = SlaveDB.connection.select_all(sql_query)

  filter ? filter.select(results) : results

end


Implementing a queue in Java for a technical interview

You don't have a parameterless constructor which forces each queue to be instantiated with 2 elements. This isn't userfriendly since I might very wel want to construct it first and then add 5 bazillion elements to it.

Your code already allows it though: technically first can never be null as it is, yet you check for it. You can add a constructor without a problem.

Right now you only accept integers; perhaps you could make it generic to allow for each type?

Node is public; I'd change that to private since the outside world should never use it anyway.

Your code doesn't work; executing it will return 2 and -1, not 3. This happens because you don't set the next property of the nodes that are passed in to the Queue(Node, Node) constructor.


Improving function that compares two strings

Your parameters should be const and your loop need only to check one string for the terminator.  For example in the following loop, if *s1 is 0 the loop stops because of the first condition. If *s1 is not 0 but *s2 is 0,  the loop stops because of the 2nd condition.

int match(const char *s1, const char *s2)
{
    while (*s1 && (*s1++ == *s2++)) {
         // nothing
    }
    return *s1 - *s2;
}


Hardcoding

The path to the file with words is hardcoded. That's not so good. It would be better to make it a command line argument.

If you must hardcode something, make it a constant, for example:

private static final String PATH = "src/app/wordlist.txt";

fileName was not a good name for a path anyway. A "file name" usually refers to the last path segment without parent directories.

Single responsibility principle

The class violates the single responsibility principle: it picks a random word, and it reads a file. I would do something like this instead:

class RandomStringPicker {
    private final List<String> list;

    public RandomStringPicker(List<String> list) {
        this.list = list;
    }

    public String pick() {
        return list.get(pickRandomNumber(list.size()));
    }

    private int pickRandomNumber(int max) {
        return (int) (Math.random() * max);
    }
}

This class only does one thing. You could read the file somewhere else and create a RandomStringPicker class from the lines you read. But now you can reuse this class for other purposes too, for example in unit tests.

This implementation also improves the weakness of your init method. If you forget to call init, your program will compile, but not work. In the example above there is no such uncertainty: if the class is not constructed with a list, it will not compile.

Or you could even go one step further and make the class work with anything, not only strings:

class RandomPicker<T> {
    private final List<T> list;

    public RandomPicker(List<T> list) {
        this.list = list;
    }

    public T pick() {
        return list.get(pickRandomNumber(list.size()));
    }

    private int pickRandomNumber(int max) {
        return (int) (Math.random() * max);
    }
}

Bugs

If you have 10 lines, the call getRandomNumber(1, lines.size()) will return a random number between 1 and 10, inclusive, which has two problems:

The first line (index = 0) will never be picked
If 10 is picked, that will be beyond the end of your list and throw an exception

Related to this issue is that you don't really need a minimum parameter for your random number picker. So that's an unnecessary detail in your implementation, you could use something more simple like the example above.

Printing, validating and logging

It's not good practice to print things on the console. If you want to verify your code works, write unit tests. If you want to log messages during runtime, use a logger.

Nitpicks

The code is not well indented
Unnecessary brackets in (max - min) + 1. This is the same: max - min + 1


You have a bug. Take the following code, and output:

s = SetOfValues(iter([1, 2, 3, 4, 5, 6]))
print(list(zip(s, s)))
print(list(zip(s, s)))

[(1, 2), (3, 4), (5, 6)]
[(1, 1), (2, 2), (3, 3), (4, 4), (5, 5), (6, 6)]

This is quite a substantial bug. As I would only use SetOfValues over a list, if I can get the correct list without consuming the iterator. Because if the iterator is consumed, by the time I loop through the list again, then I've used the most long-winded version of list known.

The way I'd resolve this is to build the list whilst consuming the iterator. This would look something like:

def cache(it, l):
    for i in it:
        l.append(i)
        yield i

However, we don't know if the iterator has been consumed if we stick with that. So I'd abuse lists a little more, so that we have a 'return' list, that returns the stored data, and whether the iterator has been consumed.

Other than that, I'd use itertools.chain so that we can change the list and the iterator together. However if the iterator has been consumed, then I'd return the list as an iterator.

Resulting in:

def cache(it, ret):
    l = ret[0]
    for i in it:
        l.append(i)
        yield i
    ret[1] = True

class SetOfValues(object):
    def __init__(self, values):
        self._info = [[], False]
        self._it = cache(values, self._info)

    def __iter__(self):
        l, finished = self._info
        if finished:
            return iter(l)
        else:
            return itertools.chain(l, self._it)

However, I'd heed the advice of itertools.tee.

  In general, if one iterator uses most or all of the data before another iterator starts, it is faster to use list() instead of tee().

And so, I'd use list instead in most cases.


Use a switch statement.  Add a default condition to show the error message when the entered train type is not matched with any specified train type.

switch (train) {
    case 23: InterCity++; break;
    case 51: Regional++;  break;
    case 72: Overnight++; break;
    default: // TODO: error handling
}


C program to "monitor" NGINX status with AF_INET

Starting with main(): the signature implies that we accept command-line arguments, but we ignore argc and argv, using the hard-coded HOST_ADDR and HOST_PORT instead.  This makes the program less flexible.

An improvement would be to use those values as defaults, but allow command-line arguments to override them:

 const char *server = HOST_ADDR;
 if (argc > 1)
     server = argc[1];
 uint16_t port = 0;
 if (argc > 2)
     port = atoi(argc[2]);
 if (port == 0)
     port = HOST_PORT;

Argument processing can get much more sophisticated than this, but there's the bare minimum for the job.

Note in particular that atoi(HOST_ADDR), after expansion, is atoi("127.0.0.1") which always evaluates to 127.  What you really want is to change the signature of socket_get() to accept a string hostname:

int socket_get(const char *i_addr, uint16_t i_port)
{
    struct sockaddr_in server;
    server.sin_addr.s_addr = inet_addr(HOST_ADDR);
    if (server.sin_addr.s_addr == INADDR_NONE)
        return -1;

Notice that inet_addr may fail, so handle that gracefully.

There are severe problems in send_alarm(), apart from immediately failing if the compiled-in host names ever change.

Although we're using strncpy() to avoid overrunning from_host, that's slightly dangerous because it can leave an unterminated string in the output (and I'd change that hard-coded 24 to sizeof from_host, too).  That's minor, as the fixed strings we're using won't overflow the storage.

A more serious problem is that to is only 20 chars long, and we're initialising it with a 20-char string.  That means it won't have a null terminator, which will cause undefined behaviour when it's accessed by any string functions (such as the printf()).  A safer version is

    char to[] = "user@example.com";

Finally, the sprintf() is really dangerous.  For some reason, we haven't used snprintf() here, so it's possible to overrun the output cmd, and in fact, it's actually guaranteed to do so, given that cmd is only 100 chars in size, and just adding the lengths of the format string and to will exceed that.

Thankfully, all the issues in this function are easily fixed, because I'm going to recommend removing it completely.  Instead of invoking mailx directly from this program, it's more flexible to provide the necessary output and leave it to the caller to decide what to do with it (this is in keeping with the "Unix Philosophy" - do one thing well, and support composition of tools).

My recommendation is to delete send_alarm() and change the calling code to

    if (socket_get(server, port)) {
        fprintf(stderr, "Can't connect to NGINX service on %s:%u!\n", server, (unsigned int)port);
        return 1;
    }

By writing output and returning non-zero when (and only when) we fail, we make it easy to use in shell scripts or with cron (cron will send you email if the command produces output, and this is a perfect example of how to cooperate with that).


Comparing different string-matching functions

Since memory and performance metrics are similar, coding style is possibly the only differentiating factor.

Nothing wrong with solution 1 but it takes more time to read it, unless you are paid by the number of lines you write! I guess it is Pythonic to write code in fewer lines so long as readability is not compromised.

I prefer solution 3. It reads well and terse. Solution 2 is just as good but it appears to be doing extra work in the sense of creating a list and then counting. Why do that when you can directly sum up the matches?


Make a Map like this

Map<String, JTextField> map = new HashMap<>();
map.put("$codeA", panel1.txtFieldA);
map.put("$codeB", panel2.txtFieldB);

and change your loop like this

String code = null;
while ((code = br.readLine()) != null) {
    JTextField jtf = map.get(code);
    if (jtf != null)
        jtf.setText(br.readLine());
    else
        br.readLine();
}


Identify duplicate characters in a String and replace them

First of all, nullbyte is exactly right about the use of a StringBuilder: using a += b for Strings roughly equals new StringBuilder().append(a).append(b).toString() under the hood and thus is a quite expensive operation.

Then, your method to check whether a given charcter exists in the string after a given index is s.substring(i).contains(ch) which again creates a superfluos string object via the substring-call. Instead, simply use s.indexOf(ch, i) to find the next index of ch after the given starting index (returns -1 for "not found").

Additionally, you might consider nullbyte's suggestion to hold the seen-or-not-seen status in a set, which also was my first intuition when reading the exercise. (Right now, you have runtime complexity of O(n^2) which you could reduce to O(n)). You only have to somehow come up with a method to revisit the first character, maybe by not using  a set but by usign a map from character to number of occurrences or something like that.

Naming: why decodedString? This is not decoded in any way, this is just lowercase word. I can live with encodeString (or probably better encoded), but decodedString does not look right and does not give the reader any clue about the variable's contents.


Project Euler # 67 Maximum path sum II (Bottom up) in Python

The algorithm is very efficient, your program computes the result in fractions of a second.

Reading the file into a nested list can be simplified to

def get_triangle(triangle_filename):
    """Return a list of lists containing rows of the triangle."""
    triangle = [[int(number) for number in row.split()]
                for row in open(triangle_filename)]
    return triangle

because open() returns a file object that can be iterated over. As an additional advantage, no empty list is appended. Your reading function actually returns

[[59], [73, 41], [52, 40, 9], ... , []]

Here

triangle_file = get_triangle('p067_triangle.txt')

I find the naming confusing: The return value is not a file, and the nested list is called triangle everywhere else. Therefore I would change that here as well:

triangle = get_triangle('p067_triangle.txt')

In the main routine

def maximize_triangle(triangle, start_index=-1):

I would use a nested loop instead of recursion. That makes the start_index parameter with its default value obsolete, and the logic is easier to understand (in my opinion). Instead of start_index and index I would use row and col as variable names. Finally the function does not “maximize a triangle” but computes the “maximal path sum,” so let's name it accordingly:

def maximal_path_sum(triangle):
    """Return the maximum triangle path sum(bottom up)."""
    # Starting with the last row ...
    for row in range(len(triangle) - 1, 0, -1):
        # ... add maximum of adjacent entries to the corresponding entry
        # in the preceding row:
        for col in range(len(triangle[row]) - 1):
            maximum = max(triangle[row][col], triangle[row][col + 1])
            triangle[row - 1][col] += maximum
    return triangle[0][0]


Maintain the ordering of the original array, but group either negative or positive numbers into sub arrays

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

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

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

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


Your code looks so good, it is quite impressive of you were learning c++ few days ago. i'm sure you will get a decent code-review but here my humble attempt.

It always preferred to pass the std::string by const reference if you don't  modify it. also, std::vector is preferred over std::list. here link for more details

In your read function. It redirects all input into this container until EOF is received. However, the used container might need to reallocate memory too often, or you will end with a std::bad_alloc exception when your system gets out of memory. In order to solve these problems, you could reserve a fixed amount N of elements and process these amount of elements. here an alternative example to your read function by using std::copy_n and std::istream_iterator like so,

words.reserve(N);
std::copy_n(std::istream_iterator<std::string>(std::cin),
    N,
    std::back_inserter(words));

In your evaluate function, it is fine but not efficient doe to the call of std::string constructor in every iteration along with deleting un-palindrome elements. it is better to create another container for palindrome elements and check the main container elements if it has palindrome element. possible implementation:

std::vector<std::string> palindromes;
for (const auto& word : words)
{
    if(is_palindrome(word))
        palindromes.emplace_back(word);
}

The is_palindrome function, we test the given string for whither it is palindrome or not, like so:

bool is_palindrome(const std::string& str)
{
    if (str.empty())
        return false;

    using str_sz = std::string::size_type;
    for (str_sz i = 0, j = str.length() -1; i < j ; ++i, --j)
    {
        if (str[i] != str[j])
            return false;
    }
    return true;
}

thanks to @screwnut for suggesting to std::copy_if instead of for-range while the for-loop is bit clear and simple. but std::copy_if will allow us to use lambda instead of calling is_palindrome function, that might be helpful if you simply write it like this:

std::copy_if(words.begin(), words.end(),
             std::back_inserter(palindromes),
             [](const std::string& str)
             {return std::equal(str.begin(), str.begin() + str.size() / 2, str.rbegin());});

Lastly, for longest string, you may take advantage of STL by using std::max_element which is return iterator.

std::max_element(palindromes.begin(), palindromes.end(),
    [](const std::string& lhs, const std::string& rhs)
{
    return lhs.size() < rhs.size();
});

put all to gather:

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

int main()
{
    const std::size_t TotalWords = 3;
    std::vector<std::string> words;
    words.reserve(TotalWords);

    std::cout << "Please enter some words." << std::endl;
    std::istream_iterator<std::string> in(std::cin);
    std::copy_n(in, TotalWords, std::back_inserter(words));

    std::vector<std::string> palindromes;

    auto comp = [](const std::string& str)
    {
        return std::equal(str.begin(), str.begin() + str.size() / 2, str.rbegin());
    };

    std::copy_if(words.begin(), words.end(), std::back_inserter(palindromes), comp);

    if (palindromes.empty())
    {
        std::cout << "No palindromes were given" << std::endl;
    }
    else
    {
        auto longest = std::max_element(palindromes.begin(), palindromes.end(),
            [](const std::string& lhs, const std::string& rhs)
        {
            return lhs.size() < rhs.size();
        });
        std::ostream_iterator<std::string> out(std::cout, "\n ");
        std::cout << "Palindromes: \n ";
        std::copy(palindromes.begin(), palindromes.end(), out);
        std::cout << "\rLongest: " << *longest << std::endl;
    }
}


Checking for neighbours more elegantly in Conway's Game of Life

You're right, there's an alternate way to do this, but, first, some Java standards:

1-liners should have braces. This is a code-style that is common to many langauges because it is more maintainable, and leads to fewer future bugs. Lines like:

if (x < 0 || y < 0 || x > getWidth() - 1 || y > getHeight() -1) return;

should be:

if (x < 0 || y < 0 || x > getWidth() - 1 || y > getHeight() -1) {
    return;
}

Don't use arithmetic when you can use a simpler operator. Back to:

if (x < 0 || y < 0 || x > getWidth() - 1 || y > getHeight() -1) { ... }

Which can be simplified to:

if (x < 0 || y < 0 || x >= getWidth() || y >= getHeight()) { ...}

Use plain booleans when they make sense (again, back to...):

if (x < 0 || y < 0 || x >= getWidth() || y >= getHeight()) { ...}

which can be put in a function like (transformed to uses && instead):

public boolean getTile(int x, int y) {
    return x >= 0 && y >= 0 && x < getWidth() && y < getHeight() && tile[y][x];
}

Note, how the standard boolean short-circuit evaluation ensures that the x and y are valid before the tile[y][x] is used.

OK, about that code duplication. A trick with these sorts of problems is to use an array of offsets. Consider your grid, which has 8 neighbours (in a relative y,x format):

-1,-1     -1,0    -1,+1

 0,-1     *us*     0,+1

+1,-1     +1,0    +1,+1

We can put these neighbour offsets in to an array:

private static final int[][] NEIGHBOURS = {
    {-1, -1}, {-1, 0}, {-1, +1},
    { 0, -1},          { 0, +1},
    {+1, -1}, {+1, 0}, {+1, +1}};

Then, your check method becomes:

static int countNeighbours(Board b, int x, int y) {
    int cnt = 0;
    for (int[] offset : NEIGHBOURS) {
        if (b.getTile(x + offset[1], y + offset[0]) {
           cnt++;
        }
    }
    return cnt;
 }


I like it.

There are only a few things I would reconsider:

detectedId is a clunky name, clickedId or selectedId seems better
You hardcode 4, at least use a constant, ideally detect how many buttons there dynamically
The treatment for the first button is different enough that it can be placed outside of the loop
It seems you only need to know which button was clicked, which can be found by analyzing the selected id

if(selectedId === "btn1"){
  $(".cont-img div").css("left", "0%");
}else{
  let id = selectedId.split('btn')[1]*1;
  $(".cont-img div").css("left", "-" + (id - 1) + "00%");
}

This way you dont even need to hardcode 4


While your code might be working and written in a functional language, it also feels very procedural to me. Most idiomatic Haskell code I see has a very high signal to noise ratio. In your code I have to read every single line and figure out what it does in my head.

You can achieve this by making a lot of tiny functions (after all, composition is key for a functional program), and use as much of the domain language as possible. These tiny functions don't need to stand on their own; using let x=... in ... or where x= tends to help a lot.

Also, try to figure out what state you need to keep, and name it. I see a lot of functional programmers design their apps types-first. Having an explicit model that doesn't allow faulty state tends to help big-time.

Make sure you don't add state you can calculate from other state, use functions for that, otherwise your data structure might get inconsistent.

AFAIK tuples should mostly be used inside functions, but never exposed, except maybe for the most trivial cases like key-value pairs, and even that is debatable... After all, how much work is it to define data KeyValuePair a b= KeyValuePair a b

I am by no means an expert in Haskell, and learned these rules from doing mostly object-oriented programming. However, in a functional language these rules tend to be way easier to implement.

About operator shortcuts etc: I don't use them a lot - yet -, unless the Haskell linter suggests me to use it.

I do think that using a tool like ghc-mod and hlint is a necessity for Haskell development.

TL;DR: use proper types, extract parts into smaller functions, and try to properly name them.

import           Control.Monad (when)
import           Data.Char     (toLower)
import           Data.List     (transpose)
import           System.Random (randomIO)

wordsPath :: FilePath
wordsPath = "words.txt"-- "/usr/share/dict/words"

data GameState = GameState
    { _wordToGuess :: String
    , guesses     :: String
    }

data GameStatus = Guessing | GameWon | GameLost deriving Eq

hangmanImages :: [[String]]
hangmanImages =
    transpose
    [ [ "   ", " O ", " O ", " O ", " O " , "_O " , "_O_"  ]
    , [ "   ", "   ", " | ", " | ", " | " , " | " , " | "  ]
    , [ "   ", "   ", "   ", "/  ", "/ \\", "/ \\", "/ \\" ]
    ]

fullHangmanImage :: Int -> [String]
fullHangmanImage index =
    "=========" :
    "|    |" :
    map ("|   " ++) img
    where img = hangmanImages !! index

maxWrongGuesses :: Int
maxWrongGuesses = length hangmanImages - 1

numberOfWrongGuesses :: GameState -> Int
numberOfWrongGuesses (GameState word' guesses') =
    length $ filter charNotInWord guesses'
    where charNotInWord c = c `notElem` word'

gameStatus :: GameState -> GameStatus
gameStatus (GameState word' guesses')
    | isGuessed     = GameWon
    | isLastGuess'  = GameLost
    | otherwise     = Guessing
    where
        isGuessed = all isCharInGuesses word'
        isCharInGuesses x = x `elem` guesses'
        gameState = GameState word' guesses'
        isLastGuess' = numberOfWrongGuesses gameState == maxWrongGuesses

-- for one reason or another getChar also appends <CR>
-- so I implemented my own getChar and made sure empty input is refused
getAChar :: IO Char
getAChar = do
    line <- getLine
    case line of
        [] -> getAChar
        (c:_) -> return c

getANewChar :: GameState -> IO Char
getANewChar gameState = do
    putStrLn "Next char to guess"
    c <- getAChar
    if c `elem` guesses gameState
    then do
        putStrLn "Character already used in guesses."
        getANewChar gameState
    else
        return c

displayState :: GameState -> IO ()
displayState (GameState word' guesses')  =
    putStrLn $ unlines $ case gameStatus gameState of
        Guessing -> fullHangmanImage' ++
            [ "Word to guess: " ++ wordWithGuesses
            , ""
            , "Guesses: " ++ guesses'
            ]
        GameWon -> fullHangmanImage' ++
            [ "CONGRATULATIONS!"
            , "You correctly guessed the word " ++ word'
            , " in " ++ show (length guesses') ++ " tries "
            ]
        GameLost -> fullHangmanImage' ++
            [ "YOU FAILED!"
            , "You failed to guess the word " ++ word'
            ]
    where
        gameState = GameState word' guesses'
        fullHangmanImage' = fullHangmanImage currentHangmanIndex
        currentHangmanIndex = numberOfWrongGuesses gameState
        wordWithGuesses = blankOrChar <$> word'
        blankOrChar c
            | c `elem` guesses' = c
            | otherwise = '_'

gameLoop :: GameState -> IO ()
gameLoop gameState = do
    displayState gameState
    when (gameStatus gameState == Guessing) $ do
        c <- getANewChar gameState
        gameLoop $ gameState { guesses = guesses gameState ++ [c] }

newGame :: IO GameState
newGame = do
    contents <- readFile wordsPath
    let words' = filter validWord $ lines contents
    let wordcount = length words'
    randomNumber <- randomIO
    let randomWord = words' !! (randomNumber `mod` wordcount)
    return $ GameState randomWord []
    where
        validWord word =
            '\'' `notElem` word &&
            map toLower word == word

main :: IO ()
main = do
    newGame >>= gameLoop
    putStrLn "Play again? (y/n):"
    option <- getAChar
    when (option == 'y') main


In case of errors/warnings, write your output to stderr and not just echo them to stdout stream. This might also include you returning the usage section.

Having a chain of if block is imo, always very bad. You can use a switch-case block there, and keep shifting the arguments. A simple example would be:

ARGS=()
while [[ $# -gt 0 ]]
do
    key="$1"

    case $key in
      -v|--verbose)
        verbose='/dev/stdout'
        shift # past argument
        ;;
      --dryrun)
        dryrun=1
        shift # past argument
        ;;
      -k=*|--key=*)
        access="${key#*=}"
        shift # past argument
        shift # past value
        ;;
      -k|--key)
        shift # past argument
        access="$1"
        shift # past value
        ;;
      .
      .
      .
      *)    # unknown option
        if [[ "${key#-}" != ${key} ]]; then
            badflag=1
            >&2 echo "Bad flag: ${var}"
        fi
        ARGS+=("$1") # save it in an array for later
        shift # past argument
        ;;
    esac
done

Notice how easily you can maintain the --key=KEY and --key KEY format parameters. Apart from that, it is really easy to later on add another new argument to your utility.

Keep defining your globals in UPPER_CASE, so that it easy to distinguish them later in your ever-expanding code.

echo "These can be set in the environment variables AMZ_ACCESS_KEY/AMZ_SECRET_KEY or passed with flags"

AWS recommends having the environment variables named AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY. I would consider it breaking the norm if you are not respecting defaults.

Define the usage content (or man page?) as a heredoc:

USAGE=<<EOF

Usage:
s3cp  <file> <s3-url> [--key=<key>] [--secret=<secret>] [--verbose] [--dryrun]
s3cp  <s3-url> <file> [--key=<key>] [--secret=<secret>] [--verbose] [--dryrun]

s3-url:  s3://<bucket>/<Object-Path>

EOF


You can use jQuery's .next() and .prev()

function nextPic() {
  var current = $("#container .top-pic");
  var next = current.next("img");
  if( next.length === 0 ) {
    next = $("#container img:first");
  }
  current.removeClass("top-pic");
  next.addClass("top-pic");
}

function prevPic() {
  var current = $("#container .top-pic");
  var prev = current.prev("img");
  if( prev.length === 0 ) {
    prev = $("#container img:last");
  }
  current.removeClass("top-pic");
  prev.addClass("top-pic");
}

$('#next').click(nextPic);
$('#prev').click(prevPic);

Or, avoiding the duplication:

function switchTopPic(direction) {
  var map = { prev: 'last', next: 'first' },
      current = $("#container .top-pic"),
      target = current[direction]("img");
  if( target.length === 0 ) {
    target = $("#container img")[map[direction]]();
  }
  current.removeClass("top-pic");
  target.addClass("top-pic");
}

$('#next').click(function () { switchTopPic("next") });
$('#prev').click(function () { switchTopPic("prev") });

Or given the markup in the question, the last little bit could just be:

$('#next, #prev').click(function () { switchTopPic(this.id) });

Either way, it should handle an arbitrary number of images (including none at all), and it doesn't require a top-pic to be set ahead of time. All state information is kept in the markup, and does not require the image IDs to be sequential (they don't even need an ID at all).

Edit: Here's a jsfiddle


Make a dict class from scratch in Python

The first thing you need to do is learn how to unittest your code. This will assume Python3.

# filename: test_MyMap.py
import unittest
import unittest.mock as mock
from mymap import MyMap # the filename for your code

class Test(unittest.TestCase):

    # here's where you write the tests.
    def test_default_size(self):
        m = MyMap()
        self.assertEqual(m.size, 10)

    def test_set_method(self):
        """we use unittest.mock.patch to swap in an easy version of
        _getHash that we know cannot fail. This way we don't introduce
        any other reason of failure into our test for MyMap.set"""

        def replace_getHash(self,key):
            return {'string':0, 1:1, tuple(1,2):2}[key]
        with mock.patch("__main__.MyMap._getHash", new=replace_getHash):
            for key,value in [('string','value'),
                              (1,2),
                              (tuple(1,2), tuple(3,4))]:
                m.set(key, value)
                self.assertEqual(m.internal_list[m._getHash(key)], value)

    def test_get_method(self):
        # etc for all the specs you need to fulfill

if __name__ == "__main__":
    """Will run all the TestCase classes, which each run all their methods
    that begin with test_"""
    unittest.main()

The first thing you should do when you get an assignment or any other spec to write from is design a set of unit tests (as above) to write your code to meet. It's much harder to create unit tests from a program than from a spec so do it this way first! Then every time you change the code, check it against the unit tests to make sure you haven't broken anything.

As far as the actual review goes, it's going to be hard to look at until you can design a full suite of tests against it. A quick glance looks like this will fail if you give it a non-iterable key (e.g. an int, which should be doable) and will also lead to hash collision if you give it an anagram (e.g. foo.set("test",3) == foo.set("etst", 3)). These are edge cases that your unit tests should catch.

Beyond that I'd implement item access (so you can do my_map_object[some_key] instead of having to rely on my_map_object.get(some_key)) and it really seems like you could clean up your MyMap.getkeys function. Something like:

def getkeys(self):
    return [keyvalue[0] for keyvalue in self.internal_list if keyvalue is not None]

should work perfectly. It also stands to reason that

def size(self):
    return sum([1 for key in self.getkeys()])

would work too!

Oh and your MyMap._getHash function will break completely, since string will never be iterable. It should be def _getHash(self, string) (and reference self.size()) rather than def _getHash(string, size) unless you make it a @staticmethod (which WOULD make sense except that your size modulo is dependent on your particular mapping!)


Convert geodetic coordinates to geocentric (cartesian)

I can't explain the discrepancy with the online conversion tool without seeing the source code for the latter.

1. Review

The comment for the ellipsoid parameters should give the units for the semi-major axis. Also, I think "reciprocal flattening" would be a better description of the second parameter, given that you use the name rf later on in the code.
The docstring for geodetic_to_geocentric should give the units (degrees) for the latitude and longitude parameters.
"Height above ellipsoid" would be a clearer description for the h parameter.
The parameters to geodetic_to_geocentric could be spelled out in full, which would help understand their meaning. Parameter names are documentation too.
Comments would help understand the meaning of values like N.
The expressions (N + h) * cos(lat), sin(lat), and (1 - 1 / rf) ** 2 are evaluated twice. These could be stored in local variables to avoid duplicated work.

2. Revised code

from math import cos, radians, sin, sqrt

# Ellipsoid parameters: semi major axis in metres, reciprocal flattening.
GRS80 = 6378137, 298.257222100882711
WGS84 = 6378137, 298.257223563

def geodetic_to_geocentric(ellipsoid, latitude, longitude, height):
    """Return geocentric (Cartesian) Coordinates x, y, z corresponding to
    the geodetic coordinates given by latitude and longitude (in
    degrees) and height above ellipsoid. The ellipsoid must be
    specified by a pair (semi-major axis, reciprocal flattening).

    """
    φ = radians(latitude)
    λ = radians(longitude)
    sin_φ = sin(φ)
    a, rf = ellipsoid           # semi-major axis, reciprocal flattening
    e2 = 1 - (1 - 1 / rf) ** 2  # eccentricity squared
    n = a / sqrt(1 - e2 * sin_φ ** 2) # prime vertical radius
    r = (n + height) * cos(φ)   # perpendicular distance from z axis
    x = r * cos(λ)
    y = r * sin(λ)
    z = (n * (1 - e2) + height) * sin_φ
    return x, y, z


You've spotted the problem. Your merge function is flawed.  For each pair it processes, it introduces an extra unnecessary call to merge.  That is, for a list like [1,2,3,4,5,6], instead of your first merge call expanding directly to:

merge [[1],[2],[3],[4],[5],[6]]
= merge $ mpair [1] [2] : mpair [3] [4] : mpair [5] [6] : []

it expands to:

merge [[1],[2],[3],[4],[5],[6]]
= merge $ mpair [1] [2] : merge $ mpair [3] [4]
  : merge $ mpair [5] [6] : merge []

As a result, your count of merge calls is O(n) and your count of mpair calls is O(n^2) (or similar -- I didn't check exactly). when they should be O(log n) and O(n log n) respectively.

Instead, you want to split merge up into two functions:

merge :: Ord a => [[a]] -> [[a]]
merge [] = []
merge [x] = [x]
merge ls = merge (mergePairs ls)

mergePairs :: Ord a => [[a]] -> [[a]]
mergePairs (l1:l2:ls) = mpair l1 l2 : mergePairs ls
mergePairs ls = ls

This will speed up the algorithm enormously, and it will now run on tens of millions of integers.


Naive Solution

(I say naive because of the assumption that the file you are working with is small. From a general design standpoint, it is usually best to prepare for the worst case and assume you may have to read a super long file at some point in time.)

Read the file line by line into a List or a Map, then fetch a random entry from here. (Judging by the brief details of what you're working on, a Map of different objects might be a good choice if you'd like to parse a file into different objects for flights, etc.)

Probably Better Solution

There's a good discussion over here about Reservoir Sampling.

Side notes

Because you posted in Code Review...

Split the reading from the BufferedReaders into separate try-catch blocks. This is just good practice, as in some cases your program may be able to continue executing even if an exception is thrown.
Close your readers in a finally. Check out this post on closing readers
Make file final.
I'm assuming you just slapped the "Puppies" in the return statement, but if you post more complete code, you can get comments and suggestions on all of your code, not just your approach to finding a random line :)


Is it bad when my code is throwing exceptions, but doesn't crash during runtime?

Yes, absolutely, you should be concerned.

Exceptions should be exactly that, exceptional. and should never be part of the regular code path. If there are circumstances that you know would be normal (like not yet being connected to a server) then you should code in a  condition that represents that situation, and make your code a conditional-execution path. For example, in your code, I would have an AtomicBoolean that represents the 'connected' state, and then only process mouse-events when connected.

As a side-note, Exceptions in Java are very 'expensive'. Almost all Java VM's (OpenJDK, Oracle JDK, IBM JDK, Android JDK, etc.) need to inspect the entire call-graph of the code when an exception happens, so that it can build up the stack trace for the exception. In all the VM's I know, that requires that:

The entire VM thread is 'suspended' so that the call-graph does not change
all threads are 'paused' <- looking for evidence of this still
the call stack is inspected and walked up.
all references on the call-stack are chased back in to the source (get line numbers, etc.)

This is very expensive to process. Using exceptions as part of the normal program flow is a bug, and preventing exceptions with conditional statements is always a good idea. For example, the following code example is terrible:

int val = 0;
boolean isNumber = false;
try {
   val = Integer.parseInt(somestring);
   isNumber = true;
} catch (NumberFormatException nfe) {
   // ignore not-numbers, and leave isNumber to be false
}

I see code like the above all over the place.... and it is broken. In one example, I have seen a 50% performance improvement on a CSV reading program by changing code like that to be:

int val = 0;
boolean isNumber = false;
if (someString.matches("\\d+")) {
    try {
       val = Integer.parseInt(somestring);
       isNumber = true;
    } catch (NumberFormatException nfe) {
       // ignore not-numbers, and leave isNumber to be false
    }
}

Even though the code does extra work to pre-validate that the string contains digits, it is faster! (and is even faster if you precompile the Match pattern...!)

So, in your code, I would have:

private final AtomicBoolean isConnected = new AtomicBoolean(false);

and then, in your code:

lblGrid2.addMouseMotionListener(new MouseMotionAdapter() {

    @Override
    public void mouseMoved(MouseEvent m) {

        if (!isConnected.get()) {
            return;
        }

        for (int i = 0; i < shipList.size(); i++) {
            if (shipList.get(i).isActive()) {
                shipList.get(i).moveShip(m.getX(), m.getY());
            }
        }
    }
});

Some discussions about Java Stack traces and exceptions:

Longjumps Considered Inexpensive
How slow are Java exceptions?


Here's something that seems to work OK (here's a demo)

jQuery(function ($) {
  var select   = $("select[name=city]"),
      defaults = select.children(".default"),
      others   = select.children(":not(.default)");

  others.remove();

  select.on("change", function (event) {
    var value = select.val();
    if(value === "more") {
      defaults.remove();
      select.append(others);
    } else if(value === "less") {
      others.remove();
      select.append(defaults);
    }
  });
});

Everything else is stored in the markup, like so:

<select name="city">
  <!-- default options -->
  <option class="default">A</option>
  <option class="default">B</option>
  <option class="default">C</option>
  <option value="more" class="default">Show more</option>

  <!-- expanded options -->
  <option>D</option>
  <option>E</option>
  <option>F</option>
  <option>G</option>
  <option>H</option>
  <option>I</option>
  <option value="less">Show less</option>
</select>

As I said in the comments, .hide and .show doesn't seem to have any effect on <option> elements. Neither does visibility: hidden. In either case, they all show up in the select dropdown. You can disable options, but then they're still there, just greyed out.
Hence, this code forcibly removes/appends the options.

Of course, it might make more sense to simply remove/append only the non-default options. To me it makes more sense that the default options are a subset of all the options, not an entirely different set of options.

Furthermore, it might be good to let the more/less options be added by JS, so clients that don't support JS don't see them.

Putting all that together, you get this.
Without JS, you'll just see a full list. With JS, you get the more/less options and all that jazz.


Your implementation is great and works well, but there are some things you could change.

Eliminate repetitive code

You have two large methods, moveCardLeft and moveCardRight, that do almost the same thing. These can either be combined entirely or the matching code can be separated out to a new method and referenced by the original two. For my example, I chose the former:

moveCard(card, dir) {
  let nextIndex = this.state.colIds.indexOf(card.colid);

  if (dir === "left") nextIndex--;
  else if (dir === "right") nextIndex++;
  else return null;

  if (nextIndex < 0 || nextIndex > this.state.colIds.length - 1) {
    return null;
  }

  let currentCards = this.state.cards.slice();
  for (let i = 0; i < currentCards.length; i++) {
    if (currentCards[i].task === card.task) {
      currentCards[i] = {
        ...currentCards[i],
        colid: this.state.colIds[nextIndex]
      }
    }
  }
  this.setState({ cards: currentCards });
}

With this, you just have to pass in an extra parameter for the direction (left or right). It eliminates the repetition which is almost always beneficial.

Use consistent formatting

This includes indentation, new lines, and general spacing.

As you have it written in your post (maybe this is just a copy/paste error), the map inside of a map in your render method is not indented properly, which will undoubtedly confuse you at some point. Here is what it should look like:

render() {
  let cols = [
    { name: "Winnnie", headerColor: "#8E6E95" },
    { name: "Bob", headerColor: "#39A59C" },
    { name: "Thomas", headerColor: "#344759" },
    { name: "George", headerColor: "#E8741E" }
  ];
  let colCards = this.state.cols.map(c => {
    let cards = this.getColCards(c.id).map(c => {
      return (

           this.moveCard(c, "left")}>{"<   "}
            {c.task}
           this.moveCard(c, "right")}>{"    >"}

      );
    });

    return (
      <CardCol name={c.name} headerColor={c.headerColor} addCard={this.addCard} id={c.id}>
        {cards}
      </CardCol>
    );
  });

  return (

      {colCards}

  )
}

Other thoughts

Use strict equality (===) by default and only fall back to loose/abstract equality (==) when absolutely necessary.
Consider how much more concise your code could be if you got rid of the colIds and simply used the cols instead to organize the cards. Additionally, consider placing the cards inside of their respective cols instead of separating them out into their own object. I haven't tried this on your code, so it may end up being less concise, but it's good to consider various data structures in anything you write.


Monitor progress of written lines of code

This isn't any shorter, but it is more dynamic:

exts=( py cpp h )

count_lines() {
    local find_args=( -false )
    for ext in "${exts[@]}"; do
        find_args+=( -o -name "*.$ext" )
    done

    find ./ "${find_args[@]}" -print0 |
    xargs -o grep -cve "^\s*$" |
    grep -oP '(?<=:).*' |
    awk '{s+=$1} END {print s}'
}

The extensions reside in a bash array that can live in the global scope. It's one place to store your list of interesting files.

In count_lines you can build up an array of arguments to pass on to find. I'm using -false as the first one so that it's simple to add -o -name "*.something in the loop.

You don't need to capture the command output only to print it on the next line. Just let the commands print to stdout directly.

I added -print0 to find and -0 to xargs. These options work well together to be able to handle any filename, regardless of any whitespace in the name.

Note you don't need to use line continuations when the line ends with a pipe (or with a && or ||) -- bash understands that the pipeline continues on the next line.

About your awk command: if your goal is to count lines, don't you want {s+=1} or {s++} instead of {s+=$1}? The latter will only sum any filenames that look like numbers:

$ touch 5 8 a b c
$ ls | awk '{sum += $1; count++} END {print sum, count}'
13 5


About the code itself(not the algorithm used):

Class name usually starts with a capital letter in Java.
Variables' names: it is better to use more meaningful names. For example, a name firstSum is much clearer then fsum.
Variable declaration and initialization: it is a good practice to make a scope of each variable as narrow as possible. It means that fsum and ssum should be declared inside the loop's body.
If the number of iterations of a loop is known, it makes more sense to use for loop then do ... while. Conversely, when the number of iterations is not known, using while is better than using for and break.
Surrounding operators by whitespaces makes the code more readable:

for (int j = index + 1; j < index * 2; j++)

looks better than

for (int j=index+1;j<index*2;j++){

You should use proper and consistent indentation(closing bracket indented by 4 spaces looks strange).
I would declare a magic number(204) as a final static field of this class, not as a local variable(and a name n is not good for it because it is not meaningful).

So an improved version of your code can look like this:

public class NumericalCenter {

    private final static int MAX_CANDIDATE = 204;

    public static void main(String args[]) {
        for (int candidate = 1; candidate <= MAX_CANDIDATE; candidate++) {
            int firstSum = 0;
            for (int i = 1; i < candidate; i++)
                firstSum += i;
            int secondSum = 0;
            int i = candidate + 1;
            while (secondSum < firstSum) {
                secondSum += i;
                i++;
            }
            if (firstSum == secondSum)
                System.out.println(candidate + " is a numerical center");
        }
    }
}

Now about the algorithm itself: let's assume that a candidate number is fixed(let's call it x). Then we get an equation:
$${n(n+1) \over 2} - {x(x+1) \over 2} = {x(x-1) \over 2}$$

It is a quadratic equation(with respect to n), so it can be solved in O(1)(which gives O(MAX_CANDIDATE) time complexity in total). However, it is not necessary for MAX_CANDIDATE = 204.


If you are sure that configuration.EmailFromAddress will always be valid or null, I'd suggest using the null-coalescing operator (??)

SystemUser user = (SystemUser)Session["CurrentUser"];
Data.Configuration configuration = db.Configurations.Single(c => c.ID == user.ID);
var fromAddress = configuration.EmailFromAddress ?? "noreply@test.com";
MailAddress from = new MailAddress(fromAddress, "Test");

In the case that configuration.EmailFromAddress can also be empty, I'd use the ternary operator

SystemUser user = (SystemUser)Session["CurrentUser"];
Data.Configuration configuration = db.Configurations.Single(c => c.ID == user.ID);
var fromAddress = String.IsNullOrEmpty(configuration.EmailFromAddress) ? configuration.EmailFromAddress : "noreply@test.com";
MailAddress from = new MailAddress(fromAddress, "Test");