Python - Automate code involving suffix I am trying to automate this portion of code in python (code shown for 2 polygons, I am trying to do the same for 10 000 polygons). Any ideas on how to implement this using for loop or whichever fastest way you recommend.

### Polygon 1
def checkPolygon1(row):
    point = Point((row['X']), (row['Y']))
    return polygon1.contains(point)

#Apply fn to each row of dataframe (datacopy)
datacopy.apply(checkPolygon1, axis=1)

#Create new variable InPolygon1 in data if X, Y in shapely file
datacopy['InPolygon1'] = datacopy.apply(checkPolygon1, axis=1)

### Polygon 2
def checkPolygon2(row):
    point = Point((row['X']), (row['Y']))
    return polygon2.contains(point)

datacopy.apply(checkPolygon2, axis=1)
#Create new variable InPolygon2 in data if X, Y in shapely file
datacopy['InPolygon2'] = datacopy.apply(checkPolygon2, axis=1)

Both functions checkPolygon1 and checkPolygon2 perform essentially the same actions and only differ in specific polygon instance. That's an appropriate case for Parameterize Function refactoring technique where polygon instance can serve as a factor.

Define a unified function point_in_polygon (don't forget about Python naming conventions):

def point_in_polygon(row, polygon):
    return polygon.contains(Point(row['X'], row['Y']))

Then, assuming that you already have a list or generator of polygons polygons:

for i, polygon in enumerate(polygons, 1):
    datacopy[f'InPolygon{i}'] = datacopy.apply(point_in_polygon, axis=1, polygon=polygon)


Update MediaWiki settings with dynamic IP address I'm running a MediaWiki instance from a home server where the public IP address is not guaranteed to remain static. The following script is meant to run at boot time to check if the IP address has been changed an if so to update the relevant line in the LocalSettings.php file.

Here's the script:

#!/bin/bash

echo -n 'Retrieving public IP address... '
current_ip=` dig +short myip.opendns.com @resolver1.opendns.com `

if [ "$current_ip" == "" ]; then
    echo 'no response. No action taken.'; exit 1;
# check for a well formed IP adress: 000.000.000.000
elif [[ $current_ip =~ ^[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}$ ]]; then
    echo $current_ip
else
    echo 'invalid IP address detected. No action taken'; exit 2;
fi

old_ip=`cat /var/lib/mediawiki/LocalSettings.php | grep '\$wgServer' | cut -d '"' -f2 | cut -d\/ -f3`
if [ "$old_ip" == "$current_ip" ]; then
    echo 'LocalSettings.php is already up-to-date. No action taken.'
else
    echo -n "Replacing $old_ip with $current_ip in LocalSettings.php... "
    sed -i.bak "s|\$wgServer =.*|\$wgServer = \"http://${current_ip}\";|" /var/lib/mediawiki/LocalSettings.php
    echo 'done.'
fi

That's nutty, trying to address a server by a changing IP address. Reconfiguring the wiki to adjust to it is even crazier.

The standard practice for adapting to dynamic IP addresses is to use a dynamic DNS service. Then, everything else works normally.

Pick a hostname, like afuna.noip.com. Run a DDNS client to register your hostname whenever you acquire a DHCP lease, and re-register that hostname periodically thereafter.
Use it to form the URL of your site, e.g. https://afuna.noip.com/.
Configure your MediaWiki setting using that URL once, and you won't need that hacky script anymore.


Query to plot a histogram of song ratings I wrote code to plot a rating distribution, which looks like this:

The most important function is the stats function:

function stats($songID){
    $st=$this->conn->prepare("SELECT
        (SELECT COUNT(*) FROM list WHERE SongID=:songID),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='10'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='9.5'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='9'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='8.5'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='8'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='7.5'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='7'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='6.5'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='6'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='5.5'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='5'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='4.5'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='4'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='3.5'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='3'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='2.5'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='2'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='1.5'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='1'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='0.5')
        FROM list WHERE SongID=:songID");
    $st->bindparam(":songID",$songID);
    $st->execute();
    return $st->fetchall();
}

function bars($rate,$percentage,$ratings){
    return <<<HTML

                {$rate}

                {$percentage}%({$ratings} votes)

HTML;
}

And then in another file I use these methods:

$stats=$song->stats($_GET['songid']);
    foreach($stats as $key){
        $total=$key[0];
        $rate=10;
        for($i=1;$i<21;$i++){
            echo $song->bars($rate,round(($key[$i]/$total)*100,2),$key[$i]);
            $rate-=0.5;
            }
            break;
        }

This code works perfectly, but somehow I'm not satisfied with the stats function, the way I solved it. It's just too long, and doesn't look quite good to me.

So the question is: is there a way to make that function a bit shorter, so I wouldn't repeat the same thing 20 times but with different values for the Rating column?

I was thinking that this might actually be the only way to do it, but I thought I'd give it a try here, since there are way smarter programmers here than I am.

It would be better to use GROUP by in your SQL. By this way you will get rid of long SQL query.

SELECT Rating, COUNT(id) FROM list WHERE songID=:songID GROUP BY Rating;


Inspectors & Deconstruction on Unions in a list I am still new to functional programming and found myself trying to filter and map a list of discriminating unions.  I could be wrong, but it started to feel very unnatural to do this.  I just couldn't figure out how else to get the workflow that I wanted in the CreateThing method.

The IsTrue/False and Value/Error methods are very useful, but their implementations seem awkward.  But they are what allows me to work with DUs in list form.

Is there a more elegant/natural solution to this?

module Test

open System.Collections.Generic

// some borrowed finaglery from s.o.
module Seq =
    // borrowed from: https://stackoverflow.com/a/12564899
    // takes until the condition is true plus one more
    let takeUntil predicate (s:seq<_>) =
        let rec loop (en:IEnumerator<_>) = seq {
            if en.MoveNext() then
                yield en.Current
            if predicate en.Current then
                yield! loop en
            }

        seq { use en = s.GetEnumerator()
            yield! loop en
            }

// the union in question
type Valid<'a> =
    | Valid of 'a
    | Invalid of string

// the inspectors and destructors in question
module Valid =

    let IsTrue v = match v with |Valid _-> true |_-> false
    let IsFalse v = match v with |Valid _-> false |_-> true
    let Value v = match v with |Valid v -> v | Invalid _ -> failwith "no value"
    let Error v = match v with |Valid _ -> failwith "no error" | Invalid i -> i

module UseCase =

    type Thing = {
        Values : int list
    }

    let ValidateNumber (i : int) : Valid<int> =
        if i = 3 then Invalid "no threes yo" else Valid i

    let CreateThing (inputs : int seq) : Valid<Thing> =
        // pull items through the seq until i hit a problem
        let worked = inputs |> Seq.map ValidateNumber |> Seq.takeUntil Valid.IsFalse |> Seq.toList

        // see if it all worked out or not
        let success = List.last worked |> Valid.IsTrue

        if success then
            // deconstruct my 'valids' back to primitive ints
            Valid {Thing.Values = worked |> List.map Valid.Value}
        else
            // deconstruct my last error into a new error
            Invalid (List.last worked |> Valid.Error)

Fundamentally, for working with DUs, there is no other way. A DU represents values that can be "either this or that". This means that, when you're looking at one of these values, your code has to handle all possible options of what that value can be, otherwise your program will simply not know what to do for certain cases at runtime. There is no way around it.

That said, you can express this basic idea in different ways.

First, you can use the function keyword to shorten the pattern match a bit. let f = function is just a shorthand for let f x = match x with:

let IsTrue = function Valid _ -> true | _ -> false

Second, you can express IsFalse as a negation of IsTrue:

let IsFalse = not << IsTrue

Alternatively, you could extract the pattern matching part into a function (commonly referred to as "catamorphism"):

let inline validCata ifValue ifError = function Value x -> ifValue x | Error x -> ifError x

and then express the other functions in terms of it:

let inline IsTrue x = validCata (ct true) (ct false) x
let inline IsFalse x = validCata (ct false) (ct true) x
let inline Value x = validCata id (fun _ -> failwith "") x
let inline Error x = validCata (fun _ -> failwith "") id x

(where ct is a trivial constant function, which is sadly missing in F# standard library)

As for the CreateThing function, let me see if I understood the intent correctly: you're trying to validate the sequence of numbers and bail on the first error, returning it; or return the list of numbers if all of them are valid.

If this is the correct intent, you don't need to wrap each number in a Valid at all. Just run the validation function on them until it returns an error, and if it doesn't, return the original list. To run a function on a sequence and stop when it hits the first time, there is a handy library function Seq.tryPick:

let errorFor i =
    if i = 3 then Some "no threes yo" else None

let createThing inputs =
    let firstError = Seq.tryPick errorFor inputs
    match firstError with
    | Some err -> Invalid err
    | None -> Valid { Thing.Values = List.ofSeq inputs }

If you hit this sort of problem often, and really want to use Valid as a general-purpose mechanism for dealing with errors, you can extract the "run sequence until error" part as a reusable thing:

let validSeq (s: Valid<'a> seq) : Valid<'a list> =
    let getError = function Valid _ -> None | Error err -> Some err
    let getValue = function Valid x -> x | Error _ -> Unchecked.defaultOf<'a>
    let lst = List.ofSeq s
    match List.tryPick getError lst with
    | Some err -> Error err
    | None -> lst |> List.map getValue |> Valid

let validMap f = function
    | Value v -> Valid (f v)
    | Error err -> Error err

let createThing inputs =
    inputs
    |> Seq.map ValidateNumber
    |> validSeq
    |> validMap (fun vs -> { Thing.Values = vs })

(note that here I have also extracted validMap as a reusable thing)

Final note: unless you have actually hit (and measured) performance issues, don't work with sequences, work with lists instead. Since F# is not pure, running sequences multiple times can be expensive and unpredictable.


What is the best practice for working with a project as nested modules? I come from the world of Python, so I'm used to every file representing their own enclosed module, never polluting the global environment. Up until now (I've worked with Ruby for a week) I've created one file per class in a nested structure such as this:

lib
`- project_name
   |- sub_module
   |  `- foo_bar.rb
   `- some_class.rb

Where some_class.rb contains:

module ProjectName
    class SomeClass
        # ...
    end
end

And foo_bar.rb contains:

module ProjectName::SubModule
    class FooBar
        # ...
    end
end

This has incidentally worked fine up until now, but I realized that if I require foo_bar.rb before I require some_class.rb I would get a NameError: uninitialized constant ProjectName. So now I'm exploring workarounds for foo_bar.rb, all pretty ugly in my opinion:

module ProjectName
    module SubModule
        # Double indent for all the code in the file!?
    end
end

module ProjectName end
module ProjectName::SubModule
    # Fair enough, I suppose...
end

module ProjectName module SubModule
    # Ugly
end end

I suppose any of these would work and the choice would be a matter of taste, but I'd rather use the option that most developers in this situation uses. The question is in the title, please comment.

I use the version with double indent and I think it is the common usage:

module ProjectName
    module SubModule
        # Double indent for all the code in the file!?
    end
end

If you don't like the double indention, just don't do it.

It is best practice to indent each code level (module, class, def, loops...), but no must.
In Python it is part of the syntax, but not in Ruby.

See also https://stackoverflow.com/questions/3833641/is-there-an-acceptable-way-of-putting-multiple-module-declarations-on-the-same-l


Is there some way of taking advantage of code reuse in this example? I am implementing a set, using an array as the backend.

Here is how I declared it (and some method implementations):

public class Set {
            private int[] elementData;
            private int size;
               ....
          public void remove(int value) {
             for(int count = 0; count < size; count ++) {
                    if(elementData[count] == value) {
                        elementData[count] = elementData[size - 1];
                        size --;
                        break;
                    }
              }
           }
          public boolean contains(int value) {
             for(int count = 0; count < size; count ++) {
                 if(elementData[count] == value) {
                    return true;
                 }
            }
            return false;
        }
   }

I tested these methods with the JUnit testing framework and they both ran fine and worked. However I am trying to improve code reuse because I see the same iteration and a test condition in both methods, that is

for(int count = 0; count < size; count ++) {
            if(elementData[count] == value) {

But both methods perform different operations if they make it into a conditional block, one returns while the other moves elements and then breaks. Is there a way to reduce code redundancy here or is that not necessary?

You can absolutely refine the code to reuse the for loop. You can put the for loop in a function, which take the int value as input and return the index of value. Let's call it "iteration". If not found the value in elementData, just return -1 instead.

public int getIndex(int value) {
    for(int count = 0; count < size; count ++) {
        if(elementData[count] == value) {
            return count;
        }
    }
    return -1;
}

In your remove function, if you get -1 as the return value of the "iteration" function, do nothing. If you get a positive value, remove elementData[index]. Similar in the contains function.


Concurrent stack implementations in Ruby (relative performance of mutexes/CAS?) This code is very simple, but it is intended as an experiment in the relative performance of mutexes/CAS on different platforms. The latest version can always be found at:

https://github.com/alexdowad/showcase/blob/master/ruby-threads/concurrent_stack.rb

It comes with a built-in benchmarking script. Please try running and post the results, along with your OS, version of Ruby, and CPU. Please also post if you can find any way to improve style or performance.

For convenience the first version of the code is reproduced below:

# 3 thread-safe stack implementations
# Written by Alex Dowad

# Usage:
# stack.push(1)
# stack.peek    => 1 (1 is not removed from stack)
# stack.pop     => 1 (now 1 is removed from stack)

require 'rubygems' # for compatibility with MRI 1.8, JRuby
require 'thread'
require 'atomic'   # atomic gem must be installed

# The easy one first
class ThreadSafeStack
  def initialize
    @s,@m = [],Mutex.new
  end
  def push(value)
    @m.synchronize { @s.push(value) }
  end
  def pop
    @m.synchronize { @s.pop }
  end
  def peek
    @m.synchronize { @s.last }
  end
end

# a non-locking version which uses compare-and-swap to update stack
class ConcurrentStack
  Node = Struct.new(:value,:next)

  def initialize
    @top = Atomic.new(nil)
  end
  def push(value)
    node = Node.new(value,nil)
    @top.update { |current| node.next = current; node }
  end
  def pop
    node = nil
    @top.update do |current|
      node = current
      return if node.nil?
      node.next
    end
    node.value
  end
  def peek
    node = @top.value
    return if node.nil?
    node.value
  end
end

# same as ConcurrentStack, but additionally recycles popped nodes
# (to reduce load on GC)
# a global free list is used, and is also updated using CAS,
#   in exactly the same way as the stacks themselves
class RecyclingConcurrentStack
  Node      = Struct.new(:value,:next)
  FREE_LIST = Atomic.new(nil)

  def initialize
    @top = Atomic.new(nil)
  end
  def push(value)
    node = get_node(value)
    @top.update { |current| node.next = current; node }
  end
  def pop
    node = nil
    @top.update do |current|
      return if (node = current).nil?
      node.next
    end
    result = node.value
    FREE_LIST.update do |current|
      node.next = current
      node
    end
    result
  end
  def peek
    node = @top.value
    return if node.nil?
    node.value
  end

  private
  def get_node(val)
    # if contention causes the CAS to fail, just allocate a new node
    node = FREE_LIST.value
    if node && FREE_LIST.compare_and_swap(node,node.next)
      node.value = val
      node
    else
      Node.new(val,nil)
    end
  end
end

# Test driver
if __FILE__ == $0
  require 'benchmark'
  ITERATIONS_PER_TEST = 1000000
  QUEUE,MUTEX = ConditionVariable.new,Mutex.new

  def wait_for_signal
    MUTEX.synchronize { QUEUE.wait(MUTEX) }
  end
  def send_signal
    MUTEX.synchronize { QUEUE.broadcast }
  end

  def test(klass)
    test_with_threads(klass,1)
    test_with_threads(klass,5)
    test_with_threads(klass,25)
  end

  def test_with_threads(klass,n_threads)
    stack = klass.new
    iterations = ITERATIONS_PER_TEST / n_threads
    puts "Testing #{klass} with #{n_threads} thread#{'s' if n_threads>1}, iterating #{iterations}x each"

    threads = n_threads.times.collect do
                Thread.new do
                  wait_for_signal
                  iterations.times do
                    stack.push(rand(100))
                    stack.peek
                    stack.pop
                  end
                end
              end
    n_gc = GC.count
    sleep(0.001)

    result = Benchmark.measure do
      send_signal
      threads.each { |t| t.join }
    end
    puts result
    puts "Garbage collector ran #{GC.count - n_gc} times"
  end

  test(ThreadSafeStack)
  test(ConcurrentStack)
  test(RecyclingConcurrentStack)
end

I took the latest source code from the github and have started review of this project. Here are my benchmarks results:

# Intel(R) Core(TM)2 Duo CPU     P7570  @ 2.26GHz
# ruby 1.9.2p290 (2011-07-09 revision 32553) [i686-linux]
Testing ThreadSafeStack with 1 thread, iterating 1000000x each
  2.250000   0.000000   2.250000 (  2.254392)
Garbage collector ran 0 times
Testing ThreadSafeStack with 5 threads, iterating 200000x each
  2.350000   0.020000   2.370000 (  2.533645)
Garbage collector ran 0 times
Testing ThreadSafeStack with 25 threads, iterating 40000x each
  2.890000   0.780000   3.670000 (  3.170657)
Garbage collector ran 0 times
Testing ConcurrentStack with 1 thread, iterating 1000000x each
  2.840000   0.000000   2.840000 (  2.836545)
Garbage collector ran 54 times
Testing ConcurrentStack with 5 threads, iterating 200000x each
  2.850000   0.010000   2.860000 (  2.871540)
Garbage collector ran 54 times
Testing ConcurrentStack with 25 threads, iterating 40000x each
  2.880000   0.000000   2.880000 (  2.881745)
Garbage collector ran 56 times
Testing RecyclingConcurrentStack with 1 thread, iterating 1000000x each
  3.910000   0.000000   3.910000 (  3.906049)
Garbage collector ran 0 times
Testing RecyclingConcurrentStack with 5 threads, iterating 200000x each
  3.900000   0.000000   3.900000 (  3.910919)
Garbage collector ran 0 times
Testing RecyclingConcurrentStack with 25 threads, iterating 40000x each
  3.960000   0.000000   3.960000 (  4.024026)
Garbage collector ran 0 times

With jruby 1.6.5 (ruby-1.8.7-p330) I encountered a deadlock. With JRruby it is reproduced each time I ran the benchmark, but with MRI I've seed it only once so far. I will try to find the cause and post an update here.

UPDATE

I reviewed the implementation of concurrent stacks and did not found anything that may cause deadlocks or other issues. Looks like deadlock is caused by benchmarking code. With JRuby it just hangs (very frequently). With MRI I received 'deadlock detected' exceptions:

Testing ConcurrentStack with 25 threads, iterating 40000x each
/home/alex/Projects/read-write-lock/concurrent-stack.rb:143:in `join': deadlock detected (fatal)
    from /home/alex/Projects/read-write-lock/concurrent-stack.rb:143:in `block (2 levels) in test_with_threads'
    from /home/alex/Projects/read-write-lock/concurrent-stack.rb:143:in `each'
    from /home/alex/Projects/read-write-lock/concurrent-stack.rb:143:in `block in test_with_threads'
    from /usr/share/ruby-rvm/rubies/ruby-1.9.2-p290/lib/ruby/1.9.1/benchmark.rb:295:in `measure'
    from /home/alex/Projects/read-write-lock/concurrent-stack.rb:141:in `test_with_threads'
    from /home/alex/Projects/read-write-lock/concurrent-stack.rb:120:in `test'
    from /home/alex/Projects/read-write-lock/concurrent-stack.rb:150:in `<top (required)>'
    from -e:1:in `load'
    from -e:1:in `<main>'

And

/home/alex/Projects/read-write-lock/concurrent-stack.rb:143:in `join': deadlock detected (fatal)
    from /home/alex/Projects/read-write-lock/concurrent-stack.rb:143:in `block (2 levels) in test_with_threads'
    from /home/alex/Projects/read-write-lock/concurrent-stack.rb:143:in `each'
    from /home/alex/Projects/read-write-lock/concurrent-stack.rb:143:in `block in test_with_threads'
Testing RecyclingConcurrentStack with 1 thread, iterating 1000000x each
    from /usr/share/ruby-rvm/rubies/ruby-1.9.2-p290/lib/ruby/1.9.1/benchmark.rb:295:in `measure'
    from /home/alex/Projects/read-write-lock/concurrent-stack.rb:141:in `test_with_threads'
    from /home/alex/Projects/read-write-lock/concurrent-stack.rb:118:in `test'
    from /home/alex/Projects/read-write-lock/concurrent-stack.rb:152:in `block in <top (required)>'
    from /home/alex/Projects/read-write-lock/concurrent-stack.rb:151:in `loop'
    from /home/alex/Projects/read-write-lock/concurrent-stack.rb:151:in `<top (required)>'
    from -e:1:in `load'
    from -e:1:in `<main>'

As far as I can see, the only thing that may cause deadlock is situation when main thread does QUEUE.broadcast and then does Thread#join on the thread but this thread did not executed QUEUE.wait yet. Then main thread infinitely waits for this thread and MRI may detect such deadlock situation.

When I increased sleep time from 0.001 to 0.01 and more it seemed that deadlock has gone. Looks like this issue is caused by long thread initialization time - sometimes it cannot start block execution for all threads before QUEUE.broadcast call.

Also, as a small improvement, the following code may be extracted into private function like cache_node to improve pop method readability:

FREE_LIST.update do |current|
  node.next = current
  node
end

Stack implementation looks correct and clear, I did not found other issues.

With sleep == 0.05 benchmarks for JRuby are these:

# jruby 1.6.5 (ruby-1.8.7-p330) (2011-10-25 9dcd388) (Java HotSpot(TM) Server VM 1.6.0_26) [linux-i386-java]
Testing ThreadSafeStack with 1 thread, iterating 1000000x each
  2.881000   0.000000   2.881000 (  2.881000)
Testing ThreadSafeStack with 5 threads, iterating 200000x each
  2.658000   0.000000   2.658000 (  2.658000)
Testing ThreadSafeStack with 25 threads, iterating 40000x each
  2.885000   0.000000   2.885000 (  2.885000)
Testing ConcurrentStack with 1 thread, iterating 1000000x each
  2.142000   0.000000   2.142000 (  2.142000)
Testing ConcurrentStack with 5 threads, iterating 200000x each
  1.084000   0.000000   1.084000 (  1.084000)
Testing ConcurrentStack with 25 threads, iterating 40000x each
  0.969000   0.000000   0.969000 (  0.970000)
Testing RecyclingConcurrentStack with 1 thread, iterating 1000000x each
  2.628000   0.000000   2.628000 (  2.628000)
Testing RecyclingConcurrentStack with 5 threads, iterating 200000x each
  1.436000   0.000000   1.436000 (  1.436000)
Testing RecyclingConcurrentStack with 25 threads, iterating 40000x each
  1.473000   0.000000   1.473000 (  1.473000)


MVC Model validation callback I have made a simple MVC framework of my own for my personal website to learn a thing or two about how this whole thing even works. I think I've got the idea, but there's one thing I'm not sure about.

Now I know that a model shouldn't need anything specific - everything has to be generalized so it's accessible from multiple controllers which do something different.

My main issue, though is: How the hell do I tell the model to save something the way I want it to? If it's generalized, it should have no information about any save table, any output message or any saving pattern to begin with.

Then I remembered that there's this new fancy thing in PHP called closures and this might be a good way to use them. I have decided to deal with validation in the model in this fashion:

Controller:

private function sendMail($data)
{
    $rebuilt['name'] = array("type" => "string", "value" => $data['name'], "required" => true);
    $rebuilt['email'] = array("type" => "email", "value" => $data['email'], "required" => true);
    $rebuilt['subject'] = array("type" => "string", "value" => $data['subject'], "required" => false);
    $rebuilt['message'] = array("type" => "string", "value" => $data['message'], "required" => true);

    $callback = function() use (&$rebuilt) {
        $headers   = array();
        $headers[] = "MIME-Version: 1.0";
        $headers[] = "Content-type: text/plain; charset=utf-8";
        $headers[] = "From: {$rebuilt['name']['value']} <{$rebuilt['email']['value']}>";
        $headers[] = "Subject: {$rebuilt['subject']['value']}";
        $headers[] = "X-Mailer: PHP/".phpversion();

        mail("some@email", $rebuilt['subject']['value'], $rebuilt['message']['value'], implode("\r\n", $headers));
    };

    $this->model->validateInput($rebuilt, $callback, 'contact', "SuccessEmailReceived", 'contact');
}

Model validateInput() method:

public function validateInput(&$validation, $function, $failAnchor = 'top', $successMessage = "GeneralActionSuccessful", $successAnchor = 'top')
{
    foreach ($validation as $key=>$value) {

        // trim the string before validation
        trim($validation[$key]['value']);

        if($value['required'] == true && !$value['value']) {
            $this->error = "ErrorFieldMissing";
            $this->anchor = $failAnchor;
            $this->formFields = $validation;
            return;
        }

        switch($value['type']) {
            case 'email':
                if(!filter_var($value['value'], FILTER_VALIDATE_EMAIL)) {
                    $this->error = "ErrorInvalidEmail";
                    $this->anchor = $failAnchor;
                    $this->formFields = $validation;

                    return;
                }

                break;
        }
    }
    // success, call the function passed
    $function();
    $this->success = $successMessage;
    $this->anchor = $successAnchor;
}

Basically, what this does, is the controller calls the model validateInput() method while passing the list of data for validation as a reference. An anonymous callback function, which also uses the same validation input by reference, is then called in the model. This way, no matter how the validator deals with the input array, the function behaves accordingly.

Please note that I only have email validation set up right now, because I don't need anything else. I just need to know if I'm doing things right, or if there's something I should re-learn.

What is Model in MVC?

  Now I know that a model shouldn't need anything specific - everything has to be generalized so it's accessible from multiple controllers which do something different.

A model is not a generalized concrete class, but a virtual layer that contains all business logic which in a good architecture would also contain it's own layers (application, domain and infrastructure) for more abstraction.

How to do specific things with a generalized model concrete class?

  My main issue, though is: How the hell do I tell the model to save something the way I want it to? If it's generalized, it should have no information about any save table, any output message or any saving pattern to begin with.

Because it's not generalized (as I just explained above) this problem should now be solved.

Controller

The following diagram might give you a better visual picture of the (one of the possible)  data flow in a MVC structure.

Do note that with the orthodox MVC pattern the Controller is not supposed to call or instantiate any views, but solely control the Model layer, nothing else. However, over the years this has changed due to I don't know what. But nowadays what you'll mostly see and hear is that a Controller's job is to control the model layer and call / return the appropriate view.

Your Case

I see you have a private sendMail() method in your Controller. Well, that doesn't belong there. In your model layer (specifically the application layer) I would have a class like EmailService that would contain this sendMail() method.

In your Controller you can now just get the needed parameters from a http request object and call $this->emailService->sendMail($data); or something of sorts. And finally call the appropriate view, if any (if you follow the contemporary MVC pattern that is).


Faster Iteration Functions I am trying to build a simple C#/.Net console application that runs three iterations (see code below). The process runs as intended, but because of how many iterations there are, it can run upwards of 15 to 20 minutes. I intend to run through a list of nodes that would require individual simulations. Therefore requiring iterations for each node. Maybe 50 nodes.

I am wondering if there is a better method to run these iterations. I have just started reviewing Parallel.For() functions, but have not tested. And I have not seen many examples of chained Parallel.For() functions.

I am thinking the code below requires better iteration coding and a bit of multi-threading to really see an improvement in speed. Any suggestions? Is there anything glaring that looks wrong?

Here is the code:

static void Sim()
{

    OE oe = new OE(); // node-path functionality

    string ftC = "x"; // first call
    double fC = 4; // fourth call
    double tTime = 252; // days remaining
    double calendar = 365; // full calendar days
    double rate = .0065; /// interest rate
    double sC = 1; // secondary call
    double tC = 3.897; // third call

    string path = @"c:\temp\output.csv"; // export

    //tC settings
    double i;
    double nIncrement = .05;
    double nMin = 2;
    double nMax = 5;

    // sC settings
    double vi;
    double nvIncrement = .005;
    double nvMin = .10;
    double nvMax = .50;

    // time settings
    double ti;
    double ntIncrement = 5;
    double ntMin = 1;
    double ntMax = tTime + 4;
    //double ntMax = 30;

    // begin basic increments
// where I think I can find improvement in code
    for (i = (tC - nMin); i < nMax + (tC + nIncrement); i += nIncrement) // tC loop
    {
        double nP = Math.Round(i, 2);

        for (vi = (sC - nvMin); vi < (sC + nvMax) + nvIncrement; vi += nvIncrement) // sCatility loop
        {
            double nV = Math.Round(vi, 4);

            for (ti = ntMin; ti <= ntMax; ti += ntIncrement) // time loop
            {
                double nT = ti;

                double variableResult = Math.Round(oe.Solver(ftC, fC, nP, nV, nT, calendar, rate, 4), 5);

                // output code here
            }
        }

    }
}

Since this is code review, I would like to point out that your variable naming scheme makes your code hard to read. Why abbreviate anything? Why not call fC as just fourthCall? etc.


Namespacing jQuery/jQueryUi into markup that I don't control I have a project that I've been trying to get just right for the past three months and it's still not quite there yet.

I'm injecting some jQuery and jQueryUI code into pages that I have no control over and I need to be able to "sandbox" my code away from anything that may or may not be there at DOM-ready, or at anytime there after...

I think I've got it, but I know there's most likely some edge cases and race conditions that I'm missing. Please fork this and tell me what I've missed.  I'm tired of spinning my wheels on this issue.

Markup:

<!DOCTYPE html>

    <link rel="stylesheet" href="style.css" />
    <link rel="stylesheet" href="//ajax.googleapis.com/ajax/libs/jqueryui/1.10.4/themes/smoothness/jquery-ui.css" />

Loading multiple versions of jQuery!

Some other html

JavaScript:

;(function($, jQuery){

  var $h2 = $('
').append('Namespaced $: ' + $.fn.jquery + ' | $.ui: ' + $.ui.version);
  document.write($h2[0].outerHTML);

  var $jh2 = $('
').append('Namespaced jQuery: ' + jQuery.fn.jquery + ' | jQuery.ui: ' + jQuery.ui.version);
  document.write($jh2[0].outerHTML);

  //Verify that jQueryUi works on OUR version of jQuery:
  $(document).ready(function(){
    console.log($('h2.namespaced').draggable());
  });

})(myJQuery, myJQuery);

I've worked for a media company where ads had to run with their own version of JavaScript libraries, in the end we preferred to let ads run in their own iframe. Especially since some ads required access to document.write which creates havoc to your content ;)

It is a bit scary that your solution has to modify jquery-ui.js, I would be very (very) hesitant to do that. I also dont understand why you employ })(myJQuery, myJQuery); there, since you know you only need 1 of the parameters to initialize jQuery UI.

The worst part is that I played around a bit with a fork and I could not get to a superior solution easily, so if this works for you, then perhaps you should stop researching and just go forth ;)


Merge three ByteArrays together and then split resulting ByteArrays I have a ByteArray value as avroBinaryValue , Schema Id value as short schemaId and Last Modified Date value as lastModifiedDate in long.

short schemaId = 32767;
long lastModifiedDate = "1379811105109L";
byte[] avroBinaryValue = os.toByteArray();

Now, I will write schemaId , lastModifiedDate and avroBinaryValue together into a single ByteArray and then deserialize that final ByteArray value to extract schemaId , lastModifiedDate and avroBinaryValue from it.

Below is the code, I have got so far...

public static void main(String[] args) throws Exception {

    String os = "whatever os is";
    byte[] avroBinaryValue = os.getBytes();

    long lastModifiedDate = 1379811105109L;
    short schemaId = 32767;

    ByteArrayOutputStream byteOsTest = new ByteArrayOutputStream();
    DataOutputStream outTest = new DataOutputStream(byteOsTest);
    outTest.writeShort(schemaId); // first write schemaId
    outTest.writeLong(lastModifiedDate); // second lastModifiedDate
    outTest.writeInt(avroBinaryValue.length); // then attributeLength
    outTest.write(avroBinaryValue); // then its value

    byte[] allWrittenBytesTest = byteOsTest.toByteArray();

    DataInputStream inTest = new DataInputStream(new ByteArrayInputStream(allWrittenBytesTest));

    short schemaIdTest = inTest.readShort();

    long lastModifiedDateTest = inTest.readLong();

    int sizeAvroTest = inTest.readInt();
    byte[] avroBinaryValue1 = new byte[sizeAvroTest];
    inTest.read(avroBinaryValue1, 0, sizeAvroTest);

    System.out.println(schemaIdTest);
    System.out.println(lastModifiedDateTest);
    System.out.println(new String(avroBinaryValue1));

    writeFile(allWrittenBytesTest);
}

I am trying to see whether there is any efficient way of doing this in Java or this is the only correct way of doing it in Java?

The way I am serializing all the three ByteArrays into one ByteArray and the way I am deserializing the resulting ByteArrays to extract the schemaId, lastModifiedDate, avroBinaryValue looks correct or not?

The way you are doing it is fine, however it seems like a better idea to just put all of the data you want to serialize into a Serializable object and then just serialize that automatically. For example you could do something like :

public static void main(String[] args) throws IOException,
    ClassNotFoundException {
  String os = "whatever os is";
  long lastModifiedDate = 1379811105109L;
  short schemaId = 32767;
  Thing myData = new Thing(os, lastModifiedDate, schemaId);

  ByteArrayOutputStream byteOsTest = new ByteArrayOutputStream();
  ObjectOutputStream outTest = new ObjectOutputStream(byteOsTest);
  outTest.writeObject(myData);

  byte[] allWrittenBytesTest = byteOsTest.toByteArray();

  ObjectInputStream inTest = new ObjectInputStream(new ByteArrayInputStream(
      allWrittenBytesTest));
  Thing myDataTest = (Thing) inTest.readObject();

  short schemaIdTest = myDataTest.getSchemaId();
  long lastModifiedDateTest = myDataTest.getLastModifiedDate();
  String avro = myDataTest.getAvro();

  System.out.println(schemaIdTest);
  System.out.println(lastModifiedDateTest);
  System.out.println(avro);

  writeFile(allWrittenBytesTest);
}

// in its own java file
public final class Thing implements Serializable {

  private final String avro;

  private final long lastModifiedDate;

  private final short schemaId;

  public Thing(String avro, long lastModifiedDate, short schemaId) {
    this.avro = avro;
    this.lastModifiedDate = lastModifiedDate;
    this.schemaId = schemaId;
  }

  public String getAvro() {
    return avro;
  }

  public long getLastModifiedDate() {
    return lastModifiedDate;
  }

  public short getSchemaId() {
    return schemaId;
  }
}


Program to take a number from 0 to 9 as input I'm trying to make a function that requests a number from 0 to 9 to the user and that is robust. I've been looking at code to guide me and most fail to find any of these situations, usually due to the use of cin:

If the user enters spaces before or after the number, the entry is valid
If you end the input (Ctrl+Z in Windows; Ctrl+D on most Unix) an infinite cycle is produced
If the user typed 2ff the entry is valid
The entry is waiting for us to type something because of a line jump

I've made the following code and I don't think it fails with its purpose. I would like to know your opinion and if you believe in any case where it could fail or if I can improve it.
#include <iostream>
#include <string>

using namespace std;

int input_number()
{

    string data = "";
    while (true)
    {

        cout << "Input: ";
        if (!getline(cin, data))
        {
            return -1;
        }
        if (data.length() != 1 || isspace(data[0]))
        {
            cerr << "Invalid number, try again!" << endl;
            continue;
        }
        try
        {
            return stoi(data);
        }
        catch (const exception &e)
        {
            cerr << "Invalid number, try again!" << endl;
            cin.clear();
        }
    }
}

int main()
{

    cout << input_number() << endl;
}

This can be rewritten using regular expression \s*(\d)\D* (zero or more whitespace characters, a single digit, zero or more non digit characters. If I understand correctly, heading and trailing spaces are ok, 2ff is ok but ff2 is not ok and 23 is not ok either.
int input_number() {
  static const std::regex valid_number("\\s*(\\d)\\D*");
  string data;
  std::smatch matches;
  while (true) {
    cout << "Input: ";
    if (getline(cin, data) &&
          std::regex_match(data, matches, valid_number)) {
        return stoi(matches[1]);
    }
    cerr << "Invalid number, try again!" << endl;
  }
}

The exception on parsing should never be thrown, regular expression takes care to feed only valid number as the input. The initial version contained try/catch construct to catch it anyway and assert false but this idea is probably over-engineering.


Android image with rounded corners I've followed this blog to get images with nice pretty rounded corners in Android.

I've managed to strip out a lot of the superfluous arrayAdapter stuff and make it much more simple.

I've largely tweaked this just until I made it work.  Does anyone have any feedback?

class StreamDrawable extends Drawable {

    private final float mCornerRadius;
    private final RectF mRect = new RectF();
    private final BitmapShader mBitmapShader;
    private final Paint mPaint;
    private final int mMargin;

    StreamDrawable(Bitmap bitmap, float cornerRadius, int margin) {
        mCornerRadius = cornerRadius;

        mBitmapShader = new BitmapShader(bitmap, Shader.TileMode.CLAMP, Shader.TileMode.CLAMP);

        mPaint = new Paint();
        mPaint.setAntiAlias(true);
        mPaint.setShader(mBitmapShader);

        mMargin = margin;
    }

    @Override
    protected void onBoundsChange(Rect bounds) {
        super.onBoundsChange(bounds);
        mRect.set(mMargin, mMargin, bounds.width() - mMargin, bounds.height() - mMargin);
    }

    @Override
    public void draw(Canvas canvas) {
        canvas.drawRoundRect(mRect, mCornerRadius, mCornerRadius, mPaint);
    }

    @Override
    public int getOpacity() {
        return PixelFormat.TRANSLUCENT;
    }

    @Override
    public void setAlpha(int alpha) {
        mPaint.setAlpha(alpha);
    }

    @Override
    public void setColorFilter(ColorFilter cf) {
        mPaint.setColorFilter(cf);
    }
}

public void drawit(){
    final int CORNER_RADIUS = 24; // dips
    final int MARGIN = 12; // dips

    final int mCornerRadius;
    final int mMargin;

    final float density = this.getResources().getDisplayMetrics().density;
    mCornerRadius = (int) (CORNER_RADIUS * density + 0.5f);
    mMargin = (int) (MARGIN * density + 0.5f);

    ImageView imgvw = (ImageView) findViewById(R.id.imageView2);

    Bitmap myBitmap = BitmapFactory.decodeResource(this.getResources(), R.drawable.london);

    Bitmap resizedBitmap = Bitmap.createScaledBitmap(myBitmap, imgvw.getWidth(), imgvw.getHeight(), false);

    StreamDrawable d = new StreamDrawable(resizedBitmap, mCornerRadius, mMargin);
    imgvw.setBackground(d);
}

This looks very clean to me. Except perhaps:

imgvw - that's disemvoweling. nd t's bd. Not to mention it's breaking your camelCasing convention for locals.
drawit - while the name says what the function does, I would avoid DoThatThing names and perhaps go for a more sober render method name, if not just draw.

Lastly, I don't know Java well enough to know if it's convention, but I wouldn't use that m prefix.


Ruby class to encapsulate Retrying (for instance, with email) One of my clients sends quite a few emails a day ... I would think over 500. They are mostly reports to clients or employees. The volume has become sufficient that intermittent issues with gmail (like the occasional and annoying "please try again later" error, or "too many logins" error) are causing missed reports and headaches. We also have some issues with the MapQuest API and this might help.

So I'm writing this class to encapsulate the process of the immediate retry (as opposed to the "give it back to Resque to try again later" process.)

Dependencies:

Uses my gem Valuable which makes modeling easier.

Feedback welcome.

class Retryer < Valuable
  has_value :attempts, :default => 3
  has_value :logger, default: lambda { Rails.logger }

  has_value :tries, :default => 0

  has_collection :reraise, :default => [NotImplementedError]

  def fire
    raise "You must provide a block of code to try" unless block_given?

    begin
      self.tries += 1
      yield
    rescue Timeout::Error => err
      logger && logger.warn("    - Timeout!")
      sleep(0.5)
      retry if self.tries < self.attempts
    rescue => err
      if reraise.include? err.class
        logger && logger.warn("    - try #{self.tries} of #{attempts} failed and will be reraised: #{err}")
        raise err
      else
        logger && logger.warn("    - try #{self.tries} of #{attempts} failed with #{err}")
        retry if self.tries < self.attempts
      end
    end
  end
end

One thing I'd like to add is the option to run another proc/block if all attempts raise. For instance, notify Rollbar. I imagine it working somewhat like js promises. But I would only do it if I could figure out a readable interface...

Other than the cosmetic suggestion we discussed about removing the unnecessary self prefixes, my central critique is that you're mixing concerns in this code.

Currently, you have specific logging strategies and even logging messages hardcoded in the retryer.  Which will make it hard to use the class in other contexts.

The retryer should do just one thing: retry X number of times and return a result.

Exactly what you want the return values to be is up to you (you could even make a RetryResult class), but one simple option would be an array of results from each attempt: true would be the final array value if it eventually succeeds; and the array would rescued errors from each failed attempt.

The calling code could then decide how to process that information: logging it, sending emails, ignoring it, etc.


Cycle detection in undirected graphs with recursive DFS I wrote a very simple implementation of cycle detection in an undirected graph; I'm not interested in applying it to any real-life case: it's just for explaining the basic idea behind cycle detection in a CS lesson.
I went for recursive DFS, and unlike other implementations I found on the internet, I used just one set for the visited nodes (instead of having a set for the visited nodes and another one for the ancestors):
boolean hasCycleDfs(Node current, Set<Node> visited) {

   if (visited.contains(current)) {
      return true;
   }

   visited.add(current);
   for (Node neighbour: current.neighbors) {
      if (hasCycleDfs(neighbour, visited)) {
         return true;
      }
   }
   visited.remove(current);

   return false;
}

I wrote a couple of tests and they're green:
   @Test
   public void test() {

      List<Node> nodes = IntStream
                .range(1, 8)
                .mapToObj(Node::new)
                .collect(Collectors.toList());

      Node n1 = nodes.get(0);
      Node n2 = nodes.get(1);
      Node n3 = nodes.get(2);
      Node n4 = nodes.get(3);
      Node n5 = nodes.get(4);
      Node n6 = nodes.get(5);
      Node n7 = nodes.get(6);

      n1.add(n2).add(n3);
      n2.add(n4);
      n3.add(n4).add(n6);
      n4.add(n6).add(n7);
      n5.add(n1);
      n6.add(n5);
      assertTrue(hasCycleDfs(n1, new HashSet<>()));

      cleanNodes(nodes);
      n1.add(n2);
      n2.add(n3);
      n3.add(n4);
      n4.add(n1);
      assertTrue(hasCycleDfs(n1, new HashSet<>()));

      cleanNodes(nodes);
      n1.add(n2).add(n5).add(n3);
      n2.add(n3).add(n5);
      n3.add(n4).add(n5);
      n4.add(n5);
      assertFalse(hasCycleDfs(n1, new HashSet<>()));

      cleanNodes(nodes);
      n1.add(n2).add(n3);
      n2.add(n3);
      assertFalse(hasCycleDfs(n1, new HashSet<>()));
   }

   void cleanNodes(List<Node> nodes) {
      nodes.forEach(Node::clearNeighbours);
   }

This is the Node class:
class Node {
   int val;
   Set<Node> neighbors = new HashSet<>();

   Node(int val) {
      this.val = val;
   }

   public Node add(Node child) {
      neighbors.add(child);
      return this;
   }

   public void clearNeighbours() {
      neighbors.clear();
   }

   @Override
   public String toString() {
      return "[" + val + "]";
   }

   @Override
   public boolean equals(Object o) {
      Node node = (Node) o;
      return val == node.val;
   }

   @Override
   public int hashCode() {
      return val * 31;
   }
}

Do you see any edge case I didn't take into account that will give a wrong answer?

The code looks clean and organized, but there is an issue with the implementation.
Undirected graph
Consider an undirected graph A - B:

B is neighbor of A
A is neighbor of B

Using the current implementation I would create the graph like this:
Node a = new Node(1);
Node b = new Node(2);

a.add(b);
b.add(a);

Such graph is not cyclic, but the method hasCycleDfs returns true.

unlike other implementations I found on the internet, I used just one
set for the visited nodes (instead of having a set for the visited
nodes and another one for the ancestors)

The reason for the set of ancestors is to handle this case.
Testing

Is good practice to have one method for test case, instead of a single method with all the tests. It will help you pinpoint the failing test quicker without having to look at the code.
Once you have multiple @Test methods, you can clean the state in a method annotated with @Before (@BeforeEach in Junit5) or simply recreate the graph from scratch every time.

Minor improvements

The methods of Node are public unlike the class and instance variables. Use the access modifiers consistently.
In a non‐academic context, the method hasCycleDfs should be private as it needs to be called with and empty set:
boolean hasCycleDfs(Node current) {
    return hasCycleDfs(Node current, new HashSet<>());
}
private boolean hasCycleDfs(Node current, Set<Node> visited) {
    //...
}

Rewrite
Actually, there is no need for a set of ancestors, a pointer to the parent is enough:
boolean hasCycleDfs(Node current, Set<Node> visited, Node parent) {
    visited.add(current);
    for (Node neighbour : current.neighbors) {
        if (!visited.contains(neighbour)) {
            if (hasCycleDfs(neighbour, visited, current)) {
                return true;
            }
        } else if (!neighbour.equals(parent)) {
            // If the node is visited and not parent of
            // the current node, then there is a cycle
            return true;
        }
    }
    return false;
}

And you can use it like:
hasCycleDfs(n1, new HashSet<>(), null);


(Leetcode) 24 game in Python This is a Leetcode problem -

  You have 4 cards each containing a number from 1 to 9. You need to
  judge whether they could be operated through *, /, +, -, (,)
  to get the value of 24.

  Note -

  1. The division operator / represents real division, not integer division. For example, 4 / (1 - 2/3) = 12.

  2. Every operation done is between two numbers. In particular, we cannot use - as a unary operator. For example, with [1, 1, 1, 1]
  as input, the expression -1 - 1 - 1 - 1 is not allowed.

  3. You cannot concatenate numbers together. For example, if the input is [1, 2, 1, 2], we cannot write this as 12 + 12.

Here is my solution to this challenge -

class Solution(object):
    def judge_point_24(self, nums):
        """
        :type nums: List[int]
        :rtype: bool
        """
        if len(nums) == 1:
            return nums[0] == 24 or abs(nums[0] - 24.0) <= 0.1
        for i in range(len(nums)):
            for j in range(len(nums)):
                if i == j:
                    continue
                indexes = set([x for x in range(len(nums))])
                indexes.remove(i)
                indexes.remove(j)
                operations = []
                operations.append(float(nums[i]) + float(nums[j]))
                operations.append(float(nums[i]) - float(nums[j]))
                operations.append(float(nums[j]) - float(nums[i]))
                operations.append(float(nums[i]) * float(nums[j]))
                if nums[j] != 0:
                    operations.append(float(nums[i] / float(nums[j])))
                if nums[i] != 0:
                    operations.append(float(nums[j] / float(nums[i])))
                next_items = [nums[index] for index in indexes]
                for x in operations:
                    if self.judge_point_24(next_items + [x]) == True:
                        return True
        return False

Explanation - My solution is to reduce the nums size until it becomes the size of one. Once it is the size of one, check if we have reached 24 or close enough. Basically, I tried to pick two elements from the array and apply all the possible computations and recursively call itself now with the array that has the result of the computation and the rest. As soon as I see True from the call, I return True as well.

Here are some example outputs -

#output = Solution()

#print(output.judge_point_24([4, 1, 8, 7]))

>>> True

#Explanation - (8-4) * (7-1) = 24

#output = Solution()

#print(output.judge_point_24([1, 2, 1, 2]))

>>> False

Here are the times taken for these outputs -

%timeit output.judge_point_24([4, 1, 8, 7])
2.16 ms ± 46 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

%timeit output.judge_point_24([1, 2, 1, 2])
43 ms ± 902 µs per loop (mean ± std. dev. of 7 runs, 10 loops each)

Here is my Leetcode result -

So, I would like to know whether I could make this program shorter and more efficient.

Any help would be highly appreciated.

Docstring

Is nums really of type List[int]?  Perhaps the first time it is, but after the first step, it is going to turn into a List[float].

Efficiency

Assuming nums is a list of numbers (int or float), you never need to call float() on any of the values.  If you expect a list of 4 strings ("1", through "9") from the contest input, turn them into numbers using int(_) and then pass the List[int] to your judge function.

The test nums[0] == 24 or abs(nums[0] - 24.0) <= 0.1 could be simplified to just abs(nums[0] - 24.0) <= 0.1.

Your doing a lot more work than you think you are.  This code is looping through your list, finding pairs of values:

    for i in range(len(nums)):
        for j in range(len(nums)):
            if i == j:
                continue

For each pair, i & j, you compute all possible combinations of those values, including reverse subtraction and reverse division.

And later, you'll encounter the same pair only as j & i!  And you'll compute all possible combinations of those values a second time.

If your inner loop started at one index after the current outer loop index, then you'd never generate a pair of indices with i > j ... or even i == j, so you can omit that test as well.  Finally, since the inner loop starts at 1 index above the outer loop, the outer loop should end one index early:

    for i in range(len(nums)-1):
        for j in range(i+1, len(nums)):

You could also use enumerate( ) to loop over the values along with their indices at the same time.

    for i, a in enumerate(nums[:-1]):
        for j, b in enumerate(nums[i+1:], i+1):

And then you can refer to a instead of nums[i], and b instead of nums[j], which will prevent repeated indexing into the nums[] array, which will result in decreased performance.

Management of remaining values:

            indexes = set([x for x in range(len(nums))])
            indexes.remove(i)
            indexes.remove(j)
            next_items = [nums[index] for index in indexes]

First set([x for x in range(len(nums))]) could simply be set(range(len(nums))).  There is no need for the list comprehension.

But this is still a lot of extra work.  Simply copy the numbers to a new list, and then delete the i-th & j-th entries.  Since i < j is guaranteed, deleting the j-th element first won't change the location of the i'th element:

            next_items = nums[:]
            del next_items[j]
            del next_items[i]

Or, build up the new list with just the elements you want by omitting the i-th and j-th elements:

            next_items = nums[:i] + nums[i+1:j] + nums[j+1:]

2+2 == 2*2 and 1*1 == 1/1 == reverse_div(1,1).  It is a small optimization, but instead of making a list of all possible operations values, you could make a set() of the values, which would eliminate some redundant recursive steps.

If you don't need this to be a class as a requirement of the leetcode automated judging, you can gain some efficiency by not passing an extra self argument all the time.  This can simply be a function.

Reworked Code

def judge_point_24(nums):

    for i, a in enumerate(nums[:-1]):
        for j, b in enumerate(nums[i+1:], i+1):
            operations = {a+b, a*b, a-b, b-a}
            if a:
                operations.add(b/a)
            if b:
                operations.add(a/b)

            if len(nums) > 2:
                next_items = nums[:i] + nums[i+1:j] + nums[j+1:]
                for x in operations:
                    if judge_point_24(next_items + [x]) == True:
                        return True
            else:
                return any(abs(x-24) < 0.1 for x in operations)

    return False

if __name__ == '__main__':

    def test(expected, nums):
        print(nums, judge_point_24(nums))
        assert judge_point_24(nums) == expected

    test(True, [4, 1, 8, 7])
    test(False, [1, 2, 1, 2])


Merge consecutive numbers in array into a range Input: I have sorted array of numbers [ 12, 15, 16, 17, 19, 21, 22 ]

Output: I need to merge all consecutive numbers into a dash-separated ranges like this "12, 15-17, 19, 21, 22"

I came up with such code (jsbin - https://jsbin.com/dusuvihiko/edit?js,console):

const input = [ 12, 15, 16, 17, 19, 21, 22 ];

const output = input.reduce((acc, curr, i, arr) => {
  if (curr - 1 === arr[i - 1] || i === 0) {
    acc.currSet.push(curr);
  } else {
    if (acc.currSet.length > 2) {
      acc.out.push(`${acc.currSet.shift()}-${acc.currSet.pop()}`);
    } else {
      acc.out.push(acc.currSet.join(','));
    }
    acc.currSet = [curr];
  }
  if (i === arr.length - 1) {
    if (acc.currSet.length > 2) {
      acc.out.push(`${acc.currSet.shift()}-${acc.currSet.pop()}`);
    } else {
      acc.out.push(acc.currSet.join(', '));
    }
  }
  return acc;
}, {
  out: [],
  currSet: [],
})
console.log(output.out.join(', '));

But I can see it looking ugly and not really optimized (code duplicating)

I would appreciate any suggestions on how to improve the code and make it look better and possibly work faster

Optimization review

Q1

  But I can see it looking ugly and not really optimized (code duplicating)

Ugly is somewhat subjective (2 spaced indents are ugly in my book), and sometimes unavoidable, as long as it is readable then looks don't matter.

Code duplication is not a code optimization problem, in fact it is often code duplication that creates the optimal performance. Duplication is a source code management / maintenance issue. Case in point 10th line the join string missing a space while the duplicated line 18 has the space. Duplication is a easy place to introduce a bug, it is a place where the brain will say "same" for what is just a little different

Duplication is simple to rectify using functions.

You have two identical blocks of code (lines 7-11 and 15-19). You can wrap those lines in a function and call the function rather than have the code inline. You may opt to pass the function the arguments or use closure to hold the variable and access them directly.

function collapseSequences(input){
   function pushResult(acc){
      const cs = acc.currSet;
      acc.out.push(cs.length > 2 ?
          `${cs.shift()}-${cs.pop()}` :
          cs.join(',')
      );
  }

  const output = input.reduce((acc, curr, i, arr) => {
      if (curr - 1 === arr[i - 1] || i === 0) {
        acc.currSet.push(curr);
      } else {
        pushResult(acc);
        acc.currSet = [curr];
      }
      if (i === arr.length - 1) {
        pushResult(acc);
      }
      return acc;
    }, { out: [], currSet: [] }
  );
  return output.out.join(', ');
}

Q2

  I would appreciate any suggestions on how to improve the code and make it look better and possibly work faster.

As a function

Always use a function to wrap any code. Code running in global scope is inherently slower due to how JS manages variables. The whole code should look more like

function collapseSequences(input) {
    ... the code

    // return the result
    return output.out.join(', ');
}

console.log(
    collapseSequences([ 12, 15, 16, 17, 19, 21, 22 ])
);

Use strict mode

Adding the run-time directive "use strict"; to the first line of a function or first line of a script allows the execution environment to make some assumptions and take some shortcuts improving performance (also forces you to write better Javascript)

Iteration

Loops are faster than iterators

const acc = foo;
for(const x of values) { ... code }

// or

const acc = foo;
for(let i = 0; i <values.length; i ++){ const x = values[i]; ...code }

will always be faster than

values.reduce((acc, x)=>{ ...code }, foo );

Strings

Strings are quicker than arrays. A common pattern is to use arrays when a string result requires a separator.

    // CPU benchmark 4.6 lower is faster
    function test(values){
        const res = [];
        for(const x of values){
            res.push(x * 10);
        }
        return res.join(", ");
    }

In terms of memory use GC impact, and CPU use the following can do the same in ~70% the time and much less memory.

    // CPU benchmark 3.8 lower is faster
    function test(values){
        var result = "";
        var join = "";
        for(const x of values){
            result += join + (x * 10);
            if (!join) { join = ", " }
        }
        return result;
    }

Growing a string is quicker than growing an array.
A string uses less memory than an array.
Array.join requires an additional iteration of items to build the string.

If you can, use a string rather than an array.

Benchmarking

If you want to write fast code then it is well worth investing time to learn how to correctly benchmark JS .

Bench-marking is one of the most difficult JS  coding skills to master.

Assuming code is faster based on time complexity is a common performance trap in JS . There is not always a 1 to 1 relation between bench-marked performance, and apparent time complexity. This is because complexity can be hidden in the native code.

JS performance is not always intuitive, nor can you assume that optimizations that worked yesterday still work today, or that slower code yesterday is still slower today. Browsers can update daily and updates do not always mean faster (New language features are particularly dynamic in terms of performance between updates)

Rewrite

Taking in the above performance points the function can be rewritten as

function collapseSequences(arr) {
    var i, val;
    var idx = 0;            // Array index
    var str = "";           // The result string
    const len = arr.length; // This is for readability not performance

    // The loop is terminated in its body to manage the seperator
    while (true) {
        i = 0;
        str += (val = arr[idx++]);

        // Look ahead for a sequence
        while(idx + i < len && arr[idx + i] === val + i + 1 ) {
            i++;
        }
        // Was here a sequence found ?
        if (i > 1) {        // add it to the string and skip over the sequence
            str += "-" + arr[(idx += i) - 1];
        }
        if (idx === len ) { // All done then return result
            return str;
        }
        str += ", ";        // add the separator
    }
}

Resulting in a 3 fold improved execution time.

Bench-marking details.

Tests results show mean execution time on same data sets

25,793µs ±1,853µs. Using strings
30,418µs ±1,346µs. Using arrays and array.join
79,534µs ±1,142µs. Your code

The test functions run on Win10 Firefox, run in strict mode.

//===============================================================
// Test 1 Using strings. 25,793µs ±1,853µs
function collapseSequences(arr){
    var i, idx = 0, str = "", val, len = arr.length;
    while (idx < len) {
        i = 0;
        str += (val = arr[idx++]);
        while(idx + i < len && arr[idx + i] === val + i + 1 ) { i++ }
        if (i > 1) { str += "-" + arr[(idx += i) - 1] }
        if (idx === len ) { return str }
        str += ", "
    }
}

//===============================================================
// Test 2 using arrays and array.join. 30,418µs ±1,346µs.
function collapseSequences(arr){
    var i, idx = 0, val, len = arr.length;
    const res = [];
    while (idx < len) {
        i = 0;
        val = arr[idx++];
        while(idx + i < len && arr[idx + i] === val + i + 1 ) { i++ }
        res.push(val +  (i > 1 ? "-" + arr[(idx += i) - 1] : "") );
    }
    return res.join(", ")
}

//===============================================================
// Test 3 OP original. 79,534µs ±1,142µs.
function collapseSequences(input){
    const output = input.reduce((acc, curr, i, arr) => {
      if (curr - 1 === arr[i - 1] || i === 0) {
        acc.currSet.push(curr);
      } else {
        if (acc.currSet.length > 2) {
          acc.out.push(`${acc.currSet.shift()}-${acc.currSet.pop()}`);
        } else {
          acc.out.push(acc.currSet.join(','));
        }
        acc.currSet = [curr];
      }
      if (i === arr.length - 1) {
        if (acc.currSet.length > 2) {
          acc.out.push(`${acc.currSet.shift()}-${acc.currSet.pop()}`);
        } else {
          acc.out.push(acc.currSet.join(', '));
        }
      }
      return acc;
    }, {
      out: [],
      currSet: [],
    })
    return output.out.join(', ');
}


Fetching and displaying the provisioning status of a phone number I have the following function:

def dnd_return(number,my_timeout):
    try:
        r =requests.get("https://dndcheck.p.mashape.com/index.php?mobilenos= {}".format(number), timeout =my_timeout)
    except (requests.exceptions.Timeout, requests.exceptions.ConnectionError) as err:
        return "Error fetching DND info"
    else:
        return r.text

Then I have the following route:

@app.route('/track_mobile_numbers', methods=['POST','GET'])
def mobile():
    if request.method == 'POST':
        if request.form.get("number", "") != "":
            number = request.form['number']
            number = number.replace(" ","")
            number = number.replace('+91', '')
            number = number.strip()
            strnumber = number
            dnd_list = []
            if len(number) == 10:
                try:
                    dnd_dump = dnd_return(number,9)
                    json_dump = json.loads(dnd_dump)
                    j = json_dump[0]

                    if j['DND_status'] != "deactivated" and j['DND_status'] != "off":
                        dnd_list.append("DND status : "+j['DND_status'])
                        dnd_list.append("Date of Activation : "+j['activation_date'])
                        dnd_list.append("Preference : "+j['current_preference'])

                    else:
                        dnd_list.append("DND status : "+j['DND_status'])
                except Exception as e:
                    dnd_list.append(e)
            else:
                dnd_list.append("The number should contain 10 digits")

            number = number[:4]
            try:
                number = int(number)
                num = mobileNumbers.query.filter_by(number = number).first()
                return render_template('mobilenumbers.html',
                                        strnumber = "Mobile number : " + strnumber,
                                        operator = "Operator : " + num.operator,
                                        state = "State : " + num.state,
                                        dnd = dnd_list
                                      )
            except Exception as err:
                return render_template('mobilenumbers.html',
                                        error = err
                                      )
        else:
            return render_template('mobilenumbers.html',
                                    error = "Please enter a valid number"
                                  )
    else:
        return render_template('mobilenumbers.html')

Is there a better way to do this? Can you recommend improvements to this code?

You may want to use a different template for error messages. You can still reuse the overall design by inheriting from a base template.
You replace variable values a lot, making it hard to follow what the meaning of variables like num, number and strnumber are throughout the code. Instead write functions which create the various transformations of the input that you need, making your variables unchanged throughout the code. For example:

def format_number(number):
    return number.replace(" ","").replace('+91', '').strip()

Now you can get from number to strnumber without ever changing the former.
In some error cases you render the template with error set, but in at least one you use a different error message delivery: dnd_list.append("The number should contain 10 digits"). This seems inconsistent.
The various large branches of code should be moved into separate functions.
Single letter variables like e, r and j make this code hard to maintain. Yes, it's kinda obvious from context, but the less context you have to keep in your head while developing, the better.


Looping through a tree to get branch or leaf This is my solution to a problem from MIT's 6.034 Artificial Intelligence course (see the heading Tree reference on 6.034 Artificial Intelligence, Lab 0). Can you please help me improve my solution (keep in mind I am a beginner with no formal education in Computer Science)? To be clear, I am NOT taking this for school credit, this is an old class from 2010 available through MIT OpenCourseWare. SOURCE

  Your job is to write a procedure that is analogous to list
   referencing, but for trees. This "tree_ref" procedure will take a tree
   and an index, and return the part of the tree (a leaf or a subtree) at
   that index. For trees, indices will have to be lists of integers.
   Consider the tree in Figure 1, represented by this Python tuple: (((1, 2), 3), (4, (5, 6)), 7, (8, 9, 10))

  To select the element 9 out of it,
   we’d normally need to do something like tree[3][1]. Instead, we’d
   prefer to do tree_ref(tree, (3, 1)) (note that we’re using zero-based
   indexing, as in list-ref, and that the indices come in top-down order;
   so an index of (3, 1) means you should take the fourth branch of the
   main tree, and then the second branch of that subtree). As another
   example, the element 6 could be selected by tree_ref(tree, (1, 1, 1)).

  Note that it’s okay for the result to be a subtree, rather than a
   leaf. So tree_ref(tree, (0,)) should return ((1, 2), 3).

My Solution:

def tree_ref(tree, index):
  try:
    index = list(index)
    if not isinstance(tree, tuple) or len(index) == 0:
      return tree
    else:
      return tree_ref(tree[index[0]], index[1:])
  except IndexError:
    return "INPUT OUT OF RANGE"

all in all this code does what it needs to do, but czan be improved in some small points

Exception

Instead of the try-except clause, just let the exception bubble up to the caller if he passes a wrong index. returning a string in this case is almost never the correct way to treat it. Now the caller has to check each response he gets and compare it to  "INPUT OUT OF RANGE". Even returning None would be better than this errorstring

list

index = list(index) is useless. tuples can be sliced too, and you can get the length of them

code

The algorithm becomes very simple. It raises a TypeError or IndexError if the caller passes in something illegal, but that's his responsibility

def tree_ref_recursion(tree, index):
    if index:
        return tree_ref_recursion(tree[index[0]], index[1:])
    else:
        return tree

The if index: just checks whether index is a non-empty tuple. There is no need to check for the type of tree. If this is no more tuple, and there is still an index, the caller passed in an illegal tree or index.

recursion

This was a recursive method. Because of the recursion, you need to check the index. You can also use a loop:

def tree_ref(tree, index):
    for idx in index:
        tree = tree[idx]
    return tree


Simulation of Multiple Producer Consumer I am trying to implement the Multiple Producer-Consumer problem. I have made the buffer as the lock to maintain consistency of the system. Does this system actually implement multithreading? How can I improve the system using wait and notify?

Producer.java

import java.util.*;
import java.io.*;

class Producer extends Thread
{
    private Buffer buffer;
    private String name;

    Producer(Buffer buffer,String name)
    {
        this.buffer = buffer;
        this.name = name;
    }
    @Override
    public void run()
    {
        while(true)
        {
            Random r = new Random();
            int x = r.nextInt(100);
            produce(x);
            try
            {
                //(r.nextInt(6)+1)*
                Thread.sleep(100);
            }
            catch (Exception e)
            {
                e.printStackTrace();
            }
        }
    }
    public void produce(int x)
    {
        synchronized(buffer)
        {

            if(buffer.size()==buffer.cap)
            {
                System.out.println("QF");
            }
            else
            {
                buffer.add(x);
                System.out.println(name+": "+x);
            }
        }
    }
}

Buffer.java

class Buffer
{
    private Vector<Integer> buffer;
    int cap;
    Buffer(int cap)
    {
        buffer = new Vector<Integer>();
        this.cap = cap;
    }
    int size()
    {
        return buffer.size();
    }
    boolean isEmpty()
    {
        return buffer.isEmpty();
    }
    void add(int x)
    {
        buffer.add(x);
    }
    int remove()
    {
        if(buffer.isEmpty())
            return -1;
        int x = buffer.firstElement();
        buffer.remove(buffer.firstElement());
        return x;
    }
}

Consumer.java

class Consumer extends Thread
{
    private Buffer buffer;
    private String name;

    Consumer(Buffer buffer,String name)
    {
        this.buffer = buffer;
        this.name   = name;
    }
    @Override
    public void run()
    {
        while(true)
        {
            consume();
            try
            {
                Random r = new Random();
                Thread.sleep(100);
            }
            catch (Exception e)
            {
                e.printStackTrace();
            }
        }
    }
    public void consume()
    {
        synchronized(buffer)
        {
            int x = buffer.remove();
            if(x==-1)
            {
                System.out.println("QE");
            }
            else
            {
                System.out.println(name+": "+x);
            }
        }
    }
}

App.java

public class App
{
    public static void main(String args[]){
        Buffer b = new Buffer(5);
        Producer p = new Producer(b,"P1");
        Producer p2 = new Producer(b,"P2");
        Consumer c = new Consumer(b,"C1");
        p.start();
        p2.start();
        c.start();
    }
}

The buffer is the resource. So it should take care about the access to itself by itself or another monitor. This monitor should not be the consumer nor the producer. So synchronization should NOT be located in the consumer or the producer.

If you want to omit effort (that will be technical debts) the first approach is to make the resource (in this case the Buffer) a monitor by introducing synchronized as the keyword in the method signatures of add() and remove(). size() and isEmpty() should not be available for clients, make them private. Now the buffer will be thread-safe.

To make use of wait and notify, you have to exactly know what is happening. wait() will suspend the current thread WITHIN a monitor.

A consumer-thread therefore may be suspended with wait() in the method remove() if nothing is in the buffer. A producer may be suspended in the method add() with wait() if he want wants to produce something but the cap is reached.

Both the consumer and the producer should notify all other suspended threads with notifyAll() after they changed the state of the resource. A simple notify() is not applicable here because in your current construct you do not distinguish between locks for consumers and locks for producers. After notifying the "notifiers" must leave the monitor by waiting or exiting the synchronized method (this will be maybe your solution).

A notified thread should first check, what has happend during its absence. So a consumer should check if something is in the buffer. If not then again wait(), consume() otherwise. A producer should check if the cap is reached. If not then produce(), wait() otherwise.

I see two further improvements:

First separate the locking responsibility from the resource. Introduce a "BufferMonitor" class for example delegating add() and remove() to the encapsulated real buffer and make its methods synchronized.

Second introduce additional locks for consumers and producers to be able to distinguish when to wake up one consumer or one producer. notifyAll() on the BufferMonitor will then become a notify() on the specific lock. Because I haven't implemented this scenario yet I do not know how much effort it is. After all it should be possible. But recognize: The BufferMonitor will not be obselete.

One sentence to the remove()-method. I suggest to somehow avoid returning -1 even it SEEMS to be impossible. A solution that does not need to return -1 is the most elegant one and I admit the most difficult. You may need something like a consumption announcement mechanism before calling "remove()". Maybe you take the challenge. But be careful: this is often called "developing gold taps". ;-)

Finally you should think about proper shutdown mechanisms of your threads. So waiting threads should be notified on shutdown and evaluate this shutdown request on their own.


Hello World using Python embedded in C++, with RAII library initialization I have written a small Hello World application that uses Python's C-API. The C library requires some functions to be called for global library (de-)initialization and the objects created by the library eventually need to be released by passing the pointer to a specific function. I think this is a fairly common pattern in C libraries.

I attempted to use RAII, to get automatic cleanup, even when exceptions interfere with the planned control flow.

#include <tr1/memory>
#include <Python.h>

// helper to get function pointer for the macro
void Python_DecreaseRefCount(PyObject* p) {
    Py_DECREF(p);
}

int main() {
    // declare struct and create instance that handles global initialization and
    // deinitialization of Python API
    static struct PythonApi {
        PythonApi() { Py_Initialize(); }
        ~PythonApi() { Py_Finalize(); }
    } python_api;
    // create object
    // use smart pointer for automatic refcount decrement at scope end
    std::tr1::shared_ptr<PyObject> string(
        PyString_FromString("Hello World!\n"),
        &Python_DecreaseRefCount);
    // do something with object
    PyObject_Print(string.get(), stdout, Py_PRINT_RAW);
}

Compiles and runs with:

$ g++ test.cc `pkg-config --cflags --libs python2` && ./a.out
Hello World!

I'm interested in all kinds of feedback, as to whether my attempt makes sense and if there is room for improvement. Are there best practices, or certain idioms that facilitate this, that I might have missed in my approach?

Are you sure also scoping de-initialization of Python to main (by removing static) is not an option?
As-is, you cannot use Python in a globals initializer or destructor anyway.
Having taken a look at Py_Finalize, I actually would not deinitialize it at all, at least not in a release-build, as none of the reasons that option is provided apply to you:

  This function is provided for a number of reasons. An embedding application might want to restart Python without having to restart the application itself. An application that has loaded the Python interpreter from a dynamically loadable library (or DLL) might want to free all memory allocated by Python before unloading the DLL. During a hunt for memory leaks in an application a developer might want to free all memory allocated by Python before exiting from the application

  Bugs and caveats: [... lots ...]

Next, you shouldn't be using a function-pointer, use an empty function-object instead.
Using a function-object facilitates inlining and as it's empty, reduces memory-overhead.
Actually, are you sure you actually need / want a std::shared_ptr?
Either because you are already irretrievably locked into using that type, or because you actually might need weak pointers somewhere?

If the answer is no, look into boost::intrusive_ptr, as that will save you an allocation every time you hand a PyObject to a smart-pointer's oversight, and reduces smartpointer-size by half, to raw-pointer-size.

// Hooks in same scope as PyObject (global scope), so ADL in intrusive_ptr finds them:
void intrusive_ptr_add_ref(PyObject* p) noexcept { Py_INCREF(p); }
void intrusive_ptr_release(PyObject* p) noexcept { Py_DECREF(p); }
// typedef for ease of use:
typedef boost::intrusive_ptr<PyObject> PyPtr;
// Use as:
PyPtr string(PyString_FromString("Hello World!\n"), false);


Viewing test scores from one of three CSV files Is there any way I could tidy this code and make it look better and more appealing?

import csv
math_class = input ("Which math class would you like to see the results of?")
while math_class not in ['1','2','3']:
    math_class = input ("Which math class would you like to see the results of?")
sort = input ("How would you like to sort the results alphabetical, highest or average:\n")
while sort not in ["alphabetical","highest","average"]:
    sort = input ("How would you like to sort the results")
class_file_names = ['Class 1.csv', 'Class 2.csv', 'Class 3.csv']
scan = open(class_file_names[int(math_class)-1], 'r+')
rows = csv.reader(scan)
data = []
next(scan)

for row in rows:
    data.append(row)
scan.close()

if(sort == "alphabetical"):
    word = "- Highest Score on test:"
    sorted_= sorted(data, key=lambda name: name [0])
    field = 5

if(sort == "average"):
    word = "- Average Score"
    sorted_ = sorted(data, key=lambda average: average[4], reverse=True)
    field = 4

if(sort == "highest"):
    word = "- Highest Score on test:"
    sorted_ = sorted(data, key=lambda total: total[5], reverse=True)
    field = 5

for row in sorted_:
    print(str(row[0]), word, str(row[field]))
input ("Sort Complete, press any key to close the program.")

If there is no way I could clean up the code, then that's fine as it does what I want it to (it's just that I thought the code could look more appealing than it already is).

If you see a whole bunch of if/else statements with the only difference being some function, take advantage of the fact that functions are first class objects in python. For instance, you could create a dictionary that maps your sort options to sorting functions; while you're at it, explore the built ins (such as operator) as well:

from operator import itemgetter

sort_options = {
    # itemgetter(0) is equivalent to lambda x: x[0]
    # >>> x = [1, 2, 3]
    # >>> f = itemgetter(0)
    # >>> g = itemgetter(1)
    # >>> f(x)
    # 1
    # >>> g(x)
    # 2
    # sort_option: (key_of_sort, your_word, sort_in_reverse?, your_field)
    'alphabetical': (itemgetter(0), "- Highest Score on test:", False, 5),
    'average': (itemgetter(4), "- Average Score", True, 4),
    'highest': (itemgetter(5), "- Highest Score on test:", True, 5),
}

def custom_sort(data, sort):
    fn, word, rev, field = sort_options[sort]
    sorted_ = sorted(data, key=fn, reverse=rev)
    return word, sorted_, field

word, sorted_, field = custom_sort(data, sort)


Check that elements in list follow a specified date-format schema and are not null/na/nan I'm practicing some data-analysis and I'm currently checking the integrity of my data. The elements that do not follow my date-schema, should be funneled to a new separate list so I can work on strategies and suggestions on how to handle these exceptions.
The following code checks the format successfully:
def datetime_check_format(col):
    tracker = []
    false_dates = []
    true_dates = []
    counter = 0
    for element in col:
        if not isinstance(pd.to_datetime(element, errors="ignore", format= "%d-%m-%Y"), date):
            counter += 1
            true_dates.append(pd.to_datetime(element, errors="raise", format= "%Y-%m-%d"))
        else:
            counter +=1
            false_dates.append(element)
            tracker.append(counter)

    if len(tracker) == 0:
        return "column is ok."
    else:
        return tracker, false_dates, true_dates

I was wondering if someone has an idea on how to make this code better. It seems to be based on some backwards bending mobius-ring kind of twisted logic.
I used this one as guide, since it told me of error-handling:
pandas.to_datetime
best regards,
Jacob Collstrup

One of my friends on Discord helped me out. He didn't like me attempt either! =oP
I created this function:
import datetime

def str2date(string):
   try:
       datetime_holder = datetime.datetime.strptime(string, "%Y-%m-%d")
       return datetime_holder.date()
   except ValueError:
       return string
   except TypeError:
       return string

And looped over it like this:
def datetime_check_format(col):
tracker = []
false_dates = []
true_dates = []
counter = -1
for element in col:
    counter +=1
    if isinstance(str2date(element), date):
        true_dates.append(str2date(element))
    else:
        tracker.append(counter)
        false_dates.append(str2date(element))

return tracker, false_dates, true_dates


Measuring CPU frequency in *nix I have this small function for reading the TSC (time-stamp counter) in *nix. Using it seems to report my CPU frequency more or less correctly.

#include <stdint.h>
#include <stdio.h>
#include <unistd.h>

uint64_t read_time_stamp_counter()
{
    uint32_t my_edx;
    uint32_t my_eax;

    asm ("cpuid\n\t" /* Discourage out-of-order execution. */
         "rdtsc\n\t"
         "movl %%edx, %0\n\t"
         "movl %%eax, %1"
         :"=r"(my_edx), "=r"(my_eax)
         :
         : /* List of registers clobbered by both RDTSC and CPUID. */
           "%eax", "%ebx", "%ecx", "%edx"
         );

    return (((uint64_t) my_edx) << 32) | my_eax;
}

int main(int argc, char** argv) {
    uint64_t tmp;
    uint64_t cycles;
    uint64_t sleep_overhead;

    tmp = read_time_stamp_counter();
    sleep(1);
    cycles = read_time_stamp_counter() - tmp;

    tmp = read_time_stamp_counter();
    sleep(0);
    sleep_overhead = read_time_stamp_counter() - tmp;

    printf("Sleep overhead: %llu cycles.\n", sleep_overhead);

    cycles -= sleep_overhead;

    printf("Cycles: %llu\n", cycles);
    printf("Estimated frequency: %.3f GHz\n", cycles / 1e9);

    return 0;
}

The current implementation does not look good: there should be a better way. Also, is it possible to make it more precise?

From a "How to get the right answer" point of view, David's comment which explains why asking the OS is the only trustworthy solution best answers the question.

But this is Code Review, not Stack Overflow.  From that point of view:

The cpuid instruction returns different data depending on the input value of eax.  Since you never set it before executing your asm, (conceivably) the length of the operation could vary between calls, mucking up your calculation.

Also, the mov statements are unnecessary:

asm ("cpuid\n\t"
     "rdtsc"
     : "=d"(my_edx), "=a"(my_eax)
     : "a"(0)
     : "%ebx", "%ecx");

It might also be worth looking at rdtscp as a replacement for cpuid + rdtsc.


Getting all likes from a specific user on a news wall with fewer graph API calls Right now, my solution counting likes of a specific user is

Starting a recursive call on the "/{user-id}/posts" edge
Iterate through all posts
On each post iterate through each like on the likes edge

If someone knows a better, easier way with less graph API calls necessary, please let me know!

/*
*
*   Count my_user_id likes in wall of user "123"
*
*   ex: url = /123/posts
*
*   setTimeout to avoid exceed api level rate limit, code 613
*/

function findMyLikes(url,my_user_id,count,callback) {
    console.log(url);
    FB.api(url, function (response) {
        if (response.data) {
            response.data.forEach(function (post) {
                //console.log(post);
                if (post.likes) {
                    post.likes.data.forEach(function (like) {
                        if (like.id == my_user_id) {
                            count++;
                        }
                    });
                }
            });
        }

        if (response.paging && response.paging.next) {
            setTimeout(function() {findMyLikes(response.paging.next,my_user_id,count,callback)},1000);
        } else {
            callback(count);
        }
    });

};

Interesting / fun question.

I don't think there is much to improve there to reduce the API call count.

From a once over, these are some things you might want to look at:

forEach is still slower than the good old for loop, not sure that matters for you.
Don't use console.log in production code
Don't keep commented out code in production code
Does it make sense to keep comparing to my_user_id if you already found a match ? It is not like you could like the same post twice ? (One more (better) argument to not use forEach here)
my_user_id -> myUserId -> id , always apply lowerCamelCase plus keep it simple

Not relevant after comment

You say you are using the likes edge in your comment, so you might want to mention that in your example ex: url = /123/posts/likes ?
If post.likes is not set while using the likes edge, then something serious is going on , consider an early 'return callback(count);`
Same thing for if (response.data)


Find kth element from tail of singly linked list in one pass I have tried the below program to find (Kth element from tail) of a singly linked list. I have tried to utilize the length field which I am incrementing whenever an item is added into the list or vice versa.

public Node<T> findKthFromTail(int kthElem) {
    // length has the size of the list
    int kthPos = length - kthElem;

    if (kthPos < 0) {
        return null;
    }

    Node<T> node = head;
    int start = 0;
    while (node != null) {
        if (start == kthPos) {
            // found the node
            break;
        }

        node = node.getNext();
        start++;
    }

    return node;
}

Trust... it's all about trust.

Trust your length is accurate (if it is not, you have other problems).

Then, your code becomes a simple loop:

// length has the size of the list
int kthPos = length - kthElem;

if (kthPos < 0) {
    return null;
}

Node<T> node = head;
while (kthPos-- > 0) {
    node = node.getNext();
}
return node;


Forming sets from pairs of numbers I do not want to use a new data structure but the existing APIs in Java. Here's a solution. I am thinking if this can be made simpler. Input say {"1 2", "4 3", "5 5", "2 3"} and output is [1, 2, 3, 4] [5]

    static void formSet(String[] pairs) {

    List<Set<Integer>> list = new ArrayList<>();
    for(String s : pairs){
        String[] values = s.split(" ");
        Integer val1 = Integer.parseInt(values[0]);
        Integer val2 = Integer.parseInt(values[1]);

        Set<Integer> pairSet = new HashSet<>();
        pairSet.add(val1);
        pairSet.add(val2);

        Set<Integer> val1_set = null, val2_set = null;
        for(Set<Integer> set : list){
            if(set.contains(val1)) {
                val1_set = set;
            }

            if(set.contains(val2)) {
                val2_set = set;
            }
        }
        if(val1_set == null && val2_set == null)
            list.add(pairSet);
        if(val1_set != null && val2_set == null)
            val1_set.addAll(pairSet);
        if(val1_set == null && val2_set != null)
            val2_set.addAll(pairSet);
        if(val1_set != null && val2_set != null){
            list.remove(val2_set);
            val1_set.addAll(val2_set);
        }
    }

    for(Set<Integer> set : list){
        System.out.println(set);
    }
}

First and foremost : your code contains a subtle bug.
Try this input : {"1 2", "4 3", "5 5", "2 3", "1 3"}

As for style;

please respect standard naming conventions.
seperate concerns : parsing the string into a pair should be done in a separate method, this will improve readability.
it's unclear from just looking at your code what the algorithm does exactly. Try finding a better name for the method and certain variables. If that's insufficient, some documentation can also help.
probably this is an exercise, as your algorithm simply prints its result, rather than return it. But generally such methods are useless, plus it mingles concerns again. Make the method return its result, and make a client method print the result. The main method would then call three methods

List<Set<Integer>> pairs = parse(input);
List<Set<Integer>> clusters = calculateClusters(pairs);
print(clusters);


Determining if a string is an Isogram I have two solutions to the task and they are all passing the visible tests but when it is time to submit, I keep getting my failed to pass all tests. There are still some hidden tests my code isn't passing.

The task reads as follow:

Write a program that checks if a word supplied as the argument is an Isogram. An Isogram is a word in which no letter occurs more than once.

Create a method called is_isogram that takes one argument, a word to test if it's an isogram. This method should return a tuple of the word and a boolean indicating whether it is an isogram.

If the argument supplied is an empty string, return the argument and False: (argument, False). If the argument supplied is not a string, raise a TypeError with the message 'Argument should be a string'.

Example:

is_isogram("abolishment")

Expected result:

("abolishment", True)

The visible tests:

from unittest import TestCase

class IsogramTestCases(TestCase):
  def test_checks_for_isograms(self):
    word = 'abolishment'
    self.assertEqual(
      is_isogram(word),
      (word, True),
      msg="Isogram word, '{}' not detected correctly".format(word)
    )

  def test_returns_false_for_nonisograms(self):
    word = 'alphabet'
    self.assertEqual(
      is_isogram(word),
      (word, False),
      msg="Non isogram word, '{}' falsely detected".format(word)
    )

  def test_it_only_accepts_strings(self):
    with self.assertRaises(TypeError) as context:
      is_isogram(2)
      self.assertEqual(
        'Argument should be a string',
        context.exception.message,
        'String inputs allowed only'
      )

I wrote two different solutions to the task and they both passed the tests without errors but when I try submitting I got
 "Test Spec Failed
Your solution failed to pass all the tests".

Please any elegant way of solving this task?

Solution 1:

def is_isogram(word):
    if type(word) != str:
        raise TypeError('Argument should be a string')

    elif word == "":
      return (word, False)
    else:
        word = word.lower()
        for char in word:
            if word.count(char) > 1:
                return (word, False)
            else:
                return (word, True)

Solution 2:

def is_isogram(string):
    if isinstance(string, str) and len(string) != 0:
        string = string.lower()
        if len(string) == len(set(string)):
            result = True
        else:
            result = False
    elif not string:
      result = False
    else:
        raise TypeError('Argument should be a string')

    return string, result

I would write it as a mix of your two solutions. Checking if the set of the string has the same length as the string is probably the best way to check for an isogram.

On the other hand, the control structures in your first code are nicer, because there is less nesting and it does the same thing.

You should make sure you use a consistent number of spaces for indentation. You seem to be using 2, 4 and 6. PEP8, Python's official style-guide, recommends 4.

def is_isogram(word):
    if not isinstance(word, str):
        raise TypeError('Argument should be a string')
    if not word:
        isogram = False
    else:
        isogram = len(word) == len(set(word.lower()))
    return word, isogram

Does the specification say anything about if A and a should count as the same letter?