jQuery-animated navigation menu On the attached fiddle is what I hope will the be future UI for navigation on my own personal portfolio site. I was hoping you guys could critique the JavaScript I wrote to accomplish the most compatibility with the most browsers. I've tested as far back as IE 7 with this code and it works. Whereas the CSS solution is only IE 10+, and the jQuery Animate method is only usable in Chrome.

Below is the attached fiddle and all of the script I use to facilitate this process. Please be as brutal as possible.  I am very new to JS and would like to know as much as possible!

http://jsfiddle.net/PhilFromHeck/um8Pp/

var key = "off";
var menuKey=["off","off","off","off"];
var menuID = [];
var menuState=[0,0,0,0];

$(document).ready(function () {
    init();
    $('.menu').mouseenter(function () {
      for (var i = 0; i < 4; i++) {
        if(menuID[i] == this){
          menuKey=["off","off","off","off"];
          menuKey[i] = "on";
        }
      }
      if(key=="off"){
        key="on";
        doMove();
      }
    });
    $('.menu').mouseleave(function () {
          menuKey=["off","off","off","off"];
    });
});

function init() {
  menuID[0] = document.getElementById('menuOne');
  menuID[1] = document.getElementById('menuTwo');
  menuID[2] = document.getElementById('menuThree');
  menuID[3] = document.getElementById('menuFour');
}

function doMove() {
  var check = "no";
  if(key == "on"){
    for (var i = 0; i < 4; i++) {
      if(menuKey[i] == "on"){
        check = "yes";
        if(menuState[i] < 100){
          menuState[i] = menuState[i]+2;
          menuID[i].style.backgroundPosition="0px " + menuState[i] + "px";
        }
      }
      if(menuKey[i] == "off"){
        if(menuState[i] > 0){
          check = "yes";
          menuState[i] = menuState[i]-2;
          menuID[i].style.backgroundPosition="0px " + menuState[i] + "px";
        }
      }
    }
  if(check=="yes"){
    setTimeout(doMove,1); // call doMove in 20msec
  }
  else{
    key = "off";
  }
  }
}

For beginner code, that's reasonably good.

Instead of calling setTimeout() yourself, why not let jQuery handle the animation for you with .animate()?
Your indentation is wrong, as evidenced by

  }
  }
}

at the end.  I believe that two spaces is too stingy for readability, and also encourages excessive nesting, which is poor programming practice.  For example, the entire doMove() function should do nothing unless key == "on", so you could eliminate one layer of nesting by returning early:

function doMove() {
    if (key != "on") {
        return;
    }
    ...
}

You use magic numbers 4 and 100.  Don't hard-code 4; detect it as menuID.length instead.  You should be able to detect the height as well; it not, then hard-code 100 as a named constant rather than being casually embedded as a loop limit.
Storage of state feels clumsy.  Use booleans (true and false) instead of strings ("on" and "off").  Naming could be better — I suggest menuElements rather than menuID, and animationPosition rather than menuState.  You should be able to use fewer variables.  For example, key and menuKey could be collapsed down to one scalar called activeMenuItem.

var activeMenuItem;
var menuElements = [];
var animationPosition = [0, 0, 0, 0];

$(document).ready(function () {
    init();
    $('.menu').mouseenter(function () {
        activeMenuItem = this;
        doMove();
    });
    $('.menu').mouseleave(function () {
        activeMenuItem = null;
    });
});

function doMove() {
    var check = false;
    for (var i = 0; i < menuElements.length; i++) {
        if (menuElements[i] == activeMenuItem) {
            if (animationPosition[i] < 100) {
                animationPosition[i] += 2;
                menuElements[i].style.backgroundPosition="0px " + animationPosition[i] + "px";
            }
            check = true;
        } else if (animationPosition[i] > 0) {
            animationPosition[i] -= 2;
            menuElements[i].style.backgroundPosition="0px " + animationPosition[i] + "px";
            check = true;
        }
    }
    if (check) {
        setTimeout(doMove, 1); // call doMove in 20msec
    }
}

Initialization of menuID scales poorly with additional menu items.  Try selecting all $('.menu') instead of individually named elements.


Bash Shell Script uses Sed to create and insert multiple lines after a particular line in an existing file This code seems to work, but I'd like someone with shell-fu to review it.

Is the temp file a good idea?  How would I do without?
Is sed the right tool?
Any other general advice for improving my shell script

Script/Code to Review:

# Grab max field lengths from each .hbm.xml file and
# put them into the corresponding .java file
for myFile in $(find generatedSchema/myApp/db/ -name *.hbm.xml)
do
    # Calculate the java file name
    javaFile=${myFile/%\.hbm\.xml/.java}
    echo $javaFile

    # Find each field name and length and format it for the java file.
    # Save result lines to temp.txt
    sed -n '/<property name="[^"]\+" type="string">/{
        N
        s/ \+<property name="\([^"]\+\)" type="string">\n \+<column name="[^"]\+" length="\([0-9]\+\)".*/    @SuppressWarnings({"UnusedDeclaration"}) public static final int MAX_\1 = \2;/p}' $myFile >temp.txt

    # Load results from temp file into a shell variable
    str=""
    while read line
    do
        echo $line
        str="$str\n    $line"
    done < temp.txt

    # Put new lines after the serialVersinUID line in the existing Java file.
    sed -i -e "s/^    private static final long serialVersionUID = [0-9]\+L;$/\0\n$str/" $javaFile

    # clean up
    rm temp.txt
done

Existing Input .hbm.xml file excerpt:

<?xml version="1.0"?>
<!DOCTYPE hibernate-mapping PUBLIC "-//Hibernate/Hibernate Mapping DTD 3.0//EN"
          "http://hibernate.sourceforge.net/hibernate-mapping-3.0.dtd">
<hibernate-mapping>
    <class name="com.myCo.CodeReview" table="code_review" catalog="stackexchange">
        <!-- ... -->
        <property name="address1C" type="string">
            <column name="address1_c" length="50" />
        </property>
        <property name="address2C" type="string">
            <column name="address2_c" length="50" />
        </property>
        <!-- etc. -->

Existing Input Java File:

// ... preserve java code above

private static final long serialVersionUID = 20130318164201L;

// ... preserve java code below

Desired Output Java File:

// ... preserved java code above

private static final long serialVersionUID = 20130318164201L;

@SuppressWarnings({"UnusedDeclaration"}) public static final int MAX_address1C = 50;
@SuppressWarnings({"UnusedDeclaration"}) public static final int MAX_address2C = 50;

// ... preserved java code below

Background

This script generates Java objects that represent database tables.  It modifies the output of the Hibernate Database Reverse-Engineering tools to add maximum field length tokens to my Java objects that match the length of those columns in the database.

1. Comments on your code

Your example input file is not a well-formed XML document. An XML document must have a single root element, but your example has two. What tool did you use to generate this file? You should ask yourself how it went wrong.

(If you're just writing it "by hand" in some other program using a bunch of print statements, you might ask yourself why you are using XML at all rather than something simple like CSV. You clearly can't be intending to process these file using standard XML tools, since most XML tools will refuse to process ill-formed documents.)
In the find command:

find generatedSchema/myApp/db/ -name *.hbm.xml

the pattern *.hbm.xml would be expanded by the shell (rather than the find command) if there happened to be any files matching the pattern in the directory where this command is being run. So single-quote the pattern to make sure this can't happen:

find generatedSchema/myApp/db/ -name '*.hbm.xml'

You might also consider adding the argument -type f to ensure that the names returned all refer to regular files. (It's not very likely in your case that someone will name a directory foo.hbm.xml, but adding a -type argument to a find command is often good practice.)
In this line you don't need to escape the dots:

javaFile=${myFile/%\.hbm\.xml/.java}

The pattern in ${VAR/pat/rep} here is not a regular expression, so dots are not special. For example:

$ FOO='a.b.c'
$ echo ${FOO/./|}
a|b.c

You use regular expressions to transform your XML into Java source. Now, as I'm sure you're aware, you can't reliably process XML using regular expressions. Perhaps what you've got here works because you control the format of these XML files. But really you should be using an XML-aware tool. See section 2 below.
You use a temporary file to store the output of your sed command, and then load it into a variable using a loop over its lines.

You don't need this loop, because you can load the contents of a file into a variable by writing:

VARIABLE=$(<filename)

And you don't need the temporary file either, because you can load the output of a command into a variable by writing

VARIABLE=$(command)

So just write

str=$(sed -n "...")

You could choose better variable names than myFile (why yours?) and str (what's in the string?).
In your last sed command, you use the expression s/pattern/\0\n$str/ to append $str after every match of pattern. You might consider using p;c instead so that you can simplify the pattern (it doesn't have to match the whole line any more):

sed -i -e "/^private static final long serialVersionUID/{p;c\
$str
}"

You expand variables myFile and javaFile in your script in contexts like this:

sed -i -e "..." $javaFile

This will go wrong if the variable javaFile contains spaces. You should double-quote the variable to ensure that it's treated as a single argument to sed even if it contains spaces. (Perhaps you know in this case that it can't contain spaces, but double-quoting variables containing filenames is still a good habit to get into.)

sed -i -e "..." "$javaFile"

I don't like the way you modify the Java source code in-place. This would be dangerous if this is your original Java source code, because a bug in your script could overwrite the Java source code with nonsense and break your build. (But since I can't see the rest of your build system I can't tell if this concern is justified.)

If the whole Java file is automatically generated, wouldn't it be better to do it in one go rather than piecemeal like this? I have a feeling that there are more improvements to be made to this code generation process.

2. Using a programming language with XML support

If you need to process data from XML documents, then use a programming language together with an XML parsing library.

For example, here's how you might rewrite this same script in Python. First, you'd change the way you generate your .hbm.xml files so that they are well-formed XML documents. Let's suppose that they now look like this:

<properties>
  <property name="address1C" type="string">
    <column name="address1_c" length="50" />
  </property>
  <property name="address2C" type="string">
    <column name="address2_c" length="50" />
  </property>
</properties>

Then you might write the following Python program:

#!/usr/bin/python

import fileinput
import sys
import xml.etree.ElementTree

def main():
    # Check command-line argument.
    hbm_xml_filename = sys.argv[1]
    ext = '.hbm.xml'
    if not hbm_xml_filename.endswith(ext):
        raise RuntimeError("Filename {} doesn't end with {}"
                           .format(hbm_xml_filename, ext))

    # Load property names and maximum lengths from input file.
    tree = xml.etree.ElementTree.parse(hbm_xml_filename)
    properties = []
    for property in tree.findall('property[@type="string"]'):
        column = property.find('column')
        properties.append(dict(name = property.attrib['name'],
                               length = column.attrib['length']))

    # Rewrite Java in-place to add "MAX_name = length;" declarations.
    java_filename = hbm_xml_filename.replace(ext, '.java')
    for line in fileinput.input(files=[java_filename], inplace=True):
        sys.stdout.write(line)
        if line.startswith('private static final long serialVersionUID'):
            sys.stdout.write('\n')
            for p in properties:
                sys.stdout.write(
                    '@SuppressWarnings({{"UnusedDeclaration"}}) '
                    'public static final int MAX_{name} = {length};\n'
                    .format(**p))

if __name__ == '__main__':
    main()

You'll see that I've used the xml.etree.ElementTree module to parse and query the XML, I've used an XPath query to find all properties with the attribute type="string", and I've used the fileinput module to update the Java source code in-place.

Then you can run this Python program from your shell script:

find generatedSchema/myApp/db/ -type f -name '*.hbm.xml' |
while read property_file; do
    add_property_lengths.py "$property_file"
done

3. Addendum

You said you were having some difficulty getting VAR=$(command) to work. You should be able to verify that it works in general by testing it in the shell:

$ NUMBERS=$(yes '' | head | nl -ba)
$ echo $NUMBERS
1 2 3 4 5 6 7 8 9 10

Clearly we have different versions of sed (I'm using the BSD-ish version that comes with Mac OS X, so I had to specify -E for extended regular expressions and change the regular expression syntax a bit) but the following works for me:

VAR=$(sed -E -n '/ *<property name="([^"]+)" type="string">/{
    N
    s/ *<property name="([^"]+)" type="string">\n *<column name="[^"]+" length="([0-9]+)".*/    @SuppressWarnings({"UnusedDeclaration"}) public static final int MAX_\1 = \2;/
    p
}' "$myFile")

so you should be able to figure out how to make something like it work for you (if you must do it this way).


Capitalize words in sentence public static class StringHelpers
{
    public static string CapitalizeEachWord(this string sentence)
    {
        CultureInfo cultureInfo = Thread.CurrentThread.CurrentCulture;
        TextInfo textInfo = cultureInfo.TextInfo;

        if (String.IsNullOrEmpty(sentence))
        {
            return String.Empty;
        }

        return textInfo.ToTitleCase(sentence);
    }

    public static string CapitalizeFirstWord(this string sentence)
    {
        if (String.IsNullOrEmpty(sentence))
        {
            return String.Empty;
        }

        var words = sentence.Split();
        StringBuilder resultBuilder = new StringBuilder();

        var letters = words[0].ToCharArray();
        List<string> lettersHolder = new List<string>();
        lettersHolder.Add(letters[0].ToString().ToUpper());
        for (int i = 1; i < letters.Length; i++)
        {
            lettersHolder.Add(letters[i].ToString().ToLower());
        }

        foreach (var letter in lettersHolder)
        {
            resultBuilder.Append(letter);
        }

        resultBuilder.Append(" ");

        for (int i = 1; i < words.Length; i++)
        {
            resultBuilder.Append(words[i].ToLower());
            resultBuilder.Append(" ");
        }

        return resultBuilder.ToString();
    }
}

The first method, is supposed to correctly capitalize each word in a sentence. I don't think there's much room for improvement in that one.

The second method, seems a little confusing and even after I've written it and tested it, I cannot seem to fully put what it's doing into my buffer. That's a sign of code smell.

If I kinda get it now, what will happen down the line when I have to change it or something.

You're right, your second method is way too complicated. It also doesn't handle the case where the string starts with a space (in which case words[0] will be the empty string and letters[0] will be out of bounds). Since all your method seems to be doing is to capitalize the first letter of the string and lower case everything else, why not just do that?

public static string CapitalizeFirstWord(this string sentence)
{
    if (String.IsNullOrEmpty(sentence))
    {
        return String.Empty;
    }

    String firstLetter = sentence.Substring(0,1);
    String rest = sentence.Substring(1);
    return firstLetter.ToUpper() + rest.ToLower();
}


Get initials from name input by user This was an exercise from cs50. The goal was to get the initials in upper letter of a name that we got from user. So for "barack obama" we would get "BO", input like "abc" should output "A" and so on. I am interested in knowing what I can do to simplify it or be more efficient.

Do I need the check against '\0' in the while loop? I ask because the way I see it, when a user inputs a name, there will always be a '\n'(right ?), so it gets out of the while loop when its position has '\n' and then out of for loop so '\0' is never actually checked. Let me know if logic is right. I kept it there just in case.

#include <stdio.h>
#include <ctype.h>

#define NAME_LEN 80 + 1

int main(void)
{
    char userName[NAME_LEN], userInitials[NAME_LEN];
    int i, pos = -1;

    // ask user for a name
    fgets(userName, NAME_LEN, stdin);
    for(i = 0; userName[i] != '\n' && userName[i] != '\0'; i++)
    {
        if (userName[i] >= 'a' && userName[i] <= 'z')
        {
            userInitials[++pos] = userName[i] - ('a' - 'A');
        }
        else
        {
            userInitials[++pos] = userName[i];
        }
        while ((!isblank(userName[i])) && (userName[i] != '\n') && (userName[i] != '\0'))
        {
            i++;
        }
    }
    userInitials[++pos] = '\0';
    printf("%s\n", userInitials);

}

The compiler doubles as a code-analysis tool

Compiling your file like: gcc usename.c -Wall -pedantic Warns you of a bad practice in your code:

usename.c:12:5: warning: ignoring return value of ‘fgets’, declared with attribute warn_unused_result [-Wunused-result]
     fgets(userName, NAME_LEN, stdin);

You should use the return value to check if input reading happened correctly.

Small helper function to increase readibility

The line

userInitials[++pos] = userName[i] - ('a' - 'A');

is not so immediate to understand, I suggest writing a tiny function:

char to_uppercase(char letter) {
    return letter - ('a' - 'A');
}

and calling it like:

 userInitials[++pos] = to_uppercase(userName[i]);

The C99 Standard and variable scope

All major C compiler support C99, I suggest using the appropriate flag to enable it and write:

for(int i = 0; userName[i] != '\n' && userName[i] != '\0'; i++)

to shorten the scope of i.

Excellent parametrization

You defined a constant for NAME_LEN and named it meanigfully instead of sprinkling a random number all over your code, excellent!

For further improvement I suggest using a

static const int NAME_LEN = 80 + 1

For reasons discussed here.


PDO class and security I have started a project in PHP with PDO and I'm almost done, but I've read somewhere that PDO escape alone is not secure and we have to consider some settings of PDP. I am a little confused about my PDO class and usage.

public function __construct(){
    //set DNS
    $dns = 'mysql:host=' . $this->host . ';dbname=' . $this->dbname.';charset=utf8';
    //set options
    $options = array(PDO::ATTR_PERSISTENT => true, PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, PDO::MYSQL_ATTR_INIT_COMMAND => "SET NAMES utf8",PDO::ATTR_EMULATE_PREPARES => false );
    //create a new PDO instance
    try{
        $this->dbh = new PDO($dns, $this->user, $this->pass, $options);
    } //catch any errors
    catch(PDOException $e){
        $this->error = $e->getMessage();
    }
}

public function query($query){
    $this->stmt = $this->dbh->prepare($query);
}

public function bind($param, $value, $type = null){
    if(is_null($type)){
        switch(true){
            case is_int($value):
                $type = PDO::PARAM_INT;
                break;
            case is_bool($value):
                $type = PDO::PARAM_BOOL;
                break;
            case is_null($value):
                $type = PDO::PARAM_NULL;
                break;
            default:
                $type = PDO::PARAM_STR;
        }
    }
    $this->stmt->bindValue($param, $value, $type);
}

public function execute(){
    return $this->stmt->execute();
}

public function resultset(){
    $this->execute();
    return $this->stmt->fetchAll(PDO::FETCH_OBJ);
}

public function single(){
    $this->execute();
    return $this->stmt->fetch(PDO::FETCH_OBJ);
}

public function rowCount(){
    return $this->stmt->rowCount();
}

Queries:

$db->query("SELECT * FROM XYZ WHERE some=:some");
$db->bind(':some', $$somevar);
$result= $db->resultset();   ///for select

$result= $db->single();  ///if we want single row

For inserting and updating:

$db->query("INSERT INTO XYZ (x,y,z) VALUES (:x,:y,:z)");
$db->bind(':x', $x);
$db->bind(':y', $y);
$db->bind(':z', $z);
$db->execute();

Is there anything wrong which will affect me in future? I have searched many things for PDO and have updated my class accordingly, but I want to be sure about my project.

There are a few things which I consider required. First of all I think your code does a good job. Below I have written some of the things I would also implement.

First of all. Be careful when echoing exception message directly. Especially database exceptions has a tendency to contain connection credentials. Imagine an error happens in your application and a malicious user sees this message. He could then log-in onto your database and wreck havoc! Try to keep the amount of sensitive information out of exception messages users are able to see.

From what I can see your class wraps a PDO instance to make certain tasks easier. When you wrap your PDO instance inside a class your lose some of PDO capabilities. As far as I can see from the code I have no way to perform a transaction. This may not be useful to you now due to an older MySql version, but when you sometimes in the future upgrade (I hope you do :D) this might be desired. I would provide a way to interact with the PDO instance from outside the class. A simple pdo() method would solve that problem.

/**
 * Provides access to the application PDO instance.
 *
 * @return \PDO
 */
public function pdo() {
    return $this->dbh;
}

When looking at your execute, resultset and single methods I see three flaws. The first being a potential double execution of your query. Imagine you sometime in the future call the execute method. Then you follow that by calling the resultset or single method. Now you have executed the query twice. This may not cause bugs when you are fetching data, but if the query is an insert statement, which populates an unique column, you will get an SQL error. I would remove the calls to the PDO execute method inside the resultset and single methods. This would effectively eliminate all possible bugs of this kind. Keep in mind you should also perform some error checking to see if the current statement has been executed or you would otherwise get an error. This could be solved by setting a flag indicating if the statement has been executed.

private $executed = false;

public function execute() {
    $this->stmt->execute();
    $this->executed = true; // Set the flag.
}

The second flaw is that you are unable to close the cursor for the current statement. By closing the cursor you gain the ability to execute the statement once more with different parameters. You would also gain some (small) performance increase on huge queries. Take a look at the documentation for the closeCursor method.

public function closeCursor() {
    $this->stmt->closeCursor();
    $this->executed = false; // Reset the execution flag.
}

The third flaw which I really think is required regards your resultset and single methods once more. By default they will fetch the result-set into an object. This is a good practice when writing OOP code, but often you will find yourself fetching result sets into specific classes. Consider passing the data directly into an User class or passing an object of StdClass into the user class, so the user class can fetch the data. By passing directly into the class you save some performance and make the code more readable. I would also change the code, so you are able to fetch the result set as an array. Sometimes this may be preferred and you cannot predict the future. You could change the method signature of resultset (and single too) to something like the following:

/**
 * Fetch entire result-set of the current query.
 *
 * @param integer     $mode  A PDO constant declaring the fetch mode.
 * @param string|null $class A qualified class name to fetch the result into.
 * @param array       $args  Constructor arguments for the custom class.
 *
 * @return mixed Returns the result set according to the specified fetch mode.
 *               The default mode is to fetch an object of type StdClass.
 */
public function resultset($mode = PDO::FETCH_OBJ, $class = null, array $args = []) {

    if(!is_null($class) && in_array($mode, [PDO::FETCH_CLASS, PDO::FETCH_OBJ])) {

        return $this->stmt->fetchAll(PDO::FETCH_CLASS; $class, $args);

    }

    return $this->stmt->fetchAll($mode);

}

/**
 * Fetch one row of the current query.
 *
 * @param integer     $mode  A PDO constant declaring the fetch mode.
 * @param string|null $class A qualified class name to fetch the result into.
 * @param array       $args  Constructor arguments for the custom class.
 *
 * @return mixed Returns the result set according to the specified fetch mode.
 *               The default mode is to fetch an object of type StdClass.
 */
public function single($mode = PDO::FETCH_OBJ, $class = null, array $args = []) {

    if(!is_null($class) && in_array($mode, [PDO::FETCH_CLASS, PDO::FETCH_OBJ])) {

        return $this->stmt->fetchObject($class, $args);

    }

    return $this->stmt->fetch($mode);

}

There is also another thing which I think is important. You are creating your PDO connection in the constructor effectively making the parameters for the connection hardcoded (or so it looks from $this->user due to lack of a constructor argument). I would extract the connection functionality into a separate class. This will also allow you to use different connections with the same database class. I would write something like the following. First I would alter the constructor in your current class.

public function __construct(PDOConnection $connection) {

    $this->dbh = $connection->getConnection();

}

Then I would write the PDOConnection class.

class PDOConnection {

    private $instance;

    private $dsn;
    private $username;
    private $password;
    private $options = [];

    public function __construct($dsn, $username, $password, array $options = []) {
        $this->dsn      = $dsn;
        $this->username = $username;
        $this->password = $password;
        $this->options  = $options;
    }

    public function setAttribute($name, $value) {

        if(!$this->instance instanceof PDO) {
            throw new LogicException('Cannot set PDO attribute. Please make sure you are connected using the connect() method.');
        }

        if($this->instance->setAttribute($name, $value) === false) {
            throw new LogicException('Could not set PDO attribute: ' . $name);
        }

    }

    public function setOption($name, $value) {
        $this->options[$name] = $value;
    }

    public function getConnection() {

        if(!$this->instance instanceof PDO) {
            throw new LogicException('No database connection established.');
        }
        return $this->instance;

    }

    public function connect() {

        try {

            $this->instance = new PDO($this->dsn, $this->username, $this->password, $this->options);

        }catch(PDOException $exception) {

            throw new ErrorException('Could not connect to the database!', null, $exception);

        }

    }

    public function disconnect() {
        $this->instance = null;
    }

}

Keep in mind this code has no error checking and not been tested, so there might be an error.

The usage of these classes would be like the following. I have named your class Database since I couldn't see the original name from your post.

$dsn  = 'mysql:host=127.0.0.1;dbname=AWESOMENESS;charset=utf8';
$user = 'username';
$pass = 'password';

$connection = new PDOConnection($dsn, $user, $pass, [
    PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION,
    PDO::ATTR_CURSOR  => PDO::CURSOR_FWDONLY, // Scrollable cursors may be unavailable or very expensive on MySql
    PDO::ATTR_EMULATE_PREPARES => false,
    PDO::ATTR_PERSISTENT => false
]);

$connection->connect();

$database = new Database($connection);

I hope this can help guide you in the right direction, happy coding!


Getting a valid datetime format from a string I have this code in a class that gets a string, parses it, and makes some adjustments to get a valid datetime format from the string.

An input string for this function could be = "A0X031716506774"

import datetime
def _eventdatetime(self, string):
    base_date = datetime.datetime.strptime("1980-1-6 0:0", "%Y-%m-%d %H:%M")
    weeks = datetime.timedelta(weeks = int(string[5:9]))
    days = datetime.timedelta(days = int(string[9]))
    seconds = datetime.timedelta(seconds = int(string[10:15]))
    stamp = base_date + weeks + days + seconds

    adjustUTC = datetime.timedelta(hours = 3)
    stamp =  stamp - adjustUTC

    return str(stamp)

Total time: 0.0483931 s

How can I improve the performance for this? The third line seems to be the slowest.

import datetime
def _eventdatetime(self, string):
    base_date = datetime.datetime.strptime("1980-1-6 0:0", "%Y-%m-%d %H:%M")

Why are you specifying the date as a string only to parse it? Just do: base_date = datetime.datetime(1980,1,6,0,0). You should also make it a global constant so as not to recreate it all the time.

    weeks = datetime.timedelta(weeks = int(string[5:9]))
    days = datetime.timedelta(days = int(string[9]))
    seconds = datetime.timedelta(seconds = int(string[10:15]))

There is no need to create several timedelta objects, you can do datetime.timedelta(weeks = ..., days = ..., seconds = ...)

    stamp = base_date + weeks + days + seconds

    adjustUTC = datetime.timedelta(hours = 3)
    stamp =  stamp - adjustUTC

I'd use stamp -= datetime.timedelta(hours = 3) # adjust for UTC

    return str(stamp)


Pythonic indenting `with` statement blocks Python doesn't remove the variables once a scope is closed (if there's such a thing as scope in Python, I'm not sure). I mean, once I finish a for variable in range(1, 100), the variable is still there, with a value (99 in this case).

Now I usually indent a whole block when I use the with ... as statement. But should I close the file and end the indentation as soon as I'm done with it? I mean, should I write:

with open('somefile', 'r') as f:
    newvar = f.read()

newvar.replace('a', 'b') # and etc

Instead of:

with open('somefile', 'r') as f:
    newvar = f.read()
    newvar.replace('a', 'b') # and etc

Yes, ideally, you close the with block as soon as possible, that way the file gets closed quickly - which means you are not (potentially) blocking access to it, having less file handles in use, etc...

Python does have scoping, it just doesn't make new scopes very often - only for functions and classes (and some other smaller cases, like list comprehensions and cousins in 3.x and above).

Unlike in Java, Python doesn't take the approach that every block is a scope, therefore loops, as you say in your question, don't introduce a new one.


Sum of left and right nodes (individually) in a Binary Search Tree I have written a program which calculates the sum of all the left nodes and the sum of right nodes in a Binary search tree.

I have used BFS to traverse the tree. The code is as follows:

import java.util.ArrayList;
import java.util.LinkedList;
import java.util.List;
import java.util.Queue;

public class SumLeftRightNodes {

/**
 * This class represents the individual nodes of the binary tree
 * Each node has a left, right pointer of type Node
 * and Value to hold the value
 * @author Aneesh
 *
 */
static class Node {
    Node left;

    Node right;

    int value;

    public Node(int value) {
        this.value = value;
    }

    @Override
    public String toString() {
        return "Node value=" + value + "";
    }

}

/**
 * This function inserts an element into the binary tree
 * @param node
 * @param value
 */
public static void insert(Node node, int value) {
    if (value < node.value) {
        if (node.left != null) {
            insert(node.left, value);
        } else {
            System.out.println("  Inserted " + value + " to left of "
                    + node.value);
            node.left = new Node(value);
        }
    } else if (value > node.value) {
        if (node.right != null) {
            insert(node.right, value);
        } else {
            System.out.println("  Inserted " + value + " to right of "
                    + node.value);
            node.right = new Node(value);
        }
    }
}

public static void main(String[] args) {
    // TODO Auto-generated method stub
    Node root = new Node(5);
    System.out.println("Binary Tree Example");
    System.out.println("Building tree with root value " + root.value);
    insert(root, 1);
    insert(root, 8);
    insert(root,-2);
    insert(root, 6);
    insert(root, 3);
    insert(root, 9);
    insert(root,-3);
    insert(root,-1);

    System.out.println("sum of all left and right nodes is as follows\n");
    sumOfLeftandRightNodes(root);
}

public static void sumOfLeftandRightNodes(Node root) {
    // TODO Auto-generated method stub
    long leftSum = 0, rightSum = 0;

    Queue<Node> nodes = new LinkedList<Node>();

    //add the root to the Queue
    nodes.add(root);

    List<Node> leftNodes = new ArrayList<Node>() , rightNodes = new ArrayList<Node>();

    while (!nodes.isEmpty()){

        //BFS traversal
        Node temp = nodes.poll();

        //if left child is present then add the value of the node to the left Count
        if (temp.left!=null){
            leftSum += temp.left.value;
            leftNodes.add(temp.left);
            nodes.add(temp.left);
        }
        //if right child is present then add the value of the node to the right Count
        if (temp.right!=null){
            rightSum += temp.right.value;
            nodes.add(temp.right);
            rightNodes.add(temp.right);
        }
    }

    System.out.println("left = "+leftNodes+" \nRight Nodes = "+rightNodes);
    System.out.println("left sum ="+leftSum+"\nRight sum="+rightSum);
}

}

Any suggestion and improvements are welcome. If there is an easier method to this problem then I would like to know.

This code works and it seems quite fine.

  If there is an easier method to this problem then I would like to know.

You could do it without a Queue,
using a recursive solution,
if your tree is not too deep to lead to a stack overflow.
But since you need to track two kinds of values,
you would need to accumulate them in a helper object,
for example:

private static class Result {
    private long leftSum;
    private long rightSum;
}

public static void sumOfLeftandRightNodes(Node node, Result result) {
    if (node.left != null) {
        result.leftSum += node.left.value;
        sumOfLeftandRightNodes(node.left, result);
    }
    if (node.right != null) {
        result.rightSum += node.right.value;
        sumOfLeftandRightNodes(node.right, result);
    }
}

You could call this method with:

Result result = new Result();
sumOfLeftandRightNodes(root, result);

One advantage of this method is that the solution will become unit-testable,
thanks to this helper class.
When the above call returns, you could add test cases to validate your solution,
for example:

assertEquals(2, result.leftSum);
assertEquals(19, result.rightSum);

Coding style

I would recommend to make everything private when you can,
and final when you can.
The Node class also has a bit too much vertical spacing:

static class Node {
    Node left;

    Node right;

    int value;

I suggest to write this way:

private static class Node {
    private Node left;
    private Node right;
    private final int value;

In the breadth-first search implementation,
temp for the loop variable is not a great name.
I recommend to rename it to node.


Reading a textfile and calling a class with each text file line How could I get a better performance in my C# application? Or what should I change in my source code?

I would appreciate any kind of help.

This code is reading each line of a textfile which stores values like var1:var2 and calls a class with those values.

Function which calls the class:

private void bunifuFlatButton2_Click(object sender, EventArgs e)
{
    using (var streamReader = File.OpenText(filePath))
    {
        var lines = streamReader.ReadToEnd().Split(new char[] { '\n' });
        var count = lines.Count();

        bunifuProgressBar1.MaximumValue = count;

        for (int i = 0; i < count; i++)
        {
            string[] words = lines[i].Split(':');
            string hash = mega.GenerateHash(words[0].ToLowerInvariant(), mega.PrepareKey(words[1].ToBytes()));

            bunifuCustomLabel4.Text = "Working: Total: " + count + ". Currently: " + (i + 1) + ".";
            bunifuCustomLabel4.Update();
            bunifuProgressBar1.Value = i + 1;
        }
    }
}

The class:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Security.Cryptography;
using System.Text;
using System.Threading.Tasks;

namespace MegaTool
{
    public static class mega
    {

        private static readonly Rijndael RijndaelCbc;
        private static readonly byte[] DefaultIv = new byte[16];

        static mega()
        {
            RijndaelCbc = Rijndael.Create();
            RijndaelCbc.Padding = PaddingMode.None;
            RijndaelCbc.Mode = CipherMode.CBC;
        }

        public static byte[] ToBytes(this string data)
        {
            return Encoding.UTF8.GetBytes(data);
        }

        public static byte[] PrepareKey(byte[] data)
        {
            byte[] pkey = new byte[] { 0x93, 0xC4, 0x67, 0xE3, 0x7D, 0xB0, 0xC7, 0xA4, 0xD1, 0xBE, 0x3F, 0x81, 0x01, 0x52, 0xCB, 0x56 };

            for (int it = 0; it < 65536; it++)
            {
                for (int idx = 0; idx < data.Length; idx += 16)
                {
                    byte[] key = data.CopySubArray(16, idx);

                    pkey = EncryptAes(pkey, key);
                }
            }

            return pkey;
        }

        public static byte[] EncryptAes(byte[] data, byte[] key)
        {
            using (ICryptoTransform encryptor = RijndaelCbc.CreateEncryptor(key, DefaultIv))
            {
                return encryptor.TransformFinalBlock(data, 0, data.Length);
            }
        }

        public static T[] CopySubArray<T>(this T[] source, int length, int offset = 0)
        {
            T[] result = new T[length];
            while (--length >= 0)
            {
                if (source.Length > offset + length)
                {
                    result[length] = source[offset + length];
                }
            }

            return result;
        }

        public static string GenerateHash(string email, byte[] passwordAesKey)
        {
            byte[] emailBytes = email.ToBytes();
            byte[] hash = new byte[16];

            for (int i = 0; i < emailBytes.Length; i++)
            {
                hash[i % 16] ^= emailBytes[i];
            }

            for (int it = 0; it < 16384; it++)
            {
                hash = EncryptAes(hash, passwordAesKey);
            }

            byte[] result = new byte[8];
            Array.Copy(hash, 0, result, 0, 4);
            Array.Copy(hash, 8, result, 4, 4);

            return result.ToBase64();
        }

        public static string ToBase64(this byte[] data)
        {
            StringBuilder sb = new StringBuilder();
            sb.Append(Convert.ToBase64String(data));
            sb.Replace('+', '-');
            sb.Replace('/', '_');
            sb.Replace("=", string.Empty);

            return sb.ToString();
        }
    }
}

public static T[] CopySubArray<T>(this T[] source, int length, int offset = 0)
{
    T[] result = new T[length];
    while (--length >= 0)
    {
        if (source.Length > offset + length)
        {
            result[length] = source[offset + length];
        }
    }

    return result;
}

I wonder why did you write this method? You can replace it with Array.Copy that, strangely enough, you already use in another place and even the exact same overload:

Array.Copy(hash, 0, result, 0, 4);

Signature:

public static void Copy(
    Array sourceArray,
    int sourceIndex,
    Array destinationArray,
    int destinationIndex,
    int length
)

public static string GenerateHash(string email, byte[] passwordAesKey)

This method should only generate a hash and not an encoded string. Encoding the hash is a responsibility for a different method.

ToBase64

Likewise is this method doing too much. It not only creates a base64 string but also replaces some characters. This should moved to a new method like CleanUpBase64String or whatever it does. It can be private but this kind of custom processing should be encapsulated.

Basically this is how it should look like:

public static byte[] GenerateHash(string email, byte[] passwordAesKey)
{
    ..
}

public static string GenerateHashBase64String(string email, byte[] passwordAesKey)
{
    var hash = GenerateHash(email, passwordAesKey);
    return ToEncodedBase64(hash);
}

public static string ToEncodedBase64(this byte[] data)
{
    var base64String = Convert.ToBase64String(data)
    return CleanUpBase64String(base64String);
}

private static string CleanUpBase64String(string value)
{
    ...
}


Programming Pearls -- Column 1: Sorting unique numbers with bitfield I've implemented a simple program to sort an input of unique  numbers given as decimals on a separate line.  The idea is, since all numbers are unique, to use a (long) bitfield where a 1 on index n represents that the number n is in the sorted list.  The algorithm thus consists of these phases (as described in the book):
/* phase 1: initialize set to empty */
    for i = [0, n)
        bit[i] = 0
/* phase 2: insert present elements into the set */
    for each i in the input file
        bit[i] = 1
/* phase 3: write sorted output */
    for i = [0, n)
        if bit[i] == 1
            write i on the output file

In order to (re-)freshen my Rust, I chose to implement this in Rust, to be an efficient replacement for sort(1) when given the -n option, assuming unique input.  Thus the program can be called as:
$ bitfield # sorts stdin, writes to stdout
$ bitfield foo # sorts foo, writes to stdout
$ bitfield foo - bar # sorts foo, stdin, bar, writes to stdout

As with the exercise, I assume n (MAXNUM) to be 10,000,000:
use std::io::*;
use std::fs::File;
use std::env;

const MAXNUM: usize = 10_000_000;
const _MAXLINES: usize = 1_000_000;

fn fill(src: impl BufRead, bitfield: &mut[u8; MAXNUM >> 3]) -> Result<()> {
    for line in src.lines() {
        let num = line?.parse::<usize>().unwrap();
        if num > MAXNUM {
            return Err(Error::new(ErrorKind::Other, "number too large!"));
        }

        bitfield[num >> 3] |= 1 << (num & 0b111);
    }

    return Ok(());
}

fn print_sorted(bitfield: &[u8; MAXNUM >> 3], dest: impl Write) -> Result<()> {
    let mut dest = BufWriter::new(dest);

    for (i, byte) in bitfield.iter().enumerate() {
        for bit in 0..=0b111 {
            if byte & (1 << bit) != 0 {
                write!(&mut dest, "{}\n", (i << 3) | bit)?;
            }
        }
    }

    return Ok(());
}

fn main() -> Result<()> {
    let mut bitfield: [u8; MAXNUM >> 3] = [0; MAXNUM >> 3];

    let args: Vec<_> = env::args().skip(1).collect();
    if args.is_empty() {
        fill(stdin().lock(), &mut bitfield)?;
    }

    for arg in args {
        if arg == "-" {
            fill(stdin().lock(), &mut bitfield)?;
        } else {
            let f = match File::open(&arg) {
                Err(e) => {
                    eprintln!("{}: {}", &arg, e);
                    continue;
                }
                Ok(f) => BufReader::new(f),
            };
            fill(f, &mut bitfield)?;
        }
    }
    print_sorted(&bitfield, stdout())?;

    return Ok(());
}

To test, I used GNU/shuf(1), time(1), and cmp(1):
$ shuf -i 1-10000000 -n 1000000 > foo
$ time ./target/release/bitsort foo > bar
0.19s user 0.03s system 97% cpu 0.221 total
$ time sort -n foo > bar2
1.56s user 0.06s system 291% cpu 0.556 total
$ cmp bar bar2

Ideas?

use declarations
use std::env;
use std::fs::File;
use std::io::*;

That's a sneaky way to import io::{Error, Result}.  For the sake of
readability, it may be beneficial to spell out io:: each time.  Also
see the Error handling section below.
use declarations can be condensed.  For example:
use std::{env, fs::File, io};

Allocation
let args: Vec<_> = env::args().skip(1).collect();

This is an unnecessary allocation.  The main function can be
rewritten in terms of iterators using peekable:
fn main() -> Result<()> {
    // ...

    let mut args = env::args().skip(1).peekable();

    if args.peek().is_none() {
        // read from stdin
    }

    for arg in args {
        // handle arg
    }

    // ...
}

Error handling
Instead of unwrapping the result of parse, you can propagate the
error.
For a larger program, I would suggest using the anyhow crate
for error handling, which reduces a lot of boilerplate.
Miscellaneous
Don't write return Ok(()); at the end of a function — just use
Ok(()).
I think you meant >= here: if num > MAXNUM.
In print_sorted, you wrap the writer in a BufWriter.  What if the
writer is already buffered?  I would leave the decision to the caller.
You can use let mut bitfield = [0_u8; MAXNUM >> 3]; for less
duplication.
Using a crate like bit-set might be good for readability.


C++ algorithm that rearranges linked list based on pivot I wrote a small algorithm in C++ that rearranges linked list based on pivot. But my concern is that code is ugly with all the pointers and ->. I don't think that modern C++ is written like this.
How do I improve this code?

#include <iostream>
#include <string>

class Node {
public:
    int value;
    Node* next;
    Node(int n) {
        value = n;
        next = NULL;
    };

    Node* partition(int around, Node* node) {
        Node* head = node;
        Node* tail = node;
        Node* temp = node;

        while (temp != NULL) {
            Node *next = temp->next;
            if (temp->value < around) {
                temp->next = head;
                head = temp;
            } else {
                tail->next = temp;
                tail = temp;
            }
            temp = next;
        }

        tail->next = NULL;
        return head;
    }

    void printList() {
        Node *node = this;
        std::cout << "List: ";
        while (node) {
            std::cout << node->value << " ";
            node = node->next;
        }
        std::cout << std::endl;
    }
};

int main() {

    Node n0 = Node(6);
    Node n1 = Node(5);
    Node n2 = Node(4);
    Node n3 = Node(3);
    Node n4 = Node(2);
    Node n5 = Node(1);

    n0.next = &n1;
    n1.next = &n2;
    n2.next = &n3;
    n3.next = &n4;
    n4.next = &n5;

    n0.printList();
    Node *result = n0.partition(3, &n0);
    result->printList();

    return 0;
}

Those -> are really a standard.

You could change them to the * dereference operator but the parenthesis apocalypse it requires to make it works in larger expressions will bother you more than everything.
In a small implementation like this one it's do-able though.

Or, you may want to write your stuff in an immutable fashion. Meaning you don't use pointers and just use return values and arguments to functions to pass values.
Without modifying them (as variables) so you never have to use pointers.


Parsing a String to see if it contains another String, and then returning a monad if it does I was writing a function (named listenString) in haskell to see if a string contains one of the strings in a list of strings, and to then return a value (of the type m ()) that corresponded with the string it found. The type of that function is (Eq a, Monad m) => [a] -> [([a], m ())] -> m ().

The reason why I used a list to store the strings and their corresponding values ([([a], m ())]) instead of a Map was because I wanted to make sure that the function evaluated strings in the order that they were in the list, e.g. listenString "hello there" [("lo",print True),("hello",print False)] would printed True. If I used a map, the order might have been lost, and the function might have printed False.

Here's the code. I'm specifically wondering if there are any bugs, and if I'm using ListT correctly (of if I should be using something other than ListT), but any other feedback would be helpful and appreciated.

import Data.Either
import Control.Monad.Trans.List

searchListMapMaybe f (Search a b c) = Search a (mapMaybe f b) c

data Search a b = Search a [a] b

type Searching a b = ListT (Either b) (Search a b)

listenString [] _ = return ()
listenString a l =
                    let list = filter (\(x,_) -> if (x == []) then False else True) l
                        noEmptyList [] = error "listenForString: You need to have at least one item in the list"
                        noEmptyList x = x
                     in listenStringSub a $ ListT $ Data.Either.Right $ fmap (\(x,y) -> Search x [] y) $ noEmptyList list
    where
    listenStringSub :: (Eq a) => [a] -> Searching [a] (m b) -> m b
    listenStringSub _ (ListT (Data.Either.Left a)) = a
    listenStringSub (c:cs) x = listenStringSub cs $ (listenStringMap c x) >>= searchFinal
        where
        listenStringMap char x = fmap ((searchReduce char) . (searchPop char)) x
    searchPop :: (Eq a) => a -> Search [a] b -> Search [a] b
    searchPop char (Search a@(x:_) l b)
      | char == x = Search a (a:l) b
      | otherwise = Search a l b
    searchReduce :: (Eq a) => a -> Search [a] b -> Search [a] b
    searchReduce char search = searchListMapMaybe (function char) search
        where
        function _ [] = Just []
        function c l@(x:xs)
          | c == x = Just xs
          | otherwise = Nothing
    searchFinal :: (Eq a) => Search [a] b -> Searching [a] b
    searchFinal s@(Search a b c)
      | [] `elem` b = ListT (Data.Either.Left c)
      | otherwise = return s

Edit: listenString should decide what values to return in the following order:

Which list comes first: listenString "hello there" [("lo",print True),("he",print False)] would output False.
If two or more strings end in the exact same place, e.g.  listenString "hello there" [("lo",print True),("hello",print False)], I want the tuple closest to the beginning of the list to be displayed (the above expression would output True.

Why do you need the element type to be a monad? The only point you require it is when you do return () in order to signal failure. I'd propose implementing a more general function

lookupSublist :: Eq a => [a] -> [([a], b)] -> Maybe b

Which could be implemented as:

lookupSublist xs = fmap snd . listToMaybe . filter (flip isInfixOf xs . fst)

So we first filter out all alternatives where the first element doesn't have xs as infix. Then we return the second element of the first remaining element, if any.

Some random notes on your code:

Using noEmptyList as a guard mid-expression isn't a good idea. Using error is pretty "evil" in the first place, you don't want to bury it like that. Putting the list in the where would allow you to say listenString a l | null list = error...
(\(x,_) -> if (x == []) then False else True) is simply not . null . fst. I'm not quite sure why you filter for that, to be honest. Wouldn't having [] in the list be a pretty elegant way to implement a default fallback value?
I am actually quit struggling to understand the rest of your code. I am pretty sure that your usage of ListT actually hurts more than it helps - generally when you start pattern-matching on the contents of a monad you are doing something wrong. Could you explain what you tried to do?

Edit: If the position of the match end should take priority, I would propose going over the prefixes of the string and suffix-test the options:

lookupFirstSublist str opts = listToMaybe $ mapMaybe f $ inits opts
  where f s = fmap snd $ listToMaybe $ filter (flip isSuffixOf s . fst) opts


Removing synchronization from an agent for a trading system Is there any way to easily remove synchronization from this code, when there is a validation step before acquiring a lock?

I tried an AtomicReference<Pair<Integer, Double>> to hold the state. But that complicates the algorithm a lot because I have to use compareAndSet() in accept(), and then if that returns false, I have to redo the checks for isFinished(), isAboveDailyLimit(), aboveTradingLimit().

import wojtek.events.*;
import wojtek.tinytype.CounterParty;
import wojtek.tinytype.DailyLimit;
import wojtek.tinytype.TradeValue;
import wojtek.tinytype.TradingLimit;

import java.time.Clock;
import java.time.ZonedDateTime;

public class CheckTradeAgent {
    private final CreditCheckAPI creditCheckAPI;
    private final Clock clock;
    private final CounterParty counterParty;
    private final TradingLimit tradingLimit;
    private final DailyLimit dailyLimit;
    private final CheckTradeEventListener eventListener;

    private final Object limitAndFinishLock = new Object();
    private int utilisedDailyLimit;
    private boolean finished;

    public CheckTradeAgent(CreditCheckAPI creditCheckAPI, CheckTradeEventListener eventListener, Clock clock, CounterParty counterParty, TradingLimit tradingLimit, DailyLimit dailyLimit) {
        this.creditCheckAPI = creditCheckAPI;
        this.clock = clock;
        this.counterParty = counterParty;
        this.tradingLimit = tradingLimit;
        this.dailyLimit = dailyLimit;
        this.eventListener = eventListener;
    }

    public void handle(String counterParty, int tradeValue) {
        try {
            handle(new TradeSubmitted(new CounterParty(counterParty), new TradeValue(tradeValue), now()));
        } catch (RuntimeException e) {
            eventListener.onError(e);
        }
    }

    private synchronized void handle(TradeSubmitted trade) {
        if (isFinished()) {
            reject(trade);
            return;
        }

        if (wrongCounterParty(trade)) {
            skip(trade);
        } else {
            if (isAboveDailyLimit(trade)) {
                reject(trade);
            } else {
                if (aboveTradingLimit(trade)) {
                    creditCheck(trade);
                }
                accept(trade);
            }
        }
    }

    private void accept(TradeSubmitted submitted) {
        synchronized (limitAndFinishLock) {
            utilisedDailyLimit = utilisedDailyLimit + submitted.tradeValue.value;
            if (utilisedDailyLimit == dailyLimit.value) {
                this.finished = true;
            }
        }
        eventListener.accepted(new TradeAccepted(submitted, now()));
        if (isFinished()) {
            eventListener.onFinished();
        }
    }

    private void creditCheck(TradeSubmitted submitted) {
        try {
            creditCheckAPI.validate(submitted.counterParty.value, submitted.tradeValue.value);
        } catch (CheckException e) {
            eventListener.onError(e);
        }
    }

    private boolean aboveTradingLimit(TradeSubmitted submitted) {
        return submitted.tradeValue.value > tradingLimit.value;
    }

    private boolean isAboveDailyLimit(TradeSubmitted submitted) {
        return getUtilisedDailyLimit() + submitted.tradeValue.value > dailyLimit.value;
    }

    private boolean wrongCounterParty(TradeSubmitted submitted) {
        return !counterParty.equals(submitted.counterParty);
    }

    private void reject(TradeSubmitted submitted) {
        eventListener.rejected(new TradeRejectedByDailyLimit(submitted, now()));
    }

    private void skip(TradeSubmitted submitted) {
        eventListener.skipped(new TradeSkippedByWrongCounterParty(submitted, now()));
    }

    private ZonedDateTime now() {
        return ZonedDateTime.now(clock);
    }

    public int getUtilisedDailyLimit() {
        synchronized (limitAndFinishLock) {
            return utilisedDailyLimit;
        }
    }

    private boolean isFinished() {
        synchronized (limitAndFinishLock) {
            return finished;
        }
    }
}

You may be able reduce the amount of synchronisation, but you cannot eliminate it entirely. CAS or Fetch-And-Add are lower level operations that are abstracted away by Java so you cannot use them directly.

Firstly, some observations: due to the mechanisms of how the lock is acquired, your synchronization of handle will cause a random caller to be selected when multiple are waiting. This could cause a situation where trades are handled out of order, which would be evidenced in the timestamps. If this isn't desired, you should use a queue to ensure trades are processed fairly (although what constitutes fair is a matter of debate).

You shouldn't use your CheckException as a convenient way to skip past a bunch of code. Have creditCheck(...) return a boolean and use proper flow control, otherwise your code appears to accept a trade even if the credit check fails.

Some of your preconditions to a trade may allow you to reject a trade before entering the synchronisation block. If the CounterParty doesn't match, you will always reject a trade. If  a trade is above the daily limit, you will reject the trade outright. If a trade is above the trading limit, you can run the credit check without synchronising and reject it if it fails. It also appears that you only ever increment utilisedDailyLimit so if any trade's value would exceed the daily limit (and assuming trade values cannot be negative) you may reject it with an unsynchronised check against the daily limit.

Once you've checked all these preconditions, you have to worry about concurrency. You will have to check the daily limit again, but if this is done on a single-threaded consumer thread, you do not have to synchronise at all because there is no concurrent access and you know the value cannot change. With this approach, may also wish to submit the eventListener.accepted(...) call to a thread pool to execute separately as this may also slow down execution calling the listener inline.

A BlockingQueue would be ideal for this kind of design.


Select multiple minimum items in an Array I have an array and need to identify minimum value in the array - This is easy. However, this array can have duplicates and there exist multiple minimum values and we need to identify them.

The following code works and is done through 2 loops. First loop identifies the minimum value and the second loop all values which equate to the minimum value.

Please let me know your thoughts and if any other efficient way is possible.

public static List<Integer> getMinIndex(double... arrayList){
    int minIndex = 0;
    int i = 0;
    int len = arrayList.length;
    while(i < len){
        if(arrayList[i] < arrayList[minIndex]  )
            minIndex = i;
        i++;
    }

    //Below is specifically for case where are multiple minimums and we want all of them
    i=0;
    List<Integer> minList = new ArrayList<Integer>();
    while(i < len){
        if(arrayList[i] == arrayList[minIndex]  )
            minList.add(i);
        i++;
    }
    logger.debug("Postition of min-item in arraylist {}",Arrays.toString(minList.toArray()));
    return minList;
}

You can achieve what you want, just with one for loop:

int minIndex = 0;
List<Integer> list = new ArrayList<>();

for (int i = 0; i < array.length ; i++) {
    if (array[i] == array[minIndex]) {
        list.add(i);
    } else if (array[minIndex] > array[i]) {
        list.clear();
        list.add(i);

        minIndex = i;
    }
}

return list;

Changes and recommendations:

I think that you must change the name of the method variable to 'array', since there is a Java collection class named ArrayList and it could confuse some readers of your code
You should also change the method name, since from its name it is not clear that it returns a list of min indexes. Maybe getMinIndexes could be used
Regarding the old code, I think that you should use less temporal variables, in your case len variable, doesn't give any more semantic to the code, so it could be removed. When you works with arrays consider the usage of for loops.

Update

As additional recommendations I would add:

I think that 'find' could be used as prefix instead of 'get', since you are actually searching for those indexes, you don't getting them from some POJO
I also think that it is better if you are using curly brackets even in a single line block statements
Also there is no need of transforming the list into array and then into string in the debug log. Lists has nice implementation of toString function that would print the elements the same way as Arrays.toString


Getting a list of all fields in an object I've got the following code that I use to output a list of all (public) fields in an object in an easy(ish) to read way. The issue with it is that the code is not easy to look at, and I'm not sure what I could do to improve it.

I'm not too fond of all of the StringBuilders, the append calls are ugly and chaining them does not help the situation however I can't see a better way of doing it.

public abstract class Component {
    private String[] getFields() {
        Class<? extends Component> componentClass = getClass();
        List<String> lines = new ArrayList<>();

        for (Field field: componentClass.getFields()) {
            StringBuilder lineBuilder = new StringBuilder();

            lineBuilder.append(field.getName());

            field.setAccessible(true);

            try {
                Object value = field.get(this);

                lineBuilder.append(" = ");
                lineBuilder.append(value);
            } catch (IllegalAccessException e) {
                lineBuilder.append(" > ");
                lineBuilder.append(e.getClass().getSimpleName());
            }

            lines.add(lineBuilder.toString());
        }

        return lines.toArray(new String[lines.size()]);
    }

    @Override
    public final String toString() {
        StringBuilder builder = new StringBuilder(getClass().getSimpleName());
        boolean firstIteration = true;

        builder.append('(');

        for (String field: getFields()) {
            if (!firstIteration) {
                builder.append(", ");
            } else {
                firstIteration = false;
            }

            builder.append(field);
        }

        builder.append(')');

        return builder.toString();
    }
}

I'm not entirely sure why I separated getFields into it's own method, however it might be better to move it back into toString.

If you are using Java 8, you can do:

private String[] getFields() {
    Class<? extends Component> componentClass = getClass();
    Field[] fields = componentClass.getFields();
    List<String> lines = new ArrayList<>(fields.length);

    Arrays.stream(fields).forEach(field -> {
        field.setAccessible(true);
        try {
            lines.add(field.getName() + " = " + field.get(this));
        } catch (final IllegalAccessException e) {
            lines.add(field.getName() + " > " + e.getClass().getSimpleName());
        }
    });

    return lines.toArray(new String[lines.size()]);
}

or to improve formatting:

private String[] getFields() {
    Class<? extends Component> componentClass = getClass();
    Field[] fields = componentClass.getFields();
    List<String> lines = new ArrayList<>(fields.length);

    Arrays.stream(fields)
            .forEach(
                    field -> {
                        field.setAccessible(true);
                        try {
                            lines.add(field.getName() + " = " + field.get(this));
                        } catch (final IllegalAccessException e) {
                            lines.add(field.getName() + " > "
                                    + e.getClass().getSimpleName());
                        }
                    });

    return lines.toArray(new String[lines.size()]);
}

Note that this is adding to @EricStein's excellent answer on normal concatenation instead of StringBuilders.

(I'm just starting to look into Streams, so if this is bad, please comment on why)

Otherwise...

Just a small comment:

lineBuilder.append(" = ");
lineBuilder.append(value);

can easily be:

lineBuilder.append(" = ").append(value);

Same with:

lineBuilder.append(" > ");
lineBuilder.append(e.getClass().getSimpleName());

becomes:

lineBuilder.append(" > ").append(e.getClass().getSimpleName());

Also, I would use a simple array instead of an ArrayList:

private String[] getFields() {
    Class<? extends Component> componentClass = getClass();
    Field[] fields = componentClass.getFields();
    String[] lines = new String[fields.length];

    int index = 0;
    for (Field field : fields) {
        StringBuilder lineBuilder = new StringBuilder();

        lineBuilder.append(field.getName());

        field.setAccessible(true);

        try {
            Object value = field.get(this);

            lineBuilder.append(" = ");
            lineBuilder.append(value);
        } catch (IllegalAccessException e) {
            lineBuilder.append(" > ");
            lineBuilder.append(e.getClass().getSimpleName());
        }

        lines[index++] = lineBuilder.toString();
    }

    return lines;
}

EDIT: The above review, if not using Java 8, should be replaced by @EricStein's code.

Also, getFields() is a bad name. Try getFieldDescriptions().


Recurrent merge sort (without recursion) For educational purpose I implement recurrent merge sort without recursion. The algorithm idea seems absolutely clear but my implementation appears to be really slow:

5 seconds to sort 10,000 values;
6 minutes to sort 100,000 values;
it was not able to finish 10,000,000 in several hours (while lib sort() function does it in aboit 6 seconds);

I guess, the problem is in my understanding of how C++ works. Maybe, I must not copy vectors to funtion and use pointers insted?

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

void check_sort(std::vector<int> array); //test sorted arrays
void merge_sort(std::vector<int> array); //recurrent merge sort
std::vector<int> merge(std::vector<int> array, unsigned int b, unsigned int c, unsigned int e);

int main()
{
    unsigned int size;
    std::cout << "Please enter the size of your array:" << std::endl;
    std::cin >> size;

    std::vector<int> initial_array(size); //array of random elements

    //prefill arrays
    srand(time(0));
    for (unsigned int i; i < initial_array.size(); ++i) {
        initial_array[i] = rand();
    }

    merge_sort(initial_array);

 /* DEBUGGER
    std::cout << "initial array" << std::endl;
    for (unsigned int i = 0; i < initial_array.size(); ++i) {
        std::cout << initial_array[i] << " ";
    }
    std::cout << std::endl;
*/

    return 0;
}

void merge_sort(std::vector<int> array)
{
    std::cout << "merge sort" << std::endl;
    check_sort(array);
    /* DEBUGGER
    std::cout << "initial array" << std::endl;
    for (unsigned int i = 0; i < array.size(); ++i) {
        std::cout << array[i] << " ";
    } */

    float start_time =  clock();
    unsigned int n = array.size();
    for (unsigned int s = 1; s < n; s *= 2)
    {
        for (unsigned int b = 0; b < n; b += s*2)
        {
            unsigned int c = std::min(b + s - 1, n - 1);
            unsigned int e = std::min(c + s, n - 1);
            array = merge(array, b, c, e);
        }
    }
    float end_time = clock() - start_time;
    /* DEBUGGER
    for (unsigned int i = 0; i < array.size(); ++i) {
        std::cout << array[i] << " ";
    } */
    check_sort(array);
    std::cout << "time: " << end_time/1000 << std::endl;
}

std::vector<int> merge(std::vector<int> array, unsigned int b, unsigned int c, unsigned int e) {
    std::vector<int> C(array);
    unsigned int i1 = b; unsigned int i2 = c + 1; //start point in each piece
    unsigned int n1 = c; unsigned int n2 = e; //end point of each piece
    for (unsigned i = b; i <= e; ++i)
    {
        if (i1 > n1) //
        {
            C[i] = array[i2];
            ++i2;
        }
        else if (i2 > n2)
        {
            C[i] = array[i1];
            ++i1;
        }
        else if (array[i1] <= array[i2]) {
            C[i] = array[i1];
            ++i1;
        }
        else
        {
            C[i] = array[i2];
            ++i2;
        }
    }
    return C;
}

void check_sort(std::vector<int> array)
{
    for (unsigned int i = 0; i < (array.size() - 1); ++i)
    {
        if (array[i] > (array[i + 1]))
        {
            std::cout << "unsorted" << std::endl;
            return;
        }
    }
    std::cout << "sorted" << std::endl;
}

I see a number of things that may help you improve your code.

Rearrange the code to reduce the need for declarations

A function declaration is only needed by the compiler if it hasn't already encountered the function itself.  With that said, if the functions in the file are in the order check_sort, merge, merge_sort and then main, no separate function declarations would be needed.

Separate I/O from logic

The check_sort routine is not bad as it is, but I'd advocate separating the I/O from the logic.  That is, rename check_sort to is_sorted and have it return a bool value of true only if the vector is sorted.  Leave the printing to the calling function instead.

Use const where practical

The check_sort function does not alter the passed vector, so that should be passed as const &:

void check_sort(const std::vector<int> &array)

Consider using references

While this code works, there is still some room for improvement.  The easy thing to do to speed up the code is to simply use references like this:

std::vector<int> merge(std::vector<int> &array, unsigned int b, unsigned int c, unsigned int e) {

By having it &array instead of array you tell the compiler that it may use the passed object directly rather than making a copy.  This single change makes the code run about twice as fast on my machine.

Omit return 0

When a C++ program reaches the end of main the compiler will automatically generate code to return 0, so it is not necessary to put return 0; explicitly at the end of main.

There's more, but that's all I have time for at the moment.


Locate user of an Android device I am new to oriented object paradigm and I work on Android using Java project for an internship.
I must be able to locate the user and some around buildings I read stuff about how to setup LocationListener and all, and decided that I better write a class that manage everything for me.

public class NetCampusLocation {
    private Activity activity;
    private LocationManager locationManager;
    private LocationListener locationListener;
    private Location freshLocation;

    public NetCampusLocation(Activity activity) {
        this.activity = activity;
        this.locationManager = (LocationManager) this.activity.getSystemService(Context.LOCATION_SERVICE);
        this.locationListener = new LocationListener() {
            @Override
            public void onLocationChanged(Location location) {

            }

            @Override
            public void onStatusChanged(String provider, int status, Bundle extras) {

            }

            @Override
            public void onProviderEnabled(String provider) {

            }

            @Override
            public void onProviderDisabled(String provider) {

            }
        };
        this.freshLocation = this.locationManager.getLastKnownLocation(LocationManager.GPS_PROVIDER);
        if (this.freshLocation == null) {
            this.freshLocation = this.locationManager.getLastKnownLocation(LocationManager.NETWORK_PROVIDER);
        }
    }

    public void setLocationUpdate(String provider, int minTimeInterval, int minDistance, LocationListener listener) {
        this.locationManager.requestLocationUpdates(provider, minTimeInterval, minDistance, listener);
    }

    public void stopLocationUpdate() {
        this.locationManager.removeUpdates(this.locationListener);
    }
}

That is the first time I am writing a class on my own (without any pedagogic goal) and I ever seen an example with an interface nested to a class, I am wondering if this is even logical or totally absurd.

I am also wondering if this a good practice to pass my activity to the constructor or the class or not.

All kind of advices would be really good.

It is good practice to mark as many fields as possible as final.

private final Activity activity;
private final LocationManager locationManager;
private final LocationListener locationListener;

I assume that freshLocation will be changed in your locationListener so that one should not be final.

All fields that only get initialized once can be marked final.

You seem to only be using your activity inside the constructor, so you don't need to keep that as a field at all.

Also, instead of passing an Activity, it's enough to pass a Context (Activity extends Context so you can still pass an activity). The only method you use on the activity is getSystemService which is part of the Context class. Contexts are often required to pass to various methods in Android, so that is perfectly fine. It is better to pass a Context than an Activity.

About this code:

this.locationListener = new LocationListener() {
    @Override
    public void onLocationChanged(Location location) {

It is totally fine to do it this way. This is an anonymous inner class. The alternative is to use an inner class (non-anonymous), this would reduce code from your constructor but that code would be added in other parts of the class instead so which way you go doesn't matter much. This is fine.


Is this a good way to create a Javascript object? I'm building a simple quiz app and wanted to make it more scalable by putting my code into an object. The parameters list looks a bit messy to me. Is this a normal way to use an object or is there a better way?

Thanks

// above here I have a JSON object with questions:
var quiz = new question($('#quiz-container'), $('.alert'), 0, [], questions[0].level[0]);

// for debugging
console.log(quiz);

function question(quizContainer, alert, questionCounter, selection, level) {
    this.quizContainer = quizContainer;
    this.alert = alert;
    this.questionCounter = questionCounter;
    this.selection = selection;
    this.level = level;

    function createQuestion(index) {
        //TODO
    }
}

If you want to access the createQuestion function you could simply do the following

this.createQuestion = function(index){
    // your code
};

You can create public and private variables in a object. The easiest example I can give for this is using the following code.

function someObject(publicInput, privateInput) {

    this.public = publicInput;

    var private = privateInput;

    this.getPrivate = function(){
        return private;
    }
}

var newObject = new someObject(true, false);

console.log(newObject.public); // returns true
console.log(newObject.private); // returns undefined
console.log(newObject.getPrivate()); // returns false

As you can see the private variable can only be accessed from inside the object, which is why I've added a getPrivate() function to return this variable.


Angular directive to toggle sidebar visibility based on window width I changed the normal way of using Angular. I'm using my controllers just to talk to my API routes and am using directives to manipulate the data and DOM as well.

I am using the object literal pattern where I have my INIT method that initializes global variables (object) and then calls other methods, bind clicks methods, etc. So, the logic is all the directives.

I'm finding it very confusing and would like the opinion of someone better.

'use strict';
var eventSidebarDirective = {

    /**
     * Initialize sidebar directive and return the link
     * calling all nested methods.
     *
     */
    init: function() {
        return {
            link: function(scope) {
                scope.$watch('events', function() {
                    if (scope.events === undefined) {
                        return;
                    }

                    /**
                     * Every time the user access the event page, this methods
                     * will be called.
                     *
                     */
                    EventSidebar.init();
                });
            },
            restrict: 'M'
        };
    }
};

var EventSidebar = {
    init: function() {
        this.element        = $('[data-event-sidebar]');
        this.indicator  = $('[data-event-sidebar-to]');

        /**
     * To manipulate the element, bind a click in the indicator.
     *
     */
        this.indicator.bind('click', this._toggle.bind(this));
    },

    _toggle: function(e) {
        var state = $(e.currentTarget).data('event-sidebar-to');

        this.element
            .css({

                /**
                 * If the window width is less than 768px, i just want toggle
                 * between display block and none.
                 *
                 */
                display: 768 >= $(window).width()
                        ? (state
                            ? 'block'
                            : 'none')
                        : false
            })
            .animate({

                /**
                 * But if the window.width is bigger the 768px, i want show
                 * or hide the sidebar.
                 *
                 */
                right: 768 <= $(window).width()
                        ? (state
                            ? '0'
                            : '-500')
                        : false
            });
    }
};

angular
    .module('adminApp')
    .directive('eventSidebarDirective', eventSidebarDirective.init.bind(eventSidebarDirective));

Yes, you're right. Your code is very hard to read.

I think you don't need that init stuff. Just remove it and add initialization to the controller.
You don't need jQuery for your directive. This is possible with Angular JS / pure Js. Use ng-show or ng-if for showing hiding your sidebar and use ng-class to add your class conditionally depending on a variable from your controller to do things differently for large screens.
Animation could be done with ngAnimate and animate.css.

There are some points that could be improved in my code:

Add options so it's better reusable.
Add default options for example start visible = false. with angular.module('adminApp').constants(...) then you can angular.extend(...) these in the controller of your directive.
Add an isolated scope for the directive so it can't modify the scope of your app. Somthing like scope: {} would be OK or {scope: { options: '@' } } for the sidebar options.
Remove restrict: M because it's not recommended to use directives as comments (see angular docs).

Please have a look at the demo below or in this jsfiddle.

function EventSidebarDirective($window) {
    var directive = {
        restrict: 'EAM',
        templateUrl: 'components/sidebar/template.html',
        controllerAs: 'ctrl',
        controller: function ($scope) {
            var self = this, // self needed because toggle is not bound to this ctrl.
             screenWidth;

            angular.extend(this, {
             visible: false,
             toggle: function () {
                    screenWidth = $window.innerWidth; //$(window).width();
                    self.visible = !self.visible; // toggle visibility
                    self.largeScreen = ( screenWidth >= 768 );
    }
            });
        },
        link: function (scope, element, attr, ctrl) {

        }
    };

    return directive;
}

EventSidebarDirective.$inject = ['$window']; // only need with-out ng-annotate for minification

angular.module('adminApp', ['ngAnimate'])
    .directive('eventSidebar', EventSidebarDirective);
.sidebar-content {
    width: 200px;
    height: 200px;
    background: lightgray;
}

.sidebar-content-animation.ng-hide-remove {
    -webkit-animation: fadeInLeft 1s;
    -moz-animation: fadeInLeft 1s;
    -ms-animation: fadeInLeft 1s;
    -o-animation: fadeInLeft 1s;
    animation: fadeInLeft 1s;
}

.sidebar-content-animation.ng-hide-add {
    -webkit-animation: fadeOutUp 1s;
    -moz-animation: fadeOutUp 1s;
    -ms-animation: fadeOutUp 1s;
    -o-animation: fadeOutUp 1s;
    animation: fadeOutUp 1s;
}

<link href="https://cdnjs.cloudflare.com/ajax/libs/animate.css/3.4.0/animate.min.css" rel="stylesheet"/>

    <event-sidebar></event-sidebar>


Expression evaluator with Shunting-Yard algorithm I am especially concerned about:

Correctness: Does my algorithm always output the correct answer given a correct input?
Readability: Is my algorithm easy to understand for an experienced Ruby programmer (or, in other words: how many WTFs does a experienced Ruby programmer say while reading my code?)

Performance is not a main goal (if I had performance as a main goal, I wouldn't use Ruby), yet I want to know if I did something really bad somewhere in my code.

My method to transform a string into a list of symbols (which is called symbolize) is really poor but it doesn't matter. It does not need to be reviewed. And yes, it accepts only positive numbers.

Shunting-Yard implementation

class ShuntingYard
  def initialize
    @precedence = {
      "(" => 0,
      ")" => 0,
      "+" => 1,
      "-" => 1,
      "*" => 2,
      "/" => 2,
      "^" => 3,
    }

    @associativity = {
      "+" => :left,
      "-" => :left,
      "*" => :left,
      "/" => :left,
      "^" => :right
    }
  end

  def symbolize input
    stripped = input.lstrip

    if stripped.size == 0 then
      []
    elsif is_operator stripped[0] then
      [stripped[0]].concat symbolize stripped[1..-1]
    else
      n = stripped[/\d+(\.\d+)?/, 0]
      [n.to_f].concat symbolize stripped[n.size..-1]
    end
  end

  def parse input
    @stack = []
    @rpn = []

    (symbolize input).each do |symbol|
      shunt symbol
    end

    @rpn.concat @stack.reverse
  end

  def shunt symbol
    if not is_operator symbol then
      @rpn << symbol
    elsif symbol == "(" then
      @stack << symbol
    elsif symbol == ")" then
      until @stack.last == "("
        @rpn << @stack.pop
      end

      @stack.pop
    elsif @stack.empty? or @stack.last == "(" then
      @stack << symbol
    elsif is_higher_or_right symbol then
      @stack << symbol
    else
      until @stack.empty? or (is_higher_or_right symbol)
        @rpn << @stack.pop
      end

      @stack << symbol
    end
  end

  def is_higher_or_right symbol
    higher = @precedence[symbol] > @precedence[@stack.last]
    equal = @precedence[symbol] == @precedence[@stack.last]
    right = @associativity[symbol] == :right

    higher or (equal and right)
  end

  def is_operator symbol
    not not @precedence[symbol]
  end
end

Interpreter implementation

class Interpreter
  def initialize
    @eval = {
      "+" => lambda { |a, b| a + b },
      "-" => lambda { |a, b| a - b },
      "*" => lambda { |a, b| a * b },
      "/" => lambda { |a, b| a / b },
      "^" => lambda { |a, b| a ** b },
    }
  end

  def evaluate input
    result = []

    (ShuntingYard.new.parse input).each do |symbol|
      result << (if @eval[symbol] then
                  right = result.pop
                  left = result.pop
                  @eval[symbol].call left, right
                else
                  symbol
                end)
    end

    result.pop
  end
end

Some tests to easily run it all

inputs = {
  "15 * 2 - 30 / 3 + 7" => 27,
  "5 + ((1 + 2) * 4) - 3" => 14,
  "10 * (30 / (2 * 3 + 4) + 15)" => 180,
  "25 + (14 - (25 * 4 + 40 - (20 / 2 + 10)))" => -81,
}

interpeter = Interpreter.new

inputs.each do |input, expected|
  puts "Got:      #{input}"
  puts "Expected: #{expected}"
  puts "Result:   #{interpeter.evaluate input}"
  puts
end

For completeness, I am using this Ruby interpreter

ruby 2.3.1p112 (2016-04-26) [x86_64-linux-gnu]

This is a strong question with easy-to-read code.  I'm impressed with it.

I will pick on formatting some, because that's easier than the substantive, but I don't get the feeling that there's are many real issues with this code.

Vertical whitespace in methods

It is my opinion that vertical whitespace in code is of limited value, makes the boundaries between methods less visually apparent, and can usually be replaced with stronger mechanisms for revealing meaning.

  def symbolize input
    stripped = input.lstrip

    if stripped.size == 0 then
      []
    ...
  end

Here, the purpose of the vertical whitespace is not apparent to me, so I cannot comment on what might replace it.  Some alternatives:

Move the related lines into a method, or
Use a comment to indicate why the lines are special, or
Just get rid of the whitespace

Use parentheses when defining methods

Whether or not you use parentheses when calling methods, use them when defining methods.  Prefer:

  def shunt(symbol)

to:

  def shunt symbol

Reasons:

If I recall, there are some argument declarations that just won't work without parentheses anyhow.
The parentheses help clue the eye in that a method declaration is happening.
At least one standards guide recommends this.

Omit then on most ifs

With a multi-line if (most if expressions):

    (ShuntingYard.new.parse input).each do |symbol|
      result << (if @eval[symbol]
                  right = result.pop
                  left = result.pop
                  @eval[symbol].call left, right
                else
                  symbol
                end)

If the parentheses around the if...else...end were driven by the then, then the parentheses can (and should) now be removed.

Use #map

This iterative code builds a new array based on an enumeration:

    result = []

    (ShuntingYard.new.parse input).each do |symbol|
      result << (if @eval[symbol] then
                  right = result.pop
                  left = result.pop
                  @eval[symbol].call left, right
                else
                  symbol
                end)

This could use #map instead (note: untested):

    result = (ShuntingYard.new.parse input).map do |symbol|
      if @eval[symbol]
        right = result.pop
        eft = result.pop
        @eval[symbol].call left, right
      else
        symbol
      end
    end


Web-crawler for iTunes I've written a script in Python to scrape app name, developer and price from ITunes site.

In its first page there are 10 different recommended app links which my scraper is able to track down and scrape the aforesaid fields going to their concerning page. The quantity of the apps should be 11. Previously, the scraper was printing only 10 results skipping the content from first page. However, there is a line in my scraper (I've written a comment next to it to make you identify) which I've added in such a way so that the scraper do not fail to skip the content of first page from printing.

With this extra line in it, the scraper has got a clumsy look. It didn't impact on the performance, though! It runs smoothly now and brings results without leaving anything to complain. How can I give it a better look along with any improvement?

This is what I've written:

import requests
from bs4 import BeautifulSoup

main_link = "https://itunes.apple.com/us/app/candy-crush-saga/id553834731?mt=8"

def get_links(url):
    response = requests.get(url)
    get_content(url)              # This is the very line which let the scraper harvest documents from the first page
    soup = BeautifulSoup(response.text,"html.parser")
    for title in soup.select("a.name"):
        get_content(title.get("href"))

def get_content(new_links):
    req = requests.get(new_links)
    broth = BeautifulSoup(req.text,"html.parser")
    item ={
    "app_name": broth.select("h1[itemprop=name]")[0].text,
    "developer": broth.select("div.left h2")[0].text,
    "price": broth.select("div.price")[0].text
    }
    print(item)

get_links(main_link)

Instead of having a get_content function you could add a function that only parses a source passed to it, now it can be either the main page or the suggested apps. Note that you're requesting for main page's content twice even though you had it already.
In addition to the above point you could make few more improvements:

Make the code PEP 8 compatible. Currently you will see decent number of issues if you run it on http://pep8online.com/.

Use session to re-use connections. Since we are making requests to the same host this would speed up the requests. From docs:

The Session object allows you to persist certain parameters across requests. It also persists cookies across all requests made from the Session instance, and will use urllib3's connection pooling. So if you're making several requests to the same host, the underlying TCP connection will be reused, which can result in a significant performance increase (see HTTP persistent connection).

Use __name__ == '__main__' to prevent your code from running when it is imported as a module.

After making the changes your code may look like this:
import requests
from bs4 import BeautifulSoup

session = requests.Session()

def get_links(url):
    source = session.get(url).text
    main_app = parse_content(source)
    print(main_app)

    for linked_app in get_linked_app_links(source):
        print(linked_app)

def get_linked_app_links(source):
    soup = BeautifulSoup(source, "html.parser")
    for title in soup.select("a.name"):
        linked_app = get_app_data(title.get("href"))
        yield linked_app

def get_app_data(url):
    source = session.get(url).text
    return parse_content(source)

def parse_content(source):
    broth = BeautifulSoup(source, "html.parser")
    item = {
        "app_name": broth.select("h1[itemprop=name]")[0].text,
        "developer": broth.select("div.left h2")[0].text,
        "price": broth.select("div.price")[0].text
    }
    return item


Finding Minimum and Maximum Value I'm doing a code-challenge problem:

Captain Kaban: National Treasure is a popular football simulation game which can be played on most smartphones. In this game, you put up a team of 11 players and then compete in either story mode or multi-player mode. As you might already know, a football match is played by 11 players, which consists of one goalkeeper and ten outfield players.

Each outfield player's capability is measured with scores in 3 areas: Attack (dribble, shoot, pass), Defense (tackle, block, intercept), and Physical (speed, power, technique). For example, a player with A:4103 D:2837 P:3410 is a typical attacker player (forward position) as his Attack (4103) is higher than his Defense (2837) and Physical (3410). On the other hand, a player with A:1546, D:5209, P:2708 is a typical defensive player as his Defense is much higher than his Attack and Physical. A goalkeeper has a different measurement (Saving and Physical), but it's not our concern in this problem.

A team strength is simply the sum of all the main outfield player's capability-scores. Note that the team strength score does not consider the team's balance, e.g., a team of 10 players where each player has A:8000, D:1000, P:1000 is stronger than a team where each player has A:3000, D:3000, P:3000; the first team's team strength is 100,000 (10 * (8000 + 1000 + 1000)) while the second one is 90,000 (10 * (3000 + 3000 + 3000)).

Let say you are given a team with ten main outfield players and N reserve players. If you are allowed to substitute at most 1 main outfield player (with a reserve player), what is the maximum team strength you can obtain?

Input

Input begins with an integer: T (1 ≤ T ≤ 20) denoting the number of cases.

Each case contains the following input block: Each case begins with an integer: N (1 ≤ N ≤ 10) denoting the number of reserve players. The next 10 lines each contains three integers: Ai Di Pi (100 ≤ Ai, Di, Pi ≤ 20,000) representing the Attack, Defense, and Physical score for the ith main player. The next N lines each contains three integers: Aj Dj Pj (100 ≤ Aj, Dj, Pj ≤ 20,000) representing the Attack, Defense, and Physical score for the jth reserve player.

Output

For each case, output in a line "Case #X: Y" where X is the case number (starts from 1) and Y is the output for the respective case.

#include <iostream>

using namespace std;

int main () {
    const int totalOriginalPlayer {10};
    int testCases {};
    cin >> testCases;

    for (int i = 1; i <= testCases; i++) {
        int reserve {}, total {}, lowest {500000}, highest {};
        int attack {}, defense {}, physical {};
        cin >> reserve;

        int reservePlayers [reserve];
        int powerReservePlayers [reserve];

        int powerOriginalPlayers [totalOriginalPlayer];

        // Get power of all original players, calculate sum and find the minimum
        for (int i = 0; i < totalOriginalPlayer; i++) {
            cin >> attack >> defense >> physical;
            powerOriginalPlayers[i] = attack + defense + physical;
            total += powerOriginalPlayers[i];

            if (powerOriginalPlayers[i] < lowest) {
                lowest = powerOriginalPlayers[i];
            }
        }

       // Get power of all reserve players and find the maximum power
       for (int j = 0; j < reserve; j++) {
            cin >> attack >> defense >> physical;
            powerReservePlayers[j] = attack + defense + physical;

            if (powerReservePlayers[j] > highest) {
                highest = powerReservePlayers[j];
            }
       }

       // Subtract total with lowest and add highest
       if (lowest < highest) {
            total = total - lowest + highest;
       } else if (lowest >= highest) {
            total = total;
       }

       // Print out total
       cout << "Case #" << i << ": "<< total << endl;
    }
}

Don't do using namespace std;.
Use an appropriate type for the range. The number of test-cases can never be negative, so we should use an unsigned integer. It looks like the player capabilities are similarly unsigned.
for (int i = 1; i <= testCases; i++) In C++, it's more usual to use != for the end condition, and use the pre-increment operator (since we don't a temporary value). It's usually safer to use a zero-based index, and simply add one for printing if we need to convert to one-based: for (int i = 0; i != testCases; ++i).
int attack {}, defense {}, physical {};. Declare variables (especially POD variables) as close to the point of use as possible to minimize their scope and avoid reusing them for multiple things. These should be declared in each of the innermost loops.
reserve is not a constant value, so this is not valid C++: int reservePlayers [reserve];, even if some compilers allow it. We should use a std::vector instead. (This particular array also appears to be unused!)
for (int i = 0; i < totalOriginalPlayer; i++) The loop condition variable shadows the i variable of the outer loop. This is likely to cause confusion. It would be better to pick a meaningful name for the loop variables, e.g. outfielderIndex.
It might be neater to separate the logic of the program from reading the input. Then we could reuse the input reading code for both sets of players.

std::vector<std::uint32_t> outfielders = ReadPlayerData(numOutfielders);
std::vector<std::uint32_t> reserves = ReadPlayerData(numReserves);
std::uint32_t total = // ...
std::uint32_t worstOutfielder = // ...
std::uint32_t bestReserve = // ...

// ... do output

We can use the standard library to do the necessary math. Specifically std::accumulate, std::min_element and std::max_element.