Find length of non-repeated numbers in array How can I optimize this code which can get the length of non-repeated numbers in an array?
package com.sujay.practice;

import java.util.HashMap;
import java.util.Map;

/**
 * Find the length of non-repeated numbers in an array
 * @author Sujay
 * 08-May-2018
 */
public class DuplicateValuesInArray {

    public static void main(String[] args) {
        int[] array = { 1, 2, 2, 3, 4, 5, 6, 2, 4, 5, 3 };
        usingMap(array);
        usingString(array);
    }

    private static void usingString(int[] array) {
        String str = "";
        for (int i = 0; i < array.length; i++) {
            String temp = String.valueOf(array[i]);
            if (!str.contains(temp)) {
                str = str + temp;
            }
        }
        System.out.println(str.length());
    }

    private static void usingMap(int[] array) {
        Map<Integer, Integer> map = new HashMap<Integer, Integer>();
        for (int i = 0; i < array.length; i++) {
            map.putIfAbsent(array[i], i);
        }
        System.out.println(map.size());
    }
}

The correct data structure to use to eliminate duplicates is a Set. The String processing is more expensive, and the Map code is less clear because you don't care at all about the values.
It's nice to provide a vararg parameter so that clients don't need to define an array if they don't want to.
It's a better practice to return your result from the method. If the client wants to print it, that's up to them. But if you print it, and the client changes what they want to do with it, they're out of luck.
You can used the enhanced for loop to make your code a little prettier.
More descriptive names are better than less descriptive names.

If I were to take a stab at writing your code with the above changes in mind, it might look something like:

private static int countUniqueValues(final int... values) {
    final Set<Integer> uniqueValues = new HashSet<>(values.length);
    for (final int value : values) {
        uniqueValues.add(value);
    }
    return uniqueValues.size();
}


Test if two IDictionary objects contain the same values I'm adding a function to my test library to assist in testing if two IDictionary objects contain the same keys/values.

I need the method to be generic and support a dictionary that has collections as values. So I thought I'd try to do it without using class types.

It appears to be working, but I would like to clarify that I didn't make any mistakes that would impact my tests later.

    /// <summary>
    /// Checks if two dictionaries contain the same values. Supports recursive
    /// dictionaries and collections as values.
    /// </summary>
    /// <param name="pExpect">Expected value</param>
    /// <param name="pActual">Actual value</param>
    // ReSharper disable CanBeReplacedWithTryCastAndCheckForNull
    public static void dictionary(IDictionary pExpect, IDictionary pActual)
    {
        Assert.IsNotNull(pExpect);
        Assert.IsNotNull(pActual);

        if (pExpect.Keys.Count != pActual.Keys.Count)
        {
            Assert.Fail("Expected {0} keys, but contains {1} keys.", pExpect.Keys.Count, pActual.Keys.Count);
        }

        object[] expectKeys = new object[pExpect.Keys.Count];
        object[] actualKeys = new object[pExpect.Keys.Count];

        pExpect.Keys.CopyTo(expectKeys, 0);
        pActual.Keys.CopyTo(actualKeys, 0);

        // check if the two key sets are the same
        CollectionAssert.AreEquivalent(expectKeys, actualKeys);

        for (int i = 0, c = expectKeys.Length; i < c; i++)
        {
            object expect = pExpect[expectKeys[i]];
            object actual = pActual[actualKeys[i]];

            // both can be null
            if (expect == null && actual == null)
            {
                continue;
            }

            // both must be assigned a value
            Assert.IsNotNull(expect);
            Assert.IsNotNull(actual);

            // must be same types
            Assert.AreEqual(expect.GetType(), actual.GetType());

            if (expect is IDictionary)
            {
                // support recursive dictionary checks
                dictionary((IDictionary)expect, (IDictionary)actual);
            }
            else if (expect is ICollection)
            {
                CollectionAssert.AreEquivalent((ICollection)expect, (ICollection)actual);
            }
            else
            {
                Assert.AreEqual(expect, actual);
            }
        }
    }

I am wondering if my last check Assert.AreEqual(expect, actual) will cover most remaining test cases.

EDIT: Fixed testing dictionaries that have keys in different orders.

Your code doesn't work, because items in a dictionary aren't ordered in any way.

Example code that fails your test:

dictionary(
    new Dictionary<int, int> { { 0, 0 }, { 1, 1 } },
    new Dictionary<int, int> { { 1, 1 }, { 0, 0 } });

Some more notes about your code:

public static void dictionary(IDictionary pExpect, IDictionary pActual)

dictionary is a bad name for this method, because it doesn't really explain what it's supposed to do. Something like DictionaryEqualityTest would be better.

Also, I would avoid using the non-generic IDictionary if possible, IDictionary<TKey, TValue> would be better (though it would complicate the recursion).


Extracting and analyzing data from SAP I developed a script for the company I work for in order to extract data from SAP (CJ74) and analyze the data.

One particular issue I am finding is that for any data sets which have over 1000 lines, it struggles to execute the section of code below, therefore I am searching for answers.

This code takes free hand text from cells in Column J into an array, it then splits the text up and tries to find matches to first and last names located in Column F and G.

This works functionally but it runs like a slug with data sets over 1000 lines, and potentially crashes the program if there are data sets which are at 3000 - 4000 lines and above.  Why would this be the case? Have I coded this inefficiently or is this one of VBA's gotchas?  If so would there be a better way to write this code?

Option Compare Text
Option Explicit

Sub Loader()
Dim I As Long, J As Long
Dim T As Variant
Dim match_txt As String

'** Takes text from Column J and wraps it into an array**

 For I = 2 To Range("E50000").End(xlUp).row
    typ = Range("F" & I).Value
    If typ = "" Then
        txt = Range("J" & I).Value

 '** Split array into separate lines **

        txtArray = Split(txt, " ")

 '** Check Column F for matches, if it finds a match, put the matching text into column F under the assumption that it is a first name **

        For Each T In txtArray
            For J = 2 To Range("F50000").End(xlUp).row
                If Range("F" & J).Value = T Then
                    match_txt = T
                    Range("F" & I).Value = match_txt
                End If
            Next J
        Next T

 '** Check Column G for matches, if it finds a match, put the matching text into column G under the assumption that it is a last name **

        For Each T In txtArray
            For J = 2 To Range("F50000").End(xlUp).row
                If Range("G" & J).Value = T Then
                    match_txt = T
                    Range("G" & I).Value = match_txt
                End If
            Next J
        Next T
    End If
Next I
End Sub()

You can move the range values first to the array before doing your loops.
You can try below:

Dim tbArray, fcArray, gcArray ' Variant type
Dim elr As Long, flr As Long, glr As Long, i As Long
Dim T

With Sheets("YourSheetName") ' always be explicit when working with objects
    elr = .Range("E" & .Rows.Count).End(xlUp).Row
    flr = .Range("F" & .Rows.Count).End(xlUp).Row
    'glr = .Range("G" & .Rows.Count).End(xlUp).Row
    tbArray = .Range("F2:J" & elr) ' pass Range values, arr becomes 2D array
    fcArray = .Range("F2:F" & flr) ' creates horizontal 2D array but regardless
    gcArray = .Range("G2:G" & flr) ' this will be used for matching only

    For i = LBound(tbArray, 1) To UBound(tbArray, 1)
        If Len(tbArray(i, 1)) = 0 Then ' you can use tbArray(i, 1) = "", just preference
            For Each T In Split(tbArray(i, 5), " ")
                If Not IsError(Application.Match(T, fcArray, 0)) Then
                    tbArray(i, 1) = T ' corresponds to F
                ElseIf Not IsError(Application.Match(T, gcArray, 0)) Then
                    tbArray(i, 2) = T ' corresponds to G
                End If
            Next
        End If
    Next
    .Range("F2:J" & elr) = tbArray ' return the array to range
End With

This is compiled but not tested. We took advantage of using the Match Function.
You based your last rows to Columns E, F and G respectively so I just mimicked that.
No idea on how much data it can handle, but a 10K wouldn't hurt I guess.


Find How Many Times Numbers Repeat on a Digital Clock Project Overview:

See how many times numbers would repeat x times on a digital clock output. This particular code only counts from 12:00 to 11:59, so it's technically only half of a day, I could take the (output x 2) for a full day.

Functions:

input_io():
Input the number of times you would like to see if a number repeats (for example 11:11 will have the number "1" repeated four times. 12:11 would have 3 repeated "1"s, etc.)

gen_times():
Generate a list of all the "digital clock" outputs starting from 12:00 to 11:59 - including leading zeros for minutes (e.g. 12:04 would need the zero before the "4").

count_repeat():
Test each "clock output" (from gen_times()) to see how many times the numbers 0-9 repeat in each one. If it is greater than the input time add one to the counter.

Questions:

In the code, you'll see that I return values from functions and immediately assign them to variables. I'm not sure the proper conventions of this in Python. Are there ways to pass-through variables from function to function - is it reasonable to assign variables outside of functions to function outputs?
Is there a better way to run this program without all of the coercion? The main use case of the coercion is for the leading zeros when generating times.

Code:

def input_io():
    input_repeats = input("Please input the amount of times you'd like to see if numbers repeat")

    while True:
        try:
            input_repeats = int(input_repeats)
            break
        except:
            print("Please enter an integer.")
            input_repeats = input()
    return input_repeats

input_repeats = input_io()

def gen_times():

    total_minutes = (24 * 60) / 2
    print(f"Total Minutes {total_minutes}")
    times = ["0"] * int(total_minutes)
    hours = "12"
    minutes = "00"
    i = 0

    while (i < total_minutes):

        if(hours==13):
            hours=1

        times[i] = str(hours)+str(minutes)
        #print(int(minutes))

        if(minutes == "59"):
            minutes = "00"
            hours = int(hours) + 1

            #ADDING "0" IN FRONT OF HOURS
            #if(int(hours) >= 10):
            #    hours = hours
            #else:
            #    hours = "0"+str(hours)

        else:
            minutes = str(int(minutes)+1)

            if (int(minutes) >= 10):
                minutes = minutes
            else:
                minutes = "0" + str(minutes)

        i += 1
    return times

times = gen_times()
print(times)

def count_repeat():
    counter_repeat = 0
    i=0
    ii=0
    iterationcounter = 0

    while(i != len(times)):

        ii=0

        while(ii < 10):
            count = times[i].count(str(ii))
            #iterationcounter+=1
            if count >= int(input_repeats):
                counter_repeat+=1
                break

            ii+=1
        i+=1
    return counter_repeat

counter_repeat = count_repeat()

print(f"Number of Times w/ {input_repeats} repeats: {counter_repeat}")
print(counter_repeat,"/",len(times))
percent_repeat = (counter_repeat/len(times))
print("{:.2%}".format(percent_repeat))
#print(iterationcounter)

Input loop

Your

        print("Please enter an integer.")

can probably be lumped into the input statement after it.

Time math

Usually doing your own should be avoided. As an example, this:

total_minutes = (24 * 60) / 2

can be

from datetime import timedelta
# ...
total_minutes = timedelta(hours=12) / timedelta(minutes=1)

General approach

Currently you're doing a lot of string manipulation, including a stringly-typed hours and minutes in gen_times. Your code would be simpler if, instead of maintaining and incrementing those strings, increment a single datetime.time instance. From there, to get individual digits, you could  strftime it to a string and feed that to a Counter. The strftime format string in this case would be most useful as '%I%M'.

Variable names

Try to avoid names like i and ii. In particular, this:

    while(ii < 10):
        count = times[i].count(str(ii))
        #iterationcounter+=1
        if count >= int(input_repeats):
            counter_repeat+=1
            break

        ii+=1

is better represented as

for digit in range(10):
    count = times[i].count(str(digit))
    if count >= int(input_repeats):
        counter_repeat+=1
        break

Though that times[i].count should be replaced with the use of a Counter.


Faster solution for row-wise matrix subtraction I have 2 matrices. I have to calculate the euclidean distance between every row of matrix A and every row of matrix B.

In the first solution I loop over the rows of the two matrices and for each row in matrix A, I take a row from B, take the square of the element wise subtraction, then sum it and take the square root.

I end up with a matrix of the size #rows in A x #rows in B:

import numpy as np

A = np.random.random((50, 3072))
B = np.random.random((500, 3072))

# First solution
d = np.zeros((50, 500))
for i in range(50):
    for j in range (500):
        d[i,j] = np.sqrt(np.sum(np.square(B[j] - A[i])))

In the second solution I did this by broadcasting, and it works. But when I increase the amount of rows in A and B ... it becomes very very slow. Is there a faster way without looping?

Solution 2:

# Second solution
test = np.sqrt(np.sum(np.square(A[:,np.newaxis]-B),axis=2))

#print check
print np.unique(test==d)

It's not pretty, but this gives a factor-3 speed improvement:

d = (A**2).sum(axis=-1)[:, np.newaxis] + (B**2).sum(axis=-1)
d -= 2 * np.squeeze(A.dot(B[..., np.newaxis]), axis=-1)
d **= 0.5

This is based off of the fact

$$
(a - b)^2 = a^2 + b^2 - 2ab
$$

and so, ignoring the fudging with indices,

$$
\sum(a - b)^2 = \sum a^2 + \sum b^2 - 2\sum ab
$$

The squared terms are just

(A**2).sum(axis=-1)[:, np.newaxis] + (B**2).sum(axis=-1)

and \$\sum ab = \vec A \cdot \vec B\$. This can be broadcast with a bit of fudging the axes:

np.squeeze(A.dot(B[..., np.newaxis]), axis=-1)


Optimizing O(m n) solution for longest common subsequence challenge Given two strings string X of length x1 and string Y of length y1, find the longest sequence of characters that appear left to right (but not necessarily in contiguous block) in both strings.

  e.g. if X = ABCBDAB and Y = BDCABA, the LCS(X,Y) = {"BCBA","BDAB","BCAB"} and LCSlength is 4.

I used the standard solution for this problem, whichever is greater:

if(X[i]=Y[j]) :1+LCS(i+1,j+1)
if(X[i]!=Y[j]) :LCS(i,j+1)
LCS(i+1,j)

and then I used memorization, making it a standard DP problem.

#include<iostream>
#include<string>
using namespace std;
int LCS[1024][1024];
int LCSlen(string &x,int x1,string &y,int y1){
    for(int i = 0; i <= x1; i++)
    LCS[i][y1]=0;

    for(int j=0;j<=y1;j++)
    LCS[x1][j]=0;

    for(int i = x1 - 1; i >= 0; i--){
    for(int j = y1 - 1; j >= 0; j--){

    LCS[i][j] = LCS[i+1][j+1];

    if(x[i] == y[j])
    LCS[i][j]++;

    if(LCS[i][j+1]>LCS[i][j])
    LCS[i][j]=LCS[i][j+1];

    if(LCS[i+1][j]>LCS[i][j])
    LCS[i][j]=LCS[i+1][j];

    }
    }
return LCS[0][0];
}

int main()
{
string x;
string y;
cin >> x >> y;
int x1 = x.length() , y1 = y.length();
int ans = LCSlen( x, x1, y, y1);
cout << ans << endl;
return 0;
}

Running here, this solution I used in SPOJ and I got a time limit exceeded and/or runtime error.

Only 14 solutions are yet accepted.  Is there a smarter trick to decrease the time complexity of this question?

The algorithm is correct, but the implementation is sort of naive; it definitely doesn't scale to 50000-long strings. Even for the 1024-long strings it is very cache-unfriendly; most likely you spend most of the time in cache misses.

The wiki article mentions that

  dynamic programming approach only needs the current and previous
  columns of the matrix

Modify your code to run in a linear space. That shall give you a boost.


Process POSTed data and add default string if missing My code takes the input submitted by $ _POST, via a checkbox array. I process the array and check the sent data if there is a particular string in it, which is set as my default. If I have a string, which is my default, I just go through implode and add ",", if my default string is missing in the data. I add and sort the array anew, with the first string being the default. I want to shorten the code because I can't make it shorter.
<?php
if(isset($_POST['submit'])) {
    //for update
    $site_id['locale'] = 'tr';

    //new
    $exampleArray = isset($_POST['lala']) ? $_POST['lala'] : '';

    $example = null;
    if(($key = array_search($site_id['locale'], $exampleArray, true)) !== false) {
        unset($exampleArray[$key]);

        $FirstString = array($site_id['locale']);
        $exampleArray = array_diff($exampleArray, $FirstString);
        usort($exampleArray);
        $exampleArray = array_merge($FirstString, $exampleArray);
        //print_r($exampleArray);
        $myArray = array();
        foreach ( $exampleArray as $key => $value ) {
            $myArray[] = $value;
        }
        echo implode( ', ', $myArray );
    } else {
        $FirstString = array($site_id['locale']);
        $exampleArray = array_diff($exampleArray, $FirstString);
        usort($exampleArray);
        $exampleArray = array_merge($FirstString, $exampleArray);
        //print_r($exampleArray);
        $myArray = array();
        foreach ( $exampleArray as $key => $value ) {
            $myArray[] = $value;
        }
        echo implode( ', ', $myArray );
    }

}
?>
<form method="POST" action="">
<input type="checkbox" name="lala[]" value="bg" />
<input type="checkbox" name="lala[]" value="us" />
<input type="checkbox" name="lala[]" value="gr" />
<input type="submit" name="submit" />
</form>

Addressing two issues here:
Setting a default value
PHP now has a null coalesce operator (??).
This allows you to easily set a default value if the variable is null.
$exampleArray = $_POST['lala'] ?? [];

(Here, I am setting the default value to an empty array)
Refactoring code
Any time you catch yourself copy/pasting a chunk of code, that’s a signal to start refactoring.  So the code you provided had almost everything in the if and else block repeated.  Just pull the repeated stuff out. In this case, you don’t even need the else.

if(isset($_POST['submit'])) {
    //for update
    $site_id['locale'] = 'tr';

    //new
    $exampleArray = $_POST['lala']) ?? [];

    $example = null;
    if(($key = array_search($site_id['locale'], $exampleArray, true)) !== false) {
        unset($exampleArray[$key]);
    }

    $FirstString = array($site_id['locale']);
    $exampleArray = array_diff($exampleArray, $FirstString);
    usort($exampleArray);
    $exampleArray = array_merge($FirstString, $exampleArray);
    //print_r($exampleArray);
    $myArray = array();
    foreach ( $exampleArray as $key => $value ) {
        $myArray[] = $value;
    }
    echo implode( ', ', $myArray );

    // Note, should always redirect after a post.
    // Post means change data, so after you have
    // stored the changes, redirect to the next (or
    // back to same) page. Cannot be any output
    // before this (print, echo, blank lines, html)

}
```


Validating email addresses through temporary mail message I was given this project and have changed a lot of code on it, now I am going over the code to make sure that I know what everything does and can maintain it efficiently, and to make sure I want my name on it.

Here is the method they have been using to validate e-mail addresses from a textbox input.

private string ValidateEmailAddresses(string addressesIn)
{
    MailMessage msg = new MailMessage();

    if (!String.IsNullOrWhiteSpace(addressesIn))
    {
        string[] allAddresses = addressesIn.Split(";,".ToCharArray(), StringSplitOptions.RemoveEmptyEntries);

        foreach (string addy in allAddresses)
        {
            try
            {
                msg.To.Add(addy);
            }
            catch (FormatException)
            { }
        }
    }
    return msg.To.ToString();
}

Is there a better way to accomplish this task?

Well, it really depends on why addressesIn is a string. If it's input from the user, as I suspect, it is a fine method.
If I was to be a harsh critic I would ask why a method named "Validate" returns a string.
I think if you rename the method to like "ScrubEmailAddresses" or "SanitizeEmailAddresses" and envision it in an app that lets a user input email addresses to send an email to, it's kind of nifty. Basically instead of a roll-your-own-validations approach this says "hey, let the framework do it for me!" by taking advantage of the fact that .NET will just throw a FormatException if you try to add an invalid address. It's a fairly decent server-side equivalent to a RegularExpressionValidator using the predefined email regex. I also agree with Mike McCaughan's comment mentioning there is a resource hit when having the framework throw an exception.


Printing a nested data structure in Clojure I wrote the following code which transforms a nested data structure

(def board [[{:mine true}             {:warn 1 :explored true} {:explored true}]
           [{:warn 1 :explored true}  {:warn 1 :explored true} {}              ]
           [{}                        {}                       {:flag true }   ]])

into a printable form and prints it

(("[ ]" "[1]" "[E]") ("[1]" "[1]" "[ ]") ("[ ]" "[ ]" "[F]"))

The functions for the transformation are the following:

(defn cell->icon
  [cell]
  (letfn [(cell->str [v] (format "[%s]" v))]
    (if (:explored cell)
      (cond (:mine cell) (cell->str "M")
            (:warn cell) (cell->str (:warn cell))
            :else        (cell->str "E"))
      (cond (:flag cell) (cell->str "F")
            :else        (cell->str " ")))))

(defn board->icons
  [board]
  (map (partial map cell->icon) board))

So far so good (if you have any recommendations for a nicer approach though, do not hesitate to mention).

The function which I don't like though is the following:

(defn print-board
  [board]
  (doall
    (map println
       (map (partial clojure.string/join " ")
            (board->icons board)))))

I don't like to use println together with map since it is not a pure function (has side-effects)!? Maybe I am a bit too critical but I would be glad if somebody could advice me or give a hint how to do it in a nicer Clojure like way.

Separate impure IO from data transformations

  I wrote the following code which transforms a nested data structure ... into a printable form

Not only does your code transform the data into a printable form, it also prints it.

To better isolate the side effects of printing, you should separate these two operations: first assemble a formatted string, then you can do whatever you want with it -- print it with a single call to println, save it, send it over the network, etc.

(defn format-board
  [board]
  (->> (board->icons board)
       (map (partial str/join " "))
       (str/join "\n")))

(def print-board
  (comp println format-board))

doseq

As a side note, instead of (doall (map println seq-to-print)) it is common to see

(doseq [line seq-to-print]
  (println line))

which is usually what you want, since

doseq does not hold onto the return value of println, whereas doall builds up an entire return sequence in memory (important when the sequence is big!) -- which is why your original print-board returns (nil nil nil) instead of nil; and
this makes it more visually clear that you are doing something with each item in the sequence, instead of generating a new sequence with map


Implementing equals(), hashCode(), toString() in the hierarchy like the Optional from Guava I have simple hierarchy which looks [a little bit] like the Optional<?> from Guava:

public abstract class Result<T> {

    public static <ResultType> Result<ResultType> get(ResultType result) {
        return new PresentResult(result);
    }

    public static Result<?> absent(Throwable exception) {
        return new AbsentResult(exception);
    }

    Result() {
        // clients of the package can't extend this class
    }

    public abstract boolean isPresent();
    public abstract T get();
    public abstract Throwable getException();
}

class PresentResult<T> extends Result<T> {

    private final T result;

    public PresentResult(T result) {
        this.result = result;
    }

    @Override
    public boolean isPresent() {
        return true;
    }

    @Override
    public T get() {
        return result;
    }

    @Override
    public Throwable getException() {
        throw new UnsupportedOperationException();
    }

    @Override
    public boolean equals(Object obj) {
        if (obj == this) {
            return true;
        }
        if (!(obj instanceof PresentResult)) {
            return false;
        }
        PresentResult<?> other = (PresentResult<?>) obj;
        return Objects.equal(get(), other.get());
    }

    @Override
    public int hashCode() {
        return Objects.hashCode(get());
    }

    @Override
    public String toString() {
        return Objects.toStringHelper(this)
                .add("result", get())
                .toString();
    }
}

class AbsentResult extends Result {

    private final Throwable exception;

    public AbsentResult(Throwable exception) {
        this.exception = exception;
    }

    @Override
    public boolean isPresent() {
        return false;
    }

    @Override
    public Object get() {
        throw new UnsupportedOperationException();
    }

    @Override
    public Throwable getException() {
        return exception;
    }

    @Override
    public boolean equals(Object obj) {
        if (obj == this) {
            return true;
        }
        if (!(obj instanceof AbsentResult)) {
            return false;
        }
        AbsentResult other = (AbsentResult) obj;
        return Objects.equal(getException(), other.getException());
    }

    @Override
    public int hashCode() {
        return Objects.hashCode(getException());
    }

    @Override
    public String toString() {
        return Objects.toStringHelper(this)
                .add("exception", getException())
                .toString();
    }
}

But I'm not sure whether I implemented the methods equals(), hashCode(), toString() correctly.

What do you think about them?

Some remarks:

Why does getException() return a Throwable instead of an Exception? Or even better, a RuntimeException? More generally, what is the purpose of this method? If I were you I'd "drop" it and make .get() throw the wanted exception instead. Since the exception would be a RuntimeException, there would be no need to declare it.
Is it on purpose that PresentResult and AbsentResult are not final? What do you expect further subclasses to extend if anything?
Is it legal to initialize a PresentResult with a null value, an AbsentResult with a null exception?


Collecting users' data as an array I want to collect my users' data as an array:

h = {}
a = [["user_3", 765], ["user_1", 2], ["user_1", 1], ["user_2", 124],["user_1", 3], ["user_2", 223], ["user_2", 334]]
# Expected output :: {"user_3"=>[765], "user_1"=>[2, 1, 3], "user_2"=>[124, 223, 334]}
a.each do |x|
  if h.keys.include?(x.first)
    h[x.first] = (h[x.first] << x.last)
  else
    h[x.first] = [x.last]
  end
end

Everything is working fine in my Ruby script, but is there any better way to achieve my result?

You very rarely need to modify a variable from inside an each block.

Instead, check out all the methods available to you in Array and Enumerable.

Among them, there's #each_with_objects, which you can use like so:

a.each_with_object({}) do |(name, id), hash|
  hash[name] ||= []
  hash[name] << id
end

which'll produce the same hash as your current code.

You could also do:

a.group_by(&:first).map do |name, pairs|
  [name, pairs.map(&:last)]
end

That returns an array of arrays, but you can turn that into a Hash with Hash[]. You may not need to, though. A lot of things treat an Array object with [key, value]-style elements the same as a Hash object. Depends on your needs.


Excel VBA - in a list, if cell equals specific value then I started working on some code, and it works, but I feel like it could be done more effiently. Below is a portion of it to show what I'm doing. To simplify the idea, I've made it here so if the cell in column M is A, B, or C, it puts a 1 in column L. If column M is a D, E, or F, it puts a 2 in column L. And if column M is a G, H, or I, it puts a 3 in column L.

Is there an easier way to do this than how I'm doing it? I'm going to be dealing with a couple hundred possible values. Alex P suggested I build a reference table. I've never done that before, so I'm not sure if that's my best bet or not.

Sub ChangeTest()
    Dim LastRow As Long
    Dim i As Long
    LastRow = Range("M" & Rows.Count).End(xlUp).Row
    For i = 2 To LastRow
        If Range("M" & i).Value = "A" Or Range("M" & i).Value = "B" Or Range("M" & i).Value = "C" Then
        Range("L" & i).Value = "1"
        End If
        If Range("M" & i).Value = "D" Or Range("M" & i).Value = "E" Or Range("M" & i).Value = "F" Then
        Range("L" & i).Value = "2"
        End If
        If Range("M" & i).Value = "G" Or Range("M" & i).Value = "H" Or Range("M" & i).Value = "I" Then
        Range("L" & i).Value = "3"
        End If
    Next i
End Sub

Don't know if it would be faster, but it would look cleaner to use a Select.

Sub ChangeTest()
    Dim LastRow As Long
    Dim i As Long
      LastRow = Range("M" & Rows.Count).End(xlUp).Row
      For i = 2 To LastRow
        Select Case Range("M" & i)
          Case "A", "B", "C"
            Range("L" & i) = "1"
          Case "D","E","F"
            Range("L" & i) = "2"
          Case "G","H","I"
            Range("L" & i) = "3"
        End Select
      Next i
End Sub


Implement jquery like functions addClass, removeClass and delay in Javascript Assume $ is not the browser. Now have to implement $, it will take a string, which is a query, it will use the querySelector to select the element. (Reference: https://developer.mozilla.org/en-US/docs/Web/API/Document/querySelector)

$('text')

Now implement jquery like functions addClass and removeClass. (Reference: https://developer.mozilla.org/en-US/docs/Web/API/Element/classList)

$('#test').removeClass('blue').addClass('red');

Now implement jquery like functions delay. (Reference: https://api.jquery.com/delay/)

$('#test').removeClass('blue').delay(2000).delay(1000).addClass('red');

Code Sample

function $(selector) {
    let element = document.querySelector(selector)
    element.queue = []
    element.active = false
    return element
}

Element.prototype.next = function () {
    if (this.queue.length) this.runTask(this.queue.shift())
}

Element.prototype.runTask = function (callBack) {
    this.active = true
    callBack().then(() => {
        this.active = false
        this.next()
    })
}

Element.prototype.register = function(callBack){
    if (this.active) {
        this.queue.push(callBack)
    } else {
        this.runTask(callBack)
    }
}

Element.prototype.addClass = function (className) {
    that = this
    let callBack = () => new Promise(resolve => setTimeout(function () {
        that.classList.add(className)
        resolve()
    }, 0))
    this.register(callBack)
    return this
}

Element.prototype.removeClass = function (className) {
    that = this
    let callBack = () => new Promise(resolve => setTimeout(function () {
        that.classList.remove(className)
        resolve()
    }, 0))
    this.register(callBack)
    return this
}

Element.prototype.delay = function (ms) {
    that = this
    let callBack = () => new Promise(resolve => setTimeout(function () {
        resolve()
    }, ms))

    this.register(callBack)
    return this
}

$('#test')
.removeClass("red").delay(500)
.addClass("blue").delay(500).delay(500).removeClass("blue")
.delay(500).addClass("red").delay(500).removeClass("red")
.delay(500).addClass("blue").delay(500).removeClass("blue")
.delay(500).addClass("red").delay(500).removeClass("red")
<!DOCTYPE html>

Can anyone please review the Code?

Mutating built-in prototypes is not a good idea. It's not only inelegant, it can also lead to conflicts when other scripts on the page expect the prototypes to be unmutated. (It can even damage future attempts to integrate functionality officially, if enough sites use the bad code.)

Instead of using Element.prototype, create your own class, and put methods onto its prototype.

If you want to emulate jQuery, you should probably select all elements that match the selector, not just the first one. (Using a collection of elements will also avoid errors if the collection is empty - jQuery doesn't error when methods are called on an empty collection)

The active flag would be more informative if it was named more precisely, perhaps call it methodInProgress.

that = this is an antipattern in modern Javascript. If you need to use the calling context from the outer scope, use arrow functions instead. (You're already using ES2015 in multiple places)

callback is a single word, not two, so the proper way to camelCase it would be to keep it as-is. Calling the arguments callBack instead could result in bugs later when other readers/writers of the code expect it to be formatted conventionally.

Promises should be reserved for asynchronous actions. If you're going to run something that's completely synchronous, using a Promise adds unnecessary and confusing noise. Consider having your functions return Promises only if they're asynchronous, and while processing the queue, if a callback returns a Promise, wait for the Promise to resolve before moving onto the next callback. If the callback doesn't return a Promise, you can still await it without errors, and the next queue callback will run immediately.

Since you have multiple methods which add callback to the queue and return this, consider passing those methods through a helper function to make the code a bit more DRY.

Refactored:

const $ = selector => new PsuedoJquery(selector);
class PsuedoJquery {
  constructor(selector) {
    this.elements = [...document.querySelectorAll(selector)];
    this.queue = [];
  }
  async runNext() {
    await this.queue.shift()();
    if (this.queue.length) {
      await this.runNext();
    } else {
      this.methodInProgress = false;
    }
  }
  register(callback) {
    this.queue.push(callback);
    // If errors are a possibility, catch them here
    if (!this.methodInProgress) {
      this.methodInProgress = true;
      this.runNext();
    }
  }
  makeChainable(callback) {
    this.register(callback);
    return this;
  }
  addClass(className) {
    return this.makeChainable(() => {
      for (const elm of this.elements) {
        elm.classList.add(className);
      }
    });
  }
  removeClass(className) {
    return this.makeChainable(() => {
      for (const elm of this.elements) {
        elm.classList.remove(className);
      }
    });
  }
  delay(ms) {
    return this.makeChainable(() => new Promise((resolve) => {
      setTimeout(resolve, ms);
    }));
  }
}

$('#test')
  .removeClass("red").delay(500)
  .addClass("blue").delay(500).delay(500).removeClass("blue")
  .delay(500).addClass("red").delay(500).removeClass("red")
  .delay(500).addClass("blue").delay(500).removeClass("blue")
  .delay(500).addClass("red").delay(500).removeClass("red")
.blue {
  background-color: blue;
}

.red {
  background-color: red;
}


Count of request per second, minute in java I want to count number of requests per second, minute. I want do it fast and thread-safe. I have 2 variables to count:
private AtomicInteger count;
private AtomicInteger minutesCount;

And 2 variables for last second and minute:
private AtomicInteger currentSecond;
private AtomicInteger currentMinute;

Constructor of my service:
public StatisticServiceImpl() {
   final Calendar calendar = Calendar.getInstance();

   this.currentSecond = new AtomicInteger(calendar.get(Calendar.SECOND));
   this.currentMinute = new AtomicInteger(calendar.get(Calendar.MINUTE));

   this.count = new AtomicInteger();
   this.minutesCount = new AtomicInteger();
}

And my method to count:
public void count() {
   final Calendar calendar = Calendar.getInstance();
   int newSecond = calendar.get(Calendar.SECOND);
   int newMinute = calendar.get(Calendar.MINUTE);

   currentSecond.getAndUpdate(prev -> {
      //if it is new second, then clear second count and set new current second
      if (newSecond != prev) {
         count = new AtomicInteger();
         return newSecond;
      } else {
         count.incrementAndGet();
         return prev;
      }
   });

   currentMinute.getAndUpdate(prev -> {
      //if it is new minute, then clear minute count and set new current minute
      if (newMinute != prev) {
         minutesCount = new AtomicInteger();
         return newMinute;
      } else {
         minutesCount.incrementAndGet();
         return prev;
      }
   });
}

So, is it better (in a sense of performance and accuracy) then solution from this post?
(use LinkedList)
Or suggest better solution for this, please.

The first thing I noticed.
The calendar in the constructor. If you wanted to test this class it would be really difficult because you
are dependent on the Calendar implementation, and you have no control over the value it will return. It would be better to inject the calendar in to the
constructor, or Abstract the desired functionality in to an interface and inject that. This will also allow you to use one instance for your entire application.
instead of constructing a separate one in the count method.
injecting the calendar:
public StatisticService(final Calendar calendar) {
        this.calender = calendar;
        this.currentSecond = new AtomicInteger(calendar.get(Calendar.SECOND));
        this.currentMinute = new AtomicInteger(calendar.get(Calendar.MINUTE));
        this.count = new AtomicInteger();
        this.minutesCount = new AtomicInteger();
    }

Creating an interface:
Using an interface gives you flexibility around the implementation of the counter. If in the future you want to change from using
a Calendar to something else only the code implementing the interface will change and classes dependent on it will be unchanged.
public interface TimeProviderService {
    Integer getSecond();

    Integer getMinute();
}

Second
It is a lot more expensive to create a new AtomicInteger class then to just set it to zero.
So here instead of constructing a new class just reset the integer.
//if it is new second, then clear second count and set new current second
            if (newSecond != prev) {
                count.set(0);
                return newSecond;
            } else {
                count.incrementAndGet();
                return prev;
            }

Third suggestions for locking.
Atomic classes are great but in this instance since you are wanting the entire count method to
be atomic I would suggest using your own lock. It simplifies the code for people trying to read it later and the entire
method would be synchronized instead of just when you're accessing the atomic integers.
Also, locking inside of a lock is asking for a deadlock. See Here.
currentMinute.getAndUpdate(prev -> {
            ...
                minutesCount.incrementAndGet();
            ...
        });

Finally
Here is what the class would look like if you implemented the suggestions I am proposing.
Time Providers
public interface TimeProviderService {
    Integer getSecond();

    Integer getMinute();
}

public class CalendarTimeProviderService implements TimeProviderService {
    private final Calendar calendar;

    public CalendarTimeProviderService(Calendar calendar) {
        this.calendar = calendar;
    }

    @Override
    public Integer getSecond() {
        return this.calendar.get(Calendar.SECOND);
    }

    @Override
    public Integer getMinute() {
        return this.calendar.get(Calendar.MINUTE);
    }
}

Statistic Service
public class StatisticService {
    private final TimeProviderService timeProviderService;
    private final Lock lock = new ReentrantLock();
    private int currentSecond;
    private int currentMinute;
    private int count;
    private int minutesCount;

    public StatisticService(TimeProviderService timeProviderService) {
        this.timeProviderService = timeProviderService;

        this.currentSecond = this.timeProviderService.getSecond();
        this.currentMinute = this.timeProviderService.getMinute();

        this.count = 0;
        this.minutesCount = 0;
    }

    public void count() {
        this.lock.lock();

        try {
            int newSecond = this.timeProviderService.getSecond();
            int newMinute = this.timeProviderService.getMinute();

            // seconds
            if (newSecond != currentSecond) {
                this.count = 0;
                this.currentSecond = newSecond;
            } else {
                this.count = this.count + 1;
            }

            // minutes
            if (newMinute != currentMinute) {
                this.minutesCount = 0;
                this.currentMinute = newMinute;
            } else {
                this.minutesCount = this.minutesCount + 1;
            }

        } finally {
            // make sure we are unlocking no matter what happens
            this.lock.unlock();
        }
    }
}

one final note
I did not change anything about how you were calculating the time I just changed the way you are doing it. I do not the problem you
are trying to solve, so I cannot speak on the best way.


Detect prefered language I am using this script to detect the most prefered language by the user.

function Get_Client_Prefered_Language ($getSortedList = false, $acceptedLanguages = false)
{

    if (empty($acceptedLanguages))
        $acceptedLanguages = $_SERVER["HTTP_ACCEPT_LANGUAGE"];

    preg_match_all('/([a-z]{1,8}(-[a-z]{1,8})*)\s*(;\s*q\s*=\s*(1|0\.[0-9]+))?/i', $acceptedLanguages, $lang_parse);
    $langs = $lang_parse[1];
    $ranks = $lang_parse[4];

        // (create an associative array 'language' => 'preference')
    $lang2pref = array();
    for($i=0; $i<count($langs); $i++)
        $lang2pref[$langs[$i]] = (float) (!empty($ranks[$i]) ? $ranks[$i] : 1);

        // (comparison function for uksort)
    $cmpLangs = function ($a, $b) use ($lang2pref) {
        if ($lang2pref[$a] > $lang2pref[$b])
            return -1;
        elseif ($lang2pref[$a] < $lang2pref[$b])
            return 1;
        elseif (strlen($a) > strlen($b))
            return -1;
        elseif (strlen($a) < strlen($b))
            return 1;
        else
            return 0;
    };

        // sort the languages by prefered language and by the most specific region
    uksort($lang2pref, $cmpLangs);

    if ($getSortedList)
        return $lang2pref;

        // return the first value's key
    reset($lang2pref);
    return key($lang2pref);
}

Example:

print_r(Get_Client_Prefered_Language(true, 'en,en-US,en-AU;q=0.8,fr;q=0.6,en-GB;q=0.4'));

Output:

Array
    (
        [en-US] => 1
        [en] => 1
        [en-AU] => 0.8
        [fr] => 0.6
        [en-GB] => 0.4
    )

Is this reliable? How about performance, this script will get called many times per seconds.

The following is imho easier to read and should give you best performance.

<?php

/**
 * Get a sorted list of client accepted languages.
 *
 * @staticvar array $examined
 *   Used to cache the sorted list for <var>$accept_languages</var> strings.
 * @param boolean $list [optional]
 *   Whether to get a sorted list or the language with the highest preference, defaults to FALSE and
 *   the language with the highest preference will be returned.
 * @param string $accept_languages [optional]
 *   The client submitted accept languages string in the format defined in RFC 2616.
 * @param string $default [optional]
 *   The default ISO 639-1 alpha-2 language code to use if no other language could be determined, defaults to
 *   "en".
 * @return array|string
 *   Associative array where the key is the language code or locale and the value the preference if <var>$list</var>
 *   is set to TRUE, otherwise the ISO 639-1 alpha-2 code or locale of the language with the highest
 *   preference.
 */
function get_client_preferred_language($list = false, $accept_languages = null, $default = "en") {
  static $examined = array();

  // Either use the supplied string or use the string from server input, this construct ensures highest performance
  // because a positive check is performed.
  $accept_languages || ($accept_languages = filter_input(INPUT_SERVER, "HTTP_ACCEPT_LANGUAGE", FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW | FILTER_FLAG_STRIP_HIGH));

  // Only examine this string if we haven't done so in the past.
  if (empty($examined[$accept_languages])) {
    // Prepare default fallback in our cache for this string.
    $examined[$accept_languages] = array($default => 1.0);

    // Only continue if we have a string to examine.
    if ($accept_languages) {
      // Prepare array for collecting the preferred languages.
      $languages = array();

      // Extract all languages from the accept languages string, the format is defined in RFC 2616.
      preg_match_all("/([a-z]+(?:-[a-z]+)?)\s*(?:;\s*q\s*=\s*(1|0?\.[0-9]+))?/i", $accept_languages, $matches);

      // Only go through all extracted languages if we have any.
      if (!empty($matches[1])) {
        foreach ($matches[1] as $delta => $code) {
          // Empty means highest preference.
          $languages[$code] = empty($matches[2][$delta]) ? 1.0 : (float) $matches[2][$delta];
        }

        // Sort the accepted languages by preference, if sorting fails use default.
        if (arsort($languages, SORT_NUMERIC) === true) {
          $examined[$accept_languages] = $languages;
        }
      }
    }
  }

  return $list ? $examined[$accept_languages] : array_values($examined[$accept_languages])[0];
}

// ----------------------------------------------------------------------------------------------------------------- Test

function get_client_preferred_language_test() {
  assert(array(
    "en-US" => 1.0,
    "en"    => 1.0,
    "en-AU" => 0.8,
    "fr"    => 0.6,
    "en-GB" => 0.4,
  ) === get_client_preferred_language(true, "en,en-US,en-AU;q=0.8,fr;q=0.6,en-GB;q=0.4"));
}

get_client_preferred_language_test();


"Who are you, and where do you live?" — a Q&A exercise This is my first attempt at a programme. Is it good form?  Does it follow best practice? Or am I completely off the mark?

#include "stdafx.h"
#include <iostream>
#include <string>
#include <Windows.h>
using namespace std;

string a;
string b;

string enterName()
{
    while (true)
    {
        cout << "what is your name?: ";
        getline(cin, a);
        if (a == "")
            cout << "Let's try that again." << endl;
        if (a != "")
            return a;
    }
}

string enterAge()
{
    while (true)
    {
       cout << "What is your age " << a << "?: ";
        getline(cin, b);
        if (b == "")
            cout << "You did not answer the question." << endl;
        if (b != "")
            return b;
    }
}

int verifyAge()
{
    while (true)
    {
        int x = atoi(enterAge().c_str());
        if (x > 0 && x < 95)
            return x;

        cout << "That is not possible, put your real age please." << endl;
    }
}

string address()
{
    while (true)
    {
        cout << "Where do you live " << a << "?: ";
        string c;
        getline(cin, c);
        if (c == "")
            cout << "You must live somewhere." << endl;
        if (c != "")
            return c;
    }
}

int _tmain(int argc, _TCHAR* argv[])
{
    system("Color 1A");
    string a = enterName();
    cout << endl;
    int y = verifyAge();
    cout << endl;
    string c = address();
    cout << endl;
    system("cls");
    cout << endl << endl << endl;
    cout << "So your name is " << a << " you are " << y << " years old"
        << " and you live in " << c << endl << endl << endl;
    cout << "Press any key to exit";
    cin.get();
    return 0;
}

I have found a couple of things that could help you improve your code.

Don't abuse using namespace std

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

Avoid the use of global variables

I see that a and b are declared as global variables rather than as local variables. It's generally better to explicitly pass variables your function will need rather than using the vague implicit linkage of a global variable. It's a bit strange because you have done that with variable c in your _tmain function.

Use meaningful variable names

The variable names a, b, and c are not at all descriptive. Better names might be name, age and address.  Doing so makes your code easier to read, understand and maintain.

Don't use system("cls")

There are two reasons not to use system("cls") or system("Color 1A").  The first is that it is not portable to other operating systems which you may or may not care about now.  The second is that it's a security hole, which you absolutely must care about.  Specifically, if some program is defined and named cls or Color, your program will execute that program instead of what you intend, and that other program could be anything.  First, isolate these into a separate functions cls() and color() and then modify your code to call those functions instead of system.  Then rewrite the contents of those functions to do what you want using C++.

General portability

This code could be made portable if, in addition to the changes in the previous point, you omit the Windows-only include files #include "stdafx.h" and #include <Windows.h> and use the standard int main() rather than the Windows-only _tmain.  See this stackoverflow question for a longer explanation.

Fix your formatting

There are abundant examples here of C++ code that is well formatted.  This code has peculiar indentation that makes it difficult to tell when a function begins and ends.  Fixing that would help.

Make it clear when loops end

The code currently uses while (true) in a number of places, but it doesn't really loop infinitely.  It's better, generally, to have the loop exit condition explicitly stated.  For example we could rewrite your enterName() function:

string enterName()
{
    do
    {
        cout << "what is your name?: ";
        getline(cin, a);
        if (a == "")
            cout << "Let's try that again." << endl;
    } while (a == "");
    return a;
}

Variable range checking

Your verifyAge() routine seems to think that it's impossible for people to be 95 year old or older, but I personally know people that old who still use a computer.  A number of 130 might be a safer bet to use as an upper range.

In all, the program actually functions as intended, so you are indeed on your way.


Simplify simple object clear function with Javascript I have a clear function that accepts an array, string, or no arguments. If no arguments, it should clear the entire data object. It works fine, but looking for a code review.

The possibilities should be:

clear('one');
clear(); //clear all
clear(['one', 'three']);

var data = {
  one: 'test1',
  two: 'test2',
  three: 'test3'
};

var clear = function(fields) {
  if (fields !== undefined) {
    if (typeof fields === 'string') {
      data[fields] = '';
    } else {
      _.each(fields, function(field) {
        if(data.hasOwnProperty(field)) {
          data[field] = '';
        }
      });
    }
  } else {
    // clear all
    _.each(data, function(value, key) {
      data[key] = '';
    });
  }
};

clear(['one', 'two']);
clear();
clear(['one']);
<script src="https://cdnjs.cloudflare.com/ajax/libs/underscore.js/1.8.3/underscore-min.js"></script>

The clear function relies on the global data variable. This could be more portable by passing the object in as the first argument, and then the fields as the second argument

clear(data); // clear all
clear(data, "one"); // clears the "one" field
clear(data, ["one", "two"]); // clears multiple fields

Checking if (fields !== undefined) will accept null values as a field name. A "truthy" check is probably what you need:

if (fields) {
  // ...
} else {
  // ...
}

What does "clear" mean? Your code assumes that all fields in an object should be cleared out to an empty string. What if the original value was a boolean, or number? Maybe you want to set that field to null instead. Better yet, maybe set it to null then delete the field:

data[field] = null;
delete data[field];

Other than that, it looks pretty good.


Haskell BigInt addition I have a function add that adds two lists of integers:

add [1, 2] [4, 5, 8] == [4, 7, 0]
add [9, 8, 9] [1, 3] == [1, 0, 0, 2]

I know that Integer from Prelude is already arbitrary large, I'm reinventing the wheel for exercise.

Here's the code:

import Data.Int(Int8)

-- Adds two integer lists.
-- Examples:
-- add [1, 2] [4, 5, 8] == [4, 7, 0]
-- add [9, 8, 9] [1, 3] == [1, 0, 0, 2]
add :: [Int8] -> [Int8] -> [Int8]
add xs ys = toIntList $ foldr addCarry [] sumWithCarry
  where (paddedXs, paddedYs) = padToSameLength xs ys
        sumWithCarry = foldr getSumAndCarry [] $ zip paddedXs paddedYs
        -- Turn the list of sums with carry into a list of sums
        toIntList [] = []
        -- If the first pair has a carry of one, add that to the list.
        -- This ensures that the resulting list can grow one digit
        toIntList xs@((_,carry):_) | carry >0  = carry : map fst xs
                                   | otherwise = map fst xs

-- Pads two lists of ints to the same length
-- by prepending zeros to the shorter one
padToSameLength xs ys =
  if lenDiff < 0
  then (padWithLeadingZeros xs (lenDiff * (-1)), ys)
  else (xs, padWithLeadingZeros ys lenDiff)
  where lenDiff = length xs - length ys
        padWithLeadingZeros list nr = take nr (repeat 0) ++ list

-- Given two same-sized list of ints, returns them
-- zipped as pairs of their sum and carry.
-- Example: getSumAndCarry [1, 2] [4, 8] == [(5, 0), (0, 1)]
getSumAndCarry :: (Int8, Int8) -> [(Int8, Int8)] -> [(Int8, Int8)]
getSumAndCarry (x, y) pairs = sumWithCarry x y : pairs
  where sumWithCarry x y | x + y >= 10 = ((x + y) - 10, 1)
                         | otherwise   = (x + y, 0)

-- Once we have added two lists like [7, 9, 8] and [2, 1, 4] to a list
-- of sums and carries, [(9, 0), (0, 1), (2, 1)],
-- we want to add the carry: [(9, 1), (1, 0), (2, 0)]
addCarry :: (Int8, Int8) -> [(Int8, Int8)] -> [(Int8, Int8)]
-- The rightmost pair. We don't do much since only the pair's left
-- neighbor will use its carry
addCarry pair [] = [pair]
addCarry (sum, carry) pairs@((_, prevCarry):_) =
  sumWithCarry sum carry prevCarry : pairs
  where sumWithCarry sum carry prevCarry | sum + carry + prevCarry >= 10 = ((sum + carry + prevCarry) - 10, 1)
                                         | otherwise = (sum + carry + prevCarry, 0)

It seems to work correctly but I'd really like to improve the code:

Generally, make it shorter wherever possible
Specifically, combine the functions getSumAndCarry and addCarry if possible since both  traverse the list, and I'd prefer add to be O(n).

Thanks a lot for your feedback!

For anyone interested, here's a version incorporating Zeta's feedback. It's tested with QuickCheck and does not suffer from the carry bug of the original code.

Your code contains at least one bug, therefore this review is a little bit shorter than usual since you need to fix that bug either way.

So let's instead have a look at testing and how you can find bugs like this.

Testing

I'll start with testing since the presented code contains bugs. That's due to the carry logic. It works for your small examples, but QuickCheck can come up with several examples that won't work.

First, we need some additional functions:

import Data.List (foldl')
import Test.QuickCheck

type Digit = Int8

fromDigits :: [Digit] -> Integer
fromDigits = foldl' (\a x -> a * 10 + x) 0 . map fromIntegral

toDigits :: Integral n => n -> [Digit]
toDigits n | n < 0 = []
toDigits 0 = [0]
toDigits n = reverse . map fromIntegral . go $ n
  where
    go 0 = []
    go n = let (q,r) = n `quotRem` 10 in r : go q

We should test that fromDigits and toDigits work correctly:

prop_digitIdentity1 (NonNegative x) = fromDigits (toDigits x) === x

prop_digitIdentity2 =
    forAll validDigits $ \x ->
      toDigits (fromDigits x) === x
  where
    validDigits = fixEmpty . dropWhile (== 0) <$> listOf (choose (0,9))
    fixEmpty [] = [1]
    fixEmpty xs = xs

QuickCheck tells us that our properties hold:

*Main> quickCheck prop_digitIdentity1
+++ OK, passed 100 tests.
*Main> quickCheck prop_digitIdentity2
+++ OK, passed 100 tests.

Now it's time to write a test for add:

prop_add (NonNegative x) (NonNegative y) =
    toDigit x `add` toDigit y === toDigit (x + y)

If we run this now we get a counter example:

*Main> quickCheck prop_add
*** Failed! Falsifiable (after 63 tests and 5 shrinks):
NonNegative {getNonNegative = 50}
NonNegative {getNonNegative = 50}
[1,0] /= [1,0,0]

So add doesn't work for [5,0] [5,0]. Essentially, it doesn't work whenever the carry needs to propagate. I didn't look into this in detail, though.

Further remarks

padToSameLength should get a type signature. Also, take n (repeat x) is replicate n x.

We can make padToSameLength a little bit easier to read if we add additional whitespace:

padToSameLength :: Num a => [a] -> [a] -> ([a],[a])
padToSameLength xs ys =
  if lenDiff < 0
      then (padWithLeadingZeros xs (lenDiff * (-1)), ys)
      else (xs, padWithLeadingZeros ys lenDiff)
  where
      lenDiff = length xs - length ys
      padWithLeadingZeros list nr = take nr (repeat 0) ++ list

Or we could remove the if completely, since replicate with non-positive arguments yields an empty list:

padToSameLength :: Num a => [a] -> [a] -> ([a],[a])
padToSameLength xs ys = (pad diff xs, pad (-diff) ys)
  where
    diff = length ys - length xs
    pad n ps = replicate n 0 ++ ps

Use map g instead of foldr f [] where applicable

If you have a function f value list = g value : list, then foldr f [] list is the same as map g list. This certainly holds for an empty list:

foldr f [] []     = map g [] = []

So let's say it holds that foldr f [] xs == map g xs, we have to check that foldr f [] (x:xs) == map g (x:xs) for all x:

foldr f [] (x:xs) = x `f` foldr f [] xs
                  = g x : foldr f [] xs -- induction
                  = g x : map g xs
                  = map g (x:xs)

Therefore, we can simplify getSumWithCarry:

add :: [Int8] -> [Int8] -> [Int8]
add xs ys = toIntList $ foldr addCarry [] sumWithCarry
  where (paddedXs, paddedYs) = padToSameLength xs ys
        sumWithCarry = map getSumWithCarry $ zip paddedXs paddedYs

        -- Turn the list of sums with carry into a list of sums
        toIntList [] = []
        -- If the first pair has a carry of one, add that to the list.
        -- This ensures that the resulting list can grow one digit
        toIntList xs@((_,carry):_) | carry >0  = carry : map fst xs
                                   | otherwise = map fst xs

        -- Combines two digits to their sum and their carry
        getSumWithCarry (x,y) | x + y >= 10 = ((x + y) - 10, 1)
                              | otherwise   = (x + y, 0)

Use zipWith f xs instead of map (uncurry f) . zip xs

However, map f $ zip xs ys is the same as zipWith (curry f) xs ys, so let's further simplify add:

add :: [Int8] -> [Int8] -> [Int8]
add xs ys = toIntList $ foldr addCarry [] sumWithCarry
  where (paddedXs, paddedYs) = padToSameLength xs ys
        sumWithCarry = zipWith getSumWithCarry paddedXs paddedYs

        toIntList [] = []
        toIntList xs@((_,carry):_) | carry >0  = carry : map fst xs
                                   | otherwise = map fst xs

        getSumWithCarry x y | x + y >= 10 = ((x + y) - 10, 1)
                            | otherwise   = (x + y, 0)

We cannot get rid of foldr addCarry, since addCarry v l isn't g v : l for any suitable l.

Use a simpler algorithm (first)

Usually, with addition, we want to start with the least significant digit. This is a lot easier if we reverse the digits first:

addBase :: Integral n => n -> [n] -> [n] -> [n]
addBase base as bs =
    cleanup $ mapAccumL go $ zipWithPad 0 (+) (reverse as) (reverse bs)
  where
    go carry sum = (carry + sum) `quotRem` base
    cleanup      = -- exercise, but very similar to addCarry

zipWithPad :: a -> (a -> a -> a) -> [a] -> [a]
zipWithPad a f xs ys = -- left as exercise


Conditionally write to a compressed stream My Import method takes some data and writes it to a stream.  If the CompressedStore property is true, the contents of that stream should be compressed.

This code works, however I just don't like it.  For one, I call serializer.Serialize() twice.  I feel this code can be made more concise.  Any ideas?

public void Import(IProvisionSource source)
{
   InitializeStore();

   // Call source.Export and populate local data store
   var data = source.Export();
   var serializer = new XmlSerializer(data.GetType());

   var file = CompressedStore ? "KPCData.gz" : "KPCData.xml";
   var path = Path.Combine(DataDirectory, file);
   using (var fileWriter = new FileStream(path, FileMode.Create))
   {
      if (CompressedStore)
      {
         using (var writer = new GZipStream(fileWriter, CompressionLevel.Optimal))
         {
            serializer.Serialize(writer, data);
         }
      }
      else
      {
         serializer.Serialize(fileWriter, data);
      }
   }
}

By default, GZipStream owns the underlying stream, so it disposes it when it's disposed itself. If you're okay with relying on that, then you could put the right Stream into a variable and then have just one using around it. Something like:

var writer = new FileStream(path, FileMode.Create);

if (CompressedStore)
    writer = new GZipStream(writer, CompressionLevel.Optimal);

using (writer)
{
   serializer.Serialize(writer, data);
}

This avoids the duplication and so it's shorter, but it's also less obviously correct (using combined with new is a good pattern, and it's not used here), so I think your original code is actually the better option. If there was more repetition than just one method call, it could make sense to extract that into a separate method, but that's not the case here.


Modifying `sakila` database This is not "real life" code.  I'm trying to expand upon the well known Sakila sample database for MySQL to make it more complex. Step 7 (or 6) is running surprisingly slow.

PS: Note these are all separate queries executed against the same database in the order specified.

Add columns to sakila.customer table:

USE sakila;
ALTER TABLE customer
    ADD COLUMN multiplier DECIMAL(3,2) AFTER active;
ALTER TABLE customer
    ADD COLUMN cust_ranking VARCHAR(10) AFTER multiplier;

Duration: 0.289 sec
Create a proc to randomly distribute multiplier:

DROP PROCEDURE IF EXISTS sp_randCustMult;
DELIMITER //
CREATE PROCEDURE sp_randCustMult()

BEGIN
    -- declare a counter
    SET @start = (SELECT MIN(customer_id) FROM customer);
    SET @stop  = (SELECT MAX(customer_id) FROM customer);

    -- start while loop
    WHILE @start <= @stop
        DO
        -- select a random float variable
        SET @RAND = RAND();
        -- update NULL field
        UPDATE customer
        SET multiplier = (SELECT
            (CASE
                WHEN @RAND <= 0.65 THEN 1.00
                WHEN @RAND <= 0.90 THEN 0.85
                WHEN @RAND <= 1.00 THEN 1.05
                END))
        WHERE customer_id = @start;
            -- tick counter one up
            SET @start = @start + 1;
    END WHILE;
END//
DELIMITER ;

Duration: 0.001 sec
Call the proc to populate the rows:

CALL sp_randCustMult;

Duration: 0.761 sec
With Safe Update Mode OFF, add human-friendly values to cust_ranking based on multiplier.

UPDATE customer
    SET cust_ranking = 'Standard'
    WHERE multiplier = 1.00;
UPDATE customer
    SET cust_ranking = 'Premium'
    WHERE multiplier = 0.85;
UPDATE customer
    SET cust_ranking = 'Uplift'
    WHERE multiplier = 1.05;

Duration:
0.057 sec
0.037 sec
0.035 sec
Add a real_amount column to table payment:

ALTER TABLE payment
ADD COLUMN real_amount DECIMAL(5,2)
AFTER amount;

Duration: 0.749 sec
Create another proc to populate payment with real amounts:

DROP PROCEDURE IF EXISTS sp_RealAmtPayment;
DELIMITER //
CREATE PROCEDURE sp_RealAmtPayment()

BEGIN
-- declare a counter
SET @start = (SELECT MIN(payment_id) FROM payment);
SET @stop  = (SELECT MAX(payment_id) FROM payment);

WHILE @start <= @stop
DO
    UPDATE payment AS p
        INNER JOIN customer AS c
            ON p.customer_id = c.customer_id
    SET real_payment = (p.amount * c.multiplier)
    WHERE p.payment_id = @start;
    SET @start = @start + 1;
END WHILE;
END//
DELIMITER ;

Duration: 0.001 sec
CALL sp_RealAmtPayment;

Duration: 17.082 sec
SELECT * FROM payment;

1000 row(s) returned
0.002 sec / 0.002 sec

Step 7 seems extremely long considering the very small number of records.  What am I missing? All comments/critiques welcome!

I've never played with mysql, so this may be completely wrong, but if I get it right the WHILE loop would be the equivalent of a T-SQL CURSOR, which is inherently slow.

You're essentially looping on payment_id, incrementing at each iteration - this assumes the ID's are contiguous, which isn't a safe assumption to make with data: if records were deleted, you have more iterations than records:

SET @start = (SELECT MIN(payment_id) FROM payment);
SET @stop  = (SELECT MAX(payment_id) FROM payment);

WHILE @start <= @stop
DO
    UPDATE payment AS p
        INNER JOIN customer AS c
            ON p.customer_id = c.customer_id
    SET real_payment = (p.amount * c.multiplier)
    WHERE p.payment_id = @start;
    SET @start = @start + 1;
END WHILE;

In pseudo-code, this can read as follows:

For each payment_id in payment...
... update the real_payment column to p.amount*c.multiplier

I don't see why you need a loop to do this, I think this would be equivalent... and faster:

UPDATE payment AS p
    INNER JOIN customer AS c
        ON p.customer_id = c.customer_id
SET real_payment = (p.amount * c.multiplier)


Async selector in LINQ LINQ Select usually works like this:

IEnumerable<Task<int>> res = new[] {1,2,3}.Select(async i => i);

I would like to have the following though:

IEnumerable<int> res = new[] {1,2,3}.Select(async i => i);

This extension method helps to get it:

static class SelectAsync
{
    public static IEnumerable<TResult> Select<T, TResult>(
        this IEnumerable<T> source, Func<T, Task<TResult>> selector)
    {
        return source
            .ToObservable()
            .Select(value => Observable.FromAsync(() => selector(value)))
            .Concat()
            .ToEnumerable();
    }
}

So the following is true:

public class AsyncSelect_Should
{
    [Test]
    public void Not_Block()
    {
        var src = new[] { 1, 2, 3 };
        var res = src.Select(async i => i);
        CollectionAssert.AreEqual(src, res);
    }
}

There should be something bad, really bad about it. Does it? :)

The bad part is that ToEnumerable returns an IEnumerable that blocks for each element until it is produced by the IObservable it is called on. This means that you might as well be doing:

public static IEnumerable<R> SelectAsync<T, R>(this IEnumerable<T> source, Func<T, Task<R>> selector)
{
    foreach (var item in source)
        yield return selector(item).Result;
}

and depending on what kind of app you're writing, this might deadlock, though I think your version of going through Rx might avoid that and be more functionality close to:

public static IEnumerable<R> SelectAsync<T, R>(this IEnumerable<T> source, Func<T, Task<R>> selector)
{
    foreach (var item in source)
        yield return Task.Run(() => selector(item)).Result;
}

Either way, you're blocking a Thread which is seen as a bad thing to do in the age of async/await and the TPL. It would be as bad as using Thread.Sleep with Task.Run. It is not likely to lock up your app, especially if your app is multi-threaded, but it most certainly will block a Thread on the thread pool.

I've certainly seen people get away with using Thread.Sleep with Task.Run with no real consequences, but keep in mind that it can cause issues when you scale.

As for how you might know this without being intimately familiar with Rx and the details of how its operators work? That's easy. Just know that you can never go from having an asynchronous value (Task<T>) to having a synchronous value (T) without awaiting (which requires that you stay in an async context) or blocking. Even Rx doesn't have enough magic to change that :)


Prime factorization in F# In the pursuit of learning F#, I have been working through some Project Euler problems.

This is my solution for problem 3:

open System
open System.Collections.Generic

let number = 600851475143L
let limit = Convert.ToInt32(sqrt (float number))

let sieve = Array.create (limit+1) true
sieve.SetValue(false,0)
sieve.SetValue(false,1)

let rec markNotPrime prime multiple =
    if multiple > limit then prime
    else
        sieve.SetValue(false, multiple)
        let nm = prime+multiple
        markNotPrime prime nm

let smallPrimes =
    Seq.unfold(fun a -> Some(a, (a+1))) 2
    |> Seq.takeWhile(fun a -> a <= limit)
    |> Seq.filter( fun a -> sieve.[a])
    |> Seq.map( fun a ->
        let b = a+a
        markNotPrime a b)
    |> Seq.filter( fun a -> number % (int64 a) = 0L)

let bigPrimes = new List<int>()

let rec addBigPrime potentialPrime =
    match potentialPrime with
    | p when bigPrimes.Contains(potentialPrime) ->
        ignore()
    | p when smallPrimes |> Seq.forall(fun a -> (potentialPrime % a) <> 0) ->
        bigPrimes.Add(p)
    | _ ->
        smallPrimes
            |> Seq.filter(fun a ->
                (potentialPrime % a) = 0)
            |> Seq.iter(fun a ->
                addBigPrime (potentialPrime / a))

smallPrimes
    |> Seq.map(fun a -> (int (number/(int64 a))))
    |> Seq.iter(addBigPrime)

let answer = smallPrimes |> Seq.append(bigPrimes) |> Seq.max

printfn "smallPrimes: %A" smallPrimes
printfn "bigPrimes: %A" bigPrimes
printfn "answer: %d" answer

I am aware of how simply this could have been done, but I was trying to do this in a vacuum as much as possible.

I am new to both prime factorization and F#; however, I am really only looking for comments on F#, not how poorly my factorization algorithm works (I know it's bad) i.e. what style mistakes am I making, or how the code could be made more functional.

Of particular interest, where do you think I am missing the mark entirely?

Update

After thinking more about my solution overnight, I had more thought about exactly what my solution was doing, and why the simpler methods are so much simpler.

During my research on factoring primes, I think I got too caught up in the sieve of Eratosthenes, and didn't focus on the actual problem.

What I should have been trying to do:

Search for the smallest prime factor
Once found save it to the list of prime factors
Divide this factor out of the number to be factored
Repeat from step 1 substituting the number to be factored with the quotient from step 3, and start the search from the prime factor found in step 2.
Get the biggest prime from the list.

What I actually did:

Search for all prime numbers from 2 to the square root of the number to be factored.
During the search, check each prime to see if it divides evenly.
Once complete divide each of these "small primes" to generate a list potential big primes.
Divide each of the big primes by all of the small primes as a test for primality.
Combined the lists, and get the biggest one.

(Apologies in advance for any bad F# syntax in this answer)

One issue is that you're doing things statefully. In a functional style, you want functions to be pure, as much as possible- i.e. their purpose is to return something, not to change state. Whereas you have global collections like sieve and bigPrimes, and functions whose purpose is to modify those collections.

Taking the last part in particular, you do:

smallPrimes
|> Seq.map(fun a -> (int (number/(int64 a))))
|> Seq.iter(addBigPrime)

let answer = smallPrimes |> Seq.append(bigPrimes) |> Seq.max

Instead, you want to be able to do something like:

let answer = smallPrimes
             |> getBigPrimes
             |> Seq.max

In this case getBigPrimes would just calculate the big primes from the small ones, probably using a recursive inner function. A useful technique to allow you to do this without any state mutation is to have an accumulator collection as a parameter to your recursive function (often called acc). Then instead of having some list you repeatedly add results to, you pass a new acc to each call of the recursive function, created by prepending the result to the previous one acc.

So as an example, instead of:

let primesUpTo n =
    let primes = new List<int>()
    let rec loop i =
        if isPrime n primes then primes.Add(n)
        if i = n then primes else loop i+1
    loop 2

You'd do:

let primesUpTo n =
    let rec loop i acc =
        if i = n then acc
        elif isPrime i acc then loop i+1 i::acc
        else loop i+1 acc

Notice that by replacing iteration with this tail recursive style, you no longer have to have mutable state in the form of a collection which gets updated. This (and the fact that recursive algorithms often read as more declarative than iterative ones), mean that tail recursion is generally preferred over iteration in F#.

As you already said in your question, your algorithm is a bit strange. Your alternative algorithm is much better. Writing it in the same recursive style, an outline would look something like:

let rec largestFactor n primes =
    let factor = smallestFactor n primes
    if factor = n then factor else largestFactor n/factor primes

let answer = primesUpTo n |> largestFactor n

You'd then just need to implement primesUpTo (which you'd want to make lazy, so that you don't calculate primes higher than you need) and smallestFactor


Find the starting indices of all occurrences of the pattern in the string The task

  Given a string and a pattern, find the starting indices of all
  occurrences of the pattern in the string. For example, given the
  string "abracadabra" and the pattern "abr", you should return [0, 7].

My solution

function findStartingIndex(T, pattern) {
  let S = T;
  const res = [];
  while(true) {
    const i = S.indexOf(pattern);
    if (i === -1) { return res; }

    S = S.substring(i + 1);
    res.push(i ? i + 1 : i);
  }
  return res;
}

Your code contains a bug.

For banana and a, it should return [1, 3, 5], but it doesn't.


Complexity of bubble sort I have a simple integer sorting problem at hand and to solve it, I am planning to write a variation of bubble sort. It seems to be working fine but I am not sure about it's complexity in big-O. What it could be ?

public class TempBubbleSort
{

static Integer[] myArray = {10,9,8,7,6,5,4,3,2,1};

static int counter = 0;

public static void main(String[] args)
{
    for(int anchor=0; anchor<myArray.length; anchor++)
    {
        for(int compare=anchor+1; compare<myArray.length; compare++)
        {
            counter++;
            // sort ascending
            if(myArray[anchor] > myArray[compare])
            {
                int tmp = myArray[compare];
                myArray[compare] = myArray[anchor];
                myArray[anchor] = tmp;
            }
        }
    }

    System.out.println("Comparision Count : "+counter);

    for(int i : myArray)
        System.out.println(i);
}

}

Output when you run above is :

Comparision Count : 45
1
2
3
4
5
6
7
8
9
10

Strictly answering your question about what the \$Big-O\$ run-time complexity,

public class TempBubbleSort
{

static Integer[] myArray = {10,9,8,7,6,5,4,3,2,1};

static int counter = 0;

public static void main(String[] args)
{
    // **This is O(n), as we're touching every point in your array.
    for(int anchor=0; anchor<myArray.length; anchor++)
    {
        // **This is O(n-1), but since we don't care about constants as n grows, we consider this as O(n)
        for(int compare=anchor+1; compare<myArray.length; compare++)
        {
            // **Everything in here then is a bunch of constant operations so we'll just ignore them.
            counter++;
            // sort ascending
            if(myArray[anchor] > myArray[compare])
            {
                int tmp = myArray[compare];
                myArray[compare] = myArray[anchor];
                myArray[anchor] = tmp;
            }
        }
    }
    // **So the total of the brunt of your work would be O(n)*O(n) or O(n^2), because for every element in your array n, you touch every other element in your array, so you can look at it as touching n things in your array, n times.

    System.out.println("Comparision Count : "+counter);

    // **In case you're curious, this is also O(n)
    for(int i : myArray)
        System.out.println(i);

    // **Which would bring the grand total to O(n^2+n), again with Big O notation, we only care about what happens as n continues to grow, and since n^2 grows faster than n, as our n gets super big, the second term n becomes insignificant, so we look at this overall total as O(n^2). In fact, for any polynomial time complexity, you can safely drop all terms that are not your largest degree.
}

}

In closing, its usually easy to tell the time-complexity of simple iterative functions like this by the number of nested for loops you have, which can be a good rule of thumb for a nice approximate guess for time complexity in a pinch.


Simple function to verify if a number is integer Sometimes, there's the need to verify if a given number is an integer or a decimal. Since JavaScript doesn't distinguish between both, I've made an extremelly basic function.

function isInteger(num){
    var numCopy = parseFloat(num);
    return !isNaN(numCopy) && numCopy == numCopy.toFixed();
}

As you can see, it relies on checking if the number without decimals is the same as the parsed value. I'm not so confident about this.

Notice that this may return true for some scientific notation values.

Examples of results:

1.0: true (JavaScript treats <n>.0 as an integer)
1e3: true
1.2345e2: false (equivalent to 123.45)
1.2345e5: true (equivalent to 123450)
1e200: true (equivalent to Infinity)

Is there any more reliable way of checking? The results seem to be consistent with the expected results. Regular expressions are an option but they are clunky.

In ES6, the next version of Javascript, there will be a function exactly for this purpose called Number.isInteger(). Currently, MDN offers the following polyfill which I assume therefore does the best Javascript can to determine whether something is an integer. There are edge cases of course, but for most everyday use it will do the trick:

Number.isInteger = Number.isInteger || function(value) {
    return typeof value === "number" &&
           isFinite(value) &&
           Math.floor(value) === value;
};

I think this is the best we will do in the meanwhile, as passing string to a number function is an effective way of doing it, but calling toString on a number makes it go through conversion as well, resulting in potential issues.

There are already a couple of browsers that support it like Chrome and Firefox, but the big one (Safari) doesn't yet and neither does IE/Edge, so the polyfill will have to do. For more info, see MDN's writeup, it's quite insightful:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/isInteger#Browser_compatibility

Note: Polyfills like this are useful, but there are some concerns about amending these core features of JS. As I say in the comments, as long as you implement something that doesn't exist or replace anything, I think it's alright. Use in moderation.

You can always wrap it in it's own function and call it as a global function if you prefer not to use this polyfill.


Find the median value of a list I am taking an online training course on Python, and am at the following question:

  Write a function called median that takes a list as an input and
  returns the median value of the list.
  For example: median([1,1,2]) should return 1.
  The list can be of any size and the numbers are not guaranteed to be
  in any particular order. If the list contains an even number of
  elements, your function should return the average of the middle two.

My code seems correct and passes the tests:

def median(thelist):
    median = 0
    sortedlist = sorted(thelist)
    lengthofthelist = len(sortedlist)
    centerofthelist = lengthofthelist / 2
    if len(sortedlist) == 1:
        for value in sortedlist:
            median += value
        return median

    elif len(sortedlist) % 2 == 0:
        temp = 0.0
        medianparties = []
        medianparties = sortedlist[centerofthelist -1 : centerofthelist +1 ]
        for value in medianparties:
            temp += value
            median = temp / 2
        return median

    else:
        medianpart = []
        medianpart = [sortedlist[centerofthelist]]
        for value in medianpart:
            median = value
        return median

I am (pretty) sure that, despite passing the tests, the code is neither nice, efficient, nor Pythonic. I am wondering if the community may highlight what I did wrong and how I can improve it.

First things first, you can remove the if len(sortedlist) == 1.
If we do a quick run through we should get:

theList = [1]
conterofthelist = 1 / 2
medianpart = [sortedlist[0]]
median = 1

This removes the first if.
You can remove most of the code in the else:

The first medianpart is immediately overridden.
Median part could be just a number, not a list.
You don't need to loop through medianpart.
You can just return sortedlist[centerofthelist]

This means that the code you have at the moment is:

def median(thelist):
    median = 0
    sortedlist = sorted(thelist)
    lengthofthelist = len(sortedlist)
    centerofthelist = lengthofthelist / 2
    if len(sortedlist) % 2 == 0:
        temp = 0.0
        medianparties = []
        medianparties = sortedlist[centerofthelist -1 : centerofthelist +1 ]
        for value in medianparties:
            temp += value
            median = temp / 2
        return median
    else:
        return sortedlist[centerofthelist]

If we go through the if now:

You don't need the first medianparties.
You can use sum rather than your loop.
You should move the temp / 2 out of the loop, to the return.

This should result in something like:

return sum(sortedlist[centerofthelist -1 : centerofthelist +1 ]) / 2.0

This should show you that you use lengthofthelist twice, and median never.
You could probably remove these.

Python has a style guide called PEP8, it's a fancy name I know.
It gives a few rules that you should probably follow:

Variable names should be lower_snake_case.
This means that sortedlist should be sorted_list and theList should be the_list.
Binary operators need a space either side of them. centerofthelist - 1.
List slices : have to have no spaces either side.
Use good variable names.

This should result in something like:

def median(numbers):
    numbers = sorted(numbers)
    center = len(numbers) / 2
    if len(numbers) % 2 == 0:
        return sum(numbers[center - 1:center + 1]) / 2.0
    else:
        return numbers[center]


Passing parameters to a query I have a function that takes a Dictionary<String, Object> as an argument.  It uses this Dictionary to create parameters for a query string.

private static TableAdapters dataSource = new TableAdapters();
private void findScriptsQueryButton_Click(object sender, EventArgs e)
{
  string fillBy = findByComboBox.SelectedItem.ToString();
  string server = serverComboBox.SelectedItem.ToString();
  Dictionary<String, Object> parameters = new Dictionary<String, Object>()
  {
    {"findBy", findByTextBox.Text}
  };
  dataGridView.DataSource = dataSource.GetDataSource(fillBy, server, parameters).Tables[0];
}

public class TableAdapters
{
  public DataSet GetDataSource(string fillBy, string server, Dictionary<String, Object> parameters)
  {
    string query = GetQuery[fillBy];
    SqlConnection connection = new SqlConnection();
    connection.ConnectionString = GetSqlConnection[server];
    SqlDataAdapter adapter = new SqlDataAdapter(query, connection);
    foreach (KeyValuePair<String, Object> p in parameters)
    {
      adapter.SelectCommand.Parameters.AddWithValue("@" + p.Key, (p.Value.ToString().Length == 0) ? DBNull.Value : p.Value);
    }
    DataSet dataSet = new DataSet();
    dataSet.Tables.Add(new DataTable());
    adapter.Fill(dataSet.Tables[0]);
    return dataSet;
  }
}

My primary problem with my code is:

I have to create a new dictionary of parameters every time before I call the function.
The parameter names are hard coded into both the dictionaries and the query strings.

The number of parameters for a query string in this particular program can be 1-11. Is there a better way for me to pass parameters to this function where I don't have to hard code the names?  Mind you I don't want to go off parameter number either, because then I've become dependent on ensuring I pass the parameters to the function in the right order.

For brevity here is an example of one of the queries from the query dictionary that this particular instance of the function calls:

private static readonly Dictionary<String, String> GetQuery = new Dictionary<String, String>()
{
  {"Target VDN",
   "SELECT * FROM table_name WHERE column_name = ISNULL(@findBy, column_name) "
  },
  // ...
}

In this case I am only dealing with one parameter but as I mentioned some of these queries in the dictionary can take up to 11 parameters.

You can take advantage of C#'s variable parameter arguments (keyword params). Like this -

public DataSet GetDataSource(string fillBy, string server, params KeyValuePair<String, Object>[] parameters)
    {
        string query = GetQuery[fillBy];
        SqlConnection connection = new SqlConnection();
        connection.ConnectionString = GetSqlConnection[server];
        SqlDataAdapter adapter = new SqlDataAdapter(query, connection);
        foreach (KeyValuePair<String, Object> p in parameters)
        {
            adapter.SelectCommand.Parameters.AddWithValue("@" + p.Key, (p.Value.ToString().Length == 0) ? DBNull.Value : p.Value);
        }
        DataSet dataSet = new DataSet();
        dataSet.Tables.Add(new DataTable());
        adapter.Fill(dataSet.Tables[0]);
        return dataSet;
    }

params is the keyword to use for taking variable numbers of arguments. Then you can just call it with variable arguments like -

   dataGridView.DataSource = dataSource.GetDataSource(fillBy, server,
        new KeyValuePair<string, object>("findBy",findByTextBox.Text)
        ).Tables[0];

and in another time you can call same function with -

   dataGridView.DataSource = dataSource.GetDataSource(fillBy, server,
            new KeyValuePair<string, object>("findBy",findByTextBox.Text),
            new KeyValuePair<string, object>("findBy2",findByTextBox2.Text)
        ).Tables[0];

This will atleast save you from defining a dictionary all the time.


Wipe sensitive information from memory in Python I have come up with the following method to remove sensitive information with the help of the garbage collector:

def wipe_sensitive_info(d, keys):
    """
    In Python we are not able to overwrite memory areas. What we do is:
    - remove references to the specified objects, in this case keys in a dictionary
    - immediately call garbage collection
    """
    # TODO: the following points are pending
    #   - this relies on the gc to remove the objects from memory (de-allocate memory).
    #   - the memory itself is not guaranteed to be overwritten.
    #   - this will only deallocate objects which are not being referred somewhere else (ref count 0 after del)
    for key in keys:
        del d[key]
    logger.info('Garbage collected {} objects'.format(gc.collect()))

This would be called from a django app as follows:

wipe_sensitive_info(request.data, ['password'])

Any ideas or comments on how to improve this implementation?

One advantage of the logger module is that you can pass it strings and objects to log at some level, and the string formatting is only performed if it is actually needed (i.e. the logging level is low enough). Also, having the gc.collect() actually hides it a lot, I was looking for it for some time. So I would do this instead:

def wipe_sensitive_info(d, keys):
    """
    In Python we are not able to overwrite memory areas. What we do is:
    - remove references to the specified objects, in this case keys in a dictionary
    - immediately call garbage collection
    """
    # TODO: the following points are pending
    #   - this relies on the gc to remove the objects from memory (de-allocate memory).
    #   - the memory itself is not guaranteed to be overwritten.
    #   - this will only deallocate objects which are not being referred somewhere else (ref count 0 after del)
    for key in keys:
        del d[key]
    n_garbage_collected = gc.collect()
    logger.info("Garbage collected %s objects", n_garbage_collected)

You could add a check for objects which have reference counts left:

import sys

...

for key in keys:
    if sys.getrefcount(d[key]) > 2:
        # The count returned is generally one higher than you might expect,
        # because it includes the (temporary) reference as an argument to
        # getrefcount().
        logger.debug("Object potentially has references left: %s", key)
    del d[key]

Note that this might give you some false positives for interned objects, like small integers or strings. Case in point, in an interactive session, sys.getrefcount("s") just gave me 1071.


Check what items are on-screen This function takes an array of HTML objects and returns those currently on the screen. I try to optimize for readability and performance. I myself find this function a bit confusing.

function(elements, margin) {
    var inViewport = [];

    if(elements.constructor !== Array){ elements = [elements]; }
    margin = margin || 0;

    for (var i = elements.length - 1; i >= 0; i--) {
      var element = elements[i],
        bounds = element.getBoundingClientRect();
        if( bounds.top+margin < window.innerHeight && bounds.bottom-margin > 0 ){
          inViewport.push(element);
        }
    }
    return inViewport;
  }

For improved readability, this writing style is generally recommended:

if (elements.constructor !== Array) {
    elements = [elements];
}

In conditions with range checks, it's a good practice to organize the elements of the condition by increasing numerical order. So instead of this:

bounds.top+margin < window.innerHeight && bounds.bottom-margin > 0

This is better:

0 < bounds.bottom - margin && bounds.top + margin < window.innerHeight


Dropdown list of 8 items I have an input which contains a dropdown list of 8 items. Depending on the option the user picks, I want to change the value of their input into a a different string value. In order to do this, I am using a ton of if else statements, which make this look very bulky and I would like to condense this if at all possible. I have the following code:

if (inputFive == "Corporation"){
inputFive = "534"
}else if (inputFive == "LLC"){
inputFive = "535"
}else if(inputFive == "LLP"){
inputFive = "536"
}else if(inputFive == "Partnership"){
inputFive = "537"
}else if(inputFive == "Sole Proprietorship"){
inputFive = "538"
}else if(inputFive == "Limited Partnership"){
inputFive = "539"
} else {
inputFive = "540"
}

As you can see, this looks a little old-school, and I would like to see if there is a better/simpler way to make this happen. Just looking to condense this code if at all possible. All options/hints will be appreciated!

I am not a Javascript programmer, so I will try my best how to explain to use a dictionary. A dictionary means you can add a value to a key

Example from dotnet perls

# Creations of a dictionary
var dictionary = {"Corporation":534,"LLC":535,"LLP":536,"Partnership":537,"Sole Proprietorship":538,"Limited Partnership":539};

# Getting values of dictionary
new_inputFive = dictionary[inputfive]

# For instance when inputfive="LLC" new_inputFive = 535


Do I need to trim this jQuery code? I had three multi-select boxes from this page. I want to customize each box so I simply repeat the code three times with different options: Check the fiddle

$(".choose").multiselect({
   header: "Choose only One item!",
   multiple: false,
   noneSelectedText: "Select an Option",
   selectedList: 1
});

$(".pick").multiselect({
    header: "Pick Any",
selectedList: 5
});

$(".filter").multiselect({
    header: "Filter Three",
   selectedList: 3
});

It's working fine but I'm curious if there's any better way to do that.

With only three different items I don't think there's much benefit from iterating through a list and instantiating the multi-selects automatically. Your version IMO is much more legible than a version iterating through an embedded data structure. Although with lots of selects this could change fast.

You could take the approach of specifying the attributes of the selects (header, noneSelectedText etc.) as data attributes on the HTML. Then you could wire up your jQuery multiselect code to read the information from the selects themselves. So your jQuery would only have to specify the type of select to trigger the multiselect call on and you could add as many as you like without having to update your scripts.

So something like this:

<select class="dynamic-multiselect" data-header="Choose only One item!" data-none-selected-text="Select an Option" ...>
    <option ...>
    ...
</select>

<select class="dynamic-multiselect" data-header="Pick Any"  ...>
    <option ...>
    ...
</select>
...

And the Javascript would simply be:

$('.dynamic-multiselect').multiselect();


Stack that does Push, Pop and Median in O(1) Here are two implementation of a stack that does Push, Pop and Median in O(1). Invite comments on complexity and betterment.

  Option - 1

public class MedianStack {

    private Deque<Integer> stack;
    private PriorityQueue<Integer> minHeap;
    private PriorityQueue<Integer> maxHeap;

    public MedianStack(){
        stack = new ArrayDeque<>();
        minHeap = new PriorityQueue<>();
        maxHeap = new PriorityQueue<>(Collections.reverseOrder());
    }
    public void push(int item){
        stack.offerFirst(item);
        minHeap.add(item);
        maxHeap.add(item);
    }
    public int pop(){
        int item = stack.poll();
        minHeap.remove(item);
        maxHeap.remove(item);
        return item;
    }

    public int median(){
        return (minHeap.peek() + maxHeap.peek())/2;

    }
}

  Option - 2

Modified and added after realizing the first implementation was not correct.

public class MedianStack {

    private Deque<Integer> stack;
    private MedianHeap medianHeap;

    public MedianStack(){
        stack = new ArrayDeque<>();
        medianHeap = new MedianHeap();

    }
    public void push(int item){
        stack.offerFirst(item);
        medianHeap.add(item);

    }
    public int pop(){
        int item = stack.poll();
        medianHeap.remove(item);
        return item;
    }

    public int median(){
        return medianHeap.median();
    }
}

Median Heap Implementation

public class MedianHeap {

    private PriorityQueue<Integer> maxHeap;
    private PriorityQueue<Integer> minHeap;

    public MedianHeap(){
        maxHeap = new PriorityQueue<>(Collections.reverseOrder());
        minHeap = new PriorityQueue<>();
    }

    public void add(int item){
        if(isEmpty()) minHeap.add(item);
        else{
            if(Integer.compare(item, median()) >= 0) minHeap.add(item);
            else maxHeap.add(item);
        }
        balance();
    }

    public boolean isEmpty(){
        return maxHeap.size() == 0 && minHeap.size() == 0;
    }
    public void balance(){
        if(Math.abs(maxHeap.size() - minHeap.size()) > 1){
            if(minHeap.size() > maxHeap.size()){
                maxHeap.add(minHeap.poll());
            }else minHeap.add(maxHeap.poll());

        }
    }
    public void remove(int item){
        if(!minHeap.contains(item) || !maxHeap.contains(item)) throw new IllegalStateException("Illegal item removal");
        if(minHeap.contains(item)) minHeap.remove(item);
        else maxHeap.remove(item);
        balance();
    }

    public int median(){
        if (minHeap.size() == maxHeap.size()) return (maxHeap.peek() + minHeap.peek())/2;
        else if(minHeap.size() > maxHeap.size()) return minHeap.peek();
        else return maxHeap.peek();
    }
}

minHeap.add(item);
        maxHeap.add(item);

Adding to a PriorityQueue is not \$O(1)\$.  It is \$O(\log{n})\$.

Same thing for removal.

    public int median(){
        return (minHeap.peek() + maxHeap.peek())/2;

    }

This is not a median.  The smallest and largest elements are totally irrelevant to a median.  In 4, 5, 6, then 5 is the median.  In -4000, 5, 6000, then 5 is still the median.  The only time you take an average to get a median is if there are an even number of elements in the set.  E.g. in 2, 4, 6, 8, then \$(4+6)/2 = 5\$ is the median.  Note that it is only the middle items in the set that matter.