Removing entries with duplicate columns from large .vcf files First of all, please note that the input (.vcf) file is very large (>60 GB).

So I am reading the .vcf files line by line, but since I have to check for duplicate of two columns present in the input vcf file, I extract those two columns from the input file to a text file ("dup_posID.txt") and then check for duplicates, and remove the duplicates and save it to a new text file ("dup_posID_re.txt").
Then I create a new vcf file (.redp.vcf) by comparing the recoded text file.

The code works fine and am getting the desired result. But its taking a lot of time. more that 24 hours. How can I reduce that time?

vcf_File_name = "chr7_dup_fin"

with  open(vcf_File_name+".vcf","r") as f, open(vcf_File_name+"dup_posID.txt", "w+") as D:

    for v_line in  f:

        if v_line[0] != '#':
            v_cols = re.split('[ ]+|[   ]+',v_line.strip())
            r= v_cols[2].strip() +"\t"+ v_cols[1].strip() + "\n"
            req1 += r
            #print(req1)
    D.write(req1)

D1=open(vcf_File_name+"dup_posID.txt","r")

D2 = open (vcf_File_name+"dup_posID_re.txt", "w+")

lines1 = D1.readlines()

for line1 in lines1:
    for line1 in lines1:
        cols= re.split('[   ]', line1.strip())
        try:
            dup[cols[0]] += 1
            #print cols[0]
        except:

            try:
                dup_pos[cols[1]] += 1
                #print "Duplicate Position: "+cols[1]
            except:
                dup_pos[cols[1]] = 0
                dup[cols[0]] = 0

                p = cols[0].strip()+"\t"+cols[1].strip()+"\n"
                q += p
D2.write(q)

D2.close()

with  open(vcf_File_name+".vcf","r") as f, open(vcf_File_name+".redp.vcf","w+") as f1 :
    D2=open(vcf_File_name+"dup_posID_re.txt", "r")
    lines2=D2.readlines()
    for v_line in  f:

        if v_line[0] != '#':
            v_cols = re.split('[ ]+|[   ]+',v_line.strip())

            st = v_cols[2]+"\t"+v_cols[1]+"\n"
            if st in lines2:
                lines2.remove(st)
                s = "\t".join(v_cols)
                s += "\n"

                #req += s
                f1.write(s)
                #f1.write("\n")
        else:
            s = v_line
            f1.write(s)
    f1.close()

The input .vcf file has 2512 columns and is tab separated.
Example:

2   26813   rs574377045  C  A   100 PASS    AC=2;AF=0.000399361;AN=5008;NS=2504;DP=18198;EAS_AF=0.001;AMR_AF=0.0014;AFR_AF=0;EUR_AF=0;SAS_AF=0;VT=SNP;AA=C GT   0|0
2   26816   rs535194004  C  T   100 PASS   AC=1;AF=0.000199681;AN=5008;NS=2504;DP=18346;EAS_AF=0;AMR_AF=0;AFR_AF=0.0008;EUR_AF=0;SAS_AF=0;VT=SNP;AA=C      GT   0|0

Here, one line extends till 2512 columns.

Some notes:

Run it through pycodestyle to learn how your style differs (at least in machine detectable ways) from idiomatic Python. This makes code not only easier to review, but also easier for seasoned Python developers to read and maintain. I would recommend not silencing any of the complaints, and instead fixing every one of them. It quickly becomes second nature to write idiomatic Python that way.
Abbr. var nams hindr rdability, makin it hardr to rd. Unless D1 is absolutely clear from context to anyone in your field I would rename it until it is. The same of course goes for f, which could be semi-tolerable if your script only ever processed one file, but is baffling if you're processing several. "More typing" is not an argument against this, because there are editors which will happily auto-complete any identifier for you.
You have a nested loop where both loops use the same instance variable. That means that the outer loop variable is useless.
You have huge files you want to process, so you really, really don't want to read whole files before starting processing. The way to avoid that is to process each line as you read it, as in for line in file_pointer rather than lines = file.readlines() + a loop.
The main parts of this should be split into functions, called by a short main function which is triggered only if __name__ == '__main__': - this rather unfortunate pattern is idiomatic Python, and allows reuse of the functions and objects in your module.
You have completely open ended except clauses, which is almost universally a bad idea. Exception handling is expensive, and you'll be much better off checking for the existence of an index than throwing an exception and catching it again immediately. In general, the anti-pattern is to use exceptions for flow control.
There are several patterns in use which may be fine for most processing, but which are not optimal for large amounts of data, such as splitting and joining of strings, regular expressions, and accumulating strings and then writing once rather than just writing every substring. From a cursory look it seems that your processing would be very much quicker if done with a database, if at all possible.
You are taking care to close some files by using with open(), but the second D2 is not closed.
Reusing variables for different things also makes code really hard to read, and it a common misconception that it is more efficient. Maybe in some niche corner cases, but for this kind of processing you will be much better off with evidence-based optimisation - prove where the code is slow with a profiler and fix those parts.


Making JSON requests to fetch relationships asynchronously I would like to refactor the fetchRelationships function to use async await. I am not sure what the best way is to do it as this code contains nested .then at response.json().then((json) =>....

Could sby pls post the refactored version?

export const fetchRelationshipsError = (error) => {
  return {
    type: FETCH_RELATIONSHIPS_FAILURE,
    payload: { error }
  };
};

export const fetchRelationshipsRequest = () => {
  return { type: FETCH_RELATIONSHIPS_REQUEST };
};

export const fetchRelationshipsSuccess = (relationships) => {
  return {
    type: FETCH_RELATIONSHIPS_SUCCESS,
    payload: relationships
  };
};

export const fetchRelationships = (url) => {
  return (dispatch) => {
    dispatch(fetchRelationshipsRequest());

    fetch(url, config)
    .then((response) => {
      const responseObj = {
        response: response,
        payload: response.json().then((json) => {
          return { body: json }
        })
      };

      if (!response.ok) {
        const error = new Error(response.statusText);
        responseObj.payload.then((response) => {
          show_error_alert(response.body);
          dispatch(fetchRelationshipsError(response.body));
        });
        throw error;
      }

      return responseObj.payload;
    })
    .then((response) => {
      dispatch(fetchRelationshipsSuccess(response.body))
    })
    .catch((error) => { console.log('Request failed', error); });
  };
};

First, I would get that error check out of the way instead of in the middle of the operation. Just throw an error in the middle of the operation and let the catch handle the dispatch.

Also, fetch will always resolve. A common procedure is to use the very first then to evaluate the response and determine how the sequence proceeds. It's usually either resolving with the data or rejecting with the error. But in your case, since both success and error actions dispatch body, the only difference is the action creator. Instead of a resolution/rejection, we just resolve with the correct action based on ok.

export const fetchRelationships = url => {
  return async dispatch => {
    dispatch(fetchRelationshipsRequest())

    fetch(url, config)
      .then(res =>
        // We cannot return res.json() and do its then as part of the
        // outer sequence since we need res. This needs to nest.
        res.json().then(body =>
          res.ok ? fetchRelationshipsSuccess(body) : fetchRelationshipsError(body)))
      .then(action => dispatch(action))
      .catch((error) => dispatch(genericErrorAction())
  }
}

Now for async-await. The returned anonymous function from fetchRelationships will have to be async in order for its body to be able to use await. All promise-returning functions will need to use await. And then turn everything into synchronous-looking code. Here's the same procedure as above, but in async-await form.

export const fetchRelationships = url => {
  return dispatch => {
    dispatch(fetchRelationshipsRequest())

    try {
      const res = await fetch(url, config)
      const body = await res.json()
      const action = res.ok ? fetchRelationshipsSuccess(body) : fetchRelationshipsError(body)
      dispatch(action)
    } catch(error){
      dispatch(genericErrorAction())
    }
  }
}


Return Kth to Last: Implement an algorithm to find the kth to last element of a singly linked list Implement an algorithm to find the kth to last element of a singly linked list.

Any comments or feedback regarding this?

I came up with another algorithm that uses HashMap to find the kth to the last element in a LinkedList.

public static int getKthToLastElement(LinkedListNode head, int k){
    LinkedListNode n = head;

    if(head == null || k < 0)
        return -1;

    int index = 0;
    HashMap<Integer,Integer> map = new HashMap<Integer, Integer>();
    while (n != null){
        map.put(index,n.data);
        index++;
        n = n.next;
    }

    if(k > map.size() -1){
        return  -1;
    }
    return map.get(map.size() - 1 - k);
}

Your if(head == null || k < 0) special case is unnecessary, since it is covered by the if(k > map.size() -1) test.  You should also never omit the "optional" braces for a multi-line block.

Actually, the if(k > map.size() -1) test could also be eliminated, if you used Map.getOrDefault(…, -1).

The loop would be more easily recognized if written as a for loop.

public static int getKthToLastElement(LinkedListNode head, int k) {
    int index = 0;
    Map<> map = new HashMap<Integer, Integer>();
    for (LinkedListNode n = head; n != null; n = n.next) {
        map.put(index++, n.data);
    }
    return map.getOrDefault(map.size() - 1 - k, -1);
}

I think that converting the linked list to an ArrayList would be better than converting it to a HashMap.  You would then be converting a cumbersome linked list into a kind of list that is useful and idiomatic in Java.


Get all data from an external and paginated Swagger API using Java / Feign / Spring I have written the following method that allows me to import from another API all the items stored there using Feign.
The only issue is that the external API provides a size limit of 2000 and therefore I need to take pagination into account to get all the records.
I think that my solution is not very clean and maintainable and I'd like to use streams and newer Java features to make it shorter (like I did in the final part) but I am still learning and I cannot think of good solutions for the main part of the code.
@Service
public class ClientService {

    @Autowired
    private Client Client;

    @Autowired
    private ItemRepository itemRepository;

    public List<Code> importItems() {

        int itemsPerPage = 2000;

        RestPage<Item> restPage = Client.getItems(itemsPerPage, 0);
        List<Item> items = restPage.getContent();
        int totalpages = restPage.getTotalPages();

        List<Item> result = new ArrayList<Item>();
        result.addAll(items);
        for (int pageNumber = 1; pageNumber < totalpages; pageNumber ++) {
            result.addAll(Client.getItems(itemsPerPage, pageNumber).getContent());
        }

        return result.stream()
                .filter(itemo -> !itemRepository.existsByCode(item.getCode()))
                .map(item -> itemRepository.save(new Code(item.getCodemsl)))
                .collect(Collectors.toList());

    }

}

Can you help me in improving this?

I see these lines in your code :
List<Item> items = restPage.getContent();
List<Item> result = new ArrayList<Item>();
result.addAll(items);

You can use the ArrayList(Collection<? extends E> c) constructor obtaining the same result:
List<Item> result = new ArrayList<>(restPage.getContent());

Your filtering code looks good to me and for the for loop adding elements to an existing list for me you made the right choice. Although it is possible to add new elements to an existing list using streams this is discouraged because easy cause of errors, see for example this thread.


Can this foreach loop be improved? Can the below foreach loop be improved?  Since I use the same method twice, maybe use continue or break?

int? index = null;
int count = 0;
foreach (Break b in breaks)
{
    if (index.HasValue)
    {
        if (index == count)
            b.Remove();
    }
    else
    {
        b.Remove();
    }
    count++;
}

So I turned it into this:

foreach (Break b in breaks)
{
    if ((index.HasValue && index == count) || !index.HasValue)
    {
        b.Remove();
    }
    count++;
}

Which is the same.  Any other optimizations possible?

How about removing the loop at all like:

if (!index.HasValue)
{
     breaks.Clear();
}
else
{
     breaks.RemoveAt((int)index);
}

if you want/need to keep the loop, i would change your second way like:

foreach (Break b in breaks)
{
    if (!index.HasValue || index == count)
    {
        b.Remove();
    }
    count++;
}


Find scrambled words within word list I'm just getting started with Python -- completing a simple exercise from a practice website.  My code works -- just looking for feedback on better ways to achieve the goal.

Goal:

  Find the original (unscrambled) words, which were randomly taken from
  a wordlist. Send a comma separated list of the original words, in the
  same order as in the list below.

I stored the scrambled words in words.txt and the wordlist in wordlist.txt

#!/usr/bin/env python3

# open word files
wordlist = open("wordlist.txt", "r").read().split()
words = open("words.txt", "r").read().split()

s = list()

# loop through scrambled words
for word in words:
    # break scrambled word into characters
    chars = list(word)
    # loop through comparison list
    for compare in wordlist:
        # break compare word into characters
        compare_chars = list(compare)
        # make copy of scrambled characters
        char_list = chars[:]
        # loop through scrambled word characters
        for char in chars:
            # if character not in compare word go to next compare word
            if not char in compare_chars:
                break
            # if character found remove it (in case character exists more than once)
            compare_chars.remove(char)
            char_list.remove(char)
        # if all compare characters exhausted we *may* have found the word
        if not compare_chars:
            # if all scrambled characters are utilized then we have the right word
            if not char_list:
                s.append(compare)
            else:
                s.append('???')
            break

# create comma separated list of words
print(",".join(s))

I'm not a Python programmer but I would take this approach, as if I understand the problem correctly, it seems like an easy solution. Along side that I believe it makes us of the language features also.

#!/usr/bin/env python3

# open word files
wordlist = open("wordlist.txt", "r").read().split()
words = open("words.txt", "r").read().split()

s = list()

# loop through scrambled words
for word in words:
    # break scrambled word into characters
    chars = sorted(list(word))
    # loop through comparison list
    for compare in wordlist:
        if sorted(list(compare)) == chars:
            s.append(compare)

# create comma separated list of words
print(",".join(s))

This works on the bases that we do not care for order, so an ordered list of each will be a good check.

I have not broken out of the inner loop, as it is unclear weather multiple words could match and all of those words should be returned or not.

This is untested, as stated, I'm not a Python programmer.


Iterative in-place quicksort I have implemented an iterative in-place quicksort algorithm in Python, which by default selects the pivot as the last element:

def quicksort(array):
    """
    Sorts an array of integers using the quick-sort algorithm.

    :param array: the array to be sorted
    :type array: list<int>
    :return: the sorted array
    :rtype: list<int>
    """
    indexes_stack = list()
    idx = (0, len(array) - 1)
    indexes_stack.append(idx)
    for idx in indexes_stack:
        elem_idx = idx[0]
        pivot_idx = idx[1]
        while pivot_idx > elem_idx:
            pivot = array[pivot_idx]
            elem = array[elem_idx]
            if elem > pivot:
                array[pivot_idx] = elem
                array[elem_idx] = array[pivot_idx - 1]
                array[pivot_idx - 1] = pivot
                pivot_idx -= 1
            else:
                elem_idx += 1

        boundar_low = idx[0]
        boundar_high = idx[1]
        if pivot_idx > 1 and boundar_low < pivot_idx - 1:
            indexes_stack.append((boundar_low, pivot_idx - 1))
        if pivot_idx < len(array) -1 and pivot_idx + 1 < boundar_high:
            indexes_stack.append((pivot_idx + 1, boundar_high))

    return array

# Test the sorting function
counter = 0
while counter < 1000:
    test = [int(100 * random.random()) for i in xrange(15)]
    assert (quicksort(list(test)) == sorted(test)), test
    counter += 1

I've tested it with various inputs and it seems to be correct, but I wonder if it can become more efficient or clean.

The implied any aspect of the code posted is fair game for feedback and criticism justifies asking on CR even without an(y) explicit question. I'm taken against guessing what any piece of code is there for: Why another python quicksort? doesn't get answered in the code.
While the docstring looks charming, the parameter to quicksort() is over-specified: everything needed is "subscripting" to get&set items and comparable items. (Numeric items could be deemed handy for picking as a pivot the mean value of several.)
The code mentions neither iterative nor in-place:
quicksort() should be specified to return its (ordered) parameter.

clean, like beauty, lies in the eye of the beholder - to mine, both are close regarding code. I like python for its ability to express things with minimum ado.
(I don't quite like elem_idx&pivot_idx - sometimes use up&down.)

In demonstration code, it seems to have grown conventional to factor out partitioning the current sequence from quicksort().

True? The code doesn't use indexes_stack as a stack. As far as in-place means o(n) space (in addition to output is where input used to be), the implementation presented is not in-place.

More efficient? Sure -

Space: handle the smallest partition first (with impeding out-of-memory, this is a correctness issue)
Time:

choice of pivot is pivotal, using a value from either end of the partition disastrous for pre-ordered input.
Hoare partition scheme instead of Lomuto - three-way if lots of repeated values are to be expected
don't bother with less than two items, better yet:
use a different algorithm for small partitions
don't push the top-priority partition to immediately pop it

In stead of further ranting (keeping elem, elem_idx & pivot_idx for recognition value):

# non-recursive quicksort - re-inventing the wheel finger exercise

# docs.python.org tutorial:
# To add an item to the top of the stack, use append()
class stack(list):
    ''' push() to add an item to the top of the stack '''
    push = list.append

# non-recursive quicksort
def quicksort(items):
    """
    Sort a sequence using the quick-sort algorithm.

    :param items: the sequence to be sorted
    :return: items, sorted
    """
    nItems = len(items)
    if nItems < 2:
        return items
    todo = stack([(0, nItems - 1)])
    while todo:
        elem_idx, pivot_idx = low, high = todo.pop()
        elem = items[elem_idx]
        pivot = items[pivot_idx]
        while pivot_idx > elem_idx:
            if elem > pivot:
                items[pivot_idx] = elem
                pivot_idx -= 1
                items[elem_idx] = elem = items[pivot_idx]
            else:
                elem_idx += 1
                elem = items[elem_idx]
        items[pivot_idx] = pivot

        lsize = pivot_idx - low
        hsize = high - pivot_idx
        if lsize <= hsize:
            if 1 < lsize:
                todo.push((pivot_idx + 1, high))
                todo.push((low, pivot_idx - 1))
        else:
            todo.push((low, pivot_idx - 1))
        if 1 < hsize:
            todo.push((pivot_idx + 1, high))
    return items

if __name__ == '__main__':
# run the sorting function
    from random import randint
    for _ in range(99):
        test = [str(randint(0, 100)) for _ in range(randint(15, 99))]
        assert (quicksort(test[:]) == sorted(test)), test


Creating SQL Table from a Go Struct I have been building an API in Go, which I want to link to a Postgres SQL table. I have always used NoSQL previously but thought it would be good to try a SQL database.
Here are my structs
type Activity struct {
    Activity string `json:"activity"`
    Payload Payload `json:"payload"`
}

type Payload  struct {
    UserID   string `json:"userId"`
    Duration int    `json:"duration"`
    Type     string `json:"type"`
    Date     int    `json:"date"`
}

Based on the above I thought it would be good to have a table for payload which was made via the following query.
CREATE TABLE payload (
    userid SERIAL PRIMARY KEY,
    duration INT,
    type TEXT,
    date INT
);

and then created the activity table like the following:
CREATE TABLE activity (
    activity TEXT PRIMARY KEY
    SELECT userid, duration, type, date
    FROM payload;
);

None of your columns are marked non-nullable. This is important for data integrity.
duration and date both seem mis-represented as int. You're in PostgreSQL, so read https://www.postgresql.org/docs/current/datatype-datetime.html and then consider instead

interval for duration, and
date for date


Checking form input data on submit with pure PHP I have some experience with PHP but I have never even try to do this wit pure PHP, but now a friend of mine asked me to help him with this task so I sat down and write some code. What I'm asking is for opinion if this is the right way to do this when you want to use only PHP and is there anything I can change to make the code better. Besides that I think the code is working at least with the few test I made with it.

Here it is:

<?php
session_start();

// define variables and initialize with empty values
    $name = $address = $email = "";
    $nameErr = $addrErr = $emailErr = "";
    $_SESSION["name"] = $_SESSION["address"] = $_SESSION["email"] = "";
    $_SESSION["first_page"] = false;

    if ($_SERVER["REQUEST_METHOD"] == "POST") {
        if (empty($_POST["name"])) {
            $nameErr = "Missing";
        }
        else {
            $_SESSION["name"] = $_POST["name"];
            $name = $_POST["name"];
        }

        if (empty($_POST["address"])) {
            $addrErr = "Missing";
        }
        else {
            $_SESSION["address"] = $_POST["address"];
            $address = $_POST["address"];
        }

        if (empty($_POST["email"]))  {
            $emailErr = "Missing";
        }
        else {
            $_SESSION["email"] = $_POST["email"];
            $email = $_POST["email"];
        }
    }
    if ($_SESSION["name"] != "" && $_SESSION["address"] != "" && $_SESSION["email"] != "") {
    $_SESSION["first_page"] = true;
    header('Location: http://localhost/formProcessing2.php');
    //echo $_SESSION["name"]. " " .$_SESSION["address"]. " " .$_SESSION["email"];
    }
?>
<DCTYPE! html>

<form method="POST" action="<?php echo htmlspecialchars($_SERVER["PHP_SELF"]);?>">
Name <input type="text" name="name" value="<?php echo htmlspecialchars($name);?>">
<?php echo $nameErr;?>

Address <input type="text" name="address" value="<?php echo htmlspecialchars($address);?>">
<?php echo $addrErr;?>

Email <input type="text" name="email" value="<?php echo htmlspecialchars($email);?>">
<?php echo $emailErr;?>

<input type="submit" name="submit" value="Submit">
</form>

It's incredibly repetitive.  Repetitive code is hard to maintain because you have to apply the same change in many places.  It's also hard to see at a glance where the similarities and differences lie.

Here is how it could be rewritten:

<?php
    $params = array('name', 'address', 'email');

    session_start();
    $_SESSION['first_page'] = true;

    foreach ($params as $p) {
        $$p = $_SESSION[$p] = '';
        if ($_SERVER['REQUEST_METHOD'] == 'POST') {
            if (empty($_POST[$p])) {
                $errorVarName = "${p}Error";
                $$errorVarName = 'Missing';
            } else {
                $$p = $_SESSION[$p] = $_POST[$p];
            }
        }
        if ($_SESSION[$p] == '') {
            $_SESSION['first_page'] = false;
        }
    }
    if ($_SESSION['first_page'])) {
        # None of the expected params is an empty string
        header('Location: http://localhost/formProcessing2.php');
    }
?>

To go one step further, I would suggest using $val[$p] and $err[$p] instead of $$p and $$errorVarName, respectively.  That's because variably named variable names are icky, and smell vaguely of the stink associated with register_globals.  An additional benefit of using an associative array is that you can pass all of the processed parameter values as a single argument to a function, which you can't do if they are individual variables.  The template would need to be modified to say

Name <input type="text" name="name" value="<?php echo htmlspecialchars($val['name']);?>">
<?php echo $err['name'];?>


Centroid of a polygon in Clojure I am studying Clojure and functional programming.  I wrote this function to compute the centroid of a polygon specified as a vector of points, e.g. [[10 20] [0 11] [50 30]]).

Here you can read how to compute the centroid of a polygon.

Can you give me hints and suggestions on how to improve this code from a functional coding style POV?

(defn centroid [p]
  "Calcola il centroide del poligono p"
  (let [six*area     (* 6 (polygon-area p))
        n            (count p)
        first-vertex (p 0)
        polygon      (conj p first-vertex)
        x-terms      (atom [])
        y-terms      (atom [])]
     (dotimes [i n]
            (let [point_i   (polygon i)
                  point_i+1 (polygon (inc i))
                  x_i       (point-x point_i)
                  y_i       (point-y point_i)
                  x_i+1     (point-x point_i+1)
                  y_i+1     (point-y point_i+1)]
               (swap! x-terms conj (* (+ x_i x_i+1)
                                      (- (* x_i y_i+1)
                                         (* x_i+1 y_i))))
               (swap! y-terms conj (* (+ y_i y_i+1)
                                      (- (* x_i y_i+1)
                                         (* x_i+1 y_i))))))
     (make-point (/ (reduce + 0 @x-terms) six*area)
                 (/ (reduce + 0 @y-terms) six*area))))

You could replace the dotimes/atoms with more functional loop/recur recursion.

(defn centroid [p]
  (let [six*area     (* 6 (polygon-area p))
        n            (count p)
        first-vertex (p 0)
        polygon      (conj p first-vertex)]
     (loop [i       0
            x-terms []
            y-terms []]
       (if (< i n)
         (let [point  (polygon i)
               point' (polygon (inc i))
               x      (point-x point)
               y      (point-y point)
               x'     (point-x point')
               y'     (point-y point')
               dxy    (- (* x y')
                         (* x' y))]
           (recur (inc i)
                  (conj x-terms (* (+ x x') dxy))
                  (conj y-terms (* (+ y y') dxy))))
         (make-point (/ (reduce + 0 x-terms) six*area)
                     (/ (reduce + 0 y-terms) six*area))))))

You could also do it through sequences entirely:

(defn centroid [p]
  (let [six*area         (* 6 (polygon-area p))
        polygon          (map (juxt point-x point-y) p) ;; turns the points in [x y] pairs
        ;; `juxt` applies each function in order and outputs the results in a list.

        polygon'         (drop 1 (cycle polygon))       ;; circular permutation of polygon
        ;; `cycle` lazily repeats and concats the input sequence.

        term           (fn [[[x y] [x' y']]]            ;; destructuring a pair of
                                                        ;; [x y] pairs
                         (let [dxy (- (* x y')
                                      (* x' y))]
                           [(* dxy (+ x x')) (* dxy (+ y y'))]))
        [x-terms yterms] (->> (map list polygon polygon') ;; makes the [point point'] pairs
        ;; when `polygon` is exhausted, the generated sequence stops, so the fact that
        ;; `polygon'` is infinite has no effect.
                              (map term)
                              (apply (partial map list)))] ;; turns the sequence of pairs
                                                           ;; into a pair of sequences
        ;; `apply` turns the list argument in an argument list that it feeds to the function
        ;; - here, a sequence of pairs. `partial` takes a function and some of its arguments
        ;; and returns it with its first arguments set - here, `map list ...`. `map list`
        ;; applied to a list of pairs will call `list` with the first elements of all pairs,
        ;; then with the seconds.

    (make-point (/ (reduce + 0 x-terms) six*area)
                (/ (reduce + 0 y-terms) six*area))))

EDIT: a(n unefficient) version with map galore:

(defn centroid [p]
  (let [six*area (* 6 (polygon-area p))
        polygon  (map (juxt point-x point-y) p)
        polygon' (drop 1 (cycle polygon))
        x        (map first polygon)
        x'       (map first polygon')
        y        (map second polygon)
        y'       (map second polygon')
        x**x     (map * x x')
        y**y     (map * y y')
        x**y     (map * x y')
        y**x     (map * y x')
        dxy      (map - x**y y**x)
        x-terms  (map * x**x dxy)
        y-terms  (map * y**y dxy)]
    (make-point (/ (reduce + 0 x-terms) six*area)
                (/ (reduce + 0 y-terms) six*area))))

A full loop/recur version:

(defn centroid [[h & t :as p]]
  (let [six*area (* 6 (polygon-area p))
        a        (point-x h)
        b        (point-y h)]
    (loop [x      a
           y      b
           t      t
           x-term 0
           y-term 0]
      (if-let [[h & t] t]
        (let [x'     (point-x h)
              y'     (point-y h)
              dxy    (- (* x y')
                        (* x' y))
              x-term (+ x-term (* dxy (+ x x')))
              y-term (+ y-term (* dxy (+ y y')))]
          (recur x' y' t x-term y-term))
        (let [dxy    (- (* x b)
                        (* a y))
              x-term (+ x-term (* dxy (+ x a)))
              y-term (+ y-term (* dxy (+ y b)))]
          (make-point (/ x-term six*area)
                      (/ y-term six*area)))))))


Python Find Tuple by Geographic Priority Requirement:

Python Function will read a LIST of TUPLES. The structure of the tuple is ("STRING", "STRING","STRING",INT,"GEOGRAPHIC VALUE")
This Geographic value can have 5 values "zip", "city", "county", "state" and "country"
Now if a Tuple has elements where geographic values are (....,"zip"),(....,"county"), (....,"country") then function should return the element/tuple which contains "zip"
Examples :
Input : [('aa', 'bb', 'cc', 1, 'state'), ('aa', 'bb', 'cc', 2, 'zip')]
Output : ['aa', 'bb', 'cc', 2, 'zip']
In other words 'zip' is higher priority(hp) than 'city'.
'city' is HP than 'county',
'county' is HP than 'state'
and 'state' is HP than 'country'.

I have written code for that but being new to Python, the code looks clunky. Is there any shorter method?
def unique_list(input):
    my_list = input
    print(my_list)
    last_list = []
    zipcnt = -1
    citicnt = -1
    countycnt = -1
    statecnt = -1
    return_list_zip = []
    return_list_city = []
    return_list_county = []
    return_list_state = []
    return_list_country = []
    for j in range(len(my_list)):
          if(my_list[j][4]) == "zip":
             zipcnt = len(my_list)
             return_list_zip = list(my_list[j])
             continue
          elif (my_list[j][4] == 'city' and zipcnt == -1):
              citycnt = len(my_list)
              return_list_city = list(my_list[j])
              continue

          elif (my_list[j][4] == 'county' and zipcnt == -1 and citicnt == -1):
              countycnt = len(my_list)
              return_list_county = list(my_list[j])
              continue
          elif (my_list[j][4]  == 'state'and zipcnt == -1 and citicnt == -1 and countycnt == -1):
              statecnt = len(my_list)
              return_list_state = list(my_list[j])
              continue
          elif (my_list[j][4] == 'country'and zipcnt == -1 and citicnt == -1 and countycnt == -1 and statecnt == -1):
              return_list_country = list(my_list[j])
              continue
    if(zipcnt != -1):
        return_list = return_list_zip
    elif(citycnt != -1):
        return_list = return_list_city
    elif(countycnt != -1):
        return_list = return_list_county
    elif (statecnt != -1):
        return_list = return_list_state
    else:
        return_list = return_list_country
    return return_list
input_list = [ ("aa", "bb", "cc", 1,"state"), ("aa","bb","cc",2,"zip")]
input_list1 = [("XX","YY","NN",1,"country"),("XX","YY","NN",2,"city"),  ("XX","YY","NN",3,"state")]
input_list2 = [("NN","FF","PP",10,"state"),("NN","FF","PP",11,"country"),("NN","FF","PP",10,"city")]
print(unique_list(input_list))
print(unique_list(input_list1))
print(unique_list(input_list2))

You're right, it is clunky.

The first important step is to pick a function name that succinctly describes the task that it performs, namely, picking the most specific address component.

Once you do that, you might realize that "the most X of Y" means that you should use the max() function.  You just have to map each tuple to some quantitative measure of "specificity".

def most_specific_address(addresses):
    SPECIFICITY = {
        'country': 1, 'state': 2, 'county': 3, 'city': 4, 'zip': 5,
    }
    return max(addresses, key=lambda addr: SPECIFICITY[addr[4]])


Script to download Annual Reports (~70,000 in total) from a Website It's only 38 lines of code and I haven't worked with web scraping that much before.

  How heavy would my code be on their server?
  It's for Deep Learning purposes and I haven't ran it yet but would it result in my IP getting banned a quarter way through once I start
  downloading the 70k pdfs?

Also, I'm not sure how efficient this is (right now I don't think I'm even checking if a file exists and I'm overwriting it each time I run my program, so if my code is interrupted halfway through, I'll have to run it again and it'll start downloading everything again from scratch. I'll have to fix that).

But anyways, here is the code:

# Scrapes all pdfs off from www.annualreports.com
# Haven't tested yet but should be somewhere around 70,963 pdfs since my empty search returned 5,479/5,674 stated companies

import requests
from urllib.parse import urljoin
import pandas as pd
from bs4 import BeautifulSoup as bs
import os, re
import pickle

def extract_table():
    r = requests.get('http://www.annualreports.com/Companies?search=')
    soup = bs(r.content, 'lxml')
    df = pd.DataFrame([(i.text, 'http://www.annualreports.com' + i['href']) for i in soup.select('tbody td:nth-of-type(1) a')], columns = ['Company','Link'])
    df.to_pickle('links.pkl') # saves into a dataframe the name of the company plus the href link it points to

def scrap_pdfs():
    df = pd.read_pickle('links.pkl')
    a = 0 # for naming the filenames numerically

    for x in range(df.Link.count()):

        url = df['Link'][x] # reads the "Link" column from the dataframe

        folder_location = r'/home/duke/Annual_Reports/Data' # SPECIFY FULL DIRECTORY

        response = requests.get(url)
        soup= bs(response.text, "html.parser")

        for link in soup.select("a[href$='.pdf']"): #goes through all the .pdfs in all of the href links

            #filename = os.path.join(folder_location, ['href'].split('/')[-1]) # Names the pdf files using the last portion of each link

            filename = os.path.join(folder_location, str(a))
            a+=1

            with open(filename, 'wb') as f:
                f.write(requests.get(urljoin(url,link['href'])).content)

Yes, right now this would be inefficient, I'd definitely say that using
a predictable filename would be the first key change here, if the link
target or description isn't usable as part of the filename, consider
hashing it.  Simply enumerating all links is, well, suboptimal:  What
if, even if previous files are kept around and not overwritten, between
runs the contents of the website changes (seems likely, doesn't it?) and
suddenly the order of the files being download doesn't match what's on
disk?  Definitely do something about that.

Secondly, the code's separating the creation of that pickle file because
it's supposed to be run multiple times?  A bit odd, but as an impromptu
database, why not.  I usually consider pickle files somewhat ephemeral
since the format is (was?) specific to Python.

And then simply check if the file exists and skip it (maybe also check
if it's non-empty; going further before downloading the full data the
HTTP header for the size of the returned content could also be compared,
again, before actually downloading the full files).

The range over the data frame I'd have expected to be simpler, but
right now I can't find if there's an easier way (not using the index,
but simply looping over the values themselves that is).

f.write(requests.get(...).content) - that's gonna buffer the full file
in memory I think.  Better don't do that, instead either requests
might have some facility to write directly to files (or a file-like
object), or alternatively the download content would have to be read
piece-by-piece into a smallish buffer, alternating it with writing that
buffer to the output file.

Finally the formatting could be more consistent (whitespace between
expressions mostly).  Take a look at
PEP8 and perhaps an
autoformatter to do that
automatically.  Also leftover comments should be removed.

Right so overall it's effective, but for production code there's lots of
things that could be done, especially making it a more fully fleshed out
script (read: parse command line arguments, have default parameters for
e.g. the output directory; etc.).  The parsing is fine and the selectors
again are succinct and easy to read.


Adding support to Busybox vi for reading file from stdin I am writing a patch for Busybox's implementation of vi, the classical command line code editor and predecessor of vim. It can open up a files through filenames given as command line arguments. My patch allows initial text to be loaded in to the buffer through a pipe on standard input too.

# normal usage
vi file.txt
# with my patch ('-' tells vi to read from stdin)
ls -l | vi -

The Busybox project is a minimal implementation of many Unix commands, meant to be run on embedded devices and has a very small executable size. The project has a script to measure the executable size delta of two versions. Below is the output for my current implementation.

function                                             old     new   delta
fd_insert                                              -     292    +292
edit_file                                           1299    1370     +71
init_text_buffer                                     333     398     +65
.rodata                                            88972   89004     +32
------------------------------------------------------------------------------
(add/remove: 1/0 grow/shrink: 3/0 up/down: 460/0)             Total: 460 bytes

init_text_buffer does some initialisation and then calls  file_insert to read from the given filename. If it fails, the buffer will be left empty (spare a newline). text is a global pointer to the actual char buffer of the document.

I've modified this function to read from standard input if the filename argument is "-". LONE_DASH is a macro also used in other parts of Busybox.

if (fn && LONE_DASH(fn)) {
    rc = fd_insert(STDIN_FILENO, text);
} else {
    rc = file_insert(fn, text, 1);
}

file_insert was originally the only function to fill the text buffer with initial data. I haven't made any modifications to this function, but I used it as a template for fd_insert.

text_hole_make resizes the buffer if it's too small and returns the pointer delta.

full_read reads characters from a file descriptor into a buffer. The third argument is the maximum characters to read. It returns the number of characters read.

I wrote fd_insert from scratch to mimic the behaviour of file_insert, but to be able to read from file descriptors like STDIN_FILENO.

static int fd_insert(int fd, char *p)
{
    int size = BUFSIZ;
    int cnt = 0;
    int total = 0;

    do {
        total += cnt;
        p += text_hole_make(p + total, size);
        cnt = full_read(fd, p + total, size);
    } while (cnt == size);

    if (cnt < 0) {
        status_line_bold_errno("can't read from fd");
        if (total)  // un-do buffer insert
            p = text_hole_delete(p, p + total - 1, NO_UNDO);
        total = cnt;
    } else {
        total += cnt;
        // shrink unused space
        p = text_hole_delete(p + total, p + total + size - cnt - 1, NO_UNDO);
    }

    return total;
}

I've tried to remove the fd argument to reduce size, but this actually increased the size by 2 bytes (+7 to fd_insert and -5 to init_text_buffer).

There is some duplicated file reading logic in file_insert and fd_insert, but I'm not sure how to extract it. My goal is to reduce the executable size and to make my logic as clear as possible for others to read and maintain.

Can be slightly smaller
I was able to shrink your function a little by combining the two calls to text_hole_delete into one call.  I'm not sure what compiler options you are using, but when I used gcc -Os on a 32-bit x86 target, it saved 16 bytes.  Here is the modified code:
int fd_insert(int fd, char *p)
{
    int size       = BUFSIZ;
    int cnt        = 0;
    int total      = 0;
    int unusedSize = 0;
    int ret        = 0;

    do {
        total += cnt;
        p     += text_hole_make(p + total, size);
        cnt    = full_read(fd, p + total, size);
    } while (cnt == size);

    if (cnt < 0) {
        status_line_bold_errno("can't read from fd");
        ret        = cnt;
        unusedSize = total;
    } else {
        total     += cnt;
        ret        = total;
        p         += total;
        unusedSize = size - cnt;
    }

    // shrink unused space
    if (unusedSize)
        text_hole_delete(p, p + unusedSize - 1, NO_UNDO);
    return ret;
}


Representing multiple errors in Go The MediaWiki API may return multiple errors. Is this a idiomatic way of representing them?

type MediaWikiError struct {
    Code    string      `json:"code"`
    Data    interface{} `json:"data"`
    Message string      `json:"text"`
    Module  string      `json:"module"`
}

func (e *MediaWikiError) Error() string {
    return fmt.Sprintf("MediaWiki module \"%s\" reported an error: %s (%s)", e.Module, e.Message, e.Code)
}

type MediaWikiErrors []*MediaWikiError

func (e MediaWikiErrors) Error() string {
    switch len(e) {
    case 0:
        return ""
    case 1:
        return e[0].Error()
    default:
        codes := make([]string, len(e))
        for i, err := range e {
            codes[i] = err.Code
        }
        return fmt.Sprintf("MediaWiki API reported multiple errors: %s", strings.Join(codes, ", "))
    }
}

1. Don't use \" in string

Go provide a very nice way to handle quote (") in strings:

instead of

fmt.Sprintf("MediaWiki module \"%s\" reported an error: %s (%s)", ...)

you can use

fmt.Sprintf(`MediaWiki module "%s" reported an error: %s (%s)`, ...)

It's easier and nicer to read

2. Use range loop instead of switch

Instead of specifiying behavior for 3 cases ( 0, 1, default), we can use a simple range loop

So this

func (e MediaWikiErrors) Error() string {
    switch len(e) {
    case 0:
        return ""
    case 1:
        return e[0].Error()
    default:
        codes := make([]string, len(e))
        for i, err := range e {
            codes[i] = err.Code
        }
        return fmt.Sprintf("MediaWiki API reported multiple errors: %s", strings.Join(codes, ", "))
    }
}

become simply this:

func (e MediaWikiErrors) Error() (errMsg string) {
    for _, err := range e {
        errMsg += err.Error() + "\n"
    }
    return errMsg
}

I prefer joining errors with a line return (\n) instead of a comma, but it's just my personal taste. Multiple errors would be printed like this:

MediaWiki module "module_1" reported an error: parsing error (3)
MediaWiki module "module_2" reported an error: access denied (22)


Optimizing code: longest repeating substring I know i have some real problems with the following code (as others said). can you please help me to optimize it a little bit? Maybe some example of how to do it.

(find the longest repeating substring with length between x and y)

    public static String longestDuplicate(String text, int x, int y)
{
    String longest = "";
    for (int i = 0; i < text.length() - 2 * longest.length() * 2; i++)
    {
        OUTER: for (int j = Math.min((text.length() - i -1)/2, y) ; j > longest.length() && j >=x; j--)
        {
            String find = text.substring(i, i + j);
            for (int k = i + j; k <= text.length() - j; k++)
            {
                if (text.substring(k, k + j).equals(find))
                {
                    longest = find;
                    continue OUTER;
                }
            }
            break;
        }
    }
    return longest;
}

You may want to roll your own comparison method to compare two pieces of a string in-place, instead of using substring(), which as I understand creates an unnecessary temporary copy. It may look like this:

bool CompareInPlace(String text, int from1, int len1, int from2, int len2)
{
    // ...
}

Furthermore, instead of keeping a copy of the longest duplicat found, keep its indices from and len.
Then in the end you can return text.substring(longestFrom, longestLen);.


Setting new values in array based on query results I have some code which uses nested foreach loops to take the results of a query and drill down into the data, reassigning into a main data array at each level of the foreach nesting. The problem is that required reference variables are passed into the loops deeper down the tree, making it difficult to refactor.

I'd like to refactor this into a more functional style. I've looked into array_walk() and array_map() to try and accomplish this but they seem to be more suited to amending the array argument rather than reassigning to an external array.

I'm looking more for general approaches to refactoring out foreach loops in PHP with this sort of context rather than a direct solution to my problem.

foreach($grid_fields as $gridFieldKey => $gridFieldValue) {
    $grid_field_id = $gridFieldValue['field_id'];
    $grid_field_str = 'field_id_' . $grid_field_id;
    $grid_entry = $this->fetchGridEntry($grid_field_id, $data['entry_id']);

    if(sizeOf($grid_entry) > 0) {
        $data[$grid_field_str] = array();
        $data[$grid_field_str]['entry_id'] = $cloned;

        foreach($grid_entry as $gridEntryKey => $gridEntryValue) {
            $grid_row = 'new_row_' . $gridEntryKey;
            $data[$grid_field_str]['rows'][$grid_row] = array();

            foreach($gridEntryValue as $geK => $geV) {
                if(strpos($geK, 'col_id_') !== false || strpos($geK, 'row_id') !== false) {
                    $data[$grid_field_str]['rows'][$grid_row][$geK] = $geV;
                }
            }
        }
    }
}

array_filter  removes elements from an array
array_map changes
each element of an array to something else
array_walk steps
through each element of an array (usually to create side effects)
(does not return anything)
array_reduce create something new from
an existing array

Every traditional loop (that does one thing) is one of the above.

<?php
// Inside your object.
$data = array_reduce($grid_fields, add_grid_fields_to_data($cloned, $this), $data);

// Outside your object.
function add_grid_fields_to_data($cloned, $object) : array {
    return (function (array $data, array $grid_field) use ($cloned) {

        $grid_entry = $this->fetchGridEntry($grid_field['field_id'], $data['entry_id']);
        $grid_field_str = 'field_id_' . $grid_field_id;

        $data[$grid_field_str] = array();
        $data[$grid_field_str]['entry_id'] = $cloned;

        $no_grid_entry = sizeOf($grid_entry) == 0;
        $no_grid_entry ?: $data = array_reduce(array_keys($grid_entry), add_grid_entry_values_to_data($grid_entry, $grid_field_str), $data);

        return $data;
    })->bindTo($object);
}

function add_grid_entry_values_to_data(array $grid_entry, $grid_field_str) : Closure {
  return function (array $data, string $gridEntryKey) use ($grid_entry, $grid_field_str) : array {
    $gridEntryValue = $grid_entry[$gridEntryKey];
    $grid_row = 'new_row_' . $gridEntryKey;

    $data[$grid_field_str]['rows'][$grid_row] = array();

    $data = array_reduce(array_keys($gridEntryValue), add_grid_row_to_data($grid_row, $gridEntryValue), $data);
    return $data;
  };
}

function add_grid_row_to_data(string $grid_row, array $gridEntryValue) : Closure {
    return function (array $data, $geK) use ($grid_row, $gridEntryValue) : data {
        $geV = $gridEntryvalue[$geK];

        $row_col_id_exists = strpos($geK, 'col_id_') !== FALSE || strpos($geK, 'row_id') !== FALSE;
        $row_col_id_exists ? $data[$grid_field_str]['rows'][$grid_row][$geK] = $geV : NULL;

        return $data;
    };
}


Calculate probability that knight does not attempt to leave chessboard after at most k random moves (LeetCode #688) I'm learning competitive programming and came across this question on LeetCode : 688. Knight Probability in Chessboard

On an n x n chessboard, a knight starts at the cell (row, column) and
attempts to make exactly k moves. The rows and columns are 0-indexed,
so the top-left cell is (0, 0), and the bottom-right cell is (n - 1, n-1).
A chess knight has eight possible moves it can make, each move is two cells in a cardinal direction,
then one cell in an orthogonal direction. Each time the knight is to move, it chooses one of eight > possible moves uniformly at random (even if the piece would go off the chessboard) and moves there.
The knight continues moving until it has made exactly k moves or has moved off the chessboard.
Return the probability that the knight remains on the board after it has stopped moving.

I wrote following BFS solution for it
from collections import deque

class Solution:
    def knightProbability(self, N: int, K: int, r: int, c: int) -> float:
        i = 0
        pos_moves = deque([[r, c, 1]])  # (r, c, prob)
        self.moves_memo = {}
        while i < K and len(pos_moves):
            increase_i_after = len(pos_moves)
            for j in range(increase_i_after):
                move = pos_moves.popleft()

                for next_move in self.get_pos_moves(N, move[0], move[1], move[2]):
                    pos_moves.append((next_move[0], next_move[1], next_move[2]))

            i += 1

        ans = 0
        for m in pos_moves:
            ans += m[2]

        return ans/len(pos_moves) if len(pos_moves) > 0 else 0

    def get_pos_moves(self, n, r, c, prev_p):
        # Returns a list of possible moves
        if (r, c) in self.moves_memo:
            pos_moves = self.moves_memo[(r, c)]
        else:
            pos_moves = deque([])
            if r+2 < n:
                if c+1 < n:
                    pos_moves.append([r+2, c+1, 0])
                if c-1 >= 0:
                    pos_moves.append([r+2, c-1, 0])
            if r+1 < n:
                if c+2 < n:
                    pos_moves.append([r+1, c+2, 0])
                if c-2 >= 0:
                    pos_moves.append([r+2, c-2, 0])
            if r-2 >= 0:
                if c+1 < n:
                    pos_moves.append([r-2, c+1, 0])
                if c-1 >= 0:
                    pos_moves.append([r-2, c-1, 0])
            if r-1 >= 0:
                if c+2 < n:
                    pos_moves.append([r-1, c+2, 0])
                if c-2 >= 0:
                    pos_moves.append([r-1, c-2, 0])

            self.moves_memo[(r, c)] = pos_moves

        l = len(pos_moves)
        if l == 0:
            return pos_moves
        else:
            for move in pos_moves:
                move[2] = prev_p*l/8

            return pos_moves

For n = 8, K = 30, r = 6, c = 4, this solution exceeds time limits. I am not able to figure out why is this solution less time efficient than this. I'm looking for reasons why my code is 'slow'. Thank you!

Both answers basically try all k-length paths and count the number of paths that remain on the board compared to all possible paths.
A major difference between your answer and the recursive solution you link to is what is cached (i.e., memoized).  You code caches the possible moves from a square.  The linked code caches partial solutions.  That is, the cache key is the position on the board and the number of moves remaining.
Consider starting at (0,0) with k steps. One path might start (0,0,k)->(1,2,k-1)->(3,3,k-2),.... When calculating the path that starts with (0,0,k)->(2,1,k-1)->(3,3,k-2), the linked code has already calculated and cached the partial solution for (3,3,k-2) and doesn't need to calculate all those paths again.  In contrast, you solution only caches the legal moves from (3,3) and does need to go through all the paths again.
Note, the linked answer could be sped up significantly by using the symmetry of the board to cache partial solutions for reflected and rotated squares. For the paths mentioned above, the solution for (2,1,k-1) and (1,2,k-1) are the same because of board symmetry. For each square on the diagonal or horizontal/vertical centerlines (if any) there are 3 squares with the same solutions due to symmetry. For other squares there are 7 other equivalent squares. A caching algorithm that uses this symmetry could be much faster.


Image upload and thumbnail creation script using PHP GD Ignoring my shocking use of the mysql_* extension (and any associated issues), could you take a look at my script that uploads an image, adds it to a database, creates a thumbnail of the image 578px in width, and adds that to the database too?

<?php
require_once("validate.php");

while ($hq = mysql_fetch_assoc($hash_query)) {
    $ukip_name = $hq['ukip_name'];
    $speaker_id = $hq['speaker_id'];
    $conference_id = $hq['conference_id'];
    $conf_title = $hq['conf_title'];
    $webmaster_id = $hq['webmaster_id'];
    $first_name = $hq['first_name'];
    $family_name = $hq['family_name'];
    $job_title = $hq['job_title'];
    $company_name = $hq['company_name'];
    $country = $hq['country'];
    $speaker_photo = $hq['speaker_photo']; // Default = NULL
    $company_logo = $hq['company_logo']; // Default = NULL

}
$errors   = $_FILES['file']['error'];
$tempname = $_FILES['file']['tmp_name'];
$filename = $_FILES['file']['name'];
$filetype = $_FILES['file']['type'];
$filesize = $_FILES['file']['size'];
$extensions = array("jpg", "jpeg", "JPG", "JPEG");
$expl = explode(".", $filename);
$extension = end($expl);
if ((($filetype == "image/jpg") ||
    ($filetype == "image/jpeg")) && in_array($extension, $extensions)) {
    if ($errors == 0) {
        if ($filesize <= 10485760) {
            $timestamp = date("ymd-His");
            $filename = $speaker_id . "_" . $timestamp . "_photo." . $extension;
            $db_insert = mysql_query("INSERT INTO temp_uploads
                                     SET speaker_id = $speaker_id, image_filename = '$filename', image_type = 1");
            move_uploaded_file($tempname, "uploads/" . $filename);
            // Resize JPEG
            $md = explode(".", $filename);
            if (end($md) != "eps") {
                $filedims = getimagesize("uploads/" . $filename);
                $md_width = 578;
                $md_height = ($filedims[1] / $filedims[0]) * $md_width;
                $filename_md = $md[0] . "_md." . end($md);
                $quality = 100;
                $src = "uploads/" . $filename;
                $new = imagecreatefromjpeg($src);
                $canvas = imagecreatetruecolor($md_width, $md_height);
                imagecopyresampled($canvas, $new, 0, 0, 0, 0, $md_width, $md_height, $filedims[0], $filedims[1]);
                imagejpeg($canvas, "uploads/" . $filename_md, $quality);
                $db_insert_md = mysql_query("INSERT INTO temp_uploads
                                            SET speaker_id = $speaker_id, image_filename = '$filename_md', image_type = 1");
            }
            mysql_close();
            header("Location: crop.php?hash={$hash}&file={$filename_md}");
            exit();
        } else {
            closeCon(105, "index", "&hash={$hash}");
        }
    } else {
        closeCon(104, "index", "&hash={$hash}");
    }
} else {
    closeCon(103, "index", "&hash={$hash}");
}
?>

I do checks on the filesize, so that something over 10MB is disallowed, but I have hit out-of-memory PHP errors on line 43 ($new = imagecreatefromjpeg($src);) a couple of times recently - one on an 8MB file, and one on a 3.5MB file, which is worrying. These aren't dropping me out of the script in the expected way (closeCon(105...)) but with a PHP fatal error.

I have temporarily set my initiation file to memory_limit=-1 whilst I try and resolve the error, but I was hoping someone here could look at ways of streamlining my code so as to free up as much memory as possible, as well as suggesting general improvements (again, ignoring any mysql_* related ones - I know, and I would use mysqli_* if I could).

It's considered better to omit the closing ?> of the PHP file.

If you're wondering why, there are a few rare situations in which you can accidentally have whitespace trailing the closing tag and issues will result. Since PHP doesn't require the closing tag anyway, it's safer to just always omit it.

I'm a bit skeptical of validate.php. What exactly is it doing? Is it processing user input, or bootstrapping, or what? I suspect it's doing way too many things (processing user input and bootstrapping). It's a bit uncomfortable to me that $hash_query likely has some kind of user input going into it, yet it cascades through another file.

It would be a lot cleaner and cleaner to instead have a getSpeakerByHash() function.

Why is the mysql_fetch_assoc in a loop? It seems it's only actually operating on one row.

Since you don't use most of the variables that come from the hash_query stuff, I wouldn't bother even defining them. They're just clutter.

Also, instead of pulling the values out into independent variables, I would instead give $hq a more meaningful name (perhaps $speaker or something). That makes it a lot clearer that the variables are related, and it makes it a bit easier to see where they came from.

What happens if the mysql_fetch_assoc call doesn't return anything? You should handle that situation. (Although it might not actually be possible for there to not be a result since I have no idea where hash_query is coming from.)

Once again, with the $_FILES stuff, I would defined a $file = $_FILES['file'] and then access into that array. Not only does it cut down on the clutter of defining 5 variables only to use each one once or twice, it makes it clearer that everything is related in one group.

$errors   = $_FILES['file']['error']; is prone to emit a warning if a request is submitted without a file field. This means that you should be checking if the field exists:

if (isset($_FILES['file'])) {
    $file = $_FILES['file'];
} else {
    // Error
}

$extension = pathinfo($filename, PATHINFO_EXTENSION);

Should be a little cleaner than exploding and accessing the last array element.

Since the filetype comes from the client side, you can't trust it at all. I wouldn't even bother checking it. I would just check the extension, and then if the extension is allowed, check if it can actually be opened as a jpg.

You should pull 10485760 into some kind of configuration management. That way, if you want to change it to something else later, you don't have to go digging around for it. Also, it might be clearer to have it as 10*1024*1024 since that's immediately clear to be 10MB.

Since $timestamp is only used once, I would just do the date call inline instead of having it out in a variable.

Instead of a deeply nested if-tree, I would use an if-else flow to keep things flatter:

if (!blah) {
    closeCon(x, "...", "...");
} else if (!bleh) {
    closeCon(y, "...", "...");
} else {
    // Do actual work
}

Don't use $errors == 0. UPLOAD_ERR_OK is much clearer.

You should make sure move_uploaded_file doesn't fail. If it does, you'll end up with a DB record that has no matching file with it. This can be fixed by either using a transaction and rolling back if the move fails, or more simply, just do the insert after the file moving. (Technically you can then have a file without a DB record, but that will likely be less bad.)

Don't reuse variable names ($filename). It's confusing to others reading your code, and it's prone to error.

I've cut out the irrelevant code, but consider this:

if ((($filetype == "image/jpg") ||
    ($filetype == "image/jpeg")) && in_array($extension, $extensions)) {
    $filename = $speaker_id . "_" . $timestamp . "_photo." . $extension;
    $md = explode(".", $filename);
    if (end($md) != "eps") {

$md can't be eps. md will always be a value that's in $extensions.

With regards to the Location header, technically it's supposed to have a full url (i.e. http://.../... rather than a relative url). No browser actually requires this, and all of them act as expected, but it's worth knowing that they're not technically required to, nor is there a guarantee that future browsers will.

It would be cleaner for closeCon to take an array as the third parameter and build the query string internally with http_build_query. It also means that callers to closeCon don't have to handle potential URL escaping.

Resizing an image seems a pretty clear cut requirement that might be nice to push into a function rather than embedding in the upload functionality. resizeImageToMaxWidth is also clear enough that it's understandable at a glance rather than having to either comment it or read 10 lines of code.

Rather than using "uploads/" . $filename twice, I would create a $filepath variable that contains the entire relative path. This means that you can avoid the potential bug of changing one of the uploads and not the other.

It might also be nice to make uploads configurable.

With your insert queries, you should be checking that they succeed. The query never should feasibly fail, but you'll very much want to know if it ever does for some weird reason.

And, if you're not going to check that they succeed, there's no point in assigning them to variables.

I tried not to harp on this since you already seem aware of it, but PDO's exception throwing is a rather nice way of having errors crop up from queries when you don't actually expect them to happen, but you really should know if they do, as in this case.

With regards to the first parameters of closeCon, it would be nice to pull those numbers into constants. I have no idea what any of them mean, and you likely won't either 3 months from now.

Unfortunately my knowledge of image processing is very limited, but I can tell you that I tend to prefer imagemagick over GD. I have no proof of this, but in the few situations I've used both, IM seems to perform the same functionality a bit snappier than GD.

The only other thing I can tell you is that image processing is surprisingly memory intensive and you might just need to crank up the amount of memory script executions can use. GD at some point probably represents the image as a matrix of pixels. This means that the amount of memory required isn't the size of the image but rather the resolution multiplied by colors per pixel. That means that a simple 8 megapixel picture could be 32MB of memory whereas the JPG compressed version would likely only be 5MB.

This SO question seems to have had a similar problem to you and might be worth looking into.


Creating a presigned URL I've created software in Go to create a presigned URL:

package main

import (
    "fmt"
    "log"
    "time"

    "github.com/minio/minio-go"
    "github.com/kelseyhightower/envconfig"
)

type AWS struct {
  Endpoint          string
  Access_Key_Id     string
  Secret_Access_Key string
  SSL               bool
  Bucket            string
  Bucket_Key        string
  Bucket_Expiry     string
}

func main() {
    var aws AWS
    // Add configurations from environment variable
    // e.g:
    //    AWS_ENDPOINT         -> aws.Endpoint
    //    AWS_ACCESS_KEY_ID    -> aws.Access_Key_Id
    //    ...
    if err := envconfig.Process("aws", &aws); err != nil {
        log.Fatal(err.Error())
    }

    svc, err := minio.New(aws.Endpoint, aws.Access_Key_Id, aws.Secret_Access_Key, aws.SSL)
    if err != nil {
    }

    fmt.Println(aws)
    url, err := svc.PresignedPutObject(aws.Bucket, aws.Bucket_Key, aws.Bucket_Expiry)

    if err != nil {
        log.Fatalln(err)
    }

    fmt.Println("Successfully generated presigned URL", url)
}

I would like to know if it is possible to avoid all the if statements for error validations. This is my second program in Go, and I have mainly a Node.js background, so I don't know exactly how to handle errors in Go-style, and also how to make the code more idiomatic.

I would like to know if it is possible to avoid all the if statements for error validations.

No.

This is the way errors are handled in Go. You get the error as return value and you handle it. You may return it, log it, print it or mute. You can panic with it, which is somewhat similar to exceptions. It's all up to you.

Check out this blog post to see reasoning behind the way errors are implemented.

  This is my second program in Go, and I have mainly a Node.js background, so I don't know exactly how to handle errors in Go-style, and also how to make the code more idiomatic.

Node.js also has an error convention which is carefully followed by community users. Same stands for Go.

To make code more idiomatic I suggest you to read more standard library code. It will help a lot. Simply every time you look through the docs click on function (or other identifier) to see the actual code behind it. It's is super easy and may reveal how the things work in Go.

Here follows annotated rewrite:

package main

import (
    "fmt"
    "os"
    "github.com/minio/minio-go"
    "github.com/kelseyhightower/envconfig"
)

// CamelCase is idiomatic in Go, but snake_case.
type AWS struct {
    Endpoint        string `envconfig:"ENDPOINT"`
    AccessKeyId     string `envconfig:"ACCESS_KEY_ID"`
    SecretAccessKey string `envconfig:"SECRET_ACCESS_KEY"`
    SSL             bool   `envconfig:"SSL"`
    Bucket          string `envconfig:"BUCKET"`
    BucketKey       string `envconfig:"BUCKET_KEY"`
    BucketExpiry    string `envconfig:"BUCKET_EXPIRY"`
}

// Exit will write error text to stderr and exit
// with non zero return code.
func Exit(err error) {
    fmt.Fprintln(os.Stderr, err)
    os.Exit(1)
}

func main() {
    var aws AWS

    // Fill configuration from enviroment variables
    err := envconfig.Process("aws", &aws)
    if err != nil {
        Exit(err)
    }

    svc, err := minio.New(aws.Endpoint, aws.AccessKeyId, aws.SecretAccessKey, aws.SSL)
    if err != nil {
        Exit(err)
    }

    url, err := svc.PresignedPutObject(aws.Bucket, aws.BucketKey, aws.BucketExpiry)
    if err != nil {
        Exit(err)
    }

    // stdout is used only to print received URL
    fmt.Println(url)
}


Determine number of times an id was used with a domain IP This is a slow query and the table has 50 million rows.

Table named 'log':

| id     | domainIP        |
| foo    | 158.132.34.5    |
| bob    | 128.12.244.3    |
| bob    | 128.12.244.3    |
| bob    | 19.152.134.4    |
| bob    | 168.152.34.9    |
| alice  | 178.132.64.10   |
| alice  | 188.152.214.200 |
| peter  | 208.162.36.153  |
| peter  | 208.162.36.153  |
| peter  | 208.162.36.153  |
| peter  | 198.168.94.201  |

The following query, to get the number of times id was used with each 'domainIP', and the percentage of each:

SELECT
    `log`.`id`,
    `log`.`domainIP`,
    COUNT(`log`.`domainIP`) AS "Times",
    totalsTable.Totals,
    (COUNT(`log`.`domainIP`)/totalsTable.Totals)*100 AS "Percentage"
FROM `log`
JOIN
    (
    SELECT
        `id`,
        COUNT(`domainIP`) AS Totals
    FROM `log` GROUP BY `id`
    ) AS totalsTable

ON (`log`.`id` = totalsTable.`id`)

GROUP BY `log`.`domainIP` ORDER BY `log`.`id` ASC, "Percentage"  DESC

It returns:

| id     | domainIP        | Times | Totals | Percentage
| foo    | 158.132.34.5    | 1     | 1      | 100
| bob    | 128.12.244.3    | 2     | 4      | 50
| bob    | 19.152.134.4    | 1     | 4      | 25
| bob    | 168.152.34.9    | 1     | 4      | 25
| alice  | 178.132.64.10   | 1     | 2      | 50
| alice  | 188.152.214.200 | 1     | 2      | 50
| peter  | 208.162.36.153  | 3     | 4      | 75
| peter  | 198.168.94.201  | 1     | 4      | 25

It returns the result I need, but it's unusable slow.

EDIT: Here's the table structure exported from phpmyadmin.

CREATE TABLE `log` (
  `id` varchar(150) COLLATE utf8_unicode_ci DEFAULT NULL,
  `eDate` datetime DEFAULT NULL,
  `domainIP` varchar(150) COLLATE utf8_unicode_ci DEFAULT NULL,
  `event` varchar(150) COLLATE utf8_unicode_ci DEFAULT NULL
) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;

ALTER TABLE `log`
  ADD UNIQUE KEY `logUnique` (`id`,`eDate`,`event`),
  ADD KEY `eDate` (`eDate`),
  ADD KEY `id` (`id`,`eDate`),
  ADD KEY `event` (`id`,`eDate`,`event`);

Results of EXPLAIN query:

id | select_type | table | type  | possible_keys      | key       | key_len | ref            | rows  | Extra
1 | PRIMARY | <derived2> | ALL   | NULL               | NULL      | NULL    | NULL           | 100   | Using where; Using temporary; Using filesort
1 | PRIMARY | log        | ref   | logUnique,id,event | logUnique | 453     | totalsTable.id | 1     |
2 | DERIVED | log        | index | NULL               | id        | 459     | NULL           | 100   |

First, I would ask if you really need the percentage calculation to happen in the query result set (vs. being able to calculate this in application layer).  This is really what is causing complexity in your use case, as being able to get row-level data and summary data (the percentage) in a single query is a bit of a challenge in most relational databases.

If you don't require this in your actual query, then you can greatly simplify this query to something like:

SELECT
    id,
    domainIP,
    COUNT(domainIP) AS times
FROM log
GROUP BY id, domainIP
ORDER BY id ASC, times DESC

This would eliminate your subselect and the resulting filesort that is happening because of it. Of course, the query above would require an index on domainIP to be optimized.

This would give a result set like:

| id     | domainIP        | times
| foo    | 158.132.34.5    | 1
| bob    | 128.12.244.3    | 2
| bob    | 19.152.134.4    | 1
| bob    | 168.152.34.9    | 1
| alice  | 178.132.64.10   | 1
| alice  | 188.152.214.200 | 1
| peter  | 208.162.36.153  | 3
| peter  | 198.168.94.201  | 1

This may or may not be trivial to compile this into your percentage results in your application layer depending on how many unique id values you have.

You could potentially make it even easier by using WITH ROLLUP clause in your GROUP BY (see https://dev.mysql.com/doc/refman/5.7/en/group-by-modifiers.html for more info).

That query might look like this:

SELECT
    id,
    domainIP,
    COUNT(domainIP) AS times
FROM log
GROUP BY id ASC, domainIP ASC WITH ROLLUP

This would yield a result set like:

| id     | domainIP        | times
| foo    | 158.132.34.5    | 1
| foo    | NULL            | 1
| bob    | 128.12.244.3    | 2
| bob    | 19.152.134.4    | 1
| bob    | 168.152.34.9    | 1
| bob    | NULL            | 4
| alice  | 178.132.64.10   | 1
| alice  | 188.152.214.200 | 1
| alice  | NULL            | 2
| peter  | 208.162.36.153  | 3
| peter  | 198.168.94.201  | 1
| peter  | NULL            | 4
| NULL   | NULL            | 11

Note here that there are added rows where the NULL values represent case where you have subtotal for the preceding aggregation set (i.e. for each of you id values).  This could make it even easier for application logic to pick up you group-level totals.

Now, if you truly need to get percentages totals in the result set, then I would first take a step back and really understand what the use case is. Do you really need to make this query on an ad hoc basis with a need for exact real-time results?  If so, you may just have to live with the fact that your query may take a long time to run, as you will always have to calculate the totals as aggregated at id level and at id/domainIP level.

If however you can live with periodically updating the aggregations (like scheduled via cron, or simlar), then your options open up greatly to deliver better performance.  Perhaps you runs a query like I have shown above and place the results into a temporary table every X minutes and then use that table as the basis for your query.  Perhaps you look at wholly separate summary tables (as suggested in the answer by @Siegen), with trigger-based updates.  Perhaps you have a wholly separate ETL process which takes this data and populates in into a data store more geared towards data reporting (i.e. a data warehouse). Whatever the case may be, I would suggest that you might need to take a step back and think about what you use case really requires, and my guess is that you will find it does not require ad-hoc, real-time, query results like what you are trying to get here.

With regards to your table schema:

Your id and event index are totally unnecessary and are likely hurting insert performance on this table.  event is an exact duplicate of logUnique (just withut the uniqueness constraint, which makes no sense). id index use cases can be satisfied via the logUnique index as well, as the first two fields in the index are exactly the same and in the same order.
You might consider doing away with camelCasing of any database object identifiers (table names, field names, etc.) as case-sensitivity is typically quite meaningless in MySQL and may lead to some problems when applied across different operating systems..  Many MySQL admin folks consider it best practice to simply use snake_case for naming all database object identifiers to keep things simple.  Read https://dev.mysql.com/doc/refman/5.7/en/identifier-case-sensitivity.html for more on this.


How to sort and search in JavaScript large set The array "scores" tells the total points for each person involved in a contest. So for example:
User A: 100 points
User B: 90 points
User C: 90 points
User D: 80 points
User E: 75 points
User F: 60 points

According to above scores we also have this ranking:
User A: #1
User B: #2
User C: #2
User D: #3
User E: #4
User F: #5

This ranking method follows the Dense Ranking method.
Then we have a user named alice. If she gets 55 points, she will rank at position #6 (according to ranking above).
If she scores 90 points, she will rank at position #2. And so on.
I actually have an array containing different "sessions" for alice. So having for example:
[55, 90]
This means that first time will alice be ranked at position #6. While second time she will be ranked at position #2.
I coded this, and it work. However, this does not seem to be very effective. For large datasets, with half million entries in the scores-array, it times out. This is the code:
const getPosition = (element, scores) => {
    scores.push(element);
    scores.sort(function (a,b) { return b-a; });
    return scores.indexOf(element)+1;
}

function climbingLeaderboard(scores, alice) {
    var uniqueSet = new Set(scores);
    scores = [...uniqueSet];
    var positions = [];
    let aliceIndex = 0;
    while(aliceIndex < alice.length){
        positions.push(getPosition(alice[aliceIndex], scores));
        aliceIndex++;
    }
    return positions;
}

function main() {
    const scores = [100, 90, 90, 80, 75, 60];
    const alice = [50, 65, 77, 90, 102];
    let result = climbingLeaderboard(scores, alice);
    console.log(result.join("\n") + "\n");
}

I guess the "sort"-function and/or searching for the element in the array with indexOf is the problem. But I could not find a way to make these two operations more efficient.

The following function sort of agrees with your function. Except that for example your function given
scores = [ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 ] and alice = [ 0, 0, 0, -1 ] gets 1,1,1,5, while I think it should be 1,1,1,2.
Anyway, it easily handles scores arrays of sizes 10^7 for scores and 10^5 for alice.
The idea is, we already know scores is sorted as per your comment. We can then go through the scores array. We sort the alices array while remembering the original order. Then we go through the scores array, we first look at the largest alice, once we hit an element >= it, it is time to note that down, we remember the score and whether it was a tie and the position it happened at. We continue traversing scores, now looking at the next biggest alice, etc.
At the end of this we know where each alice would be ranked in the original array. Now we just need to account for the rank increases that come from prior alices, basically the rule is, if you are alice 7, then you look at the previous scores from alices 1 to 6 which resulted in a rank increase, and count 1 each time their score was larger than yours. We can do this fairly quickly by maintaining a sorted array of the previous scores using a binary search to find where to insert the next element. Remembering to account for ties amongst Alices scores.
function binarySearch(array, targetValue) {
    if (array.length === 0) {
        return 0;
    }
    let min = 0;
    let max = array.length;
    let guess;

    while (max - min > 1) {
        guess = Math.floor((max + min) / 2);

        if (array[guess] > targetValue) {
            min = guess;
        } else {
            max = guess;
        }
    }

    return array[min] < targetValue ? min : min + 1;
}

function climbingLeaderboard(scores, alice) {
    if (alice.length === 0) {
        return [];
    }
    const sortedAliceWithIndex = alice.map((s, i) => [s, i]).sort(([a], [b]) => b - a);

    let aliceInd = 0;
    let currAlice = sortedAliceWithIndex[aliceInd][0];
    const ranks = [];
    let i = 0;
    let effectiveCount = 0;
    while (true) {
        const currScore = scores[i];
        if (currScore !== undefined && currScore > currAlice) {
            i++;
            if (!(scores[i] === scores[i - 1])) {
                effectiveCount++;
            }
            continue;
        }

        ranks.push([
            effectiveCount,
            sortedAliceWithIndex[aliceInd][1],
            currAlice === currScore,
            currAlice
        ]);
        aliceInd++;

        if (aliceInd >= alice.length) {
            break;
        }
        currAlice = sortedAliceWithIndex[aliceInd][0];
    }

    const inOriginalAliceOrder = ranks.sort(([, i1], [, i2]) => i1 - i2);

    const accountingForRanks = [];
    let rankIncreases = [];
    for (let [i, _, tie, score] of inOriginalAliceOrder) {
        let rankUp = binarySearch(rankIncreases, score);

        if (rankIncreases[rankUp - 1] === score) {
            accountingForRanks.push(i + rankUp);
        } else {
            accountingForRanks.push(i + rankUp + 1);
            if (!tie) {
                rankIncreases.splice(rankUp, 0, score);
            }
        }
    }
    return accountingForRanks;
}

The following runs in a couple of seconds for me.
function main() {
    let scores = new Array(10 ** 6);
    for (let i = 0; i < scores.length; i++) {
        scores[i] = 1 * Math.random();
    }
    scores.sort((a, b) => b - a);
    let alice = new Array(10 ** 5);
    for (let i = 0; i < alice.length; i++) {
        alice[i] = 2 * Math.random() - 1;
    }

    let result1 = climbingLeaderboard(scores, alice);

    console.log(result1);
}


Upload csv file to sqlserver with python I have to weekly upload a csv file to sqlserver, and I do the job using python 3.
The problem is that it takes too long for the file to be uploaded (around 30 minutes), and the table has 49000 rows and 80 columns.

Here is a piece of the code, where I have to transform the date format and replace quotes as well. I have already tried it with pandas, but took longer than that.

import csv
import os
import pyodbc
import time
srv='server_name'
db='database'
tb='table'
conn=pyodbc.connect('Trusted_Connection=yes',DRIVER='{SQL Server}',SERVER=srv,DATABASE=db)
c=conn.cursor()

csvfile='file.csv'

with open(csvfile,'r') as csvfile:
reader = csv.reader(csvfile, delimiter=';')
    cnt=0
    for row in reader:
        if cnt>0:
            for r in range(0,len(row)):
                #this is the part where I transform the date format from dd/mm/yyyy to yyyy-mm-dd
                if (len(row[r])==10 or len(row[r])==19) and row[r][2]=='/' and row[r][5]=='/':
                    row[r]=row[r][6:10]+'-'+row[r][3:5]+'-'+row[r][0:2]

                #here I replace the quote to nothing, since it is not important for the report
                if row[r].find("'")>0:
                    row[r]=row[r].replace("'","")

        #at this part I query the index to increment by 1 on the table
        qcnt="select count(1) from "+tb
        resq=c.execute(qcnt)
        rq=c.fetchone()
        rq=str(rq[0])

        #here I insert each row into the table that already exists
        insrt=("insert into "+tb+" values("+rq+",'"+("', '".join(row))+"')")
        if cnt>0:
            res=c.execute(insrt)
            conn.commit()
        cnt+=1
conn.close()

Any help will be appreciated. Thanks!

First of all, when in doubt, profile.

Now a not-so-wild guess. Most of the time is wasted in

    qcnt="select count(1) from "+tb
    resq=c.execute(qcnt)
    rq=c.fetchone()
    rq=str(rq[0])

In fact, the rq is incremented by each successful insert. Better fetch it once, and increment it locally:

    qcnt="select count(1) from "+tb
    resq=c.execute(qcnt)
    rq=c.fetchone()

    for row in csvfile:
        ....
        insert = ....
        c.execute(insert)
        rq += 1
        ....

Another guess is that committing each insert separately also does not help with performance. Do it once, after the loop. In any case, you must check the success of each commit.

Notice that if there is more than one client updating the table simultaneously, there is a data race (clients fetching the same rq), both with the original design, and with my suggestion. Moving rq into a column of its own may help; I don't know your DB design and requirements.

Consider a single insert values, wrapped into a transaction, instead of multiple independent inserts.

Testing for cnt > 0 is also wasteful. Read and discard the first line; then loop for the remaining rows.

Figuring out whether a field represents a date seems strange. You shall know that in advance.


Number to Roman in Javascript Is this a good solution for the Number to Roman Numerals Kata in Javascript?

function toRomans(n) {
    var nString = n.toString();
    var result = "";
    var allLetters = [["I", "V", "X"], ["X", "L", "C"], ["C", "D", "M"], ["M"]];
    var letterSet = nString.length - 1;
    for (var index in nString) {
        result = result + conversor(nString[index], allLetters[letterSet]);
        letterSet --;
    }
    console.log(n + " --> " + result);
}

function conversor(digit, letterSet) {
    switch(digit) {
        case "1":
            return letterSet[0];
            break;

        case "2":
            return letterSet[0] + letterSet[0];
            break;

        case "3":
            return letterSet[0] + letterSet[0] + letterSet[0];
            break;

        case "4":
            return letterSet[0] + letterSet[1];
            break;

        case "5":
            return letterSet[1];
            break;

        case "6":
            return letterSet[1] + letterSet[0];
            break;

        case "7":
            return letterSet[1] + letterSet[0] + letterSet[0];
            break;

        case "8":
            return letterSet[1] + letterSet[0] + letterSet[0] + letterSet[0];
            break;

        case "9":
            return letterSet[0] + letterSet[2];
            break;

        case "0":
            // skip character
            return "";
            break;

        default:
            break;
        }

}

toRomans(2);
toRomans(7);
toRomans(32);
toRomans(456)
toRomans(1243);
toRomans(3500);

Invalid input

toRomans(21463);

There we go. I broke your function. Sorry.

Even though the Roman Numerals Kata specifies that only numbers between 1 and 3000 needs to be returned correctly, I think it is better to handle numbers outside the working range in a specific way, such as throwing an exception.

Return, do not log

Imagine that I am writing a program and I want to use your method to count how many unique characters there is in the resulting string.

console.log(n + " --> " + result);

This doesn't let me do that, really. It would be much better to do:

return result;

And then I can log, show an alert, or count the number of unique characters after calling the function. This is how it is properly done in programming.

console.log(toRomans(56)); is more clear what the code is doing than simply toRomans(56);

Returning and breaking

In your switch statement, you don't need both return and break. When calling return, the method ends, so further statements will not be executed. Your breaks are dead code that will never be called.

I would also return ""; in the default case as well, which lets 0 be handled together with default.

function conversor(digit, letterSet) {
    switch(digit) {
        case "1":
            return letterSet[0];
        case "2":
            return letterSet[0] + letterSet[0];
        case "3":
            return letterSet[0] + letterSet[0] + letterSet[0];
        case "4":
            return letterSet[0] + letterSet[1];
        case "5":
            return letterSet[1];
        case "6":
            return letterSet[1] + letterSet[0];
        case "7":
            return letterSet[1] + letterSet[0] + letterSet[0];
        case "8":
            return letterSet[1] + letterSet[0] + letterSet[0] + letterSet[0];
        case "9":
            return letterSet[0] + letterSet[2];
        default:
            return "";
        }
}

I believe the conversor function can be further optimized, but I'll leave as-is for now.


Checking alert dialog validation I have set alert dialog on textview's onclicklistener, and there I have added some items using array adapter. Now what I want to do is check if the user selects an item from the alert dialog and clicks on submit. It should submit, otherwise not. But as per my if I select or not it displays message fill the details.

 boolean isSetDay = false;
 sp3=(TextView)findViewById(R.id.daydates);
 btnsubmit=(Button)findViewById(R.id.btnreg);

 final String[] items = new String[] {"01","02","03","04","05","06","07","08","09","10","11","12","13","14","15","16","17","18","19","20","21","22","23","24","25","26","27","28","29","30","31"};
 adapter123 = new ArrayAdapter<String>(this,
    android.R.layout.simple_spinner_dropdown_item, items);

 sp3.setOnClickListener(new OnClickListener() {

@Override
public void onClick(View w) {
      new AlertDialog.Builder(RegistrationForm.this)
      .setTitle("Select Day")
      .setAdapter(adapter123, new DialogInterface.OnClickListener() {

        @Override
        public void onClick(DialogInterface dialog, int which) {

            sp3.setText(adapter123.getItem(which).toString());
            isSetDay=true;

          dialog.dismiss();
        }
      }).create().show();
    }
  });
     btnsubmit.setOnClickListener(new OnClickListener()
  {

 @Override
 public void onClick(View v)
 {
    if(emailtext.getText().toString().trim().length() > 0 &&   passtext.getText().toString().trim().length() > 0 && confirmpass.getText().toString().trim().length() > 0 && username.getText().toString().trim().length() > 0 && firstname.getText().toString().trim().length() > 0 && lastname.getText().toString().trim().length() > 0 && isSetDay==true && isSetMonth==true &&    isSetYear==true && isSetPro==true)
{
isSetDay=false;
isSetMonth=false;
isSetYear=false;
isSetPro=false;
new AttemptLogin().execute();

}

  else
 {
    Toast.makeText(getApplicationContext(), "Fill the details", Toast.LENGTH_LONG).show();
 }
  break;
 }
 });

Please pay attention to formatting.
The code as you posted is hard to read,
and not pleasant to review.

Most notably, the indentation is inconsistent or sometimes random,
and you have very long horizontal lines forcing readers to scroll.
For example this one:

if(emailtext.getText().toString().trim().length() > 0 &&   passtext.getText().toString().trim().length() > 0 && confirmpass.getText().toString().trim().length() > 0 && username.getText().toString().trim().length() > 0 && firstname.getText().toString().trim().length() > 0 && lastname.getText().toString().trim().length() > 0 && isSetDay==true && isSetMonth==true &&    isSetYear==true && isSetPro==true)

Broken up to multiple lines it becomes a lot easier to read:

    if (emailtext.getText().toString().trim().length() > 0
            && passtext.getText().toString().trim().length() > 0
            && confirmpass.getText().toString().trim().length() > 0
            && username.getText().toString().trim().length() > 0
            && firstname.getText().toString().trim().length() > 0
            && lastname.getText().toString().trim().length() > 0
            && isSetDay == true
            && isSetMonth == true
            && isSetYear == true
            && isSetPro == true)

Note that using == true with boolean expressions is really pointless,
as you could use the expressions directly:
if (something) { ... } is exactly the same as
if (something == true) { ... }.
So you could drop all the == true from the above.

When checking if a string is empty, instead of str.length() > 0,
a better way is !str.isEmpty().

To avoid code duplication of .getText().toString().trim().isEmpty() it would be a good idea to add a helper function:

private boolean notEmpty(EditText input) {
    return !input.getText().toString().trim().isEmpty();
}

So that you can rewrite the condition in the if like this:

    if (notEmpty(emailtext)
            && notEmpty(passtext)
            && notEmpty(confirmpass)
            && notEmpty(username)
            && notEmpty(firstname)
            && notEmpty(lastname)
            && isSetDay
            && isSetMonth
            && isSetYear
            && isSetPro)

Even better, as @hkj suggested,
the notEmpty method could take varargs:

private boolean notEmpty(EditText ... args) {
    for (EditText input : args) {
        if (input.getText().toString().trim().isEmpty()) {
            return false;
        }
    }
    return true;
}

which will allow you to shorten the condition even more:

    if (notEmpty(emailtext, passtext, confirmpass, username, firstname, lastname)
            && isSetDay
            && isSetMonth
            && isSetYear
            && isSetPro)

This line is also too long, and also error-prone to type it:

final String[] items = new String[] {"01","02","03","04","05","06","07","08","09","10","11","12","13","14","15","16","17","18","19","20","21","22","23","24","25","26","27","28","29","30","31"};

It would be better to fill this array programmatically, for example:

    final String[] items = new String[31];
    for (int i = 0; i < items.length; ++i) {
        items[i] = String.format("%02d", i + 1);
    }


Traversing javascript/json based on a dot-separated lookup string I have an object of arbitrary depth, and a string (or array) containing the keys which have to be traversed to get to the value.

As far as I know, there's no method to get to the value directly:

var object = {
    user: {name: 'bob'},
    'user.name': 'error'
};

object['user.name']; // contains 'error'

And the usual:

object['user']['name'];

is not possible without doing some very dirty eval stuff, because you do not know how far you will need to traverse through the object beforehand.

So I think this is only solvable using a recursive function:

var object = {
    company: {
        users: [
            {name:'alice',age:'20'},
            {name:'bob',age:'22'},
            {name:'eve',age:'30'},
        ]
    }
};

lookup = function recurse(array,object) {
    var next = (array.length) ? object[array.shift()] : object;
    return (next instanceof Object && next[array[0]]) ? recurse(array,next) : next;
}

var search = 'company.users.0.name';
lookup(search.split('.'), object);

I think this works well enough, but would love some feedback.

Efficiency is important here, could I rewrite the function to shave of a few milliseconds somewhere? Are there any pitfalls, bugs or exceptional cases to consider?

The very first problem I see is:

next instanceof Object

That's because almost everything, if not everything, in JS is an instance of Object. Go try new Date() instanceof Object and even that is true. That said, you can do it this way but you must assume that it's just a structure containing objects and arrays. Otherwise, you'll have to check for every possible kind of object out there. This means dates, regexes, DOM objects, functions etc.

Another way to do this is to create a function dynamically. I've seen frameworks do this, especially when it deals with templates containing JS expressions. Instead of lugging around a full-blown expression parser, they let the browser's parser do it. The advantage of this method over eval is that you only evaluate the expression once. You can then cache the generated function in some object for reuse. You can even have it return undefined or null if the entire subtree is missing instead of getting an error.

function createResolver(keypath){
  return new Function('root', `
    try { return root.${keypath}; }
    catch(e){ return undefined; }
 `);
}

// Return foo.bar.baz
var obj1 = { foo: { bar: { baz: 'bam!!!' }}};
var resolver1 = createResolver('foo.bar.baz');
var value1 = resolver1(obj1);

// Even when the subtree is missing, you can safely call it
var obj2 = { foo: {}};
var resolver2 = createResolver('foo.bar.baz');
var value2 = resolver2(obj2);

// Normally, this throws an error:
var value3 = obj2.foo.bar.baz


Setting a request header with or without Context in Golang I have two ways (middleware) of setting a specific header (if not set) to request but want to know if one is better/beneficial than the other and the reasons why.

Note: At some point I will need to log X-Request-Id value in every single application log I have. If this plan affects your answer, I don't know. Just saying!

Context version

package middleware

import (
    "context"
    "github.com/google/uuid"
    "net/http"
)

type CtxKey string
const CtxReqIdKey CtxKey = "X-Request-Id"

func RequestId(handler http.Handler) http.Handler {
    return http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) {
        CtxKey := req.Header.Get("X-Request-Id")
        if CtxKey == "" {
            CtxKey = uuid.New().String()
        }

        ctx1 := context.WithValue(req.Context(), CtxReqIdKey, CtxKey)
        ctx2 := req.WithContext(ctx1)

        handler.ServeHTTP(res, ctx2)
    })
}

# Elsewhere: request.Context().Value(middleware.CtxReqIdKey)

Without context version

package middleware

import (
    "github.com/google/uuid"
    "net/http"
)

func RequestId(handler http.Handler) http.Handler {
    return http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) {
        if req.Header.Get("X-Request-Id") == "" {
            req.Header.Set("X-Request-Id", uuid.New().String())
        }

        handler.ServeHTTP(res, req)
    })
}

Usage

err := http.ListenAndServe(":8080", middleware.RequestId(Router))

Long form comment:  Yes, the context version.  I'd consider it a bad pattern to modify incoming data inline (because it's harder to track later what came in and what the application did with it).  Especially in applications which do not only handle HTTP traffic, but also, say, gRPC calls, it's even more important that this data isn't stored for one type of handler only, but for all of them in a unified fashion.  Plus, context is (for better and worse) a kitchen sink for all layers of the code, even in the deepest layers you'll still be able to get that ID for various purposes, maybe auditing, or so.

Maybe it doesn't even need to go into the context, simply add it to the logger data (if there's such a thing), or however else that's being populated.  Doing that in a wrapper like this will again allow you to keep it out of the rest of the application - problem solved and no need to mess with the context object.  In any case it's not difficult to change that later.