Automatically update one-to-many bi-directional relationship In an example game engine server, the game world could be represented by a World object. The game might have multiple game worlds and a player who is in a world only needs to receive position data of units in that world.

class World
{
    public List<Unit> Units { get; set; }
}

However a piece of code also needs to be able to look up what world a unit is in easily, so the unit object itself keeps track of a world reference.

class Unit
{
    public World World { get; set; }
}

This works for lookup, but quickly becomes problematic when changing data when the programmer isn't aware of the relationship between objects going on, so I changed Units in World to be readonly and have the following code in Unit.

public virtual World World
{
    get { return _world; }
    set
    {
        // If already this, don't do anything
        if (value == _world) return;

        var oldWorld = _world;
        _world = value;

        if(oldWorld != null) oldWorld.UpdateUnitEntry(this);
        _world.UpdateUnitEntry(this);
    }
}

This works, but it feels like there's a better way to do this. Especially as I add more stuff that needs to be linked the same way (a World also has Structures and Players, and Structures maintain a list of Units as well), a lot of repeated functionality comes in. As well when I remove a unit from the game, it continues to be kept alive by this, so I have to remember to set the unit's structure to null every time (which I already forgot multiple times, resulting in weird bugs). Is there a better way to achieve this one-to-many relationship without manually updating both sides?

The best way to accomplish this is to have each World, Structure, and anywhere else a unit can move to inherit an interface IEnterable, which looks like

public interface IEnterable
{
    void Enter(Unit unit);
    void Leave(Unit unit);
}

Your unit would then have this as a property, and a MoveTo(IEnterable location) method

public class Unit
{
     // ...

     public IEnterable CurrentLocation { get; set; }

     // ...

     public void MoveTo(IEnterable location)
     {
         if (CurrentLocation != null)
         {
              CurrentLocation.Leave(this);
         }

         location.Enter(this);
     }
}

The Enter and Leave would handle all the logic for the Unit movement.

If you wanted to get more fancy, you could have a Movement class that could describe the movement a little more accurately.

public class Movement
{
     private readonly IEnterable _destination;

     public Movement(IEnterable destination)
     {
         if (destination== null) throw new ArgumentNullException("destination");

         _destination = destination;
     }

     public MoveResult MoveUnit(Unit unit)
     {
         if (unit == null) throw new ArgumentNullException("unit");

         var currentLocation = unit.CurrentLocation;
         if (currentLocation  == _destination) return MoveResult.SameLocation;

         if (!IsValidMove(currentLocation, _destination) return MoveResult.InvalidMovment;

         currentLocation.Leave(unit);

         _destination.Enter(unit);
     }
}

This will get the movement logic out of the Unit class because a unit should not care about how it has to move, just that it has.

I might be missing something, but my main point is to use an interface for each object that a Unit can enter, and the ease of moving a Unit becomes much better.


Finding pairs of numbers with a given difference Without changing the outputs or the inputs of the method, how would you make this code more readable ?

I am interested in any general improvement it can be made in terms of readability without WORSEN the time complexity.

Above all I'm interested of know if there is a way of doing all this steps in one line:

check if the key exist in the map.
if exist create a list with two elements (pair) and then append the
pair to list of pairs that will be the result.
if not exist do nothing.

Also is there a more elegant way to return array[][] from an ArrayList<ArrayList<Integer>> ?

static int[][] findPairsWithGivenDifference(int[] arr, int k) {

    int n = arr.length;
    Map<Integer, Integer> map = new HashMap<Integer, Integer>();

    ArrayList<ArrayList<Integer>> all = new ArrayList<ArrayList<Integer>>();

    for (int i = 0; i < n; i++){
      map.put(arr[i]-k,arr[i]);
    }

    for (int i = 0; i < n; i++){
      int x = arr[i];
      if (map.get(arr[i]) != null) {
        int y = map.get(x);
        ArrayList<Integer> al = new ArrayList<Integer>();
        al.add(y);
        al.add(x);
        all.add(al);
      }
    }

    int m = all.size();
    int[][] res = new int[m][2];

    for (int i = 0; i < m; i++) {
      for (int j = 0; j < 2; j++) {
        res[i][j] = all.get(i).get(j);
      }
    }

  return res;
}

You don't need to use a map here. A set will work just as well, since you can recreate the original value by adding the difference again.

I'm not sure if it's possible to do it in one line (which of course is not necessarily the most readable solution), but you can get close by converting the input array into a set and then using streams:

static int[][] findPairsWithGivenDifference(final int[] values, final int difference) {

    Collection<Integer> valuesSet = Arrays.stream(values).boxed().collect(Collectors.toSet());

    return valuesSet.stream()
            .map(v -> v - difference)
            .filter(valuesSet::contains)
            .map(v -> new int[] { v, v + difference })
            .toArray(size -> new int[size][2]);
}


Store, retrieve and delete settings in C# Registry My code writes, reads, and removes program settings in the registry. I'm using Visual Studio 2019 and my target Framework is .NET Framework 4.8.
I'm looking for more elegant functions.
Is there error handling I missed in these functions?
How can I optimize the code?
I have the following functions at the moment:
public static class Settings
{

    public static void SaveSetting(string sCompanyName, string sAppName, string sSection, string sValueName, string sValue)
    {
        RegistryKey softKey = Registry.CurrentUser.OpenSubKey("Software", true);
        RegistryKey compKey = softKey.CreateSubKey(sCompanyName);
        RegistryKey appKey = compKey.CreateSubKey(sAppName);
        RegistryKey secKey = appKey.CreateSubKey(sSection);
        secKey.SetValue(sValueName, sValue);
    }

    public static string GetSetting(string sCompanyName, string sAppName, string sSection, string sValueName, string sDefault)
    {

        RegistryKey softKey = Registry.CurrentUser.OpenSubKey("Software", true);
        RegistryKey compKey = softKey.CreateSubKey(sCompanyName);
        RegistryKey appKey = compKey.CreateSubKey(sAppName);
        RegistryKey secKey = appKey.CreateSubKey(sSection);

        return (string)secKey.GetValue(sValueName, sDefault);
    }

    public static void DeleteSetting(string sCompanyName, string sAppName, string sSection, string sValueName)
    {
        RegistryKey softKey = Registry.CurrentUser.OpenSubKey("Software", true);
        RegistryKey compKey = softKey.CreateSubKey(sCompanyName);
        RegistryKey appKey = compKey.CreateSubKey(sAppName);
        RegistryKey secKey = appKey.CreateSubKey(sSection);

        if (secKey.GetValue(sValueName) != null)
        {
            secKey.DeleteValue(sValueName);
        }

    }

}

Quick remarks:

When you're copy-pasting code, you should realize you're doing the wrong thing. The first four lines of each of your methods are exactly the same, and thus should become a method that returns a RegistryKey.

Do not needlessly abbreviate. softKey, compKey, appKey and secKey are all unclear.

Do not use Hungarian notation. Even if the type isn't clear from a parameter's name, your IDE will tell you what type it is.

Why doesn't DeleteSetting use GetSetting to retrieve the value?


Removing nodes from a linked list This is my second iteration of a function to remove all nodes containing value 'x'.  The previous iteration is here.

This function takes as input the head of a list and has to delete all the nodes that contain a given value taken.  Is this an improvement on the previous iteration?  Is there anything else that can be improved?

typedef struct node {
    int val;
    struct node *next;
} node_t;

node_t *rimuovi(node_t *head, int x){

    node_t *temp = head;
    node_t *curr = head;

    if (head == NULL){
        printf("Lista vuota, finito!\n");
        return head;
    }

    while(head->val == x){
        if (head->next == NULL){
            free(head);
            printf("La lista adesso e' vuota. Finito\n");
            return head;
        }
        else {
            curr = head->next;
            free(head);
            head = curr;
        }
    }

    temp = head;
    curr = temp->next;

    while (curr != NULL){
        if (curr->val == x){
            if(curr->next != NULL){
                temp->next = curr->next;
                free (curr);
                curr = temp->next;
            }
            else {
                temp->next = NULL;
            }
        }
        else {
            temp = curr;
            curr = curr->next;
        }
    }

    return head;
}

Bug: Returning a free'd pointer!

After freeing a pointer, do not use it.

        free(head);
        printf("La lista adesso e' vuota. Finito\n");
        // return head;
        return NULL;

  Is there anything else that can be improved?

Rather than 2 or 3 cases to handle an empty list, list of only matches or mixed list, consider a code simplification.

Create a temporary pre-head node and assign its next to the head.  Only 1 case  needed.  This approach is reasonable when node_t is not large.

Some untested code.

node_t *rimuovi(node_t *head, int x) {
  node_t pre_head;
  node_t *p = &pre_head;
  pre_head.next = head;  // Other members are not used.

  while (p->next) {
    if (p->next->val == x) {
      node_t *next = p->next->next;
      free(p->next);
      p->next = next;
    } else {
      p = p->next;
    }
  }
  return pre_head.next;
}


Integration of data using Simpson's rule I wrote a Python program that performs an integration of data using Simpson's rule. The program takes the areas to be integrated from a foo.ref file, with the following syntax:

# peak          m/z             charge  window
LAG_H4N4F1S1    4244.829467     1+      0.06
LAG_H4N4F1S1    4245.832508     1+      0.06
LAG_H4N4F1S1    4246.835549     1+      0.06
LAG_H4N4F1S1    4247.838590     1+      0.06

The data that the program parses is taken from all the .xy files in the current directory, with the following syntax:

3497.7552 0
3497.7619 410646
3497.7685 397637
3497.7752 363672
3497.7819 321723
3497.7886 286960

The program's final output is a tab seperated file that can be opened directly in excel, the performance is ... a lot worse than I was expecting however and I was wondering if anyone could offer some tricks to increase that (and also to have a general look at the code, if it is correct Python).

#! /usr/bin/env python
import glob
import os
import fileinput
print "<SimpInt.py> a script to quickly calculate the area for a set"
print "of analytes. Written by Bas Jansen, Leiden University Medical"
print "Center, March 20th 2013."
print ""

# Simpson's rule
def integrate(y_vals,h):
  i=1
  total=y_vals[0]+y_vals[-1]
  for y in y_vals[1:-1]:
    if i%2 == 0:
      total+=2*y
    else:
      total+=4*y
    i+=1
  return total*(h/3.0)

# Reads the reference file
ref=[]
for refs in glob.glob("*.ref"):
  fr=open(refs)
  for line in fr.readlines():
    parts=line.rstrip('\n').split('\t')
    ref.append(parts)
  fr.close()

# Initialize the output file
f=open('output.xls','w')
for i in range(1,len(ref)):
  f.write(ref[i][0]+"\n")
f.close()

# Find XY files in current directory
xy_files=[]
for files in glob.glob("*.xy"):
  xy_files.append(files)
  x=[]
  area=[]
  print "Parsing: "+files
  fh=open(files)
  for line in fh.readlines():
    y=[float(i) for i in line.split()]
    x.append(y)
  fh.close()

  # Pick the XY window
  print "Progress indicator:"
  print "0%"
  for i in range(1,len(ref)):
    obs=[]
    vals=0
    low=float(ref[i][1])-float(ref[i][3])
    high=float(ref[i][1])+float(ref[i][3])
    # Progress indicator
    if i %(len(ref)/10) == 0:
      print str(i/(len(ref)/10))+"0%"
    for j in range(0,len(x)):
      if x[j][0] > low and x[j][0] < high:
        coords=[float(l) for l in x[j]]
        obs.append(coords)
        vals+=1
    if obs:
      y_vals=[]
      h=(obs[len(obs)-1][0]-obs[0][0])/len(obs)
      for k in range(0,len(obs)):
        y_vals.append(obs[k][1])
      area.append(integrate(y_vals,h))

  # Neat trick to append the area from other input files
  l = 0
  for line in fileinput.input('output.xls',inplace=1):
    print "%s\t%s\n" % (line.rstrip(),area[l]),
    l+=1

# Display the legend (& filenames) at bottom of output file
f=open('output.xls','a')
legend = "Peak"
for m in range(0,len(xy_files)):
  legend+="\t"+str(xy_files[m])
legend+="\n"
f.write(legend)
f.close()

# Being polite
raw_input("Finished, press any key to exit")

I have performed some various tests today with disabling chunks of the code and seeing what causes the biggest increase in processing time and that appears the searching of the lists.

#! /usr/bin/env python
import glob
import os
import fileinput
print "<SimpInt.py> a script to quickly calculate the area for a set"
print "of analytes. Written by Bas Jansen, Leiden University Medical"
print "Center, March 20th 2013."
print ""

# Simpson's rule
def integrate(y_vals,h):

The recommendation for python is four spaces for indentation, you appear to be using 2

  i=1
  total=y_vals[0]+y_vals[-1]

It's also recommended to have spaces around binary operators

  for y in y_vals[1:-1]:

Use for i, y in enumerate(y_vals, 1): rather then counting your own i value

    if i%2 == 0:
      total+=2*y
    else:
      total+=4*y
    i+=1
  return total*(h/3.0)

# Reads the reference file
ref=[]

I'd recommended not using an abbreviation her

for refs in glob.glob("*.ref"):
  fr=open(refs)

It's recommended to use a with block:

  with open(refs) as fr:
      do stuff with file
  file will be automatically closed here

That ensures your file will be closed even if an exception occours

  for line in fr.readlines():
    parts=line.rstrip('\n').split('\t')
    ref.append(parts)

You shouldn't really store the header ref here.

  fr.close()

# Initialize the output file
f=open('output.xls','w')
for i in range(1,len(ref)):
  f.write(ref[i][0]+"\n")
f.close()

# Find XY files in current directory
xy_files=[]
for files in glob.glob("*.xy"):
  xy_files.append(files)
  x=[]
  area=[]
  print "Parsing: "+files
  fh=open(files)
  for line in fh.readlines():

Actually this is the same as for line in fh:

    y=[float(i) for i in line.split()]
    x.append(y)
  fh.close()

  # Pick the XY window
  print "Progress indicator:"
  print "0%"
  for i in range(1,len(ref)):

Whenever possible, don't iterate over indexes. Instead iterate over the lists. So something like for line in ref[1:]: If you really need the indexes us enumerate

    obs=[]
    vals=0
    low=float(ref[i][1])-float(ref[i][3])
    high=float(ref[i][1])+float(ref[i][3])
    # Progress indicator
    if i %(len(ref)/10) == 0:
      print str(i/(len(ref)/10))+"0%"

Why divide by 10 just to add the 0 back?

    for j in range(0,len(x)):

Use for j in x: and then avoid the indexing

      if x[j][0] > low and x[j][0] < high:
        coords=[float(l) for l in x[j]]
        obs.append(coords)
        vals+=1

What's this for?

    if obs:
      y_vals=[]
      h=(obs[len(obs)-1][0]-obs[0][0])/len(obs)
      for k in range(0,len(obs)):
        y_vals.append(obs[k][1])
      area.append(integrate(y_vals,h))

  # Neat trick to append the area from other input files
  l = 0
  for line in fileinput.input('output.xls',inplace=1):

If you need to iterate over multiple sources at once, use a zip.

    print "%s\t%s\n" % (line.rstrip(),area[l]),
    l+=1

Your neat trick is what's killing you. This is designed for processing the output of some other program. Its not good for continually adding to the end of lines you have already written. What you should do is store everything in a big list and then output it at the end.

# Display the legend (& filenames) at bottom of output file
f=open('output.xls','a')
legend = "Peak"
for m in range(0,len(xy_files)):
  legend+="\t"+str(xy_files[m])
legend+="\n"
f.write(legend)
f.close()

# Being polite
raw_input("Finished, press any key to exit")


PowerShell script creates a computer list from an AD OU, and add exclusions to a specific file via Invoke-Command My code is working, however, in the foreach section, when it runs the invoke-command if I don't use "-ErrorAction SilentlyContinue" the red error text that states a computer cannot be connected to shows up. The code still works fine, I just wanted to know if there is a way to add "nicer" text, so that when the invoke-command fails to connect, it'll show "This computer cannot be reached"

Any help is appreciated.

$output_dir = "$env:USERPROFILE\Desktop\ad_computer_list"
$output_file = "$output_dir\computerlist.txt"
$computers_input_file = "computerlist.txt"
$script_name = "Create Spectrum Protect exclusions script"

Import-Module ActiveDirectory
New-Item -Force -ItemType Directory $output_dir | Out-Null
New-Item -Force -ItemType File $output_file

Set-Location $env:USERPROFILE\Desktop\ad_computer_list

#
# MAIN code body
#

Write-Host "`n`n################################################################" -ForegroundColor Gray
Write-Host "Running $($script_name)..." -ForegroundColor Cyan
Write-Host "################################################################" -ForegroundColor Gray

function Get-ComputerList {
param(
$OU_Base = $(Read-Host "Enter distinguished name of OU")
)

Get-ADComputer -Filter "*" -SearchBase "$OU_Base" |
Select-Object -ExpandProperty Name |
Sort |
Out-File -Encoding ASCII -FilePath $env:USERPROFILE\Desktop\ad_computer_list\computerlist.txt

try {
# Get entries in text file, strip away whitespace, blank lines and lines starting with '#' characters
$computers = Get-Content $computers_input_file -ErrorAction Stop | Where-Object { ($_.Trim() -ne '') -and ($_ -NotMatch "^#") }
}
catch {
    Write-Error "Unable to open $computers_input_file. Please create or move the file to the same directory as this script."
break
}

Write-Verbose "$($computers.Count) computer entries found in input file."

foreach ($computer in $computers) {

Write-Host "Opening connection to $computer ..."

try {Invoke-Command -ErrorAction SilentlyContinue -ComputerName $computer -ScriptBlock {if(!(Get-Content -Path "C:\program files\tivoli\tsm\baclient\dsm.opt" -ErrorAction SilentlyContinue | Select-String "Excludedir", "Program Files"))

{Write-Host "Spectrum Protect is not installed or exclusions already exist." -ForegroundColor "Yellow"}

else

{add-content -Path "C:\program files\tivoli\tsm\baclient\dsm.opt" -Value `r`n'EXCLUDE.DIR "C:\Program Files"'}}

} catch {
    Write-Host "Cannot connect to computer!"
}

}

}
Get-ComputerList

Your try block will be unable to pass the error to catch block if you don't use -ErrorAction Stop. Also there is an -ErrorAction Ignore on powershell v3.0+ that completely ignores the error.


Python scraping tables I started practicing in web-scraping few days ago. I made this code to extract data from a wikipedia page. There are several tables that classify mountains based on their height. However there is a problem with the size of the matrices. Some of them contain 5 columns while others 4. So I made this algorithm to extract all the names and the attributes of the mountains into separate lists. My approach was to create a length list that contains the number of
<td> within the <tr> tags. The algorithm finds which table contains four columns and fills the column in excess (in the case of 5 columns) with NONE. However, I believe that there is a more efficient and more pythonic way to do it especially in the part where I use the find.next() function repedetly.  Any suggestions are welcomed.

import requests
from bs4 import BeautifulSoup
import pandas as pd

URL="https://en.wikipedia.org/wiki/List_of_mountains_by_elevation"
content=requests.get(URL).content

soup=BeautifulSoup(content,'html.parser')

all_tables=soup.find_all("table",{"class":["sortable", "plainrowheaders"]})

mountain_names=[]
metres_KM=[]
metres_FT=[]
range_Mnt=[]
location=[]

lengths=[]

for table in range(len(all_tables)):

  x=all_tables[table].find("tr").find_next("tr")
  y=x.find_all("td")
  lengths.append(len(y))

  for row in all_tables[table].find_all("tr"):

        try:
            mountain_names.append(row.find("td").text)
            metres_KM.append(row.find("td").find_next("td").text)
            metres_FT.append(row.find("td").find_next("td").find_next("td").text)
            if lengths[table]==5:
                range_Mnt.append(row.find("td").find_next("td").find_next("td").find_next("td").text)
            else:
                range_Mnt.append(None)
            location.append(row.find("td").find_next("td").find_next("td").find_next("td").find_next("td").text)
        except:
            pass

You're just looping on the rows, but not on the cells:

 for row in all_tables[table].find_all("tr"):

Rather than using multiple find_next("td") one after the other, add another loop using row.find_all('td') and append each row and cell to a 2D array.

Manipulating a 2D array is much easier and will make your code look much cleaner than row.find("td").find_next("td").find_next("td").

Good luck!

Those questions contain some answers that might interest you:

How to convert an HTML table to an array in python
python BeautifulSoup parsing table

To be more specific, this code snippet from @shaktimaan:

data = []
table = soup.find('table', attrs={'class':'lineItemsTable'})
table_body = table.find('tbody')

rows = table_body.find_all('tr')
for row in rows:
    cols = row.find_all('td')
    cols = [ele.text.strip() for ele in cols]
    data.append([ele for ele in cols if ele])


Prime power mathematical problem in Python I have a mathematical problem about prime power. A number \$x\$ can be considered a prime power if \$x = p^k\$ where \$p\$ is a prime and \$k\$ is a non-negative integer. For example, \$81\$ is a prime power because \$81 = 3^4\$. Now, form a sequence of numbers in the following way. Start by taking a random number.

If the number is a prime power, end the sequence.
If the number is not a prime power, minus that number with the biggest prime power that's not more than the number itself. Do it over and over again until a prime power is met, then stop.

For example, 34 is not a prime power. Hence we minus it with the highest prime power that's not more than it which is \$32\$, hence \$34-32=2\$. \$2\$ is a prime power, so we stop. In this case, the length of the operation is 2 (because 34 -> 2).

Another example is \$95\$. \$95\$ isn't a prime power, so \$95-89=6\$. \$6\$ is also not a prime power, so we minus it again with the highest prime power, \$6-5=1\$. \$1\$ is a prime power (because \$2^0 = 1\$), so we stop. In this case, the length of the operation is 3 (because 95 -> 6 -> 1).

Known that:

1 is the smallest initial number that can be formed with a length of
1 operation.
6 is the smallest initial number that can be formed with
a length of 2 operations.
95 is the smallest initial number that can
be formed with a length of 3 operations.
360748 is the smallest
initial number that can be formed with a length of 4 operations.

I made an application using Python to find the smallest initial number that can be formed with a length of 5, 6 and 7 operations. It is using looping to solve the problem. However, to find the smallest initial number that can be formed with a length of 4 or more operations, the program takes a very very long time to run.

Is there any way I could improve my code to make the program to run much faster? I just want to focus on getting the number with 5, 6 and 7 operations.

from sympy.ntheory import factorint
def pp(q): #to check whether it is a prime power
    fact=factorint(q)
    p=int(list(fact.keys())[0])
    n=int(list(fact.values())[0])
    if q!=p**n:
        return False
    else:
        return True

a=[1,6][-1]

b=a*2
d=1
while d!=0:
    c=b-a
    if pp(b)==False and pp(c)==True:
        for i in range(b-1,c-1,-1):
            if i==c:
                d=0
            elif pp(i)==True:
                b=i+a
                break
    elif pp(b)==False:
        for i in range(b-1,c-1,-1):
            if i==c:
                b=b+a+1
            elif pp(i)==True:
                b=i+a
                break
    elif pp(b)==True:
        b=b+a
b

Welcome, there are some aspects to comment about.

First is that, you should use specific names for your functions pp is ok and sounds quite interesting, but it would be better if you use is_prime_power (as mentioned by Konrad Rudolph in the comments).
Now, for the first function, I would change only two things:

def is_prime_power(q):
    fact = factorint(q)
    p = int(list(fact.keys())[0])
    n = int(list(fact.values())[0])
    return q == p**n # this line asks 'is prime power?'

# could also be:
def is_prime_power(q):
    return q == int(list(fact.keys())[0])**int(list(fact.values())[0])
# but is quite strange because p,n are not specified.

# this inverses the boolean answer, as you see
    if q != p**n:
        return False
    else:
        return True

For the procedure, you should avoid using comparisons with True and False. If the value is a boolean, Python deduces if it is true or false, in the case of compare with False use not:

a = [1,6][-1]

b = a*2
d = 1
while d != 0:
    c = b-a
    if not is_prime_power(b) and is_prime_power(c):
        for i in range(b-1, c-1, -1):
            if i == c:
                d = 0
            elif is_prime_power(i):
                b = i+a
                break
    elif not is_prime_power(b):
        for i in range(b-1, c-1, -1):
            if i == c:
                b = b+a+1
            elif is_prime_power(i):
                b = i + a
                break
    elif is_prime_power(b):
        b = b+a
b

Well, thanks for writing easy to read code, it is so good to see something like it.


Funny string python solution Problem Statement

  Suppose you have a string \$S\$ which has length \$N\$ and is indexed from \$0\$ to \$N−1\$. String \$R\$ is the reverse of the string \$S\$. The string \$S\$ is funny if the condition \$|S_i−S_{i−1}|=|R_i−R_{i−1}|\$ is true for every \$i\$ from \$1\$ to \$N−1\$.

  (Note: Given a string \$str\$, \$str_i\$ denotes the ascii value of the \$i\$th character (0-indexed) of \$str\$. \$|x|\$ denotes the absolute value of an integer \$x\$)

Example

is_funny("acxz") returns True

is_funny("bcxz") returns False

Solution

def is_palindrome(ary):
    return ary == ary[::-1]

def create_difference_list(s):
    res = []
    for i in range(1, len(s)):
        res.append(abs(ord(s[i]) - ord(s[i-1])))
    return res

def is_happystr(s):
    return is_palindrome(create_difference_list(s))

Notes

Have used slow version of palindrome check assuming string is not going to be too long. Hope it doesn't affect performance that much.
Approached problem via functional style, could have done all the computation in the same loop but is it a good practice?

Answering your specific questions:

  Have used slow version of palindrome check assuming string is not going to be too long. Hope it doesn't affect performance that much.

You made a very good choice. This solution is simpler and more readable, you may switch to a new faster implementation later iff performance becomes a problem and you are sure is_palindrome is the problem.

  Approached problem via functional style, could have done all the computation in the same loop but is it a good practice?

To be precise on terminology, you used an imperative style with subroutines. I think that the use of subroutines ("functions") like you do greatly improves readability, so well done.

You may re-factor into functions further by writing a function to return pairs from a list and calling it from create_difference_list


Find list of valid continuing positions in a game My kids got a dice game for Christmas. I'm trying to mathematically analyze it. The first step is to come up with a list of all possible positions.

Each player in the game has a certain number of chips. A valid position is when more than one player has chips. If only one player has chips, they win and the game is over -- that's easy to identify.

I'm trying to come up with sensible code that uniquely lists all non-terminating positions. I use flatten() from more_itertools which is in PIP. Here's what I've got:

from more_itertools import flatten
from itertools import permutations

def all_nonzero_positions(num_players, num_chips, min_chips, max_chips):
    """ Returns all monotonically descending lists
        where num_chips are divided among num_players
        with each player having at least one chip, subject to the
        min_chips and max_chips constraints. Recursive. """
    if num_players==1:
        return [[num_chips]] if min_chips <= num_chips <= max_chips else []
    return [[p1] + y for p1 in range(min_chips,max_chips+1)
            for y in all_nonzero_positions(num_players-1, num_chips-p1,min_chips,p1)]

def all_positions_sorted(num_players, num_chips):
    """ Returns all monotonically descending lists
        where num_chips are divided among num_players
        where at least two players have one or more chips.
        Works by repeatedly calling all_nonzero_positions()
        for smaller num_players and appending necessary zero elements. """
    return [position + [0]*zeroes for zeroes in range(0,num_players-1)
            for position in all_nonzero_positions(num_players-zeroes,num_chips,1,1000)]

def all_valid_positions(num_players, num_chips):
    return set(flatten(list(permutations(x))
                       for x in all_positions_sorted(num_players,num_chips)))

def main():
    print(*all_valid_positions(3,3), sep='\n')

if __name__ == "__main__":
    main()

What I don't like:

The lines are too long, but I don't see any place I can create temp variables or functions.
Can I get rid of more_itertools.flatten() without lengthening the code?
Can I get rid of list(), which I'm using to instantiate the generator?
Can I get rid of set(), which I'm using to trim the duplicates?
Is there a better way to permute than using itertools.permutations()?
Can all_nonzero_positions() be written without recursion?
I'm creating a ton of temporary lists here, can that be reduced?
What other comments should I add? The code is really short, I don't know where to put them.

Disclaimer: this is not a code review, but a suggestion for an alternative implementation.

First, there is not that many terminating positions (just num_players), so it seems easier to enumerate all positions, and filter the terminating ones out.

Second, given a position it is easy to generate lexicographically next one. In pseudocode,

    find the leftmost player holding non-zero amount of chips
    if it is the last player,
        we are done
    otherwise
        pass one chip to the next player, and
        give her remaining chips (if any) to player 0


Fractional Knapsack in Python I wrote this in order to solve the fractional knapsack problem. Any feedback is appreciated.

def get_optimal_value(capacity, weights, values):
    value = 0.
    if sum(weights) <= capacity:
        return sum(values)
    if len(weights) == 1:
        value = values[0]/float(weights[0])*capacity
    densities = [(v/float(w), w) for (v,w) in zip(values, weights)]
    densities = sorted(densities, key=lambda density:density[0], reverse=True)
    fraction = 0.
    #print(densities)
    for i in range(len(weights)):
        fraction = sum([f for (d,f) in densities[0:i]])
        #print(i, fraction, capacity)
        if fraction >capacity and i >0:
            value = sum([d*f for (d,f) in densities[0:i-1]]) + densities[i][0]*(1.0000000-densities[i][1]/float(capacity))*capacity
        if fraction == capacity and i >0:
            value = sum([d*f for (d,f) in densities[0:i]])

    return value

The program does not solve the problem:

>>> get_optimal_value(1, (1, 1, 1, 1), (1, 1, 1, 1))
2.0

Here we have a knapsack with capacity 1, and four kinds of item, each with weight 1 and value 1. So all ways of filling the knapsack have value 1.


Basic form validation script Is this a good form validation script?

$(document).ready(function() {

    $("form").submit(function() {

        var shouldSubmit = true;

        $(this).children(":input:not(:button, [type=\"submit\"], [type=\"reset\"])").each(function() {

            if ($(this).val() == "")
            {
                shouldSubmit = false;
                return shouldSubmit;
            }

        });

        if ($("input:checkbox").length > 0)
        {
            if ($("input:checkbox:checked").length == 0)
                shouldSubmit = false;
        }

        if ($("input:radio").length > 0)
        {
            if ($("input:radio:checked").length == 0)
                shouldSubmit = false;
        }

        if (shouldSubmit == false)
            alert("All form fields must be filled out.");

        return shouldSubmit;

    });

});

The if statements could be cleaner:

    if ($("input:checkbox").length > 0 && $("input:checkbox:checked").length == 0)
    {
        shouldSubmit = false;
    }

    if ($("input:radio").length > 0 && $("input:radio:checked").length == 0)
    {
        shouldSubmit = false;
    }

    if (!shouldSubmit)
    {
        alert("All form fields must be filled out.");
    }


Project Euler 40: Champernownes's Constant I just finished Project Euler 40 with a brute force method. As usual, any optimizations would be helpful. (I feel like I'm getting better at this scripting thing)

from timeit import default_timer as timer

def product(n):
    from functools import reduce # Valid in Python 2.6+, required in Python 3
    from operator import mul
    return reduce(mul, n, 1)

def find_champernowne_index(d):
    constant = ""
    for k in xrange(1, d + 1):
        constant += str(k)
        if len(constant) >= d:
            return int(constant[d-1])

start = timer()
ans = product(find_champernowne_index(10**x) for x in range(0, 7))
elapsed_time = (timer() - start) * 1000 # s --> ms

print "Found %d in %r ms." % (ans, elapsed_time)

This probably plenty fast enough, but a brute force method strikes me as a very inefficient approach. I can think of a way that should be faster and more efficient, and avoids you constructing a million-character string.

Here are some suggestions/hints:

You calculate the same constant string each time, despite the fact that it’s always going to be the same. Do you really need to construct it every time? (Or at all?)
For every value of k you add to constant, you know how long it is. Do you actually need to add it to the string if you’re not going to look at it? Or could you just decrement that from the number of digits left before you get to the digit you want?
How many one-digit numbers are in the string? And two-digit numbers? And so on.

My actual solution in spoiler text:

   We can show that there are always \$ 9 \times 10^{n-1} \$ numbers which have \$n\$ digits, so by computing the sum
 \$ 9 + 90 + 900 + \dotsb \$
 until we go over the target value, we can work out how many digits the number we land in has (say \$ N \$). You can then jump back to the start of the \$ N \$-digit numbers and count up from there. You don't even need to be constructing the string; you can just use lengths to work out where we end up.

Oh, and one style comment:

Quoting from PEP 8 on imports:

  Imports are always put at the top of the file, just after any module comments and docstrings, and before module globals and constants.

Don’t put them inside your product definition. The way that Python caches module imports means that it won’t be a performance hit, but it does make it harder to find your module dependencies in a large file. I realise that this isn’t much of an issue in short snippets like this, but it’s a good habit to put your import statements in the standard location.


Best way to check for specific types Normally I do this:

if (Animal is Dog)
{
    Doc d = (Dog)Animal;
    // d. etc..
}
else if (Animal is Cat)
{
    Cat c = (Cat)Animal;
    // c. ....
}

Is this a good way or are there better ways to implement this code above (performance, ...) ?

Should it be like this?:

Dog d = Animal as Dog;

if (d != null;)
{
    // d. etc..
}
else if (Animal is Cat)
{
    Cat c = (Cat)Animal;
    // c. ....
}

Or maybe like this?:

Dog d = Animal as Dog;
Cat c;

if (d != null;)
{
    // d. etc..
}
else if ((c = Animal as Cat) != null)
{
    // c. ....
}

Or maybe something else?

This question is better suited for Programmers Stack Exchange than Code Review...

To answer your question - if you do evaluations like that it means that most likely you have a design issue at the first place. In most cases you should not branch your logic based on specific type you're working with, since it breaks Open/Closed principle and Liskov substitution principle.
Either your class should be a generic class accepting type of the animal to work with, or there must be a virtual method on Animal that should be overridden in Cat and Dog, or something else, depending on specifics of your case.

In most cases the is/as checks are used to check whether a certain object supports an interface or well-known behaviour like:

object message = ....;
if (message is IEnumerable) //now we know that we can iterate on it

or

int Count(this IEnumerable source) //"simplified" version of LINQ's extension method
{
    ICollection collection = source as ICollection;
    if (collection != null)
        return collection.Count;

    int count = 0;
    foreach (var element in source)
        count++;

    return count;
}


PHP implementation of "depth-first search" on the graph data structure As I am a totally self-taught 'person writing code', I decided to learn algorithms with one online course that taught them using Java for demonstrative purposes. The first topic I covered is the graph data structure and API.

I undertook to rewrite Java examples into PHP for training purposes. I would be really grateful if you could review this code and point to its biggest shortcoming.

It was not always possible to reproduce certain behaviours and data structure that are found in Java so I had to resort to some workarounds.

Here is the code with explanations:

/**
* Bag stores all vertices adjacent to a given vertex.
* It's here to imitate Java's 'Bag' data structure
*/
class Bag implements IteratorAggregate
{
    public $vs = array();

    public function getIterator()
    {
        return new ArrayIterator($this->vs);
    }
}

-

class Graph
{

    private $V;

    /* Adjacent vertices */
    private $adj = array();

    /**
    * @param int $V The total number of vertices in the graph
    */
    public function __construct($V)
    {
        $this->V = $V;
        for ($v = 0; $v < $V; $v++) {
            $this->adj[$v] = new Bag;
        }
    }

    /**
    * Throw all adjacent vertices of a given vertex into this vertex' Bag.
    * This will create 'edges' - connection between vertices.
    *
    * @param int $v Vertex to connect
    * @param int $w Vertex to connect
    */
    public function addEdge($v, $w)
    {
        $this->adj[$v]->vs[] = $w;
        $this->adj[$w]->vs[] = $v;
    }

    /**
    * @return Bag All ajacent vertices of a given vertex
    */
    public function returnBag($v)
    {
        return $this->adj[$v];
    }

    public function returnAllVertices()
    {
        return $this->adj;
    }

}

-

class DepthFirstPath
{
    private $marked = array();

    /* This array stores parent-link representation  of a tree rooted at $s (source) */
    private $edgeTo = array();

    private $s;

    /**
    * @param Graph $G An instance of the graph
    * @param int $s Source vertex to start the recursive traversal from
    */
    public function __construct(Graph $G, $s)
    {
        $allVertices = $G->returnAllVertices();

        /* Set all verices in the graph as unvisited/unmarked */
        foreach($allVertices as $key => $value) {
            $this->marked[$key] = false;
            foreach($value->vs as $v) {
                $this->marked[$v] = false;
            }
        }

        $this->s = $s;
        $this->dfs($G, $s);

    }

    /**
    * 'Depth-first search'
    *
    * A recursive function that will visit all adjacent vertices of each vertex
    * and mark each of them as 'visited'
    */
    private function dfs(Graph $G, $v)
    {
        $this->marked[$v] = true;
        $adjToV = $G->returnBag($v);
        foreach($adjToV as $w) {
            if ( ! $this->marked[$w] ) {
                $this->dfs($G, $w);
                $this->edgeTo[$w] = $v;
            }
        }
    }

    public function hasPathTo($v)
    {
        return $this->marked[$v];
    }

    /**
    * Find a path from the source to a given vertex
    */
    public function pathTo($v)
    {
        if ( ! $this->hasPathTo($v)) {
            return null;
        }

        $path = new \SplStack;
        for ($x = $v; $x != $this->s; $x = $this->edgeTo[$x]) {
            $path->push($x);
        }
        $path->push($this->s);
        return $path;
    }
}

This is how the client might use it (feel free to copy, paste and try it):

$graph = new Graph(10);
$graph->addEdge(1,2);
$graph->addEdge(2,3);
$graph->addEdge(3,4);
$graph->addEdge(1,6);
$graph->addEdge(8,9);

$path = new DepthFirstPath($graph, 1);
$path->pathTo(4);

What have I done wrong, what could I improve, what rules have I violated ?

It's totally unnecessary to have a Bag class if all you do is reach through to its only field.

Generally, all your classes should either have multiple fields or engage in some kind of non-trivial validation or data-hiding to justify the necessity for a class.


Getting a value with timeout using Kotlin coroutines So, here's a task. A background thread may or may not call all of it's listeners some time after sendRequest() was called (in other words sendRequest() schedules a possible event). The required function must:

Subscribe to those events
Call sendRequest() periodically until the function returns anything
Return the first suitable result (filtering is done in the listener and is omitted in this snippet) or null after a timeout
Unsubscribe from those events regardless of the result

suspend fun findServer(): Server? {
    var listener: ((Server) -> Unit)? = null
    return try {
        withTimeoutOrNull(10000) {
            suspendCancellableCoroutine<Server> { continuation ->
                listener = { server: Server ->
                    continuation.resume(server)
                }.also { discoveryService.addListener(it) }

                launch {
                    while (continuation.isActive) {
                        discoveryService.sendRequest()
                        delay(1000)
                    }
                }
            }
        }
    } finally {
        listener?.let { discoveryService.removeListener(it) }
    }
}

The current implementation seems to work fine, but the nullable listener variable feels kind of clunky and requires using also to keep the listener reference, which in turn breaks type inference and makes me specify listener's parameter type (no big deal, but...). I could use a lateinit var here, but it probably can lead to exceptions when withTimeoutOrNull() stops before the listener was initialized.

Any recommendations on improving this snippet? Or am I, in fact, doing everything totally wrong here?

Think it's pretty good and you can't think of much else. Some tiny bits I can think of:

Make discoveryService parameter.
Do not use launch without specifying CoroutineScope.


Displaying text extracted from XML using PHP simplexml I have this xml file and i am parsing it with php simplexml.

My XML file looks like this:

<?xml version="1.0" encoding="UTF-8"?>

<noticias>
    <noticia url="noticia-1">
        <titulo>título da notícia 1</titulo>
        <desc>plain text.</desc>
        <texto>plain text.</texto>

        <in>Publico</in>
    </noticia>
    ...
</noticias>

In my php page i parse it like this:

$file = 'xml/noticias.xml';
if(file_exists($file)) {
    $xml = simplexml_load_file($file);
    foreach($items as $item) {
        $titulo = htmlentities($item->titulo, ENT_QUOTES, 'utf-8');
        $desc = htmlentities($item->desc, ENT_QUOTES, 'utf-8');
        $url = htmlentities($item['url'], ENT_QUOTES, 'utf-8');

        if(strlen($titulo) < 3) {$titulo = 'T&iacute;tulo em falta';} else {$titulo = $titulo;}
        if(strlen($desc) < 3) {$desc = 'Descri&ccedil;&atilde;o em falta';} else {$desc = $desc;}
        if(strlen($url) < 3) {$h3 = '
'.$titulo.'
';} else {$h3 = '
'.$titulo.'
';}

        ?>

                        <?php echo $h3; ?>

<?php echo $desc; ?>

        <?php
    }
}
else {
    // do something... throw error message
}

Is this ok? i mean, i escape values when i get from xml. Is it ok to do it like this or should i escape values on echo? is there any danger to leave as i have?

other thing... i have the xml files protected with htaccess. right now, they can only be edited directly. no scripts to edit them.

My preference is to leave the html escaping until the last minute.

Here are some changes you could make to simplify your code.

Code should work but there is no link between $xml and $items, so unable to test.

<?php

$file = 'xml/noticias.xml';

// guard clause exit early, instead of last
if (!file_exists($file)) {
    // do something... throw error message
    throw new \Exception('File does not exist');
}

$xml = simplexml_load_file($file);

foreach ($items as $item) {
    $titulo = $item->titulo;
    $desc = $item->desc;
    $url = $item['url'];

    // use mb_ function as you are dealing with unicode text, strlen will report incorrect length
    if (mb_strlen($titulo) < 3) {
        $titulo = 'Título em falta';
    }
    // no need to reassign value to itself
    //else {
    //    $titulo = $titulo;
    //}

    if (mb_strlen($desc) < 3) {
        $desc = 'Descrição em falta';
    }
//    else {
//        $desc = $desc;
//    }

    // not expecting url to be unicode
    if (strlen($url) >= 3) {
        $url = "noticia/{$url}";
    } else {
        $url = null;
    }

    $h3 = $titulo;

//        $h3 = '
' . $titulo . '
';
//    } else {
//        $h3 = '
' . $titulo . '
';
//    }
    ?>

                <?php if ($url): ?>
                    <a href="<?php echo $url; ?>">
                <?php endif; ?>

<?php echo htmlentities($h3); ?>

                <?php if ($url): ?>

                <?php endif; ?>

<?php echo htmlentities($desc); ?>

    <?php
}


Make C# Unity Script database methods more re-usable and concise I have a C# file, that makes some modifications on screen (changes the text on the user interface) but I have some doubts:

How I can re-use the below methods to be more concise and reusable?
How I can separate the database methods in another file class?

private void GetDoencaNome(){
    using(IDbConnection dbConnection = new SqliteConnection(connectionString)){
        dbConnection.Open();

        using(IDbCommand dbCommand = dbConnection.CreateCommand()){
            string sqlQuery = "SELECT nome FROM doenca WHERE id=@id";
            dbCommand.Parameters.Add(new SqliteParameter("@id", doencaId));
            dbCommand.CommandText = sqlQuery;

            using(IDataReader reader = dbCommand.ExecuteReader()){
                while(reader.Read()){
                    doenca.nome = reader.GetString(0);
                }
            }
        }
    }
}

private void GetDoencaDescricao(){
    using(IDbConnection dbConnection = new SqliteConnection(connectionString)){
        dbConnection.Open();

        using(IDbCommand dbCommand = dbConnection.CreateCommand()){
            string sqlQuery = "SELECT descricao FROM descricao_doenca WHERE id=@id";
            dbCommand.Parameters.Add(new SqliteParameter("@id", doencaId));
            dbCommand.CommandText = sqlQuery;

            using(IDataReader reader = dbCommand.ExecuteReader()){
                while(reader.Read()){
                    doenca.descricao = reader.GetString(0);
                }
            }
        }
    }
}

private void GetDoencaSintomasIds(){
    using(IDbConnection dbConnection = new SqliteConnection(connectionString)){
        dbConnection.Open();

        using(IDbCommand dbCommand = dbConnection.CreateCommand()){
            string sqlQuery = "SELECT sintoma_id FROM doenca_sintoma WHERE doenca_id=@id";
            dbCommand.Parameters.Add(new SqliteParameter("@id", doencaId));
            dbCommand.CommandText = sqlQuery;

            using(IDataReader reader = dbCommand.ExecuteReader()){
                while(reader.Read()){
                    listaIds.Add(reader.GetInt32(0));
                }
            }
        }
    }
}

private void GetDoencaSintomas(){
    using(IDbConnection dbConnection = new SqliteConnection(connectionString)){
        dbConnection.Open();

        using(IDbCommand dbCommand = dbConnection.CreateCommand()){
            string sqlQuery = "SELECT descricao FROM sintoma WHERE";
            sqlQuery += " id=" + listaIds[0].ToString();
            for(int i = 1; i < listaIds.Count; i++){
                sqlQuery += " OR id=" + listaIds[i].ToString();
            }
            dbCommand.CommandText = sqlQuery;

            using(IDataReader reader = dbCommand.ExecuteReader()){
                while(reader.Read()){
                    doenca.sintomas.Add(reader.GetString(0));
                }
            }
        }
    }
}

General
Good
Except for the non english part you are naming your methods and variables in a good way using the correct casing style.
You are using using statements to properly dispose the IDisposable's which is the way to go.
You are coding against interfaces rather then concrete implementations which is the way to go as well.
Not so good
You are using methods GetXXX but the methods returning type is void. If one read GetXXX or FetchXXX one expect that the method is in fact returning something.
Your "problem"
The first two methods are doing basically the same. They open a connection to a database, executing a SQL statement and using the value of the first column which is read by the reader as a string.
The easiest way would be to pass the SQL statement and the ID to a method which returns the read string which then can be assigned to the desired property of the doenca (whatever that is).

Because usually an ID is unique I expect that query will only return one item, so you should use ExecuteScalar() instead of having the while(reader.Read()) loop.
This could look like so
private string ExecuteScalar(string sqlQuery, string id)
{
    using(IDbConnection dbConnection = new SqliteConnection(connectionString))
    {
        dbConnection.Open();

        using(IDbCommand dbCommand = dbConnection.CreateCommand())
        {
            dbCommand.Parameters.Add(new SqliteParameter("@id", id));
            dbCommand.CommandText = sqlQuery;

            return ConvertFromDBVal<string>(dbCommand.ExecuteScalar());
        }
    }
}

where the ConvertFromDBVal<T>() method is
public static T ConvertFromDBVal<T>(object obj)
{
    if (obj == null || obj == DBNull.Value)
    {
        return default(T); // returns the default value for the type
    }
    else
    {
        return (T)obj;
    }
}

now the assignment of that properties could look like so
doenca.nome = ExecuteScalar("SELECT nome FROM doenca WHERE id = @id",  doencaId);
doenca.descricao = ExecuteScalar("SELECT descricao FROM descricao_doenca WHERE id = @id", doencaId);

If you want to hide that implementation you could add two more methods like so
private string FetchNomeById(string id)
{
    return ExecuteScalar("SELECT nome FROM doenca WHERE id = @id",  id);
}
private string FetchDescricaoById(string id)
{
    return ExecuteScalar("SELECT descricao FROM descricao_doenca WHERE id = @id",  id);
}

which results in assigning the properties like so
doenca.nome = FetchNomeById(doencaId);
doenca.descricao = FetchDescricaoById(doencaId);

but I would like to encourage you to use english for naming classes, methods and variables, because if Sam the maintainer isn't familiar with your language he/she will at least speak english because thats just what each developer does, so it would be easier for Sam to get what your code is about.

The second two methods are doing basically the same as well.  They open a connection to a database, executing a SQL statement and using the values of the first column. The difference is that they "return" different types so this is screaming for a generic version like so
private IList<T> FetchValuesOfSingleColumn<T>(string sql, string condition)
{
    IList<T> values = new List<T>();

    using (IDbConnection dbConnection = new SqliteConnection(connectionString))
    {
        dbConnection.Open();

        using (IDbCommand dbCommand = dbConnection.CreateCommand())
        {
            string sqlQuery = string.IsNullOrWhiteSpace(condition)
                            ? sql
                            : sql + " WHERE " + condition;

            dbCommand.CommandText = sqlQuery;

            using (IDataReader reader = dbCommand.ExecuteReader())
            {
                while (reader.Read())
                {
                    values.Add(ConvertFromDBVal<T>(reader[0]));
                }
            }
        }
    }
    return values;
}

which can be used to get all values from a single column without a condition as well.
Assuming doenca.sintomas is a List<string> the usage could of that method could then look like so
string condition =
IList<string> descratios = FetchValuesOfSingleColumn<string>("SELECT descricao FROM sintoma", condition);

doeanca.sintomas.AddRange(descratios);

where condition could be constructed like so
private string BuildCondition<T>(IEnumerable<T> values, string columnName, string @operator, string conditionType)
{
    IList<T> items = values.ToList();
    if (items.Count == 0) { return string.Empty; }
    if (conditionType==null) { throw new ArgumentNullException("conditionType)"); }
    if(string.IsNullOrWhiteSpace(conditionType)) { throw new ArgumentException("parameter may not be empty", "conditionType"); }

    var builder = new StringBuilder();
    string format = columnName + @operator + "{0}";
    string conditionTypeFormat = " " + conditionType + " " + format;

    builder.AppendFormat(format, items[0]);

    for (int i = 1; i < items.Count; i++)
    {
        builder.AppendFormat(conditionTypeFormat, items[i]);
    }
    return builder.ToString();
}

resulting in
string condition = BuildCondition<int>(listaIds, "id", "=","OR");
IList<string> descratios = FetchValuesOfSingleColumn<string>("SELECT descricao FROM sintoma", condition);
doeanca.sintomas.AddRange(descratios);

and for the other property assignment
string condition = BuildCondition<int>(new int[]{ doencaId }, "id", "=","OR");
IList<int> descratios = FetchValuesOfSingleColumn<int>("SELECT sintoma_id FROM doenca_sintoma", condition);
listaIds.AddRange(descratios);

The BuildCondition<T>() method will only work correctly with T being a numeric types because this types won't need quotes for the query.

If you need more queries with datamappings I would like to encourage you to dig into some object relational framework like for instance Entity Framework or any other relational mapping framework.


Clubbing three queries that return mutually exclusive records There is a table in Oracle database with the following data

+----+-----+-------+---------+---------+---------+
| Id | Key | Value | Is_Orig | Is_Prev | Is_Curr |
+----+-----+-------+---------+---------+---------+
|  1 |  10 |  1000 |       1 |       0 |       0 |
|  2 |  10 |  2000 |       0 |       0 |       0 |
|  3 |  10 |  3000 |       0 |       1 |       0 |
|  4 |  10 |  4000 |       0 |       0 |       1 |
|  5 |  20 |   100 |       1 |       0 |       0 |
|  6 |  20 |   300 |       0 |       1 |       0 |
|  7 |  20 |   400 |       0 |       0 |       1 |
|  8 |  30 |    10 |       1 |       0 |       0 |
+----+-----+-------+---------+---------+---------+

I need to query the table and display the data as follows

+-----+------------+------------+------------+
| Key | Orig Value | Prev Value | Curr Value |
+-----+------------+------------+------------+
|  10 |       1000 | 3000       | 4000       |
|  20 |        100 | 300        | 400        |
|  30 |         10 | n/a        | n/a        |
+-----+------------+------------+------------+

I was thinking on the following lines.

SELECT t1.key, t1.orig_value, t2.prev_value, t3.curr_value
FROM (
(SELECT key, value as orig_value
 FROM statistics
 WHERE is_orig = 1
) AS t1,
(SELECT key, value as prev_value
 FROM statistics
 WHERE is_prev = 1
) AS t2,
(SELECT key, value as curr_value
 FROM statistics
 WHERE is_curr = 1
) AS t3
)

This works; but is there a better way to get the desired result?

Kindly note that is_orig=1, is_prev=1, and is_curr=1 are all mutually exclusive of each other. Hence, the three separate queries.

You can use aggregation like this:

select "Key",
   MAX(CASE WHEN "Is_Orig" = 1 THEN "Value" END) "Orig Value",
   MAX(CASE WHEN "Is_Prev" = 1 THEN "Value" END) "Prev Value",
   MAX(CASE WHEN "Is_Curr" = 1 THEN "Value" END) "Curr Value"
from statistics
group by "Key"

The CASE statements select the value in each case and return NULL in the other cases. MAX is just to get that one value from the nulls.

Have a look at https://modern-sql.com/use-case/pivot for some background. It also refers to an Oracle specific feature called "modeling" that allows a similar thing.


Integer Palindrome Check in C This method should check whether a number is a palindrome or not:

typedef enum { false, true } Bool;

Bool is_pal(int n) {
  Bool is_pal_helper(const char *arr, size_t size);

  char temp[12];                         // INT_MIN == '-' + 10 digits (+ null-Terminator)
  sprintf(temp, "%d", n);
  return is_pal_helper(temp, strlen(temp));
}

Bool is_pal_helper(const char *arr, size_t size) {
  if (size <= 1)
    return true;

  if (*arr != *(arr+size-1))
    return false;

  return is_pal_helper(arr+1, size - 2);
}

This was my intuitive solution and I was wondering, if it would be more efficient to keep the number an int, copy and reverse (with modulo and divide-operations) the int and check whether the (reversed) copy is equal to the original int.

A few notes:

Why aren't you using stdbool.h?  Use that instead of rolling your own.
Why are you using recursion?  If you're looking for efficiency, you will use iteration in almost every single circumstance.
Why are you constraining yourself to just ints as palindromes if you are already accepting a string with your helper?  If you want to stay true to this you could keep your method that you have (with a few changes):

bool isIntPalindrome(int n)
{
    // palindromes only apply to the natural numbers
    if (n < 0) return false;

    // INT_MAX == 32767 + '\0' -> 6
    // double that to be safe on weird machines
    char temp[12];
    sprintf(temp, "%d", n);
    return isPalindrome(temp);
}

Here's my take on your problem, should be faster.  I'll provide speed test results in a bit

#include <stdio.h>
#include <stdbool.h>
#include <string.h>
#include <ctype.h>

bool isPalindrome(char* str)
{
    size_t len = strlen(str);
    for (char *ptr1 = str, *ptr2 = str + len - 1; ptr2 >= ptr1; ptr1++, ptr2--)
    {
        // not super strict check, just test if same lowercase ASCII value
        if(tolower(*ptr1) != tolower(*ptr2)) return false;
    }
    return true;
}

int main(void)
{
    puts(isPalindrome("123454321") ? "true" : "false");
    puts(isPalindrome("12") ? "true" : "false");
    puts(isPalindrome("racecar") ? "true" : "false");
    puts(isPalindrome("test") ? "true" : "false");
}

Output:

$ time ./op
true
false
# had to exclude string examples

real  0m0.025s
user  0m0.011s
sys   0m0.012s

$ time ./syb0rg
true
false
true
false

real  0m0.006s
user  0m0.001s
sys   0m0.002s

Now the resulting time differences aren't that big, but my code didn't try and put the method to its limits to really see how it would perform with something a bit bigger for example.  But in the end mine is a bit faster!


Encrypting a user's file with a key that isn't stored but only known by the user I want to make some kind of file vault. I want to store encrypted files in a MySQL table, and the key to encrypt/decrypt them is never stored but only known to the user (that's the goal, anyway).

The user has to be logged in to view a list of his files. Then, to view a file, the user has to enter a key to decrypt a file to read it. So assuming a user's account was hacked and his password was discovered, I want the hacker to still be unable to decrypt the files, since the hacker doesn't know the key used to encrypt it.

What are the biggest holes in this? Please help me improve it rather than just saying 'this is terrible', 'use a library', or 'don't do it yourself'. I know the security of this depends on other code that I didn't put here. I just tried to put the most relevant parts.

Here is some code from the controller to add a new file:

public function addFile($request, $response) {

    $user = Security::getUser();
    if (!$user) { $this->denyAccess('add-file'); }

    $unencrypted_content = $request->getParam('content');
    $key = $request->getParam('key');
    if (Security::authenticate($user->username, $key)) {
        $this->flash->addMessage('error', 'You can not use your password as the key to your files.');
        return $this->redirect('new-file');
    }

    $otherFile = $user->files()->first();
    if ($otherFile && !Crypt::decrypt($otherFile->content, $key, $otherFile->iv)) {
        $this->flash->addMessage('error', 'The key you entered failed to decrypt another one of your files. You must always use the same key.');
        return $this->redirect('new-file');
    }

    $iv = Crypt::iv();
    $encrypted_content = Crypt::encrypt($unencrypted_content, $key, $iv);

    $file = new File;
    $file->owner()->associate($user);
    $file->name = $request->getParam('name');
    $file->content = $encrypted_content;
    $file->iv = $iv;
    $file->save();

    return $this->redirect('all-files');
}

Here is the Crypt class:

public static function iv() {
    $letters = 'abcdefghijklmnopqrstuvwxyz0123456789';
    $iv = '';

    for ($i = 0; $i < 16; $i++) {
        $iv .= substr($letters, rand(0, 35), 1);
    }

    return $iv;
}

public static function encrypt($content, $key, $iv) {
    return openssl_encrypt($content, 'aes-256-cbc', $key, 0, $iv);
}

public static function decrypt($content, $key, $iv) {
    return openssl_decrypt($content, 'aes-256-cbc', $key, 0, $iv);
}

And again in the controller to view a file:

public function viewFile($request, $response) {
    $csrf_name = $request->getAttribute('csrf_name');
    $csrf_value = $request->getAttribute('csrf_value');

    $file = File::find($request->getAttribute('id'));
    if ($file->owner->id !== Security::getUser()->id) { $this->denyAccess('view-file/' . $file->id); }

    $key = $request->getParam('key');
    $decrypted_content = Crypt::decrypt($file->content, $key, $file->iv);

    if (!$decrypted_content) {
        $this->flash->addMessage('error', 'The key you entered is not valid.');
        return $this->redirect('authorize', ['id' => $file->id]);
    }

    $file->content = $decrypted_content;
    $file->key = $key;

    return $this->render($response, 'file/view.html.twig', compact('file', 'csrf_value', 'csrf_name'));
}

I honestly disagree with this whole approach.  First, storing files IN a relational database is rarely a good strategy, limited almost exclusively to use cases where you want to perform binary searches against the binary artifacts.  That is not your use case here. Outside of that, you are just adding a lot of overhead to your typical database management operations (making DB backups/snapshots, replicating data across DB servers, etc.) without really any added value in having the files in the DB.

Your use case is simply one of needing to operate on the file in it's entirety, so I would think typical directory-based disk storage or cloud-based file storage mechanisms would make much more sense in terms of being able to operate against the collection of files as you typically would a collection of files.  Your database could then only store reference to the files.  I would have a strong bias towards using OS-based mechanisms for archiving, syncing, encrypting/decrypting, etc.   PHP obviously can give you entry points to these sorts of operations.

I also VERY much question your encryption approach (and the approach for trying to roll your own logic).  You are talking about what would seem to be an assymmetric cryptography use case here, but you are attempting to execute this using a symmetric encryption approach.  This approach seems to not make sense, because, as soon as the end user passes their key to the application, the application can now decrypt the data, as it now has the shared symmetric encryption key.

In this case you then need to generate a shared key for each user. This would beg the question as to why the user even needs to pass the shared key, since the application should theoretically already store it.  This means the application could decrypt the data without user interaction, which seems to not be what you desire.

If you truly want an assymmetric approach, then you probably need to forfeit the idea of the application performing ANY decryption at all. That should be up to the end user and/or a client-side application where this operation takes place.  Outside of that, it seems you are defeating the entire purpose of what you are trying to do, which is storage of an encrypted file without the ability for the application to decrypt it, without the user having a key.


Perform a predetermined set of operations on a large sequence This is a programming challenge.

I have been given the following input:

  \$n\$ where n is size of sequence
  \$k\$ is an integer
  a sequence \$A_0, A_1, A_2, \ldots A_{n-1}\$

I need to perform for each index from 0 to (k-1) inclusive

find a,b
where a = A[i % n] and b = A[n - (i % n) - 1]
then set A[i % n] = a XOR b

and output the final sequence, separated by spaces

The inputs are within the ranges:

  \$n \leq 10^4\$
  \$k \leq 10^{12}\$
  \$A_i \leq 10^7\$

I have applied the following naive approach

n,k=map(int,input().split())
arr=list(map(int,input().split())) #read input sequence and store it as list type
for i in range(k): #iterate over 0 to (k-1)
    t=i%n          #module of i wrt n
    arr[t]=arr[t]^arr[n-(t)-1]  #xor between two list elements and then set result to the ith list element
for i in arr:
    print(i,end=" ")  #print the final sequence

This code runs in 2 seconds when \$K \leq 10^6\$, but it shows Time Limit exceeded for large test cases.
I am looking for an alternate suggestion for the problem

Now that that pesky "Not looking for the review of my code" is gone...

Step 1: White space

Follow the PEP 8 guidelines, specifically (but not limited to) put a space around operators and after commas:

n, k = map(int, input().split())
arr = list(map(int, input().split()))   # read input sequence and store it as list type
for i in range(k):                      # iterate over 0 to (k-1)
    t = i % n                           # module of i wrt n
    arr[t] = arr[t] ^ arr[n - (t) - 1]  # xor between two list elements and then set result to the ith list element
for i in arr:
    print(i, end=" ")                   # print the final sequence

Much easier to read.

Step 2: Avoid multiple lookups

Python is an interpreted language, and the meaning of a line of code -- or even a fragment of code -- can change by the time the interpreter returns to execute the code as second time.  This means the interpreter cannot truly compile the code; unless something is a well defined short-circuiting operation, every operation must be executed.

Consider:

arr[t] = arr[t] ^ arr[n - (t) - 1]

The interpreter must compute the address of arr[t] twice; once to fetch the value, and a second time to store the new value, because some side-effect which occurs during the execution of arr[n - (t) - 1] may change the meaning of arr[t].  In your case, arr is a list, and n and t are simple integers, but with user-defined types, anything can happen.  As such, the Python interpreter can never make the following optimization:

arr[t] ^= arr[n - (t) - 1]

It is a tiny speed-up, but considering the code may execute \$10^{12}\$ times, it can add up.

Step 3: Avoid calculations

Speaking of avoiding work: because we know the length of the array is fixed, arr[n - 1] is the same as arr[-1].  So we can further speed up the line of code as follows:

arr[t] ^= arr[-1 - t]

Instead of two subtractions, we now have only one.  Yes, Python has to index from the back of the array, which internally is going to involve a subtraction, BUT that will be an optimized, C-coded subtraction operation on ssize_t values, instead of subtraction on variable byte length integers, which must be allocated and deallocated from the heap.

Step 4: Printing space-separated lists

The following is slow:

for i in arr:
    print(i, end=" ")

This is faster:

print(*arr)

And for long lists, this may be fastest:

print(" ".join(map(str, arr)))

For a detail discussion, including timing charts, see my answer and this answer on another question.

Step 5: The Algorithm

Consider the list [A, B, C, D, E].

After applying a single pass of the operation on it (ie, k = n), you'll get:

[A^E, B^D, C^C, D^(B^D), E^(A^E)]

which simplifies to:

[A^E, B^D, 0, B, A]

If we apply a second pass (ie, k = 2*n), you'll get:

[(A^E)^A, (B^D)^B, 0^0, B^((B^D)^B), A^((A^E)^A)]

which simplifies to:

[E, D, 0, B^D, A^E]

A third pass, (ie, k = 3*n) gives:

[E^(A^E), D^(B^D), 0^0, (B^D)^(D^(B^D)), (A^E)^(E^(A^E))]

or:

[A, B, 0, D, E]

Now k does not need to be an exact multiple of n, so you'll have to figure out what to do in the general cases, but you should be able to use the above observation to eliminate a lot of unnecessary calculations.

Implementation left to student.


My Own ThreadPool implementation I tried to make a Server using Java with a Thread Pool. Any review is welcome:

/* User: koray@tugay.biz Date: 21/02/15 Time: 21:12 */

import java.io.IOException;
import java.net.ServerSocket;
import java.net.Socket;
import java.util.Stack;

public class MyConnectionAccepter {

    private Stack<MySocketThread> mySocketThreads = new Stack<MySocketThread>();
    private volatile int currentNumberOfConnections = 0;

    public MyConnectionAccepter() {

        MySocketThread mySocketThreadOne = new MySocketThread(this);
        MySocketThread mySocketThreadTwo = new MySocketThread(this);

        mySocketThreadOne.setDaemon(true);
        mySocketThreadTwo.setDaemon(true);

        mySocketThreadOne.start();
        mySocketThreadTwo.start();

        mySocketThreads.push(mySocketThreadOne);
        mySocketThreads.push(mySocketThreadTwo);

    }

    public void start() throws IOException {

        ServerSocket serverSocket = new ServerSocket(8888);

        while (true) {
            while (currentNumberOfConnections < 2) {
                Socket accept = serverSocket.accept();
                MySocketThread mySocketThread = mySocketThreads.pop();
                mySocketThread.setSocket(accept);
                currentNumberOfConnections++;
            }
        }

    }

    public void informIAmDone(MySocketThread mySocketThread) {
        mySocketThreads.push(mySocketThread);
        currentNumberOfConnections--;
    }
}

and

/* User: koray@tugay.biz Date: 21/02/15 Time: 21:04 */

import java.io.IOException;
import java.net.Socket;
import java.util.Scanner;

public class MySocketThread extends Thread {

    private volatile Socket socket;
    MyConnectionAccepter myConnectionAccepter;

    public MySocketThread(MyConnectionAccepter myConnectionAccepter) {
        this.myConnectionAccepter = myConnectionAccepter;
    }

    @Override
    public synchronized void run() {
        serve();
    }

    public void setSocket(Socket socket) {
        this.socket = socket;
    }

    public void serve() {
        while(socket == null) {

        }
        while (socket != null) {
            Scanner scanner = null;
            try {
                scanner = new Scanner(socket.getInputStream());
            } catch (IOException e) {
                e.printStackTrace();
            }
            String readLine;
            while (!(readLine = scanner.nextLine()).equals("bye")) {
                System.out.println(readLine);
            }
            try {
                socket.close();
            } catch (IOException e) {
                e.printStackTrace();
            }
            socket = null;
            myConnectionAccepter.informIAmDone(this);
        }
        serve();
    }

}

and the Test Class:

/* User: koray@tugay.biz Date: 21/02/15 Time: 21:18 */

import java.io.IOException;

public class MyTestClass {

    public static void main(String[] args) throws IOException {
        MyConnectionAccepter myConnectionAccepter = new MyConnectionAccepter();
        myConnectionAccepter.start();
    }
}

My goal was to allow 2 connection at any time. So I can connect from Terminal like this:

Korays-MacBook-Pro:~ koraytugay$ telnet localhost 8888
Trying ::1...
Connected to localhost.
Escape character is '^]'.

But a new Thread will not be created, an available Thread will popped from the Stack and that will be used. If no Threads are available in the pool, the new connection will simply need to wait until some other connection closes. Closing a connection does not stop a Thread, it only makes it free.

Any review is welcome from any aspect.

Generic code review
This is know as a busy loop:
    while(socket == null) {

    }

Not a good idea in any language.
Do you hear your fans spin up when your threads start executing this loop?
You should probably block the thread until the condition becomes true: See is there a 'block until condition becomes true' function in java?
Recursively calling your own function is not a good idea:
 public void serve() {
    .. DO WORK ITEM 1
    serve();
}

That would not explode immediately. But in a production system that is running a long time that will eventually fill up the stack and cause the application to crash. People will spend years trying to work out why it crashed (because its not obvious that this is a slow and steady memory leak that is undetectable).
Why not put that in a loop?
public synchronized void run() {
    while(!finished) {
        serve();
    }
     // remove the recursive call inside serve()
}

This is another hidden busy loop:
        while (currentNumberOfConnections < 2) {
            Socket accept = serverSocket.accept();
            MySocketThread mySocketThread = mySocketThreads.pop();
            mySocketThread.setSocket(accept);
            currentNumberOfConnections++;
        }

When you have two active connections then this will spin like mad and just eat CPU resources. Put another block in there so that the thread is paused until your worker threads can keep going
Design Review:
I think your socket and your threads are to tightly coupled. I would rather separate them and make them independent of each other.
Have your socket class accepts incoming requests and for each request create a work item in a queue. That way it keeps accepting requests and does not need to drop any. If the threads get there two slowly then the client will close the connection and your thread will see this when it tries to service the client rquest.
One the other side make your threads collect work items from the queue (mentioned above). You then can add more threads without the socket class knowing about any of the threads (they just know about the queue).


TPL Dataflow BatchBlock with Batch Size & Timeout I needed BatchBlock with support for timeout (act greedy on what it received at that time) and by some search and study, this is the outcome.

What are drawbacks or rooms for improvement in this code?

class BatchedQueue
{
    static readonly Logger SlotClassLogger = LogManager.GetCurrentClassLogger();
    protected static Logger ClassLogger { get { return SlotClassLogger; } }
}

class BatchedQueue<T> : BatchedQueue
{
    readonly int _interval;
    readonly TimeSpan _loopWait;
    readonly BatchBlock<T> _queue;
    readonly Timer _timer;
    readonly BufferBlock<T> _source;

    public event Action<T[]> Received;

    protected virtual void OnReceived(T[] obj)
    {
        var handler = Received;
        if (handler != null) handler(obj);
    }

    public BatchedQueue(int interval, int size)
    {
        _interval = interval;

        _loopWait = TimeSpan.FromMilliseconds(2.19 * (1.3 + _interval)); //study

        _queue = new BatchBlock<T>(size, new GroupingDataflowBlockOptions { Greedy = true });
        _timer = new Timer(_ => _queue.TriggerBatch());

        var transformer = new TransformBlock<T, T>(v =>
        {
            _timer.Change(_interval, Timeout.Infinite);
            return v;
        });
        transformer.LinkTo(_queue);
        _source = new BufferBlock<T>();
        _source.LinkTo(transformer);

        Utl.SafeTask(Loop, t =>
        {
            if (t == null || t.Exception == null) return;
            ClassLogger.Error(t.Exception);
        }, TaskCreationOptions.LongRunning);
    }

    public virtual void Post(T t) { _source.Post(t); }
    void Loop()
    {
        while (true)
        {
            var buffer = _queue.Receive(_loopWait);
            if (buffer != null) { OnReceived(buffer); }
        }
    }
}

class BatchedQueue

What's the reason for having this separate class? Is it so that there is only one instance of SlotClassLogger? If that's the case, that sounds like premature optimization to me.

class BatchedQueue<T> : BatchedQueue

You're not implementing (or exposing in other way) any of the dataflow interfaces. That means this class can't easily be used as a part of a dataflow network.

readonly int _interval;

When you have a value that is of some unit of measure, you should always very clearly specify what unit is uses. (You don't want your Mars probe to crash, right?) You can do that either in the name of variable or in a comment.

Though it seems you're not using any common unit. That should also be document very clearly and it should also have a very good reason. If the unit is something specific to your domain, then I don't think it belongs here, a class like this should be reusable.

If the condition for triggering it not actually some time elapsing, then consider using that condition directly, instead of approximating it using a timer.

transformer.LinkTo(_queue);
_source = new BufferBlock<T>();
_source.LinkTo(transformer);

I don't see any reason for the _source buffer block here. You could send items directly into transformer.

while (true)
{
    var buffer = _queue.Receive(_loopWait);
    if (buffer != null) { OnReceived(buffer); }
}

This code is blocking a thread unnecessarily when there is no work to do. The simplest solution would be to use ActionBlock to execute OnReceived.

There is no way to tell the queue to stop processing or for your to wait until its processing is done. Consider adding the Complete()/Completed pair used in dataflow blocks.

When dealing with time, Rx is often better than Dataflow. A behavior similar to yours could be achived by using Buffer():

_source = new Subject<T>();
_source.Buffer(_loopWait, size)
       .Subscribe(OnReceived);

Though this measures the timeout since the last batch, not since the last element. I think you could achieve that using something like (based on this SO answer):

_source.Buffer(() =>
    Observable.Merge(_source.Throttle(_loopWait).Take(1), _source.Skip(size - 1).Take(1)))
    .Subscribe(OnReceived);


Censoring words in lua (repost: https://stackoverflow.com/questions/59665338/censoring-words-in-lua#comment105491094_59665338, as advised)

Im currently making a discord censor bot for my friend, and I was wondering if anyone with experience could help me, or if theres an easier way to complete my code.

local BadWords = {
    ['poop']=true,
}

client:on('messageCreate', function(msg)
    Msg = msg.content
    local msgs = string.split(Msg,' ')
    for i,v in pairs(msgs) do
        if BadWords[v:lower()] then
            msg:reply({
                embed = {
                    fields = {
                    {name = "Bad Word",value=v,inline = true},
                    },
                    color = discordia.Color.fromRGB(114, 137, 218).value,

                    --footer = os.time(),
                }
            })
            --msg:reply(('Blacklisted word: %s'):format(v))
        end
    end
    --[[table.foreach(msgs,function(i,v)
        if BadWords[i] or BadWords[v] then
            msg:reply(('Blacklisted word: %s'):format(v))
        end
    end)--]]
end)
client:run(Settings.Token)

There's nothing wrong with it, nor is it complete but my idea was to split, or concatenate all the words they send respectively, mutate into all versions of the word (i.e. farm, f4rm, type shit) and then checking the BadWord list.

Mostly what I was wondering was if there's a more efficient way to do this, or if I just gotta do what I gotta do

If you care about performance, you should have a look at LPeg. It makes it way easier to handle large numbers of string matching and substitution rules.

For example, you could define patterns and their substitution values like this:

local lpeg = require 'lpeg'

local p = lpeg.P
local bad_word =
   p"bad" / "good" +
   (p"dumb" + p"stupid") / "$#&*"

local sanitize = lpeg.Cs( (bad_word + p(1))^1 )

print(sanitize:match("this is a bad word"))
print(sanitize:match("python is dumb!"))

Which makes it much easier both to add new entries and to eventually move them into a table or even a separate file.


How to reduce runtime of gene data processing program? This program takes a huge data set as input, processes it, calculates and then writes the output to an array.  Most calculations may be quite simple, such as summation. In the input file, there are about 100 million rows and 3 columns.

first column is the name of the gene (total 100 millions)
second column is the specific value
third column is another value of each gene

The problem I face is a long runtime. How can I reduce it?

I need to write all new values (from GenePair to RM_pval with header) I calculated from the new file.

fi = open ('1.txt')
fo = open ('2.txt','w')

import math
def log(x):
    return math.log(x)

from math import sqrt

import sys
sys.path.append('/tools/lib/python2.7/site-packages')
import numpy
import scipy
import numpy as np
from scipy.stats.distributions import norm

for line in fi.xreadlines():
    tmp = line.split('\t')

    GenePair = tmp[0].strip()

    PCC_A = float(tmp[1].strip())
    PCC_B = float(tmp[2].strip())

    ZVAL_A = 0.5 * log((1+PCC_A)/(1-PCC_A))
    ZVAL_B = 0.5 * log((1+PCC_B)/(1-PCC_B))

    ABS_ZVAL_A = abs(ZVAL_A)
    ABS_ZVAL_B = abs(ZVAL_B)

    Var_A = float(1) / float(21-3) #SAMPLESIZE - 3
    Var_B = float(1) / float(18-3) #SAMPLESIZE - 3

    WT_A = 1/Var_A #float
    WT_B = 1/Var_B #float

    ZVAL_A_X_WT_A = ZVAL_A * WT_A #float
    ZVAL_B_X_WT_B = ZVAL_B * WT_B #float

    SumofWT = (WT_A + WT_B) #float
    SumofZVAL_X_WT = (ZVAL_A_X_WT_A + ZVAL_B_X_WT_B) #float

    #FIXED MODEL
    meanES = SumofZVAL_X_WT / SumofWT #float
    Var = float(1) / SumofWT #float
    SE = math.sqrt(float(Var)) #float
    LL = meanES - (1.96 * SE) #float
    UL = meanES - (1.96 * SE) #float
    z_score = meanES / SE #float
    p_val = scipy.stats.norm.sf(z_score)

    #CAL
    ES_POWER_X_WT_A = pow(ZVAL_A,2) * WT_A #float
    ES_POWER_X_WT_B = pow(ZVAL_B,2) * WT_B #float
    WT_POWER_A = pow(WT_A,2)
    WT_POWER_B = pow(WT_B,2)
    SumofES_POWER_X_WT = ES_POWER_X_WT_A + ES_POWER_X_WT_B
    SumofWT_POWER = WT_POWER_A + WT_POWER_B

    #COMPUTE TAU
    tmp_A = ZVAL_A - meanES
    tmp_B = ZVAL_B - meanES
    temp = pow(SumofZVAL_X_WT,2)

    Q = SumofES_POWER_X_WT - (temp /(SumofWT))
    if PCC_A !=0 or PCC_B !=0:
        df = 0
    else:
        df = 1

    c = SumofWT - ((pow(SumofWT,2))/SumofWT)
    if c == 0:
        tau_square = 0
    else:
        tau_square = (Q - df) / c

    #calculation
    Var_total_A = Var_A + tau_square
    Var_total_B = Var_B + tau_square

    WT_total_A = float(1) / Var_total_A
    WT_total_B = float(1) / Var_total_B

    ZVAL_X_WT_total_A = ZVAL_A * WT_total_A
    ZVAL_X_WT_total_B = ZVAL_B * WT_total_B

    Sumoftotal_WT = WT_total_A + WT_total_B
    Sumoftotal_ZVAL_X_WT= ZVAL_X_WT_total_A + ZVAL_X_WT_total_B

    #RANDOM MODEL
    RM_meanES = Sumoftotal_ZVAL_X_WT / Sumoftotal_WT
    RM_Var = float(1) / Sumoftotal_WT
    RM_SE = math.sqrt(float(RM_Var))
    RM_LL = RM_meanES - (1.96 * RM_SE)
    RM_UL = RM_meanES + (1.96 * RM_SE)
    RM_z_score = RM_meanES / RM_Var
    RM_p_val = scipy.stats.norm.sf(RM_z_score)

The first thing for solving a problem like this is to find out what the actual bottleneck is. In your case the two most likely parameters are disk IO or CPU power.

Disk IO

Make sure the data sits on local drives and not on a network share or USB stick or similar.
Make sure your disks are reasonably fast (SSDs might help).
Put your input file and your output file on two different hard drives.
mmap might gain you some speed in reading and/or writing the data. At least in this case it seemed to make a difference.

CPU

There are several calculations you perform for each line in the input file but those numbers seem to be static (unless the script you posted is incomplete)

Var_A = float(1) / float(21-3) #SAMPLESIZE - 3 looks like a constant to me which can be calculated once.
Similar Var_B
From these two a bunch of other variables are calculated which would also be static: WT_A, WT_B, SumofWT, Var, SE, WT_POWER_A, WT_POWER_B, SumofWT_POWER, c
Might not make a huge impact but still seems redundant

As each line seems to be independently calculated this code would be a prime example for parallelization. I have not much experience with python and parallel programming but the ProcessPoolExecutor seems like a good start. Bascially utilize as many processes as you have cores and split your input into chunks to be distributed to the processes. You'd have to collect the results (and presumably make sure you write them out in the correct order) so this will make your code a bit more complicated but has the potential to speed up your calculations close to a factor of N (assuming N is the number of CPU cores) provided disk IO is not killing you.

You could also split the input, distribute it to other computers in the office and get the processing done there and collect the results.


javascript multiple inheritance code review i had made a small code snippet for multiple inheritance in js. this code is working for me but actually i like to get the review, that its good and if any problem; provide the solution. etc. the code snippet is below.

/**
 * WebbaseUtility provide the bundle of utilities used for support the webbase
 * functionalities;
 * @package : WebbaseUtility;
 * @version : 1.0.0;
 */
var WebbaseUtility = {};

/**
 * ObjectExt module extends the object properties and functionalities like inheritance etc;
 * @module : ObjectExt;
 * @package : Webbase;
 * @version : 1.0.0;
 * @return Object;
 */

var ObjectExt = WebbaseUtility.ObjectExt = (function()  {

    /**
     * ObjectExt Constructor;
     */
    var ObjectExt = function()  {
    };

    /**
     * This method provide inheritance to the object supplied; this method inherit the
     * public methods from the Parent class to the Child class. this also provide
     * multiple inheritance, in which the method ambiguity is solved by the overriding
     * the last inherited class's method;
     * @access public;
     * @method inherit;
     * @param Object Parent;
     * @param Object Child;
     * @return Object;
     */
    ObjectExt.prototype.inherit = function(Parent, Child)   {
        var TempObj = function(){}, MultipleInheritanceTempObj = function(){};

        MultipleInheritanceTempObj.prototype = Child.prototype;
        TempObj.prototype = Parent.prototype;

        for(var key in MultipleInheritanceTempObj.prototype)    {
            TempObj.prototype[key] = MultipleInheritanceTempObj.prototype[key];
        }
        MultipleInheritanceTempObj = null;
        Child.prototype = new TempObj();

        if(Child.uber === undefined)    {
            Child.uber = Parent.prototype;
        }else   {
            for(var key in Parent.prototype)    {
                Child.uber[key] = Parent.prototype[key];
            }
        }
        Child.prototype.constructor = Child;

        return Child;
    };

    return new ObjectExt();

})();

Child = WebbaseUtility.ObjectExt.inherit(ParentA, Child);
Child = WebbaseUtility.ObjectExt.inherit(ParentB, Child);

First of all, I responded to the post in c.l.js, and this content in part reiterates the comments there.

My first thought is that you should look at some of the available mixin libraries.  There are many mixin techniques available.  And on top of that, there are many interesting approaches layered on top of them.  Peter Michaux published one recently on applying mixins to constructor functions.  Angus Croll published one on creating functions out of mixins, and together with Dan Webb he also presented this technique at the recent Fluent Conference.  I published a rough draft of an article on a still different approach to mixins.

I don't understand the reason for your complex syntactic structure.
You assign your ObjectExt variable to the result of an immediately
invoked function expression (IIFE).  That IIFE involves defining a
constructor function, attaching the inherit function to its
prototype`, and then constructing and returning an instance of that
constructor:

 var ObjectExt = (function () {
     var ObjectExt = function () { };
     ObjectExt.prototype.inherit = function (Parent, Child) {
         // content here
     };
     return new ObjectExt();
 })();

I don't see that any of that serves any purpose over the much simpler
version here:

 var ObjectExt = {inherit: function (Parent, Child) {
     // content here
 };

Can you give any reason for the existence of this constructor
function, the IIFE, the prototype, etc?  It seems to be plain
overhead, and serves only to obfuscate your code.

One other minor point is that you probably should declare your key
variable once in the top of the inherit function.

But the main issue is that you add properties to the parent.prototype object.  In this code:

if(Child.uber === undefined)    {
    Child.uber = Parent.prototype;               // (1)
} else {
    for(var key in Parent.prototype)    {
       Child.uber[key] = Parent.prototype[key];  // (2)
    }
}

in the line marked (1), you create the Child.uber property as reference to the Parent.prototype.  So any properties you add when you come through the if-block again and hit (2) will be added to the orignial prototype object.  This is a pretty fundamental flaw.  An example would be the following:

var Pet = function(name) {this.name = name;}
Pet.prototype.speak = function(){
    console.log(this.name + " speaks.");
};
var WalkingAnimal = function() {};
WalkingAnimal.prototype.walk = function() {
    console.log(this.name + " is walking.");
};
var SwimmingAnimal = function() {};
SwimmingAnimal.prototype.swim = function() {
    console.log(this.name + " is swimming.");
};
var Dog = function(name) {this.name = name;};
WebbaseUtility.ObjectExt.inherit(Pet, Dog);
WebbaseUtility.ObjectExt.inherit(WalkingAnimal, Dog);

var ClownFish = function(name) {this.name = name;};
WebbaseUtility.ObjectExt.inherit(Pet, ClownFish);
WebbaseUtility.ObjectExt.inherit(SwimmingAnimal, ClownFish);

var rover = new Dog("Rover");
var nemo = new ClownFish("Nemo");
rover.walk();  // "Rover is walking";
nemo.swim();   // "Nemo is swimming";
nemo.walk();   // "Nemo is walking";   // WTF?

This is a fairly fundamental design flaw.  I think you need to rethink your approach to this problem.

Cheers,

-- Scott


C gets_s() implementation I'm in a class that is using C, and my instructor has unfortunately used gets() in sample code.

As this is obviously a heinous oversight, likely to cause undefined behavior and other various issues (only a little sarcasm), I decided to implement gets_s(), because it was a fun exercise and sometimes it's just not worth it to do full error checking with fgets() and you just want to truncate unexpectedly long lines.

I'm not concerned whether this fully implements gets_s() as specified in the C11 standard -- this is just supposed to be a drop-in replacement for gets() that doesn't overrun your buffer.

However, what is very important is that this function actually does what it advertises: it's safe and doesn't overrun the buffer.

This is my first time working in C (I usually use java or kotlin), and I appreciate all tips, though I'd like at least some mention of the safety of this code, and am also interested in portability (to current compilers).

gets_s.h

#include <stdio.h>
#include <string.h>

#define GETS_S_OK 0
#define GETS_S_ERROR 1
#define GETS_S_OVERRUN 2

static inline int gets_s( char str[], int n )
{
  char *str_end, *fgets_return;
  int temp;

  fgets_return = fgets( str, n, stdin );

  /* If fgets fails, it returns NULL.
  This includes the case where stdin is exhausted. */
  if ( fgets_return == NULL )
  {
    str[0] = '\0';
    return GETS_S_ERROR;
  }

  str_end = str + strlen(str) - 1;

  if ( str_end == '\n' )
  {
    *str_end = '\0';
    return GETS_S_OK;
  }

  temp = fgetc( stdin );
  if (temp == EOF || temp = '\n')
    return GETS_S_OK;

  do temp = fgetc( stdin );
  while ( temp != EOF && temp != '\n' );

  return GETS_S_OVERRUN;
}

and a small test file:

gets_s.c

#include "gets_s.h"
#include <stdio.h>

int main()
{
  char buffer[10];
  int gets_s_return;

  printf("Enter up to %d characters safely.\n", sizeof(buffer) - 1);

  gets_s_return = gets_s( buffer, sizeof(buffer) );

  printf("buffer = %s", buffer);
  printf("gets_s return = %d", gets_s_return);

  return 0;
}

It compiles successfully with gcc -Wall -Wextra -Wpedantic -Werror gets_s.c, so that's a plus.

First, don't call it gets_s as the signature and behavior differs in some subtle and not-so-subtle ways, that just leads to confusion and frustration. Anyway, you really wouldn't want the gets_s-contract.
Call it something descriptive like getline_truncated.

Do you know that n <= 0 is UB in your implementation?

if (temp == EOF || temp = '\n')
                        ^

I'm sure your compiler warns you about the typo above.
Or don't you ask it for all warnings (-Wall -Wextra -std=...)?

I suggest re-defining and re-naming your return-codes to allow better result-testing:

#define GETS_S_TRUNCATED 1
// Because truncation is not neccessarily an error
#define GETS_S_OK 0
#define GETS_S_ERROR EOF
// Because we already have an appropriate negative constant

This way you can test for ==0 or >=0 depending on what you decide is "success", respectively !=0 or <0 for "failure".


Ensuring a non-zero radius when constructing a circle Is there a little "better" way of writing the line:

raise RuntimeError if not radius.is_a? Numeric or radius == 0

I want to only allow numbers bigger than 0.

class Shape
  def initialize(name)
    @name = name
  end

  def to_s
    "name: #{@name}"
  end
end

class Circle < Shape
  def initialize(name, radius = 0)
    raise RuntimeError if not radius.is_a? Numeric or radius == 0
    super(name)
    @radius = radius
  end

  def area
    @radius ** 2 * Math::PI
  end

  def to_s
    "name: #{@name} - radius: #{@radius}"
  end
end

This is my spec:

describe Circle do
  it "should raise exception if radius is 0 or nil" do
    expect {
      Circle.new("0", 0)
    }.to raise_error(RuntimeError)

    expect {
      Circle.new("nil", nil)
    }.to raise_error(RuntimeError)
  end
end

Maybe this?

raise RuntimeError unless radius.is_a?(Numeric) && radius != 0

As mentioned in the comments by 200_success:

Why is zero the default, if it's invalid?
And: What about a negative radius?

Conceptually, a circle with a zero or negative radius might not make a lot of sense, but there's nothing in your code that would break because of it. The area will never be negative since the radius is multiplied by itself. The area might be zero, but again, that is the area of a zero-radius circle, so strictly speaking it's still valid.

You're also checking the type of the radius argument. You can argue either way if that's a good idea. It just raises a RuntimeError, with no extra explanation of what went wrong. If the check wasn't there, and you called, say, Circle.new("banana", "banana").area, you'd just get a NoMethodError instead. Not saying that's better, just saying that it's much worse either. Of course the current check happens in the constructor, which might be nice, but is it necessary?

For most cases, I lean toward saying "Garbage in, garbage out". If you've set the radius to "banana", then don't expect the math to make sense. Since Ruby isn't strictly typed and there's no compile step, the exceptions will occur at runtime anyway. So even if you try to add a lot of type-checking, it's debatable how much value it adds. Especially in this case, since raising a RuntimeError with no extra explanation seems less useful than letting, say, a NoMethodError occur, which will at least tell you things like undefined method**' for "banana"`.

Regardless, no reason to be dogmatic either way; type-checking definitely has its uses, and I'm not saying to always avoid it. Just make sure it adds some value.

Besides, it'd be better to raise an TypeError, since that's a lot more specific to this case than a general things-just-kinda-broke-RuntimeError.

I'd still argue that radius can indeed be zero, and that's fine. It's a strange circle to contemplate, but it's not impossible.

Again, I'd probably just skip the checking entirely, but if I were to add it, I'd write

raise TypeError.new("Radius must be numeric") unless radius.is_a?(Numeric)

And if you do want to avoid strange radii, you can add a separate check after that:

raise ArgumentError.new("Radius must be > 0") if radius <= 0