Console component in JavaFX I would like a review on my Console class I made in JavaFX 8. It features the following:

A text field where you can enter input, the input gets copied into the text area.
A text area where you can observe output and see the input.
When you press enter, you send the message.
When you press arrow up, you can go back in history.
When you press arrow down, you can go forward in history.
You can hook an event listener when a message gets sent.

public class Console extends BorderPane {
    protected final TextArea textArea = new TextArea();
    protected final TextField textField = new TextField();

    protected final List<String> history = new ArrayList<>();
    protected int historyPointer = 0;

    private Consumer<String> onMessageReceivedHandler;

    public Console() {
        textArea.setEditable(false);
        setCenter(textArea);

        textField.addEventHandler(KeyEvent.KEY_RELEASED, keyEvent -> {
            switch (keyEvent.getCode()) {
                case ENTER:
                    String text = textField.getText();
                    textArea.appendText(text + System.lineSeparator());
                    history.add(text);
                    historyPointer++;
                    if (onMessageReceivedHandler != null) {
                        onMessageReceivedHandler.accept(text);
                    }
                    textField.clear();
                    break;
                case UP:
                    if (historyPointer == 0) {
                        break;
                    }
                    historyPointer--;
                    runSafe(() -> {
                        textField.setText(history.get(historyPointer));
                        textField.selectAll();
                    });
                    break;
                case DOWN:
                    if (historyPointer == history.size() - 1) {
                        break;
                    }
                    historyPointer++;
                    runSafe(() -> {
                        textField.setText(history.get(historyPointer));
                        textField.selectAll();
                    });
                    break;
                default:
                    break;
            }
        });
        setBottom(textField);
    }

    @Override
    public void requestFocus() {
        super.requestFocus();
        textField.requestFocus();
    }

    public void setOnMessageReceivedHandler(final Consumer<String> onMessageReceivedHandler) {
        this.onMessageReceivedHandler = onMessageReceivedHandler;
    }

    public void clear() {
        runSafe(() -> textArea.clear());
    }

    public void print(final String text) {
        Objects.requireNonNull(text, "text");
        runSafe(() -> textArea.appendText(text));
    }

    public void println(final String text) {
        Objects.requireNonNull(text, "text");
        runSafe(() -> textArea.appendText(text + System.lineSeparator()));
    }

    public void println() {
        runSafe(() -> textArea.appendText(System.lineSeparator()));
    }
}

public final class GUIUtils {
    private GUIUtils() {
        throw new UnsupportedOperationException();
    }

    public static void runSafe(final Runnable runnable) {
        Objects.requireNonNull(runnable, "runnable");
        if (Platform.isFxApplicationThread()) {
            runnable.run();
        }
        else {
            Platform.runLater(runnable);
        }
    }
}

public class WordWrapConsole extends Console {
    public WordWrapConsole() {
        super();
        textArea.setWrapText(true);
    }
}

The calls that modify textArea and textField are wrapped in a GUIUtils.runSafe(), because modifications on JavaFX UI elements may only be done from the JavaFX thread and it is quite common that code from the public methods can run in different threads.

As someone who's been working with your project and your Console Gui here's my 2 cents:

protected final TextArea textArea = new TextArea();
protected final TextField textField = new TextField();

these could use a rename. The names do not contain any information and in all the unit tests you wrote, this was confusing me very much: Which is in, which is out?

I'd rather have something like:

protected final TextArea output = new TextArea();
protected final TextField input = new TextField();

textField.addEventHandler(KeyEvent.KEY_RELEASED, keyEvent -> {
    //long block of code...

I like very much that you are using a comprehensively wired lambda. I don't like that you are mixing abstraction levels here. on the one hand, you have that "I configure my console" code like textArea.setEditable(false), and then you have that switch (keyEvent.getCode())

Personally I'd have instead extracted the handler code to a separate handler (which needs a better name than what I am about to propose):

private final EventHandler<KeyEvent> keyReleasedHandler = new EventHandler<>() {
        handle(final KeyEvent keyEvent){
            //your code here
        }
    }
textField.setOnKeyReleased(keyReleasedHandler);

This also makes it easier to understand, where your constructor ends, without seeing the method signature that follows. While we're at it:

    setCenter(textArea);

    //here comes the whole event handling

    setBottom(textField);

Why not put together what belongs together. Both of these are layouting commands, the other two are behavior commands. I'd go for:

setCenter(output);
setBottom(input);

output.setEditable(false);
input.setOnKeyReleased(keyReleasedHandler);

Other than that, your code is very concise and understandable. I like your defensive programming style ;)


Get average post score of TOP 10 tags on StackOverflow using Stack Exchange Data Explorer I want to use the Stack exchange data explorer in order to display some basic information about the top tags.

The query should print the number of questions and answers, the average number of answers per question and the average score of posts(questions and answers under such questions). The query is ordered by the total number of questions.

This is my SQL code:

SELECT TOP(10)
  Tags.TagName,
  COUNT(DISTINCT Questions.Id) AS 'Number of Questions',
  COUNT(Answers.Id) AS 'Number of Answers',
  (COUNT(Answers.Id)+0.0)/COUNT(DISTINCT Questions.Id) AS 'Answers per question',
  (
    SELECT AVG(post.Score+0.0)
    FROM Posts post,PostTags tag
    WHERE
      (
      post.Id=tag.PostId
      OR post.ParentId=tag.PostId
      )
      AND tag.TagId=Tags.Id
      ) AS 'average score of posts under this tag(questions and answers)'
FROM Posts Questions
LEFT JOIN Posts Answers
  ON Answers.ParentId = Questions.Id
INNER JOIN PostTags
  ON Questions.Id=PostTags.PostId
INNER JOIN Tags
  ON PostTags.TagId=Tags.Id
GROUP BY Tags.Id,Tags.TagName
ORDER BY COUNT(*) DESC

You can look at the script in the data explorer here.

The problem is that the script times out(in around 100 seconds). If I remove the OR post.ParentId=tag.PostId, it still takes around 25 seconds to run (for Stack Overflow) but it does not time out.

If I run my script on Super User, it does not time out. (It runs around 4 seconds, I think it is because Super User is smaller than Stack Overflow)

Is there a way to run optimize the script so that it does not time out on Stack Overflow without losing functionality and why does that simple OR decrease the performance that much?

The OR is that answers do also count for the average score.

I've added the execution plan files here.

Oh well, I didnt notice the sql-server tag...

What a detailed execution plan they have :)

But anyway the answer may be as simple as that there is more answers than there are questions.

And because you are making another subselect for the column and you are selecting what you already have in the outer select, this may get expensive quite fast.

So I would move the subquery from columns to source tables.

Something like this (sorry, I'm not familiar with sql server syntax):

SELECT TOP(10)
  Tags.Id,
  Tags.TagName,
  COUNT(Questions.Id) AS 'numOfQuestions',
  SUM(Answers.AnswerCount) AS 'numOfAnswers',
  (SUM(Answers.AnswerCount) + 0.0) / COUNT(Questions.Id) AS 'answersPerQuestion',
  (SUM(Questions.Score) + SUM(Answers.ScoreSum) + 0.0) / (COUNT(Questions.Id) + SUM(Answers.AnswerCount)) AS 'averageScore'
FROM Tags
INNER JOIN PostTags
  ON PostTags.TagId = Tags.Id
INNER JOIN Posts Questions
  ON Questions.Id = PostTags.PostId
LEFT JOIN (
  SELECT
    Question.Id AS QuestionId,
    COUNT(Answer.Id) AS AnswerCount,
    Sum(Answer.Score) AS ScoreSum
  FROM Posts Question
  INNER JOIN Posts Answer
    ON Question.Id = Answer.ParentId
  GROUP BY Question.Id
) AS Answers
  ON Answers.QuestionId = Questions.Id
GROUP BY Tags.Id, Tags.TagName
ORDER BY COUNT(Questions.Id) + SUM(Answers.AnswerCount) DESC

Few notes:

I'm intentionally doing left join with the subquery and then inner join in the subquery itself to limit the size of the subquery result. But this means that the outer query must handle possible nulls and replace them with zeroes, which I have not done (because idk sql server syntax), but you should know what I mean.

There is a lot of aggregations going on, but only top 10 are selected in the end. It is possible that the above approach may still not be the most performant. In which case I would recommend to split the query into few separate queries. First extract top 10 using only what is needed for this decision. Then aggregate the other stuff on subset of all posts limited to those 10 tags.

EDIT: I have now noticed that in your question you state:

  The query is ordered by the total number of questions

But your query is not doing that. It orders by COUNT(*) which means that a question without answers is counted as +1 and question with N (N>0) answers is counted as +N.

At first I made my query sort it by number of questions and answers together. Which is yet something different. But to comply with your statement, it should be ORDER BY COUNT(Questions.Id) DESC only. And should have been COUNT(DISTINCT Questions.Id) in your original query...


Searching a dictionary of words using difflib I am trying to write a spellchecker and I wanted to use difflib to implement it. Basically I have a list of technical terms that I added to the standard Unix dictionary (/usr/share/dict/words) that I'm storing in a file I call dictionaryFile.py.

I have another script just called stringSim.py where I import the dictionary and test sample strings against it:

import os, sys
import difflib
import time
from dictionaryFile import wordList

inputString = "dictiunary"
print "Search query: "+inputString
startTime = time.time()

inputStringSplit = inputString.split()
for term in inputStringSplit:
    termL = term.lower()
    print "Search term: "+term
    closeMatches = difflib.get_close_matches(termL,wordList)
    if closeMatches[0] == termL:
        print "Perfect Match"
    else:
        print "Possible Matches"
        print "\n".join(closeMatches)

print time.time() - startTime, "seconds"

It returns the following:

$ python stringSim.py
Search query: dictiunary
Search term: dictiunary
Possible Matches
dictionary
dictionary's
discretionary
0.492614984512 seconds

I'm wondering if there are better strategies I could be using for looking up similar matches (assuming a word is misspelled). This is for a web application so I am trying to optimize this part of the code to be a little snappier. Is there a better way I could structure the wordList variable (right now it is just a list of words)?

Using difflib is probably the best choice. So the problem comes down to the size of your word list. If we can pare down the number of words difflib needs to compare, then we could get a faster time.

One idea to implement, is take only words that are near-enough in length:

# Returns the min and max thresholds for word length
def get_thresholds(word, min=3, max=3):
    length = len(word)
    return max(1, length-min), length+max

# Returns only words whose length is within a certain range
def filter_word_list(min, max):
    return [word for word in words_list if min <= len(word) <= max]

So the call to get_close_matches() would be:

closeMatches = difflib.get_close_matches(termL,
                       dictionaryFile.filter_word_list(*get_thresholds(termL)))

Another idea would be to filter words that begin with a letter that is spatially related to the word's first letter on the keyboard. However, this suggestion is not as plausible due to different keyboard layouts.

As some general comments:

Take a look at your variable names. They are descriptive (for the most part) but the official Python style guide recommends using underscores_between_words instead of camelCaseNames.
Same basic idea for module names. Pythonic module names are written as MyModule instead of myModule. Also, each different module import gets its own line:

# Not `import os, sys`
import os
import sys


SQL Query to display minimum prices per country I have developed a Tender management System sort of code for one of my client. This is the piece of code :

try {
            Connection con = Mycon.getConnection();
                PreparedStatement ps = con.prepareStatement("TRUNCATE tempcalcplan");
                ps.executeUpdate();
                ps.clearBatch();
                ps = con.prepareStatement("INSERT INTO tempcalcplan SELECT v.conid,c.conname, v.rate, v.venid FROM venprices v LEFT JOIN  country c ON c.conid = v.conid WHERE (v.conid, v.rate) IN ( SELECT v.conid, MIN(v.rate) FROM venprices v GROUP BY v.conid) GROUP BY v.conid");
                ps.executeUpdate();
                ps.clearBatch();
                ps = con.prepareStatement("select * from tempcalcplan");

         ResultSet rs = ps.executeQuery();
         jTable1.setModel(DbUtils.resultSetToTableModel(rs));
        }catch(Exception e){
                e.printStackTrace();
        }

This piece of code is designed to function as :

Empty the table tempcalcplan(Assuming that previous results are stored.)
Generating desired data with the help of 2 different tables(Tables are shown below) and storing it in the tempcalcplan table
Displaying the results of tempcalcplan table on a jTable.

Here Are the country and tempcalcplan table.

country (Contains 30,000 records)

+---------+-------------+------+-----+---------+-------+
| Field   | Type        | Null | Key | Default | Extra |
+---------+-------------+------+-----+---------+-------+
| conid   | int(10)     | NO   | PRI | NULL    |       |
| conname | varchar(50) | YES  |     | NULL    |       |
+---------+-------------+------+-----+---------+-------+

tempcalcplan (Contains 1,80,000+ Records)

+---------+-------------+------+-----+---------+-------+
| Field   | Type        | Null | Key | Default | Extra |
+---------+-------------+------+-----+---------+-------+
| conid   | int(10)     | NO   | PRI | NULL    |       |
| conname | varchar(50) | YES  |     | NULL    |       |
| rate    | double      | YES  |     | NULL    |       |
| venid   | varchar(50) | YES  |     | NULL    |       |
+---------+-------------+------+-----+---------+-------+

venprices

+-------+--------------+------+-----+---------+-------+
| Field | Type         | Null | Key | Default | Extra |
+-------+--------------+------+-----+---------+-------+
| conid | int(10)      | NO   | PRI | NULL    |       |
| rate  | double       | YES  |     | NULL    |       |
| venid | varchar(50)  | NO   | PRI |         |       |
+-------+--------------+------+-----+---------+-------+`

The results take up to 12-15 minutes to show in the jTable.
I want reduce it to 1-2 minutes.

try {
            Connection con = Mycon.getConnection();

This probably could be

    try (Connection con = Mycon.getConnection()) {

Assuming that this is the Java.sql.Connection interface.

Then the try-with-resources would be able to close the connection for you--exception or no exception.

But that won't help with your speed issues.

You need to determine where the problem lies.  Is it the query at the end?  Is it the population of the table?  The creation of the jTable?  Where?

Are there any conid values in venprices that aren't in country?  It would seem that there would have to be, as there are only 30,000 records in country and 180,000 in tempcalcplan.  Both have conid as their primary key, so no duplicates.

Is there an index on conid and rate in venprices?


Code Hunter 2.06: Count occurrences of 'a' in a string I'm playing Code Hunt on level 2.06 (loops).  This level is about counting the number of "a" in a String.

public class Program {
public static int Puzzle(String s) {
   int ret = 0;
   for(int x = 0; x < s.length(); x++){
       if(s.charAt(x).equals('a')) ret++;
    }
return ret;
}

It actually works, but the game scores your code from 1 to 3 depending on the elegance of the code.  Any ideas on improving that elegance?

This is a nice short piece of code, though it has a number of 'features' which can be improved.

Code Style

Java code style recommends a lower-case letter to start method names.... Puzzle should be puzzle, or better, getLetterCount. You may be limited by the testing tool though.

Efficiency

Inside the method, the biggest issue I have is the poor efficiency. A string in Java is essentially an array of 'char' primitive values: char[]

Your code takes each char, converts it to a Character (autoboxing), runs the equals('a') method on that Character, which converts the char 'a' to a Character a and does the equals on that.

It's normally more efficient to compare the primitives using == instead.

Consider the following code:

public class Program {
    public static int countAChars(String s) {
        int count = 0;
        for(int i = s.length() - 1; i >= 0; i--) {
            if('a' == s.charAt (i)) {
               count++;
            }
        }
        return count;
    }
}

Note: in the loop above I go backwards from the end of the String, which has occasionally been faster than going forwards, at least in my testing.

The following alternative code is more readable, but converts the String to an array first, which will be slightly slower....

public class Program {
    public static int countAChars(String s) {
        int count = 0;
        for(char c : s.toCharArray()) {
            if('a' == c) {
               count++;
            }
        }
        return count;
    }
}

Performance

I took a moment to code up the performance of a number of different solutions that have been posted so far. I used 'MacBeth' as an input file (this link will download the XML).

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Paths;

public class Counter {

    private interface ACount {
        int countA(String line);

        String name();
    }

    private static final ACount[] COUNTERS = { new ACount() {

        @Override
        public int countA(String line) {
            int cnt = 0;
            for (int i = line.length() - 1; i >= 0; i--) {
                if (line.charAt(i) == 'a') {
                    cnt++;
                }
            }
            return cnt;
        }

        @Override
        public String name() {
            return "charAt()";
        }
    }, new ACount() {

        @Override
        public int countA(String s) {
            int ret = 0;
            for (int x = 0; x < s.length(); x++) {
                if (((Character) s.charAt(x)).equals('a'))
                    ret++;
            }
            return ret;
        }

        @Override
        public String name() {
            return "OPCode";
        }
    }, new ACount() {

        @Override
        public int countA(String line) {
            return line.length() - line.replace("a", "").length();
        }

        @Override
        public String name() {
            return "replace";
        }
    }, new ACount() {

        @Override
        public int countA(String line) {
            int cnt = 0;
            for (char c : line.toCharArray()) {
                if (c == 'a') {
                    cnt++;
                }
            }
            return cnt;
        }

        @Override
        public String name() {
            return "toCharArray";
        }
    }, };

    private static final int countAllA(ACount counter, String[] lines) {
        int cnt = 0;
        for (String line : lines) {
            cnt += counter.countA(line);
        }
        return cnt;
    }

    private static final void reportCounts(String name, String[] lines) {
        for (ACount counter : COUNTERS) {
            long start = System.nanoTime();
            int ac = countAllA(counter, lines);
            long nanos = System.nanoTime() - start;
            System.out.printf("%10s %12s %8d  %12.3fms%n", name,
                    counter.name(), ac, nanos / 1000000.0);
        }
        System.out.println();
    }

    public static void main(String[] args) throws IOException {
        String[] lines = Files.readAllLines(
                Paths.get(args.length == 0 ? "macbeth.xml" : args[0])).toArray(
                new String[0]);
        for (int i = 0; i < 100; i++) {
            reportCounts("Warmup " + i, lines);
        }
        reportCounts("Real Run", lines);

    }

}

This produces the results:

  Real Run     charAt()     5065         0.311ms
  Real Run       OPCode     5065         0.872ms
  Real Run      replace     5065         3.933ms
  Real Run  toCharArray     5065         0.387ms

The 1-liner answer you have accepted (and posted as your answer), is 10 times slower than the alternatives.

Your original code (if I cast the char to Character), is twice as slow as the alternatives I suggest.

What you determine to be elegant, is not, in my opinion, a good solution. If "Code Hunt" thinks the slow solution is the best solution, then I would find a different system for evaluating my code.


gcc installation script for Centos6.6 I have written this script to install gcc (here in version 4.9.2) on Centos6.6. I am only interested in C and C++ support.

Please comment if this is any good and can I make it better before I get the packages for those.

#!/bin/bash

LOGFILE=/tmp/gcc_install.log

# $1 - package name
# $2 - status
errorIf(){
if [ $2 -ne 0 ]; then
    echo "Something was wrong with $1"
    exit 1
fi
}

################################################################################
################################# SCRIPT START #################################

if [[ $EUID -ne 0 ]]; then
   echo "This script must be run as root" 1>&2
   exit 1
fi

# only one param - gcc's source directory absolute location
if [ $# -ne 1 ]; then
    echo "Please give the gcc source directory as argument"
    exit 1
fi

if [ ! -d "$1" ]; then
    echo "gcc's source directory that you have passed doesn't exist!"
    exit 1
fi

SOURCE_DIR="$1"
SOURCE_DIR_NAME="${SOURCE_DIR##*/}"
cd "$SOURCE_DIR"/contrib/

################################################################################
################################# gmp, mpgr, mpc  ##############################
# this script assumes that you have ran download_prerequisites from gcc's contrib dir
# contrib with downloaded gmp, mpgr, mpc (used download_prerequisites)
if [ ! -d gmp ] || [ ! -d mpfr ] || [ ! -d mpc ]; then
    echo "There is no gmp or mpfr or mpc directory in `pwd`. Please run download_prerequisites."
    exit 1
fi

echo "Installing gcc and dependencies. You can check the status and results in $LOGFILE"

#gmp
cd gmp && ./configure --enable-shared --enable-static --prefix=/usr/local >$LOGFILE 2>&1 && make >$LOGFILE 2>&1 && make check >$LOGFILE 2>&1 && make install >$LOGFILE 2>&1 && cd ..
ret="$?"
errorIf gmp $ret
echo "gmp Installed!:)"

#mpfr
cd mpfr && ./configure --enable-shared --enable-static --prefix=/usr/local --with-gmp=/usr/local >$LOGFILE 2>&1 && make >$LOGFILE 2>&1 && make check >$LOGFILE 2>&1 && make install >$LOGFILE 2>&1 && cd ..
ret="$?"
errorIf mpfr $ret
echo "mpfr Installed!:)"

#mpc
cd mpc && ./configure --enable-shared --enable-static --prefix=/usr/local --with-gmp=/usr/local --with-mpfr=/usr/local >$LOGFILE 2>&1 && make >$LOGFILE 2>&1 && make check >$LOGFILE 2>&1 && make install >$LOGFILE 2>&1 && cd ..
ret="$?"
errorIf mpc $ret
echo "mpc Installed!:)"

echo /usr/local/lib/ >> /etc/ld.so.conf
echo /usr/local/lib64/ >> /etc/ld.so.conf
ldconfig

cd ../../
ulimit -s 32768 # for gcc tests
mkdir -p /usr/share/gdb/auto-load/usr/lib
mkdir -p gcc-build
cd gcc-build

# find mv .py command is due to this ldconfig error (gcc copies some .py files into /usr/local/lib64/)
# ldconfig: /usr/local/lib/../lib64/libstdc++.so.6.0.20-gdb.py is not an ELF file - it has the wrong magic bytes at the start.
../$SOURCE_DIR_NAME/configure --enable-shared --disable-bootstrap --with-system-zlib --enable-languages=c,c++ --enable-libgomp --enable-threads=posix --with-gmp=/usr/local --with-mpfr=/usr/local --with-mpc=/usr/local --with-fpmath=sse --disable-multilib >$LOGFILE 2>&1 && make >$LOGFILE 2>&1 && make install >$LOGFILE 2>&1 && find /usr/local/lib64 -iname "*.py" -exec mv {} /usr/share/gdb/auto-load/usr/lib/ \;  >$LOGFILE 2>&1 && ldconfig >$LOGFILE 2>&1

echo "gcc from $SOURCE_DIR_NAME Installed!:)"

For the most part, your script is quite fine.

Taming the long lines

These blocks of code have some issues:

cd gmp && ./configure --enable-shared --enable-static --prefix=/usr/local >$LOGFILE 2>&1 && make >$LOGFILE 2>&1 && make check >$LOGFILE 2>&1 && make install >$LOGFILE 2>&1 && cd ..

Issues:

The first line is too long

It's easy to be lazy to scroll to the right
Code that's less visible, is more error-prone, easy to overlook something
Long lines are harder to edit in practice

Wrapping a bunch of commands in cd somewhere; ...; cd .. is error prone:
it's an extra mental burden to make sure that you'll end up in the right directory after the chain of commands, keeping in mind possible errors too.
As much as possible, it's good to look for alternatives.
Too much repetition of >$LOGFILE 2>&1

It would be better this way:

(
    cd gmp && \
        ./configure --enable-shared --enable-static --prefix=/usr/local && \
        make && make check && make install
) >$LOGFILE 2>&1

The (...) is a sub-shell,
so directory changes won't affect the enclosing script.
Once the sub-shell is finished,
you'll be back in the original directory.
Grouping commands like this (also with {...}) makes it easier to redirect all output.
I didn't break up the lines of make a b c,
because the line was already short enough,
and quite idiomatic that way.
The exit code after the (...) will be as you would expect,
the grouping won't affect it.

Do similarly for all the long lines.

Avoid changing directories in the middle of scripts.
When you must do, consider wrapping the relevant commands within (...).

Simplifying the exit code handling

The exit code handling can be simpler:

ret="$?"
errorIf gmp $ret
echo "gmp Installed!:)"

There's no need to save $? in ret.
You could use it directly.
In fact, the function can use it directly,
no need to pass it explicitly.
You can rewrite the errorIf function like this:

errorIf() {
    if [ $? -ne 0 ]; then
        echo "Something was wrong with $1"
        exit 1
    fi
}

And then simplify the exit code handling to this:

errorIf gmp
echo "gmp Installed!:)"

ShellCheck

The http://www.shellcheck.net/# website is pretty awesome.
It points out a few additional issues I recommend to fix.


Bottom-up singly linked list merge sort in C++ This is an implementation of a singly linked list merge sort I did on paper during a job interview recently, using a bottom-up approach.

The requirement was simply "implement a merge sort for a singly linked list whose last node is indicated by a null value in its next pointer. An empty list is simply a null pointer to a node."

After having reviewed the code, the company decided I wasn't fit for their needs, but didn't want to disclose what in the code wasn't up to their standards. They however stressed that they not only care about whether the program works, but also whether it's elegant and easy to read.

I used a bottom-up approach because of its in-place merging abilities that make it optimal in space: O(1).

Would anybody tell me what is possibly wrong with my implementation and how it could be improved?

template <typename T>
struct Node {
    Node *next;
    T data;
};

template <typename T>
Node<T> **skip(Node<T> **pN, size_t s) {
    while (s-- && *pN)
        pN = &(*pN)->next;

    return pN;
}

template <typename T>
void insert_before(Node<T> **pAfter, Node<T> *n) {
    n->next = *pAfter;
    *pAfter = n;
}

template <typename T>
Node<T> *remove(Node<T> **pN) {
    Node<T> *n = *pN;
    *pN = n->next;
    n->next = 0;

    return n;
}

template <typename T>
Node<T> **merge_two_first_sublists(Node<T> **pN, size_t s) {
    Node<T> **pLeft = pN;
    Node<T> **pRight = skip(pN, s);

    while (*pRight && s != 0 && pLeft != pRight) {
        if ((*pRight)->data < (*pLeft)->data) {
            insert_before(pLeft, remove(pRight));
            --s;
        }
        else
            pLeft = &(*pLeft)->next;
    }

    return skip(pRight, s);
}

template <typename T>
Node<T> *merge_sort(Node<T> *n) {
    size_t s = 1;
    size_t l;

    do {
        Node<T> **pNextFirst = &n;

        for (l = 0; *pNextFirst; ++l)
            pNextFirst = merge_two_first_sublists(pNextFirst, s);

        s *= 2;
    } while (l > 1);

    return n;
}

I can see one obvious small change: merge the remove() and insert_before() functions into one move_before() function, which would  make the n->next = 0; line in the remove() function unnecessary, but do you think it's worth it?

Another change could be to use int rather than size_t, as many nowadays generally consider it harmful to use unsigned integers, but given that the type is used only internally by the merge algorithm itself and that underflows are guaranteed to not happen, is that an issue worth targetting?

A compile-and-run version on coliru: http://coliru.stacked-crooked.com/a/81638c30d4bc524f

For what it's worth, I feel your pain about not getting any useful feedback from an interview. At least they actually told you that you didn't get the job! I've had companies (very well known, well-respected companies) not contact me to let me know I didn't get the job. In any event, here are some improvements that I think you could make.
Naming
Your functions are named reasonably well, but your variables are not. Instead of pN, just write node. Instead of s, write size or length. It will make your code far easier to read. Also, Hungarian notation (prefixing variables with a letter to indicate type) is out-of-fashion. Even if it weren't, I'm not sure you've done it correctly. A pointer to a pointer to a node should be ppN, not pN, shouldn't it?
The name merge_two_first_sublists() is really confusing. Why are they called first? And why don't you pass the function 2 lists? Even if you implement it in O(1) space, you can still pass in the pointer to each list. And if you do it that way you can use it even for lists that aren't contiguous.
In merge_sort() you have a variable simply named l. This is a dangerous name. In many fonts, it looks like a numeral 1. In general 1-letter variable names are a bad idea. I can give a pass for loop counters i and j, but beyond that, you should really avoid them.
Classes
You're programming in an object-oriented language, but you aren't using objects, which is a little odd. Why aren't insert_before(), remove() and skip() methods on a List class? It's not clear to me from the requirements you posted that it wouldn't be acceptable to do it that way. It would encapsulate the list in a convenient way and protect it from being modified elsewhere.
Overuse of Pointers
C++ has a lot of ways to avoid using pointers. The reason it has them is because pointers are extremely error prone and code that uses raw pointers is a maintenance headache. You've doubled-down on pointers and are using pointers-to-pointers in every function. It doesn't look to me like you need them in most of the functions. For example, why can't skip() be written like this:
template  <typename T>
Node<T> *skip(Node<T> *pN, size_t s) {
    while (s-- && pN)
        pN = pN->next;

    return pN;
}

What do you gain by using a pointer to a pointer?
How does insert_before() even work? The first argument is a pointer-to-a-pointer to the node we wish to insert after the second argument, n. So we have to end up with n->next assigned to the address of the "after" node. Don't we also need to have the "after" node's next pointer pointing to whatever n->next was pointing to before the call? I don't see that happening anywhere. And for the life of me, I can't figure out what the last line of the function is doing. Why do you set the pointer that was pointing to the pointer to the "after" node to point to the "before" node? What does that accomplish?
Why does remove() return anything? I wouldn't expect a remove() function to return any value. I'd just expect it to remove the node in question from the list. But it doesn't even take a list an as argument, just a pointer to a pointer to a node. This is going to be difficult to use and maintain, in my experience.
I recommend avoiding pointers at all if you can. In C++ you can use references instead of pointers in many cases. With C++11 and later, you can use shared_ptr and unique_ptr where appropriate. The requirements do say that the list uses pointers, so at least encapsulate them as best you can. If you have to have a function that modifies a pointer that's an argument you can pass in a reference to a pointer instead of a pointer to a pointer.


Time Converter - Python Tkinter - General comments on code I have been learning python since being in lockdown, this is the first Tkinter application I've built without consulting the internet or seeking help anywhere.
It is a time converter where you can select the input unit via a checkbox and the other entry windows will fill with the correct conversion.
I'd like to hear feedback on how it's written, in terms of if I have demonstrated best practice or areas where I could have used less code.
from tkinter import *
from tkinter.ttk import*

root = Tk()
Second = IntVar()
Minute = IntVar()
Hour = IntVar()

Var1 = IntVar()
Var2 = IntVar()
Var3 = IntVar()

def ConvertTime():
    if Var1.get() == 1:
        Minute.set(Second.get()/60)
        Hour.set(Second.get()/3600)

    if Var2.get() == 1:
        Second.set(Minute.get()*60)
        Hour.set(Minute.get()*0.01667)

    if Var3.get() == 1:
        Second.set(Hour.get()*3600)
        Minute.set(Hour.get()*60)

class GUI:

    def __init__(self, master):

        self.master = master
        master.title('Time Converter')
        master.geometry('+600+300')

        master.bttn = Button(root, text = 'Convert', command = ConvertTime)
        master.bttn.grid(pady= 5, padx=20, column=1, row=3)

        master.ChooseSecond = Checkbutton(root, text = 'Seconds', variable=Var1, onvalue = 1, offvalue = 0)
        master.ChooseSecond.grid(pady=5, padx=20, column=0, row=0)

        master.entry1 = Entry(root, textvariable=Second)
        master.entry1.grid(pady=5, padx=20, column=0, row=1)

        master.ChooseMinute = Checkbutton(root, text = 'Minutes', variable=Var2, onvalue = 1, offvalue=0)
        master.ChooseMinute.grid(pady=5, padx=20, column=1, row=0)

        master.entry2 = Entry(root, textvariable=Minute)
        master.entry2.grid(pady=5, padx=20, column=1, row=1)

        master.ChooseHour = Checkbutton(root, text = 'Hours',variable=Var3, onvalue=1, offvalue=0)
        master.ChooseHour.grid(pady=5, padx=20, column=2, row=0)

        master.entry3 = Entry(root, textvariable=Hour)
        master.entry3.grid(pady=5, padx=20, column=2, row = 1)

def main():
    GUI(root)
    root.mainloop()

main()

One issue I can see on first glance is the *. It's considered bad practice to import everything like that, and here is why from this Stack Overflow post:

Because it puts a lot of stuff into your namespace (might shadow some other object from previous import and you won't know about it).
Because you don't know exactly what is imported and can't easily find from which module a certain thing was imported (readability).
Because you can't use cool tools like pyflakes to statically detect errors in your code.


Print max occurring character in a String This prints the character that occurs the most in a given string.

Input: String of word(s)

Output: Max occurring character

public class mostOccurringCharacter {
public static  Character findMaxOChar(String text){
    HashMap<Character, Integer> map = new HashMap<Character, Integer>();
    Character maxChar = ' ';
    long max = Integer.MIN_VALUE;
    for(int i=0;i<text.length();i++){
        Character current = text.charAt(i);
        if(map.containsKey(current)){
            map.put(current,map.get(current)+1);
        }
        else{
            map.put(current,1);
        }
        if(map.get(current)>max){
            maxChar = current;
            max = map.get(current);
        }
    }
    return maxChar;
}

public static void main(String args[]){
    String text = "wfaveqr caaaaaaaaaaaafwefwgqrvwerwbhqfvwrfwvbetqfwef q fwgwfw erfq";
    System.out.println("The max character is : "+ findMaxOChar(text));
}
}

Primitives over wrapper classes

public static Character findMaxOChar(String text) { ... }

It's usually recommended to prefer the primitive type over its wrapper class, unless you have a clear intention to return null values for indicating the absence of a value.

Interfaces over implementations

HashMap<Character, Integer> map = new HashMap<Character, Integer>();

Using the Map interface over the HashMap implementation is often recommended, as subsequent usage of the object only needs to know it's a Map. Since Java 7, you can rely on type inference for generic object creation to eliminate the redundant type specification on the right, i.e.:

Map<Character, Integer> map = new HashMap<>();

Java 8 stream-based processing

Assuming you are on Java 8 and eager to learn about its new stream-based processing, this is a good candidate for adopting this approach. :) For starters, you need to understand:

String.chars() can give you an IntStream of the string's characters, as long as we're talking about Western-based encoding.
A Collector that lets you apply some processing on a Stream to return some form of results, e.g. a Map in your case. Converting an IntStream to Stream<Integer> by calling IntStream.boxed().

Building the Map can be just:

Map<Character, Long> map = text.chars()
                                .boxed()
                                .collect(Collectors.groupingBy(
                                            i -> Character.valueOf((char) i.intValue()),
                                            Collectors.counting()));

Here, we are groupingBy mapping the Integer values into Characters, and counting the number of occurrences.

Following the same idea,  you can call Map.entrySet().stream(), i.e. to stream on map's entries, to:

Group by the entries' values now, into a TreeMap, by mapping the entries' keys into a List,
Get the TreeMap's last entry, and from that entry's value, find any value to return as the result.


Sum of all the primes less than or equal to N public static long sumOfPrimes(long max){
    long sum = 0;
    long primes[] = new long[((int)max/2)+1];
    int index = 0;
    for(long counter = 2; counter <= max; counter++){
        if(isPrime(counter, primes)){
            sum += counter;
            primes[index++] = counter;
        }
    }
    return sum;
}

public static boolean isPrime(long num, long[] primes){
    if(num == 2 || num == 3){
        return true;
    }

    //System.out.println(Arrays.toString(primes));
    long primesCount = primes.length;
    for(int i = 0; i < primesCount; i++){
        if(primes[i] !=0 && num % primes[i] == 0){
            return false;
        }
    }

    long range = (long) Math.sqrt((double)num);
    if(primesCount > 0 && primes[primes.length-1] < range)
    {
        for(long counter = 2; counter <= range; counter++){
            if(num % counter == 0){
                return false;
            }
        }
    }
    return true;
}

This code works fine for smaller N. For larger N, it throws timeout exception.

How can I improve my code? How do I fix timeout issue?

Sieve of Eratosthenes

The first possibility would be to implement the Sieve of Eratosthenes.  It's one of the more efficient ways to find all primes between 1 and N.  But there are other things we can do with your existing algorithm.

Initialize with 2

    long sum = 0;
    long primes[] = new long[((int)max/2)+1];
    int index = 0;

But you know the first prime number (at least I hope you do).  So try this

    if (max < 2) {
        return 0;
    }
    long sum = 2;

    long primes[] = new long[((int)max/2)+1];
    primes[0] = 2;
    int index = 1;

This helps a little now, but it helps a lot later since we know that 2 is the only even prime.  Since all even numbers are divisible by 2 and all primes are only divisible by themselves and 1, 2 is the only possible even prime.

Check fewer numbers

    for(long counter = 2; counter <= max; counter++){

This checks every number from 2 to max, but we can trivially reduce this to every odd number from 3 to max if we initialize the array with 2.

    for (long counter = 3; counter <= max; counter += 2) {

I also added some additional spaces, as I find they make it easier to read the code.

No evens or divisible by three

And we can actually do even better.  Every third odd number is divisible by three.  So

    if (max < 2) {
        return 0;
    } else if (max == 2) {
        return 2;
    }
    long sum = 5;

    long primes[] = new long[((int)max/2)+1];
    primes[0] = 2;
    primes[1] = 3;
    int index = 2;

    int increment = 4;
    for (long counter = 5; counter <= max; counter += increment) {
         increment = 6 - increment;

Now the increment varies between 2 and 4.  So we have 5, 7, 11 (skipping 9), 13, 17...  So we skip every number that is divisible by three.  And you can never add an even number to an odd number and get an even number, so we skip all the evens as well.

Don't use a generic isPrime

Your code uses a generic isPrime method that will work regardless of the caller.  You don't need to do that.

    if(num == 2 || num == 3){
        return true;
    }

You can skip this, as you never pass 2 or 3 to this method.

    //System.out.println(Arrays.toString(primes));

This is debugging code and shouldn't be sent to review.

    long primesCount = primes.length;
    for(int i = 0; i < primesCount; i++){
        if(primes[i] !=0 && num % primes[i] == 0){
            return false;
        }
    }

You can simplify this to just

    for (long prime : primes) {
        if (prime == 0) {
            return true;
        }

        if (num % prime == 0) {
            return false;
        }
    }

Now it will check that num is not divisible by any number in the primes array.  Once it reaches the first 0, that means that it is done processing and can return true immediately.

Note that if primes were a List rather than an array, you wouldn't need to check for 0, as the List would only have prime values in it.  Perhaps the performance improvement from not checking for 0 values would outweigh the increased overhead of the List.

    long range = (long) Math.sqrt((double)num);
    if(primesCount > 0 && primes[primes.length-1] < range)
    {
        for(long counter = 2; counter <= range; counter++){
            if(num % counter == 0){
                return false;
            }
        }
    }

You don't need this block of code.  You already checked that it's not divisible by any of the primes less than it.  You don't need to check it again with both primes and non-primes.

It's possible that you could save some time with the following optimizations:

    long range = (long) Math.sqrt((double)num);
    for (long i = 2; primes[i] <= range; i++) {
        if (num % primes[i] == 0) {
            return false;
        }
    }

    return true;

This relies on there always being at least one prime between range and num in the primes array.  You might have to preload 5 into the primes array to make this work, as this skips past 3 which is greater than range.  Alternatively you could check that primes[i] is not 0, but that would never happen if you start checking with 7.

Since we don't pass numbers divisible by 2 or 3 to this method, we don't need to attempt to divide by them.  So we start with the third prime, primes[2] which is 5.

Note that this makes isPrime dangerously fragile.  You may want to make it private rather than public to make it clear that it should only be called under limited circumstances.  Unless performance is a big issue, the previous version is more robust.  And the previous version may be faster anyway.


Differentiate simple expressions This is the first program I have written in Rust and I would like to know what some Rust experts think of it. I must be doing some stuff wrong, such as breaking code style, etc. The code differentiates simple expressions.

Two points of interest: there are verbose Box structures everywhere, and it makes the code less readable. Is there an alternative for this?

Second, I have a let ee = e.clone(); just so that I can print the value of e after calling diff. Coming from C++, Haskell, etc, this is quite strange. I understand that ownership is lost when the function is called, but I am hoping for a nicer way to do the print. One option I see is to let diff return the input as well, but this is strange.

use self::Expression::{Prod, Sum, Pow, Var, Cons};

#[derive(Debug, PartialEq, Clone)]
enum Expression {
    Cons(i32),
    Var(char),
    Prod(Box<Expression>, Box<Expression>),
    Sum(Box<Expression>, Box<Expression>),
    Pow(Box<Expression>, i32)
}

fn diff(e : Box<Expression>) -> Box<Expression> {
    match *e {
        Prod(ref a, ref b) => Box::new(Sum(Box::new(Prod(diff(a.clone()), b.clone())),
            Box::new(Prod(a.clone(), diff(b.clone()))))),
        Sum(ref a, ref b) => Box::new(Sum(diff(a.clone()), diff(b.clone()))),
        // only valid for Var^n
        Pow(ref a, ref b) => Box::new(Prod(Box::new(Cons(*b)), Box::new(Pow(a.clone(), *b - 1)))),
        Var(_) => Box::new(Cons(1)),
        Cons(_) => Box::new(Cons(0)),
    }
}

fn main() {
    let e = Box::new(Prod(Box::new(Cons(5)), Box::new(Pow(Box::new(Var('x')), 2)))); // 5*x^2
    let ee = e.clone(); // this is ugly..
    let d = diff(e); // d/dx 5*x^2 = 0*x^2 + 5*2*x^1

    println!("Differentiating {:?} yields {:?}", ee, d);
}

I strongly recommend using Expression as your basic type, not Box<Expression> as you are. The boxing should be purely an implementation detail of the recursive type, it should not escape beyond that. For that reason, diff will return Expression rather than Box<Expression>; as far as the argument it takes is concerned, there are a couple of approaches that may be used:

You may choose to take an Expression by value; in this case, stop using ref a et al. in the match patterns and just use a, thus improving efficiency ever so slightly in some of those cases, and reducing the number of clones needed by one (a in Pow).
You may choose to take it by reference, &Expression. In this case, you won’t need to clone the entire expression e in main. This will require a mite less cloning than the first choice for your specific case.

There is little practical difference between the two; it is mostly just a matter of shuffling things around. I would probably go with the second, taking &Expression.

For the Pow(ref a, ref b) pattern, taking ref b rather than b, given b is i32, is rather pointless.

That is the basic stuff out of the way, and leaves us with this code (note a couple of minor stylistic things as well, such as e: instead of e :, and having a trailing comma after the last variant in the enum definition, and using visual indent on line 15):

use self::Expression::{Prod, Sum, Pow, Var, Cons};

#[derive(Debug, PartialEq, Clone)]
enum Expression {
    Cons(i32),
    Var(char),
    Prod(Box<Expression>, Box<Expression>),
    Sum(Box<Expression>, Box<Expression>),
    Pow(Box<Expression>, i32),
}

fn diff(e: &Expression) -> Expression {
    match *e {
        Prod(ref a, ref b) => Sum(Box::new(Prod(Box::new(diff(a)), b.clone())),
                                  Box::new(Prod(a.clone(), Box::new(diff(b))))),
        Sum(ref a, ref b) => Sum(Box::new(diff(a)), Box::new(diff(b))),
        // only valid for Var^n
        Pow(ref a, b) => Prod(Box::new(Cons(b)), Box::new(Pow(a.clone(), b - 1))),
        Var(_) => Cons(1),
        Cons(_) => Cons(0),
    }
}

fn main() {
    let e = Prod(Box::new(Cons(5)), Box::new(Pow(Box::new(Var('x')), 2))); // 5*x^2
    let d = diff(&e); // d/dx 5*x^2 = 0*x^2 + 5*2*x^1

    println!("Differentiating {:?} yields {:?}", e, d);
}

Other alterations I would be inclined to make are making diff a method rather than a free function (fn diff(&self) -> Expression). Your naming is also inconsistent: the enum has a full-word name, Expression and not Expr, while the variants have short names, Cons and not Constant and so forth. I would recommend making them consistent.

I also have a personal fondness for struct variants, e.g. Power { base: Box<Expression>, exponent: i32 } instead of Power(Box<Expresion>, i32) (in patterns, Power(ref a, b) would become Power { ref base, exponent } with the values being bound to the better names base and exponent instead of a and b, another change you should consider anyway), but this is subjective.

Now for the big usability wins: implementing operators. Why should Expression not implement std::ops::{Add, Mul} to create Product and Sum? And while there is no power operator, why not add a similar method for it? These things can make it much simpler to look at and read. I’m only going to impl Add<Expression, Output = Expression> for Expression (i.e. Expression + Expression -> Expression) but it would be possible to also implement it for combinations of boxedness on Self and RHS, which could allow a.clone() instead of (**a).clone() in the Product differentiation below, but I do not believe it to be a particularly good idea; it is not any more efficient and clutters things ever so slightly; I think it is best if Box is kept purely an implementation detail.

We might as well also make a simple implementation of std::fmt::Display for bonus marks. std::fmt::Debug output gets a bit tiring to look at occasionally.

Well, here’s what I end up with:

use std::fmt;
use std::ops;
use self::Expression::{Product, Sum, Power, Variable, Constant};

#[derive(Debug, PartialEq, Clone)]
enum Expression {
    Constant(i32),
    Variable(char),
    Product(Box<Expression>, Box<Expression>),
    Sum(Box<Expression>, Box<Expression>),
    Power(Box<Expression>, i32),
}

impl ops::Mul for Expression {
    type Output = Expression;

    fn mul(self, rhs: Expression) -> Expression {
        Expression::Product(Box::new(self), Box::new(rhs))
    }
}

impl ops::Add for Expression {
    type Output = Expression;

    fn add(self, rhs: Expression) -> Expression {
        Expression::Sum(Box::new(self), Box::new(rhs))
    }
}

impl Expression {
    // Yeah, I’m calling this method pow because for a *method* it’s the standard name.
    fn pow(self, power: i32) -> Expression {
        Expression::Power(Box::new(self), power)
    }

    fn diff(&self) -> Expression {
        match *self {
            Product(ref a, ref b) => a.diff() * (**b).clone() + (**a).clone() * b.diff(),
            Sum(ref a, ref b) => a.diff() + b.diff(),
            // only valid for Variable^n
            Power(ref a, b) => Constant(b) * a.clone().pow(b - 1),
            Variable(_) => Constant(1),
            Constant(_) => Constant(0),
        }
    }
}

impl fmt::Display for Expression {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        match *self {
            Product(ref a, ref b) => write!(f, "({} * {})", a, b),
            Sum(ref a, ref b) => write!(f, "({} + {})", a, b),
            Power(ref a, b) => write!(f, "({} ^ {})", a, b),
            Variable(v) => fmt::Display::fmt(&v, f),
            Constant(c) => fmt::Display::fmt(&c, f),
        }
    }
}

fn main() {
    let e = Constant(5) * Variable('x').pow(2);
    let d = e.diff(); // d/dx 5*x^2 = 0*x^2 + 5*2*x^1

    println!("Differentiating {} yields {}", e, d);
}

One final comment—if your power differentiation rule is only applicable to Variable^n (of course, mathematically that’s requiring that you are differentiating with regards to that variable) then it would be good to enforce that and fail rather than produce a bad result. One might even make diff take a char argument as the variable that you are differentiating with regards to, if one wished to make it more correct. All of this sort of thing might lead to you returning a Result from diff so that you can return an error if you wind up with something that you cannot differentiate.


Python beginner looking for feedback on simple dice game Dice "Game"
So, just started python a weeks time ago, so I'm looking for feedback/critique on things i can do to improve my code. I wrote a little user/pc dice rolling game. Any feedback at all is appreciated!

def dice(n: int) -> tuple:
    """
    Simple function to roll 'n' number of dices.
    Returns numbers of each roll and a total.

    :param n: number of dices to roll
    :return: dices_numbers, total
    """
    numbers = []
    total = 0
    for dices in range(n):
        number = random.randint(1, 6)
        numbers.append(number)
        total += number
    dices_numbers = ", ".join(map(str, numbers))
    return dices_numbers, total

def dice_check(user_total: int = 0, pc_total: int = 0) -> None:
    """
    Function to check whether user or pc won dice roll.

    :param user_total: user's total roll
    :param pc_total:  pc's total roll
    :return: None
    """
    if user_total > pc_total:
        print("Congratulations! You won")
    elif user_total < pc_total:
        print("Sorry, BOT wins!")
    else:
        print("Tie!")

number_of_dices = ""
user_roll = ""
pc_roll = ""
while True:
    number_of_dices = int(input("How many dices would you like to roll?"
                                " (0 to quit) >"))
    if number_of_dices == 0:
        break
    else:
        user_roll = dice(number_of_dices)
        pc_roll = dice(number_of_dices)
        print(f"You: Roll: {user_roll[0]}, a total of: {user_roll[1]}")
        print(f"BOT: Roll: {pc_roll[0]}, a total of: {pc_roll[1]}")
        dice_check(user_roll[1], pc_roll[1])

I'm pretty new as well, but I like to create an exception whenever I try to convert user input to an integer. Something like this-
while True:
    try:
        number_of_dices = int(input("How many dices would you like to roll?"
                                    " (0 to quit) >"))
    except ValueError:
        print("Please enter a number.")


jQuery vertical dropdown menu This code works great and does exactly as it should, but there has to be a better way of doing this and I just can't figure out how!

I'm pretty new to jQuery and am trying to use it more myself instead of using plugins and I can get stuff to work fine. It just isn't the tidiest or neatest code, which I'm trying to move away from!

I've included a fiddle here just so you can see the functionality as it currently stands.

HTML

Example 1

Example 2

Sub Example 2.1

Sub Example 2.2

Sub Example 2.3

Sub Example 2.4

Example 3

Sub Example 3.1

Sub Example 3.2

Sub Example 3.3

jQuery

var menu = $('li.open ul');
menu.css('display','none');
     $('li.open')
     .mouseenter(function(){
          menu.slideDown(400);
     })
     .mouseleave(function(){
          menu.slideUp(400);
     });

var menu2 = $('li.open2 ul');
menu2.css('display', 'none');
    $('li.open2')
    .mouseenter(function(){
         menu2.slideDown(400);
     })
     .mouseleave(function(){
         menu2.slideUp(400);
     });

Instead of selecting .open and .open2, you could change your selector to see if there is a child <ul>.

$('li:has(ul)').hover(function() {
  $(this).find('ul').slideDown(400);
}, function() {
  $(this).find('ul').slideUp(400);
})

Edit: you can also hide all the child <ul> nodes at once instead of individually.

$('li ul').hide();

Reference

Here are the three main features of jQuery that I used. Each link uses <api.jquery.com> which is a great place to learn about various jQuery functions.

:has() selector
.hover()
.find()


Function to combine list elements I was working on a little project that at the end of the script would write to a CSV file. I had to create a function that would combine multiple list elements of the same size into a single one. I would like to see if there is a more efficient way of doing this since I feel the way I did might not be the most efficient one. I created the function due to the need of capturing different number instances from a large CSV file. The reason on hwy I'm using "-" is because it was easier for me to count list elements.

def clean(li):
    tl = len(li)#2
    amount = len(li[0].split(","))
    if tl == 1:
        return ",".join(li)
    else:
        hold = "-," * amount
        hold = hold[:-1]
        d = hold.split(",")
        for i in li:
            con = i.split(",")
            for z, x in enumerate(d):
                if d[z] == con[z]:
                    pass
                elif d[z] > con[z]:
                    d[z] = d[z]
                else:
                    d[z] = con[z]
        return ",".join(d)

a = ['-,-,17,-,-,-,', '-,-,-,17,-,17,']
b = ['-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,5,-,-,-,-,-,-,-,-']
c = ['19,-,-,-,-,-,','-,-,19,-,-,19,','-,-,-,-,19,-,']
ss = clean(a)
se = clean(b)
sq = clean(c)

Output:

-,-,17,17,-,17,-
-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,5,-,-,-,-,-,-,-,-
19,-,19,-,19,19,-

It is quite hard to understand what your code is supposed to do. Let's try to progress little by little.

I've based my analysis on the test cases you have provided.

if tl == 1:

A list with a single element seems to be handled in a special way. If we replace the check with if tl == 1 and False:, we can see that the result is the same if we don't handle this case in a particular way.

I've removed the corresponding check.

hold

You are performing some super tricky logic involving string manipulation, list slicing and split. I realised that the whole thing does the same thing as d = ['-'] * amount.

Comparison of d[z] and con[z]

You are handling the three different cases here. Funny thing is that you have a case where you do nothing (pass) and one where you do something which looks like nothing to me (d[z] = d[z]). At the end of the day, I have the feeling that this could be written : d[z] = max(x, con[z]).

Hard coded strings

You have ',' in multiple places. If you ever want to change this, this will be pretty akward. You could store this in a variable on its own. Even better, you could make this a parameter with a default value. You have the benefit of making things easier for you as a developer of the function but also as a user of the function because it is easier to change its behavior if needed.

At this point, the code looks like:

def clean(li, sep=','):
    amount = len(li[0].split(sep))
    d = ['-'] *  amount
    for i in li:
        con = i.split(sep)
        for z, x in enumerate(d):
            d[z] = max(x, con[z])
    return sep.join(d)

Splitting string once

As you only care about the splitted version of the string, it would probably be interesting to split them at the very beginning so that you don't have to worry about this later on in the function.

splitted = [i.split(sep) for i in li]

Input handling

A few special cases could lead to troubles :

if len(li) == 0, your code will crash.
if the elements of splitted (which are lists of strings) have different lengths, your code will either crash (if a list is smaller that the first one) or ignore additional elements (if a list is bigger than the first one).

You might want to handle this properly.

Now, the code looks like :

def clean(li, sep=','):
    splitted = [i.split(sep) for i in li]
    if not splitted:
        return "invalid input" # or anything you like
    amount = len(splitted[0])
    if any(len(s) != amount for s in splitted):
        return "invalid input 2" # or anything you like
    d = ['-'] *  amount
    for s in splitted:
        for z, (dz, sz) in enumerate(zip(d, s)):
            d[z] = max(dz, sz)
    return sep.join(d)


C++: Am I managing memory the right way? Also, how would I go about putting my class in another file? Thanks!

#include <iostream>

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

class Array
{

int *arrayPointer;
public:

  Array(int size);
  ~Array();

  int get(int index);
  void set(int index, int value);

protected:
  int array[];
  int size;
};

Array::Array(int theSize){
  size = theSize;
  arrayPointer = new int[size];
}

Array::~Array(){
  delete[] arrayPointer;
}

int Array::get(int index){
  return *(arrayPointer + index);
}

void Array::set(int index, int value){
  if(index >= size || index < 0){
    cout << "Ooops, index out of bounds" << endl;
  }
  else{
    *(arrayPointer + index) = value;
  }
}

/* Let's see it in action now! */

int main(){
  int size = 3;
  Array array (size);
  array.set(0, 1);
  array.set(1, 2);
  array.set(2,3);
  cout << "Index 0 = " << array.get(0) << endl;
  cout << "Index 1 = " << array.get(1) << endl;
  cout << "Index 2 = " << array.get(2) << endl;
  array.set(4, 3);
  array.set(-1, 2);
}

First of all, you have no copy constructor (or assignment operator). This will be a problem in this case:

void foo(Array a) {
   //...
}

int main() {
   Array a(10);
   foo(a);
   //a is still around, but the pointer has been destroyed.
}

Since there will be two instances of Array using the same pointer. Once one of them goes out of scope, the destructor is called, so the delete[] happens. However, there is still another instance of Array with that same pointer, which has no way to know that delete happened. This is solved by following the "Rule of Three" See "Dynamically allocating an array of objects for a simple example.

In second place, you have a member called array. What's it for? It's never used so why have it?

In third place, some members are protected. Why? Does this mean you plan to have Array be inherited? In that case, you might want to make the destructor virtual. But I'd advise against inheritance, as it would feel counter-intuitive.

The implementation of get and set use unnecessary pointer arithmetic.
Just do:

return arrayPointer[index];

This does the same thing and it's much cleaner to look at.

In fourth place, you do a check on the values of index for set, but not for get. Why not?

In fifth place, index is a signed integer. You may want to consider using size_t instead. It will also make the < 0 check redundant.

In sixth place, usage of get and set feels unnatural. You may want to look at operator overloading.

In seventh place, I would not use cout for the error message. Use assert(), cerr or an exception, depending on what your exact needs are.

In eighth place, you call it Array but you only allow integers to be stored. I'd either name it IntArray or make it a template to allow other types to be stored.

As for "Putting your class in another file", that might be a good idea, especially if you want Array to be used in multiple files. Put it in a header file.

Also, why create your own Array class? Is this for learning purposes? If not, you may want to use std::array or std::vector instead.


Folder to Zip - code Maintainability From the code metrics in VS it says that the maintainability index is 64.
I think the code is really easy written.

How can I get a higher Maintainability Index?

public void ExportProjectFolderToZip()
{
    Log4NetLogger.Logger.Info("Start of Method ExportProjectFolderToZip");
    var startPath = new Uri(Path.GetDirectoryName(System.Reflection.Assembly.GetExecutingAssembly().GetName().CodeBase)).LocalPath; //get current directory (Release/Debug)
    var zipPath = new Uri(@"" + startPath + "-" + DateTime.Now.ToString("yyyy-MM-dd-hh-mm") + ".zip").LocalPath;  //zip filename
    try
    {
        CreateFromDirectory(startPath, zipPath, CompressionLevel.Fastest, true); //create zip folder
    }
    catch (Exception ex)
    {
        Log4NetLogger.Logger.Error("Error: " + ex.Message);
    }
    Log4NetLogger.Logger.Info("End of Method ExportProjectFolderToZip");
}

First a quote from Code Metrics Values on MSDN to make sure everyone knows what it is:

  Maintainability Index – Calculates an index value between 0 and 100 that represents the relative ease of maintaining the code. A high value means better maintainability. Color coded ratings can be used to quickly identify trouble spots in your code. A green rating is between 20 and 100 and indicates that the code has good maintainability. A yellow rating is between 10 and 19 and indicates that the code is moderately maintainable. A red rating is a rating between 0 and 9 and indicates low maintainability.

So is your code maintainable? Well, even though the score is not so bad I don't think it's. Why? Because it does two things that it shouldn't which are:

var startPath = new Uri(Path.GetDirectoryName(System.Reflection.Assembly.GetExecutingAssembly().GetName().CodeBase)).LocalPath; //get current directory (Release/Debug)
var zipPath = new Uri(@"" + startPath + "-" + DateTime.Now.ToString("yyyy-MM-dd-hh-mm") + ".zip").LocalPath;  //zip filename

You can see this by looking at another even more important score the

  Class Coupling – Measures the coupling to unique classes through parameters, local variables, return types, method calls, generic or template instantiations, base classes, interface implementations, fields defined on external types, and attribute decoration. Good software design dictates that types and methods should have high cohesion and low coupling. High coupling indicates a design that is difficult to reuse and maintain because of its many interdependencies on other types.

I measured your code and this score is 8. There is a way to improved it if you encapsulate the code creating the paths. Currently those two lines cannot be tested so you actually have to debug or run your application to be able to verify that both paths are the ones you expect.

If you put them inside another tool like a PathBuilder

class PathBuilder : IPathBuilder
{
    public string CreateStartPath()
    {
        return new Uri(Path.GetDirectoryName(System.Reflection.Assembly.GetExecutingAssembly().GetName().CodeBase)).LocalPath;
    }

    public string CreateZipPath(string startPath)
    {
        return new Uri(@"" + startPath + "-" + DateTime.Now.ToString("yyyy-MM-dd-hh-mm") + ".zip").LocalPath;
    }
}

the class-coupling score drops to 3 and 2 for each of the PathBuilder methods which is much better now:

public static void ExportProjectFolderToZip(PathBuilder pathBuilder)
{
    try
    {
        var startPath = pathBuilder.CreateStartPath();
        var zipPath = pathBuilder.CreateZipPath(startPath);

        CreateFromDirectory(startPath, zipPath, CompressionLevel.Fastest, true);
    }
    catch (Exception ex)
    {
        Logger.Error("Error: " + ex.Message);
    }
}

Btw this is not how you should use the logger for exceptions: Logger.Error("Error: " + ex.Message); There is an overload that accepts an exception so that you don't loose the stack-trace.


ConcurrentThrottledList - Will this implementation be thread safe? I want a thread safe list with a max item count, will the following implementation correctly provide this?

public class ConcurrentThrottledList<T>
{
    private readonly List<T>  items = new List<T>();
    private readonly int maxItems;
    private object _lock = new object();

    public ConcurrentThrottledList(int maxItems)
    {
        this.maxItems = maxItems;
    }

    public bool TryAdd(T item)
    {
        lock (_lock)
        {
            if (items.Count < maxItems)
            {
                items.Add(item);
                return true;
            }
            return false;
        }
    }

    public bool TryRemove(T item)
    {
        lock (_lock)
        {
            return items.Remove(item);
        }
    }

    public bool IsFull { get { lock (_lock) { return items.Count >= maxItems; } } }

    public T[] Items { get { lock(_lock) { return items.ToArray(); } } }
}

@ChrisW made valuable comments, I'll add mine:
Well done:

I like that private fields are all1 readonly, this makes intent explicit, which works towards greater maintainability.
I like that the class is highly cohesive and focused, this works towards greater readability, maintainability and extensibility.

Nitpicks:

I don't see why _lock gets an underscore and items and maxItems don't; being consistent with the underscore prefix for private fields would mootinate the this qualifier in the constructor.

Items could be called ToArray, making it more consistent with the framework's lingo, and more representative of its semantics.

TryAdd and TryRemove both look like they were named after the TryParse pattern; a developper looking at the API would then also be expecting an Add and a Remove method:

Add would add the specified item, throw an exception if it can't, and return void.
TryAdd would add the specified item, return true if successful and false otherwise.
Remove would remove the specified item, throw an exception if it can't, and return void.
TryRemove would remove the specified item, return true if successful and false otherwise.

However, List<T>.Remove() is already taking care of returning false if the specified item cannot be removed - throwing an exception in that case would be utter nonsense. Therefore, I'll second @ChrisW in saying that TryRemove should be simply called Remove.
I agree with TryAdd being called as such, because I would seriously expect an Add method to return void, so I'd consider also having an Add method that throws some CapacityExceededException (or whatever is more appropriate) when it can't add the specified item.

1 I would be led to believe _lock could also be made readonly, but I don't work with enough multithreaded code to know whether that would/could impact anything.


Product of 2 numbers equals sum of numbers between them Problem:

  Find all the pairs (a, b) whose product is equal to the sum of all numbers in the sequence [1..n] excluding both a and b.

I have made code for one program and need refactoring for code to get execution in less time. I am currently getting 13 sec for 500 number range.

def get_correct_array(n)
 beginning_time = Time.now
 puts " Begin with #{Time.now}"
 mul = 0
 arr = []
 last_arr = []

(1..n).each do |x|

 (x+1..n).each do |y|

    mul = x * y
    result = (1..n).reject {|n| n == x || n == y}.inject(&:+)
    if result == mul
        arr.concat([x,y])
        @x = x
        @y = y
    end
 end
end

last_arr << arr

if arr != []
 last_arr << (@x,@y = @y,@x)
end
if arr == []
 []
end

end_time = Time.now
puts " Ended with #{Time.now}"
puts "Time Elapsed -> #{(end_time - beginning_time)}"
p last_arr
end

Some notes:

Use 2 spaces for indentation.
mul = 0 No need to define variables in Ruby.
@x = x. @name is a instance variable, but there is no class here. Use only local variables.
(1..n).reject {|n| n == x || n == y}.inject(&:+). That's very slow, you should pre-compute the sum before, and just subtract the two elements in every loop.
last_arr << (@x,@y = @y,@x). Only the last pair is reversed?
Make functions/methods return something.
You can use the simple formula n * (n + 1) / 2 for the sum of (1..n).
Your code is very, very imperative. For mathematical problems (if not for all), favor a functional style (immutable data-structures)

I'd write:

def get_pairs(n)
  range_sum = (n * (n + 1)) / 2
  pairs = (1..n).to_a.combination(2)
  matching_pairs = pairs.select { |x, y| x * y == range_sum - x - y }
  matching_pairs.flat_map { |x, y| [[x, y], [y, x]] }
end

p get_pairs(26) #=> [[15, 21], [21, 15]]

With n = 500 it takes ~0.1s in my machine. If this is not still fast enough, you can still filter out pair values on the range that are too small to possibly match the sum range.


MinMax Problem Implementation Problem Statement:

  You will be given a list of integers, arr, and a single integer k.
  You must create an array of length k from elements of arr such that
  its unfairness is minimized. Call that array subarr.

  Unfairness of an array is calculated as: max(subarr) - min(subarr)

  Where:

  max denotes the largest integer in subarr
  min denotes the smallest integer in subarr

  Complete the maxMin function in the editor below. It must return  an integer that denotes the minimum possible value of unfairness.

Solution:

 static int maxMin(int k, int[] arr) {
    int arrLen = arr.length;
    Arrays.sort(arr);
    int[] subArr = Arrays.copyOfRange(arr, 0, k);
    int minUnfairness = subArr[k - 1] - subArr[0];

    for ( int j = 1; j < arrLen - k + 1; j++ ) {

        subArr = Arrays.copyOfRange(arr, j, j+k);

        int tempMinUnfairness = subArr[ k - 1 ] - subArr[0];

        if ( tempMinUnfairness < minUnfairness ) {
            minUnfairness = tempMinUnfairness;
        }
    }

    return  minUnfairness;
 }

  /**
   * Valid Test Case For Reference
   */
   private static void validTest() {
        int[] arr = { 10,100,300,200,1000,20,30 };
        int expected = 20;
        int result = maxMin(3, arr);
        assert (result == expected);
    }

The main function to review is maxMin().

Looking forward to your reviews on -

What other improvements can be done?
What other ways could be there    to the problem, as the current
approach is Greedy?

Thank you.

Hello and Welcome to Code Review!
There are two main concerns I have with your solution:

Side Effects

Your program has side effects outside of its scope: Arrays.sort(arr) When you call this function you are sorting the array object whose reference you have been passed. This means you are sorting the array that exists outside of your method scope, thus transforming any array you have been passed.

Since Arrays.sort() causes side effects, I'd recommend using Arrays.copyOf() to make a local copy, then sort the local copy.

Over-copying

You use Arrays.copyOf() inside your for loop, meaning you are frequently copying and making new arrays. These memory operations are expensive, and unnecessary.

You copy a range, but check only the end points of the range. Would it not make more since to simply check the values that would otherwise become those endpoints? Essentially, while looping, maintain two indeces, the head and tail pointers(i and i+k-1), and check those values rather than creating a new copy.

A good way to view this approach: We are essentially maintaining a abstract sliding range of k elements, and starting at 0, we slide our range along the array, checking each endpoint for a minimum without the need to create concrete copies.

With the above I reduced your solution to the below

static int maxMin(int k, int[] arr) {
    int arrLen = arr.length;
    //1)Make Sorted Local Copy to prevent Side Effects
    int[] localArr = Arrays.copyOf(arr, arrLen);
    Arrays.sort(localArr);

    int minUnfairness = Integer.MAX_VALUE;
    //2)Loop while maintaining two pointers
    // NOTE: we use J instead to prevent side effects on the int k
    int i = 0;
    int j = k - 1;
    while(j < arrLen){
        int tempMinUnfairness = localArr[j] - localArr[i];
        if ( tempMinUnfairness < minUnfairness ) {
            minUnfairness = tempMinUnfairness;
        }
        i++;
        j++;
    }
    return  minUnfairness;
}


Creating an object in Python with lots of long arguments See the proposed changes in this Pull Request under def add_talk.

date = self.talkDetailsWidget.dateEdit.date()
time = self.talkDetailsWidget.timeEdit.time()
presentation = Presentation(
    unicode(self.talkDetailsWidget.titleLineEdit.text()).strip(),
    unicode(self.talkDetailsWidget.presenterLineEdit.text()).strip(),
    unicode(self.talkDetailsWidget.descriptionTextEdit.toPlainText()).strip(),
    unicode(self.talkDetailsWidget.categoryLineEdit.text()).strip(),
    unicode(self.talkDetailsWidget.eventLineEdit.text()).strip(),
    unicode(self.talkDetailsWidget.roomLineEdit.text()).strip(),
    unicode(date.toString(Qt.ISODate)),
    unicode(time.toString(Qt.ISODate)))

There's a lot of boilerplate code (e.g. unicode(), seld.talkDetailsWidget, text(), strip(), etc.) How could you reduce that and still have the code be easy to understand?

My thinking is if something along the lines of this were possible:

map(str.strip,
    map(unicode,
        map(QLineEdit.text,
            map(self.talkDetailsWidget, fields))))

Your TalkDetailsWidget is underdeveloped, I think.  You could say that you have a view but no model, and that is causing you problems.

You want to be able to write

talk = self.talkDetailsWidget
presentation = Presentation(title=talk.title,
                          speaker=talk.presenter,   # ← Why the inconsistent vocabulary?
                      description=talk.description,
                         category=talk.category,
                            event=talk.event,
                             room=talk.room,
                             date=unicode(talk.date.toString(Qt.ISODate)),
                             time=unicode(talk.time.toString(Qt.ISOTime)))

Therefore, you'll need to implement new properties in TalkDetailsWidget.  To avoid copy-and-paste programming in TalkDetailsWidget, I suggest writing those getters using metaprogramming.

class TalkDetailsWidget(QWidget):
    …

    def _field_reader(field, method='text'):
        return property(fget=lambda self: unicode(getattr(getattr(self, field), method)()).strip(),
                        doc="Returns a Unicode string from the %s field with spaces stripped from each end" % (field))

    title       = _field_reader('titleLineEdit')
    presenter   = _field_reader('presenterLineEdit')
    description = _field_reader('descriptionTextEdit', method='toPlainText')
    category    = _field_reader('categoryLineEdit')
    event       = _field_reader('eventLineEdit')
    room        = _field_reader('roomLineEdit')

    @property
    def date(self):
        self.dateEdit.date()

    @property
    def time(self):
        self.timeEdit.time()


Generate the Norwegian equivalent to social security numbers This is just a little hobby project I've been working on. I've been using it to learn/improve my knowledge of C++.

The program generate Norwegian personal-numbers (11 digits) for males born a given year. It works just fine, but I'd like some input on how to improve the code. For now I have a basic class structure.

Norwegian personal-numbers is built up like this.

#include <iostream>

using namespace std;

const int DATE_LENGTH = 6;
const int BUFFER = 30;

class ID {
    private:
        int date[DATE_LENGTH];
        int i1, i2, i3;
        int k1, k2;
        int year;
    public:
        ID(int i1 = 0, int i2 = 0, int i3 = 0, int k1 = 0, int k2 = 0) {
            for (int i = 0; i < DATE_LENGTH; i++) { date[i] = 0; }
        }

        int *getDate() { return date; }

        void setYear(int year) {
            this->year = year;
        }

        int getYear() {
            return year;
        }

        void individSiffer() {
            static int individ = -1;
            if(year >= 1900 && year <= 1999 ){
                    individ++;
                    i1 = i2 = i3 = 0;
                }
                i3 = individ % 10;
                if(individ < 100){
                    i1 = 0;
                    i2 = individ / 10 % 10;
                }else if(individ < 10){
                    i1 = i2 = 0;
                }else{
                    i1 = individ / 100 % 10;
                    i2 = individ / 10 % 10;
                }
        }

        void generate() {
            k1 = 11 - ((3*date[0] + 7*date[1] + 6*date[2] + 1*date[3] + 8*date[4] + 9*date[5] + 4*i1 + 5*i2 + 2*i3) % 11);
            if(k1 == 11){
                k1 = 0;
            }
            k2 = 11 - ((5*date[0] + 4*date[1] + 3*date[2] + 2*date[3] + 7*date[4] + 6*date[5] + 5*i1 + 4*i2 + 3*i3 + 2*k1) % 11);
            if(k2 == 11){
                k2 = 0;
            }
            if((i3 != 0 && i3 != 2 && i3 !=4 && i3 !=6 && i3 !=8) && (k1 != 10 && k2 !=10)){
                for (int i = 0; i < DATE_LENGTH; i++) {
                    printf("%d", date[i]);
                }
                printf("%d%d%d%d%d\n", i1, i2, i3, k1, k2);
            }
        }
};

int main(int argc, char *argv[]) {
    char dateString[BUFFER];
    int i = 0;
    ID generator;

    printf("%s", "Your date of birth (DDMMYY): ");
    cin.getline(dateString, BUFFER);

    for(int i = 0; i < DATE_LENGTH; i++){
        *(generator.getDate() + i) = int(dateString[i]-'0');
    }
    generator.setYear(1900 + (*(generator.getDate() + 4) * 10) + *(generator.getDate() + 5));
    while(i < 500){
        generator.individSiffer();
        generator.generate();
        i++;
    }
    cout << "\n\n\n";
    return 0;
}

Comment on C++ interface:

You are doing work outside of ID that belongs as part of the class:

ID generator;

for(int i = 0; i < DATE_LENGTH; i++){
    *(generator.getDate() + i) = int(dateString[i]-'0');
}

generator.setYear(1900 + (*(generator.getDate() + 4) * 10) + *(generator.getDate() + 5));

while(i < 500){
    generator.individSiffer();
    generator.generate();
    i++;
}

Because this work is not part of a method you are exposing implementation details of your ID class via getter methods. All of which is bad:

A better interface would have been:

std::string date = getDate(); // Get a valid date or throw.
ID          generator(date);

std::cout << generator;

Comments on Code

Don't use this interface.
What happens if the user types an extra long line of stuff.

cin.getline(dateString, BUFFER);

Its not going to crash but you are left with lots of crap on the input stream. User input is line based so read a line from the user and then try and parse the line. Because a line can be any length don't use that version of get line.

std::string line;
while(std::getline(std::cin, line))
{
    // Valid line has been read.
    if (validDate(line))
    {    return line;
    }
    std::cerr << "Invalid date please try again\n";
}
throw std::runtime_error("The user give up trying to enter a date correctly");

Comments on C functions

C based functions for printing printf() are dangerous, because they are not type safe. Prefer to use the C++ type safe streams.

Comments on Data values

generator.setYear(1900 + (*(generator.getDate() + 4) * 10) + *(generator.getDate() + 5));

What about people born after 31st Dec 1999?

Comments on Loop structures

while(i < 500){
    // STUFF
    i++;
}

// This kind of loop is better written as a for:

for(int loop=0; loop < 500; ++loop) {
    // STUFF
}

Also notice that I am declaring my loop variable as close to the point of usage as possible. Do not use the C style of putting all the variables at the top of the function. Put your variables where you can see there declaration and type easily.


Age in days, months and years var AgeConvertor = {

    Age: function (formattedDate) {
      var now = new Date();
      var yearNow = now.getFullYear();
      var monthNow = now.getMonth() + 1;
      var dayNow = now.getDate();

      // Calculating in days
      var ONE_DAY = 1000 * 60 * 60 * 24;
      var ONE_MONTH = 1000 * 60 * 60 * 24 * 30;
      var date1_ms = new Date().getTime()
      var date2_ms = formattedDate.getTime()
      var difference_ms = Math.abs(date1_ms - date2_ms)
      var yearAge =  Math.round(difference_ms/ONE_DAY);

      if(yearAge < 30) {
            return yearAge =  Math.round(difference_ms/ONE_DAY) + 'D';
      // This condition checks if the number of days is more than 30
      } else if(yearAge > 30 && yearAge < 365){
            return yearAge =  Math.round(difference_ms/ONE_MONTH) + 'M';
      } else {

      // Calculating in years
      var today = new Date(yearNow,monthNow,dayNow);

      if (yearNow < 100) {
        yearNow=yearNow+1900;
      }

      yearAge = yearNow - formattedDate.getFullYear();

      if (monthNow <= formattedDate.getMonth()) {
        if (monthNow < formattedDate.getMonth()) {
          yearAge--;
        } else {
          if (dayNow < formattedDate.getDay()) {
            yearAge--;
          }
        }
      }
      return yearAge + 'Y';
     }
   }

}

How well can this code be improved?
Is there any fault in this code?
Does this code give desired results?

This code returns your age in days, months and years.

You should check if formattedDate is actually a Date object before using it (and change the variable name, since it sounds like it contains a formatted string).
It's strange, if not wrong, to consider months to be exactly 30 days.
You unnecessarily calculate Math.round(difference_ms/ONE_DAY) twice.
yearAge is a bad name for the variable at the beginning, since it doesn't contain anything concerning the year. And you reuse the variable later (then the name fits).
var today = new Date(yearNow,monthNow,dayNow); is wrong (and unnecessary), considering you just got monthNow from the current date and added 1.  Fortunately you don't actually use it.
Testing for the full year to be below 100 is unnecessary. Only getYear returns such values.
It probably would be better just to compare the day, month and year of the birthday and the current day, and do those calculation of the age in days and months only if needed.


Eliminate nested HTML elements for jQuery animation? Background

When a user leaves a page, if they haven't submitted form data, a button is animated to draw the user's attention. (This is a poor UX choice--all the form data should be submitted--but it is what I have to work with at the moment).

Only the text, which is centered on the button, must wiggle: the button itself must not shake.

Problem

The solution entails adding a button, a div, and a span. Without these elements, the shaking does not seem to work as desired.

Code

Button code:

<button type="submit" id="accept-list" style="width:100%">
&#x2714;
</button>

jQuery code:

$("#accept-list > div > span").effect( "shake", { distance: 5 } );

Fiddle

Question

What is a simpler way (i.e., that does not require the inner div and span elements) to shake button text, which works cross-browser?

I honestly don't know if this will work properly in all browsers (read: I doubt it), but you could conceivably animate the text-indent property to oscillate between +n and -n.

Here's a quick, quick hack based loosely on jQuery UI's own shake effect.

Again, though, I have no idea if this will work any better. It seems pretty neat, but I have a suspicion it won't be as robust as using a <button> with nested elements.


Testing lower- and upper bound values in Python Code functionality
The following code tests whether the values that a user specifies for a lower bound and upper bound on two properties:

The lower bound is smaller than the upper bound.
The values are larger than 0 (positive).

Since it first checks whether the lower bound is smaller than the upper bound, it implicitly verifies the upper bound is larger than 0, if it tests whether the lower bound is larger than 0. Therefore, my consideration is: I can make the code more compact by omitting the check for "is the upper bound larger than 0".
Code
# Object getting labels
class Get_labels:
    def __init__(self,lower_bound,upper_bound,configuration_name):
        self.lower_bound = lower_bound
        self.upper_bound = upper_bound
        self.configuration_name = configuration_name
        self.check_threshold_validity()

    # Verifies if the chosen thresholds are valid values.
    def check_threshold_validity(self):
        if self.lower_bound>=self.upper_bound:
            raise Exception(f'Sorry, the lower threshold={self.lower_bound} should be smaller than the upper bound={self.upper_bound} for configuration={self.configuration_name}')

        # checks if lower bound (and implicitly upper bound) are above zero
        if self.lower_bound<=0:
            raise Exception(f'Sorry, the lower threshold={self.lower_bound} should be larger than 0 for configuration={self.configuration_name}')

if __name__ == '__main__':
    get_labels = Get_labels(-1,25,"first")

Design choice
However, if the code is modified it might not be obvious the upper bound also needs to be checked because that is done implicitly. That might result in the edge case with upper bound below zero is not being caught after modifications. Hence to prevent this scenario, I can implement two unit test that check if an error is raised for:

The lower bound negative, upper bound negative
The lower bound zero, upper bound negative
The lower bound positive, upper bound negative

Question
Is it recommended to include the explicit check in the main code anyways, even though it is tested in the unit tests?

Except under unusual circumstances, classes are things or entities -- hence the
term objects -- while functions or methods are actions or operations. You
want to name them accordingly. For that reason, Get_labels strikes me as an
oddly named class. Based on what you've shown us, I might suggest the name
Bounds as an alternative. A side benefit of that name is that it allows you
to shorten up the attribute names without loss of meaning.
A separate method for checking the basic validity of the bounds seems like
over-engineering to me -- unless the checking logic becomes much more complex
or unless it will be used elsewhere in the code. So I would just do simple
validation in __init__() in this case.
Avoid the temptation to use chatty or verbose messages in your code. It won't
help you or your users over the long run -- at least that's my experience. Keep
things direct and rigorously concise. In fact, it's often beneficial to keep
the messages very technical rather than natural in their stylistic orientation.
What I mean by that is rather than describing the problem the way one might
verbally to a human ("The lower bound, which was 1000, must be less than the
upper bound, which as 125"), you are often better off to describe the problem
in a formulaic, schematic, computer-like way. Among other things, that approach
allows you to adopt a conventional format for all error messages in an
application. The error message format shown in the rewrite below could be
described generically as PROBLEM: SELF. A consistent approach makes it
easier to write validation code in the first place and to maintain it over
time. Consistency also conveys professionalism to users.
Along those lines, you can often simplify the creation of such validation
messages by first defining __repr__() for your class, as illustrated below.
For the validations you have so far, a ValueError is a closer
fit than raising a general Exception. Also, you might consider checking
for other kinds of errors: for example, are bounds restricted to integers only?
If you do check for that, raise a TypeError.
Finally, a stylistic and admittedly subjective fine point. Below is a stack
trace from your code as written. We see the verbose message twice, first as an
f-string, and then with parameters filled in. What's wrong with that? Nothing
serious, but it's heavy, tedious, even lacking a certain elegance. At a
minimum, one could say that the repetition of the verbose message is mildly
distracting to the user, putting an extra increment of a visual or cognitive
burden on the user to figure out what's going on. Compare that to the stack
trace from the revised code.
# ORIGINAL.

Traceback (most recent call last):
  File "bounds.py", line 19, in <module>
    get_labels = Get_labels(-1,25,"first")
  File "bounds.py", line 7, in __init__
    self.check_threshold_validity()
  File "bounds.py", line 17, in check_threshold_validity
    raise Exception(f'Sorry, the lower threshold={self.lower_bound} should be larger than 0 for configuration={self.configuration_name}')
Exception: Sorry, the lower threshold=-1 should be larger than 0 for configuration=first

# REVISED.

Traceback (most recent call last):
  File "bounds.py", line 72, in <module>
    b1 = Bounds(1000, 125, 'first')
  File "bounds.py", line 67, in __init__
    raise Exception(msg)
Exception: Upper bound must be greater than lower: Bounds(1000, 125, first)

Code with some possible edits for you to consider:
class Bounds:

    def __init__(self, lower, upper, name):
        self.lower = lower
        self.upper = upper
        self.name = name

        if lower <= 0 or upper <= 0:
            msg = f'Bounds must be positive: {self}'
            raise ValueError(msg)

        if upper <= lower:
            msg = f'Upper bound must be greater than lower: {self}'
            raise ValueError(msg)

    def __repr__(self):
        return f'Bounds({self.lower}, {self.upper}, {self.name!r})'