Checking for the nth prime number between 1 and 1,000,000, must compute 1,000,000th in under 3 minutes My mission is as the title states.
Here is the original code, before optimizing slightly (but still not enough).
Revised code is in my answer, and it still needs work.

Also, I want to write this without having to import any libraries. Time is only there for me to test, of course, time.

Here's the original code, before optimizing:

print("Enter a number n, 1-1,000,000 to find the nth prime number.")
goal=int(input("Number: "))

if goal>1000000:         #closes the program if the number doesn't fit the parameters
    exit()
if goal==1:              #takes care of the first prime, which is 2
    print("2")
if goal==2:              #and then the second, which is 3
    print("3")

chk=5         #the next odd number after 3, which is already taken care of above
primes=2    #the number of primes recorded, after counting 2 and 3.
x=0         #the last recorded prime number
while primes<goal:
    if  chk<9:     #if the number is odd and below 9....
        primes+=1  #it's prime
        x=chk      #record that number
        chk+=2     #and check the next odd number

    else:              #if it's =>9......
        odds=3         #check to see if it's divisible by the smallest odd number
        sqrtk=chk**.5  #all the way up to the square root of k

        while odds<sqrtk or odds==sqrtk:   #if at any time...
            if chk%odds==0:                #...k is divisible by this odd number....
                chk+=2                     #then raise the value of k to the next odd number
                break
            else:            #if not...
                odds+=2         #check to see if it's divisible by the next odd number.

        if odds>sqrtk:             #if k isn't divisible by any r
                primes+=1        #we have a prime number
                x=chk         #record that number
                chk+=2        #check the next odd number to see if it's prime
                odds=3         #and reset r back to 3 to use it next time

if primes==goal:                  #once we've reached the desired amount of prime numbers
    print("The",goal,"prime number is:",x)      #print that prime number

Style (and related tools)

As described in Vogel612's answer, many things are wrong concerning the style. You can use pep8 (or its online version) to check it automatically and autopep8 to try to fix this automatically. You'll find various other convenient tools to check your code such as pylint, pyflakes or pychecker.

Separation of concerns and code organisation

You could create a function to look for the prime number and then call it when you want to print the result.

You'd get something like :

def get_nth_prime(goal):
    if goal > 1000000:
        return None
    if goal == 1:  # first prime is 2
        return 2
    if goal == 2:  # second prime is 3
        return 3

    chk = 5  # the next odd number after 3, which is already taken care of above
    primes = 2  # the number of primes recorded, after counting 2 and 3.
    while primes < goal:
        if chk < 9:  # if the number is odd and below 9....
            primes += 1  # it's prime
            chk += 2  # and check the next odd number

        else:  # if it's =>9......
            odds = 3  # check to see if it's divisible by the smallest odd number
            sqrtk = chk ** .5  # all the way up to the square root of chk

            while odds < sqrtk or odds == sqrtk:  # if at any time...
                if chk % odds == 0:  # ...chk is divisible by this odd number....
                    chk += 2  # then raise the value of chk to the next odd number
                    break  # and start the check from the beginning with next k
                else:  # if not...
                    # check to see if it's divisible by the next odd number.
                    odds += 2

            # if chk isn't divisible by any odds up to the square root of chk
            if odds > sqrtk:
                primes += 1  # we have a prime number
                chk += 2  # check the next odd number to see if it's prime
                odds = 3  # and reset odds back to 3 to use it next time

    if primes == goal:  # once we've reached the desired amount of prime numbers
        return chk - 2

goal = int(input("Enter a number n, 1-1,000,000 to find the nth prime number."))
print("The", goal, "prime number is:", get_nth_prime(goal))

Also, the usage is to write your code "that actually does stuff" behind an if main guard so that you can import your code if you want to re-use your function without having any polluting. In your case, it becomes :

def main():
    """Main function"""
    goal = int(input("Enter a number n, 1-1,000,000 to find the nth prime number."))
    print("The", goal, "prime number is:", get_nth_prime(goal))

if __name__ == "__main__":
    main()

Testing

Having extracted a function to get the n-th prime number, you can more easily write test. You can play it simple, manually :

assert get_nth_prime(1) == 2
assert get_nth_prime(2) == 3

Or you can go further :

assert [get_nth_prime(i) for i in range(1, 100)] == [2, 3, 5, 7, 11, 13, 17, 19, 23, 29, 31, 37, 41, 43, 47, 53, 59, 61, 67, 71, 73, 79, 83, 89, 97, 101, 103, 107, 109, 113, 127, 131, 137, 139, 149, 151, 157, 163, 167, 173, 179, 181, 191, 193, 197, 199, 211, 223, 227, 229, 233, 239, 241, 251, 257, 263, 269, 271, 277, 281, 283, 293, 307, 311, 313, 317, 331, 337, 347, 349, 353, 359, 367, 373, 379, 383, 389, 397, 401, 409, 419, 421, 431, 433, 439, 443, 449, 457, 461, 463, 467, 479, 487, 491, 499, 503, 509, 521, 523]

Now that we have test cases, rewriting the code is much easier.

A bit of logic

You can rewrite while odds < sqrtk or odds == sqrtk: : while odds <= sqrtk:

Then you can rewrite this using range : while odds < sqrtk or odds == sqrtk: and then the second test can be written using the infamous else on the for loop (the best way to understand it is to read it as nobreak). The code becomes :

        for odds in range(3, int(chk ** .5) + 1, 2):
            if chk % odds == 0:  # ...chk is divisible by this odd number....
                chk += 2  # then raise the value of chk to the next odd number
                break  # and start the check from the beginning with next k
        else: # no break => no divisors => prime
            primes += 1  # we have a prime number
            chk += 2  # check the next odd number to see if it's prime

The check if  chk<9 looks like a premature optimisation to me. We can get rid of it.

Then you have :

while primes < goal:
    for odds in range(3, int(chk ** .5) + 1, 2):
        if chk % odds == 0:  # ...chk is divisible by this odd number....
            chk += 2  # then raise the value of chk to the next odd number
            break  # and start the check from the beginning with next k
    else: # no break => no divisors => prime
        primes += 1  # we have a prime number
        chk += 2  # check the next odd number to see if it's prime

Then you have chk += 2 that appear in two different places : let's move it to a single place :

while primes < goal:
    for odds in range(3, int(chk ** .5) + 1, 2):
        if chk % odds == 0:  # ...chk is divisible by this odd number....
            break
    else: # no break => no divisors => prime
        primes += 1
    chk += 2

if primes == goal:  # once we've reached the desired amount of prime numbers
    return chk - 2

There is no need to compare primes and goal at every single iteration as it won't be updated that often : you can move this to the place where primes is increased :

chk = 5  # the next odd number after 3, which is already taken care of above
primes = 2  # the number of primes recorded, after counting 2 and 3.
while True:
    for odds in range(3, int(chk ** .5) + 1, 2):
        if chk % odds == 0:  # ...chk is divisible by this odd number....
            break
    else: # no break => no divisors => prime
        primes += 1
        if primes == goal:
            return chk
    chk += 2

More functions

The test of primality can easily be written in a function on its own.

Luckily, I have this ready in my toolbox :

def is_prime(n):
    """Checks if a number is prime."""
    if n < 2:
        return False
    return all(n % i for i in range(2, int(math.sqrt(n)) + 1))

def get_nth_prime(goal):
    if goal > 1000000:
        return None
    if goal == 1:  # first prime is 2
        return 2
    if goal == 2:  # second prime is 3
        return 3

    chk = 5  # the next odd number after 3, which is already taken care of above
    primes = 2  # the number of primes recorded, after counting 2 and 3.
    while True:
        if is_prime(chk):
            primes += 1
            if primes == goal:
                return chk
        chk += 2

This is getting into more advanced Python stuff so you might want to read a bit more about this.

More idiomatic

You are trying to generate prime numbers as you count them. A more pythonic way to do so would be to define a function to generate prime numbers one by one and another one to stop when we've reached what we want to reach. This could look like :

import math
import itertools

def is_prime(n):
    """Checks if a number is prime."""
    if n < 2:
        return False
    return all(n % i for i in range(2, int(math.sqrt(n)) + 1))

def yield_primes():
    """Yields prime number by checking them individually"""
    yield 2
    yield 3
    for i in itertools.count(5, 2):
        if is_prime(i):
            yield i

def nth(iterable, n, default=None):
    """Returns the nth item or a default value.
    From http://stackoverflow.com/questions/12007820/better-ways-to-get-nth-element-from-an-unsubscriptable-iterable ."""
    return next(itertools.islice(iterable, n, None), default)

def get_nth_prime(goal):
    if goal > 1000000:
        return None
    return nth(yield_primes(), goal - 1)

I just noticed I've re-used my non efficient is_prime function going through odd and even integers. I'll leave an an exercise to the reader to fun of going only through odd numbers :-)


Building and querying an immutable phone book I wanted to get your opinion on this immutable implementation of a Phone Book using recursion to process a stream. I am especially interested in ways to speed up the code execution.

Here is an example of expected input:

8
add 811 Mom1
add 12376213 Mom2
add 1732239 Bob
find 76213
find 910
find 811
del 811
find 811

Now, here is the code

import scala.collection.immutable.HashMap

object phoneBook extends App {
override def main(args: Array[String]): Unit = {
  import scala.io.Source.fromInputStream
  import scala.annotation.tailrec

  val verbose = false

 lazy val rawLines = fromInputStream(System.in).getLines.toStream

val nQueries = (rawLines.head).head.toInt
if(verbose) println(nQueries)

   def parseLine(s:String):(Int,Array[String]) = {
      val a = s.split(" ")
     if(verbose) println(a.mkString(","))
      a(0) match {
        case "add" => {if(verbose) println(0); (0, a)}
        case "find" =>{if(verbose) println(1); (1, a)}
        case "del" => {if(verbose) println(2); (2, a)}
      }
   }
lazy val lines = (rawLines drop 1).map(parseLine)
  @tailrec  def doAction(stream:Stream[(Int,Array[String])],acc:HashMap[Int,String]=HashMap.empty[Int,String]):Unit = {

  if(stream.isEmpty) Unit
  else{
    val nextAction = stream.head
    val actionDetail = nextAction._2

    nextAction._1 match {
      case 0 => doAction(stream.tail,acc.+(actionDetail(1).toInt -> actionDetail(2)) )
      case 1 => {
        println(acc.getOrElse(actionDetail(1).toInt,f"not found"))
        doAction(stream.tail,acc)
         }
      case 2 =>     doAction(stream.tail,acc.-(actionDetail(1).toInt ) )
    }
  }

}

doAction(lines)

  }
}

Your indentation needs cleaning up.

The compiler says:

$ scalac -deprecation phoneBook.scala
phoneBook.scala:5: warning: overriding method main in trait App is deprecated: main should not be overridden

Following the example in the documentation, inside the App, you should just put your code in the object itself, without any main function.

Translating the three verbs into a numeric code is unnecessary obfuscation.  Trying to handle all three kinds of commands in the same doAction function is also cumbersome.

Converting the Iterator into a Stream is counterproductive, when you intend to process each line just once.  It just makes you have to (rawLines drop 1) explicitly.

You're starting with an empty phone book (cryptically named acc).  Then you want to have each command transform it and pass it along to the next command.  Instead of explicit recursion in the doAction function, you would be better off using a fold.

Instead of instantiating scala.collection.immutable.HashMap, you can just use a Map and let Scala do the right thing.

import scala.io.Source.fromInputStream

object phoneBook extends App {

  def add(phoneBook: Map[Int,String], number: Int, name: String) = {
    phoneBook + (number -> name)
  }

  def del(phoneBook: Map[Int,String], number: Int) = {
    phoneBook - (number)
  }

  def find(phoneBook: Map[Int,String], number: Int) = {
    println(phoneBook.getOrElse(number, "not found"))
    phoneBook
  }

  lazy val lines = fromInputStream(System.in).getLines
  val nQueries = lines.next.toInt

  lines.take(nQueries).foldLeft(Map.empty[Int,String])((phoneBook, line) => {
    line.split(" ") match {
      case Array("add", number, name) => add(phoneBook, number.toInt, name)
      case Array("find", number)      => find(phoneBook, number.toInt)
      case Array("del", number)       => del(phoneBook, number.toInt)
    }
  })

}

Actually, there is no need to convert the phone numbers to integers, unless you want to ensure that they are indeed numeric.  In fact, in general, phone numbers should not be treated as integers, since they may contain many digits, or may have significant leading zeroes.


Usertool to Edit Forum Members from Desktop I've created a Forum website tool for admins to add, edit, and delete users from their desktop, saving to a file, and uploading the file to the database, the program is not completed I'm posting it here for review thus far

(People I'm creating it for would prefer me not having access to database, I understand I would be using activerecord model/structure if I where saving directly to the database.)

What the program does is simple:

It prompts the user for which he/she would like to do via add, edit,
delete users.
The program then runs through a case statement of available options
and if the option isn't found it exits the program
In the add_user method the program saves the input of the user into a YAML file, and prompts the user for more users, if none follow exits the program.
During the delete_user method, the program does the same as adding except deletes the users from the YAML file there is a bug where the program deletes the entire YAML file, not just the edited users
In the edit_user the program will edit various elements of the hash inside the YAML file and save over the old element of the file. Not ready yet will post follow up later

  The edit user section hasn't been setup yet, still working out the kinks in delete_user method.

So what I'm looking for is:

A possible explanation of why the delete_user method wipes the
entire file and not just the user entered, is this because I'm deleting
the entire section and not just the users input?
Would using a JSON file be better then using a YAML file, I'm aware
that A LOT of websites don't support YAML's so would it be a good idea to convert the require 'yaml' to a require 'json' and regroup from there?
Any possible areas of improvement? Meaning do you notice anything
right off the bat that should be changed, can my syntax be updated or more developed?

Source Code:

#!/usr/bin/env ruby

require 'yaml'
require 'open-uri'

def menu
    print "Welcome to User Menu. Would you like to [A]dd users, [D]elete users, [E]dit users, [V]iew all users, or [Q]uit the system?\n"
    input = gets.chomp!
    case input
    when /a/i
        add_user
    when /d/i
        delete_user
    when /e/i
        edit_user
    when /v/i
        view_all
    else
        puts "Exiting system.."
        exit
    end
end

def add_user
    puts "Please enter Username:"
    username = gets.chomp
    data = {username: username}
    puts "Please enter user Email:"
    email = gets.chomp
    data[:email_address] = email
    puts "Please enter users status(member, vip, gold, etc..):"
    status = gets.chomp!
    data[:member_status] = status
    File.open('users.yml', 'a') { |s| s.write(data.to_yaml) }
    add_user if restart
end

def delete_user
    puts "Enter Username:"
    username = gets.chomp
    delete_data = {username: username}
    puts "Enter Email:"
    email = gets.chomp
    delete_data[:email_address] = email
    puts "Enter Status:"
    status = gets.chomp
    delete_data[:status] = status
    hash = YAML.load_file('users.yml')
    hash.delete("#{username}")
    hash.delete("#{email}")
    hash.delete("#{status}")
    File.open('users.yml', 'w') { |f| YAML.dump(delete_data.to_yaml) }
    delete_user if restart
end

def edit_user
    edit_user if restart
end

def view_all
    content = URI('#Website URL here taken out for security of site').read
end

def restart
    puts "Would you like to edit another user?"
    input = gets.chomp
    if input =~ /yes/i
        return true
    else
        puts "Exiting program.."
        exit
    end
end
menu

Overview of YAML file:

---
:username: TEST
:email_address: TEST
:member_status: TEST

prompt function

So many times you do:

puts "Please enter Username:" # Or something else instead of Username.
username = gets.chomp

In short you print and then read input, what about a function:

def prompt(message)
  puts message
  gets.chomp
end

You can reuse it and save a lot of repetition.

Functional Programming -- No mutation

Ruby leans towards the Function Programming paradigm, so you should stick to it.

One thing is avoiding mutation:

data = {username: username}
...
data[:email_address] = email
...
data[:member_status] = status

You modify data piece by piece, instead do:

...
...
data = {username: username, email_address: email, member_status: status}

It also looks more organized to me.

In fact you may write:

data = { username: prompt("The username? "),
         email_address: prompt("The email address? ")
         member_status: prompt("The member status? ") }

And make the code shorter and more readable.


Image transfer script How can I optimize this?

<cfftp connection = "myConnection"
   username = "#username#"
   password = "#password#"
   server = "#server#"

   action = "open"
   timeout = "500"
   stopOnError = "Yes">

<cfloop index="i" from="1" to="#ArrayLen(ImagesArray)#">

    <cfset slocal = "temp\" & ImagesArray[i] >
    <cfset sremoteFile = "Images\" & ImagesArray[i]>

    <cftry>
        <cfftp
            connection = "myConnection"
            action = "getFile"
            name = "uploadFile"
            transferMode = "binary"
            failIfExists = "false"
            localFile = "#slocal#"
            remoteFile = "#sremoteFile#" />
    <cfcatch>
        <cfabort>
    </cfcatch>
    </cftry>

    <cfset files = files & "|" & slocalFile>
    <cfset fileliste = fileliste & arFiles[i] & "|">
</cfloop>

<cfftp connection = "myConnection"
    action = "close"
    stopOnError = "Yes">

In my experience, FTP can use up a lot of network packets and CPU time just negotiating the starting and finishing of a single file.  If there exists the option of having an archive uncompressed at the destination, I would suggest zipping the images into a single file for transport.

To get even more performance for the end user making the CF request you could perform the FTP transfer in an asynchronous thread via cfthread.  Launch the thread that zips and sends the file and let the user know that the data is on its way.

Hope this helps, or at least inspires some new ideas.


Rename subkeys of nested dictionary dynamically My nested (ordered) dictionary looks like this:

OrderedDict([
    ('1', OrderedDict([('1994_fte_count', '4'), ('1995_fte_count', '2'), ('1996_fte_count', '5'), ('1997_fte_count', '7')])),
    ('2', OrderedDict([('1994_fte_count', '5'), ('1995_fte_count', '22'), ('1996_fte_count', '6'), ('1997_fte_count', '3')])),
    ('3', OrderedDict([('1994_fte_count', '6'), ('1995_fte_count', '7'), ('1996_fte_count', '7'), ('1997_fte_count', '81')]))
    ])

The subkeys ("1994_fte_count", "1995_fte_count", etc.) all range up until 2000. I want to replace the year with the difference to a given year. This shall happen dynamically for a lot of years. For example, I start with 1995, and "1995_fte_count" shall be renamed "lag0_fte_count", while "1994_fte_count" shall be renamed "lag1_fte_count". I read the file in a several times, each time with a different benchmark year.

I currently use the following nested loop:

year = "1996" # comes as string
lag_length = 2
year_range = range(int(year)-lag_length, int(year))
replacement = ["lag" + str(i) for i in reversed(range(0,lag_length))]
for ID in my_dict.keys():
    for subkey in my_dict[ID].keys():
        for i in range(0, len(year_range)):
            acutal_year = str(year_range[i])
            if subkey.startswith(acutal_year):
                newkey = subkey.replace(acutal_year, replacement[i])
                my_dict[ID][newkey] = my_dict[ID].pop(subkey)

Given the nature of the task (simple text replace), I do think that there must be a shorter, more readable and faster way than to iterate over all the subkeys multiple times. Who knows how to?

Clarity probably trumps speed, but then again, it's not quite clear
why you need this format and what is further going to happen to that
dictionary.

You can definitely get rid of the innermost loop by computing the
replacement index on the fly instead of testing all possibilities:

from collections import OrderedDict

year = int("1996") # comes as string
lag_length = 2
replacement = ["lag%s_fte_count" % (i - 1) for i in range(lag_length, 0, -1)]

for ID, value in my_dict.iteritems():
    for subkey in value.keys():
        year_string = subkey[:4]

        if not year_string.isdigit():
            continue

        parsed_year = int(year_string)

        if not year - lag_length <= parsed_year < year:
            continue

        value[replacement[lag_length + parsed_year - year]] = value.pop(subkey)

I also used iteritems to get both values at the same time; for
value.keys() that's sadly not possible while iterating.  The year
input should also be parsed immediately as it'll always be used in the
integer form.

Next I'm assuming the keys start with the four-character year, so then
the year can be parsed and compared with the target range and if it's in
the range, the key is renamed by calculating the correct index.

Furthermore you could remove the isdigit call if the format allows for
it and replacement could be removed by always generating the string
immediately, e.g. like this:

value["lag%s_fte_count" % (year - parsed_year - 1)] = value.pop(subkey)


Make my draw graph looks better I have simple code, that draw graph of my histogram. But code looks ugly, do you have idea, how can I make it looks better ?

            for (int i = 0; i < hueCount - 2; ++i)
            {
                if( i % 6 == 0)
                g.DrawLine(borderPen,
                   margin+ (width - 2 * margin) * ((float)i / (float)hueCount),
                   height - margin,
                   margin+ (width - 2 * margin) * ((float)i / (float)hueCount),
                   height - margin - 5);

                g.DrawLine(redPen,
                    margin + ((float)i / (float)hueCount) * (width - 2 * margin),
                    (-(float)reds[i] / (float)maxRed) * (height - 2 * margin) - margin + height,
                    margin + ((float)(i + 1) / (float)hueCount) * (width - 2 * margin),
                    (-(float)reds[i + 1] / (float)maxRed) * (height - 2 * margin) - margin + height);

                g.DrawLine(greenPen,
                    margin + ((float)i / (float)hueCount) * (width - 2 * margin),
                    (-(float)greens[i] / (float)maxGreen) * (height - 2 * margin) - margin + height,
                    margin + ((float)(i + 1) / (float)hueCount) * (width - 2 * margin),
                    (-(float)greens[i + 1] / (float)maxGreen) * (height - 2 * margin) - margin + height);

                g.DrawLine(bluePen,
                    margin + ((float)i / (float)hueCount) * (width - 2 * margin),
                    (-(float)blues[i] / (float)maxBlue) * (height - 2 * margin) - margin + height,
                    margin + ((float)(i + 1) / (float)hueCount) * (width - 2 * margin),
                    (-(float)blues[i + 1] / (float)maxBlue) * (height - 2 * margin) - margin + height);
            }

Here's what I came up with using a helper class:

            Draw draw = new Draw(g, borderPen, hueCount, margin, width, height);

            for (int i = 0; i < hueCount - 2; ++i)
            {
                draw.DrawBorder(i);
                draw.DrawLine(redPen, i, reds, maxRed);
                draw.DrawLine(greenPen, i, greens, maxGreen);
                draw.DrawLine(bluePen, i, blues, maxBlue);
            }

...

internal sealed class Draw
{
    private readonly Graphics g;

    private readonly Pen borderPen;

    private readonly int hueCount;

    private readonly int margin;

    private readonly int doubleMargin;

    private readonly int width;

    private readonly int height;

    internal Draw(Graphics g, Pen borderPen, int hueCount, int margin, int width, int height)
    {
        this.g = g;
        this.borderPen = borderPen;
        this.hueCount = hueCount;
        this.margin = margin;
        this.doubleMargin = 2 * this.margin;
        this.width = width;
        this.height = height;
    }

    internal void DrawBorder(int i)
    {
        if (i % 6 == 0)
        {
            float x = margin + (width - doubleMargin) * (float)i / hueCount;
            float y = height - margin;

            g.DrawLine(borderPen, x, y, x, y - 5);
        }
    }

    internal void DrawLine(Pen pen, int i, int[] colors, int maxColor)
    {
        float wm = width - doubleMargin;
        float hm = height - doubleMargin;
        float mh = margin - height;

        g.DrawLine(
            pen,
            margin + ((float)i / hueCount) * wm,
            (-(float)colors[i] / maxColor) * hm - mh,
            margin + ((float)(i + 1) / hueCount) * wm,
            (-(float)colors[i + 1] / maxColor) * hm - mh);
    }
}


Networking in iOS Swift This piece of code is working - It gives me a lot of JSON back, which I have yet to parse, but I'd like to show my code here first to get tips on how to improve what I already have:

VenueService.swift

    class VenueService {

    private let __kVenueServiceClientID = "CLIENT_ID"
    private let __kVenueServiceClientSecret = "CLIENT_SECRET"

    func performVenueLocationRequest(location: CLLocationCoordinate2D, identifier: NSString, completion: (venues: NSDictionary?, error: NSError?) -> ()) {
        var error: NSError?
        var response: NSURLResponse?

        let queue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0)

        dispatch_async(queue, {
            if let data = NSURLConnection.sendSynchronousRequest(self.buildRequestForVenueLocation(location, identifier), returningResponse: &response, error: &error) {
                if(error == nil) {
                    let responseDictionary: AnyObject? = NSJSONSerialization.JSONObjectWithData(data, options: nil, error:&error)
                    if let responseDictionary = responseDictionary as? NSDictionary {
                        completion(venues: responseDictionary, error: nil)
                    }
                } else {
                    completion(venues: nil, error: error)
                }
            } else {
                println("There was a problem with the request...")
            }
        })

    }

    func todaysDate() -> NSString {
        let date = NSDate()
        let formatter = NSDateFormatter()
        formatter.dateFormat = "yyyMMdd"
        return formatter.stringFromDate(date)
    }

    func buildRequestForVenueLocation(location: CLLocationCoordinate2D, _ identifier: NSString) -> NSURLRequest {
            return NSURLRequest(URL: NSURL(string: "https://api.foursquare.com/v2/venues/search?ll=\(location.latitude),\(location.longitude)&radius=500&limit=50&categoryId=\(identifier)&client_id=\(__kVenueServiceClientID)&client_secret=\(__kVenueServiceClientSecret)&v=\(self.todaysDate())")!)
    }
}

I use it like this in my ViewController:

struct Location {
    let location: CLLocationCoordinate2D

    init() {
        self.location = CLLocationCoordinate2D(latitude: 37.332331, longitude: -122.031219)
    }

}

class ViewController: UIViewController {

    let v = VenueService()
    let l = Location()

    override func viewDidLoad() {
        super.viewDidLoad()
    }

    @IBAction func testJSON(sender: AnyObject) {
        v.performVenueLocationRequest(l.location, identifier: "4d4b7105d754a06374d81259", completion: { dict, err in
            dispatch_async(dispatch_get_main_queue(), {
                if let err = err {
                    println("if let \(err)")
                } else {
                    if let dict = dict {
                        println("else: \(dict)")
                    }
                }
            })
        })
    }
}

One thing I'm really confused about:

Say I have an optional. I could use the if let syntax to unwrap it, but is it the same to just say if optinalVariable == nil {} else optinalVariable!?

In playground I get the same results.

To use:

if optionalVariable == nil {} else {
    // do stuff with optionalVariable!
}

would absolutely not be recommended.

For starters,

if optionalVariable != nil {
    // do stuff with optionalVariable!
}

would be slightly better, but still, I can't even remotely recommend this.

Given that optionalVariable is nullable, what happens if it is made null inside this closure?

if optionalVariable != nil {
    doStuff(optionalVariable!)
    optionalVariable = funcThatCouldReturnNil()
    doStuff(optionalVariable!)
}

At the end of the day, we should simply minimize our use of forced unwraps.

if let variable = optionalVariable {
    doStuff(variable)
    variable = funcThatCouldReturnNil() // <-- compile time error
}

If variable is not a nullable, then you can't assign the result of a function that could return nil to it without force unwrapping the result.

The exclamation point should mark to us that something special is happening here... and if we can use the exclamation point, we might need to rethink our method.

At the end of the day, we shouldn't be asking ourselves purely whether or not our results remain the same for both methods.  We should also ask ourselves whether one way or the other opens the path for future maintainers to do something stupid and write some bugs into your code.

If I'm assigning the result of a non-optional returning function to a non-optional variable, if it is later determined that the function needs to return an optional, then Swift and Xcode will politely alert me to every occurrence that I need to go look at for my variable.

And if I'm not using the if let syntax and instead using the ! force unwrap everywhere, then Xcode can't remind me, and I'm asking myself for a huge swampy headache of bugs...


Collecting data from General Ledger accounts lines I have a pair of General Ledger accounts to scan for data. I need to find a keyword or phrase from the descriptions and then collect certain bits of data from those lines. The source tables (there are two) are on separate tabs and the data from that needs to populate 5 other tables of data for analysis.

This is the code I have so far but it is exceedingly slow. I would like suggestions to speed up the process. My thought is to load all the data into an array but I am not sure if that would be faster.

Sub Table9052()
Dim i As Integer
Dim wsA As Worksheet: Set wsA = ThisWorkbook.Sheets("QA Data A")
Dim wsK As Worksheet: Set wsK = ThisWorkbook.Sheets("QA Data K")
Dim wsT As Worksheet: Set wsT = ThisWorkbook.Sheets("Tables")
Dim Locker As String
Dim GC As String
Locker = "9052 Electronic Lockers"
GC = "9042 Dunkin Donuts Gift Card"
MBD = "MERCHANT BANKC"
DDSV = "DD STORED VALU"
SS = "STARBUCKS STOR"
ColARow = 3
ColFRow = 3
ColKRow = 3
ColPRow = 3
ColURow = 3

Application.Calculation = xlCalculationManual
Application.ScreenUpdating = False
For i = 6 To wsA.Range("C65536").End(xlUp).Row
    Eval = wsA.Cells(i, 4)
    If Eval = Locker Then 'If a locker line copy the cells to ELTable
        wsA.Cells(i, 3).Copy wsT.Cells(ColARow, 1) 'Copy Date to column A
        wsA.Cells(i, 4).Copy wsT.Cells(ColARow, 2) 'Copy Desc to column B
        wsA.Cells(i, 5).Copy wsT.Cells(ColARow, 3) 'Copy Ammt to column C
        wsA.Cells(i, 7).Copy wsT.Cells(ColARow, 4) 'Copy Lodge to column D
        ColARow = ColARow + 1
    End If
    If Eval = GC Then 'If a locker line copy the cells to DDCardTable
        wsA.Cells(i, 3).Copy wsT.Cells(ColPRow, 16) 'Copy Date to column A
        wsA.Cells(i, 4).Copy wsT.Cells(ColPRow, 17) 'Copy Desc to column B
        wsA.Cells(i, 5).Copy wsT.Cells(ColPRow, 18) 'Copy Ammt to column C
        wsA.Cells(i, 7).Copy wsT.Cells(ColPRow, 19) 'Copy Lodge to column D
        ColPRow = ColPRow + 1
    End If
Next i
For i = 6 To wsK.Range("C65536").End(xlUp).Row
    Eval = Left(wsK.Cells(i, 4), 14)
    If Eval = MBD Then 'If a locker line copy the cells to ELTable
        wsK.Cells(i, 3).Copy wsT.Cells(ColFRow, 6) 'Copy Date to column F
        wsK.Cells(i, 4).Copy wsT.Cells(ColFRow, 7) 'Copy Desc to column G
        wsK.Cells(i, 5).Copy wsT.Cells(ColFRow, 8) 'Copy Ammt to column H
        wsK.Cells(i, 7).Copy wsT.Cells(ColFRow, 9) 'Copy Lodge to column I
        ColFRow = ColFRow + 1
    End If
    If Eval = DDSV Then 'If a locker line copy the cells to DDCardTable
        wsK.Cells(i, 3).Copy wsT.Cells(ColKRow, 11) 'Copy Date to column K
        wsK.Cells(i, 4).Copy wsT.Cells(ColKRow, 12) 'Copy Desc to column L
        wsK.Cells(i, 5).Copy wsT.Cells(ColKRow, 13) 'Copy Ammt to column M
        wsK.Cells(i, 7).Copy wsT.Cells(ColKRow, 14) 'Copy Lodge to column N
        ColKRow = ColKRow + 1
    End If
    If Eval = SS Then 'If a locker line copy the cells to DDCardTable
        wsK.Cells(i, 3).Copy wsT.Cells(ColURow, 21) 'Copy Date to column U
        wsK.Cells(i, 4).Copy wsT.Cells(ColURow, 22) 'Copy Desc to column V
        wsK.Cells(i, 5).Copy wsT.Cells(ColURow, 23) 'Copy Ammt to column W
        wsK.Cells(i, 7).Copy wsT.Cells(ColURow, 24) 'Copy Lodge to column X
        ColURow = ColURow + 1
    End If
Next i
Application.Calculation = xlCalculationAutomatic
Application.ScreenUpdating = True
End Sub

Here's a suggestion for simplifying your code by refactoring a bit and copying out the common part to a separate sub.  Also sets values directly in place of copy/paste.

This could be refactored completely to (eg) read the values to be matched from a table on a worksheet, along with the destination for each matched row.

Sub Table9052()

    'use constants for fixed values
    Const Locker As String = "9052 Electronic Lockers"
    Const GC As String = "9042 Dunkin Donuts Gift Card"
    Const MBD As String = "MERCHANT BANKC*"
    Const DDSV As String = "DD STORED VALU*"
    Const SS As String = "STARBUCKS STOR*"

    Dim i As Long '<< use long in place on Integer
    Dim ColA As Range, ColF  As Range, ColK  As Range, ColP As Range, ColU  As Range

    Dim wsA As Worksheet: Set wsA = ThisWorkbook.Sheets("QA Data A")
    Dim wsK As Worksheet: Set wsK = ThisWorkbook.Sheets("QA Data K")
    Dim wsT As Worksheet: Set wsT = ThisWorkbook.Sheets("Tables")

    Dim Eval, rw As Range

    'set the starting points for each of the destination tables
    With wsT
        Set ColA = .Cells(3, "A")
        Set ColF = .Cells(3, "F")
        Set ColK = .Cells(3, "K")
        Set ColP = .Cells(3, "P")
        Set ColU = .Cells(3, "U")
    End With

    Application.Calculation = xlCalculationManual
    Application.ScreenUpdating = False

    For i = 6 To wsA.Range("C65536").End(xlUp).Row
        Set rw = wsA.Rows(i)
        Select Case rw.Cells(4).Value
            Case Locker: CopyRow rw, ColA
            Case GC: CopyRow rw, ColP
        End Select
    Next i

    For i = 6 To wsK.Range("C65536").End(xlUp).Row
        Set rw = wsK.Rows(i)
        Eval = rw.Cells(4)

        If Eval Like MBD & "*" Then CopyRow rw, ColF
        If Eval Like DDSV & "*" Then CopyRow rw, ColK
        If Eval Like SS & "*" Then CopyRow rw, ColU

    Next i
    Application.Calculation = xlCalculationAutomatic
    Application.ScreenUpdating = True
End Sub

'extract out the common code to a separate sub
'   copy values and advance to next row
Sub CopyRow(rw As Range, cDest As Range)
    cDest.Resize(1, 4).Value = _
            Array(rw.Cells(3).Value, rw.Cells(4).Value, _
            rw.Cells(5).Value, rw.Cells(7).Value)
    Set cDest = cDest.Offset(1, 0) 'next row...
End Sub


Basic queue implementation in C I'm practicing data structures in C and I implemented a queue based on its description:

#include <stdio.h>
#include <stdlib.h>

struct QueueNode {
    int data;
    struct QueueNode* nextNode;
};

struct Queue{
    struct QueueNode* priorNode;
    struct QueueNode* lastNode;
};

struct Queue* initQueue(){
    struct Queue* queue = malloc(sizeof(struct Queue)); //only one is needed
    if(!queue){
        fprintf(stderr,"memory error");
        exit(1);
    }
    queue->priorNode = NULL;
    queue->lastNode = NULL;
    return queue;
}

void enqueue(struct Queue* queue, int num){
    struct QueueNode* queuenode = malloc(sizeof(struct QueueNode));
    if(!queuenode){
        fprintf(stderr,"memory error");
        exit(1);
    }
    queuenode->data = num;
    queuenode->nextNode = NULL;

    if(!queue->priorNode){
        queue->priorNode = queuenode;
    }

    if(queue->lastNode){
        struct QueueNode* temp = queue->lastNode;
        queue->lastNode = queuenode;
        temp->nextNode = queue->lastNode;
    } else {
        queue->lastNode = queuenode;
    }
}

struct QueueNode dequeue(struct Queue* queue){
    if(!queue->priorNode){
        fprintf(stderr,"Queue is empty.\n");
        struct QueueNode nullnode = {0, NULL};
        return nullnode;
    }
    struct QueueNode prior = *queue->priorNode;
    free(queue->priorNode);
    if(prior.nextNode){
        queue->priorNode = prior.nextNode;
    } else {
        queue->priorNode = NULL;
    }
    return prior;
}

void peekQueue(struct Queue* queue){
    printf("\"%d\" is the current prior element in the queue\n", queue->priorNode->data);
}

void listQueue(struct Queue* queue){
    struct QueueNode* qn = queue->priorNode;

    printf("elements in the queue from left(the most prior) to right(the least prior) are:\n");
    while(qn){
        if(!qn->nextNode){
            printf("%d",qn->data);
            break;
        }
        else{
            printf("%d - ",qn->data);
            qn = qn->nextNode;
        }
    }
    printf("\n");
}

void closeQueue(struct Queue* queue){

    struct QueueNode* next = NULL;
    while(queue->priorNode){
        next = queue->priorNode->nextNode;
        free(queue->priorNode);
        if(!next) break;
        queue->priorNode = next;
    }

    free(queue);
}

int main(){
    struct Queue* queue = initQueue();

    enqueue(queue,15);
    enqueue(queue,4);
    enqueue(queue,8);
    enqueue(queue,13);
    enqueue(queue,7);

    listQueue(queue);

    peekQueue(queue);

    struct QueueNode node;

    node = dequeue(queue);
    printf("\"%d\" came out of the queue\n",node.data);
    node = dequeue(queue);
    printf("\"%d\" came out of the queue\n",node.data);
    node = dequeue(queue);
    printf("\"%d\" came out of the queue\n",node.data);
    node = dequeue(queue);
    printf("\"%d\" came out of the queue\n",node.data);
    node = dequeue(queue);
    printf("\"%d\" came out of the queue\n",node.data);
    node = dequeue(queue);
    printf("\"%d\" came out of the queue\n",node.data);

    closeQueue(queue);
return 0;
}

I especially didn't know what to do in my dequeue function when the queue was empty. I don't want the 0 to be returned as well because that would of course be confusing to the user.

I'm not able to review the functionality of the code currently, but to address your concerns:

In dequeue(), rather than returning the node, add a parameter (pointer to QueueNode) to store that node in. This is similar to functions like memcpy(3), strtoul(3) (end pointer), and so on. It allows you to return an error value however you wish.

int dequeue(struct Queue *queue, struct QueueNode *out);

If the queue is empty, don't bother modifying out, and just return an error code.

I would also avoid printing & exiting from within the queue's functions upon error. Instead return some kind of error value and leave it for the user to check.

For example, consider if malloc fails. The user may want to check errno, or use a fallback data structure.

int enqueue(struct Queue *queue, int num) {
    struct QueueNode *queuenode = malloc(sizeof(struct QueueNode));

    if(queuenode == NULL){
        return -1;
    }

    queuenode->data = num;
    queuenode->nextNode = NULL;

    if(!queue->priorNode){
        queue->priorNode = queuenode;
    }

    if(queue->lastNode){
        struct QueueNode* temp = queue->lastNode;
        queue->lastNode = queuenode;
        temp->nextNode = queue->lastNode;
    } else {
        queue->lastNode = queuenode;
    }

    return 0;
}


Shell install.sh for repository (rpi_videoloop) I've written my first installation script in shell and was wondering on how I can improve the install procedure to make it more clean?

Here's the code:

#!/bin/bash

#Variables
DEPENDENCIES="omxplayer screen cron usbmount ntfs-3g"
PTH="/home/pi/rpi_videoloop"
INIT="/etc/init.d"
SCRIPT="videoloop_v2.sh"
CONTROLLER="vid_controller"
USBCONF="usbmount.conf"
CRONSTUFF=$(cat cron.txt)

# Checking | Installing Dependancies
if ! apt list "$DEPENDENCIES" | grep -v installed | grep -E 'omxplayer|screen|cron|usbmount|ntfs-3g' > /dev/null
then
echo "Dependencies already installed. Continuing."
else
echo "Installing dependencies."
apt-get update
apt-get install "$DEPENDENCIES" -y
fi

echo "Installing Configurations..."

#Configuring
if [ ! -d "$PTH" ]; then
    mkdir $PTH
fi
cp $SCRIPT $PTH
if [ ! -f $INIT/$CONTROLLER ]; then
    sudo cp $CONTROLLER $INIT
fi
cp $USBCONF /etc/usbmount/
sudo chmod 755 $PTH/$SCRIPT
sudo chmod 755 $INIT/$CONTROLLER
sudo update-rc.d $CONTROLLER defaults
sudo echo -n " consoleblank=10" | sudo tee -a /boot/cmdline.txt
#cat alias.txt >> ~/.bashrc
#sudo cat alias.txt >> ~/.bashrc
(crontab -u pi -l; echo "$CRONSTUFF" ) | crontab -u pi -

echo "FINISHED INSTALLATION: Service control -> /etc/init.d/vid_controller {start|stop|check|repair}"

Set these options at the beginning:

set -eu

so that errors fail early

Don't use sudo in a script, because it can hang when not connected to a terminal.  Possible exception: once at the beginning:

if ! test $UID -eq 0
then
   test -t 0 && exec sudo "$0" "$@"
   # Not a terminal - or exec failed
   echo "Insufficient privileges - try sudo $0 $*" >&2
   exit 1
fi

You probably want to be using the standard install command instead of cp and mkdir.  That lets you specify the permissions as you create the targets.

Consider using a $DESTDIR prefix when specifying the targets.  This makes it easier for package builders (such as debhelper) to re-use your script.  They will need to omit the dependency checks, so make that separable if you can (or at least disable it when $DESTDIR is set).


Getting a list of all computer objects in each AD domain So I wrote this script and although it does work I can't help but think that it can be more efficient with all the repetitive code. Can anyone offer any suggestions on making it fewer lines and eliminating the repeat lines?

$Domains = (Get-ADForest).domains | %{Get-ADDomain $_ | Select DistinguishedName,ReplicaDirectoryServers}

#Gets list of all Computer objects in each domain
foreach ($Domain in $Domains){
Write-Output "Working on $($Domain.DistinguishedName)"
  #Iterates through Computer array
   foreach ($Comp in (Get-ADComputer -Server $($Domain.ReplicaDirectoryServers[0]) -SearchBase $($Domain.DistinguishedName) -filter {OperatingSystem -notlike "*Server*"} -Properties     Name,LastLogonDate,DistinguishedName| ?{ $_.DistinguishedName -notlike "*OU=Disabled*" -and $_.DistinguishedName -notlike "*OU=Servers*"}|Select     Name,LastLogonDate,DistinguishedName)){
      #Performs test on date, if date is greater then $Days moves to $OU.
      if(((New-TimeSpan -Start ($Comp.LastLogonDate) -End (Get-Date)).days) -gt $Days -and $Exclusion -notcontains $Comp.name){
         $Computers += $Comp
         #Finds what OU each object should go to.  Checks if object is already in dumpster OU
         if($($domain.DistinguishedName) -like "*DomainA*"){
         Write-Output ((Get-Date).ToString('MM-dd-yyyy:hh:mm:ss') + ": Moved $($comp.DistinguishedName) to $DomainAOUDump") | Out-File $log -Append
         Move-ADObject -Identity $comp.DistinguishedName -Server $($Domain.ReplicaDirectoryServers[0]) -TargetPath $DomainAOUDump
         }
         elseif($($domain.DistinguishedName) -like "*DomainB*"){
         Write-Output ((Get-Date).ToString('MM-dd-yyyy:hh:mm:ss') + ": Moved $($comp.DistinguishedName) to $DomainBOUDump") | Out-File $log -Append
         Move-ADObject -Identity $comp.DistinguishedName -Server $($Domain.ReplicaDirectoryServers[0]) -TargetPath $DomainBOUDump
         }
         elseif($($domain.DistinguishedName) -like "*DomainC*"){
         Write-Output ((Get-Date).ToString('MM-dd-yyyy:hh:mm:ss') + ": Moved $($comp.DistinguishedName) to $DomainCOUDump") | Out-File $log -Append
         Move-ADObject -Identity $comp.DistinguishedName -Server $($Domain.ReplicaDirectoryServers[0]) -TargetPath $DomainCOUDump
         }
         elseif($($domain.DistinguishedName) -like "*DomainD*"){
         Write-Output ((Get-Date).ToString('MM-dd-yyyy:hh:mm:ss') + ": Moved $($comp.DistinguishedName) to $DomainDOUDump") | Out-File $log -Append
         Move-ADObject -Identity $comp.DistinguishedName -Server $($Domain.ReplicaDirectoryServers[0]) -TargetPath $DomainDOUDump
         }
         else{
         Write-Output ((Get-Date).ToString('MM-dd-yyyy:hh:mm:ss') + ": I don't know where $($comp.name) should be moved") | Out-File $log -Append
         }
       }
    }
 }

One thing that I notice, you can change this repetitive fragment

if($($domain.DistinguishedName) -like "*DomainA*"){
    Write-Output ((Get-Date).ToString('MM-dd-yyyy:hh:mm:ss') + ": Moved $($comp.DistinguishedName) to $DomainAOUDump") | Out-File $log -Append
    Move-ADObject -Identity $comp.DistinguishedName -Server $($Domain.ReplicaDirectoryServers[0]) -TargetPath $DomainAOUDump
}

Into the following (minus some variable definitions, e.g. $TimeStamp):

$DomainPrefix = Get-DomainPrefix $Domain
$TargetPath = Get-Variable -Name "Domain$($DomainPrefix)OUDump"

Write-Output "$TimeStamp: Moved $DistinguishedName to $TargetPath" | Out-File $log -Append
Move-ADObject -Identity $DistinguishedName -Server $Server -TargetPath $TargetPath

with Get-DomainPrefix defined to capture the first character after "Domain", e.g.

Function Get-DomainPrefix([String] $DomainName){
    $DomainName -match "Domain(?<Prefix>.)" | Out-Null
    $Matches.Prefix
}

So, the script will become (minus definition for necessary functions to hide the complexity)

foreach ($Domain in Get-Domains) {
    foreach ($Comp in (Get-ComputerArray -Domain $Domain)) {
        if ( ShouldBeMoved -Computer $Comp ) {
            $DomainPrefix = Get-DomainPrefix $Domain
            $TargetPath = Get-Variable -Name "Domain$($DomainPrefix)OUDump"
            $TimeStamp = Get-TimeStamp
            $CompName = $Comp.DistinguishedName
            $Server = Get-Server

            Write-Output "$TimeStamp: Moved $CompName to $TargetPath" | Out-File $log -Append
            Move-ADObject -Identity $CompName -Server $Server -TargetPath $TargetPath
        }
    }
}

Note that the else clause is not handled in the simplified script above.

For further cleaning, you can redefine how you store the $TargetPath into a more friendly structure so that we don't need to use Get-Variable. Perhaps a map from DomainPrefix to the TargetPath.

Note: I'm not familiar with ADObject and stuff, so the variable naming might be a little off.


Bifurcating recursive calculation with redundant calculations def T(n):
  if n <= 0:
    return 1
  else:
    return T(n-1) + (n-1) * T(n-2)

print T(4)

I need an effective way to print out the output of the function T(n) generated using the recurrence relation above. Is there any way to make the above code more efficient?
Sample output:
10

One more option is to not recurse at all, but to instead compute the sequence until we get the desired term. This not only avoids the original algorithm's exponential complexity; it also avoids the at least linear (in terms of counting values) memory complexity of memoization. (Recursion/memoization can also run into implementation-specific issues such as recursion depth errors or memory errors, which are not a concern with the code below.)
def T(n):
    if n<=0: return 1
    # Set a, b to T(-1), T(0)
    a, b = 1, 1
    for i in range(n):
        # Change a, b from T(i-1), T(i) to T(i), T(i+1)
        a, b = b, b+i*a
    # i=n-1 set b to T(n)
    return b


Database threading design I am new to threading and I am a junior developer, so I guess there are many mistakes. My scenario is this:

look into the database
if there are data which should be sent

add the data to the queue

if queue is not empty

send the next message and remove it from the queue
sleep 10 seconds

if a message from another thread arrived

stop waiting and go to next message in queue

if no message from another thread arrived

go back to the first step, but do this only 2 times
if a message still hasn't arrived from another thread

pass to the next message until queue is empty

return to the first step

I am trying to do this like this:

private Thread ReceiveThread;
private Thread SendThread;
internal static Thread ServiceThread;

These 3 threads:

ReceiveThread = new Thread(ReceiveTask);
ReceiveThread.Start();
ServiceThread = new Thread(SerAutoThread.SendServiceMsg);
ServiceThread.Start();
SendThread = new Thread(SendTask);
SendThread.Start();

class SerAutoThread
{
    internal static object[] NextService;
    public static readonly object _locker = new object();
    internal static Queue<object[]> Services;
    internal static int sendingTime = 0;
    private static DatabaseFirebird DB;
    internal static void SendServiceMsg()
    {
        DB = new DatabaseFirebird();
        DB.Open(ConnectionStr);
        Services = new Queue<object[]>();
        while (true)
        {
            if (Services.Count != 0)
            {
                SetNextSerAndSend();
            }
            else
            {
                CheckAndSetServices();
            }
        }
    }

    private static void SetNextSerAndSend()
    {
        NextService = Services.Dequeue();
        for (int j = 0; j < 4; j++)
        {
            if (sendingTime == TRANSMITTED)
            {
               //pass to next msg
               sendingTime = 0;
               j = NEXTMSG;
            }
            else if (sendingTime < 3)
            {
                sendingTime++;
                Byte[] data = SetNextPckage();
                DeviceManager.MessageSendQueue.PostItem(new SendMessage("UDPCmd",
                NextService[(int)NextMsg.DeviceId].ToString(),
                data, data.Length));
                MyDebug.WriteLine("Sended...");
                lock (_locker)
                {
                    Monitor.Wait(_locker, TimeSpan.FromSeconds(10));
                }
            }
            else
            {
                // pass to next msg
                j = NEXTMSG;
            }
        }
    }
}

private void ReceiveTask()
{
    ReceiveMessage receiveMsg;
    while (true)
    {
        receiveMsg = Com.MessageReceiveQueue.GetItem(-1);
        SerAutoThread.sendingTime = SerAutoThread.TRANSMITTED;
        lock (SerAutoThread._locker)
        {
            Monitor.Pulse(SerAutoThread._locker);
        }
    }
}

 private void SendTask()
 {
     SendMessage msg;
     while (true)
     {
         msg = MessageSendQueue.GetItem(-1);
         String rtrn = PushData(msg);
     }
 }

Is this thread-safe or not?  I'm not sure if something is wrong with the design or I'm doing something wrong elsewhere.

And this is not my entire code (for understanding).  When I actually run the app, this works fine.  Yesterday just ones my Database management system (Firebird) locked.  Then I stopped my app, stopped Firebird server then started again and ran my app again.  Now it is ok again. I am not using threads with deep understanding. So I am worried about thread safety.

Is it possible for the thread-locked Firebird?

This looks pretty good to me.  I am not that good at threads myself, so I won't comment on the thread safety, but these are a few things I noticed.

This is fine as it is, but you might want to consider making this a ternary:

if (Services.Count != 0)
{
    SetNextSerAndSend();
}
else
{
    CheckAndSetServices();
}

Here it is in ternary form:

Services.Count == 0 ? CheckAndSetServices() : SetNextSerAndSend();

I'm not sure how this would affect the performance of your code, or whether it is important that the variable stay the same, but it is good to always keep your variables in as tight a scope as possible:

private void SendTask()
{
    SendMessage msg;
    while (true)
    {
        msg = MessageSendQueue.GetItem(-1);
        String rtrn = PushData(msg);
    }
}

This could possibly become:

private void SendTask()
{
    while (true)
    {
        SendMessage msg = MessageSendQueue.GetItem(-1);
        String rtrn = PushData(msg);
    }
}

The same principle can be applied to ReceiveTask.


pos+ (value added to position) - ex 3.5 in “ANSI Common Lisp” (p57) The problem is to take a list of integers and map them to another list containing those integers added to their respective position in the list.

Example runs with required outputs:

> (pos+ '(1 1 1 1))
(1 2 3 4)
> (pos+ '(7 5 1 4))
(7 6 3 7)

We are required to develop three alternate solutions. One using recursion, one using iteration, and one using mapcar.

I present my solutions below and seek comments on style, best coding conventions & habits to adopt. Also the question of which solution is preferred for this particular case arises, including justification for the choice.

Observations (on which I seek discussion) are included as comments in the code.

;;; (1) Using recursion
(defun pos+ (lst)
  (pos-helper lst 0))
(defun pos-helper (lst n)
  (if (null lst)
      nil
      (cons (+ (first lst) n)
            (pos-helper (rest lst) (+ n 1)))))
;; Elegant, as recursion often is, but not a tail call (cant abandon stack frame),
;; hence less efficient at scale (which may not matter).

;;; (2) With iteration
(defun pos+2 (lst)
  (do ((i 0 (+ i 1)))
      ((>= i (length lst)) lst)
      (setf (nth i lst) (+ (nth i lst) i))))
;; Fewer lines, but i have a feeling of disappointment to be learning lisp
;; and not be using a functional style. Q. Is that intuition well-placed?

;;; (3) With mapcar
(defun pos+3 (lst)
  (mapcar #'(lambda (x y) (+ x y))
          lst
          (range (length lst))))

;; This would be my 'most beautiful' solution except for one problem.
;; Could not find any built in range function. I found range [footnote 1]
;; and pasted it here. The syntax doesn't look anything like CL to me!
(defun range (max &key (min 0) (step 1))
   (loop for n from min below max by step  ;; did the lisp alien write this?! :)
      collect n))

link to range function used above

For loop you can see the excellent book by P. Seibel “Practical Common Lisp”, available on-line, in particular chapter 7 and chapter 22.

Let’s start from the last function: the idea is ok, we can just simplify the function noting that (lambda (x y) (+ x y)) is nothing more than the original +:

(defun pos+3 (lst)
  (mapcar #'+ lst (range (length lst))))

For the first function, since an helper function is introduced, it could as well be used for tail-recursion, instead of normal recursion:

(defun pos-helper (lis n acc)
  (if (null lis)
      (reverse acc)
      (pos-helper (rest lis) (1+ n) (cons (+ (first lis) n) acc))))

(defun pos+ (lis)
  (pos-helper lis 0 nil))

where acc accumulates the result, which at the end must be reversed. Note that certain Common Lisp compilers transform the function in an iterative loop, so I do not know if this would be considered recursive. In that case, your version is ok (but note the idiomatic use of (1+ expression) instead of (+ expression 1)).

Then, for the second function, of course a loop version is much more readable (even if it is not so lispy :) :

(defun pos+2 (lis)
  (loop for i from 0
        for x in lis
        collect (+ x i)))

If you prefer more parentheses, here is a variant (without modifying the original list):

(defun pos+2 (lis)
  (let ((result nil))
    (do ((i 0 (1+ i))
         (y lis (cdr y)))
        ((null y) (reverse result))
      (push (+ (car y) i) result))))

A final note about efficiency: the function pos3 is the less efficient one since it must generate a new list with the same length as the original one (and so the list must be scanned twice, in addition to doubling the memory footprint of the program). With a simple trick we could use mapcar and avoid creating a new list:

(defun pos+3 (lis)
  (let ((index -1))
    (mapcar (lambda (x) (+ x (incf index))) lis)))


Computational verification of Collatz conjecture Prerequisites

The typedef name uint128_t designates an unsigned integer type with width exactly 128 bits.

The UINT128_MAX is maximum value for an object of type uint128_t.

Function int ctz(uint128_t n) returns the number of trailing 0-bits in n, starting at the least significant bit position. If n is 0, the result is undefined.

The macro UINT128_C(n) shall expand to an integer constant expression corresponding to the type uint128_t.

The following macros are defined.

/* all 3^n for n < 41 fits into uint64_t */
#define LUT_SIZE64 41
/* all 3^n for n < 81 fits into uint128_t */
#define LUT_SIZE128 81

The following array is defined and initialized with corresponding values.

/* lut[n] contains 3^n */
uint128_t lut[LUT_SIZE128];

Problem

My program is concerned with verifying the convergence of the Collatz problem, using this algorithm.

The convergence for all values n ≤ 87 × 260 has been proven. [Source: Christian Hercher, Uber die Lange nicht-trivialer Collatz-Zyklen, Artikel in der Zeitschrift "Die Wurzel" Hefte 6 und 7/2018.]

The following function is called for n of the form \$4n+3\$, in order from the smallest one to the largest one, only if all preceding calls returned zero.

The following function should either

return 0 if the Collatz problem for the n is convergent,
return 1 if the function cannot verify the convergence using 128-bit arithmetic,
loop infinitely if the trajectory for the n is cyclic.

Code

int check_convergence(uint128_t n)
{
    uint128_t n0 = n;
    int e;

    do {
        if (n <= UINT128_C(87) << 60) {
            return 0;
        }

        n++;

        e = ctz(n);

        n >>= e;

        if (n < UINT128_C(1) << 64 && e < LUT_SIZE64) {
            return 0;
        }

        if (n > UINT128_MAX >> 2*e || e >= LUT_SIZE128) {
            return 1;
        }

        n *= lut[e];

        n--;

        n >>= ctz(n);

        if (n < n0) {
            return 0;
        }
    } while (1);
}

Given that (from your comments) there is one and only one input which would cause overflow, I propose the following check at the beginning of the function:

int check_convergence(uint128_t n)
{
        const uint128_t n0 = n;
        int e;

        if (n == UINT128_MAX)
                return  1;

        do {
                ...
        } while (true);
}

I also added const to n0, given that it's constant through all the function.

if (n < UINT128_C(1) << 64 && e < LUT_SIZE64)
        return  0;

That can be rewritten as:

if (n <= UINT64_MAX && e < LUT_SIZE64)
        return  0;

Although maybe unneeded, I prefer to always parenthesize macros that evaluate to a value, just in case:

#define LUT_SIZE128  (81)


Password hashing and database I would like to ask someone if the code i will put here is Secured for SQL injection and my password hashing and checking while user is logging in is secure and done well.

My Database connection :

$hostname='localhost';
$username='root';
$password='';

try {
    $dbh = new PDO("mysql:host=$hostname;dbname=myshop",$username,$password);
    $dbh->query ('SET NAMES utf8');
    $dbh->query ('SET CHARACTER_SET utf8_unicode_ci');
    $dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); // <== add this line
} catch (PDOException $e) {
      echo $e->getMessage();
    }

My registration script :

include_once 'config.php';

$data = file_get_contents("php://input");
$data = json_decode($data, true);

try {
    $stmt = $dbh->prepare("INSERT INTO `accounts` (`account_id`, `firstname`, `lastname`, `username`, `password`, `email`, `city`, `postalcode`, `adress`, `country`, `role`)
      VALUES (NULL, :firstname, :lastname, :username, :password, :email, :city, :postalcode, :adress, :country, 0) ");
    $stmt->bindParam(':firstname', $data['firstname']);
    $stmt->bindParam(':lastname', $data['lastname']);
    $stmt->bindParam(':username', $data['username']);
    $stmt->bindParam(':password', password_hash($data['password'], PASSWORD_DEFAULT));
    $stmt->bindParam(':email', $data['email']);
    $stmt->bindParam(':city', $data['city']);
    $stmt->bindParam(':postalcode', $data['postalcode']);
    $stmt->bindParam(':adress', $data['adress']);
    $stmt->bindParam(':country', $data['country']);
    $stmt->execute();
  } catch (PDOException $e) {
      if (strpos($e->getMessage(), "for key 'login'") !== false) {
          echo 'Duplicate login entry';
          exit;
      } if (strpos($e->getMessage(), "for key 'email'") !== false) {
          echo 'Duplicate e-mail entry';
          exit;
      }
      else {
          throw $e;
      }
  }
  echo 'Success';

My login script :

include_once 'config.php';

$data = file_get_contents("php://input");
$data = json_decode($data, true);

try {
  $stmt = $dbh->prepare("SELECT * from accounts WHERE
    username=:username");
  $stmt->bindParam(':username', $data['username']);
  $stmt->execute();
  } catch (PDOException $e) {
          throw $e;
      }

  $rows = $stmt->rowCount();
  $result = $stmt->fetchAll(PDO::FETCH_ASSOC);
  $json = json_encode($result);

  if ($rows === 1){
    if (password_verify($data['password'], $result[0]['password'])) {
      echo $json;
      exit;
    }
    else{
      echo 'Wrong password';
      exit;
    }}
  if ($rows === 0) {
    echo 'No account';
    exit;
  }
  else {
    echo 'Something went wrong';
    exit;

Your code is secure against SQL injection but it's largely inefficient. And this army of bindParams's takes the largest part in it. All you need is just to send an array you already have directly into execute():

$stmt->execute($data);

Registration

On the other hand, your use of try and catch operator for insert is proper, which is a rare sight. The only objection is error messages that shouldn't be bluntly echoed out. Better to collect them and show nicely along with a form. So I would make the whole registration code as

$error = '';
$data['password'] = password_hash($data['password'], PASSWORD_DEFAULT);
try {
    $stmt = $dbh->prepare("INSERT INTO `accounts` (`account_id`, `firstname`, `lastname`, `username`, `password`, `email`, `city`, `postalcode`, `adress`, `country`, `role`)
      VALUES (NULL, :firstname, :lastname, :username, :password, :email, :city, :postalcode, :adress, :country, 0) ");
    $stmt->execute($data);
} catch (PDOException $e) {
    if (strpos($e->getMessage(), "for key 'login'")) {
        $error =  'Duplicate login entry';
    } elseif (strpos($e->getMessage(), "for key 'email'")) {
        $error = 'Duplicate e-mail entry';
    } else {
        throw $e;
    }
}
if ($error) {
    echo $error;
} else {
    header("Location: ...");
    exit;
}

Login

The login code could be improved as well. I've got a ready made example, Authenticating a user using PDO and password_verify(), you may check it out for the actual code. Here I will just comment on some parts

} catch (PDOException $e) {
    throw $e;
}

I fancy you just copy-pasted this part from the insert example. But if you think on it, it will become clear that this code will behave exactly the same without try, catch and throw operators! So they should be simple taken out.

$rows = $stmt->rowCount();

is essentially useless, as you have the fetch result that can be used instead.

$result = $stmt->fetchAll(PDO::FETCH_ASSOC);

fetchAll() doesn't seem to be a wise choice, as it adds unnecessary dimension to the resulting array. fetch() should be used instead.


Searching for repeated characters in a sequence of characters The following code solves this problem:

  The 3072 characters below contain a sequence of 5 characters which is
  repeated. However, there is a twist: one of the sequences has a typo.
  To be specific, you're looking for two groups of 5 characters which
  share 4 of those characters. Enter the 4 shared characters below to
  proceed.

Any suggestions on how it can be improved using idiomatic Lisp/Clojure?

(ns slurp.slurp)

(defn get-five [in]
  (if (> (.length in) 4)
    (subs in 0 5)
    nil))

(defn how-good [a b position goodness bad-position]
  (if (= position 5)
    (list goodness bad-position)
    (if (= (subs a position (+ 1 position)) (subs b position (+ 1 position)))
          (recur a b (+ 1 position) (+ 1 goodness) bad-position)
          (recur a b (+ 1 position) goodness position))))

(defn matches [a b]
  (let [x (how-good a b 0 0 -1)]
    (list (> (first x) 3) (last x))))

(defn remove-bad-char [in-string bad-index]
  (str (subs in-string 0 bad-index) (subs in-string (+ 1 bad-index))))

(defn sub-parse [left in]
  (let [right (get-five in)]
    (if (not (= right nil))
      (let [match-result (matches left right)]
        (if (first match-result)
          (let [bad-index (last match-result)]
            (println left "is a match. bad-position:" (last match-result) "ans: "(remove-bad-char right bad-index)))
          (recur left (subs in 1)))))))

(defn parse [in]
    (let [left (get-five in)] ;; as soon as we hit the ass end of the input string, get-five returns null and we stop trying to match.
      (if (not (= left nil))
        (do
          (sub-parse left (subs in 1))
          (recur (subs in 1))))))

(parse (slurp "/Users/clojure/challange.txt"))

;; https://www.readyforzero.com/challenge/491f97bc25574346aed237d43e7d0404
;; (answer is i2+r)

I'd start bottom-up on this one if you want to do it in a slightly more idiomatic / functional style. Also it's good to get the data in Clojure sequences rather than strings as it makes it a bit easier to manipulate.

You're trying to work on groups of five adjacent characters so I'd set up a sequence of such adjacent groups:

; define some source data, right answer is "1235"
(def char-seq (seq "dfyfgvbufi12345idvdfhopshbsopgeobvufpdhbugphp123a5ghauoyhewqojbvrbon"))

; now partition into adjacent overlapping groups of five (with a step of 1)
(def char-groups (partition 5 1 char-seq))

; prove it works
(take 3 char-groups)
=> ((\d \f \y \f \g) (\f \y \f \g \v) (\y \f \g \v \b))

Now we want to define what a valid match means: in this case 4 characters should be the same in two groups being compared.

; helper function to count the number of true values in a collection / sequence
(defn count-true [coll]
  (reduce
    #(+ %1 (if %2 1 0))
    0
    coll))

; function to determine if exactly four characters are equal in two groups
(defn matches? [group1 group2]
  (= 4
    (count-true (map = group1 group2))))

Finally you just need to run the matches? function over all possible groups:

(def results
  (distinct
    (keep identity
      (for [group1 char-groups
            group2 char-groups]
        (if (matches? group1 group2)
          (filter identity (map #(if (= %1 %2) %1 nil) group1 group2))
          nil)))))

results
=> ((\1 \2 \3 \5))


Code Golf Challenge: Calculate Phi (not Pi) I'm a newbie to C++ (coming from C#) and I decided to complete this Code Golf challenge for fun (though I definitely wouldn't say it's short). Essentially, the challenge was to calculate the phi(not_pi()) of all the numbers from 1 to 100, and print them out in a loop, where

phi(n) is the number of integers less than or equal to n that are relatively prime to n.
not_pi(n) is the number of composites less than or equal to n.

Here is my code (I'm using Visual C++ 2015):

stdafx.h

#pragma once

#include "targetver.h"

#include <cmath>
#include <cstdio>
#include <tchar.h>

#include <iostream>
#include <stdexcept>

golf.cpp

#include "stdafx.h"

#define __nameof(var) #var

namespace golf
{
    int gcd(int left, int right) noexcept
    {
        if (right == 0)
            return left;
        return gcd(right, left % right);
    }

    bool relatively_prime(int left, int right) noexcept
    {
        return gcd(left, right) == 1;
    }

    int phi(int number)
    {
        if (number < 0)
            throw std::invalid_argument(__nameof(number));
        if (number == 0)
            return 0;
        if (number == 1)
            return 1; // 1 is the only number relatively prime to itself

        int result = 1; // We're always relatively prime to 1
        for (int i = 2; i < number; i++)
        {
            if (relatively_prime(i, number))
                result++;
        }

        return result;
    }

    bool prime(int number)
    {
        if (number < 0)
            throw std::invalid_argument(__nameof(number));

        int bound = static_cast<int>(std::floor(std::sqrt(number)));
        for (int i = 2; i <= bound; i++)
        {
            if (number % i == 0)
                return false;
        }

        return true;
    }

    int not_pi(int number)
    {
        if (number < 0)
            throw std::invalid_argument(__nameof(number));
        if (number <= 3)
            return 0;

        int result = 1;
        for (int i = 6; i <= number; i++)
        {
            if (!prime(i))
                result++;
        }

        return result;
    }

    int main()
    {
        for (int i = 1; i <= 100; i++)
        {
            std::cout << i << " " << phi(not_pi(i)) << std::endl;
        }

        return 0;
    }
}

int main()
{
    // Glue code because I didn't realize main()
    // was supposed to be in the global namespace.
    return golf::main();
}

Any tips on how I could improve my C++ code? Specifically, is there anything I could do to make it more "modern"/C++11-like, or practices I should follow? Are there any sources for undefined behavior/gotchas I didn't see? I'm a beginner so any feedback/critique would be appreciated.

I would move your main outside of the golf namespace instead of glueing code together.
The not_pi name is confusing to me because I would assume its a function that checks if n is not 3.1415926..., which it clearly doesn't do.
Another suggestion is to expose most of this functionality in a golf.h header so that you can call this code from some where else. In that case move main completely outside golf.cpp and into main.cpp or similar.
Other than that you should add documentation/comments to all non-trivial functions.

Now let's talk performance.
GCD
If you have extra memory handy, then you can optimize your gcd calls through a technique known as memoization.
Basically, for small left and right (which I would rename to a and b) you would calculate the result once and then store it for future reference. This requires a minor modification to the gcd function to check if it has a pre-calculated result for that problem, and then return the result in that case. The reason this will help in the long run is that if you trace the calls to gcd you will note that it will usually end up in the smaller values and calculate these results over and over again.
I would use a 2D array or a vector of vectors for this purpose and store up to gcd(1000, 1000) or so (if you have more memory free, feel free to increase this while your code execution time increases on the desired input).
Prime
An easy optimization is checking the i = 2 case separately from the odd numbers which allows you to loop by 2's and so is roughly a 2x speedup.
There are also much faster algorithms for this than brute force division such as Miller-Rabin which can be made deterministic for a finite range of integers (a potentially very large range, much larger than 64 bit integer maximum).


C++ number-guessing game (computer tries to guess user's chosen number) In this program we input a number. Our PC tries to guess this number.

After every try PC asks us:"Are your number more or less than?".

We input 'l' if our number is less, and input 'h' if our number is greater?

A range of possible values is created.

I think this code is bad.  What do you think?

#include "stdafx.h"
#include <iostream>
#include <cstdlib>
#include <ctime>
using namespace std;

int Random(int min, int max) {
    return min + rand() % (max - min);
}

int main()
{
    setlocale(LC_ALL, "rus");
    int our_num;
    srand(static_cast<unsigned int>(time(0)));
    cout << "Input a positive number: " << endl;
    cin >> our_num;
    int t = 0;  //number of attempts
    int max = rand() + our_num;  //maximum possible value
    int min = 0;  //minimum possible value
    int d = Random(min, max);
    do {
        char lh;
        cout << d << " Is this your number?(my number is greater: 'h';   less: 'l')" << endl;
        cin >> lh;
        ++t;
        if (lh == 'h') {
            min = d;
            d = Random(min, max);
        }
        else if (lh == 'l') {
            max = d;
            d = Random(min, max);
        }
    } while (d != our_num);
    cout << "I guessed thus number with " << t << " attempts. " << "This number is " << d << endl;
    return 0;
}

Some things stand out here:

Don't use using namespace std
Consider not using rand()
Prefer using \n over std::endl
return 0 can be dropped from main as it's optional
You really need better variable names. Names should explain what the variable is used for. If you pick better names your superfluous comments can be removed alltogether


replacing every pair in a list I need a function (I named it applyToEveryPair) that works on lists, e.g. [x0,x1,x2,x3,...], and uses a function
f:: (a,a) -> [(a,a)]. For every (distinct) pair (xi,xj), i/=j of the list
I want all lists where xi is replaced by yik and xj is replaced by yjk where xik is the output of f.

This visualizes input and output

[a0, a1 ,a2 ,a3 ]  -- input to applyToEveryPair
  |   |
  |   |
  v   v
  f(a0, a1) =[(b0,b1),(c0,c1)]
  |   |
  v   v
[b0, b1, a2, a3]  -- to be included in output
[c0, c1, a2, a3]  -- to be included in output
   ...            -- more output
      |      |    -- now combine (a3,a1)
       \    /
        \  /
         \/
         /\
        /  \
     f(a3, a1) =
     [(d3, d1)]
        \  /
         \/
         /\
        /  \
       /    \
[a0, d1 ,a2 ,d3 ]  -- to be included in output

A use case would be to input a matrix and compute all resulting matrices for every (pairwise) line swap (f swaps lines), or, in my case, all possible move in a solitaire game from one stack of cards to another.

Long story, short code; This is how I do it so far:

applyToEveryPair :: ((a,a) -> [(a,a)]) -> [a] -> [[a]]
applyToEveryPair f (xs) = do i<-[0..length xs - 1]
                             j<-[0..length xs - 1]
                             guard (i /= j)
                             (x',y') <- f (xs !! i, xs !! j)
                             return . setList i x' . setList j y' $ xs

-- | setList n x xs replaces the nth element in xs by x
setList :: Int -> a -> [a] -> [a]
setList = go 0 where
    go _ _ _ [] = []
    go i n x' (x:xs) | i == n    = x':xs
                     | otherwise = x:(go (i+1) n x' xs)

I think comonads are overkill here, and I have not understood Lenses (yet),
but have a vague feeling lenses apply here.

The solution I use feels not very haskellish. I want to know if this is a good way to write it, but especially the double use of setList looks terrible to me. How to speed up / beautify this code?

You can use the lens library to remove the need for the setList function. The ix lens does this, giving read+write access to an element at an index. The type of this is:

ix :: Index m -> Traversal' m (IxValue m)

Or, for lists in particular:

ix :: Int-> Traversal' [a] a

Using the .~ operator, the value of a lens can be set. So to remove need for the setList funtion, just use this:

applyToEveryPair2 :: ((a,a) -> [(a,a)]) -> [a] -> [[a]]
applyToEveryPair2 f (xs) = do i<-[0..length xs - 1]
                              j<-[0..length xs - 1]
                              guard (i /= j)
                              (x',y') <- f (xs !! i, xs !! j)
                              return . (ix i .~ x') . (ix j .~ y') $ xs

The other thing to notice about your code is it will call every pair of elements twice: eg it will call the supplied function with the item at index 2 and 5, then with the items at indexes 5 and 2. I'm not sure if this is intended.


FizzBuzz with Lambda Expressions in Java I wanted to create a FizzBuzz in Java that is supposed to be open for extension.

So the initial problem is the good old, if divisible by 3 print fizz, if divisible by 5 print buzz, if divisible by both print fizzbuzz, otherwise print the number itself.

But I want to be able to change the numbers that we test against (3 and 5 can become 4 and 6) and the words we print (fizz and buzz may be come foo and bar). And there may be new ones added, such as 7, qux..

Here is what I came up with, any suggestions are welcome:

import java.util.*;
import java.util.function.Function;
import java.util.function.IntPredicate;

class App {
    public static void main(String[] args) {
        Function<Integer, IntPredicate> divisibilityPredicateBuilder
            = isDivisibleBy -> x -> x % isDivisibleBy == 0;

        final Map<Integer, String> fizzersBuzzers = new HashMap<>();
        fizzersBuzzers.put(3, "fizz");
        fizzersBuzzers.put(5, "buzz");

        for (int i = 1; i < 101; i++) {
            String fizzBuzz = "";

            for (Integer fizzerBuzzer : fizzersBuzzers.keySet()) {
                if (divisibilityPredicateBuilder.apply(fizzerBuzzer).test(i)) {
                    fizzBuzz += fizzersBuzzers.get(fizzerBuzzer);
                    fizzBuzz += " ";
                }
            }

            if (fizzBuzz.isEmpty()) {
                fizzBuzz = String.valueOf(i);
            }

            System.out.println(fizzBuzz.trim());
        }
    }
}

You used a HashMap, where the iteration order is arbitrary. There is no guarantee whether you'll get "fizz buzz" or "buzz fizz"!


Opening a file in Python I use the following function for opening files in Python:

def file_path(relative_path):
    folder = os.path.dirname(os.path.abspath(__file__))
    path_parts = relative_path.split("/")
    new_path = os.path.join(folder, *path_parts)
    return new_path

I call it with a relative path, like this:

with open(file_path("my_files/zen_of_python.txt")) as f:
    zop = f.read()

I like it because you can use a relative path works no matter where the Python script is executed from:

folder = os.path.dirname(os.path.abspath(__file__)) gets the
absolute path to the folder holding the python script.
os.path.join(folder, *path_parts) gets an os-independent path to the file to open.

To further explain, let's say I have the following folder structure:

- parent_folder
  - example.py
  - my_files
    - zen_of_python.txt

If example.py looks like this:

with open("my_files/zen_of_python.txt") as f:
    zop = f.read()

Then I have to run example.py from the parent_folder directory or else it won't find my_files/zen_of_python.txt.

But if I open my_files/zen_of_python.txt using the file_path() function shown above, I can run example.py from anywhere.

One downside as @infinitezero pointed out is that you can't use an absolute path, but for my purposes, that's okay for my purposes. The script is self-contained. I'm not passing in external files when I run it.

Can anyone see any downside to this? Or does anyone have a more Pythonic way of accomplishing the same thing?

Your code represents a well-known pattern. In Java, files like this are called resource files and they are delivered together with the code in a .jar file (which is essentially a .zip file).

As pointed out in a comment, you cannot use your code with absolute paths. This is good since the entire purpose of the code is to find a resource relative to the source code that needs this resource.

The call to abspath looks redundant to me. I'm assuming that __file__ is already an absolute path. You may want to check the documentation about it.

A downside of your function is that you have to define it in each file that wants to open relative files since it uses __file__. You cannot import that function, as it is now.


A function to scan user input as string I know that multiple functions are already available. However, I thought of writing my own because I wanted to learn the logic (and also because I thought there wasn't enough confusion :P). Please review the function I wrote and suggest me efficient changes.

Without further ado, here I go:

scan(string,size)
char** string;
size_t size;
{
    string[0]=(char*)malloc(sizeof(char));
    char keystroke=' ';
    while((keystroke=getc(stdin))!='\n') {
        string[0][size++]=keystroke;
        string[0]=(char*)realloc(string[0],size+1);
    }
    string[0][size]='\0';
    return size;
}

Your code is horrible:

You don't check for end-of-file, which will lead to endless loops
You don't check for failure of malloc or realloc
You call realloc way too often, which makes the code slow for large lines of input
You cast the result of malloc
You use the outdated style of function definition from the 1980s
The variable string is not a string but points to an array of pointers to strings, which is confusing
The #include for size_t is missing
Using [0] instead of * is extremely confusing
There is no need to initialize keystroke to a space character
The parameter size is useless since the only possible value that makes sense for it is 0
The return type of the function is implicitly int, which is obsolete
The returned value is of type size_t, which doesn't fit into an int and additionally differs in signedness


"What is the fastest solution for finding the maximum sum of a subarray?" Question

  Given an array, find the longest continuous sub-array which has maximum sum.

My Approach

First, I solved this problem using dynamic programming which effectively solved the problem in \$O(n)\$ time as opposed to the brute force approach. For each iteration, I checked if the sum is less than 0, and if true, I ran a new sum whenever the sum became less than 0.

Also, I kept track of the current maximum in each iteration.

class Solution
{
    public int maxSubArray(int[] nums)
    {
        /*
         * get the array size.
         */
        int size = nums.length;

        /*
         * initialize the variables to the first index of the array.
         */
        int sum = nums[0];
        int max = nums[0];

        /*
         * iterate using a loop from 1 to n-1.
         * set sum to 0 if current is less than 0.
         * for negative results, no need to add sum as the largest sum is the smallest negative number itself.
         * update max if sum is greater.
         */
        for (int i = 1; i < size; i++)
        {
            if (sum < 0)
                sum = 0;

            sum += nums[i];
            if (sum > max)
                max = sum;
        }
        return max;
    }
}

As you can see from the above, I have kept the code as compact as possible. I also stored the array length also in a variable for faster execution. And the solution is linear time.

Issues

When I submit on LeetCode, I am told that my code is only 17% faster as compared to the rest of Java submissions for the same problem.
How do I make this code more optimised (or rather, is there any way to optimise this code even more?)
Is there a better/different approach to this problem?
I haven't used any Java APIs, library functions etc. then could anyone please tell me where is the time being consumed (or rather, which is the most costly operation in my algorithm and is there any way to reduce the costly operation?)

Notes

The solution is correct as it passed all 200 test cases! (It's just that it is slow).

To find the required sub-array, I need to iterate over the array at least once just to see if my sum can be maximised. In such a case, the minimum time has to be linear time and if this observation were true, then all the other faster solutions must run at least linear time. Again, if the above were true, then it means the faster solutions are using techniques to make the execution faster despite it being linear time, right?

Can't get much better
First of all, I would have to agree with the commenters that your solution is fine.  Any further gains upon your solution would probably depend on random factors in the Leetcode judging system.  Your main loop does the minimal amount of work necessary to solve the problem.
Reduce code?
That being said, I went ahead and tried a few things to see how random the Leetcode judging system was.  I ended up with this code which is essentially the same as the original code except it doesn't special case the first array entry and is therefore a little smaller:
class Solution {
    public int maxSubArray(int[] nums)
    {
        int size = nums.length;
        int sum = 0;
        int max = Integer.MIN_VALUE;

        for (int i = 0; i < size; i++)
        {
            sum += nums[i];
            if (sum > max)
                max = sum;
            if (sum < 0)
                sum = 0;
        }
        return max;
    }
}

I submitted this code several times and randomly got a submission in the 98.67% percentile (13 ms).  I also had submissions that scored 85% (14 ms), 56% (15 ms), 32% (16 ms), and 18% (17 ms).  Given the fact that the same code varied in time between 13 ms and 17 ms, I would say that random factors have more to do with your score than the actual code.
You can see my 13 ms submission here.  I'm not sure if that link is private to my account or not, but you could always just copy and paste my code and submit many times yourself.


Selection sort implementation in C++ I was wondering if this implementation of selection sort could be improved. Are there any things that I have done wrong?

template<typename Element>
void selection_sort(Element arr[], size_t size) {
    auto index_toSort = 0u;
    while (index_toSort < size) {
        auto index_smallest = index_toSort;
        for (auto index = index_toSort; index < size; ++index) {
            if (arr[index] < arr[index_smallest]) {
                index_smallest = index;
            }
        }
        std::swap(arr[index_toSort], arr[index_smallest]);
        ++index_toSort;
    }
}

In all three of your examples you limit yourself to using arrays.

This is not very C++ like (even if you templatize the object). I would change the interface to use iterators. That way your sort can be applied to any container with a random access iterator.

template<typename I>
void selection_sort(I begin, I end) {
    std::size_t size = std::distance(begin, end);


Flexible multiple string comparision to determine variable value I've a web form that allows users to create clusters of three sizes: small, medium and large. The form is sending a string of small, medium or large to a message queue, were job dispatcher determines how many jobs (cluster elements) it should build for a given cluster, like that:

# determine required cluster size
if cluster['size'] == 'small':
    jobs = 3
elif cluster['size'] == 'medium':
    jobs = 5
elif cluster['size'] == 'large':
    jobs = 8
else:
    raise ConfigError()

While it works, its very ugly and non flexible way of doing it - in case I'd want to have more gradual sizes, I'd increase number of elif's. I could send a number instead of string straight from the form, but I dont want to place the application logic in the web app. Is there a nicer and more flexible way of doing something like that?

You could keep an array of jobs, and the web application would send index instead of word 'small', 'medium', 'large'.

So you could have an array [3, 5, 8] and instead of "small" you send 0, instead of "medium" you send 1, and instead of "large" you send 2. You only need to check if the index received does not exceed the table size.


Crypt-arithmetic solver (Brute force) in Python I'm working on a crypt-arithmetic solver to add two or more numbers. I've used brute-force to find all the permutations. I'm looking forward to improve this algorithm. Kindly review for all the aspects of this code and provide me your comments.

import string
import itertools
inListNumsAsStringArray = [ ['BASE', 'BALL'],
                            ['SEND', 'MORE'] ]
inResultsArray = [ 'GAMES',
                'MONEY' ]
inPossibleNumsAsStr = '0123456789'

# Input: (string, dictionary)
#   string 'AB'
#   key-value pairs for characters and numbers. Ex: dictCharAndDigit = {'A': '1', 'B': '2'}
# Output:(Number)
#   12
def getNumberFromStringAndMappingInfo(inStr, inDictMapping):
    numAsStr = ''
    for ch in inStr:
        numAsStr = numAsStr + inDictMapping[ch]
    return int(numAsStr)

def solveCryptarithmeticBruteForce(inListNumsAsString, inResultStr, inPossibleNumsAsStr):
    nonZeroLetters = []
    strFromStrList = ''
    for numStr in inListNumsAsString:
        nonZeroLetters.append(numStr[0])
        strFromStrList = strFromStrList + numStr
    nonZeroLetters.append(inResultStr[0])
    strFromStrList = strFromStrList + inResultStr
    uniqueStrs = ''.join(set(strFromStrList))
    for tup in itertools.permutations(inPossibleNumsAsStr, len(uniqueStrs)):
        dictCharAndDigit = {}
        for i in range(len(uniqueStrs)):
            dictCharAndDigit[uniqueStrs[i]] = tup[i]
        nonZeroLetterIsZero = False
        for letter in nonZeroLetters:
            if(dictCharAndDigit[letter] == '0'):
                nonZeroLetterIsZero = True
                break
        if(nonZeroLetterIsZero == True):
            continue
        result = getNumberFromStringAndMappingInfo(inResultStr, dictCharAndDigit)
        testResult = 0
        for numStr in inListNumsAsString:
            testResult = testResult + getNumberFromStringAndMappingInfo(numStr, dictCharAndDigit)

        if(testResult == result):
            strToPrint = ''
            for numStr in inListNumsAsString:
                strToPrint = strToPrint + numStr + '(' + str(getNumberFromStringAndMappingInfo(numStr, dictCharAndDigit)) + ')' + ' + '
            strToPrint = strToPrint[:-3]
            strToPrint = strToPrint + ' = ' + inResultStr + '(' + str(result) + ')'
            print(strToPrint)
            break

for i in range(len(inResultsArray)):
    solveCryptarithmeticBruteForce(inListNumsAsStringArray[i], inResultsArray[i], inPossibleNumsAsStr)

1. Review

In Python it is helpful to write a docstring for each function or class that you define, explaining what it does, what arguments it takes, and what it returns. Docstrings can be accessed from the interactive interpreter using the built-in help function. For getNumberFromStringAndMappingInfo there is a comment which could be used as the basis for a docstring.
solveCryptarithmeticBruteForce does nothing if the puzzle has no solution. This would be easy to miss in a long list of solutions. It would be better to print something like "no solution".
Python programmers generally follow the Python style guide when choosing names. According to this guide, functions and variables should have names using lower_case_with_underscores (and not mixedCase as in the code in the post). You're not obliged to follow this convention but if you do then it will make it easier to collaborate with other Python programmers.
In Python we don't use "Hungarian" naming, in which a name indicates the type of the object named. That's because Python is a "duck-typed" in which the type of an object is not as important as the set of methods it supports. In general we find that code is clearer if names are chosen to be simple and readable rather than cluttered with details.
In Python we don't need to indicate for each argument to a function whether it is used for input or output. That's because functions can easily return a tuple of results, so there is no need for functions to modify their arguments in order to return multiple results.
inPossibleNumsAsStr is built into Python as string.digits.
The puzzles are represented by two lists, one of which gives the addends for each puzzle, and the other gives the results for each puzzle in the same order:

inListNumsAsStringArray = [ ['BASE', 'BALL'],
                            ['SEND', 'MORE'] ]
inResultsArray = [ 'GAMES',
                'MONEY' ]

This kind of data organization is hard to follow and error-prone. It's just about ok here where you only have two puzzles, but imagine having a hundred puzzles — how would you check that puzzle 57 had been entered correctly and not accidentally mixed up with puzzles 56 and 58?

The approach I would take would be to represent the puzzles like this:

PUZZLES = '''
BASE + BALL = GAMES
SEND + MORE = MONEY
'''

The advantages of this representation are that it is clear what it means (no need for complex documentation) and it keeps each puzzle on one line, so that adding and removing puzzles is easy. The data structure can easily be processed by splitting each line into its constituent pieces, like this:

for puzzle in PUZZLES.strip().splitlines():
    addition, result = puzzle.split(' = ')
    addends = addition.split(' + ')
    solveCryptarithmeticBruteForce(addends, result, string.digits)

The initial letters (that must be non-zero) might include duplicates. So it would make sense to remove duplicates in the same way as you do with uniqueStrs.
The function getNumberFromStringAndMappingInfo translates a string by mapping each character through a dictionary. But this can be done using the built-in str.translate method. The way you use this is to call str.maketrans to make the translation table. So instead of:

for tup in itertools.permutations(inPossibleNumsAsStr, len(uniqueStrs)):
    dictCharAndDigit = {}
    for i in range(len(uniqueStrs)):
        dictCharAndDigit[uniqueStrs[i]] = tup[i]

I would write:

for perm in itertools.permutations(inPossibleNumsAsStr, len(uniqueStrs)):
    decipher_table = str.maketrans(uniqueStrs, ''.join(perm))
    def decipher(s):
        return s.translate(decipher_table)

and then instead of:

result = getNumberFromStringAndMappingInfo(inResultStr, dictCharAndDigit)

I would write:

result = int(decipher(inResultStr))

This would avoid the need for the getNumberFromStringAndMappingInfo function.
Instead of building a string strFromStrList that concatentates the addends and result, and then turning this into a set in order to eliminate duplicates, build the set directly. That is, instead of:

strFromStrList = ''
for numStr in inListNumsAsString:
    strFromStrList = strFromStrList + numStr
strFromStrList = strFromStrList + inResultStr

I would write:

letters = set(result)
for addend in addends:
    letters.update(addend)
letters = ''.join(letters)

And this could be made even shorter using itertools.chain:

letters = ''.join(chain(result, *addends))

Similarly, nonZeroLetters could be constructed like this:

initial_letters = ''.join(set(chain(result[0], (a[0] for a in addends))))

The sum of the deciphered addends can be computed using the built-in function sum. Instead of:

testResult = 0
for numStr in inListNumsAsString:
    testResult = testResult + getNumberFromStringAndMappingInfo(numStr, dictCharAndDigit)

I would write:

deciphered_sum = sum(int(decipher(addend)) for addend in addends)

2. Revised code

from itertools import chain, permutations
from string import digits

def solve_cryptarithm(addends, result):
    """Print a solution to the cryptarithm, if any exists.
    Arguments are the list of addends and the result of the sum.
    For example:

    >>> solve_cryptarithm(['SEND', 'MORE'], 'MONEY')
    SEND(9567) + MORE(1085) = MONEY(10652)

    """
    letters = ''.join(set(chain(result, *addends)))
    initial_letters = ''.join(set(chain(result[0], (a[0] for a in addends))))
    for perm in permutations(digits, len(letters)):
        decipher_table = str.maketrans(letters, ''.join(perm))
        def decipher(s):
            return s.translate(decipher_table)
        if '0' in decipher(initial_letters):
            continue # leading zeros not allowed
        deciphered_sum = sum(int(decipher(addend)) for addend in addends)
        if deciphered_sum == int(decipher(result)):
            def fmt(s):
                return f"{s}({decipher(s)})"
            print(" + ".join(map(fmt, addends)), "=", fmt(result))
            break
    else:
        print(" + ".join(addends), "=", result, " : no solution")