Balanced number checking Problem

Balanced number is the number that the sum of all digits to the left of the middle digit(s) and the sum of all digits to the right of the middle digit(s) are equal. The middle digit(s) should not be considered when determining whether a number is balanced or not.

If the number has an odd number of digits then there is only one middle digit, e.g. 92645 has middle digit 6; otherwise, there are two middle digits , e.g. 1301 has middle digits 3 and 0.

Given a number, find if it is balanced or not. Number passed is always Positive. Return the result as String.

My solution

I solved it. But I am not sure, how to fold the similarly looking code for odd and even lengths.

data ListLength a = ListLength {list :: [a], len :: Int} deriving Show

digs :: Integral x => x -> ListLength x
digs 0 = ListLength [0] 1
digs x = let
      helper :: Integral x => x -> [x] -> Int -> ListLength x
      helper 0 acc len = ListLength acc len
      helper n acc len = helper (n `div` 10) ((n `mod` 10) : acc) $ len + 1
  in
      helper x [] 0

balancedNum :: Int -> String
balancedNum n | n > 0 = let digits = digs n
                            middle = (len digits) `div` 2
                        in
                          if even (len digits) then
                            let (left, right) = splitAt (middle - 1) (list digits) in
                              if (sum left) == (sum $ drop 2 right) then
                                "Balanced"
                              else
                                "Not Balanced"
                          else
                            let (left, right) = splitAt middle (list digits) in
                              if (sum left) == (sum $ tail right) then
                                "Balanced"
                              else
                                "Not Balanced"
              | otherwise = error "Number must be positive"

Here my thoughts on your solution. But I'm also learning Haskell right now, so I'm not an expert..

You don't need the data type. You can use base 10 logarithm to get the number of digits. Like this:

(floor $ logBase 10 $ fromIntegral n) + 1

I like the fact that digs is tail recursive. It looks simpler without the data type:

digits :: Int -> [Int]
digits = digits_tr []
    where
        digits_tr :: [Int] -> Int -> [Int]
        digits_tr l 0 = l
        digits_tr l n = digits_tr (n `mod` 10 : l) (n `div` 10)

Why does balancedNum give a string? A boolean would be more natural.
If you want to handle errors, why not use Maybe instead of error? Like in balancedNum :: Int -> Maybe Bool. Anyway according to the problem description you wouldn't need it: a negative number is just not balanced, so the function should just give False back.

Here is my solution:

balanced :: Int -> Bool
balanced n
    | n >= 0 = (sum $ take middle $ digits n) == (sum $ take middle $ reverse $ digits n)
    | otherwise = False
    where
        len = (floor $ logBase 10 $ fromIntegral n) + 1
        middle = if len `mod` 2 == 0 then len `div` 2 - 1 else len `div` 2


Maximal pythagorean triplet that sums to n Write a function maxPythagTriple that accepts an integer n and returns the maximum product of three numbers that are both a Pythagorean triplet and sum to the integer n. If no such pythagorean triplet exists, return -1.

i.e. maxPythagTriple(4) == -1
i.e. maxPythagTriple(12) == 60
{3,4,5} --> 3+4+5 = 12 --> 3^2+4^2= 5^2 --> 3*4*5 = 60

Below is my solution the preceeding challenge. I have been told there is a better solution. Any help would be much appreciated!

package challenges;

public class MathChallenges {

    private static boolean isValidTriple(int i, int j, int k) {
        return k > 0 && i != j && j != k && i > j && j > k;
    }

    private static boolean isPythagTriple(int i, int j, int k) {
        return (i*i + j*j) == k*k;
    }

    public static int maxPythagTriple(int n) {
        for (int i = n-1; i >= 1; i--) {
            for (int j = i-1; j >= 1; j--) {
                int k = n - (i + j);
                if (isValidTriple(i, j, k) && isPythagTriple(k, j, i)) {
                    return i * j * k;
                }
            }
        }
        return -1;
    }

    public static void main(String[] args) {
        System.out.println("maxPythagTriple(4) = " + maxPythagTriple(4));
        System.out.println("maxPythagTriple(12) = " + maxPythagTriple(12));
    }

}

The code returns the first product, not the maximum product. For example, it returns 6240 instead of 7500 on input 60:

(60)  10² + 24² = 26² ; 10 * 24 * 26 = 6240
(60)  15² + 20² = 25² ; 15 * 20 * 25 = 7500

This is easy to fix: loop and take the maximum.

  I have been told there is a better solution.

There are solutions that take fewer steps. One could attempt to improve the current solution by shrinking the possible ranges. Given:

\$c^2 = a^2 + b^2 ; sum = a + b + c\$

Neither a nor b can be larger than c. This also means that neither a nor b can be larger than half the sum.

For c: it can't be smaller than any other term, and it must be at least one-third of the sum.

  // the "sum - 1" accounts for a being at least 1
  for ( int b = (sum - 1) / 2; b >= 1; b-- ) {
    for ( int c = Math.max(b, sum - 2*b); c < sum-b-1; c++ ) {
      int a = n - b - c;
      if ( a*a + b*b == c*c ) {
        // found a triple
      }
    }
  }

This is a little better, but it's still inefficient. If you're feeling brave, there is a formula to generate all Pythagorean triples.

Euclid's Formula

Briefly, Euclid's formula involves two integers, \$m > n > 0\$, that generate Pythagorean triples:

\$a = m^2 - n^2 ; b = 2mn ; c = m^2 + n^2\$

(This doesn't generate all triples; we'll get to that in a minute.)

The sum of a, b, c becomes:

\$sum = m^2 - n^2 + 2mn + m^2 + n^2 = (m - n)(m + n) + (m + n)^2 = 2m(m + n)\$

Because sum, m, and n are integers, this means that both m and (m+n) divide sum.

For minimal n = 1, m is maximally \$\sqrt{sum \over 2}\$, so that's a good upper bound to start with. And given sum and m, we have \$n = {sum \over {2m}} - m\$

/* sum = 2m(m+n) ; m > n > 0 */
public static int maxPythagTriple_Euclid(int sum) {
  // sum must be even; smallest triple has sum 12
  if ( sum % 2 != 0 || sum < 12 ) return -1;
  final int sumd2 = sum / 2; // divisible by 2
  for ( int m = (int) Math.sqrt(sumd2); m > sumd2 / m - m; m-- ) {
    if ( sum % m != 0 ) continue; // must be a divisor
    final int n = sumd2 / m - m;
    if ( n <= 0 ) continue; // rule out pathologicals
    if ( sum % (m+n) != 0 ) continue; // must be a divisor
    final int msq = m*m, nsq = n*n;
    final int a = msq - nsq, b = 2*m*n, c = msq + nsq;
    assert a * a + b * b == c * c;
    return a * b * c;
  }
  return -1;
}

// Alternative implementation, pulling n into the for-loop
public static int maxPythagTriple_Euclid(int sum) {
  if ( sum % 2 != 0 || sum < 12 ) return -1;
  final int sumd2 = sum / 2; // divisible by 2
  for ( int m = (int) Math.sqrt(sumd2), n = sumd2 / m - m;
            m > n;
            m--, n = sumd2 / m - m ) {
    if ( n <= 0 || sum % m != 0 || sum % (m+n) != 0 ) {
      continue;
    }
    final int msq = m*m, nsq = n*n;
    final int a = msq - nsq, b = 2*m*n, c = msq + nsq;
    assert a * a + b * b == c * c;
    return a * b * c;
  }
  return -1;
}

We can return at our first hit because we maximize m; the product will be largest when the terms are closest, which is the case for minimal n (and thus maximal m).

To quote myself:

  This doesn't generate all triples; we'll get to that in a minute.

Euclid's formula doesn't generate all triples, but it does generate at least all primitive triples.

An important property of Pythagorean triples is that, if \$(a,b,c)\$ is a triple, then so is \$(k*a, k*b, k*c)\$, for any integer k. Such a k is necessarily a divisor of the sum of a, b, c.

public static int maxPythagTriple_Euclid(int sum) {
  int max = -1;
  for ( int k = 1; k < sum / 2 && sum / k >= 12; k++ ) {
    if ( sum % k != 0 ) continue;

    // multiply by k^3
    int prod = maxPythagTriple_Euclid_primitive(sum / k) * (k * k * k);
    if ( prod > max ) {
      max = prod;
    }
  }
  return max;
}

Precomputing the divisors of sum may save some more steps, but I haven't looked into that.


Python Rock-Paper-Scissors via a class to handle the game Original inspiration was by this Python beginner, and it prompted me to rewrite a bunch of stuff with my flair and my Python experience: First attempt: Python Rock Paper Scissors

Okay, so I looked at the aforementioned post, and was bored and needed to kill an hour at work.  So I killed an hour - I took their RPS game and turned it into a class, and made it look less evil/ugly.

While this is by no means a fully-fledged program that I've cleanly created and really thoroughly tested, this is something that I can at least ask for input on.

Runs pretty cleanly, and uses a lot of strings that the OP of the original inspiration post had.  But, it has a lot of docstrings, too.  And the entire game resides in a class, and calls via the class and such.

Because this version uses f-strings, you must have Python 3.6 or newer to use this program/code.

rps.py:

import random

class RockPaperScissors:
    """
    Class to handle an instance of a Rock-Paper-Scissors game
    with unlimited rounds.
    """

    def __init__(self):
        """
        Initialize the variables for the class
        """
        self.wins = 0
        self.losses = 0
        self.ties = 0
        self.options = {'rock': 0, 'paper': 1, 'scissors': 2}

    def random_choice(self):
        """
        Chooses a choice randomly from the keys in self.options.
        :returns: String containing the choice of the computer.
        """

        return random.choice(list(self.options.keys()))

    def check_win(self, player, opponent):
        """
        Check if the player wins or loses.
        :param player: Numeric representation of player choice from self.options
        :param opponent: Numeric representation of computer choice from self.options
        :return: Nothing, but will print whether win or lose.
        """

        result = (player - opponent) % 3
        if result == 0:
            self.ties += 1
            print("The game is a tie! You are a most worthy opponent!")
        elif result == 1:
            self.wins += 1
            print("You win! My honor demands a rematch!")
        elif result == 2:
            self.losses += 1
            print("Haha, I am victorious! Dare you challenge me again?")

    def print_score(self):
        """
        Prints a string reflecting the current player score.
        :return: Nothing, just prints current score.
        """
        print(f"You have {self.wins} wins, {self.losses} losses, and "
              f"{self.ties} ties.")

    def run_game(self):
        """
        Plays a round of Rock-Paper-Scissors with the computer.
        :return: Nothing
        """
        while True:
            userchoice = input("Choices are 'rock', 'paper', or 'scissors'.\n"
                               "Which do you choose? ").lower()
            if userchoice not in self.options.keys():
                print("Invalid input, try again!")
            else:
                break
        opponent_choice = self.random_choice()
        print(f"You've picked {userchoice}, and I picked {opponent_choice}.")
        self.check_win(self.options[userchoice], self.options[opponent_choice])

if __name__ == "__main__":
    game = RockPaperScissors()
    while True:
        game.run_game()
        game.print_score()

        while True:

            continue_prompt = input('\nDo you wish to play again? (y/n): ').lower()
            if continue_prompt == 'n':
                print("You are weak!")
                exit()
            elif continue_prompt == 'y':
                break
            else:
                print("Invalid input!\n")
                continue

Any suggestions and input are welcome, as this is a rough attempt.  :)

I think it makes sense to use dictionaries to store values for win/tie/loss:

    def __init__(self):
        """
        Initialize the variables for the class
        """
        self.options = {'rock': 0, 'paper': 1, 'scissors': 2}
        self.outcome_count = {
            "tie": 0,
            "win": 0,
            "loss": 0,
        }

This makes check_win a bit more "mechanical" since you can now refer to the outcomes by name and by looking things up in static data, instead of needing a bunch of if/else:

    def check_win(self, player, opponent):
        """
        Check if the player wins or loses.
        :param player: Numeric representation of player choice from self.options
        :param opponent: Numeric representation of computer choice from self.options
        :return: Nothing, but will print whether win or lose.
        """

        result = ["tie", "win", "loss"][(player - opponent) % 3]

        self.outcome_count[result] += 1

        outcome_message = {
            "tie":  "The game is a tie! You are a most worthy opponent!",
            "win":  "You win! My honor demands a rematch!",
            "loss": "Haha, I am victorious! Dare you challenge me again?",
        }
        print(outcome_message[result])

though of course it ends up making print_score less self-explanatory:

    def print_score(self):
        """
        Prints a string reflecting the current player score.
        :return: Nothing, just prints current score.
        """
        wins   = self.outcome_count["win"]
        losses = self.outcome_count["loss"]
        ties   = self.outcome_count["tie"]
        print(f"You have {wins} wins, {losses} losses, and {ties} ties.")

Lastly, I think that the run_game loop can be made slightly clearer by writing instead

        while True:
            userchoice = input("Choices are 'rock', 'paper', or 'scissors'.\nWhich do you choose? ").lower()
            if userchoice in self.options.keys():
                break
            print("Invalid input, try again!")

I find an explicit "early-exit" with no else to be easier to follow [note that the condition is not inverted, which I think helps for clarity in this case] though this could be jarring if it's not conventional for the larger codebase.


Sorting an int[] array using insertion sort algorithm - follow-up Previous questions:

Is my implementation of insertion sort is correct?
Sorting an int[] array with an Insertion Sort

I'm currently learning about different sorting algorithms and after reading on the concept of the insertion sort of how it's done I've tried to implement it by myself before seeing how it was implemented in the source from which I'm learning.

This is my implementation:

void insertionSort(int arr[])
{
    for(int i = 1; i < SIZE; i++)
    {
        int value = arr[i];
        int j = i;
        for(; j > 0; j--)
        {
            if (value < arr[j - 1])
            {
                arr[j] = arr[j - 1];
            }
            else
            {
                break;
            }
        }
        arr[j] = value;
    }
}

Now I think I've implemented it correctly (not talking about efficient or anything just the algorithm itself), but when I saw the way it was implemented in the course I'm seeing I had a bit of concerns about my implementation because they're really different and I just wanted to make sure my implementation is really an insertion sort algorithm implementation and I didn't done anything else.

Here's the implementation by the source I'm learning from:

void insertionSort(int arr[])
{
  for(int i = 1;, i < SIZE; i++)
  {
      int value = arr[i];
      int j = i - 1;
      int done = 0;
      do
      {
          if (arr[j] > value)
          {
              arr[j + 1] = arr[j];
              j--;
              if (j < 0)
                  done = 1;
          }
          else
          {
              done = 1;
          }
      } while(!done)
      arr[j + 1] = value;
  }
}

Is my implementation of insertion sort correct ?

Also I would appreciate if you could compare my implementation to the one made by the source I'm learning from, any efficiency differences or any other thing you think would worth mentioning.

Nice job.  It looks like you have implemented a correct insertion sort.

The inner loop could be better written: the j > 0 condition is giving an incomplete picture of how the loop terminates.

One other change you should make is to take a size parameter for the array size.  Without it, you have a function that only works on arrays of a specific size!  If you look at examples of functions in the C library, almost every function that takes an array also takes another parameter to specify its size.  Usually, the type of that parameter is an unsigned int, but called size_t to make its purpose clear.

#include <stdlib.h>

void insertionSort(int arr[], size_t size)
{
    for (int i = 1; i < size; i++)
    {
        int value = arr[i];
        int j;
        /* Shift elements over by one slot to make way for the insertion */
        for (j = i; j > 0 && value < arr[j - 1]; j--)
        {
            arr[j] = arr[j - 1];
        }
        arr[j] = value;
    }
}

Your implementation is better than the book you're learning from.  In fact, I would recommend using a different book, if the code examples are so poor.  Specifically, the biggest problem is the done flag.  Setting a flag to be inspected by a conditional elsewhere is a cumbersome way to implement flow control.  Your break is much more direct and assertive, and I think that my loop condition is even clearer.


Pythagorean Theorem in Java I'm new to Java and am following a series of lectures on Stanford's YouTube. I know that the code works, but I'd like to know if it can be improved upon.

/*
 * File: PythagoreanTheorem.java
 * Name: Will
 * Section Leader: N/A, I'm freeloading the class off Youtube.
 * -----------------------------
 * This file is the starter file for the PythagoreanTheorem problem.
 */

import acm.program.*;

public class PythagoreanTheorem extends ConsoleProgram {
    public PythagoreanTheorem() {}
    public void run() {
        PythagoreanTheorem myCalculator = new PythagoreanTheorem();
        double a = readDouble("a= ");
        double b = readDouble("b= ");
        if(myCalculator.formula(a,b) ==
            Math.floor(myCalculator.formula(a,b))) {
            println("C= "+(int)myCalculator.formula(a,b));}
        else {
            println("C= "+myCalculator.formula(a,b));}
                      }
    private double formula(double x, double y) {
        double c = Math.sqrt(x*x+y*y);
        return c;
    }
}

I did not check the acm package but my 2 cents:

No need to instantate a PythagoreanTheorem each time

public void run() {
    PythagoreanTheorem myCalculator = new PythagoreanTheorem();

run is not static, so there already is an instance of PythagoreanTheorem availiable in the method, referencable by this.

So instead of:

if(myCalculator.formula(a,b) ==

You can use

if(this.formula(a,b) ==

Reuse results

You have a few calls to myCalculator.formula(a,b). While a and b do not change. You can store the result in a variable.

double c = myCalculator.formula(a,b)
if(c == Math.floor(c))
....


Retrieving all directorie names (relative path) Just a small code review.
Is there a better way to get all folder names?

<?php
$folders = glob(dirname(__FILE__) . '/myFolder', GLOB_ONLYDIR);
foreach ( $folders as $f ) {
    $lastDsPos = strrpos($v, DIRECTORY_SEPARATOR);
    $myFolders[] = substr($v, $lastDsPos+1);
}

Have you considered using SPLs RecursiveIteratorIterator and RecursiveDirectoryIterator? Like this:

$iterator = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($this->directory), RecursiveIteratorIterator::CHILD_FIRST);
foreach($iterator as $path){
    if($path->isDir()){
        $cache = $this->GetInfoArray($path->__toString());
        isset($cache) ? $this->listing['directories'][] = $cache : "";
        unset($cache);
    } else {
        $this->listing['files'][] = $this->GetInfoArray($path->__toString());
    }
}

This will leave you with an array with two main nodes: files and directories. I wrote this for a PHP distributed test-and-build-on-commit system and it worked well. More info on the PHP SPL Iterators can be found here: http://www.php.net/manual/en/spl.iterators.php

By the way, the GetInfoArray is this:

private function GetInfoArray($path){
    $d = new SplFileInfo($path);
    if($d->getBasename() == "." || $d->getBasename() == ".."){
        return;
    } else {
        return array(
           "pathname"    => $d->getPathname(),
           "access"      => $d->getATime(),
           "modified"    => $d->getMTime(),
           "permissions" => $d->getPerms(),
           "size"        => $d->getSize(),
           "type"        => $d->getType(),
           "path"        => $d->getPath(),
           "basename"    => $d->getBasename(),
           "filename"    => $d->getFilename()
        );
    }
}

In the end, you're left with something like this:

$this->listing['files']['smiles.jpg']['pathname'] = "/srv/www/smiles.jpg"
$this->listing['files']['smiles.jpg']['access'] = "1323546894"
$this->listing['files']['smiles.jpg']['modified'] = "2316546521"
$this->listing['files']['smiles.jpg']['permissions'] = "744"
$this->listing['files']['smiles.jpg']['size'] = "12Kb"
$this->listing['files']['smiles.jpg']['type'] = "jpg"
$this->listing['files']['smiles.jpg']['path'] = "/srv/www"
$this->listing['files']['smiles.jpg']['basename'] = "smiles"
$this->listing['files']['smiles.jpg']['filename'] = "smiles.jpg"

What the heck, heres the class itself:

# Done

class DirectoryReader{
    private $directory;
    private $listing;

    public function __construct($directory){
        try {
            $this->directory = $directory;
            $this->listing = array();
            $this->ListDir();
        } catch(UnexpectedValueException $e){
            die("Path cannot be opened.");
        } catch(RuntimeException $e){
            die("Path given is empty string.");
        }
    }

    public function GetListing(){
        return $this->listing;
    }

    private function ListDir(){
        $iterator = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($this->directory), RecursiveIteratorIterator::CHILD_FIRST);
        foreach($iterator as $path){
            if($path->isDir()){
                $cache = $this->GetInfoArray($path->__toString());
                isset($cache) ? $this->listing['directories'][] = $cache : "";
                unset($cache);
            } else {
                $this->listing['files'][] = $this->GetInfoArray($path->__toString());
            }
        }
    }

    private function GetInfoArray($path){
        $d = new SplFileInfo($path);
        if($d->getBasename() == "." || $d->getBasename() == ".."){
            return;
        } else {
            return array(
               "pathname"    => $d->getPathname(),
               "access"      => $d->getATime(),
               "modified"    => $d->getMTime(),
               "permissions" => $d->getPerms(),
               "size"        => $d->getSize(),
               "type"        => $d->getType(),
               "path"        => $d->getPath(),
               "basename"    => $d->getBasename(),
               "filename"    => $d->getFilename()
            );
        }
    }
}

As a further note, you can also just call scandir($directory) and filter via is_dir() in a foreach.


Simplifying an ajax script in my HTML I'm using Twitter Bootstrap and I'm working on a page that has several tabs that all have carousels in them (each with a ton of images) I've managed to write an AJAX script that pulls the images from a JSON file I've created (for one of the carousels). I'm planning on making similar JSON file for the rest, but what I'm wondering though is there a way to write the AJAX script so that it grabs the id from the HTML so I don't have to make a unique AJX script for each version of the carousel

Here's how my HTML looks for the carousel:

And here's my AJAX script:

I'm basically wondering if there is a way to pull the id (carousel1b) from the htnl and inject it into the AJAX strip in the "url:'jason/carousel1b and the "$('carousel').append(d)"

Depending on when you want to make the ajax request, you'll need to modify the first line of my example. Currently it just gets all of them. You might want to change it so that it gets only the div that was clicked, or whatever. Anyways here's one way you could do that:

$('div[id*="carousel"]').each(function() { //The *= selector gets all the divs with an id that contains carousel anywhere in the id.
    var $this = $(this), //Here we select the right div
        id = $this.attr('id'); //And grab the id of the div

    $.ajax({
        url: 'json/' + id,
        //You don't need to set type: GET, the GET is the default.
        success: function(data) {
            //blah blah blah...
            $this.append(d); //Boom done!
        }
    });
});

The way you have your success function set up, you're appending and changing the DOM each time that for loop goes around, and it goes around for every image. That's not good because DOM manipulations are quite expensive performance wise. So if you have 100 images, you're append one at a time - 100 times. The better way to do that would be do all your stuff and save to a variable, string, or object, then append once outside the loop.

Here's an example of what  I mean:

var totalPictures = data['totalPictures'],
    d = ""; //We add "d" outside

for (var i=0; i<totalPictures; i++) {
    if(i<1){
        d = "
";
        d += "";
        d += "";
        d += "";
        d += "
";
    } else {
        d += "
"; //Added a += here so that it'll add onto the first picture
        d += "";
        d += "";
        d += "";
        d += "
";
    }
}

$('#carousel1b').append(d); //Here we append a single time outside the loop
//This cuts our appends down from totalPictures to 1.
//What may seem like a small change will make a huge difference

$("a[rel^='prettyPhoto']").prettyPhoto(); //I assume you'll need this outside since it depends on the appended content


C++ finalizer template Consider, please, my code of the finalizer template:

#include <functional>

template<typename F, typename... Args>
class finally
{
public:

  finally(F action, const Args&... args) :
      _action(std::bind(action, args...))
  {}
  ~finally()
  {
    if(enabled)
      _action();
  }

  bool enabled = true;

private:

  std::function<void()> _action;
};

In particular I would enhance this template to avoid of using a typenames twice. Let's say we need to close some file on leaving some block of code. Then we declare an object:

finally<int(int), int> f(close, some_file);

Here I used the standard close function as an action. But I should use int twice in the <int(**int**), **int**>. Is there any way to avoid this?

You can avoid specifying argument types twice by employing a constructor template:

template<typename F>
class finally
{
public:
  template<typename... Args>
  finally<F>(F action, const Args&... args) { /* business as usual */ }
  ...
};

But that's not everything. The std::bind's arguments are either copied or moved, they are never passed by reference. Thus, despite const Args&... args, you might end up copying your arguments.

To void this issue, consider using perfect forwarding (as well as ditching std::bind):

template<typename... Args>
finally<F>(F action, Args&&... args) :
    _action([action, &args...]{ action(std::forward<Args>(args)...); }) {}

Another thing worth considering is the exception safety of action. Since you're invoking it in a destructor, you're risking facing std::terminate if it throws. A small try block will fix it. Also, as of now, enabled seems redundant.

And yes, even with all these fixes, the semantics of finally seems rather unpleasant compared to more traditional scope guards. At least to me.

Edit: full code - https://ideone.com/iThn0i


Go functions to create a directory if it doesn't exist Which of the 2 solutions is written better? Personally I prefer the one line method but it does make it slightly more difficult to debug.  Is perhaps doing it the long way first then refactoring to the second method once we know all is tested to be working.

Please note Environment var "FT_SOURCE" is set to "c:\filemover\"

Original solution...

func ensureDirold(dirName string) (string, error) {

        err := os.Mkdir(dirName, os.ModeDir)

        if err == nil || os.IsExist(err) {
            return dirName, nil
        } else {
            return dirName, err
        }
    }

Usage...

func OldWay()  {
    dir := os.Getenv("FT_SOURCE") //is set to c:\filemover
    fmt.Print(ensureDirold(dir))
    _, err := os.Create(dir + "myfile-old.txt")
    check(err)

}

Alternate Solution...

type strerr struct {
    str string
    err error
}

func ensureDirnew(dirName string) strerr {

    err := os.Mkdir(dirName, os.ModeDir)

    if err == nil || os.IsExist(err) {
        return strerr{
            str: dirName,
            err: nil,
        }
    } else {
        return strerr{
            str: dirName,
            err: err,
        }
    }
}

Usage...

func NewWay() {
    _, err := os.Create(ensureDirnew(os.Getenv("FT_SOURCE")).str + "myfile.txt")
    check(err)
}

Create a directory if it doesn't exist.

  Don't report an error if the directory exists.

Isn't that a simple wrapper for the Go standard library os.Mkdir function?

For example,

// mkdir, like os.Mkdir, creates a new directory
// with the specified name and permission bits.
// If the directory exists, mkdir returns nil.
func mkdir(name string, perm os.FileMode) error {
    err := os.Mkdir(name, perm)
    if err != nil {
        if !os.IsExist(err) {
            return err
        }
    }
    return nil
}

  Personally I prefer the one line method.

In Go, we favor readability and simplicity for correct and maintainable code. Use the Go idiom for creating a file:

f, err := Create(fileName)
if err != nil {
    // handle error
}
defer f.Close()

For your usage, use a wrapper for the Go standard library os.Create function.

For example,

// createFT, like os.Create, creates the named file with mode 0666 (before umask),
// truncating it if it already exists.
// The file is created in the directory named in the FT_SOURCE environment variable.
func createFT(fileName string) (*os.File, error) {
    dirKey := "FT_SOURCE"
    dirName, ok := os.LookupEnv(dirKey)
    if !ok {
        err := fmt.Errorf("%s environmemt variable not found", dirKey)
        return nil, err
    }

    perm := os.FileMode(0666)
    err := mkdir(dirName, perm)
    if err != nil {
        return nil, err
    }

    f, err := os.Create(filepath.Join(dirName, fileName))
    if err != nil {
        return nil, err
    }
    return f, nil
}

Functions are used to encapsulate implementation details. Functions can also be used to hide complexity and "ugly" code.


Find the longest zigzag sequence in a array of integers I think I have a brute force solution to the problem but I can't think of a more optimal solution

A sequence of integers is called a zigzag sequence if each of its elements is either strictly less than both neighbors or strictly greater than both neighbors. For example, the sequence 4 2 3 1 5 3 is a zigzag, but 7 3 5 5 2 and 3 8 6 4 5 aren't.
For a given array of integers return the length of its longest contiguous sub-array that is a zigzag sequence.

Example

For a = [9, 8, 8, 5, 3, 5, 3, 2, 8, 6], the output should be
zigzag(a) = 4.

The longest zigzag sub-arrays are [5, 3, 5, 3] and [3, 2, 8, 6] and they both have length 4.

Input/Output
[input] array.integer a

constraints:
2 ≤ a.length ≤ 25,
0 ≤ a[i] ≤ 100.

Here is my brute force solution

def zigzag(input):
    current_longest = 0
    longest_subset  = []
    input_length    = len(input)

    for x in range(input_length):
        current_list = input[x:]

        for pos_a, num_a in enumerate(current_list):
            subset      = [num_a, ]
            prev_trend  = None

            for pos_b, num_b in enumerate(current_list[pos_a + 1:]):
                last_number = subset[-1]

                if last_number == num_b:
                    # print "[DUPE] current subset state", subset, "comparing ->", subset[-1], num_b
                    break

                if last_number > num_b:
                    cur_trending = "down"
                else:
                    cur_trending = "up"

                if prev_trend:
                    # compare trends
                    if prev_trend == cur_trending:
                        # print "[TREND] current subset state", subset, "comparing ->", subset[-1], num_b
                        break
                    else:
                        prev_trend = cur_trending
                else:
                    # initial trend set
                    prev_trend = cur_trending

                subset.append(num_b)

                if len(subset) > current_longest:
                    current_longest = len(subset)
                    longest_subset = subset

    print current_longest, longest_subset

zigzag([9, 8, 8, 5, 3, 5, 3, 2, 8, 6])
zigzag([4, 2, 3, 1, 5, 3])

You can do this in \$O(n)\$ rather than \$O(n^3)\$. To force this, change input to an iterator. I'd also make a function that returns all the zigzags, so for [9, 8, 8, 5, 3, 5, 3, 2, 8, 6] it'd return [(9, 8), (8,), (8, 5), (5, 3, 5, 3), (3, 2, 8, 6)].

The reason you'd want to do this, is as you can then make zigzag a three line function, and you know that you're building actual zigzags of the maximum size.

There's a fairly simple algorithm that you can follow to build the zigzags:

Go through the input.
If two adjacent items are the same, yield the current zigzag, and a new zigzag only containing that duplicate item.
If three adjacent items don't zigzag, yield the current zigzag, and start a new zigzag starting at the second item.

In more depth, I achieved this by doing:

Build a stack, and add the first number to it.
Loop until you run out of items in input:

Force the stack to be a size of two. If it contains three items, only keep the last two. If it has one item, add an item from the input to it.
If the first item in the stack, is the same as the second item in the stack, yield the first item, and make a stack that only contains the second item. Restart the loop.
Store the direction of the zigzag. (find if item 1 > item 2 for example.)
Infinitely do:

Copy the previous item from the stack.
Get a new item from the input.
If the previous or current item are the same, or they are the same direction as the previous zigzag, then stop looping.
Add the current item to the stack.
Store the current state of the zigzag.

yield the stack as a tuple. (Don't want people mutating the data.)
Add the current item to the stack.

If the stack has been initialized and contains items, yield the stack as a tuple.

Which can be implemented as:

def zigzags(input):
    input = iter(input)
    stack = None
    try:
        stack = [next(input)]
        while True:
            if len(stack) < 2:
                stack.append(next(input))
            else:
                stack = stack[-2:]
            a, b = stack
            if a == b:
                yield (a,)
                stack = [b]
                continue
            zig = a > b
            while True:
                prev = stack[-1]
                this = next(input)
                if prev == this or zig == (prev > this):
                    break
                stack.append(this)
                zig = not zig
            yield tuple(stack)
            stack.append(this)
    except StopIteration:
        pass
    if stack:
        yield tuple(stack)

def zigzag(input):
    item = max(zigzags(input), key=len)
    print len(item), item


Binary to decimal number converter Here I am doing some exercises for my studies with Python. This is a binary to decimal number converter (currently restricted to that).

These are the very first steps, so there is no error handling. You can even input non-binary numbers and get a result. However it works for converting binary numbers, which is its task.

I'm not sure about all shenanigans going around with the main function and the argument parsing.

import argparse
import sys

###############################################################################

def main(binary_number):
    binary_number = list(binary_number)
    decimal_number = 0

    for digit in range(0, len(binary_number)):
        if binary_number[digit] != '0':
            decimal_number += power_to_the_people(2, len(binary_number)-digit-1)

    print(decimal_number)

###############################################################################

def power_to_the_people(base, exponent):
    # Return 1 if exponent is 0
    if exponent == 0:
        return 1

    power = base
    for j in range(1, exponent):
        power *= base

    return power # to the people

###############################################################################

if __name__ == '__main__':

    parser = argparse.ArgumentParser(description='Converts a binary to a decimal number.')

    parser.add_argument('binary_number', action="store")

    args = parser.parse_args()

    main(args.binary_number)

(Update: Added parentheses to print so it actually works under 3.4)

A few additional points for your learning experience.

You're not using sys, so the import line is unnecessary.

In range(...), the default start is 0,
so instead of range(0, x) you can write simply range(x).

Don't use pointless comments like this:

# Return 1 if exponent is 0
if exponent == 0:
    return 1

Instead if this:

power = base
for j in range(1, exponent):
    power *= base

It would be better this way:

power = 1
for j in range(exponent):
    power *= base

Because in this version base is referenced only once,
and range is simplified by dropping 1 to use the default 0.

action="store" is unnecessary here:

parser.add_argument('binary_number', action="store")

main is not a good name for that function does.
It would be better to rename it to binary_to_decimal, for example.

And while at it,
it would be better to move the content from the if __name__ ... inside a new main method, like this:

def main():

    parser = argparse.ArgumentParser(description='Converts a binary to a decimal number.')
    parser.add_argument('binary_number')
    args = parser.parse_args()
    binary_to_decimal(args.binary_number)

if __name__ == '__main__':
    main()

Many people miss this point, actually.
The reason to do it this way is that code in the if __name__ ... block is in the global namespace.
So variables with the same name in the rest of the code will shadow those variables,
which can lead to nasty bugs.
By moving that code to a method,
this cannot happen.

PEP8

PEP8 is the official style guide of Python,
and you have a few violations:

Put 2 blank lines before each method definition and you only put 1.
Put 2 spaces in front of # inline comments
Lines should not be longer than 79 characters


Splitting one line into multiple ones given a separator I have a file containing one long line:

"name surname" <name.surname@example.com>, 'name surname' <name.surname@example.com>, name surname <name.surname@example.com>, "'name surname'" <name.surname@example.com>, surname, <name.surname@example.com>, name <name.surname@example.com>

Note that that's 6 different forms.

I am splitting each email address into its own line, and saving the results into another file:

import sys

ifile = sys.argv[1]
ofile = sys.argv[2]

with open(ifile) as ifile, open(ofile, "w") as ofile:
    addresses = ifile.readline().split(">,")
    for n, address in enumerate(addresses):
        address = address.replace("'", "")
        address = address.replace('"', "")
        name, address = address.split("<")
        address = "<" + address
        if len(name) > 1:
            name = name.strip()
            name = '"{}" '.format(name)
            address = "".join(name + address)
        if n < len(addresses) - 1:
            ofile.write(address.strip() + ">\n")
        else:
            ofile.write(address.strip() + "\n")

Feels to me like hackery so am looking for a better solution.

Why are you first removing the quotes and then putting them back?

And why are you removing the brackets and them putting them back?

This does the same thing, except change ' to ". It also doesn't handle commas in names,
so if you have that it won't work. In that case I'd probably use a regexp.

import sys

ifile = sys.argv[1]
ofile = sys.argv[2]

with open(ifile) as ifile, open(ofile, "w") as ofile:
    for address in ifile.readline().split(","):
        ofile.write(address.strip() + '\n')

Update:

"surname, name <name.surname@example.com>" sucks, and that means your format is inconsistent and not parseable without horrid hacks. In that case your code seems OK, although I'd probably do it differently. I would most likely use a regexp to find all cases of commas that are NOT preceded by > and followed by a space to something else, say chr(128) or something like that. I'd then parse the code with my code above, extract the email from withing the brackets, strip all quotes and brackets from the remander, and replace back chr(128) with commas.

And the lastly write that to the outfile.

The difference there is that I don't try to handle a horrid format, I first try to fix the problems. It makes for cleaner code, IMO.

Update 2:

I instead replaced the commas that should be split on, making it simpler, like so:

import sys

ifile = sys.argv[1]
ofile = sys.argv[2]

with open(ifile) as ifile, open(ofile, "w") as ofile:
    data = ifile.read()
    data = data.replace('>,', '>\xF0')
    for line in data.split('\xF0'):
        name, email = line.split('<')
        email = email.replace('>', '').strip()
        name = name.replace('"', '').replace("'", "").strip()
        ofile.write('"%s" <%s>\n' % (name, email))

and then I realized I could simplify it even more:

import sys

ifile = sys.argv[1]
ofile = sys.argv[2]

with open(ifile) as ifile, open(ofile, "w") as ofile:
    data = ifile.read()
    for line in data.split('>,'):
        name, email = line.split('<')
        email = email.strip()
        name = name.replace('"', '').replace("'", "").strip()
        ofile.write('"%s" <%s>\n' % (name, email))

And as this point I'm basically doing what you are doing, but much simplified.


Parsing SOAP messages from a web service I'm consuming an ASMX service with SOAP messages. Since Xamarin doesn't support SOAP 1.2, I'm writing the Web requests manually. I'm sending a user/password to retrieve the user ID.

A successful response may look like this:

<soap:Envelope xmlns:soap="http://www.w3.org/2003/05/soap-envelope" xmlns:ns="link-to-namespace">
  <soap:Body>
    <ns:User>
      <ns:UserId>123</ns:UserId>
    </ns:User>
  </soap:Body>
</soap:Envelope>

An error response may look like this:

<soap:Envelope xmlns:soap="http://www.w3.org/2003/05/soap-envelope">
  <soap:Body>
    <soap:Fault>
      <soap:Code>
        <soap:Value>InvalidUsernameAndPassword</soap:Value>
      </soap:Code>
    </soap:Fault>
  </soap:Body>
</soap:Envelope>

What's the best way to parse the response? And how to check if there is an error?

Currently I just look for the presense of the <Fault> element.

WebService class handles the request and returns the SOAP envelope as an XDocument:

class WebService
{
    // Returns the SOAP envelope as an XDocument.
    public XDocument Invoke(string soapAction, string method, Dictionary<string, string> parameters)
    {

UserApi class acts as a proxy towards the service.

class UserApi
{
    // Sends user/pwd to server to retrieve user id.
    public string GetUserId(string username, string password)
    {
        var soapAction = "link-to-soap-action";
        var method = "GetUserId";
        var parameters = new Dictionary<string, string>
        {
            { "Username", username },
            { "Password", password}
        };
        var service = new WebService();
        var result = service.Invoke(soapAction, method, parameters);

        XNamespace nsSoap = "http://www.w3.org/2003/05/soap-envelope";
        XNamespace ns = "link-to-namespace";

        // Look for the "Fault" element in the response. If present, there was an error.
        var fault = result.Root.Element("{" + nsSoap + "}Body").Element("{" + ns_soap + "}Fault");
        if (error != null)
        {
            var codeWithNs = error.Element("{" + nsSoap + "}Code").Element("{" + ns_soap + "}Value").Value;
            var codeSplit = codeWithNs.Split(':');
            var code = codeSplit.Length == 2 ? codeSplit[1] : codeSplit[0];
            if (code == "InvalidUsernameOrPassword")
            {
                throw new InvalidUsernameOrPasswordException();
            }
            else
            {
                throw new Exception("Something went wrong.");
            }
        }

        return userId = result.Descendants(ns + "UserId").First().Value;
    }

    public async Task<string> GetUserIdAsync(string username, string password)
    {
        return await Task.Run(() =>
        {
            return GetUserId(username, password);
        });
    }
}

Currently it works, at least under optimal conditions. But my solution looks dense and fragile. If I should go with deserialization, here's some pseudo code of what I'm thinking:

try
{
    var serializer = new XmlSerializer(typeof(UserEnvelope));
    var user = serializer.Deserialize(response);
}
catch (InvalidOperationException)
{
    var serializer = new XmlSerializer(typeof(ErrorEnvelope));
    var error = serializer.Deserialize(response);
    if (error.Code == "InvalidUsernameOrPassword")
    ...etc
}

Spot the inconsistency:

        var fault = result.Root.Element("{" + nsSoap + "}Body").Element("{" + ns_soap + "}Fault");
...
            var codeWithNs = error.Element("{" + nsSoap + "}Code").Element("{" + ns_soap + "}Value").Value;
...
        return userId = result.Descendants(ns + "UserId").First().Value;

Ok, there are two inconsistencies. Firstly, nsSoap vs ns_soap. I suspect that that difference is due to refactoring the code before posting in, in which case please don't do that again: we want to review code which compiles and works.

Secondly, "{" + nsSoap + "}Body" vs ns + "UserId". The simple concatenation is far more readable.

        var service = new WebService();
        var result = service.Invoke(soapAction, method, parameters);

        XNamespace nsSoap = "http://www.w3.org/2003/05/soap-envelope";
        XNamespace ns = "link-to-namespace";

        // Look for the "Fault" element in the response. If present, there was an error.
        var fault = result.Root.Element("{" + nsSoap + "}Body").Element("{" + ns_soap + "}Fault");
        if (error != null)
        {
            var codeWithNs = error.Element("{" + nsSoap + "}Code").Element("{" + ns_soap + "}Value").Value;
            var codeSplit = codeWithNs.Split(':');
            var code = codeSplit.Length == 2 ? codeSplit[1] : codeSplit[0];
            if (code == "InvalidUsernameOrPassword")
            {
                throw new InvalidUsernameOrPasswordException();
            }
            else
            {
                throw new Exception("Something went wrong.");
            }
        }

There's a strong asymmetry here which I don't grok. WebService does the job of assembling some simple strings into the XML request document: why does it not also do the job of disassembling the XML response document? I understand that it can't fully do that, because the response might be a complex datastructure, but IMO WebService should be detecting <soap:Fault> and returning some kind of "option" type.

As it happens, there is an option type in .Net: Task<TResult>. So I think that WebService.Invoke should return a Task<XElement> which has Status Faulted and a placeholder exception, or Status RanToCompletion and the contents of <soap:Body>. Then GetUser would look like

public string GetUserId(string username, string password)
{
    var soapAction = "link-to-soap-action";
    var method = "GetUserId";
    var parameters = new Dictionary<string, string>
    {
        { "Username", username },
        { "Password", password}
    };

    var service = new WebService();
    var result = service.Invoke(soapAction, method, parameters);

    if (result.Status == TaskStatus.Faulted)
    {
        // Handle specific errors
        if (result.Exception.Message == "InvalidUsernameOrPassword")
        {
            throw new InvalidUsernameOrPasswordException();
        }
        else
        {
            throw new Exception("Something went wrong.", result.Exception);
        }
    }

    XNamespace ns = "link-to-namespace";
    return userId = result.Value.Descendants(ns + "UserId").First().Value;
}


Excluding a long list of words with grep This Bash program extracts the 20 first lines of each man page present on a system and uses grep to remove lines matching a list of selected terms.

How can I improve the grep part? (something less repetitive, with less pipe but that do the same job)

#!/bin/bash

# generate the list
compgen -c > man

# extract each man of the list
while read -r LINE; do
MANWIDHT=80 man "$LINE" | head -20 | grep -v NAME | grep -v 1 | grep -v SYNOPSIS | grep -v SYNOPSYS | grep -v OVERVIEW | grep -v VERSION | grep -v VERSION | grep -v INTRODUCTION | grep -v 5 | grep -v ABSTRACT | grep -v 4 | grep -v DESCRIPTION | grep -v COPYRIGHT | grep -v ABRT | grep -v SEE | grep -v 8 | grep -v case | grep -v 2 | grep -v STATUS | grep -v CHECK | grep -v UNIX | grep -v FORTUNE | grep -v PROLOG | grep -v Usage | grep -v Help | grep -v Introduction | grep -v Local | grep -v ARGUMENTS | grep -v SYNTAX | grep -v NOTE | grep -v QUERYING | grep -v 3 | grep -v Description | grep -v Note | grep -v Name | grep -v COPYING | grep -v COMMAND | grep -v USAGE | grep -v EXAMPLE | grep -v 3 | grep -v OPTIONS | grep -v AUTHOR | grep -v DOCUMENTS | grep -v SUMMARY | grep -v -e '^$' && echo -en '\n'
done < man > man1

# add the % to each empty line
sed -i -e 's/^$/%/' man1

# merge to fortune-rtfm db
cat man1 > fortune-rtfm

First of all, avoid excessively long lines that force reviewers to scroll horizontally far far right. You can break lines to manageable length like this:

MANWIDHT=80 man "$LINE" | head -20 | grep -v NAME | grep -v 1 | \
grep -v SYNOPSIS | grep -v SYNOPSYS | grep -v OVERVIEW | grep -v VERSION | \
grep -v VERSION | grep -v INTRODUCTION | grep -v 5 | grep -v ABSTRACT | \
...

You are using too many temporary files. You could chain the commands together in a single pipeline, without unnecessary temporary files:

compgen -c | while read -r LINE; do
    MANWIDHT=80 man "$LINE" | head -20 | grep -v NAME | ...
done | sed -e 's/^$/%/' > fortune-rtfm

Instead of a chain of grep -v word1 | grep -v word2 | grep -v word3,
you can use a single grep with multiple -e parameters like this:

grep -v -e word1 -e word2 -e word3

A single grep process instead of dozens will certainly run much faster.

A simple echo is the same as echo -en '\n', but better in every way.

Finally, instead of a long grep with all the terms you want to exclude, you can put them in a file, one word per line, and use the -f flag of grep to use that list. Then the script can be shortened to:

#!/bin/bash

compgen -c | while read -r LINE; do
    MANWIDHT=80 man "$LINE" | head -20 | grep -v -f words.txt | grep -v '^$' && echo
done | sed -e 's/^$/%/' > fortune-rtfm

@Sundeep pointed out a few important things in a comment. MANWIDHT is misspelled, it should be MANWIDTH.
Also, since the patterns are fixed strings (as opposed to regular expressions), it's good to add the -F flag so that grep can optimize the matching. Lastly, you might want to match whole words instead of parts of words. If that's the case, add the -w flag. This is equivalent to wrapping each pattern within \<...\>. (The effect is that for example "VERSIONING" is matched by grep VERSION, but by grep -w VERSION.)

The content of words.txt, sorted for easier reading, is something like this (just the first bunch of lines):

1
2
3
4
5
8
ABRT
ABSTRACT
ARGUMENTS
AUTHOR
CHECK
COMMAND
COPYING
... (and so on)

If you want to keep all the patterns inside your script file, you can do that too, for example like this:

compgen -c | while read -r LINE; do
    MANWIDHT=80 man "$LINE" | head -20 | grep -vFf <(cat << EOF
1 2 3 3 4 5 8
ABRT ABSTRACT ARGUMENTS AUTHOR
CHECK COMMAND COPYING COPYRIGHT
DESCRIPTION DOCUMENTS Description
EXAMPLE
FORTUNE
Help
INTRODUCTION Introduction
Local
NAME NOTE Name Note
OPTIONS OVERVIEW
PROLOG
QUERYING
SEE STATUS SUMMARY SYNOPSIS SYNOPSYS SYNTAX
UNIX USAGE Usage
VERSION
case
EOF
) | grep -v '^$' && echo
done | sed -e 's/^$/%/' > fortune-rtfm


Using rotation of proxies within a Python script I've written a script in Python using rotation of proxies which is able to fetch different titles from a website traversing different pages. I've tried to write this scraper in such a way so that it will try using proxies randomly until it can get the titles from that webpage, meaning it will use every single proxy cyclically if there is any difficulty getting the valid response.

The proxies and site address I've used within my scraper are just placeholders to let you know how I'm trying to do this.

As I do not have much experience about the usage of rotation of proxies within a scraper, there may be flaws within the design. It is working errorlessly, though.

I will be very glad if I get any suggestions as to how I can improve this existing script to make it more robust.

import requests
from bs4 import BeautifulSoup
from fake_useragent import UserAgent
from random import choice

ua = UserAgent()

search_urls = ['https://stackoverflow.com/questions?page={}&sort=newest'.format(page) for page in range(1,3)]

def get_proxies():
    proxies = ['128.199.254.244:3128', '95.85.79.54:53281', '128.199.125.54:2468', '178.45.8.113:53281', '206.189.225.30:3128']
    return proxies

def check_proxy(session,proxy):
    session.headers = {'User-Agent', ua.random}
    session.proxies = {'https': 'https://{}'.format(proxy)}
    try:
        response = session.get('https://httpbin.org/ip')
        item = response.json()
        print(item)
        return 0  ##if the proxy is a working one, break out of the function
    except Exception:
        proxy = random_proxy()
        check_proxy(session,proxy)  ##if the earlier one is not working, try moving on to fetch the working one

def random_proxy():
    return choice(get_proxies())

def scrape_page(url):
    proxy = random_proxy()
    session = requests.Session()
    session.headers = {'User-Agent', ua.random}
    session.proxies = {'https': 'https://{}'.format(proxy)}
    check_proxy(session,proxy) ##try validating the proxy before further attempt

    try:
        response = session.get(url)
    except Exception:
        response = None  #preventing "UnboundLocalError:"
        check_proxy(session,proxy) #if try block failed to execute the response, activate it

    soup = BeautifulSoup(response.text, 'lxml')
    for items in soup.select(".question-hyperlink"):
        print(items.text)

if __name__ == '__main__':
    for link in search_urls:
        scrape_page(link)

Every time you call scrape_page() with some URL, you end up making at least two requests: at least one request to verify that the randomly chosen proxy works, and then to make the main request.  Isn't that overhead a bit excessive?  Why not optimistically assume that a proxy works, and verify the proxy only if the main request fails?

If many requests fail — for example, if your network is down — then your program would get stuck in a tight, infinite retry loop.  Even a 0.1-second delay in the exception handler would be very helpful to prevent the CPU from going haywire.

The code in check_proxy() is a bit redundant with the code in scrape_page().  Also, check_proxy() is inappropriately recursive.  I would create a set_proxy() function that has a more comprehensive mission.

Instead of assuming that each proxy is HTTPS, I would write the URL of each proxy with the explicit protocol, then infer the protocol by parsing the URL.

from bs4 import BeautifulSoup
from fake_useragent import UserAgent
from random import choice
import requests
from time import sleep
from urllib.parse import urlparse

PROXIES = [
    'https://128.199.254.244:3128',
    'https://95.85.79.54:53281',
    'https://128.199.125.54:2468',
    'https://178.45.8.113:53281',
    'https://206.189.225.30:3128',
]

def set_proxy(session, proxy_candidates=PROXIES, verify=False):
    """
    Configure the session to use one of the proxy_candidates.  If verify is
    True, then the proxy will have been verified to work.
    """
    while True:
        proxy = choice(proxy_candidates)
        session.proxies = {urlparse(proxy).scheme: proxy}
        if not verify:
            return
        try:
            print(session.get('https://httpbin.org/ip').json())
            return
        except Exception:
            pass

def scrape_page(url):
    ua = UserAgent()
    session = requests.Session()
    session.headers = {'User-Agent': ua.random}
    set_proxy(session)

    while True:
        try:
            response = session.get(url)
            break
        except Exception as e:
            session.headers = {'User-Agent': ua.random}
            set_proxy(session, verify=True)
            sleep(0.1)

    soup = BeautifulSoup(response.text, 'lxml')
    for items in soup.select(".question-hyperlink"):
        print(items.text)


Fibonacci heap implementation I'd like this reviewed.

public sealed class FibonacciHeap<TKey, TValue>
{
    readonly List<Node> _root = new List<Node>();
    int _count;
    Node _min;

    public void Push(TKey key, TValue value)
    {
        Insert(new Node {
            Key = key,
            Value = value
        });
    }

    public KeyValuePair<TKey, TValue> Peek()
    {
        if (_min == null)
            throw new InvalidOperationException();
        return new KeyValuePair<TKey,TValue>(_min.Key, _min.Value);
    }

    public KeyValuePair<TKey, TValue> Pop()
    {
        if (_min == null)
            throw new InvalidOperationException();
        var min = ExtractMin();
        return new KeyValuePair<TKey,TValue>(min.Key, min.Value);
    }

    void Insert(Node node)
    {
        _count++;
        _root.Add(node);
        if (_min == null)
        {
            _min = node;
        }
        else if (Comparer<TKey>.Default.Compare(node.Key, _min.Key) < 0)
        {
            _min = node;
        }
    }

    Node ExtractMin()
    {
        var result = _min;
        if (result == null)
            return null;
        foreach (var child in result.Children)
        {
            child.Parent = null;
            _root.Add(child);
        }
        _root.Remove(result);
        if (_root.Count == 0)
        {
            _min = null;
        }
        else
        {
            _min = _root[0];
            Consolidate();
        }
        _count--;
        return result;
    }

    void Consolidate()
    {
        var a = new Node[UpperBound()];
        for (int i = 0; i < _root.Count; i++)
        {
            var x = _root[i];
            var d = x.Children.Count;
            while (true)
            {
                var y = a[d];
                if (y == null)
                    break;
                if (Comparer<TKey>.Default.Compare(x.Key, y.Key) > 0)
                {
                    var t = x;
                    x = y;
                    y = t;
                }
                _root.Remove(y);
                i--;
                x.AddChild(y);
                y.Mark = false;
                a[d] = null;
                d++;
            }
            a[d] = x;
        }
        _min = null;
        for (int i = 0; i < a.Length; i++)
        {
            var n = a[i];
            if (n == null)
                continue;
            if (_min == null)
            {
                _root.Clear();
                _min = n;
            }
            else
            {
                if (Comparer<TKey>.Default.Compare(n.Key, _min.Key) < 0)
                {
                    _min = n;
                }
            }
            _root.Add(n);
        }
    }

    int UpperBound()
    {
        return (int)Math.Floor(Math.Log(_count, (1.0 + Math.Sqrt(5)) / 2.0)) + 1;
    }

    class Node
    {
        public TKey Key;
        public TValue Value;
        public Node Parent;
        public List<Node> Children = new List<Node>();
        public bool Mark;

        public void AddChild(Node child)
        {
            child.Parent = this;
            Children.Add(child);
        }

        public override string ToString()
        {
            return string.Format("({0},{1})", Key, Value);
        }
    }
}

Disclaimer: I will not address the actual FibonacciHeap itself, just the c# code.

I explicitly write the private keyword on those methods and properties, so they are indented roughly the same as the public ones. A matter of preference, I guess.

These _name variables are odd, I suggest using the de facto standard of PascalCase.

_count seems to be a duplication of Root.count. I would remove it.
However, ExtractMin adds items to the Root and does not increment count! This seems like a bug, but perhaps it's not. If it is, it is a strong justification for removing _count; if not, it could use a quick comment there to signal why _count is not updated.

Because I like my variables very strongly typed, I don't like var. I changed most of its uses to an explicit type.

You can if-chain if-elses better if you do not use parenthesis around the else that takes an if inside:

if (Min == null) {
    Root.Clear();
    Min = item;
} else if (Comparer<TKey>.Default.Compare(item.Key, Min.Key) < 0) {
    Min = item;
} // else if() { } else if() { } etc

A Node does not seem to make sense without a Key and a Value, so I would add a constructor that takes those two.

You overrode Node.ToString. I assume that was for debugging purposes? I would use the DebuggerDisplay attribute in the class, instead.

Here is a proposal of reviewed code, with some highlights in comments:

public sealed class FibonacciHeap<TKey, TValue>
{
    private readonly List<Node> Root = new List<Node>();
    // TODO: is this not a duplication of Root.Count? Remove it.
    private int _count;
    private Node Min;

    public void Push(TKey key, TValue value)
    {
        Insert(new Node(key, value));
    }

    public KeyValuePair<TKey, TValue> Peek()
    {
        if (Min == null)
            throw new InvalidOperationException();
        return new KeyValuePair<TKey, TValue>(Min.Key, Min.Value);
    }

    public KeyValuePair<TKey, TValue> Pop()
    {
        if (Min == null)
            throw new InvalidOperationException();
        var min = ExtractMin();
        return new KeyValuePair<TKey, TValue>(min.Key, min.Value);
    }

    private void Insert(Node node)
    {
        _count++;
        Root.Add(node);
        if (Min == null)
        {
            Min = node;
        } else if (Comparer<TKey>.Default.Compare(node.Key, Min.Key) < 0)
        {
            Min = node;
        }
    }

    private Node ExtractMin()
    {
        var result = Min;
        if (result == null)
            return null;
        foreach (var child in result.Children)
        {
            child.Parent = null;
            // TODO: shouldn't _count be updated?? Same in Consolidate();
            Root.Add(child);
        }
        Root.Remove(result);
        if (Root.Count == 0)
        {
            Min = null;
        } else
        {
            Min = Root[0];
            Consolidate();
        }
        _count--;
        return result;
    }

    // Here be FibonacciHeap dragons.
    private void Consolidate()
    {
        var unlabeledBag = new Node[UpperBound()];
        for (int i = 0; i < Root.Count; i++)
        {
            Node item = Root[i];
            int itemChildren = item.Children.Count;
            while (true)
            {
                Node child = unlabeledBag[itemChildren];
                if (child == null)
                    break;
                if (Comparer<TKey>.Default.Compare(item.Key, child.Key) > 0)
                {
                    var swap = item;
                    item = child;
                    child = swap;
                }
                Root.Remove(child);
                i--;
                item.AddChild(child);
                child.Mark = false;
                unlabeledBag[itemChildren] = null;
                itemChildren++;
            }
            unlabeledBag[itemChildren] = item;
        }
        Min = null;
        for (int i = 0; i < unlabeledBag.Length; i++)
        {
            var item = unlabeledBag[i];
            if (item == null)
                continue;
            if (Min == null)
            {
                Root.Clear();
                Min = item;
            } else if (Comparer<TKey>.Default.Compare(item.Key, Min.Key) < 0)
            {
                Min = item;
            }
            Root.Add(item);
        }
    }

    private int UpperBound()
    {
        // Here be dragons.
        // Also, if _count is NOT Root.Count, it ought to at least have a more meaningful name.
        double magicValue = Math.Log(_count, (1.0 + Math.Sqrt(5)) / 2.0);
        return (int)Math.Floor(magicValue) + 1;
    }

    [DebuggerDisplay("{Key}, {Value}")]
    private class Node
    {
        public TKey Key;
        public TValue Value;
        public Node Parent;
        public List<Node> Children = new List<Node>();
        public bool Mark;

        public Node(TKey key, TValue value)
        {
            Key = key;
            Value = value;
        }

        public void AddChild(Node child)
        {
            child.Parent = this;
            Children.Add(child);
        }
    }
}


Update game content via AJAX On my application, I have a JavaScript cycle that updates tag contents on my page regularly:

<script type="text/javascript">
    var players = [];
    players.push($('#p0'));
    players.push($('#p1'));
    players.push($('#p2'));
    players.push($('#p3'));
    i=0;
    /*
     * cycle through the players.
     */
    function cyclePlayer() {
        players[i].parents(".card-header").addClass('border-success');
        players[i].load('play.php?p='+i);
        i = ++i % players.length;
        $('#deck').load('play.php?deck=1');
        $('#pile').load('play.php?pile=1');
        $('#feedback').load('play.php?feedback=1');
    }
    PlayerLoop = setInterval('cyclePlayer()', 1500 );

    $("#stop").click(function(){
        clearInterval(PlayerLoop);
    });

    $('#reset').click(function() {
        $('#deck').load('play.php?reset=1');
        location.reload();
    });
</script>

The server code that is being called looks like this:

<?php
require '../vendor/autoload.php';

session_start();

use Svc\Myapp\FrontHandler;

$myApp = new FrontHandler();

if (isset($_GET['p']) && !isset($_SESSION['winner'])) {
    echo $myApp->playerTurn();
}

if (isset($_GET['deck'])) {
    echo $myApp->deckUpdate();
}

if (isset($_GET['pile'])) {
    echo $myApp->pileUpdate();
}

if (isset($_GET['feedback'])) {
    echo $myApp->feedbackUpdate();
}

Is this production quality code? Is there a way to refactor it to improve it?

My answer focuses on the ajax requests.

Why 4 requests at each 1500 milliseconds to the same file (play.php)?

I would do only one request and I would "cycle" on a delay starting from the success callback... instead of a fixed interval, which does not take the request delay in account.

var players = [ // No need to use a push method when you already know the array content.
  $('#p0'),
  $('#p1'),
  $('#p2'),
  $('#p3')
];

i=0;

function cyclePlayer() {

  $.ajax({
    url: "play.php",
    data: {iteration:i},
    method: "GET",
    dataType: "json",
    success: (data)=>{

      console.log(JSON.stringify(data));  // See all the infos in the same request response.
      json = JSON.parse(data);

      players[i].html(json.playerTurn).parents(".card-header").addClass('border-success');

      i = ++i % players.length;

      $('#deck').html(json.deckUpdate);
      $('#pile').html(json.pileUpdate);
      $('#feedback').html(json.feedbackUpdate);

      PlayerLoop = setTimeout(cyclePlayer,1500);    // From here, set a new 1500ms timeout.
    },
    error: ()=>{

    },
  });
}

// on load, start a timeout.
var PlayerLoop = setTimeout(cyclePlayer, 1500 );

$("#stop").click(function(){
    clearTimeout(PlayerLoop);
});

$('#reset').click(function() {
  location.reload(true);  // true is to reload from server instead of possibly loading from cache.
});

And the PHP:

<?php
require '../vendor/autoload.php';
session_start();
use Svc\Myapp\FrontHandler;

if(!isset($_GET['iteration'])){die();}

$myApp= new FrontHandler();

if(!isset($_SESSION['winner'])){
  $results["playerTurn"] = $myApp->playerTurn();
}
$results["deckUpdate"] = $myApp->deckUpdate();
$results["pileUpdate"] = $myApp->pileUpdate();
$results["feedbackUpdate"] = $myApp->feedbackUpdate();

$results["isSetWinner"] = isset($_SESSION['winner']);

// Echo the array as a json. This will be the response of the request.
echo json_encode($result);
?>

Now, I could not test the above... I hope It does not have any typo.


Using PHP explode() to parse multi-part error messages I am dealing with a system which sends any unexpected errors back to me in a string, for example:

Error Type - Database
Error Message - Error Executing Database Something is wrong.
Error Detail - [Macromedia][SQLServer JDBC Driver][SQLServer]Incorrect syntax near '1'.

Now to create a better UI I am expected to deal with these errors, so I thought about using explode(), and found this user's response in the PHP Manual which helped me delimit the results with two delimiters instead of just one:

Then I needed to get a little deeper into the code because I wanted $key => $val pairs, so I came across this answer on StackOverflow.

Then I combined both and came across this solution:

function multiexplode ($delimiters,$string) {

    $ready = str_replace($delimiters, $delimiters[0], $string);

    list($err_typ, $err_typ_cont,
         $err_mess, $err_mess_cont,
         $err_det, $err_det_cont) = explode($delimiters[0], $ready);

    $result[ trim($err_typ) ] = trim($err_typ_cont);
    $result[ trim($err_mess) ] = trim($err_mess_cont);
    $result[ trim($err_det) ] = trim($err_det_cont);
    return  $result;
}

Then I call the function like this:

 multiexplode(array("-", PHP_EOL), $errorMessage)

The response seems to be exactly what I needed:

Array
(
    [Error Type] => Database
    [Error Message] => Error Executing Database Something is wrong.
    [Error Detail] => [Macromedia][SQLServer JDBC Driver][SQLServer]Incorrect syntax near '1'.
)

My question comes with PHP's best practices, and if there would be a more reliable or a more efficient approach I could take into solving this problem?

A couple of thoughts:

This function is designed to only deal with a very sepcific use case - parsing a formatted error string.  For that reason I would recommend changing the function name to be more representative of what is actually happening.  Perhaps something like parseErrorString() or similar.
Since you are dealing with a very specific use case here, why would you require the caller to pass in the delimiters?  This function should fully encapsulate all the parsing logic.
Using an array for return, seems odd, particularly since you have spaces in your key names.  I would consider returning a stdClass or similar object with type, message, detail properties.
Though not strictly required, I would consider using regular expressions here, as I think that can clean things up as well as allow the solution to be less fragile to changes in the string format.
Using PHP_EOL as line break may be fragile. PHP_EOL is good for working with strings within same environment, but may fail if your application is running in a different environment than the environment your string is produced in.
Your function assumes happy path execution.  What if you get a poorly/unexpectedly formatted string?  Perhaps you are validating this string somewhere else, but if not, you should do it here in your function.

A refactoring, with these thoughts in mind, might yield something like the following:

function parseErrorString($string) {
    if(empty($string) || !is_string($string)) {
        throw new InvalidArgumentException('Non-zero length string expected');
    }
    $lines = preg_split('/\R/', $string);
    $error = array_reduce(
        $lines,
        function (&$err, $line) {
            $regex = '/^Error (Type|Message|Detail) - (.*)$/';
            $matches = [];
            $result = preg_match($regex, $line, $matches);
            if ($result !== 1) {
                // no match.  Is this an error condition?
                // at the very least, do nothing
                return $err;
            }
            $err->{strtolower($matches[1])} = $matches[2];
            return $err;
        },
        new stdClass();
    );
    return $error;
}


Replace $number with $number+1 in std::string I want to detect $number substring in std::string and then replace it with $number + 1.

For example, the string Hello$9World$1 should become Hello$10World$2.

Here's my code:

#include <iostream>
#include <string>

void modifyDollarNumber(std::string &str)
{
    for (size_t i = str.length(); i --> 0 ;)
    {
        if (str[i] == '$')
        {
            size_t j = i + 1;
            while (j < str.length() && isdigit(str[j]))
            {
                ++j;
            }
            size_t len = j - (i + 1);
            if (len)
            {
                std::string sub = str.substr(i + 1, len);
                int num = std::stoi(sub) + 1;
                str.erase(i + 1, len);
                sub = std::to_string(num);
                str.insert(i + 1, sub);
            }
        }
    }
}

int main()
{
    std::string str = "!$@#$34$1%^&$5*$1$!%$91$12@$3";
    modifyDollarNumber(str);
    std::cout << "Result : " << str << '\n';
}

And I can get the result I want which is

Result : !$@#$35$2%^&$6*$2$!%$92$13@$4
Program ended with exit code: 0

But I want to improve my code so it can be as fast as possible.

How can I simplify modifyDollarNumber() function?

Few observations.

I. You don't need to scan the whole string, character-by-character. str.find('$') will do it for you (and remember to start the second search, and all subsequent ones, from the last reference detected.)

II. Rather than taking substrings, parsing them, erasing and inserting—why not increment in-place? You know exactly your number span (from the one past '$' to the one past the last digit.) If this string only consists of '9's, insert a single '1' right after '$' and change all nines into zeroes. If there are digits less than nine, increment the rightmost non-nine, and change all the subsequent nines into zeroes, if any, no insertion/erasure required. Proceed with the next search.

III. As @AJNeufeld mentioned, string reconstruction can be done in a separate buffer, though vast testing is needed to decide if this is a faster option.


Fetching files or directories in a given directory This is a self-teaching implementation to get files for a given directory in order to simplify os.Walk in Go (avoid to pass a func for recursively walking across the files and directories).

I'm pretty new to the language and feel there is already something wrong with
the way the structure ""implements"" the interface. I feel that it should not be by value but rather by reference in order to avoid redundant copies, should it?

package main

import (
    "os"
    "fmt"
    "path/filepath"
    "time"
)

type FileDetail interface {
    Path() string
    Info() os.FileInfo
}

type fileDetail struct{
    path string
    info os.FileInfo
}

func (detail fileDetail) Path() string {
    return detail.path
}

func (detail *ileDetail) Info() os.FileInfo {
    return detail.info
}

func GetFiles(path string) []FileDetail {
    var details []FileDetail

    walkFunc := func (filePath string, fileInfo os.FileInfo, err error) error {
        if err != nil {
            return err
        }
        if !fileInfo.IsDir() {
            var detail FileDetail = fileDetail{path: filePath, info: fileInfo}
            details = append(details, detail)
        }
        return nil
    }

    filepath.Walk(path, walkFunc)

    return details
}

func GetDirectories(path string) []FileDetail {
    var details []FileDetail

    walkFunc := func (filePath string, fileInfo os.FileInfo, err error) error {
        if err != nil {
            return err
        }
        if fileInfo.IsDir() {
            var detail FileDetail = fileDetail{path: filePath, info: fileInfo}
            details = append(details, detail)
        }
        return nil
    }

    filepath.Walk(path, walkFunc)

    return details
}

func main() {
    sandBoxDirectory := "F:/Perforce/eperret/XLC/R6Code/framework/source/scimitar/onlinemodule/Sandbox"

    time.Now()
    files := GetFiles(sandBoxDirectory)

    for index, file := range files {
        fileInfo := file.Info()
        fmt.Println(index, file.Path())
        fmt.Println(index, fileInfo.Name(), fileInfo.Size())
    }
}

Let's compare your two methods GetFiles and GetDirectories:

func GetFiles(path string) []FileDetail {
    var details []FileDetail

    walkFunc := func (filePath string, fileInfo os.FileInfo, err error) error {
        if err != nil {
            return err
        }
        if !fileInfo.IsDir() {
            var detail FileDetail = fileDetail{path: filePath, info: fileInfo}
            details = append(details, detail)
        }
        return nil
    }

    filepath.Walk(path, walkFunc)

    return details
}

func GetDirectories(path string) []FileDetail {
    var details []FileDetail

    walkFunc := func (filePath string, fileInfo os.FileInfo, err error) error {
        if err != nil {
            return err
        }
        if fileInfo.IsDir() {
            var detail FileDetail = fileDetail{path: filePath, info: fileInfo}
            details = append(details, detail)
        }
        return nil
    }

    filepath.Walk(path, walkFunc)

    return details
}

This is a classic case where copy&paste code is a problem. How can this be simplified? In Go, a good use-case will be a "file test" function that returns true if a file passes a test. Consider the "general purpose" private function that takes a "test" function as an input:

func filterFiles(path string, test func(os.FileInfo) bool) []FileDetail {
    var details []FileDetail
    walkFunc := func (filePath string, fileInfo os.FileInfo, err error) error {
        if err != nil {
            return err
        }
        if test(fileInfo) {
            var detail FileDetail = fileDetail{path: filePath, info: fileInfo}
            details = append(details, detail)
        }
        return nil
    }
    filepath.Walk(path, walkFunc)
    return details
}

Now, you can reuse that in your directories and files lists with:

func GetFiles(path string) []FileDetail {
    return filterFiles(path, func(file os.FileInfo) bool {
        return !file.IsDir()
    })
}

func GetDirectories(path string) []FileDetail {
    return filterFiles(path, func(file os.FileInfo) bool {
        return file.IsDir()
    })
}

Now you can also easily add interesting checks on file names, permissions, etc.

I realize that the interface FileDetail is there to teach you how they work, but in this instance, the interface is overkill, and I'd just keep things as a struct and move on.

As for the implementation of the interface, it's not horrible. Your concerns about the pointers is not really valid since all the data is small, and static.

It's OK in Go default to real data structures where the data is copied around, the memory cost is considered trivial.


Project Euler #14: Longest Collatz Sequence starting point Problem statement:

  The following iterative sequence is defined for the set of positive integers:

  n → n/2 (n is even)
  n → 3n + 1 (n is odd)

  Using the rule above and starting with 13, we generate the following sequence:

  13 → 40 → 20 → 10 → 5 → 16 → 8 → 4 → 2 → 1

  It can be seen that this sequence (starting at 13 and finishing at 1) contains 10 terms. Although it has not been proved yet (Collatz Problem), it is thought that all starting numbers finish at 1.

  Which starting number, under one million, produces the longest chain?

NOTE: Once the chain starts the terms are allowed to go above one million.
Here is my implementation written in Python, awaiting your feedback.

from time import time
from operator import itemgetter

def collatz_count(n, count={1: 1}):
    """uses cache (count) to speed up the search, returns sequence
    length for the given number"""
    try:
        return count[n]
    except KeyError:
        if n % 2 == 0:
            count[n] = collatz_count(n / 2) + 1
        else:
            count[n] = collatz_count(n * 3 + 1) + 1
    return count[n]

def run_test():
    """uses the previous function, returns number that generates the
    largest sequence"""
    time1 = time()
    items = sorted([(x, collatz_count(x)) for x in range(1, 1000000)],
                   key=itemgetter(1), reverse=True)
    maximum = items[0]
    print(f' Starting number: {maximum[0]} \n Sequence length:
    {maximum[1]} \n '
          f'Calculated in: {time() - time1} seconds.')

if __name__ == '__main__':
    run_test()

Integer Division

In Python, 10 / 2 is equal to 5.0, not 5.  Python has the integer division operator (//) which produces an integral value after division, instead of a floating point value.  To prevent storing both int and float keys in the count dictionary, you should use:

    count[n] = collatz_count(n // 2) + 1

Cache

Your count cache works nicely.  But you don’t have to implement it yourself.  Python comes with caching built-in.  You just have to request it.

import functools

@functools.lru_cache(None)
def collatz_count(n):
    if n == 1:
        return 1
    elif n % 2 == 0:
        return collatz_count(n // 2) + 1
    else
        return collatz_count(n * 3 + 1) + 1

The first time collatz_count(n) is called with any particular value of n, the function is called and the returned value is memorized in the cache.  Any subsequent call with that argument retrieves the value from the cache.  (The None value indicates that the cache size is unlimited.)

Sorting to find the maximum

Sorting, in order to find the maximum value, is an \$O(n \log n)\$ in time, and since you need to remember all the values, \$O(n)\$ in space.

There is no need to sort the list; instead, just maintain the largest value encountered so far, and the input which generated that value.  This is \$O(n)\$ in time, so much faster, and \$O(1)\$ in space.

And again, it is built-in to Python:

longest_start_number = max(range(1, 1000000), key=collatz_count)
length = collatz_count(longest_start_number)

Docstrings

You are using the docstrings incorrectly.

You shouldn’t say """uses the previous function ...  """ in a docstring, because if a user types help(run_test), they will not know what function the previous function is.  Even if they type help(module_name) and get help for all the functions in the module, the order the functions get listed in the resulting help output is not necessarily the order of the functions in the module’s source, so again “the previous function” is confusing.

Similarly, saying """uses cache (count) to speed up ...""" is useless, because a user typing help(collatz_count) is expecting to find help on how to use the function, such as what legal values can be given for n, what the return value is, and whether or not the user is expected to pass in a value for count; they don’t need implementation details about how the function was written.


Python code to find greatest common divisor of multiple numbers Hmm, I know this has been implemented countless times, and Python 3.9.5 math standard library has a built-in gcd() method that does exactly this, but this is how I do this, I think completing simple programming challenges using new ways will let gain experience that will help me find ingenious ways to overcome unprecedented practical programming challenges, so bear with me.
This implementation uses prime factorization method, it has three functions: factors(), gcd() and main(), the first function returns a dictionary object, the keys of the dictionary are the base prime factors of the inputted number, the values are the powers (how many times the factor should multiply by itself) of the keys, the dictionaries are created with one key{'1': 1};
And the second function accepts two numbers, uses each number as inputs to the first function, the compares the resultant dictionaries, removes keys of the first dictionary not contained in the second dictionary, and reduces the values of keys of the first dictionary to their respective values in the second dictionary if their values in first dictionary is greater than the second dictionary.
Then the second function gets the product of all keys ^ values of the first dictionary.
The third function applies gcd() recursively to the list of numbers if there are more than two numbers.
This is the code, it is fully functional, and works properly if the inputs are valid:
import math
import sys

def factors(n):
    factors = {'1': 1}
    f = 2
    while f <= int(math.sqrt(n)):
        while n % f == 0:
            if f'{f}' in factors.keys():
                factors.update({f'{f}': factors[f'{f}'] + 1})
            else:
                factors[f'{f}'] = 1
            n = int(n / f)
        f += 1
    if n > 1: factors[f'{n}'] = 1
    return factors

def gcd(x, y):
    f1 = factors(x)
    f2 = factors(y)
    for f in f1.copy().keys():
        if f not in f2.keys():
            f1.pop(f)
        elif f1[f] > f2[f]:
            f1[f'{f}'] = f2[f]
    cd = 1
    for f in f1.keys():
        cd *= int(f) ** f1[f]
    return cd

def main(args):
    args = list(map(int, args))
    cd = gcd(args[0], args[1])
    if len(args) > 2:
        for i in args[2:]:
            cd = gcd(cd, i)
    print(cd)

args = sys.argv[1:]
main(args)

Currently I think two areas need be improved:
1, I need a better way than dictionaries to keep track of the divisors, so that I can easily find divisors not contained in the other number and find the lower power of the same divisor.
2, make the gcd() function do what main() function does internally so that the main() function isn't needed, currently I can't figure out a way to do this.
How can this script be improved?

It's more common to use Euler's method to find GCD, rather than factorising both numbers.  However, I'll continue reviewing with the existing algorithm, as there's useful insights to be found.

First, let's look at factors().
Our factors variable is being used as a counter or multiset, and Python provides us with a collections.Counter class for that.
With from collections import Counter, we can replace

        if f'{f}' in factors.keys():
            factors.update({f'{f}': factors[f'{f}'] + 1})
        else:
            factors[f'{f}'] = 1

with
        factors[f] += 1

(I've also changed to using the numbers themselves as keys, instead of converting to string).
The division int(n / f) can be rewritten using integer divide operator n // f (and we know the result will be exact, as we tested n % f == 0).  Also, we should use math.isqrt() rather than int(math.sqrt()) when we want an integer.
from collections import Counter

def factors(n):
    factors = Counter({1: 1, n: 1})
    for f in range(2, 1 + math.sqrt(n)):
        while n % f == 0:
            factors[f] += 1
            n = n // f
    return factors

Now let's look at gcd().
We are computing the intersection of the two multisets.  The operator & does exactly that for us:
common_factors = f1 & f2
cd = 1
for f in common_factors.keys():
    cd *= f ** common_factors[f]

When we iterate over a dictionary, we don't need to get the keys and then index again.  We can iterate over its items() instead, like this:
for f,count in common_factors.items():
    cd *= f ** count

Or we could expand the multiset using elements():
for f in common_factors.elements():
    cd *= f

This allows us to then use math.prod() instead of our own loop:
return math.prod(common_factors.elements())

The whole lot then becomes a one-liner:
def gcd(x, y):
    return math.prod((factors(x) & factors(y)).elements())

Next, main().  The heart of this function is what a functional programmer would call reduce, and - you guessed it - Python provides a reduce() function, in functools:
def main(args):
    args = map(int, args)
    print(reduce(gcd, args))

Finally, it's good practice to use a main guard, so the program can become a module:
if __name__ == "__main__":
    args = sys.argv[1:]
    main(args)

Simplified code
import math
import sys
from collections import Counter
from functools import reduce

def factors(n):
    factors = Counter({1: 1, n: 1})
    for f in range(2, int(math.sqrt(n))):
        while n % f == 0:
            factors[f] += 1
            n = n // f
    return factors

def gcd(x, y):
    return math.prod((factors(x) & factors(y)).elements())

if __name__ == "__main__":
    print(reduce(gcd, map(int, sys.argv[1:])))


Swapping adjacent nodes of a linked list I just have a small question.  The code I wrote works well for the problem. But is it the best way, or can we make it better?

struct node * swapAdjacent(struct node * list)
{
   struct node * temp,*curr,*nextNode;
   temp = list;
   curr = temp->next;
   if(curr == NULL)
      return temp;
   nextNode = curr->next;
   curr->next = temp;
   if(nextNode == NULL)
   {
       temp->next = nextNode;
       return curr;
   }
   temp->next = swapAdjacent(nextNode);
   return curr;
}

Consider having swapAdjacent(NULL) return NULL.  Once it does, you can get rid of the whole if (nextNode == NULL) statement, and just unconditionally say temp->next = swapAdjacent(nextNode);.
You might want to use a couple of guard clauses to separate the null checks from the other stuff.  That can make the steps easier to follow.

With those things done:

struct node * swapAdjacent(struct node * list) {
   struct node *temp, *curr, *nextNode;

   if (!list) return NULL;
   if (!list->next) return list;

   temp = list;
   curr = list->next;
   nextNode = curr->next;

   curr->next = temp;
   temp->next = swapAdjacent(nextNode);
   return curr;
}

The meanings of the names curr, temp, and nextNode are a bit foggy.  I'd change the names to something that unambiguously refers to the nodes' positions in the list either before or after the swap.
Frankly, temp and nextNode could probably go away, and the resulting code would be simpler for it.

Watch:

struct node * swapAdjacent(struct node * list) {
   if (!list) return NULL;
   if (!list->next) return list;

   struct node *newHead = list->next;
   list->next = swapAdjacent(newHead->next);
   newHead->next = list;

   return newHead;
}

Note how there's a bit of a rotation of values going on.  It was going on in the original code, too...but it was harder to see with the variables in the way.


In-lining InvocationExpressions This is regards to one of my answers: Linq Expression Calling Combines. The question wasn't very well received, but it proved to be an interesting task that I enjoyed working on.

I ended up with the following:

public static class ExpressionHelpers
{
    public static TExpressionType InlineInvokes<TExpressionType>(this TExpressionType expression)
        where TExpressionType : Expression
    {
        return (TExpressionType)new InvokeInliner().Inline(expression);
    }

    public static Expression InlineInvokes(this InvocationExpression expression)
    {
        return new InvokeInliner().Inline(expression);
    }

    public class InvokeInliner : ExpressionVisitor
    {
        private Stack<Dictionary<ParameterExpression, Expression>> _context = new Stack<Dictionary<ParameterExpression, Expression>>();
        public Expression Inline(Expression expression)
        {
            return Visit(expression);
        }

        protected override Expression VisitInvocation(InvocationExpression e)
        {
            var callingLambda = e.Expression as LambdaExpression;
            if (callingLambda == null) //Fix as per comment
                return base.VisitInvocation(e);
            var currentMapping = new Dictionary<ParameterExpression, Expression>();
            for (var i = 0; i < e.Arguments.Count; i++)
            {
                var argument = Visit(e.Arguments[i]);
                var parameter = callingLambda.Parameters[i];
                if (parameter != argument)
                    currentMapping.Add(parameter, argument);
            }
            _context.Push(currentMapping);
            var result = Visit(callingLambda.Body);
            _context.Pop();
            return result;
        }

        protected override Expression VisitParameter(ParameterExpression e)
        {
            if (_context.Count > 0)
            {
                var currentMapping = _context.Peek();
                if (currentMapping.ContainsKey(e))
                    return currentMapping[e];
            }
            return e;
        }
    }
}

At its core, it simply in-lines Invoke() calls in the expression tree, replacing them with the called expression's code verbatim.

Examples:

INPUT:
Invoke(i => (i + 1), 3)

OUTPUT:
(3 + 1)

INPUT:
i => Invoke((i, j) => (i * j), Invoke(i => (i + 1), i), Invoke(i => (i + 2), i))

OUTPUT:
i => ((i + 1) * (i + 2))

INPUT:
b => Invoke((d, e) => (d * e), Invoke(b => (50 + Invoke(z => (25 + Invoke(h => (h * 8), z)), b)), b), Invoke(c => (c + 2), b))

OUTPUT:
b => ((50 + (25 + (b * 8))) * (b + 2))

Example usage:

Expression<Func<int, int>> f1 = i => i + 1;
Expression<Func<int, int>> f2 = i => i + 2;
Expression<Func<int, int, int>> f3 = (i, j) => i * j;

var p = Expression.Parameter(typeof(int), "i");
var r = Expression
    .Invoke(f3, new[] {
        Expression.Invoke(f1, p),
        Expression.Invoke(f2, p) })
    .InlineInvokes();

Expression<Func<int, int>> lam = Expression.Lambda<Func<int, int>>(r, p);

The only 'issue' I can come up with is that if one were to modify f1 after InlineInvokes() was called, they might be surprised if they were expecting f1 to be captured and lazily executed, but in my opinion, this is probably to be expected (any other opinions?).

In summary, my questions are:

Are there any pitfalls in in-lining invocations (that is, is my idea fundamentally flawed, and cannot be solved 'properly')?
Are there any issues with the code I've written itself?

I ended up using this code a few times, and recently was thinking through a scenario where the above will fail:

var iParam = Expression.Parameter(typeof(int));
var lParam = Expression.Parameter(typeof(int));

var original = Expression.Invoke(
    Expression.Lambda(
        Expression.Invoke(
                Expression.Lambda(
                    Expression.Add(iParam, lParam),
                    iParam),
                Expression.Constant(1)
        ),
        lParam),
    Expression.Constant(2)
);
var transformed = original.InlineInvokes();

The original can be visualized as:

Invoke(Param_0 => Invoke(Param_1 => (Param_1 + Param_0), 1), 2)

Here, the inner invocation is using a parameter from its parent. Since we're using a stack to represent the mappings between parameters, we're not taking into consideration Param_0. Here's the current output:

(1 + Param_0)

Here's the fixed code:

protected override Expression VisitInvocation(InvocationExpression e)
{
    var callingLambda = e.Expression as LambdaExpression;
    if (callingLambda == null)
        return base.VisitInvocation(e);
    var currentMapping = new Dictionary<ParameterExpression, Expression>();
    for (var i = 0; i < e.Arguments.Count; i++)
    {
        var argument = Visit(e.Arguments[i]);
        var parameter = callingLambda.Parameters[i];
        if (parameter != argument)
            currentMapping.Add(parameter, argument);
    }
    if (_context.Count > 0)
    {
        var existingContext = _context.Peek();
        foreach (var kvp in existingContext)
        {
            if (!currentMapping.ContainsKey(kvp.Key))
                currentMapping[kvp.Key] = kvp.Value;
        }
    }

    _context.Push(currentMapping);
    var result = Visit(callingLambda.Body);
    _context.Pop();
    return result;
}

Note that we only propagate the parameters if they don't already exist. This is required in situations as follows:

var iParam = Expression.Parameter(typeof(int));
var lParam = Expression.Parameter(typeof(int));

var original = Expression.Invoke(
    Expression.Lambda(
        Expression.Invoke(
                Expression.Lambda(
                    Expression.Add(iParam, lParam),
                    iParam, lParam),
                Expression.Constant(1), Expression.Constant(3)
        ),
        lParam),
    Expression.Constant(2)
);

Invoke(Param_0 => Invoke((Param_1, Param_0) => (Param_1 + Param_0), 1, 3), 2)

Here, the inner invoke is providing a value for Param_0 which takes precedence over what the outer invoke is providing. If we compile that expression (without inlining) - we see the result is 4.

Thus, we must ensure that we only add parameters from our parent context if we haven't already been provided them.