MySQL query to select messages related to a user Could someone please check my query and the table structures and let me know if there is any way to improve it.

I would like some tips for my own personal development

Query in PHP:

// Load all records so can be displayed in list
function load_all($page = NULL, $chat_type = NULL){
    if(!$page)$page = 1;
    $start = ($page - 1) * ITEMS_PER_PAGE_ADMIN;

    $query = "";
    $query .= "SELECT direct_message.*, IFNULL(direct_message_group.staff_id, 0) as staff_id FROM `direct_message`
               LEFT JOIN direct_message_group ON direct_message_group.chat_id = direct_message.id
               INNER JOIN direct_message_thread ON direct_message_thread.chat_id = direct_message.id
               WHERE";

    $query .= " (
                    (
                        direct_message.recipient_id = '".addslashes(session::getAdministratorStaff()->id)."'
                        OR direct_message.creator_id = '".addslashes(session::getAdministratorStaff()->id)."'
                        OR (
                            direct_message_group.staff_id = '".addslashes(session::getAdministratorStaff()->id)."'
                            AND direct_message_group.active = '0'
                        )
                    )
                )";

    $query .= " AND direct_message.school_id = '".addslashes(session::getAdministratorStaffSchoolID())."'
                AND direct_message_thread.school_id = direct_message.school_id
                GROUP BY direct_message.id
                ORDER BY direct_message_thread.inserted DESC LIMIT $start, ".ITEMS_PER_PAGE_ADMIN;

    $rows = database::select($query);
    foreach($rows as $row){
        $holding = new direct_message();
        $holding->_populate($row);
        $this->direct_messages[] = $holding;
    }
}

Table structures:

---------------------------------------------------

---------------------------------------------------

Some thoughts:

You should consider using prepared statements.
Remove $chat_type as parameter, as it is not used
Consider stronger validation on $page_type() parameters(like that is it null or integer). Right now you can easily break the function
Don't use * selects.  They don not help the code reader understand what sort of data is being returned without having to look a database.  They also tend to cause more bandwidth consumption than is necessary and make your queries fragile.  What if you add a new audit field to the database table?  That could then break your object instantiation later in the code if an unexpected field shows up or cause you to go modify that object's class for some reason that is unrelated to what that object does.
Consider COALESCE instead of IFNULL for determining conditional values (this is more suitable function in that it does not have potentially hidden value typing behavior). This is probably more common as well amongst MySQL developer types.
Write your query in a more readable form (not a series of concatenations, drop unnecessary parenthesis, ne consistent around using backticks, and capitalization of reserved words, etc.)

For example:

SELECT
    direct_message.{field} AS {field_alias},
    {other fields you need} AS {field_aliases},
    COALESCE (direct_message_group.staff_id, 0) AS staff_id
FROM direct_message
LEFT JOIN direct_message_group
    ON direct_message_group.chat_id = direct_message.id
INNER JOIN direct_message_thread
    ON direct_message_thread.chat_id = direct_message.id
WHERE
    (
        direct_message.recipient_id = ?
        OR direct_message.creator_id = ?
        OR (
            direct_message_group.staff_id = ?
            AND direct_message_group.active = '0'
        )
    )
    AND direct_message.school_id = ?
    AND direct_message_thread.school_id = direct_message.school_id
GROUP BY direct_message.id
ORDER BY direct_message_thread.inserted DESC
LIMIT ?, ?

Here I have applied named parameters instead of you string concatenation.

You query makes you relationship between tables seem unclear. On one hand you join direct_message to direct_message_thread on some sort of chat id value, yet you also put in a WHERE condition that school id's for these records on these tables must be equal.  Should this just be a part of the join condition as opposed to WHERE clause filter?
addslashes() may not be appropriate for escape purposes and it is not being used to escape $start (offset value).  I would suggest you either use prepared statements or an appropriate DB-connector-specific escape functionality. Also, if you know the values of your class methods calls are going to be integers, for example, do you really need to escape these?
You object instantiation for direct_message class seems weird.  Why not pass the row data upon instantiation rather than having to call separate _populate() method with the data?  Why start that method name with an underscore?  Consider UpperFirstCamelCase for all class/interface names instead of snake_case.


Group duplicate files Here is my source code which group duplicate files together, written in Python 2.7. Any advice on smarter ideas to group duplicate files together more efficient, it will be great. Any general advice on code bugs and code style are also appreciated.

Problem statement:

I am given a list of files e.g. ["1.txt", "2.txt", "3.txt", "4.txt", "5.txt", "6.txt"]. I want to group all files which have the exact same content. Suppose in this example, file "1.txt", "2.txt", "3.txt" are the same, file "4.txt", "5.txt", "6.txt" have common header, but "4.txt", "6.txt" are exactly the same whole content. Then, the output should be two groups "1.txt", "2.txt", "3.txt" and "4.txt", "6.txt".

My major idea:

To avoid reading full content for each file, I generate a hash code for a file header (in my example, I define the file header to be the first two bytes of a file)
If more than 2 (>=2) files have the same header, I will read the full content and see if they are the exact same whole content

from collections import defaultdict
import itertools
def group_duplicate_files(files):
    hash_buf = defaultdict(list)
    for f in files:
        hash_buf[hash_header(f)].append(f)
    result = []
    for file_list in hash_buf.values():
        if len(file_list) < 2:
            continue
        for file_pair in itertools.combinations(file_list,2):
            if compare_full_content(file_pair[0], file_pair[1]):
                n = set()
                n.add(file_pair[0])
                n.add(file_pair[1])
                found = False
                # try to merge with existing set
                for i,r in enumerate(result):
                    if len(r.intersection(n)) > 0:
                        found = True
                        break
                if found:
                    n = n.union(result[i])
                    result.remove(result[i])
                    result.append(n)
                else:
                    result.append(n)
    return result

def hash_header(filename):
    f = open(filename, "r")
    header = f.read(10)
    return hash(header)
def compare_full_content(filename1, filename2):
    file1 = open(filename1, 'r')
    file2 = open(filename2, 'r')
    content1 = file1.read()
    content2 = file2.read()
    i = 0
    j = 0
    while i < len(content1) and j < len(content2):
        if content1[i] != content2[j]:
            return False
        i += 1
        j += 1
    if i < len(content1):
        return False
    if j < len(content2):
        return False
    return True

if __name__ == "__main__":
    files = ["1.txt", "2.txt", "3.txt", "4.txt", "5.txt", "6.txt"]
    print group_duplicate_files(files)

As far as I can tell, your algorithm is something like \$\mathcal{O}(n!)\$, because you compare all possible pairs of files. It is possible to do this in \$\mathcal{O}(n)\$.

You just have to iterate over the files and order them into a hashmap (i.e. dictionary) based on their header hash. Then, resolve the possible conflicts of two files with the same header, but different content, which takes \$\mathcal{O}(k)\$, where \$k\$ is the number of files with the same header. So in the worst case (all files have the same header), this algorithm is \$\mathcal{O}(2n)\$.

from collections import defaultdict
import sys

def hash_content(file_name, length=-1):
    with open(file_name) as f:
        return hash(f.read(length))

def group_by_header(*file_names):
    file_groups = defaultdict(list)
    for file_name in file_names:
        file_groups[hash_content(file_name, length=10)].append(file_name)
    return file_groups

def group_by_content(*file_names):
    file_groups = group_by_header(*file_names)
    for file_names in file_groups.values():
        if len(file_names) > 1:
            while file_names:
                file_name = file_names.pop()
                file_groups[hash_content(file_name)].append(file_name)
    return filter(None, file_groups.values())

if __name__ == "__main__":
    file_names = sys.argv[1:]
    print group_by_content(*file_names)

This code has two caveats:

It assumes hash is collision-free (a fair assumption). We could pass the string content directly as a key and let python do the hashing. Then we would also get a strategy for the rare case that two strings have the same hash without being equal. But this way we avoid storing the possibly large file-contents as strings.
This uses the fact that dict.values() returns a list and not an iterator, so we can modify the dictionary in the loop here. This breaks in Python 3.x, but you specifically tagged this as Python 2.7. You can remedy this by introducing a new dictionary:

def group_by_content(*file_names):
    header_hashs = group_by_header(*file_names)
    content_hashs = defaultdict(list)
    for hash_, file_names in header_hashs.items():
        if len(file_names) > 1:
            while len(file_names):
                file_name = file_names.pop()
                content_hashs[hash_content(file_name)].append(file_name)
        else:
            content_hashs[hash_] = file_names
    return (val for val in content_hashs.values() if val)


PHP code to insert phone number and IP address into a table if not already present I am currently coding a PHP script that connects to a database and inserts a phone number and IP address if either item is not present in the table. I believe I have completed it and it is working but I want to make sure that I have used best practices in regards to security. I have read here about how to connect to MySQL and query it using a PDO and a prepared statement. I also read on Stack Overflow that had a fantastic explanation on how to do this. However, this is now a few years old.

I would like to know if my code follows current best practices and that it is secure against SQL injection.

if( isset($_POST['submit']))
{
//user data
$name = $_REQUEST['fullname'];
$numbers = $_REQUEST['number'];
$bedrooms = $_REQUEST['bedrooms'];
$date = $_REQUEST['date'];
$movingFrom = $_REQUEST['from-postcode'];
$movingTo = $_REQUEST['to-postcode'];
$typeOfJob = $_REQUEST['typeOfJob'];
$additionalInfo= $_REQUEST['message'];
$ip = $_SERVER['REMOTE_ADDR'];
$id= NULL;
//connection variables
$host='localhost';
$user='root';
$pass='';
$db='lookup';
$chset='utf8mb4';
$dns = "mysql:dbname=$db;host=$host;charset=$chset";
$options = [
PDO::ATTR_ERRMODE            => PDO::ERRMODE_EXCEPTION,
PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC,
PDO::ATTR_EMULATE_PREPARES   => false,
];

try {
    //Try to create a new PDO object and pass the vars
    $connection = new PDO($dns, $user, $pass, $options);

    //prepare the database for data and exectue
    $stmt = $connection->prepare("SELECT id, numbers, ip FROM numbers");
    $stmt->execute();

    //Loop through table and check for numbers and ips
    //If present set var to true and break from loop
    $present = false;
    foreach ($stmt->fetchAll() as $k=>$v)
    {
        if($v['ip'] == $ip or $v['numbers'] == $numbers)
        $present = true;
        break;
    }
    //If data present I will be redirecting and informing user
    if($present)
    {
        //TODO: send to different pages
        echo "present";
    }
    //Else insert the data into the table
    else
    {
        $sql = "INSERT INTO numbers (id, numbers, ip) VALUES (NULL, '$numbers', '$ip')" ;
        $stmt = $connection->prepare($sql);

        $stmt->bindParam(':id', $id);
        $stmt->bindParam(':numbers', $numbers);
        $stmt->bindParam(':ip', $ip);
        $stmt->execute();
        echo "Woohoo";
    }
} catch (PDOException $e) {
    echo 'Caught exception: ',  $e->getMessage(), "\n";
}

There is more code in the if statement but it is irrelevant here as it is for the HTML.

Obey PSR coding standards.  As a starting point, use proper indentation and spacing.  Here's a place to begin your research: https://www.php-fig.org/psr/psr-2/
If you haven't already, narrow how your incoming submitted data is delivered to your script.  If you expect your user's data to be coming from a form with  method=POST, then use $_POST instead of $_REQUEST.  This will reduce the points of data entry and make your coding intention clearer to future human readers of your script.
Variable naming is important! If you have incoming data that holds a "phone number", use a variable name that will never lead to confusion.  phone_number, phone, or mobile_phone, etc might be better choices than $numbers which comes from $_REQUEST["number"]. Truth be told, I don't know if the incoming data is a phone numbet, a serial number, or a chemical that makes things numb. What probably concerns me more than the term is the sudden change from singular to plural.  Is this data an array of numbers? I have to assume not based on the script to follow, but then why use a plural variable name.
I wanted to recommend some investigation into the possibility of using an autoincremented primary id with two unique columns, https://stackoverflow.com/q/5416548/2943403 but there seems to be some debate about stability and suitable environments.  Furthermore, I don't want to make any incorrect assumptions about your coding logic.  Ultimately, I always encourage developers to seek solutions that execute the least number of calls to the database.  If this script can be done by sending one INSERT query and processing the response, that would get a nod from me.
Because you are hunting for a specific id value and a specific numbers value, write those conditions directly into your SELECT query's WHERE clause.  This with speed up your processing, eliminate your break requirement, and simply make your code more direct / intuitive.
Since the purpose of your SELECT query is to determine if an id or number exists, you can write your SELECT clause depending on the response quality that you intend to deliver to the end user. 1. If you don't intend to inform the user about which value already exists in the database table, use COUNT(*). 2. If you are going to explain which value already exists, SELECT id, numbers with a LIMIT of 1.
Rather than extracting the full result set then feeding it to the foreach(), I recommend fetching as you iterate.  https://stackoverflow.com/a/12970800/2943403 That said, I think your code should only be processing a single row of data at most.
You could improve your variable naming in your loop.  $k actually represents the "index" of the current row in the result set, so $i would be meaningful, $index would be more so, but best would be to omit the declaration entirely because you never use it in your script.  $v is the current row's data.  I recommend $row.
Try to avoid using boolean flags like $present.  In most cases, after a bit of careful thought, you can redesign your script to break or continue or conditionally call a custom function as a means to avoid this extra declaration.
Your INSERT query is not using the prepared statement / named placeholders / binding properly.  http://php.net/manual/en/pdo.prepare.php  First, I would the id column is to receive a static NULL value -- you don't need a variable for this, just hardcode it directly into the query.  Second, never write variable values into your prepared statement -- see my link to the pdo manual regarding the proper syntax.
Finally, never present the raw $e->getMessage() details to the public.  You may wish to offer generalized feedback but don't give the specifics as a matter of good security practices.


T-SQL Getting Sequential events with first even criteria I have a query I am writing where I want output if a person has some service provided, then I want all the services they had provided after that and I don't want the individual returned if that is the only service they had (the criteria service)

I have some example data:

In this instance above I would not want the last row.

The code I have, I feel is verbose at best. I am confident it works but I think it could be written better.

This will get some sample data

declare @bmh_plm_ptacct_v as table (
    med_rec_no varchar(10)
    , ptno_num varchar(10)
    , adm_date date
    , dsch_date date
    , hosp_svc varchar(5)
)

insert into @bmh_plm_ptacct_v
values('123456','123456748','2017-12-18','2018-01-12','PSY'),
('123456','123456789','2018-01-17','2018-01-18','EME'),
('123456','123456889','2018-01-19','2018-01-21','EME'),
('123456','123478978','2018-01-25','2018-01-25','EME'),
('123456','123457979','2018-05-21','2018-05-21','EME'),
('123456','123458988','2018-06-03','2018-06-04','EME'),
('123456','123458989','2018-07-27','2018-08-14','PSY'),
('123456','123458990','2018-09-23','2018-09-24','EME'),
('123456','123459999','2018-09-25','2018-09-30','PSY')

declare @vReadmits as table (
    [index] varchar(10)
    , interim int
    , [readmit] varchar(10)
)

insert into @vReadmits
values('123458990','25','123459999')

declare @hosp_svc_dim_v as table (
    hosp_svc varchar(50)
    , hosp_svc_name varchar(100)
        , orgz_cd varchar(10)
)

insert into @hosp_svc_dim_v
values('PSY','Pyschiatry','s0x0'),
('EME','Emergency Department','s0x0')

SELECT Med_Rec_No
, PtNo_Num
, Adm_Date
, Dsch_Date
, hosp_svc
, CASE
    WHEN B.READMIT IS NULL
    THEN 'No'
    ELSE 'Yes'
    END AS [Readmit Status]
, [Event_Num] = ROW_NUMBER() over(partition by med_rec_no order by ADM_date)
, [PSY_Flag] = CASE WHEN hosp_svc = 'PSY' THEN '1' ELSE '0' END

INTO #TEMPA

FROM smsdss.bmh_plm_ptacct_v AS A
LEFT OUTER JOIN smsdss.vReadmits AS B
ON A.PtNo_Num = b.[INDEX]
    AND B.INTERIM < 31

WHERE Dsch_Date >= '01-01-2018'
AND dsch_date < '12-31-2018'

ORDER BY Med_Rec_No, A.Adm_Date
;

SELECT A.*
INTO #TEMPB
FROM #TEMPA AS A
WHERE A.hosp_svc = 'PSY'
;

SELECT B.*
INTO #TEMPC
FROM #TEMPA AS B
WHERE B.hosp_svc != 'PSY'
AND B.Med_Rec_No IN (
    SELECT DISTINCT Med_Rec_No
    FROM #TEMPB
)
;

SELECT Med_Rec_No
, PtNo_Num
, Adm_Date
, Dsch_Date
, hosp_svc
, [Readmit Status]
, Event_Num
, PSY_Flag
, [Keep_Flag] = ROW_NUMBER() OVER(PARTITION BY MED_REC_NO ORDER BY ADM_DATE)

INTO #TEMPD

FROM (
    SELECT B.*
    FROM #TEMPB AS B

    UNION ALL

    SELECT C.*
    FROM #TEMPC AS C
    WHERE C.Med_Rec_No IN (
        SELECT ZZZ.Med_Rec_No
        FROM #TEMPB AS ZZZ
        WHERE ZZZ.Med_Rec_No = C.Med_Rec_No
        AND C.Event_Num > ZZZ.Event_Num
    )
) AS A

ORDER BY MED_REC_NO, Event_Num
;

SELECT A.Med_Rec_No
, A.PtNo_Num
, CAST(A.ADM_DATE AS DATE) AS [Adm_Date]
, CAST(A.Dsch_Date AS DATE) AS [Dsch_Date]
, A.hosp_svc
, HS.hosp_svc_name
, A.[Readmit Status]
, A.Event_Num
, A.Keep_Flag

FROM #TEMPD AS A
LEFT OUTER JOIN SMSDSS.hosp_svc_dim_v AS HS
ON A.hosp_svc = HS.hosp_svc
    AND HS.orgz_cd = 'S0X0'
WHERE A.Med_Rec_No IN (
    SELECT DISTINCT ZZZ.MED_REC_NO
    FROM #TEMPD AS ZZZ
    WHERE Keep_Flag > 1
)

DROP TABLE #TEMPA;
DROP TABLE #TEMPB;
DROP TABLE #TEMPC;
DROP TABLE #TEMPD;

Review

Your temporary table names don't say much about what they present. This makes it hard to figure out what they mean. Consider using better, more meaningful names.

DROP TABLE #TEMPA;
DROP TABLE #TEMPB;
DROP TABLE #TEMPC;
DROP TABLE #TEMPD;

It gets worse by aliasing these temporary tables with different letters:

FROM #TEMPA AS A  -- Fair enough

FROM #TEMPA AS B  -- Mamma mia!

There is only a need for an order by in the resulting query and the analytical functions (row over), not in the temporary tables.

There is no need for the temporary tables, you could use CTE's instead.

Refactored Query

Fiddle containing OP + Refactored Query

This only refactors the query for readability. I am sure a more compact and optimized query could be found.

with ACC as (
  SELECT Med_Rec_No
       , PtNo_Num
       , Adm_Date
       , Dsch_Date
       , hosp_svc
       , CASE WHEN B.READMIT IS NULL THEN 'No' ELSE 'Yes' END AS [Readmit Status]
       , [Event_Num] = ROW_NUMBER() over(partition by med_rec_no order by ADM_date)
       , [PSY_Flag] = CASE WHEN hosp_svc = 'PSY' THEN '1' ELSE '0' END
  FROM bmh_plm_ptacct_v AS A
  LEFT OUTER JOIN vReadmits AS B
  ON A.PtNo_Num = b.[INDEX] AND B.INTERIM < 31
  WHERE Dsch_Date >= '01-01-2018'
  AND dsch_date < '12-31-2018'
)
, EMERG as (
  SELECT ACC.* FROM ACC WHERE hosp_svc = 'PSY'
)
, PSY as (
  SELECT ACC.*
  FROM ACC
  WHERE hosp_svc != 'PSY'
  AND Med_Rec_No IN (SELECT DISTINCT Med_Rec_No FROM EMERG)
)
, ACC_REL as (
  SELECT Med_Rec_No
       , PtNo_Num
       , Adm_Date
       , Dsch_Date
       , hosp_svc
       , [Readmit Status]
       , Event_Num
       , PSY_Flag
       , [Keep_Flag] = ROW_NUMBER() OVER(PARTITION BY MED_REC_NO ORDER BY ADM_DATE)
  FROM (
    SELECT * FROM EMERG
    UNION ALL
    SELECT * FROM PSY
    WHERE PSY.Med_Rec_No IN (
        SELECT e.Med_Rec_No
        FROM EMERG AS e
        WHERE e.Med_Rec_No = PSY.Med_Rec_No
        AND PSY.Event_Num > e.Event_Num
    )
  ) AS A
)
SELECT A.Med_Rec_No
     , A.PtNo_Num
     , CAST(A.ADM_DATE AS DATE) AS [Adm_Date]
     , CAST(A.Dsch_Date AS DATE) AS [Dsch_Date]
     , A.hosp_svc
     , HS.hosp_svc_name
     , A.[Readmit Status]
     , A.Event_Num
     , A.Keep_Flag
FROM ACC_REL AS A
LEFT OUTER JOIN hosp_svc_dim_v AS HS
ON A.hosp_svc = HS.hosp_svc AND HS.orgz_cd = 'S0X0'
WHERE A.Med_Rec_No IN (
    SELECT DISTINCT rel.MED_REC_NO
    FROM ACC_REL AS rel
    WHERE Keep_Flag > 1
)
ORDER BY Med_Rec_No, Adm_Date
;


improve performance of recursive function in c# method I'm trying to write a function which will cache objects in memory.

The function below works, but performance is unbearable. What would you recommend to improve performance?

 public void SerializeInsertCache(object item)
        {
            //Get type of item object item
            Type type = item.GetType();

            //Get all of attributes type of item object
            TypeAttributes attributesType = type.Attributes;

            //Get object type full name for cach key
            string rootName = type.Name;

            string name = type.Name;
            PropertyInfo[] properties = type.GetProperties();
            PropertyInfo idRoot = properties.Where(p => p.Name == "Id").SingleOrDefault();

            int rootId = (int)idRoot.GetValue(item);

            string rootKey = "";

            foreach (var property in properties)
            {

                string propertyName = property.Name;
                var propertyValue = property.GetValue(item);
                Type propertyType = property.PropertyType;
                string propertyTypeName = propertyType.Name;
                bool cacheStatus = true;
                SerializeInsertCache(property);//recursive function

            }

           SaveInCache(item);
         }

Vast swaths of this code have no apparent use. Cutting out the code that does not appear to have an effect, you seem to be left with:

public void SerializeInsertCache(object item)
{
        Type type = item.GetType();
        PropertyInfo[] properties = type.GetProperties();

        foreach (var property in properties)
        {
            SerializeInsertCache(property);

        }

       SaveInCache(item);
}

Ben Voigt's point about not repeating reflection might look something like this:

private Dictionary<string,PropertyInfo[]> typeProperties = new Dictionary<string,PropertyInfo[]>();

public void SerializeInsertCache(object item)
{
    Type type = item.GetType();
    string name = type.Name;
    PropertyInfo[] properties;

    if (!typeProperties.TryGetValue(name, out properties))
    {
        properties = type.GetProperties();
        typeProperties.Add(name,properties);
    }

    foreach (var property in properties)
    {
        SerializeInsertCache(property,typeProperties);
    }

   SaveInCache(item);
 }

However, that said, I also think this is overcomplicating the problem. Why are you wanting to serialize these things in the first place? If they're being kept in memory, there is almost certainly a better approach (which we can't suggest until we know more about what problem you're solving).


QuickSort of Comparable[] Here is the code for the QuickSort class that I created.

public class QuickSort {

        public static void sort(Comparable[] a) {
            quicksort(a, 0, a.length-1);
        }

        private static void quicksort(Comparable[] a, int lo, int hi) {
            if(lo >= hi) return;
            int pi = partition(a, lo, hi);
            quicksort(a, lo, pi-1);
            quicksort(a, pi+1, hi);
        }

        private static int partition(Comparable[] a, int lo, int hi) {
            int i = lo + 1;
            int j = hi;

            while(i <= j) {
                if(a[i].compareTo(a[lo]) <= 0) {
                    i++;
                }
                else if(a[j].compareTo(a[lo]) > 0) {
                    j--;
                }
                else if(j < i) {
                    break;
                }
                else
                    exchange(a, i, j);
            }
            exchange(a, lo, j);
            return j;
        }

        private static void exchange(Object[] a, int i, int j) {
            Object tmp = a[i];
            a[i] = a[j];
            a[j] = tmp;
        }
}

The code passed some simple tests (including duplicate values). I wonder is any bug in my code? Anywhere to improve?

Javadoc on every method would be nice.

Change signature to <T extends Comparable<T>> void sort(T[] a).

public static void sort(Comparable[] a) {

Standard method to specify ranges is from inclusive to exclusive.

  quicksort(a, 0, a.length - 1);
}

private static void quicksort(Comparable[] a, int lo, int hi) {

Why not "high" and "low"?

  if (lo >= hi) {

Your code never invokes quicksort with lo > hi. You'd better throw an IllegalArgument exception in this case.

    return;
  }
  int pi = partition(a, lo, hi);
  quicksort(a, lo, pi - 1);
  quicksort(a, pi + 1, hi);
}

private static int partition(Comparable[] a, int lo, int hi) {
  int i = lo + 1;
  int j = hi;

You choose the first element as pivotal. Choosing as a pivot the element at a constant position allows construction of an antitest, an array on which your sort works in O(n^2). In your particular case, the antitest is pretty simple - an array in descending order, e.g. (5, 4, 3, 2, 1).

  while (i <= j) {
    if (a[i].compareTo(a[lo]) <= 0) {
      i++;
    } else if (a[j].compareTo(a[lo]) > 0) {
      j--;
    } else if (j < i) {
      break;
    } else {
      exchange(a, i, j);
    }
  }
  exchange(a, lo, j);
  return j;
}

It's better to name it swap.

private static void exchange(Object[] a, int i, int j) {
  Object tmp = a[i];
  a[i] = a[j];
  a[j] = tmp;
}


Multiple forms in django I have an account page where I have three forms. A user can change his name, his email address, and his password.

There are two difficulties I am having from trying to do this:

1) the request.user information is not updating accordingly (e.g., it will lag behind by one change or it will update if the form is not validated)

2) I have three separate forms, however, when I submit one form, I will get validation messages from another form. For example, if I submit the "Change Name" form, I will get 'This field is required from the password fields in the password form. Here is what I currently have --

in forms:

# in forms.py
from django.contrib.auth.models import User

class ChangeNameForm(ModelForm):
    first_name = forms.CharField(required=True)
    last_name = forms.CharField(required=True)
    class Meta:
        model = User
        fields = ('first_name', 'last_name' )

class ChangeEmailForm(ModelForm):
    email = forms.EmailField(required=True)
    class Meta:
        model = User
        fields = ('email',)

in views:

@login_required
def account(request):
    name_message = password_message = email_message = ''
    change_name_form = ChangeNameForm(data=request.POST or None, instance=request.user)
    change_password_form = PasswordChangeForm(data=request.POST or None, user = request.user)
    change_email_form = ChangeEmailForm(data=request.POST or None, instance=request.user)
    if request.method == "POST":
        if "change_name" in request.POST and change_name_form.is_valid():
            change_name_form.save()
            name_message = 'Your name has been changed.'
        if "change_password" in request.POST and change_password_form.is_valid():
            change_password_form.save()
            password_message = 'Your password has been changed.'
        if "change_email" in request.POST and change_email_form.is_valid():
            ...
            email_message = 'Please click the link in your email to confirm changes.'
    return render_to_response('userprofile/account.html',
                       {'change_name_form': change_name_form,
                        'change_email_form': change_email_form,
                        'change_password_form': change_password_form,
                        'password_message': password_message,
                        'name_message': name_message,
                        'email_message': email_message,},
                        context_instance=RequestContext(request))

in template:

Change Name

    <form method="post" action="/account/change/" name='name'> {% csrf_token %}

Change name: {{user.first_name}} {{user.last_name}}

{{change_name_form.as_table}}

{{name_message}}

<input type="submit" value="Save Changes" name="change_name"/>

    </form>

Change Email

    <form method="post" action="/account/change/" name='email'> {% csrf_token %}

Change email: {{user.email}}

{{change_email_form.as_table}}

{{email_message}}

<input type="submit" value="Save Changes" name="change_email" />

    </form>

Change Password

    <form method="post" action="/account/change/" name='password'> {% csrf_token %}

Change password

{{change_password_form.as_table}}

{{password_message}}

<input type="submit" value="Save Changes" name="change_password"/>

    </form>

What do I need to do to solve the two issues of making sure the request.user information is current and making sure validation only runs for the current form? Also, would it be possible to run a for loop to reduce redundancy in the template code? If so, how would I do this given the fact that the two name fields?. Thank you.

Somewhere on "Stack Overflow" was question like your.

Main idea was remove None from form initialization.

if "change_name" in request.POST:
    change_name_form = ChangeNameForm(data=request.POST, instance=request.user)
    if change_name_form.is_valid():
        change_name_form.save()
        name_message = 'Your name has been changed.'
else:
    change_name_form = ChangeNameForm(instance=request.user)

if "change_password" in request.POST:
    change_password_form = PasswordChangeForm(user=request.user)
    if change_password_form.is_valid():
        change_password_form.save()
        password_message = 'Your password has been changed.'
else:
    change_password_form = PasswordChangeForm(user=request.user)

if "change_email" in request.POST:
    change_email_form = ChangeEmailForm(request.POST, instance=request.user)
    if change_email_form.is_valid():
        ...
        email_message = 'Please click the link in your email to confirm changes.'
else:
    change_email_form = ChangeEmailForm(instance=request.user)

This is my solution. I hope it's help you.


Get list of all combinations from a set Given a tuple of elements, in elementTupleParam and the number in each combination r, we compute all the possible combinations of r elements, in a list of tuples. This is a recursive implementation.

One "challenge" is the inputs r, n, elementTuple, and the list holding the results, combinationList, are shared for all invocations of the recursive function. One method is to use global variables and bind them like below; which I think is rather ugly/bloated. Another method is to pass these are parameters to the recursive function, which may result in long parameter lists. Yet another is to make partialCombinations an inner function of getCombinations.

Another issue is the choice of data structure to hold the partial combinations and the result. I chose tuple because it's immutable: prevTuple is shared across many subsequent invocations, and is less prone to errors caused by accidental modifications.

What do you think? Any thoughts on these or other aspects would be appreciated.

r = 0
n = 0
elementTuple = ()
combinationList = []

# get the vanilla combinations, nCr in number
def getCombinations(elementTupleParam, rParam):
    global r
    global n
    global elementTuple
    global combinationList

    r = rParam
    elementTuple = tuple(elementTupleParam)
    n = len(elementTuple)
    combinationList = []

    if r == 0 or n == 0 or r > n:
        return combinationList

    partialCombinations((), -1)

    return combinationList

def partialCombinations(prevTuple, prevVisitedIndex):
    for thisIndex in range(prevVisitedIndex + 1, n):
        thisTuple = prevTuple + (elementTuple[thisIndex],)

        # If we exhausted all indices and have not formed a long enough tuple, then this is a dead-end, and don't do anything
        if thisIndex == n - 1 and not len(thisTuple) == r:
            continue

        if len(thisTuple) == r: # Found combination
            combinationList.append(thisTuple)
            continue

        partialCombinations(thisTuple, thisIndex)

#sanity check, should be moved to a unit test in another file
if __name__ == '__main__':
    result = getCombinations((2,3,4), 2)
    print(result)

Style

I suggest you check PEP0008 https://www.python.org/dev/peps/pep-0008/ the official Python style guide while writing your code and the following goes accordingly:

Docstrings: Python Docstring is the documentation string which is string literal, and it occurs in the class, module, function or method definition, and it is written as a first statement. Docstrings are accessible from the doc attribute for any of the Python object and also with the built-in help() function can come in handy. I suggest you include docstrings to your functions indicating what they do and what they return instead of writing a comment above each function.
Python naming conventions:

def getCombinations(elementTupleParam, rParam):

def partialCombinations(prevTuple, prevVisitedIndex):

Function names should be lowercase, with words separated
by underscores as necessary to improve readability same goes for
parameter names.
Descriptive variable names: thisIndex thisTuple prevTuple are examples of bad non-descriptive names. Names should reflect the significance of the variable/function/parameter they represent and the less ambiguous they are, the more readable is your code.

Code

Global variables: are bad in Python and programming languages in general and are advised against. I suggest enclosing variables inside functions. The reason they are bad that they may allow functions to have hidden/hard to detect side effects leading to an increased complexity potentially leading to Spaghetti code. Examples of good use of global variables include: algorithm optimization - caching and memoization.
There is a built-in Python library itertools that does the same functionality using itertools.combinations() which saves you the effort of implementing the thing yourself.

Code can be shortened into the following:
And according to GZO's comment, there is no need for the warpper function get_combinations(), use combinations() directly. I included it just for the sake of the example.

from itertools import combinations

def get_combinations(n: list, r: int):
    """Return combinations iterator."""
    return combinations(n, r)

if __name__ == '__main__':
    print(list(get_combinations([2, 3, 4], 2)))

Check these references(regarding global variables):

https://softwareengineering.stackexchange.com/questions/148108/why-is-global-state-so-evil
https://stackoverflow.com/questions/19158339/why-are-global-variables-evil


Thread safe templatized singleton class #include "stdafx.h"
#include <exception>
#include <string>
#include <sstream>
#include <iostream>
#include <atomic>
template <typename T>
class Singleton
{
    //thread safe atomic variable to check whether client is creating more than  one instance
    static std::atomic_flag flag ;
public:
    static T& getinstance()
    {
        //I am delibreatly allocating it on heap so that I would not worry about its time of deinitilization
        //static variable initialization is thread safe
        static T * t = new T();
        return *t;
}
    //exception class- kept it as Internal
    class SingletonException : public std::runtime_error
    {
    public:
    SingletonException(const std::string& exString) :std::runtime_error(exString)
        {

        }
    };
protected:
    Singleton()
    {
        if (flag.test_and_set())
        {
        std::ostringstream  ex;
            ex << "More than one object of Singleton class " << typeid(T).name();
            throw SingletonException(ex.str());
        }
    }
    ~Singleton()
    {
    }
private:
        Singleton(const Singleton&) = delete;
        Singleton& operator= (const Singleton&) = delete;
        Singleton(Singleton&&) = delete;
        Singleton& operator= (Singleton&&) = delete;
};
template <typename T>
std::atomic_flag  Singleton<T>::flag;
class Myclass: Singleton<Myclass>
{
    public:
        Myclass()
        {
        }
public:
    friend Singleton<Myclass>;
};
int main()
{
try
    {
        Singleton<Myclass>::getinstance();
        Myclass obj;
        Singleton<Myclass>::getinstance();
    }
    catch (const Singleton<Myclass>::SingletonException& ex)
    {
        std::cout << ex.what();
    }
    getchar();
    return 0;
}

Few points:

I am inheriting it so that I can mark delete copy/move operations, but inheritance is mandatory
This template class should be kept as friend class of client class
If more than one object is created then it will throw exception provided client has inherited from it.

It is more like code review. But do you think it is really thread safe GOOD singleton.

Don't use the Singleton pattern

  But do you think it is really thread safe good singleton?

Please excuse my pedantry and let me say that the only good use of the Singleton pattern is to not use it at all. This is not to say that a program should never have just a single instance of a class. On the contrary, it is a perfectly reasonable thing at times. Just don't couple the “unique object” property to the type itself as the pattern does. Instead, write a normal class with a public constructor and let another class provide a unique instance of it. The internet is full of rants about the Singleton pattern and good reasons why not to use it so I'll leave it at that.

Watch your #includes

If you're using the typeid operator, you must #include the <typeinfo> header first.

The #include "stdafx.h", on the contrary, is non-portable and annoying. Get rid of it.

For std::runtime_error, you don't need the <exception> but the <stdexcept> header.

Use vertical and horizontal white-space for better readability

It might be just an artifact of copying and pasting your code from your editor into the web browser but as it stands, your code is not very well formatted.

I would put a blank line between functions and classes as well as after the #include directives at the top. The only blank line in your code is – in an empty function body.

There are many opinions about the optimal amount of horizontal space you use to indent your code and I'm okay with most styles. What is not okay is inconsistency. Pick one style and stick to it. Some lines of your code appear completely off. For example, the closing brace of Singleton::getinstance (looks as if it were the end of the class) or the try in main.

Any decent editor can re-indent the code for you at the press of a button.

What's this std::atomic_flag?

I'm not sure what problem exactly the Singleton::flag member is supposed to solve but I'm pretty sure that there's a better way. If you don't want client code to instantiate your singleton class directly (which, as mentioned, is a poor decision, but ignore that for now) just make the constructor private. Then the compiler can enforce at compile-time that nobody except the singleton provider instantiates it. This is much better than checking at run-time and crashing the program.

Reconsider the heap allocation

I don't see why you have to allocate your singleton instance dynamically. The only situation where I could imagine a problem with static deinitialization is if you refer to the object from within a function registered as an std::atexit handler. On the contrary, dynamically allocating the object and then “leaking” it might cause leak detectors such as Valgrind to give you “false” positives that obscure more severe issues with your code. I prefer code to always clean up everything such that it is Valgrind clean. If you're writing a library, it is even more frustrating for your users to get leaks reported that they cannot do anything about.

Returning a reference is fine

I see many Singleton implementations for C++ returning a pointer just because they can. I like your choice of using a reference better.

You don't need an empty destructor

Your inline definition of a non-virtual empty destructor adds nothing. It might only confuse people whether you might have actually forgotten to put a delete of the instance there. (In which case it would of course have been better to use a std::unique_ptr and again write no destructor.)

Make deleted member functions public

It might sound counter-intuitive at first but making deleted members functions public gives you better error messages. It also makes sense if you think about it: The fact that your type is not copyable is part of its public interface.

Did you mean MyClass to inherit privately from Singleton<MyClass>?

I think it would be less surprising if you'd inherit publicly because then you could also say MyClass::getinstance() instead of the more verbose and less intuitive Singleton<MyClass>::getinstance().

The code in MyClass is completely redundant

If you delete everything inside the MyClass body, your program does the exact same thing. I'm sure that you did intend something by writing it but made some mistake (such as accidentally making the constructor public) that renders it useless as it stands.

Get rid of the getchar abomination in main

Don't make your program block on an I/O operation just because you want the terminal emulator window stay open. Instead, learn how to use the emulator such that the window stays open even after the program finished.

If you really want it, it should be std::getchar and you need the <cstdio> header.

How I would do it

Putting the previous advice together, I would solve the problem like this.

template <typename T>
struct singleton_provider
{
  static T&
  instance()
  {
    static T theinstance {};
    return theinstance;
  }
};

struct my_class
{
  my_class(const my_class&) = delete;
  my_class& operator=(const my_class&) = delete;
};

using my_class_provider = singleton_provider<my_class>;

It is a lot simpler, more efficient, doesn't leak memory and – most of all – doesn't couple the logic how objects of a class are instantiated and their lifetime is managed in a program to its type.


Instantiating a kind of NumPy array I am creating a class object which must inherit from numpy ndarrays.  I perform an isinstance check and an array.shape check with the __new__ method.  Is it alright to have it here, or should it be elsewhere?  I saw suggestions to create an exceptions class to accompany it, but it doesn't seem necessary... maybe preferable.

'''
pnts_instantiation.py
'''
import numpy as np
import sys
from types import *

class Points(np.ndarray):
  '''ndarray required,info='array info',name='pnts'')
  '''
  def __new__(cls,arr=None,info=None,name='pnts'):
    '''create Points from existing data'''
    err = 'Failed...Requires an ndarray...\nProvided: {}'.format(type(arr))
    if isinstance(arr,(NoneType,StringType,UnicodeType,ListType,TupleType)):
      return err
    if arr.shape < (4,2):
      return ('Failed...Requires array shape > (4,2)\nProvided: {}'.format(arr.shape))
    self = np.asarray(arr).view(cls)   # view as Points class
    self.info = info          # set info
    self.name = name          # set name
    self.applied = None
    # other properties not pertinent to discussion removed for simplicity
    return self

  def __array_finalize__(new_arr, src_arr):
    '''new_arr: new Points object...housecleaning takes place
         for explicit, view casting or new from template...
       src_arr: None, any subclass of ndarray including our own OR another
         instance of our own array (see docs)'''
    if src_arr is None: return
    new_arr.applied = getattr(src_arr,'applied',None) # provide a default
    new_arr.name = getattr(src_arr,'name',None)

  def __array_wrap__(self,out_arr,context=None):
    '''wrap it up'''
    return np.ndarray.__array_wrap__(self, out_arr, context)

  def __repr__(self):
    '''return point info, shape and dtype'''
    s = self
    st = '==> {}'.format(s)
    if (hasattr(s,'name')) and (hasattr(s,'info')):
      st = 'name:  {}\nshape: {}\ninfo:  {}\nvalues:\n{}'.format(s.name,s.shape,s.info,s)
    elif (hasattr(s,'name')) and (hasattr(s, 'applied')):
      st = '{}.{}:  {}'.format(s.name,s.applied, s)
    else:
      st = '{}: {}'.format(s.applied,s)
    return st

def test_cases():
  '''conditional case check'''
  cases = [None,
           'string',
          [[1,2],[3,4]],
          np.asarray([[1,2],[3,4]],dtype='float64',),
          np.ma.asarray([[1,2],[3,4],[5,6]],dtype='float64'),
          np.asarray([[1,2],[3,4],[5,6]],dtype='float64'),
          np.asarray(zip(np.arange(5),np.arange(5)),dtype='float64')
          ]
  counter = 0
  for a_case in cases:
    print('\nCase: {}\nIn: {}\nOut: {}'.format(counter,a_case,Points(a_case)))
    counter += 1

#-------------------
if __name__ == '__main__':
  test_cases()

Well I agree with the exception sentiment.  Errors are communicated by
exceptions (unless it is an exceptional case, but really, what you have
seems very standard, I don't see a reason to deviate here).

The typecheck for the input should probably be revised.  It's not
(immediately) clear to me what input arrays you actually want to accept.
In any case the isinstance check should probably be done the other way
round, i.e. check what type you want to accept (just np.ndarray?)
instead of the ones you don't want (because the reader has to infer what
other types there are).  It might also be easier to defer the check
until after np.asarray has returned a result, because afaik things
like nested lists can actually be converted meaningfully as well (unless
you don't want that obviously).

The shape check needs to be tighter as well, e.g. (4, 1, 1) < (4, 2)
is still true.

The method __array_wrap__ is just passing through the values, so is it
even needed?

The first argument of __array_finalize__ is self, so in the interest
of clarity I'd use that as well, although I can see the point in
expressing that it's the new array as well.

The use of hasattr in __repr__ strikes me as a bit odd, since in the
construction of the object you do assign them, so is there a case when
neither name, info, nor applied are available?  I'd also add test
cases if you rely on the output of repr.

As far as I can see from the
subclassing doc
the code seems fine otherwise.

Style-wise the indentation in Python code is four spaces, not two, there
are spaces missing in some method argument lists and I'm sure a tool
like pep8 will pick up more issues if you were to run it on the code.


Income tax calculator I've just started coding and have written a program to calculate your tax to be paid. Is there anywhere where I can improve this?

while True:
    try:

      income = int(input("Please enter your taxable income: "))
  except ValueError:
      print("Sorry, I didn't understand that please enter taxable income as a number")

      continue
  else:

      break
  if income <= 18200:
      tax = 0

  elif income <= 37000:
      tax = (income - 18200) * 0.19

  elif income <= 90000:
      tax = (income-37000) * 0.235 + 3572

  elif income <= 180000:
      tax = (income - 90000) * 0.37 + 20797

  else:
      tax = (income - 180000) * 0.45 + 54097

  print("you owe", tax, "dollars in tax!" )

Given the bad indentation of the original code, I will assume you meant to post code that looks like this

while True:
    try:

        income = int(input("Please enter your taxable income: "))
    except ValueError:
        print("Sorry, I didn't understand that please enter taxable income as a number")

        continue
    else:
        break

if income <= 18200:
    tax = 0

elif income <= 37000:
    tax = (income - 18200) * 0.19

elif income <= 90000:
    tax = (income-37000) * 0.235 + 3572

elif income <= 180000:
    tax = (income - 90000) * 0.37 + 20797

else:
    tax = (income - 180000) * 0.45 + 54097

print("you owe", tax, "dollars in tax!" )

I would suggest you use an IDE or a linter, both of which will point out syntax problems in the code.

The idea behind the code can be summarized as

Get the income from user input
Calculate the tax
Print the amount owed

This is a good structure to have. You have three clear boundaries and have split the work appropriately. You can make this explicit with well named functions. I would lay it out like this

def get_income():
    while True:
        try:

            income = int(input("Please enter your taxable income: "))
        except ValueError:
            print("Sorry, I didn't understand that please enter taxable income as a number")

            continue
        else:
            break

    return income

def compute_tax(income):
    if income <= 18200:
        tax = 0
    elif income <= 37000:
        tax = (income - 18200) * 0.19
    elif income <= 90000:
        tax = (income - 37000) * 0.235 + 3572
    elif income <= 180000:
        tax = (income - 90000) * 0.37 + 20797
    else:
        tax = (income - 180000) * 0.45 + 54097

    return tax

if __name__ == "__main__":
    income = get_income()
    tax = compute_tax(income)
    print("you owe", tax, "dollars in tax!")

The advantage of this is that your code for working out how much tax is owed is easy to use in another python module.

# tax_credits.py
from tax import compute_tax
...

If we look at how the tax is computed, the pattern is very clear. At each tax bracket we subtract an amount, multiply by a percentage, and add back an amount

tax = (income - S) * P + A

We could change the code to first figure out the tax bracket the income falls into, then compute the tax amount. While this doesn't look any better now, it will be beneficial to explore this path.

def compute_tax(income):
    if income <= 18200:
        S, P, A = 0, 0, 0  # Values picked so the tax amount is always 0.
    elif income <= 37000:
        S, P, A = 18200, 0.19, 0
    elif income <= 90000:
        S, P, A = 37000, 0.235, 3572
    elif income <= 180000:
        S, P, A = 90000, 0.37, 20797
    else:
        S, P, A = 180000, 0.45, 54097

    tax = (income - S) * P + A
    return tax

Since this repeated code now looks a bit easier to manager, let's turn it into a loop. I'll leave the details out as there are a few features of python that might be new to you, and are worth looking up yourself.

def compute_tax(income):
    # (C,      P,       A)
    # (cutoff, percent, additive)
    # S is reused from the previous iteration of the loop
    # so no need to store it.
    tax_brackets = (
        (18200, 0, 0), (37000, 0.19, 0), (90000, 0.235, 3572), (180000, 0.37, 20797)
    )
    # The final bracket, use if the income is bigger than any cutoff
    last_bracket = (None, 0.45, 54097)
    previous_cutoff = 0
    for cutoff, percent, additive in tax_brackets:
        if income <= cutoff:
            break
        previous_cutoff = cutoff
    else:
        # If we get here we never found a bracket to stop in
        _, percent, additive = last_bracket

    tax = (income - previous_cutoff) * percent + additive
    return tax

We should probably include some sanity checks incase somebody incorrectly changes values. As an example, we could check each cutoff is bigger than the last.

Note that this code is a bit denser than the original, and I don't know if I would recommend using a loop. If the original code ever gets more complex this is how I would try and simplify it. But until then I would leave the explicit if/elif/else statements in previous suggestion.

def get_income():
    while True:
        try:

            income = int(input("Please enter your taxable income: "))
        except ValueError:
            print("Sorry, I didn't understand that please enter taxable income as a number")

            continue
        else:
            break

    return income

The logic here is pretty solid. I would be hesitant to change to much. The only thing I do not like is int, as it is perfectly reasonable to earn a fractional unit of the currency.

There are a lot of ways you could code this sort of function. Here is one alternative that removes a few unnecessary parts.

def get_income():
    while True:
        try:
            return float(input("Please enter your taxable income: "))
        except ValueError:
            print("Sorry, I didn't understand that please enter taxable income as a number")


(Python) Random Number Guessing Game I'm a beginner currently learning Python. I made a simple program, a random number guessing game, to apply in practice what I have already learned (not much) and hopefully be able to get a better understanding of the basics. Are there any more things or suggestions on how I could improve my code? I would be glad to hear some feedback and also, if possible, some tips on how to learn Python more efficiently so I can improve my programming skills. I would appreciate it a lot!

import random

print("Welcome to the Guessing Game!")
user_name = input("What should I call you? ")
print("Hello, " + user_name)

def gameplay():
    number = random.randint(1, 10)
    user_guess = ""
    guess_limit = 3
    no_of_guesses = 0
    out_of_guesses = False

    while user_guess != number and not(out_of_guesses):
        if no_of_guesses < guess_limit:
            user_guess = int(input("Guess the number from 1 to 10 that I'm thinking of... "))
            if user_guess < number:
                print("Your number was too low...")
            elif user_guess > number:
                print("Your number was too high...")
            no_of_guesses += 1
        else:
            out_of_guesses = True

    if out_of_guesses:
        print("You ran out of guesses, try again")
    else:
        print("You guessed it! Congratulations, you win the game!")

    def restart_game():
        user_reply = input("Do you want to play again? Type YES if you want to play again or NO if you want to quit... ")
        if user_reply.upper() == "YES":
            gameplay()
        elif user_reply.upper() == "NO":
            print("Script terminating. Exiting game...")
        else:
            print("Sorry, you didn't type \"YES\" or \"NO\"...")
            restart_game()
    restart_game()

gameplay()

A nice little guessing game you've got here.  Skill will only get you 70%, you still need luck to win.

Quoted Stings

The Python language gives you 4 different syntaxes for creating strings: "...", '...', """...""", and '''...'''.  The first two will allow you to embed quotes and double quotes, respectively, without escaping.  The latter two will allow you to embed either quote, as well as new-lines, without needing escapes.

            print("Sorry, you didn't type \"YES\" or \"NO\"...")

Here you want double quotes inside the string, and since you're using double quotes for your string, you've had to escape them.  If you had used a triple-quoted string:

            print('''Sorry, you didn't type "YES" or "NO"...''')

no escaping is necessary.

Code Organization

Your code presently looks like this:

import random

print("Welcome to the Guessing Game!")
user_name = input("What should I call you? ")
print("Hello, " + user_name)

def gameplay():
    ... # contents omitted for brevity

gameplay()

You've got imports, mainline code, a function definition, then more mainline code.  You should group all the mainline code together, not have it separated by other function definitions.

import random

def gameplay():
    ... # contents omitted for brevity

print("Welcome to the Guessing Game!")
user_name = input("What should I call you? ")
print("Hello, " + user_name)

gameplay()

Main Guard

It is highly recommended all mainline code be protected by a main guard:

import random

def gameplay():
    ... # contents omitted for brevity

if __name__ == '__main__':
    print("Welcome to the Guessing Game!")
    user_name = input("What should I call you? ")
    print("Hello, " + user_name)

    gameplay()

This allows the file to be imported by other modules.  You might think that this game code will never be imported by another module, because it is stand-alone code, but if you want to run any tests on the code, this file gets imported and the game starts to run messing up the test framework.  So always use this main guard.

Unnecessary Recursion

gameplay() calls restart_game().  Then restart_game() can call either gameplay() or restart_game().  Both of these are recursive calls, which add more and more frames to the program stack.  Eventually, the Python program stack will exceed its maximum limit and the program will crash.  The user will likely get bored long before the stack overflows, but some automated testing script which is testing various guessing strategies might play thousands of games within a second, and there the program would crash.

Note: Some languages utilize something called "Tail Call Optimization" (TCO) and optimize out these tail-recursive calls.  Python is not one of them; it doesn't do TCO.

Each of these recursive calls is easily replaced by a simple loop.  Let's start with the inner loop:

Do you want to play again?

This is a simple YES/NO question.  Many programs might need it (although they might not exactly phrase the question about a game.  You hid the function inside the gameplay() function, which was fine because it was tightly coupled to that outer function, but making it more general means we'll want it moved out to become its own top-level function:

def yes_or_no(prompt: str) -> bool:
    """
    Ask a "yes or no" question.

    The question will be repeated until the user responds with "YES" or "NO",
    but the user doesn't need to capitalize their response.

    Parameters:
        prompt: the yes/no question to ask the user.

    Returns:
        ``True`` if the user responds "YES", ``False`` if the user responds "NO".
    """

    user_reply = input(prompt).upper()
    while user_reply not in {"YES", "NO"}:
        print('Sorry, you didn't type "YES" or "NO")
        user_reply = input(prompt).upper()

    return user_reply == "YES"

A couple of points to highlight:

Type hints (eg, prompt: str, and -> bool) are optional, but very useful.
"""docstrings""" are also very useful.  If you run your program in an REPL (such as IDLE), type help(yes_or_no) at the >>> prompt, after the program has run.
The user input is converted to uppercase once, immediately after input() returns a value.  It is not converted to uppercase on each test (eg, user_reply.upper() == "YES" and user.reply.upper() == "NO")
The while-loop repeats the question, if invalid input is given.  No recursion.
The user_reply not in { ... } is an efficient way of testing for an invalid response.

Using the yes_or_no() reply

If the user answers "YES", your code recursively called gameplay() for another game.  Now that we have our yes_or_no() question function, let's use it and eliminate that recursion.  Let's refactor the mainline code into a guessing_game() function in the process:

import random

def gameplay():
    ... # contents omitted for brevity

def guessing_game():
    print("Welcome to the Guessing Game!")
    user_name = input("What should I call you? ")
    print("Hello, " + user_name)

    play_again = True
    while play_again:
        gameplay()
        play_again = yes_or_no("Do you want to play again? " +
                               "Type YES if you want to play again or NO if you want to quit... "):

    print("Script terminating. Exiting game...")

if __name__ == '__main__':

Now, we are calling gameplay() in a loop, instead of recursively.

The Guessing Loop

    while user_guess != number and not(out_of_guesses):
        if no_of_guesses < guess_limit:
            user_guess = ...
            ...
            no_of_guesses += 1
        else:
            out_of_guesses = True

This loop makes my head hurt.  You are looping once per guess, to a maximum of guess_limit guesses, plus one additional loop iteration to set out_of_guesses = True to terminate the loop.  It is that extra iteration that is really bizarre.  It works, but ... (shudder).

Let's try a completely different loop structure.  We have a guess_limit; let's make a loop based on that:

    for guess_number in range(guess_limit):
        user_guess = int(input("Guess the number from 1 to 10 that I'm thinking of... "))

That alone will ask the user to guess 3 times.  If they guess the number, we want to break out of the loop early.

    for guess_number in range(guess_limit):
        user_guess = int(input("Guess the number from 1 to 10 that I'm thinking of... "))

        if user_guess == number:
            print("You guessed it! Congratulations, you win the game!")
            break

But what about when they fail to guess it?  If the for loop finishes all iterations without ever break-ing out of the loop, it will execute an optional else: clause:

    for guess_number in range(guess_limit):
        user_guess = int(input("Guess the number from 1 to 10 that I'm thinking of... "))

        if user_guess == number:
            print("You guessed it! Congratulations, you win the game!")
            break

    else:
        print("You ran out of guesses, try again")

The complete function, with the low/high guessing hints added back in:

def gameplay(guess_limit=3):
    """
    A number guessing game.

    A random integer will be selected between 1 and 10.  You have to guess the
    number within the allotted number of guesses.

    Parameters:
        guess_limit: Number of guesses to allow.  Allow more guesses for an easier game
    """

    number = random.randint(1, 10)

    for guess_number in range(guess_limit):
        user_guess = int(input("Guess the number from 1 to 10 that I'm thinking of... "))

        if user_guess == number:
            print("You guessed it! Congratulations, you win the game!")
            break

        if user_guess < number:
            print("Your number was too low...")
        else:
            print("Your number was too high...")

    else:
        print("You ran out of guesses, try again")

I've made guess_limit a parameter, with a default of 3, to demonstrate another Python feature: default arguments.  gameplay() will play the game as normal, but you could call the function with gameplay(4) to make an easier variant of the game.


Nested loop to transform MongoDB results I have the following structure returned from MongoDB:

[Class]
    0:Class // -> this.data.userInfo as seen below in the code
        _id: "ksihHTAjdhsA9"
        createdAt:Tue Mar 29 2016 13:03:45 GMT-0400 (EDT)
        weightData: Array[3]
            0: Class
                name: "my name 1"
                value: "myval1"
                weight: 154
            1: Class
                name: "my name 2"
                value: "myval2"
                weight: 191
            2: Class
                name: "my name 3"
                value: "myval3"
                weight: 210

I need to loop through the weightData nested array and store the name and value in a dictionary variable to be used in a dropdown later. I have it working, however, I feel like this could be more efficient.

var dict = [];
_.each(this.data.userInfo, info => {
  if (info) {
    _.each(info.weightData, weightInfo => {
      dict.push({
          "name": weightInfo.name,
          "value": weightInfo.value,
          "weight": weightInfo.weight
      });
    });
  }
});

console.log(dict);

/*
[Object, Object, Object]
    0:Object
        name: "my name 1"
        value: "myval1"
        weight: 154
    1:Object
        name: "my name 2"
        value: "myval2"
        weight: 191
    2:Object
        name: "my name 3"
        value: "myval3"
        weight: 210
*/

It looks like you are basically doing a filter + flatMap. You can also use destructing:

var dict = _(this.data.userinfo).filter(Boolean).flatMap(info => {
  return info.weightdata.map(({name, value, weight}) => {
    return {name, value, weight};
  });
}).value();


Add values to multiple keys in one line I Have orders table, every order has different payment method and made by different user, has different total of money.

and i want to make a detailed report, i export date as a hash to the views, as follow :

orders = Order.where(state: 'paid').settled(false).order(showtime: 'ASC')

my_hash = {
  users: Hash.new { |h, k| h[k] = {
    showtimes: Hash.new { |h, k| h[k] = {
      movies:  Hash.new { |h, k| h[k] = {
        payment_methods: Hash.new { |h, k| h[k] = Hash.new(0) },
        total_cash: Hash.new(0), total_credit: Hash.new(0) } },
      total_cash: Hash.new(0), total_credit: Hash.new(0) } },
    total_cash: Hash.new(0), total_credit: Hash.new(0) } },
  total_cash: Hash.new(0), total_credit: Hash.new(0) };

orders.each do |o|
  user = o.user.name
  showtime = o.showtime.strftime('%m-%d %H:%M')
  movie = o.movie.name
  payment_method = o.payment_method

  my_hash[:users][user][:showtimes][showtime][:movies][movie][:payment_methods][payment_method][:tickets] += o.t_count
  my_hash[:users][user][:showtimes][showtime][:movies][movie][:payment_methods][payment_method][:money] += o.m_total
  my_hash[:users][user][:showtimes][showtime][:movies][movie][:total_cash][:tickets] += o.t_count
  my_hash[:users][user][:showtimes][showtime][:movies][movie][:total_cash][:money] += o.m_total
  my_hash[:users][user][:showtimes][showtime][:total_cash][:tickets] += o.t_count
  my_hash[:users][user][:showtimes][showtime][:total_cash][:money] += o.m_total
  my_hash[:users][user][:total_cash][:tickets] += o.t_count
  my_hash[:users][user][:total_cash][:money] += o.m_total
  my_hash[:total_cash][:tickets] += o.t_count
  my_hash[:total_cash][:money] += o.m_total
end

is there a better way, with less number of lines, can i fill the hash in one line every loop ?

I haven't tested this but you should be able to do something like:

my_hash = Hash.new { |h, k|
  if k == :total_cash
    h[k] = Hash.new { |h, k| h[k] = Hash.new(0) }
  else
    h[k] = Hash.new(&h.default_proc)
}

Other than that, I think it would be easier to read if you reorder it a bit:

my_hash = {
  total_cash: Hash.new(0), total_credit: Hash.new(0),
  users: Hash.new { |h, k| h[k] = {
    total_cash: Hash.new(0), total_credit: Hash.new(0) },
    showtimes: Hash.new { |h, k| h[k] = {
      total_cash: Hash.new(0), total_credit: Hash.new(0) },
      movies:  Hash.new { |h, k| h[k] = {
        total_cash: Hash.new(0), total_credit: Hash.new(0) },
        payment_methods: Hash.new { |h, k| h[k] = Hash.new(0) }
      }
    }
  }
}


Solution to the 0-1 knapsack I'm using a one-dimensional array with length \$W(\text{weight capacity})\$. Is this is any better than the solutions with a two-dimensional array \$W, n\$ where \$n\$ is the number of items? I'm looking for code review, optimizations and best practices.

class Thieves:

    def __init__(self, details, items):
        self.details = details
        self.items = items
        self.index = [None] * (self.details[1] + 1)

    def valuable(self):
        ind = 0

        while ind < len(self.items):

            to_be_added = set()

            for i in range(1, len(self.index)):

                if self.items[ind][0] <= self.details[1]:

                    if self.index[i] == self.items[ind][1] and i == self.items[ind][0]:
                        if self.index[i * 2] != None:
                            if self.index[i * 2] < self.index[i] * 2:
                                self.index[i * 2] = self.index[i] * 2
                        else:
                            self.index[i * 2] = self.index[i] * 2

                    if i == self.items[ind][0]:
                        if self.index[i] != None and self.index[i] < self.items[ind][1]:
                            self.index[i] = self.items[ind][1]

                        elif self.index[i] == None:
                            self.index[i] = self.items[ind][1]

                    if i - self.items[ind][0] > 0 and i - self.items[ind][0] != self.items[ind][0]: #and self.index[self.items[ind][0]] == self.items[ind]  #and self.index[self.items[ind][0]] == self.items[ind][0]:
                        if self.index[i - self.items[ind][0]] != None and self.index[i] == None:
                            value = self.index[i - self.items[ind][0]] + self.items[ind][1]
                            to_be_added.add((i, value))

                        elif self.index[i - self.items[ind][0]] != None and self.index[i] != None:
                            if self.index[i] < self.index[i - self.items[ind][0]] + self.items[ind][1]:
                                to_be_added.add((i,self.index[i - self.items[ind][0]] + self.items[ind][1]))

            #print to_be_added
            for i in to_be_added:
                self.index[i[0]] = i[1]
            to_be_added = set()

            ind += 1

        print self.index[self.details[1]]

def main():

    t = Thieves((5, 5), [[3, 5], [7, 100], [1, 1], [1, 1], [2, 3]])
    t.valuable()

    z = Thieves((12, 5), [[3, 5], [7, 100], [1, 1], [1, 1], [2, 3], [3, 10], [2, 11], [1, 4], [1, 7], [5, 20], [1, 10], [2, 15]])
    z.valuable()

    k = Thieves((4, 10), [[6, 30], [3, 14], [4, 16], [2, 9]])
    k.valuable()

When creating classes in Python 2, you need to have classes explicitly inherit from object, like this:

class MyClass(object):
    ...

If you're creating a class in Python 3, you don't need to explicitly inherit from object.

As the classic Stop Writing Classes puts it, one characteristic of a class that should be a function is if the class has only two methods, __init__, and another. This means that your Thieves class would become one method, like this:

def thieves(details, items):
    ...


SPOJ ADDREV challenge - Adding reversed numbers I'm a beginner Python (2.7) programmer, and I've just started solving basic problems on SPOJ. I correctly solved the ADDREV problem (http://www.spoj.com/problems/ADDREV/), however my code takes 0.06s time and 8.8M memory to run. The top Python 2.7 rankers for the problem take <0.005s and 4.00M. Since I am starting out and this is a simple code snippet, I would love to get specific tips on how I can optimise the code below to reach these levels of efficiency. I am reading popularly available tips to optimise Python code, including the use of profilers, however I would really appreciate any tips I can already get here.
The problem as described there:

Input
The input consists of N cases (equal to about 10000). The first line of the input contains only positive integer N. Then follow the cases. Each case consists of exactly one line with two positive integers separated by space. These are the reversed numbers you are to add.
Output
For each case, print exactly one line containing only one integer - the reversed sum of two reversed numbers. Omit any leading zeros in the output.

(Source: http://www.spoj.com/problems/ADDREV/)
Here's the code:
def reverse(num):   # Takes a string as input and reverses it, removing any leading or trailing zeros
    num = num.strip("0")
    return num[::-1]

num_cases = input()

for i in range(0, num_cases):
    usr_input = raw_input()
    arr = usr_input.split()
    reversed_sum = int(reverse(arr[0])) + int(reverse(arr[1]))
    print reverse(str(reversed_sum))

The biggest savings comes from computing the results directly from the textual representation of the inputs, instead of converting inputs to numbers, reversing them, adding them, reversing the result and then converting it to text.

Just add the character codes of the inputs to the current carry (initially 0, of course) and deduct the difference between '0' and 0 since the intermediate result contains it twice. If the result is greater than '9', subtract 10 and set the new carry to 1. Rinse and repeat.

P.S.: the task description talks about 'reversed' numbers but one might as well call them 'Little Endian'. That's why one can simply add the digits as they come in and push the result digits out, until the longer of the two inputs ends and the carry is down to 0...


Writing a set of JavaScript conditionals more efficiently There must be a better way to write this, maybe with a switch statement (which for some reason I can't get to work).  Basically, if one of the conditionals is true then the class in the statement is executed.  So, in this case, one or any number of combinations could be true.

if (stepOneEdited) {
    $(stepOne).addClass("complete");
}
if (stepTwoEdited) {
    $(stepTwo).addClass("complete");
}
if (stepThreeEdited) {
    $(stepThree).addClass("complete");
}
if (stepFourEdited) {
    $(stepFour).addClass("complete");
if (stepFiveEdited) {
    $(stepFive).addClass("complete");
    $(stepSix).addClass("complete");
}

I like @janos solution, but if that is not possible for you, you might extract the code into a function. I know, it's just one line of code, but I think it would still be beneficial:

// marks className element as complete if isEdit is true
function addComplete(isEdit, className) {
    if (isEdit) {
        $(className).addClass("complete");
    }
}

addComplete(stepOneEdited, stepOne);
addComplete(stepTwoEdited, stepTwo);
addComplete(stepThreeEdited, stepThree);
addComplete(stepFourEdited, stepFour);
addComplete(stepFiveEdited, stepFive);
// stepSix depends on stepFiveEdited, not stepSixEdited
addComplete(stepFiveEdited, stepSix);

This way, each additional step only takes one line instead of three, I think that this makes it a lot more readable (and also adding a new case is a lot easier). The other benefit is that it is easier to adapt this code if the specification changes. For example if the class name changes, or if instead of adding a class, the program should react differently, you only need to change it in one place.


Stable marriage in Haskell I implemented a stable marriage problem in Haskell few month ago. It's not optimized at all and I'd like to know how to make it better from performance and readability perspective.

data Sex = Male | Female deriving (Eq, Show)
data Virtue = Intelligence | Appearence | Kindness deriving (Eq, Show, Enum)

data Parameter = Parameter{
    virtue :: Virtue,
    value :: Int
} deriving (Eq, Show)

data Person = Person{
    name :: String,
    sex :: Sex,
    preferences :: [Virtue],
    parameters :: [Parameter],
    partner :: Maybe Person
} deriving (Eq, Show)

Results list of women sorted by preferences of one man by defaultRateFunction :: Person -> Person -> Int. In my implementation it depends on judges preferences and rated person parameters. I won't put it there for brevity. You can find full program in a link to Gist at the bottom of post. Imagine that function to be anything you want.

personalRating :: Person -> [Person] -> [Person]
personalRating x ys = sortBy (comparing (\y -> defaultRateFunction x y)) ys

Man makes an engagement proposal for the woman and if she don't have partner — she replies positively (True) and if she does, if new partner's rating > than the old one's — "returns" True and if it does not — False

proposal :: Person -> Person -> Bool
proposal male female
    | isNothing (partner female) = True
    | defaultRateFunction female male > defaultRateFunction female (fromJust $ partner female) = True
    | otherwise = False

Man makes a proposal for each woman in females untill he'll find the one who'll reply positively. Assumed that there are at least one of this type in the array

findTheBride :: Person -> [Person] -> Person
findTheBride male females
    | proposal male (head females) == True = head females
    | otherwise = findTheBride male (tail females)

The ugliest part is marrige algorhitm itself. As I call it recursively I have to clean person from array of corresponded sexes every time it finds partner and also check if woman has an ex-partner, and deal with thier "breakup" also.

marrige :: [Person] -> [Person] -> [Person]
marrige males females
        | sm == [] = females
        | isNothing ex = marrige
            (fsmWithNewPartner:(delete fsm males))
            (fsmPartnerWithFsm:(delete fsmPartner females))
        | otherwise = marrige
            (fsmWithNewPartner:((fromJust ex) {partner = Nothing}):(delete fsm $ delete (fromJust ex) males))
            (fsmPartnerWithFsm:(delete fsmPartner females))
    where
        sm = filter (\x -> partner x == Nothing) males -- Single males
        fsm = head sm -- Fist single male
        fsmPartner = findTheBride fsm (personalRating fsm females) -- Fist single male's partner
        ex = partner fsmPartner -- Partner's ex (Maybe)
        fsmWithNewPartner = fsm {partner = Just fsmPartner}
        fsmPartnerWithFsm = fsmPartner {partner = Just fsm}

Full version of program (where random Person data are generated) avialible on Gist.

import Safe (findJust)
import Data.Foldable (null, all)

personalRating = sortBy . comparing . defaultRateFunction

proposal m f = all (on (>) (defaultRateFunction f) m) $ partner f

remarry p x = (x {partner = p} :) . delete x

marrige :: [Person] -> [Person] -> [Person]
marrige ms fs = case find (null . partner) ms of
  Nothing -> fs
  Just m -> marrige
    (remarry (Just f} m $ maybe id (remarry Nothing) (partner f) ms)
    (remarry (Just m) f fs)
    where f = findJust (proposal m) $ personalRating m fs

You never actually use ms that have a partner. Why keep track of them? I'll assume that all start out without partners, otherwise filter once at the start. In particular, I'll assume the invariant that partnership is symmetric.

marrige :: [Person] -> [Person] -> [Person]
marrige [] fs = fs
marriage (m:ms) fs = marrige
  (maybe id (\ex -> (ex {partner = Nothing} :)) (partner f) ms)
  (f {partner = Just m} : delete f fs)
    where f = findJust (proposal m) $ personalRating m fs

For performance, you could use Data.Map instead of []'s delete.

Edit: Here's one where the explicit recursion is less global. Unsetting ex's partner may be superfluous.

import Control.Monad.State

marrige = execState . traverse_ go where go m = do
  f <- gets $ findJust (proposal m) . personalRating m
  modify $ (f {partner = Just m} :) . delete f
  for_ (partner f) $ \ex -> go $ ex {partner = Nothing}


First steps with amd64 assembly I try to learn amd64 assembler. This is the first thing I tried. This piece of assembly should replicate the functionality of the following piece of C code, which turns a binary sha-256 hash into a human readable form.

assembly code

1:  .ascii "0123456789abcdef"
    .global show_hash
show_hash:
    .type show_hash @function
    .func show_hash
    mov $32,%ecx
    .p2align 2
0:  xor %eax,%eax
    lodsb
    mov %eax,%edx
    shr $4,%al
    and $15,%dl
    mov 1b(%rax),%al
    mov 1b(%rdx),%dl
    mov %bl,%ah
    stosw
    dec %ecx
    jnz 0b
    mov %cl,(%rdi)
    ret
    .endfunc

C code

void show_hash(char *dst, unsigned char *src) {
  static const char *lookup = "0123456789abcdef";
  char lo, hi, byte;
  int i = 32;

  do {
    byte = *src++;
    hi = lookup[byte >> 4];
    lo = lookup[byte & 0xf];
    *dst++ = hi;
    *dst++ = lo;
  } while (i--);
}

Am I doing it right? I tried to move the lookup table (label 1) into .section rodata, but all references to it were changed to 0 in the linked program, so I put it into the text section for now.

I'm not sure whether it's correct:

If show_hash uses cdecl calling conventions you should preserve registers like %bl, and read the input parameter values from the stack into esi and edi.
(see section "3.2.3 Parameter Passing" of the Application Binary Interface: parameters are conveniently passed in the rdi and rsi registers)
I don't understand the syntax of mov 1b(%rax),%al: is it reading from the lookup array defined at label 1:? On re-reading, I think it is; however that will only work if the most-significant bytes of rax and rdx are all zero; perhaps you should initialize them as you did using xor %eax,%eax
(apparently xor %eax,%eax will clear the whole of %rax)
You're using stosw which writes two bytes (two ASCII characters) a time; but %ah contains  a value from %bl, and %bl wasn't previously initialized? I think that statement should have been mov %dl,%ah not mov %bl,%ah; or perhaps you could have done mov 1b(%rdx),%ah directly.
The function ends with mov %cl,(%rdi) to null-terminate the string; that is clever but could use a comment (it took me a bit to figure out). Most of the assembly questions either have good comments, or answer which say that they should have good comments.


What is better? For-loops or generator expressions (in cases like this) Which version of this code is better?

Ver 1.

a = 0
for i in xrange( 1000 ):
    if i % 3 == 0 or i % 5 == 0:
        a += i
print a

Ver 2.

from operator import add
print reduce( add,
              ( i for i in xrange( 1000 )
                if i % 3 == 0 or i % 5 == 0 ) )

Things I like about version one is that it is more readable, but it creates requires a variable. If you had to choose, which one would you argue is better?

The second version can (and should) be written using sum instead of reduce(add,...), which is somewhat more readable:

sum(i for i in xrange( 1000 ) if i % 3 == 0 or i % 5 == 0)

That is the version that I'd prefer as far as implementations of this particular algorithm go because it's more declarative than a loop and just reads nicer in my opinion. (It's also faster though that's less of a concern).

However it should be noted that for summing all numbers up to n that divisible by 3 or 5, there's actually a better algorithm than going over all those numbers and summing them.


Suggest how to extract method I have 3 methods(* BindingSource are bindingsources, context is data context, cache* -are some List for cache operations):

 private void AddUpdateRowDocuments()
        {
            try
            {
                tRANSMITDOCDOCUMENTSRELATIONSBindingSource.EndEdit();
                var t = tRANSMITDOCDOCUMENTSRELATIONSBindingSource.Current as WcfDataServiceReference.TRANSMIT_DOC_DOCUMENTS_RELATIONS;
                if (t.TRANSMIT_DOC_ID == 0)
                {
                    if (!cachetddrList.Contains(t))
                    {
                        cachetddrList.Add(t);
                    }
                    return;
                }
                context.UpdateObject(t);
            }
            catch (Exception ex)
            {
                logger.ErrorException(string.Empty, ex);
            }
        }
private void AddUpdateRowOrganizations()
    {
        try
        {
            tRANSMITDOCORGANIZATIONRELATIONSBindingSource.EndEdit();
            var t = tRANSMITDOCORGANIZATIONRELATIONSBindingSource.Current as WcfDataServiceReference.TRANSMIT_DOC_ORGANIZATION_RELATIONS;
            if (t.TRANSMIT_DOC_ID == 0)
            {
                if (!cachetdorList.Contains(t))
                {
                    cachetdorList.Add(t);
                }
                return;
            }
            context.UpdateObject(t);
        }
        catch (Exception ex)
        {
            logger.ErrorException(string.Empty, ex);
        }
    }
private void AddUpdateRowPartators()
        {
            try
            {
                tRANSMITDOCPARTATORRELATIONSBindingSource.EndEdit();
                var t = tRANSMITDOCPARTATORRELATIONSBindingSource.Current as WcfDataServiceReference.TRANSMIT_DOC_PARTATOR_RELATIONS;
                if (t.TRANSMIT_DOC_ID == 0)
                {
                    if (!cachetdprList.Contains(t))
                    {
                        cachetdprList.Add(t);
                    }
                    return;
                }
                context.UpdateObject(t);
            }
            catch (Exception ex)
            {
                logger.ErrorException(string.Empty, ex);
            }
        }

Any thoughts on how to improve them? My inner sense says that they can be turned into one generic method, but i have little experience with generics, so cant say if i am right.

Something along the lines of this. Generics aren't that difficult. Just replace every occurance of 'some desired type' with the generic identifier.

private void AddUpdateRow<T>(BindingSource bindingSource, List<T> cachedList)
    where T : ITransmitDocId
{
    try
    {
        bindingSource.EndEdit();
        T t = bindingSource.Current as T;
        if (t.TRANSMIT_DOC_ID == 0)
        {
            if (!cachedList.Contains(t))
            {
                cachedList.Add(t);
            }
            return;
        }
        context.UpdateObject(t);
    }
    catch (Exception ex)
    {
        logger.ErrorException(string.Empty, ex);
    }
}

The where constraint in the function definition should indicate a common interface/class in which TRANSMIT_DOC_ID is defined.


Comparing dates from sheets I am looking to increase the speed of this Excel VBA Nested Loop.  The loop compares dates from one sheet to a secondary sheet.  If they match, I change the border around the cell to highlight it.  It currently works fine, but takes about 30 seconds to process per sub. Is there a way to implement an array or other tactic to speed it up?

Sub Single()

Dim DateRng As Range, DateCell As Range, DateRngPay As Range
Dim cellA As Range
Dim cellB As Range
Dim myColor As Variant

Set DateRng = ActiveWorkbook.Worksheets("SS").Range("B11:F16,I11:M16,P11:t16,B19:F24,I19:M24,P19:t24,B27:F32,I27:M32,P27:t32,B35:F40,I35:M40,P35:t40")
Set DateRngPay = ActiveWorkbook.Worksheets("PS").Range("C2:C67")
myColor = Array("38")

If ActiveWorkbook.Worksheets("Info").Range("B67") = 1 Then
    With DateRng
            .Interior.ColorIndex = xlColorIndexNone
            .Borders.ColorIndex = 1
            .Borders.Weight = xlHairline
    For Each cellA In DateRng
        For Each cellB In DateRngPay
                If cellB.Value > "" And cellA.Value > "" And cellB.Value = cellA.Value Then
                With cellA.Borders
                    .ColorIndex = myColor
                    .Weight = xlMedium
                End With
                Exit For
            End If
        Next cellB
    Next cellA
    End With
End If
End Sub

Normally I'd go on and ramble about how your procedure's name should start with a verb, how the casing of your local variables isn't consistently camelCase, how the procedure is implicitly Public, how the indentation is broken and therefore confusing here:

With DateRng
        .Interior.ColorIndex = xlColorIndexNone
        .Borders.ColorIndex = 1
        .Borders.Weight = xlHairline
For Each cellA In DateRng
    For Each cellB In DateRngPay
            If cellB.Value > "" And cellA.Value > "" And cellB.Value = cellA.Value Then
            With cellA.Borders
                .ColorIndex = myColor
                .Weight = xlMedium
            End With
            Exit For
        End If
    Next cellB
Next cellA
End With

Compare to:

With DateRng
    .Interior.ColorIndex = xlColorIndexNone
    .Borders.ColorIndex = 1
    .Borders.Weight = xlHairline
    For Each cellA In DateRng
        For Each cellB In DateRngPay
            If cellB.Value > "" And cellA.Value > "" And cellB.Value = cellA.Value Then
                With cellA.Borders
                    .ColorIndex = myColor
                    .Weight = xlMedium
                End With
                Exit For
            End If
        Next cellB
    Next cellA
End With

...I'd also so mention that I find cellB.Value > "" a little weird when everyone else would have written cellB.Value <> "" or perhaps cellB.Value <> vbNullString.

I might even have wrecked my brains around trying to find a way to remove the need for a nested loop at all.

All of that would have been useful advice... but there's an even better one.

Ready?

Unless you have a VERY specific and cast-in-stone requirement for the border width, there's no need for any VBA code to do this.

First, name the range on SS!$C$2:$C$67, say MyValues - here I've populated the data with values 1-66, but anything will do:

On the PS sheet, select cell PS!$B$11 and create a conditional format for it, using this formula:

=AND(Info!$B$67=1,NOT(ISNA(MATCH(B11,MyValues,0))))

Decide how you want the conditional format to look like, and then change the applies to range to the cells you're interested in:

=$B$11:$F$16,$I$11:$M$16,$P$11:$T$16,$P$19:$T$24,$I$19:$M$24,$B$19:$F$24,$B$27:$F$32,$I$27:$M$32,$P$27:$T$32

I've shaded these target cells on my own sheet here, but as you can see, any value in these cells that's present on MyValues gets a yellow background and a black border...

...instantaneously.


Active Brownian Motion I am attempting to write a Python code to simulate many particles in a confined box. These particles behave in such a way that they move in the box in straight lines with a slight angular noise (small changes in the direction of the particle path). They should interact by acknowledging the other particle and 'shuffle/squeeze' past each other and continue on their intended path, much like humans on a busy street. Eventually, the particles should cluster together when the density of particles (or packing fraction) reaches a certain value.
However, I have a feeling there are parts of the code that are inefficient or which could be either sped up or written more conveniently.
If anyone has any improvements for the code speed or ideas which may help with the interactions and/or angular noise that would be much appreciated. I will also leave an example of an animation which is my aim: https://warwick.ac.uk/fac/sci/physics/staff/research/cwhitfield/abpsimulations
The above link shows the animation I am looking for, although I don't need the sliders, just the box, and moving particles. The whole code is shown below:
import numpy as np
import matplotlib.pyplot as plt
from matplotlib.animation import FuncAnimation

def set_initial_coordinates():
    x_co = [np.random.uniform(0, 2) for i in range(n_particles)]
    y_co = [np.random.uniform(0, 2) for i in range(n_particles)]
    return x_co, y_co

def set_initial_velocities():
    x_vel = np.array([np.random.uniform(-1, 1) for i in range(n_particles)])
    y_vel = np.array([np.random.uniform(-1, 1) for i in range(n_particles)])
    return x_vel, y_vel

def init():
    ax.set_xlim(-0.05, 2.05)
    ax.set_ylim(-0.07, 2.07)
    return ln,

def update(dt):
    xdata = initialx + vx * dt
    ydata = initialy + vy * dt
    fx = np.abs((xdata + 2) % 4 - 2)
    fy = np.abs((ydata + 2) % 4 - 2)

    for i in range(n_particles):
        for j in range(n_particles):
            if i == j:
                continue

            dx = fx[j] - fx[i]  # distance in x direction
            dy = fy[j] - fy[i]  # distance in y direction
            dr = np.sqrt((dx ** 2) + (dy ** 2))  # distance between x

            if dr <= r:
                force = k * ((2 * r) - dr)  # size of the force if distance is less than or equal to radius

                # Imagine a unit vector going from i to j
                x_comp = dx / dr  # x component of force
                y_comp = dy / dr  # y component of force

                fx[i] += -x_comp * force  # x force
                fy[i] += -y_comp * force  # y force
    ln.set_data(fx, fy)
    return ln,

# theta = np.random.uniform(0, 2) for i in range(n_particles)
n_particles = 10
initialx, initialy = set_initial_coordinates()
vx, vy = set_initial_velocities()
fig, ax = plt.subplots()
x_co, y_co = [], []
ln, = plt.plot([], [], 'bo', markersize=15)  # radius 0.05
plt.xlim(0, 2)
plt.ylim(0, 2)

k = 1
r = 0.1
t = np.linspace(0, 10, 1000)

ani = FuncAnimation(fig, update, t, init_func=init, blit=True, repeat=False)
plt.show()

Coordinates as a matrix
This:
x_co = [np.random.uniform(0, 2) for i in range(n_particles)]
y_co = [np.random.uniform(0, 2) for i in range(n_particles)]
return x_co, y_co

is a little baffling. You call into Numpy to generate uniformly distributed numbers (good) but then put it in a list comprehension (not good) and separate x and y into separate lists (not good).
Consider instead
return np.random.uniform(low=0, high=2, size=(2, n_particles))

and similarly for set_initial_velocities:
return np.random.uniform(low=-1, high=1, size=(2, n_particles))

All of the rest of your code, such as the calculations for xdata/ydata should similarly vectorize over the new length-2 dimension eliminating distinct code for x and y.


Merge the multiple observables responses into a single array Here's what I trying to do:
Given a paginated API, get all the resources using parallel requests.
The API returns a limited number of resources per call. So, you need to use an offset parameter to get to the next set of data until all data is extracted.
Here's my idea (but getting some warning because I'm using flat on the response), so maybe there's a better way to do this.

Get the total count of items.
Given the count and the limit, calculate how many requests are needed to get all data.
Trigger all requests in parallel and combine all data into a flattened array.

Here's an example:
https://stackblitz.com/edit/paginated-api?embed=1&file=index.ts&hideExplorer=1&devtoolsheight=100
getCount().pipe(
  mergeMap(count => range(0, Math.ceil(count / limit))),
  map(offset => getDevices(offset, limit)),
  combineAll(),
).subscribe(res => {
  const a = res.flat(); // <--- warning: Property 'flat' does not exist on type '{ name: string; }[][]'.
  console.log(JSON.stringify(a));
});

I feel this solution is a little hacky. It's flattening the response in the subscription. I'd like to know if there's an RXJS operator that I can use on the pipe to flatten the response, so I don't have to in the subscription?
How to merge the multiple observables responses into a single array?

Use mergeMap on the call to getDevices, then mergeAll to flatten the results, and toArray to convert back to an array:
getCount().pipe(
  mergeMap(count => range(0, Math.ceil(count / pageSize))),
  mergeMap(offset => getDevices(offset, pageSize)),
  mergeAll(),
  toArray()
).subscribe(res => {
  console.log(JSON.stringify(res));
});

See here: https://stackblitz.com/edit/so-rxjs-merge-results?file=index.ts


Writing a jQuery widget: Code layout I'm doing my first major jQuery development. It's a Widget for recurring events, and is as such a fairly complex beast. The full code is available at https://github.com/collective/jquery.recurrenceinput.js for those who want to check it out. I appreciate any kind of feedback about any part of the code, so anyone who wants to take a look at it, that would be awesome.

I have a couple of specific questions, and it seems best to put them as separate posts, so here is the first one:

Does my directory layout fit with what a jQuery developer would expect? Is it "best practice"?

The directory structure is this (somewhat simplified):

demo // Files to run the demo/tests:
    delete.png
    jquery.recurrenceinput.css
    overlays.css
    undelete.png
    input.html
    jquery.tools.dateinput.css
    pb_close.png

lib // jQuery files:
    jquery-1.4.2.js
    jquery.tmpl-beta1.js
    jquery.tools.dateinput-1.2.5.js
    jquery.tools.overlay-1.2.5.js
    jquery.utils.i18n-0.8.5.js

src // Actual code
    jquery.recurrenceinput.display.js
    jquery.recurrenceinput.form.js
    jquery.recurrenceinput.js

tests // Tests and supporting files

    jslint.js
    qunit.css
    qunit.js
    test.html
    tests.js

README.rst

lib is not really lib, but your project's dependencies. Normally you'd call it vendor

tests -> test. This one is just a convention.

I took a quick look at your code and the first thing I saw was snake_case. JS/jQuery convention is to use CamelCase.