Easy to use code preparation script for CodeReview questions We all face questions that forget to include their code. Sometimes all of it, sometimes only parts, that make the rest of the question unfortunately incomplete and therefore off-topic.

What if it was possible to remedy that behavior with a simple script? What if said script fits in a single comment, at least in a less featured variant?

Goals

A POSIX compatible script that prints all files in the current directory in the following format:

## File: <path-to-file>

     source code line 1
     source code line 2
     source code line 3
     ...

Therefore, no bashism (like variable arrays), GNU extensions to find or similar.

The script must only depend on POSIX variants of sh, find and sed. Shellcheck must pass without errors or warnings.

By default, it should print only C/C++ related files and skip the .git and build folders that are often found in those projects. Both options should be configurable via environment variables.

The complete script

#!/bin/sh
FILE_HEADING="## File: %s\n\n"
IGNORED_DIRS="${IGNORED_DIRS:-build .git}"
EXTENSIONS="${EXTENSIONS:-c cc cpp h hpp}"

PRUNE_RULES=""
for dir in ${IGNORED_DIRS}; do
  PRUNE_RULES="${PRUNE_RULES} -name ${dir} -prune"
done

RULE_GLUE=""
if [ -n "${PRUNE_RULES}" ]; then
  RULE_GLUE="-o"
fi

NAME_RULES=""
for ext in ${EXTENSIONS}; do
  if [ -z "${NAME_RULES}" ]; then
    NAME_RULES="-name '*.${ext}'"
  else
    NAME_RULES="${NAME_RULES} -o -name '*.${ext}'"
  fi
done

if [ -n "${NAME_RULES}" ]; then
  NAME_RULES="\\( ${NAME_RULES} \\)"
fi

sh << EOF
    find . ${PRUNE_RULES} ${RULE_GLUE} ${NAME_RULES} \
           -exec sh -c "printf '${FILE_HEADING}' {} && sed 's/^/    /' {}" \\;
EOF

Usage:

cd repository
EXTENSIONS=rb IGNORED_DIRS="gem .git" path/to/script.sh

The second sh call is necessary as find doesn't work with multiple-options in a single argument. Every option for find needs to be in its own argument (e.g. find . '-name' '*' is fine, find . '-name *' is not).

Wait, I was promised a command that fits in a single comment!

For a comment, we can make some adjustements. First of all, we don't need to prune, as .git folders don't contain source code (not counting hooks). However multiple spaces in comments will get replaced by a single one, even in code, so you have to use a tabulator instead:

find . -name '*.hs' -exec sh -c 'printf "## File: %s\n\n" {} && sed "s/^/\t/" {}' \;

You should adjust the file type to the users question, of course. If you're sure that they have GNU find installed, it gets a little bit easier:

find . -o -name '*.rb' -printf "## File: %P\n\n" -exec  sed "s/^/\t/" {} \;

That being said, for some languages like Rust or JavaScript, you really should ignore their additional directories, or at least tell the user to check their post.

Those two variants are also eligible for review, although the latter doesn't follow the POSIX and the settings goals anymore

An alternative to defaulting variables by self-assigning with fallback is to use ${parameter:=word} at the point of first use:
for dir in ${IGNORED_DIRS:=build .git}; do

for ext in ${EXTENSIONS:=c cc cpp h hpp}; do

Actually, since those are the only uses, we could get away with :- expansion, even there.
Also, it makes sense to allow user to specify no ignored dirs and/or no extensions (and we check for that later in the code), so perhaps remove the : from those expansions.

RULE_GLUE can also be simplified:
RULE_GLUE=${PRUNE_RULES:+-o}

Again, it's not necessary to name this, but it might aid clarity.  Personally, I'd just add it directly to the end of the PRUNE_RULES:
PRUNE_RULES="${PRUNE_RULES} ${PRUNE_RULES:+-o}

or just ensure that each prune rule contains its own -o as it should:
for dir in ${IGNORED_DIRS}; do
  PRUNE_RULES="${PRUNE_RULES} -name ${dir} -prune -o"
done

The name rules can be simplified by starting with with the -false predicate (did you ever wonder what it's useful for?):
NAME_RULES=${EXTENSIONS:+-false}
for ext in ${EXTENSIONS}; do
    NAME_RULES="${NAME_RULES} -o -name '*.${ext}'"
done

It's unspecified whether find expands {} when it's a substring of an argument; the safer POSIX way is to pass it as a positional argument and have the child shell expand it:
    -exec sh -c "printf '${FILE_HEADING}' \"\$1\" && sed 's/^/    /' \$1" -- {} ';'

It's probably a good idea to only match plain files (including symlinks to plain files), rather than everything that matches the name rules.  Especially if $NAME_RULES is empty, it's quite possible for a directory name to match.
I don't see why we need the explicit sh subshell as the final command (since we expand the "rules" predicates unquoted, so word splitting happens - though we'll need to disable globbing: set -f).  There's no good reason not to exec the final command.
Also, we need to be careful if $FILE_HEADING ever contains ': it might be safer to also pass this as a positional parameter, or to export the variable and have it expanded later:
export FILE_HEADING

exec find . ${PRUNE_RULES} ${NAME_RULES} -type f
    -exec sh -c 'printf "${FILE_HEADING}" "$1" && sed "s/^/    /" "$1"' -- {} ';'

I think that rather than -exec sh, it might be simpler to have two -exec predicates:
... -exec printf "${FILE_HEADING}" {} \; -exec sed 's/^/    /' {} \;

(and we no longer need to export $FILE_HEADING).

Modified code
#!/bin/sh

# user-modifiable parameters
FILE_HEADING='## File: %s\n\n'
DEFAULT_IGNORED_DIRS='build .git'
DEFAULT_EXTENSIONS='c cc cpp h hpp'

# end of user variables; code starts here

set -eu  # Usual safety
set -f   # Don't expand pathnames when expanding $NAME_RULES below

for dir in ${IGNORED_DIRS-$DEFAULT_IGNORED_DIRS}; do
    PRUNE_RULES="${PRUNE_RULES-} -name ${dir} -prune -o"
done

for ext in ${EXTENSIONS-$DEFAULT_EXTENSIONS}; do
    NAME_RULES="${NAME_RULES:--false} -o -name *.${ext}"
done

# We intend word splitting of rules variables
# shellcheck disable=SC2086
exec find -L . ${PRUNE_RULES-} ${NAME_RULES:+'(' $NAME_RULES ')'} -type f \
     -exec printf "${FILE_HEADING}" {} \; \
     -exec sed 's/^/    /' {} \;

Modified one-liner
find . -name \*.sh -exec printf '## File: %s\n\n' {} \; -exec sed 's/^/    /' {} \;


Get list of new questions from Stack Overflow I would like to write a little pet project using Clojure to study it, something like Stack Overflow client.

Here is a my first code to get list of new questions from site. I am in the very beginning in Clojure so any help will be appreciated.

core.clj:

(ns stackify.core
  (:require [clj-http.client :as client]
            [clojure.data.json :as json]
            [stackify.utils :as utils]
            [stackify.configuration :as config]))

; TODO: move key to environment variables
(def ^:const stackKey "put-your-key-here")
(def ^:const baseUrl "https://api.stackexchange.com/2.2/questions?page=%d&pagesize=100&fromdate=%d&order=asc&sort=creation&site=%s&key=%s")

; Fetch questions
(defn fetch-json [url]
  "fetch internet page by url and return JSON"
  (let [data (client/get url)
        body (get data :body)]

    (json/read-str body)))

(defn fetch-questions [page from site accum]
  "recursive function to get questions"
  (let [url (format baseUrl page from site stackKey)
        json-content (fetch-json url)
        questions (get json-content "items")
        has_more (get json-content "has_more")]

    (println (format "Fetching page %d (has_more=%b)" page has_more))

    (cond
      (= has_more true) (fetch-question (+ page 1) from site (concat accum questions))
      :else (concat accum questions))
    ))

(defn get-questions []
  (let [from (config/read-time)
        now (utils/current-time)]

    (println from)
    (config/save-time now)
    (fetch-questions 1 from "stackoverflow" (vector))))

(defn print-questions [questions]
    (doseq [q questions] (println (get q "title"))))

utils.clj:

(ns stackify.utils
  (:require [clj-time.core :as time]
            [clj-time.coerce :as tc]))

(defn current-time []
  "returns unix time"
  (let [moment (tc/to-long (time/now))]
    (int (/ moment 1000))))

(defn parse-int [s]
  "convert string to int"
  (Integer. (re-find  #"\d+" s )))

configuration.clj:

(ns stackify.configuration
  (:require [stackify.utils :as utils]))

(def ^:const configFileName "last_sync_time")

(defn save-time [value]
  "save value to configuration file; file will be overwritten"
  (spit configFileName value))

(defn read-time []
  "read last sync time from configuration file"
  (let [value (slurp configFileName)]
    (utils/parse-int value)))

(defn init []
  "initiliza configuration file"
  (save-time (utils/current-time)))

This is nice code for someone just learning Clojure. You haven't made any of the common pitfalls like trying to use def to create local variables. I'm going to just jump around and mention things as I see them.

Your parse-int function is much slower (and arguably more manual) than it needs to be.

If you run lein check on your project, you'll see the following warning:

call to java.lang.Integer ctor can't be resolved.

It's also highlighted in yellow in IntelliJ for the same reason. It can't tell what overload of Integer. to use since it doesn't know with certainty what type re-find returns. This is forcing it to use reflection at runtime to do lookups, which can be very slow. Add a type hint to tell it exactly what re-find returns:

(defn parse-int [s]
  "convert string to int"
  (Integer. ^String (re-find  #"\d+" s)))

Here's some timings using Criterium, a great benchmarking library. Below, cc is an alias for criterium.core:

; Without type hints

(cc/bench
  (parse-int "1234567890"))

Evaluation count : 38179800 in 60 samples of 636330 calls.
             Execution time mean : 1.636166 µs  ; <-----------------------
    Execution time std-deviation : 226.156823 ns
   Execution time lower quantile : 1.459082 µs ( 2.5%)
   Execution time upper quantile : 2.309723 µs (97.5%)
                   Overhead used : 4.213229 ns

Found 5 outliers in 60 samples (8.3333 %)
    low-severe   5 (8.3333 %)
 Variance from outliers : 82.3868 % Variance is severely inflated by outliers

; With type hints

(cc/bench
  (parse-int "1234567890"))

Evaluation count : 187133520 in 60 samples of 3118892 calls.
             Execution time mean : 318.835981 ns  ; <-----------------------
    Execution time std-deviation : 3.399965 ns
   Execution time lower quantile : 314.452726 ns ( 2.5%)
   Execution time upper quantile : 326.997655 ns (97.5%)
                   Overhead used : 4.213229 ns

Found 4 outliers in 60 samples (6.6667 %)
    low-severe   2 (3.3333 %)
    low-mild     2 (3.3333 %)
 Variance from outliers : 1.6389 % Variance is slightly inflated by outliers

That's over 5x faster just from adding type-hints.

Here's a personal favorite of mine though that's much faster, and just falls back onto Java's parseInt implementation:

(defn my-parse-int [^String str-num]
  (try
    (Integer/parseInt str-num)  ; Fall back onto Java

    (catch NumberFormatException _
      nil)))  ; Return nil on a bad parse

(cc/bench
  (my-parse-int "1234567890"))
Evaluation count : 1531312920 in 60 samples of 25521882 calls.
             Execution time mean : 35.208889 ns  ; <-----------------------
    Execution time std-deviation : 1.018581 ns
   Execution time lower quantile : 34.554978 ns ( 2.5%)
   Execution time upper quantile : 38.078887 ns (97.5%)
                   Overhead used : 4.213229 ns

Found 7 outliers in 60 samples (11.6667 %)
    low-severe   4 (6.6667 %)
    low-mild     3 (5.0000 %)
 Variance from outliers : 15.8023 % Variance is moderately inflated by outliers

35 ns!

Note how I'm wrapping the parsing call in a try and returning nil in the event of an exception. In Clojure, it's common for nil to represent the absence of a valid value; like after a bad parse. It's similar to other language's notion of an Optional type. This concept allows you to do all sorts of nice things. Like, say you have a list of possible String numbers and you want to grab the first valid number:

(some my-parse-int ["bad" "also bad" "123" "another bad"])
=> 123

some ignores all the nils because they're falsey.

(cond
  (= has_more true) (fetch-question (+ page 1) from site (concat accum questions))
  :else (concat accum questions))
))

Isn't written ideally.

A cond with only one condition should just be an if unless you have a strong suspicion that you'll need to add more cases later
(= ... true) is redundant. if is already checking if the condition evaluates to a truthy value.
(+ page 1) can be simply (inc page)
Trailing closing braces like that aren't generally considered to be idiomatic style. Ideally you're using a good IDE that uses par(enthesis)-infer. That lets you write Clojure code using indentation to indicate nesting like it's Python. I never worry about braces when writing Clojure, and never manually type closing braces.

If you have a decent computer, I recommend the Community version of IntelliJ, paired with the Cursive plugin that allows for writing Clojure (both are free for personal use). I've been using this setup for three years now, and it's excellent (heavy, but excellent).

Anyways, I'd write that chunk just as

(if has-more
  (fetch-question (inc page) from site (concat accum questions))
  (concat accum questions))

That reads nicer anyways. You write (concat accum questions) twice though. I'd probably bind that to a name to reduce redundancy:

(let [...
      more-questions (concat accum questions)]
  ...

  (if has-more
    (fetch-question (inc page) from site more-questions)
    more-questions))

With

(defn print-questions [questions]
    (doseq [q questions] (println (get q "title"))))

I wouldn't shove everything onto one line like that. I would spread that out:

(defn print-questions [questions]
  (doseq [q questions]
    (println (get q "title"))))

I'm not sure if you know about Clojure's doc-strings, but you have yours in the wrong place. The doc-string comes before the argument list. defn actually expects a String to internalize to act as a doc-string (see the docs of defn). Just move it up:

(defn save-time
  "save value to configuration file; file will be overwritten"
  [value]
  (spit config-file-name value))

Now good IDE's can show that documentation when requested.

Also, most of your casing is correct, but remember Clojure using dash-casing like I changed it to, not camelCasing, or snake_casing like you're using elsewhere. Consistency is key for readability.


Decomposing a matrix as a sum of two bitstrings I chanced upon this question and came up with the following algorithm, looking for criticisms and advice on how to improve my algorithm efficiency.

  You are given 3 arguments:

  A & B are integers within the range (0 to 100,000)
  M is an array of integers within the range (0 to 2)

  Return an output (as a string) that describes matrix M in the
  following format:

  The first part of the string should contain a description of the upper row using only the 1 and 0 characters and should add up to
  the integer A
  The second part of the string should contain a description of the lowerrow using only the 1 and 0 characters and should add up to
  the integer B
  The sum of integers at K index of the output(string1, string2) should be equal to M[K] so like: string1[K] + string2[K] == M[K]

  So for example,

  Given A = 2, B = 2, M = [2, 0, 2, 0], your function should return a
  string like "1010, 1010"

  Given A = 3, B = 2, M = [2, 1, 1, 0, 1], your function should return
  "11001, 10100"

  Given A = 2, B = 3, M = [0, 0, 1, 1, 2], your function should return
  0, because no matrix can be constructed that satisfies such
  conditions.

def convert(A, B, M):
    if (A+B) != sum(M) or max(A, B) > len(M):
        return 0

'''
logic:

set 2 arrays
if value in matrix = 2, array1 & array2 at index where 2 occurs
will be [1] and [1]

if value in matrix = 0, array1 & array2 at index where 0 occurs will
be [0] and [0]

Then, we only need to handle 1s in matrix...
same logic as above,
however,
we handle position of 1 by checking whether A > B or vice versa
'''

    array1 = [""] * len(M)
    array2 = [""] * len(M)

    # first check for 2's and 0's:
    for index, value in enumerate(M):
        if value == 2:
            array1[index] = 1
            array2[index] = 1
            A -= 1
            B -= 1
        elif value == 0:
            array1[index] = 0
            array2[index] = 0

    # then check for 1's:
        elif value == 1 and A>B:
            array1[index] = 1
            array2[index] = 0
            A -= 1
        elif value == 1 and A<=B:
            array1[index] = 0
            array2[index] = 1
            B -= 1

    array1 = ''.join(str(x) for x in array1)
    array2 = ''.join(str(x) for x in array2)
    return array1 + ', ' + array2

First thing I notice is that you initialize your lists to contain strings

array1 = [""] * len(M)
array2 = [""] * len(M)

but later assign integer values to these. This is misleading, and so I would instead do

array1 = [0] * len(M)
array2 = [0] * len(M)

This has the added benefit that we get one case for free- if we want the value to be zeros (value == 0) we don't have to do anything.

Another issue I see with these variables is that they are named arrayX, but are in fact lists. In general, its best to leave out the type in the variable name, and just provide a descriptive name for its purpose. I would name these something like bitsA and bitsB, which also follows suite in A/B vs 1/2 naming.

Moving into your for loop, I would first reorder these conditionals to check value in numerical order (zero case first, then one, etc.), however as mentioned before we can now skip case zero, and so with only two cases (one or two) its not as big of a deal in my opinion. There is still one problem though. In one conditional you check A>B and then in the other you check the exact opposite. This added redundancy is unnecessary, and if you later edit one to be A<B, neither case will cover A==B. For this reason, it should be removed.

Normally I argue for reducing nested if statements, but in this case it seems like a logical separation, since the two subcases for value == 1 really mirror each other and deserve distinguishing from the value == 2 case. I would rewrite the loop as:

for index, value in enumerate(M):
    if value == 2:
        array1[index] = 1
        array2[index] = 1
        A -= 1
        B -= 1

    elif value == 1:
        if A > B:
            array1[index] = 1
            array2[index] = 0
            A -= 1
        else:
            array1[index] = 0
            array2[index] = 1
            B -= 1

However, there is one other route we could take, which would remove the remaining repeated logic.

for index, value in enumerate(M):
    if value == 2 or value == 1 and A > B:
        array1[index] = 1
        A -= 1
    if value == 2 or value == 1 and A <= B:
        array2[index] = 1
        B -= 1

Note this does break that rule I just mentioned about not including both A > B and A <= B, because our if statements are independent- both can execute. It becomes a trade off of bug resistance vs reducing redundancy.


How can I extract these similar codes into only one method? I have implemented three ways of Singleton idiom in Java, and now I want to write some test code to see there performance. Here is my code:

public static void main(String...args) {
    long startTime, endTime;
    int MAX_ITE = Integer.MAX_VALUE;

    // Eagerly initialized static instance guarantees thread safety.
    startTime = getCurrentTime();
    for(int i = 0; i < MAX_ITE; i++) {
        IvoryTower.getInstance();
    }
    endTime = getCurrentTime();
    System.out.println("Hungry initialization time:" + (endTime - startTime));

    // Double check locking, thread safe.
    startTime = getCurrentTime();
    for (int i = 0; i < MAX_ITE; i++) {
        ThreadSafeDoubleCheckLocking.getInstance();
    }
    endTime = getCurrentTime();
    System.out.println("Lazy initialization time:" + (endTime - startTime));

    // On demand holder
    startTime = getCurrentTime();
    for (int i =  0; i < MAX_ITE; i++) {
        InitializationOnDemandHolder.getInstance();
    }
    endTime = getCurrentTime();
    System.out.println("On demand holder time:" + (endTime - startTime));
}

These blocks of codes are very similar, but since getInstance() is static, I don't know how to extract each block into a same method call. Can anyone help?

Simple, with a bit of overhead, calling a run()

    testInstance("IT", () -> IvoryTower.getInstance());
    testInstance("TSDCL", () -> ThreadSafeDoubleCheckLocking.getInstance());
    testInstance("IODH", () -> InitializationOnDemandHolder.getInstance());
}

private void testInstance(String title, Runnable r) {
    long startTime = getCurrentTime();
    for (int i =  0; i < MAX_ITE; i++) {
        r.run();
    }
    long endTime = getCurrentTime();
    System.out.println(title + ": " + (endTime - startTime));
}

Possibly with testInstance as lambda too if you do not want a second method.

OR

Runnable[] rs = {
    () -> IvoryTower.getInstance(),
    () -> ThreadSafeDoubleCheckLocking.getInstance(),
    () -> InitializationOnDemandHolder.getInstance()
};
for (r : rs) {
    ...
    r.run();
    ...
}


Lookup map for converting Strings into Android `R.raw` integers I'm currently working on an Android app and I need to translate keys of type String to Integers which are determined at compile time of R.raw.
Right now I have found two ways of creating such a Map that holds all (needed) values of R.raw:

private static final Map<String, Integer> map = ImmutableMap.<String, Integer>builder()
                                                .put("...", R.raw.id)
                                                 ...
                                                .build();

and

private static final Map<String, Integer> map = new ArrayMap<>();
static {
    map.put("...", R.raw.id);
    ...
}

My questions are:

Are there better ways of creating a Map to lookup keys and return values that are determined at compile time? I don't want to use reflection.
and
Which one of those ways would you prefer?

There is not such thing as ArrayMap in standard Java. Therefore, I am just going to assume that you meant HashMap.

I would choose neither of those options. Depending on how many elements you need to insert into that Map, you could end up with a big chunk of code that is just a group of add calls.

I recommend creating two arrays: one of the Strings, and one of the Integers.

Then, all you have to do is create a loop that inserts all the values into the map.

Here is what I came up with:

String[] foos = {...};
Integer[] bars = {...};
Map<String, Integer> map = new HashMap<String, Integer>();

for(int i = 0, length = foo.length; i < length; i++) {
    map.put(foos[i], bars[i]);
}


Add customized code to Shopify theme I am new to making shopify applications. I have made one application which helps merchants add additional features in their stores.

I have written some PHP code that is given below.

$themeList = shopify_call($token,$shop,"/admin/api/2019-07/themes.json",array(),'GET');
$themeList = json_decode($themeList['response'], JSON_PRETTY_PRINT);
$theme_id = $themeList['themes'][0]['id'];

$customTheme = shopify_call($token,$shop,"/admin/api/2019-07/themes/".$theme_id."/assets.json?asset[key]=layout/theme.liquid&theme_id=".$theme_id,array(),'GET');
$customTheme = json_decode($customTheme['response'], JSON_PRETTY_PRINT);
$currentTheme = $customTheme['asset']['value'];
$pos = strpos($currentTheme,"</body>");
$str = "{% include 'back-to-the-top' %}\n";
$currentTheme =  substr_replace( $currentTheme, $str, $pos, 0 );

This code fetches the store theme files and append my customize code into theme file.

I want to know that what is right way to do it. Please check it and tell me that this is right way to do it or not? Please tell me the right way to do it.

I haven't used shopify so I don't know if you can accomplish your task with fewer shopify calls, but I do find it a little odd that the URL in the second call requires redundant theme identification.  I mean, the theme id is in the path and the querystring.

Outside of that, I spy some basic refinements.

As a general rule, avoid declaring single-use variables. Reasonable exceptions to this rule include when:

the merging of multiple processes/declarations results in a code line which exceeds a sensible length (horizontal scrolling is a drag and I generally obey PHPStorm's visual guide line when deciding when a line is too long) or
the data being processed/produced/declared is clarified by the variable name

Within a given project, determine if variables will be written in camelCase or snake_case.  Use one style consistently throughout.
There is no JSON_PRETTY_PRINT for json_decode() -- you are looking for the true parameter to make an array.  The outcome is the same, but it is a typo worth fixing to avoid future confusion.
Write a space between all comma-separated parameters in functions.

Application:

$themeList = shopify_call($token, $shop, "/admin/api/2019-07/themes.json", array(), 'GET');
$themeList = json_decode($themeList['response'], true);
$themeId = $themeList['themes'][0]['id'];
$themePath = "/admin/api/2019-07/themes/{$themeId}/assets.json?asset[key]=layout/theme.liquid&theme_id={$themeId}";

$customTheme = shopify_call($token, $shop, $themePath, array(), 'GET');
$customTheme = json_decode($customTheme['response'], true);
$currentTheme = $customTheme['asset']['value'];
$pos = strpos($currentTheme, "</body>");
$str = "{% include 'back-to-the-top' %}\n";
$currentTheme = substr_replace($currentTheme, $str, $pos, 0);

In conclusion, I reckon only the PRETTY_PRINTing, variable formatting, and the spacing should be changed.  The choice of how/when to declare data is mostly about personal taste.


Pass by Entity or EntityId in repositories? Which is the best practice in getting data from repository? passing just by object or by Id?

public IQueryable<Attendance> GetByPersonId(int id)
    {
        return DbSet.Where(ps => ps.PersonId == id);
    }

or

public IQueryable<Attendance> GetByPerson(Person person)
    {
        return DbSet.Where(ps => ps.Person == person);
    }

Are there any performance implication? Or is it a case to case basis? If so what are the cases that you might want to pass by object and not by Id?

If the return type is IQueryable<Attendance> then use a Person object because it implies that the method could use any of the passed Person object's properties to query other persons or other related objects.

If you want to find one person with Id, change the method's signature to tomething like this:

public Attendance GetByPersonById(int id)


Game Networking between 2 players I'm making a game and I want to establish a network connection between 2 players through a server(so the players are clients).

So far all I have is:

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.PrintWriter;
import java.net.ServerSocket;
import java.net.Socket;

public class Server extends Thread {

public ServerSocket s;
public Socket p1, p2;
public PrintWriter bis1, bis2;
public BufferedReader bos1, bos2;

public Server() {
    try {
        s = new ServerSocket(5_000, 1714);
    } catch (IOException e) {
        e.printStackTrace();
    }
}

public void run() {
    try {
        p1 = s.accept();
        p2 = s.accept();
    } catch (IOException e) {
        // TODO Auto-generated catch block
        e.printStackTrace();
    }

    try {
        bis1 = new PrintWriter(p1.getOutputStream());
        bis2 = new PrintWriter(p2.getOutputStream());
        bos1 = new BufferedReader(new InputStreamReader(p1.getInputStream()));
        bos2 = new BufferedReader(new InputStreamReader(p2.getInputStream()));
    } catch (IOException e) {
        e.printStackTrace();
    }

    while (p1.isClosed() || p2.isClosed()) { // if one of the players disconnect, the match will end.
        try {
            String p1 = bos1.readLine(); // what p1 says
            String p2 = bos2.readLine(); // what p2 says

            if (!p1.equalsIgnoreCase("")) { // if what p1 says is something
                if(p1.startsWith("my position x=")) {
                    p1 = p1.substring(15);
                    float x = Float.parseFloat(p1);
                    bis2.write("his position x=" + x);
                } else if(p1.startsWith("my position y=")) {
                    p1 = p1.substring(15);
                    float y = Float.parseFloat(p1);
                    bis2.write("his position y=" + y);
                }
            }

            if (!p2.equalsIgnoreCase("")) { // if what p1 says is something
                if(p2.startsWith("my position x=")) {
                    p2 = p2.substring(15);
                    float x = Float.parseFloat(p2);
                    bis1.write("his position x=" + x);
                } else if(p2.startsWith("my position y=")) {
                    p2 = p2.substring(15);
                    float y = Float.parseFloat(p2);
                    bis1.write("his position y=" + y);
                }
            }
        } catch (IOException e) {
            e.printStackTrace();
        }
    }
}

}

This is meant to send update of the players x and y to each other.

I'll implement other data of the game later but I want to know if I'm going to a good way.

Your vertical separation is good, you've got clear regions in your code for defining the objects you need like sockets, readers and writers, etc. There's one region that needs cleaning, though:

while (p1.isClosed() || p2.isClosed()) { // if one of the players disconnect, the match will end.
    try {
        String p1 = bos1.readLine(); // what p1 says
        String p2 = bos2.readLine(); // what p2 says

        if (!p1.equalsIgnoreCase("")) { // if what p1 says is something
            if(p1.startsWith("my position x=")) {
                p1 = p1.substring(15);
                float x = Float.parseFloat(p1);
                bis2.write("his position x=" + x);
            } else if(p1.startsWith("my position y=")) {
                p1 = p1.substring(15);
                float y = Float.parseFloat(p1);
                bis2.write("his position y=" + y);
            }
        }

        if (!p2.equalsIgnoreCase("")) { // if what p1 says is something
            if(p2.startsWith("my position x=")) {
                p2 = p2.substring(15);
                float x = Float.parseFloat(p2);
                bis1.write("his position x=" + x);
            } else if(p2.startsWith("my position y=")) {
                p2 = p2.substring(15);
                float y = Float.parseFloat(p2);
                bis1.write("his position y=" + y);
            }
        }
    } catch (IOException e) {
        e.printStackTrace();
    }
}

It's filled with duplication. If this game turns into a four-player game, you're gonna have a mess on your hands.

If you could wrap all the processing in a Player object, however, you could extract the relevant parts to functions, drastically simplifying the main loop:

while (!gameEnded(players)) { // check victory conditions or disconnects
    try {
        for(Player p : players){
            handleTickForPlayer(p);//How to handle update x y?
        }
    } catch (IOException e) {
        e.printStackTrace();
    }
}

Okay, that looks rather drastic, but that's because I'm leaving some parts out.

You've combined reading and writing in your own code, and its not very future proof. If you wanted to log matches, you'd have to insert it something in between, and from the way you handle messages, you'd have to do it for every case.

What I think you should do is give your messages some sort of identifier to state the meaning of the message, like id = 0 means it's an update of position x, id = 1 is used for an update for position y, maybe id = 2 is a chat message... you could put the type in the message by doing something like so: "id;message body", so "0;my position x=10.5". Or maybe you'd be better off with well known formats like JSON, but that's a separate point.

Via using these ids, you can then say "well, this is an 'update x' message", and send it off to the "update x" message handler.

I'd propose something like this:

        for(Player p : players){
            Message message = readMessageFromPlayer(p);
            if (message == null) { continue; }

            MessageHandler handler = getHandlerForMessage(message);
            if (handler == null) {
                //error?
                //maybe ignore bad messages
                continue;
            }
            handler.handleMessageFromPlayer(message, p, players);
        }

In readMessageFromPlayer, you check if there is any incoming content, and if there is, you split it up into the body of the message and the identifier. Otherwise return null...

In getHandlerForMessage, you check if you have any handler that can handle said message. There's multiple ways to do that; one way is to have a Map<MessageType, MessageHandler> of sorts (this means max 1 handler per message type), the other is a List<MessageHandler> where each MessageHandler has a boolean canHandle(Message message) (multiple possible handlers, but first come first serve), or it could be a gigantic switch that just makes you a new MessageHandler of the correct subclass (nasty, but could work?). I'd go for the Map because it allows you to instantly see "is there a handler for this messagetype".

Then, once you have the handler, you give it the message, the player who sent the message, and the rest of the players in the game.

(Actually, you might want to wrap the game in a Game object, if games are played ON the server. What you have right now is a relay server; the server doesn't keep track of state. That's okay, if all you want is a relay server, but this does stop you from doing things like server-side anti-cheat, such as checking if players are moving into walls.)

Inside the MessageHandler, you'd go about the thing you're doing now; first, parse the message to get the relevant content, then, send out an update.

You might want to change the messages to a more easily parsable format; if you'd go with the idea that the first number; in a message is the id, why not put the rest in the same shape? Something like 0;10.5; being "update x to 10.5", with the format being id=0;x=10.5;.

You can go really far with this, as you can see. You'll have to decide how far you want to take it, but I think that at the very least, you should come up with a way for handling players one at a time by wrapping them into objects. That way you centralize message handling and reduce reliance on the availability of several variables - directly writing to sockets is low level and you don't want to deal with it next to your game logic, because the two may get mixed up.


c++17 compatible std::bind_front alternative One of the things that has been really exciting me in c++20 is std::bind_front. Using placeholders with std::bind and boost::bind has really bothered me and the code looked messier and messier with each call to bind. It was bad enough for me to decide to enable -std=c++2a and pray that I wouldn't have to fix the code in the future if something gets changed.

I was messing around with parameter pack recursion when I realized I could make a std::bind_front alternative that would work in c++17. If I removed the std::invoke it would even work in c++11.

It even seems to compile faster than my standard library's implementation.

//#include <utility>
//#include <functional>

//of course this would go into some kind of namespace

template <class F, class A>
struct _bind_obj {
    F originalFunc;
    A arg;
    template <class... Args>
    inline auto operator()(Args&&... a){
        return std::invoke(originalFunc, arg, std::forward<Args>(a)...);
    }
    _bind_obj(F &&_originalFunc, A &&_arg) :
        originalFunc(std::forward<F>(_originalFunc)),
        arg(std::forward<A>(_arg)){

    }
};

template <class F, class A>
auto bind_front(F &&func, A &&arg){
    return _bind_obj<F, A>(
        std::forward<F>(func),
        std::forward<A>(arg)
    );
}

template <class F, class FirstA, class... A>
auto bind_front(F &&func, FirstA &&firstA, A&&... a){
    return bind_front(
        bind_front(
            std::forward<F>(func),
            std::forward<FirstA>(firstA)
        ),
        std::forward<A>(a)...
    );
}

So what do you think? Are there some cases in std::bind_front that I missed? Are there any optimizations I should make?

Are there some cases in std::bind_front that I missed?

There are several significant differences between std::bind_front's behavior and your implementations.

First, your implementation unconditionally returns a value. But what if the callable in question returned a reference? The behavior is just incorrect.

Second, std::bind_front is SFINAE-friendly but yours is not. That is, I can check to see if I can invoke the result with a certain set of arguments. Here is a silly example demonstrating this:

auto f = [](int i, int j) { return i + j; };
auto g = std::bind_front(f, 1);
auto g2 = your::bind_front(f, 1);

// this is ok, static assertion doesn't trigger
static_assert(!std::is_invocable_v<decltype(g), int, int>);

// this is a compile error
static_assert(!std::is_invocable_v<decltype(g2), int, int>);

Basically, asking the question "Can I call you with two ints?" leads to an instantiation failure outside of the immediate context, and will always be a hard error.

You can fix this and the reference issue described above by specified the correct return type instead of auto:

template <class... Args>
inline std::invoke_result_t<F&, A, Args...> operator()(Args&&... a) { ... }

Third, you're capturing your arguments differently. Consider:

std::string s = "Hello";
auto f = [](std::string const& s, int i) { return s[i]; }

auto g = std::bind_front(f, s);
auto g2 = your::bind_front(f, s);
s = "Goodbye";

assert(g(0) == 'H'); // ok
assert(g2(0) == 'H'); // fails

std::bind_front owns everything. You keep references to all the lvalues. The way the standard library works is if you want to capture by reference, you use std::ref.

Basically, this:

template <class F, class A>
auto bind_front(F &&func, A &&arg){
    return _bind_obj<F, A>(
        std::forward<F>(func),
        std::forward<A>(arg)
    );
}

should be:

template <class F, class A>
auto bind_front(F &&func, A &&arg){
    return _bind_obj<std::decay_t<F>, std::decay_t<A>>(
        std::forward<F>(func),
        std::forward<A>(arg)
    );
}

Fourth, your result is only invocable on a non-const object. But if the callable I'm bind_front()-ing has a const operator(), I should be able to invoke it as const too right?

The same can be said for ref-qualifiers: if I have a function object with &- and &&-qualified overloads, bind_front should respect that.

The solution is actually to write four overloads of operator():

template <class... Args>
inline std::invoke_result_t<F&, A, Args...> operator()(Args&&... a) &;

template <class... Args>
inline std::invoke_result_t<F const&, A, Args...> operator()(Args&&... a) const&;

template <class... Args>
inline std::invoke_result_t<F, A, Args...> operator()(Args&&... a) &&;

template <class... Args>
inline std::invoke_result_t<F const, A, Args...> operator()(Args&&... a) const &&;

Note that I'm also adjusting the type of F in the type trait, and be sure to move the callable in the rvalue cases.


Display of doctors by category, ajax Apologies in advance for my english...
We have a list of categories and a list of doctors, you need to output as follows:

category1

item1
item2
item3

category2

item1
item2
item3

Here is what I was able to write:
function renderChatList() {
    let xhttp = new XMLHttpRequest();
    let categoriesArr = [];

    xhttp.onreadystatechange = function () {
      if (this.readyState == 4 && this.status == 200) {
        categoriesArr = JSON.parse(this.responseText);

        categoriesArr.forEach((category) => {
          getDoctorsGroup(category.id, category.title);
        });
      }
    };
    xhttp.open(
      "POST",
      "http://localhost/domen/api/readCategories.php",
      true
    );
    xhttp.send();
  }
  renderChatList();

  function getDoctorsGroup(id, title) {
    let xhttp = new XMLHttpRequest();
    let doctorsArr = [];

    xhttp.onreadystatechange = function () {
      if (this.readyState == 4 && this.status == 200) {
        doctorsArr = JSON.parse(this.responseText);
        let html = "";

        html += '
';
        html += '
';
        html += `<span class="text">${title}</span>`;
        html += '';
        html += '
';
        html += '
';
        doctorsArr.forEach(doctor => {
          if(doctor.category == id) {
            html += '
';
            html += '
';
            html += `<img src="images/dist/doctors/${doctor.avatar}" alt="${doctor.name}">`;
            html += '
';
            html += '
';
            html += `<h3 class="member-name truncate-text-item">${doctor.name}</h3>`;
            html += `<p class="member-job truncate-text-item">${doctor.job}</p>`;
            html += '
';
            html += '
';
          }
        })
        html += '
';
        html += '
';

        // Insert group into list
        const container = document.querySelector(".chat-list_members");
        container.insertAdjacentHTML("beforeend", html);

      }
    }
    xhttp.open(
      "POST",
      "http://localhost/domen/api/readDoctors.php",
      true
    );
    xhttp.send();
  }

is it possible to optimize the code, or improve?

There are few improvements which I will recommend:

Use fetch API instead of XMLHttpRequest. Fetch is supported by almost every major browser and definitely serves as good replacement for XMLHttpRequest.
Utilize template literals properly. I can see template literals getting used properly at some places, but converting multiple strings into one template literal string will avoid cluttering. Also, instead of creating HTML as string, proper DOM elements can also be created using createElement but I am not sure which one is performant. Maybe someone else will give us some input here.
Utilize functional nature of JS to avoid cluttering. A lot of code can be broken even further into smaller logical pieces.
Use id in querySelector at line document.querySelector(".chat-list_members"). The reason being, you can have multiple HTML elements with same classname. This leads to a possibility of bug or wrong element getting selected. Id is a better choice in these cases.
Do error handling properly. The code seem to be handling cases when api is successful. A good code should also deal with cases when api fails or some error occurs.

Here is how, I will restructure the code:
function renderChatList() {
    fetch('http://localhost/domen/api/readCategories.php', { method: 'POST', })
      .then(response => response.json())
      .then(data => {
          data.forEach((category) => {
            getDoctorsGroup(category.id, category.title);
          });
      })
      .catch(error => {
        // Error handling here like alert/toast/popup etc
      });
}

renderChatList();

function getMemberHtml(doctorsArr, id){
    let html = ""
    doctorsArr.forEach(doctor => {
      if(doctor.category == id) {
        html += `

${doctor.name}

${doctor.job}

`;
      }
    })
    return html;
}

function getDoctorsGroup(id, title) {
    fetch('http://localhost/domen/api/readDoctors.php', { method: 'POST', })
      .then(response => response.json())
      .then(data => {
        let html = "";
        html += `

            ${title}

            ${getMemberHtml(data, id)}

`;

      // Insert group into list
      const container = document.querySelector("#chat-list_members");
      container.insertAdjacentHTML("beforeend", html);
    })
    .catch(error => {
      // Error handling here like alert/toast/popup etc
    });
}

Hope it helps. Revert for any doubts/clarifications.


Method that Saves user text messaging draft I am working on saving the text messaging draft of a user. The flow currently works but I am looking to make this block of code easier to read and smaller. Could anyone help me achieve this?

if (!hasDraft) {
    /**
     * This will get triggered when there is no draft
     * and you create one therefore we need a refresh.
     */
    if (!TextUtils.isEmpty(enteredText)) {
        saveDraftAndRefreshList();
    } else {
        /**
         * This should be the happy case.
         * User has just sent a message so no need to refresh the list
         */
        sendMessage();
    }
 /**
  * This else will only be reached if there has been a draft
  **/
 } else {
     if (TextUtils.isEmpty(enteredText)) {
         /**
          * This will get reached if you had a draft but now removed the text.
          * Thus needing a refresh
          */
         removePreviousDraft();
    } else if (!TextUtils.isEmpty(enteredText)) {
        /**
         * This will get triggered if there was a draft and
         * now they decided to overwrite it.
         */
        overwritePreviousDraft();
    }
}

In both cases (!hasDraft and not !hasDraft) you check whether the enteredText is empty. You can hoist that into a boolean variable outside the if statement.

Also, you are testing if (!hasDraft) but then also doing an else on that condition, which means else (if not)not hasDraft. That can be confusing, so unless some other part of your code flow requires it, I suggest you make your if statement be if (hasDraft) {} else {} so that the implied not from the else matches the !hasDraft case.

boolean noEnteredText = TextUtils.isEmpty(enteredText);

if (hasDraft) {
    if (noEnteredText) {
        removePreviousDraft();
    else {
        overwritePreviousDraft();
    }
} else {
    if (noEnteredText) {
        sendMessage();
    } else {
        saveDraftAndRefreshList();
    }
}

This does not read well, IMO. Again, though, it might be fine in the context of your other code. But I wonder if reversing the sense of noEnteredText to be hasEnteredText would make it better?

boolean hasEnteredText = !TextUtils.isEmpty(enteredText);

if (hasDraft) {
    if (hasEnteredText) {
        overwritePreviousDraft();
    else {
        removePreviousDraft();
    }
} else {
    if (hasEnteredText) {
        saveDraftAndRefreshList();
    } else {
        sendMessage();
    }
}

This still seems off. But I suspect that's probably because I don't know your app. Is it possible you're trying to do too much, and the two decisions should be at two different levels of method call?


Python code to get Accrued interest I've wrote this code and it does what is expected.
However, I'm thinking if there are better ways to write this, specially if is useful to wrap it in a class, or other ways to make it less loosen.
I'll be glad and grateful for any others corrections or suggestions.

interest_list = [0.5,  0.4,  0.3,  0.5,  0.7, 0.4,  -0.2,  -0.5,  0.3,
            0.7,  0.9,  1.0]

def get_unitary(interest_list):
    unit_list = []
    for i in range(len(interest_list)):
        unit_list.append(1+ interest_list[i]/100)
    return unit_list

# I've tested on some lists and found this faster than using reduce()
def prod(uni_list):
    p = 1
    for i in uni_list:
        p *= i
    return p

def get_accrued(prod):
    sub_1 = prod -1
    return sub_1 * 100

accrued = get_accrued(prod(get_unitary(interest_list)))

You can make it pretty short in Python 3 if you use functools.reduce():

from functools import reduce

interest_list = [0.5, 0.4, 0.3, 0.5, 0.7, 0.4, -0.2, -0.5, 0.3, 0.7, 0.9, 1.0]

l = [1 + n / 100 for n in interest_list]

accrued = (reduce(lambda x, y: x * y, l) - 1) * 100

In Python 2.7 no imports are needed.


A find method on a Product class The following class holds an array with products that are generated from a Json file. This is what the json file looks like

[
  {
    "id": "A101",
    "description": "Screwdriver",
    "category": "1",
    "price": "9.75"
  },
  {
    "id": "A102",
    "description": "Electric screwdriver",
    "category": "1",
    "price": "49.50"
  },
  {
    "id": "B101",
    "description": "Basic on-off switch",
    "category": "2",
    "price": "4.99"
  }
]

I'm using a find method on a Product class, but I'm in doubt if storing all generated products in the class itself is a good idea

Should I maybe create a ProductCollection class?
Should I build my products[] collection differently then using my method
Should I generate a Factory for the product creation from my Json file.

This is my code

    class Product {

        private static $products = [];

        public static function initialize($url)
        {
            // Build the product from our JSON file
            $data = json_decode(file_get_contents($url));
            foreach($data as $item) {
                $product = new Product();
                $product->id = $item->id;
                $product->description = $item->description;
                $product->category = $item->category;
                $product->price = $item->price;

              self::$products[] = $product;
            }
        }

        public static function find($id)
        {
            foreach(self::$products as $product) {
                if($product->id == $id) {
                    return $product;
                }
            }

            return false;
        }
    }

Thank you.

Use indexes on products

class Product {

    private static $products = [];

    public static function initialize($url)
    {
        // Build the product from our JSON file
        $data = json_decode(file_get_contents($url));
        foreach($data as $item) {
            $product = new Product();
            $product->id = $item->id;
            $product->description = $item->description;
            $product->category = $item->category;
            $product->price = $item->price;

          self::$products[$product->id] = $product;
        }
    }

    public static function find($id)
    {
        if(isset(self::$products[$id])) {
            return self::$products[$id];
        }

        return false;
    }
}


Find outlines of a tile collection On a game I'm working curently I keep on memory a simplified representation of the map which only have solid and non-solid cells. I need to get the outline of the shapes that adjacent cells might create, something like this:

I do not have a 8×8 array of bools on memory but a container of cell1 objects:

struct point { int x{}, y{}; };
struct line { point b{}, e{}; };
struct cell
{
    int x{}, y{};
    std::array<line, 4> lines() const
    {
        return
        {{
            {{x + 0, y + 0}, {x + 0, y + 1}},
            {{x + 0, y + 0}, {x + 1, y + 0}},
            {{x + 0, y + 1}, {x + 1, y + 1}},
            {{x + 1, y + 0}, {x + 1, y + 1}},
        }};
    };
};

Mi goal is to obtain a container with all the otulines, for example the outlines of the square at the top right corner are:

{
    // Vertical line (x6) that starts at position y0 and ends at position y2
    {{6, 0}, {6, 2}},
    // Horizontal line (y0) that starts at position x6 and ends at position x8
    {{6, 0}, {8, 0}},
    // …
    {{6, 2}, {8, 2}},
    // …
    {{8, 0}, {8, 2}}
}

The algorithm I'm using have the following steps:

Transform cells to lines.
Save lines into container, if the line is already on that container: delete it (this Will get rid of lines inside shapes).
Test each line against the others, if two lines are colinear and the end of one is the beggining of the other: merge it.

template <template <typename> typename container_t>
auto vertex_of(const container_t<cell> &cells)
{
    std::set<line> outlines;

    for (const auto &cell : cells)
        // Transform cells to lines
        for (const auto &line : cell.lines())
            // Save lines into temp container
            if (auto [i, b] = outlines.insert(line); !b)
                // If the line is already on the container: delete it
                outlines.erase(i);

    std::vector<line> result;
    // Test each line against the others, if two lines are colinear and the end of one
    // is the beggining of the other: merge it.
    for (const auto &outline : outlines)
    {
        if (std::find_if(result.begin(), result.end(), [&b = outline](line &a)
        {
            bool result{};
            if ((result = (a.e == b.b) && ((a.b.x * (b.b.y - b.e.y)) + (b.b.x * (b.e.y - a.b.y)) + (b.e.x * (a.b.y - b.b.y)) == 0)))
                a.e = b.e;
            return result;

        }) == result.end())
        {
            result.push_back(outline);
        }
    }

    return result;
}

Code Snippet available here, I have the feeling that it have lots of room for improvement.

1cell is more complex, but the additional complexity is out of the scope right now, so I'll keep things simple.

Your code makes remarkably good usage of modern C++ idioms. Bravo!

The only improvement that jumps out to me is the signature of vertex_of. By reading its definition, you can deduce that it returns a std::vector<line>, but ideally this should be documented in the declaration itself. I'd advice changing the signature to either

std::vector<line> vertex_of(const container_t<cell> &cells)

or

auto vertex_of(const container_t<cell> &cells) -> std::vector<line>

depending on the stlye you prefer.


Generating user ids on a random forum website Will this code be random enough to generate user ids for a website where people can create posts? Would there be a better method other than running this function and validating it doesn't exist in the database? I would prefer to have a method that I don't need to run a database check but the resulting UID should be 8 characters at most.

/**
 * Creates a UID
 * @return string 64 Character UID
 */
public static function createUid($length) {
    $bytes = random_bytes($length / 2);
    $uid = bin2hex($bytes);

    return $uid;
}

Personally, when I need something to be secure and consistently unique is to add a secret to an incrementing ID, which I assume your DB already has.

Your members have ID's and they are incremental, but then you can design a secret phrase or some sort of algorithm to append to that number, and then mcrypt it, or, depending on how secure you need your UID, encrypt in other ways and then concatenate it to your desired length.


Different neural network activation functions and gradient descent I've implemented a bunch of activation functions for neural networks, and I just want have validation that they work correctly mathematically. I implemented sigmoid, tanh, relu, arctan, step function, squash, and gaussian and I use their implicit derivative (in terms of the output) for backpropagation.

import numpy as np

def sigmoid(x, derivative=False):
    if (derivative == True):
        return x * (1 - x)
    return 1 / (1 + np.exp(-x))

def tanh(x, derivative=False):
    if (derivative == True):
        return (1 - (x ** 2))
    return np.tanh(x)

def relu(x, derivative=False):
    if (derivative == True):
        for i in range(0, len(x)):
            for k in range(len(x[i])):
                if x[i][k] > 0:
                    x[i][k] = 1
                else:
                    x[i][k] = 0
        return x
    for i in range(0, len(x)):
        for k in range(0, len(x[i])):
            if x[i][k] > 0:
                pass  # do nothing since it would be effectively replacing x with x
            else:
                x[i][k] = 0
    return x

def arctan(x, derivative=False):
    if (derivative == True):
        return (np.cos(x) ** 2)
    return np.arctan(x)

def step(x, derivative=False):
    if (derivative == True):
        for i in range(0, len(x)):
            for k in range(len(x[i])):
                if x[i][k] > 0:
                    x[i][k] = 0
        return x
    for i in range(0, len(x)):
        for k in range(0, len(x[i])):
            if x[i][k] > 0:
                x[i][k] = 1
            else:
                x[i][k] = 0
    return x

def squash(x, derivative=False):
    if (derivative == True):
        for i in range(0, len(x)):
            for k in range(0, len(x[i])):
                if x[i][k] > 0:
                    x[i][k] = (x[i][k]) / (1 + x[i][k])
                else:
                    x[i][k] = (x[i][k]) / (1 - x[i][k])
        return x
    for i in range(0, len(x)):
        for k in range(0, len(x[i])):
            x[i][k] = (x[i][k]) / (1 + abs(x[i][k]))
    return x

def gaussian(x, derivative=False):
    if (derivative == True):
        for i in range(0, len(x)):
            for k in range(0, len(x[i])):
                x[i][k] = -2* x[i][k] * np.exp(-x[i][k] ** 2)
    for i in range(0, len(x)):
        for k in range(0, len(x[i])):
            x[i][k] = np.exp(-x[i][k] ** 2)
    return x

The input x is the result of the output matrix of the previous layer multiplied by the weight matrix. The output x is the resultant of the activation function on the matrix to be used for the next layer.

Did I implement them in the way that it was intended to be used, or do some of these not give the intended results?

Here is the gradient descent algorithm I implemented:

import numpy as np
from Neural_Network import Activation_Function as af

def GD(layers, weights, proper, acti):
    deltas = []
    llayerError = layers[-1] - proper
    llayerDelta = llayerError * acti(layers[-1], True)
    deltas.append(llayerDelta)
    for i in range(-1, -len(layers), -1):
        layerError = deltas[abs((i + 1))].dot(weights[i].T)
        layerDelta = layerError * acti(layers[i-1], True)
        deltas.append(layerDelta)

    return deltas

Your code is correct. What can be improved is readability and speed.

First thing first, don't mix up the computation of the function and the computation of the derivative. I understand that passing True to the function to get the derivative can be convenient, but consider doing something less confusing.

def sigmoid(x):
    return 1 / (1 + np.exp(-x))

def sigmoid_derivative(x)
    return x * (1 - x)

You can always pass both versions of the function to your gradient descent function, or create Sigmoid class with forward/backward versions. This is what PyBrain and Torch do for example. Let's look at the PyBrain ReLU since it's done in Python:

from pybrain.structure.modules.neuronlayer import NeuronLayer

class ReluLayer(NeuronLayer):
    """ Layer of rectified linear units (relu). """

    def _forwardImplementation(self, inbuf, outbuf):
        outbuf[:] = inbuf * (inbuf > 0)

    def _backwardImplementation(self, outerr, inerr, outbuf, inbuf):
        inerr[:] = outerr * (inbuf > 0)

I think looking at this is a good way to review your code. First, what you did is not different, congratulations! The differences are mostly about optimizations, because speed is really important in neural networks. Let's look at the differences:

They did not conflate the two functions as I explained earlier
They set up their network to avoid allocations during training: they set outbuff using inbuf inplace: once they created their neural network, they don't need to create anything, just update weights to go as fast as possible. It probably wasn't your main concern here, and that's perfectly OK if you're doing this to learn.
They don't use loops and conditions, but stick to vectorized operations. Again, it's an optimization concern: vectorized operations get executed much faster. Maybe you did not think about the inbuf > 0 trick (which will create a matrix of zeros and ones), but it will make your code faster.
They group all computation: the equivalent of inerr[:] = outerr * (inbuf > 0) in your code is layerDelta = layerError * relu(layers[i-1], True). I don't think this is a crucial difference.


Dynamically instantiating classes I have the following class:

Class Core {

public $security;
public $db;
public $extra;

public function __construct() {
    spl_autoload_register(array($this, 'loader'));
}

public function loader($name) {
    include $name.'.php';
}

public function __get($key) {
    return $this->extra[$key];
}

public function init( $extra ) {
    #error_reporting(0);
    $this->security = new Security();
    #$this->db = new Database();
    $extra = explode(",", $extra);
    if(is_array($extra)) {
        foreach($extra as $key) {
            $key = trim($key);
            $this->extra[$key] = new $key;
        }
    }
}
}

Index.php
    include 'core/Core.php';

$start = microtime(true);

$core = new Core();
$core->init('User, Control');

$core->Control->test2();

echo "<br />";

$core->User->test();

The purpose of this was to make it so I could load/instantiate classes only when I need them, but use the Core class on all pages.  I will of course check if the file exists or not, but I wanted some feedback on whether or not this was viable.  Also, is there an easier way to do what I'm trying achieve?

There's a lot of design oddities in this, and I don't quite see the value of it.  The brevity it allows now will undoubtedly cause issues once you use this in a complex application.

Why is:

$core = new Core();
$core->init('User, Control');

$core->Control->test2();

echo "<br />";

$core->User->test();

Better than:

spl_autoload_register(...);

$control = new Control();
$control->test2();
$user = new User();
$user->test();

Autoloading is typically setup in some form of bootstrap.  It is usually done in the same place that DB connections are made, application-wide configuration is put into place, etc.

This mean that your main script content should never have to worry about autoloading since your bootstrapping will have already taken care of it.

(Note that a boostrap does not need to be a huge complex process.  Sometimes it can be as simple as include 'boostrap.php';)

Anyway, your class has a few design problems and a a few implementation problems:

Design:

What if you want more than 1 User object in the core?
What if you need to pass a parameter to a constructor
init() can make the state of the object unknown.

if you pass a Core instance to some other place of code, how does that code know what properties are valid on the object
This type of magic almost always ends badly since it hides state information

Implementation:

__get should check if the key exists in $extra before blindly accessing/returning
Unnecessary string processing: Why explode a string?  Just use an array.  It's cleaner and less error prone.
If you do keep this class (which I advise that you don't), I would consider making the init() part of the constructor.  I like to ensure that all of my instances are always in a stable, usable state.  What happens if you use a Core instance before calling init?
Setting up autoloading, storing objects and creating objects are three discrete responsibilities.  This means that the class is likely doing too much.

Once again, I recommend not using the class, but if you do:
Have the constructor take in an Autoloader instance and have that class handle loading
Have the class take in instances instead of names (this decouples creation from storing -- which is good since it means you have much more control of instantiation -- but it will kill the brevity of using your class)


Homework distribution terminal app in C++ I'm working on cleaning this code up a bit and was hoping for some suggestions. Right now it works well enough on a basic level, but it doesn't check for out of bounds numbers or improper input.

Also, one of my big concerns is too much nesting. Does the code look ugly? Any suggestions on how to clean it up?

#include <iostream>
#include <fstream>
#include <cmath>
using namespace std;

int numClasses();

int main()
{
    double finalResult[4], daysUntilDue, result, numOfProblems[199], numOfDays[99];
    string assignmentName[4];
    int numOfClasses, numItems[3], i, j;
    string classNames[3];

    numOfClasses = numClasses();

    ofstream myFile;
    myFile.open ("/Users/nicolasparsons/Desktop/HWAverages.txt");

    for (i = 0; i < numOfClasses; i++)
    {
        cout << "What is the name of class #" << i + 1 << "? (do not use spaces): ";
        cin >> classNames[i];
        cout << "How many assignments do you have for " << classNames[i] << "? (must be 5 or less): ";
        cin >> numItems[i];
        for (j = 0; j < numItems[i]; j++)
        {
            cout << "Enter a name for assignment #" << j + 1 << "? (do not use spaces): ";
            cin >> assignmentName[j];
            cout << "Enter number of problems for " << assignmentName[j] << " (must be 200 or less): ";
            cin >> numOfProblems[j];
            cout << "Number of days until this assignment is due (must be 100 or less): ";
            cin >> numOfDays[j];
            finalResult[i] = numOfProblems[j] / numOfDays[j];
            //cout << finalResult[i] << endl;
            myFile << classNames[i] << endl << assignmentName[j] << ": " << finalResult[i] << " per day until due date. " << endl << endl;
        }
    }

return 0;
}

int numClasses()
{
    int result;
    cout << "How many classes do you have? (must be 4 or less): ";
    cin >> result;
    return result;
}

Here are some things that may help you improve your code.

Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid.

Make sure you have all required #includes

The code uses std::string but doesn't #include <string>.  It's important to make sure that your code includes all required library headers both the reliably compile and to give the reader a clue as to what is within the program.

Eliminate unused variables

The variables daysUntilDue and result in your code are defined but never used.  Since unused variables are a sign of poor code quality, you should seek to eliminate them.  Your compiler is probably smart enough to warn you about such things if you know how to ask it to do so.

Use objects

Since you're writing in C++, what would make more sense to me would be to create a grades object. Then each operation would be very naturally expressed as an operator on the object.

Decompose your program into functions

Almost all of the logic here is in main in one rather long and dense chunk of code.  It would be better to decompose this into separate functions.

Eliminate magic numbers

The constants 3, 4, 99 and 199 are used in multiple places.  It would be better to have them as named const values so that it would be clear what those numbers represent.

Omit return 0

When a C or C++ program reaches the end of main the compiler will automatically generate code to return 0, so there is no need to put return 0; explicitly at the end of main.

Note: when I make this suggestion, it's almost invariably followed by one of two kinds of comments:  "I didn't know that." or "That's bad advice!"  My rationale is that it's safe and useful to rely on compiler behavior explicitly supported by the standard.  For C, since C99; see ISO/IEC 9899:1999 section 5.1.2.2.3:

  [...] a return from the initial call to the main function is equivalent to calling the exit function with the value returned by the main function as its argument; reaching the } that terminates the main function returns a value of 0.

For C++, since the first standard in 1998; see ISO/IEC 14882:1998 section 3.6.1:

  If control reaches the end of main without encountering a return statement, the effect is that of executing return 0;

All versions of both standards since then (C99 and C++98) have maintained the same idea.  We rely on automatically generated member functions in C++, and few people write explicit return; statements at the end of a void function.  Reasons against omitting seem to boil down to "it looks weird".  If, like me, you're curious about the rationale for the change to the C standard read this question.  Also note that in the early 1990s this was considered "sloppy practice" because it was undefined behavior (although widely supported) at the time.

So I advocate omitting it; others disagree (often vehemently!)  In any case, if you encounter code that omits it, you'll know that it's explicitly supported by the standard and you'll know what it means.


python Guess The Number This is my take on the guess the number game. The user has three attempts to guess the number chosen at random. The number is between 0 and 9, including. It tells the user to guess higher or lower and reveals the number if the game is lost.
Input that is not a number between 0 and 9, including, is not accepted and the cursor will remain on the same line.
Green is for win and red is for retry.
I have tested the code and it works correctly. Please have a go.
guess_the_number.py
"""The user has three attempts to guess what number was chosen at random"""

__all__ = []

import random
import string

from colorama import deinit, init

right_answer = random.choice(string.digits)

as_int = int(right_answer)

MSG_ANS = 'The answer was ' + right_answer
MSG_ASK = 'Guess the number (0-9) '
MSG_OK = '\033[92mCorrect!\033[0m'
MSG_TRY = '\033[91mTry again *\033[0m'

def ask(msg_wrong):
    """
    One attempt

    Non-digits are not accepted

    Digits accepted are removed from the pool
    and are not accepted in subsequent attempts
    """
    global pool

    cursor_up = '\033[A'

    cursor_up_ask = cursor_up + '                       '

    while True:
        answer = input(MSG_ASK)

        if answer and answer in pool:
            break

        answer_sz = len(answer)
        answer_sz = answer_sz * ' '

        line = cursor_up_ask + answer_sz + cursor_up

        print(line)

        print()
        print()

        print(line, end='\r')

    if answer == right_answer:
        print(MSG_OK)

        exit()

    if msg_wrong == MSG_TRY:
        i = int(answer)

        if i > as_int:
            hint = '>'
        else:
            hint = '<'

        msg_wrong = msg_wrong.replace('*', hint)

    print(msg_wrong)

    pool = pool.replace(answer, '')

if __name__ == '__main__':
    pool = string.digits

    init()

    ask(MSG_TRY)

    ask(MSG_TRY)

    ask(MSG_ANS)

    deinit()

Positives

Your function docstring was informative and helped me understand the code better.
Good use of variables like cursor_up to manage the ANSI control characters

Small improvements

As long as you're bringin in colorama, can you use any of the named constants in provides instead of the control characters?
If you need to do something multiple times, I like to see if there's a good way to do n times instead. See if you can make the number of turns more configurable.
I'm not sure what you're doing with line = cursor_up_ask + answer_sz + cursor_up. This would be a good place for a comment or a function with a clear name.
You're taking len(answer), but shouldn't that always be 1? My understanding is that you're expecting a one-digit answer. It would be nice to have some error handling so that, e.g. 23 isn't treated as a valid guess.
You could eliminate the pool global by wrapping it and your functions together in a class. You could also eliminate the global by having more focused functions.
Instead of doing a find-replace on the * in MSG_TRY, you can do something like this:

MSG_TRY = '\033[91mTry again {}\033[0m'
msg_wrong = msg_wrong.format(hint)

Larger gripes

I'm finding the flow of the program hard to follow. Consider breaking it down into smaller functions so it's more clear what's going on. It could look something like:

def play_one_turn(final_turn = False):
    guess = get_player_input()
    if guess == right_answer:
        print(OK_MSG)
        return True
    if not final_turn:
        show_hint(guess)
    else:
        print(MSG_ANS)

(I'm not saying this is the best way to do things. It's showing how smaller well-named functions can give more clarity with the same structure)

exit() in the middle of a function is suspicious. (Is the code missing from sys import exit?) I'd expect to see it at the end of a main function to return a status code back to the user. It should not be used for control flow. It would make more sense to return True or False depending on whether the guess is correct, and let the caller manage things from there.


Find all the factors of a given number The most basic code(i guess so) to find all the factors of a number

Note:factors include 1 and the number itself

Here's the code:

c=0
x=int(input("Enter number:"))
for i in range(1,x+1):
    if x%i==0:
        print("factor",c+1,":",i)
        c=c+1
print("Total number of factors:",c)

Please make this code efficient.....ie: Help to reduce the number of iterations

Just to have a more readable (than the answer by @Justin) and complete (than the answer by @Sedsarq) version of the algorithm presented in the other answers, here is a version that keeps the factors in a set and uses the fact that factors always come in pairs:

from math import sqrt

def get_factors(n):
    """Returns a sorted list of all unique factors of `n`."""
    factors = set()
    for i in range(1, int(sqrt(n)) + 1):
        if n % i == 0:
            factors.update([i, n // i])
    return sorted(factors)

Compared to your code this has the added advantage that it is encapsulated in a function, so you can call it repeatedly and give it a clear name and docstring describing what the function does.

It also follows Python's official style-guide, PEP8, which programmers are encouraged to follow.

With regards to which code is fastest, I'll let this graph speak for itself:

For the op function I used this code which has your checking of all factors up to x:

def op(x):
    factors = []
    for i in range(1,x+1):
        if x%i==0:
            factors.append(i)
    return factors

And the factors function is from the answer by @Justin.

If all you really want is the number of factors, the best way is probably to use the prime factor decomposition. For this you can use a list of primes together with the algorithm in the answer by @Josay:

from math import sqrt
from functools import reduce
from operators import mul

def prime_sieve(limit):
    prime = [True] * limit
    prime[0] = prime[1] = False

    for i, is_prime in enumerate(prime):
        if is_prime:
            yield i
            for n in range(i * i, limit, i):
                prime[n] = False

def prime_factors(n):
    primes = prime_sieve(int(sqrt(n) + 1))
    for p in primes:
        c = 0
        while n % p == 0:
            n //= p
            c += 1
        if c > 0:
            yield p, c
    if n > 1:
        yield n, 1

def prod(x):
    return reduce(mul, x)

def number_of_factors(n)
    return prod(c + 1 for _, c in prime_factors(n))

Comparing this with just taking the len of the output of the get_factors function and this function which implements your algorithm as op_count:

def len_get_factors(n):
    return len(get_factors(n))

def op_count(n):
    c = 0
    for i in range(1, n + 1):
        if n % i == 0:
            c = c + 1
    return c

The following timings result (note the increased range compared to the previous plot):


Am I using MVC Codeigniter correctly? I'm currently writing Report's system. I have about 20 different users with different access.
Am I doing it correctly? Can I improve my style of writing this code? All my reports based on the same structure.

In my Report's Controller:

function my_report_name()
    {
        //current week by default
        $dates['from'] = $this->uri->segment(3,date('Y-m-d',strtotime(date('o-\\WW'))-60*60*24*7));
        $dates['to'] = $this->uri->segment(4,date("Y-m-d", strtotime($dates['from']) + 6 * 24* 60 * 60));

        $data['report_name']=$this->Reports_model->getMyReportDescriptionSummary($dates);

        $data['title'] = "My Report Name Description";
        //view in 'view/report' folder to use
        //name of view always the same as name of function
        $data['main'] = "report/my_report_name";
        $data['menuReport'] = "class='current'";
        $data['panel'] = "panel/panel";

        $this->load->vars($data);
        //load template
        $this->load->view('template/t_main_full', $data);

    }

In my Model:

function getMyReportDescriptionSummary($dates)
    {
        $this->db->select("some_data,
                           some_data_2,
                           some_data_3"
                          );
        $this->db->join("table1");
        $this->db->join("table2");
        $this->db->where("some_params");
        $Q = $this->db->get();

        if($Q->num_rows() > 0)
        {
            return $Q->result_array();
        }
        else
        {
            return false;
        }
    }

In my repots layout page:

//function to check permissions
<? if($this->Model_Users->accessVerify('my_report_name')) {?>

My report

<?}?>

From a CodeIgniter point of view, this is fine but CI isn't really MVC. in MVC proper, the controller would be doing very little (See: https://stackoverflow.com/questions/13813046/how-is-mvc-supposed-to-work-in-codeigniter ). However in CodeIgniter's implementation you still want to strive for "Fat model, thin controller", which you're doing for the most part. This will give you far greater reusability as the logic isn't tied to a specific controller. I'd argue that most of the things in the $data array are actually stateful or display logic (e.g. which panel to display) so don't belong in the controller.

MVC strives for a high separation of concerns with reusable components and although CodeIgniter goes against this philosophy slightly, you can still work around it. Keep your views and models reusable.


Python implementation of SHA1 Here is a implementation of the cryptographic hash function SHA1 written in Python.  It does not use any external libraries, only built-in functions. I know that it would be faster to use an external library, but my code is for learning purposes.

I want to know if I am properly implementing the algorithm.  Am I taking any unnecessary steps? What changes can I make to speed up the code?

The code properly works.  I've compared the result to a trusted implementation.

def sha1(data):
    bytes = ""

    h0 = 0x67452301
    h1 = 0xEFCDAB89
    h2 = 0x98BADCFE
    h3 = 0x10325476
    h4 = 0xC3D2E1F0

    for n in range(len(data)):
        bytes+='{0:08b}'.format(ord(data[n]))
    bits = bytes+"1"
    pBits = bits
    #pad until length equals 448 mod 512
    while len(pBits)%512 != 448:
        pBits+="0"
    #append the original length
    pBits+='{0:064b}'.format(len(bits)-1)

    def chunks(l, n):
        return [l[i:i+n] for i in range(0, len(l), n)]

    def rol(n, b):
        return ((n << b) | (n >> (32 - b))) & 0xffffffff

    for c in chunks(pBits, 512):
        words = chunks(c, 32)
        w = [0]*80
        for n in range(0, 16):
            w[n] = int(words[n], 2)
        for i in range(16, 80):
            w[i] = rol((w[i-3] ^ w[i-8] ^ w[i-14] ^ w[i-16]), 1)

        a = h0
        b = h1
        c = h2
        d = h3
        e = h4

        #Main loop
        for i in range(0, 80):
            if 0 <= i <= 19:
                f = (b & c) | ((~b) & d)
                k = 0x5A827999
            elif 20 <= i <= 39:
                f = b ^ c ^ d
                k = 0x6ED9EBA1
            elif 40 <= i <= 59:
                f = (b & c) | (b & d) | (c & d)
                k = 0x8F1BBCDC
            elif 60 <= i <= 79:
                f = b ^ c ^ d
                k = 0xCA62C1D6

            temp = rol(a, 5) + f + e + k + w[i] & 0xffffffff
            e = d
            d = c
            c = rol(b, 30)
            b = a
            a = temp

        h0 = h0 + a & 0xffffffff
        h1 = h1 + b & 0xffffffff
        h2 = h2 + c & 0xffffffff
        h3 = h3 + d & 0xffffffff
        h4 = h4 + e & 0xffffffff

    return '%08x%08x%08x%08x%08x' % (h0, h1, h2, h3, h4)

print sha1("hello world")

I'm no expert on SHA1, and I see mostly small things, but one big thing that gave me pause. As an overall observation, you seem to value readability over memory usage. Since hashes are often used for large files, this seems to be a risky tradeoff, and python does provide good ways to handle this as well. More of this in the third observation below. On to the observations:

pBits being represented as a series of characters for each bit seems unusual for such low level work. I would typically expect to see this as a hexadecimal string, or even a number. Yet except for possible difficulties comparing it to other implementations that favor less space usage, this seems to make things easy to read.
Filling out pBits to lengths congruent to 448 mod 512 could be done in a single step by figuring out how many bits of padding are necessary and doing a single pBits += "0" * padding; there's no need for a while loop.
This one's important: chunks() should probably be a generator so that you don't have three copies of your data around (one original, one expanded a bit to a byte, and one in chunked strings); correspondingly this could serve out the trailing zero and length bits. Let's examine what this would look like, as 3 copies of large data is really large. Let's remove the sliced copies, creating only one chunk at a time:

def iterchunks(bits, chunkbits=512):
    for i in range(0, len(bits), chunkbits):
        yield bits[i:i + chunkbits]

Note that if the pBits generation was here, checking i + chunkbits against len(bits) and the bytes to bits conversion, you could process a file-like stream instead of a string, taking you further down, from the original 3 copies (with two of them 8 times larger than usual) to not even a full copy of your data in memory at one time.
Back to small ideas. Your bit string could be more literally a bit string, storing \0 and \1 characters. This would let you simplify setting up w into w = map(ord, words). (Of course with the appropriate helper, you could do that with the 0 and 1 characters of your current string.)
I'm not a big fan of the for i in range(0, 80): if 0 <= i <= 19: ... section. It feels like it might read better with a series of for loops that do their different thing then call a helper:

for i in range(0, 19):
    f = (b & c) | ((~b) & d)
    k = 0x5A827999
    a, b, c, d, e = update(a, b, c, d, e, f, k, w[i])
for i in range(20, 39):
    f = b ^ c ^ d
    k = 0x6ED9EBA1
    a, b, c, d, e = update(a, b, c, d, e, f, k, w[i])
...

but that seems pretty ridiculous, and would probably hurt performance to boot. After looking more closely at the obvious alternative, I'm going to withdraw this complaint.


Getting CSS files based on production or development I have the below script which returns CSS files based on production or development environment. The development section is fine; but for production I have to do 2 loops to get the files. Is there a cleaner/better way?

function get_css_files()
{
   $CI =& get_instance();
    $return = array();

    $css_files = array(
        array('path' =>'css/jquery.gritter.css', 'media' => 'all', 'group' => '1.css'),
        array('path' =>'css/jquery-ui.css', 'media' => 'all', 'group' => '1.css'),
        array('path' =>'css/bootstrap-select.css', 'media' => 'all', 'group' => '1.css'),
        array('path' =>'css/select2.css', 'media' => 'all', 'group' => '1.css'),
        array('path' =>'css/font-awesome.min.css', 'media' => 'all', 'group' => '1.css'),
        array('path' =>'css/jquery.loadmask.css', 'media' => 'all', 'group' => '1.css'),
        array('path' =>'css/token-input-facebook.css', 'media' => 'all', 'group' => '1.css'),
        array('path' =>'css/KeyTips.min.css', 'media' => 'all', 'group' => '1.css'),
        array('path' =>'css/bootstrap-colorpicker.min.css', 'media' => 'all', 'group' => '2.css'),
        array('path' =>'css/jquery.ml-keyboard.css', 'media' => 'all', 'group' => '2.css'),
    );
    if(!defined("ENVIRONMENT") or ENVIRONMENT == 'development')
    {
        $return = $css_files;
    }
    else
    {
        $groups = array();

        foreach($css_files as $css)
        {
            $groups[$css['group']] = TRUE;
        }

        foreach(array_keys($groups) as $css_group_file)
        {
            $return[] = array('path' =>"css/$css_group_file", 'media' => 'all');
        }
    }

    if(is_rtl_lang())
    {
        $return[] = array('path' =>'css/rtl.css', 'media' => 'all');
    }
    return $return;
}

One quick one.

if(!defined("ENVIRONMENT") or ENVIRONMENT == 'development')

If ENVIRONMENT has only two defined values e.g. 'development' and 'production' then use:

if(ENVIRONMENT != 'production')

For the Group loop:

foreach(array_keys($groups) as $css_group_file)

array_keys is not needed:

 foreach($groups as $css_group_file => $val)

As far as optimization of two loops to one, it does not matter as these two loops would execute in less than a millisecond, I would guess about about 400 microseconds.

I would rather see ENVIRONMENT equal to 0 instead of 'development'
and 1 instead of 'production'

This is so if it is undefined it will equate to zero with intval().

if (intval(ENVIRONMENT)){}

The following is rather unconventional but it is simple and fast.

Run this only when CSS files needs editing:

$css_files = array(
    array('path' =>'css/jquery.gritter.css', 'media' => 'all', 'group' => '1.css'),
    array('path' =>'css/jquery-ui.css', 'media' => 'all', 'group' => '1.css'),
    array('path' =>'css/bootstrap-select.css', 'media' => 'all', 'group' => '1.css'),
    array('path' =>'css/select2.css', 'media' => 'all', 'group' => '1.css'),
    array('path' =>'css/font-awesome.min.css', 'media' => 'all', 'group' => '1.css'),
    array('path' =>'css/jquery.loadmask.css', 'media' => 'all', 'group' => '1.css'),
    array('path' =>'css/token-input-facebook.css', 'media' => 'all', 'group' => '1.css'),
    array('path' =>'css/KeyTips.min.css', 'media' => 'all', 'group' => '1.css'),
    array('path' =>'css/bootstrap-colorpicker.min.css', 'media' => 'all', 'group' => '2.css'),
    array('path' =>'css/jquery.ml-keyboard.css', 'media' => 'all', 'group' => '2.css')
);
if(!defined("ENVIRONMENT") or ENVIRONMENT == 'development'){
   $fp = '/home/user/css/p0.ser','w');
   fwrite($fp,serialize($css_files));
   fclose($fp);
   $css_files[] = array('path' =>'css/rtl.css', 'media' => 'all');
   $fp = '/home/user/css/p1.ser','w');
   fwrite($fp,serialize($css_files));
   fclose($fp);
   $groups = array();
   foreach($css_files as $css){
     $groups[$css['group']] = TRUE;
   }
   foreach(array_keys($groups) as $css_group_file){
        $return[] = array('path' =>"css/$css_group_file", 'media' => 'all');
   }
   $fp = '/home/user/css/d0.ser','w');
   fwrite($fp,serialize($return));
   fclose($fp);
   $return[] = array('path' =>'css/rtl.css', 'media' => 'all');
   $fp = '/home/user/css/d1.ser','w');
   fwrite($fp,serialize($return));
   fclose($fp);

New Production Function:

This is based on is_rtl_lang() returning a value of zero or one and

$CI =& get_instance();
$css_file = array();
$css_file ['production'][0]) = '/home/user/css/p0.ser';
$css_file ['production'][1]) = '/home/user/css/p1.ser';
$css_file ['development'][0]) = '/home/user/css/d0.ser';
$css_file ['development'][1]) ='/home/user/css/d1.ser' ;
return unserialize(file_get_contents($css_files[ENVIRONMENT][is_rtl_lang()]);

If these more efficient changes were made:

From: ENVIRONMENT = 'production';
To:   ENVIRONMENT = 1 ;

From: ENVIRONMENT = 'development';
To:   ENVIRONMENT = 0 ;

Then:

$css_file [1][0]) = '/home/user/css/p0.ser';  // production without RTL
$css_file [1][1]) = '/home/user/css/p1.ser';  // production with RTL
$css_file [0][0]) = '/home/user/css/d0.ser';  // development without RTL
$css_file [0][1]) ='/home/user/css/d1.ser' ;  // development with RTL

unserialize(file_get_contents($css_files[intval(ENVIRONMENT)][is_rtl_lang()]);

NOTE:  intval() was added to $css_files[ENVIRONMENT]   to become: $css_files[intval(ENVIRONMENT)]

Where intval(ENVIRONMENT) takes care of the undefined condition.

Better Yet:

Seeing you are using PHP to populate the CSS links, there is an even better method.

Because each CSS file requires a round trip HTTP Request/Response, why not reduce it to one.
This is especially beneficial on a mobile phone Browser.  To conserve battery a smartphone will drop the radio signal between requests.  It can take seconds for the phone to reestablish the radio connection.
This is why it is important that each file loaded by the Browser contain Connection: keep-alive in the Request Header.

Furthermore because the Browser will not begin rendering until all the CSS is loaded, it is advantageous to bring it in with a single HTTP Request.

$css = file_get_contents('/home/user/public_html/css/css/jquery.gritter.css') . "\n";
$css .= file_get_contents('/home/user/public_html/css/css/jquery-ui.css') . "\n";
$css .= file_get_contents('/home/user/public_html/css/css/bootstrap-select.css') . "\n";
$css .= file_get_contents('/home/user/public_html/css/css/select2.css') . "\n";
$css .= file_get_contents('/home/user/public_html/css/css/font-awesome.min.css') . "\n";
$css .= file_get_contents('/home/user/public_html/css/css/jquery.loadmask.css') . "\n";
$css .= file_get_contents('/home/user/public_html/css/css/token-input-facebook.css') . "\n";
$css .= file_get_contents('/home/user/public_html/css/css/KeyTips.min.css') . "\n";
$css .= file_get_contents('/home/user/public_html/css/css/bootstrap-colorpicker.min.css') . "\n";
$css .= file_get_contents('/home/user/public_html/css/css/jquery.ml-keyboard.css') . "\n";
$fp = '/home/user/public_html/css/styles.css';
fwrite($fp,$css);
fclose($fp);


Search table without repetitive for loops I have an astropy table with a column containing datetime.datetime instances. I want to isolate the rows of this table that correspond to a user specified year, month, day, or hour. The table I am working with has a little over 100,000 entries, but as a more manageable example you can consider the following table with only six entries:

data = [1, 2, 3, 4, 5, 6]
dates = [datetime.datetime(2010, 6, 25, 0, 15),
         datetime.datetime(2011, 5, 25, 0, 45),
         datetime.datetime(2011, 6, 25, 1, 15),
         datetime.datetime(2012, 7, 12, 3, 45),
         datetime.datetime(2013, 9, 15, 2, 15),
         datetime.datetime(2014, 2, 25, 2, 45)]

test_tab = Table(data=[dates, data], names=['date', 'data'])

To search this table I came up with the following function:

import numpy as np
def search(year=None, month=None, day=None, hour=None):

    indx = [True for i in test_tab['date']]
    if year:
        test_func = np.vectorize(lambda x: x.year == year)
        indx = np.logical_and(indx, test_func(test_tab['date']))

    if month:
        test_func = np.vectorize(lambda x: x.month == month)
        indx = np.logical_and(indx, test_func(test_tab['date']))

    if day:
        test_func = np.vectorize(lambda x: x.day == day)
        indx = np.logical_and(indx, test_func(test_tab['date']))

    if hour:
        test_func = np.vectorize(lambda x: x.hour == hour)
        indx = np.logical_and(indx, test_func(test_tab['date']))

    return test_tab[np.where(indx)[0]]

My problem is that this function executes 4 loops over the same table. Is it possible to accomplish the same thing more efficiently?

I am not proficient with NumPy and I am sure there is a better way, but, as a step to following the DRY principle and making the code a bit more scalable, you can dynamically determine and "and" all the filtering conditions:

import datetime

from astropy.table import Table
import numpy as np

def search(test_tab, **params):
    def vectorize_callable(item):
        return all(getattr(item, param_name) == param_value
                   for param_name, param_value in params.items()
                   if param_value is not None)

    return test_tab[np.where(np.vectorize(vectorize_callable)(test_tab['date']))[0]]

data = [1, 2, 3, 4, 5, 6]
dates = [datetime.datetime(2010, 6, 25, 0, 15),
         datetime.datetime(2011, 5, 25, 0, 45),
         datetime.datetime(2011, 6, 25, 1, 15),
         datetime.datetime(2012, 7, 12, 3, 45),
         datetime.datetime(2013, 9, 15, 2, 15),
         datetime.datetime(2014, 2, 25, 2, 45)]

test_tab = Table(data=[dates, data], names=['date', 'data'])
print(search(test_tab, year=2011, day=25))

Prints:

        date        data
------------------- ----
2011-05-25 00:45:00    2
2011-06-25 01:15:00    3

Note how all() in combination with getattr() are used to "join" the logical conditions applied to different attributes.


Insert and Remove Element in Deque using threads in C++ The following code works fine for inserting and removing an element from a deque using two threads.

I would appreciate any help on how to make it better, especially in terms of thread safety.

#include "stdafx.h"
#include <deque>

#include <condition_variable>
#include <iostream>
#include <thread>
#include <mutex>

#define SLEEP_TIME 1

std::deque <int> q;
std::mutex _mu;
std::condition_variable cond;

void function1()
{
    int count = 10;

    while (count > 0)
    {

        std::unique_lock <std::mutex> locker(_mu);
        q.push_front(count);
        locker.unlock();
        cond.notify_one();
        std::this_thread::sleep_for(std::chrono::seconds(SLEEP_TIME));

        count--;
    }

    return;
}

void function2()
{
    int data = 0;

    while (data != 1)
    {
        std::unique_lock<std::mutex> locker(_mu);
        cond.wait(locker);

        data = q.back();
        q.pop_back();
        locker.unlock(),

        std::cout << "T2 gets " << data << " from T1" << std::endl;
    }

    return;

}

int main() {

    std::thread t2(function1);
    std::thread t1(function2);

    t1.join();
    t2.join();

    std::cout << "Thread Function Executed Successfully" << std::endl;
}

Here are some things that may help you improve your code.

Isolate platform-specific code

If you must have stdafx.h, consider wrapping it so that the code is portable:

#ifdef WINDOWS
#include "stdafx.h"
#endif

In this code, with a single file, the use of stdafx.h does not gain anything on any platform, including Windows.

Don't use leading underscores in names

Anything with a leading underscore is a reserved name in C++ (and in C).  See this question for details.

Be careful with conditional variables

Right now, t2 waits for a conditional variable and t1 sets it.  If t1 never pushes anything to the deque, t2 would wait forever and never terminate.  Similarly, if t1 is launched before t2, it could execute the cond.notify_one() before any threads are waiting for that variable, so t1 would push 10 and notify, but no thread would "hear" the notification with the effect that t2 would only be notified after t1 pushes 9.  The effect is that the code would almost work but never terminate.  To allow the launching of threads in either order, I'd recommend that t2 should keep popping until the deque is empty.  This is a typical pattern in multithreaded code.

Remember that main is a thread, too

I realize that this code is probably written for practice using threads and locks and such, but it's important to remember that main is also a thread, so this program actually uses three threads and not two.  Because the main function isn't really doing anything useful while the threads are running, an alternative approach would be to launch only t1 as a thread and then simply invoke function2 directly from main, thereby using only two threads instead of three and getting the same work accomplished.

Remember that std::cout is a shared resource

It's not an issue in this code because only one thread at a time prints to std::cout, but as you create more complex code, it's important to remember that std::cout is a shared resource, too,  For that reason, you'll want to have a mutex for access to it as well if you want to avoid possible interleaving of output.