Is there a condensed way of adding multiple event listeners each with different functions to the same group of elements? I would ideally like to add event listeners to a single group of elements, topLevelDesktopNavDropdownItems , in some kind of object syntax like {event: function, event: function}. Or is there another way to do this? Here is my current code:
topLevelDesktopNavDropdownItems.forEach(
    element => element.addEventListener('mouseenter', topLevelNavItemMouseOver)
);

topLevelDesktopNavDropdownItems.forEach(
    element => element.addEventListener('mouseleave', topLevelNavItemMouseOut)
);

topLevelDesktopNavDropdownItems.forEach(
    element => element.addEventListener('keydown', topLevelNavItemLinkKeyDown)
);

Instead of iterating over the same array 3 times, iterate over it once:
topLevelDesktopNavDropdownItems.forEach((element) => {
  element.addEventListener('mouseenter', topLevelNavItemMouseOver);
  element.addEventListener('mouseleave', topLevelNavItemMouseOut);
  element.addEventListener('keydown', topLevelNavItemLinkKeyDown);
});

If you think that's too repetitive, you can put the listeners into an object and iterate over the object:
const handlers = {
  mouseenter: topLevelNavItemMouseOver,
  mouseleave: topLevelNavItemMouseOut,
  keydown: topLevelNavItemLinkKeyDown,
};
for (const element of topLevelDesktopNavDropdownItems) {
  for (const [event, handler] of Object.entries(handlers)) {
    element.addEventListener(event, handler);
  }
}

A couple suggestions regarding naming:

The mouseover event is not the same thing as the mouseenter event, Similarly, mouseleave is not exactly the same as mouseout. (One version propagates via bubbling/capturing, the other doesn't. If the elements have children, this makes a big difference.) For precision, consider naming the handlers exactly the same as the event they're listening for.

If you have lots of handlers like this, prefixing them all with topLevelNavItem may get undesirably verbose. To isolate their scope to hopefully make the prefixes less necessary, consider defining them inside a function:

const addDropdownListeners = () => {
  const mouseOverHandler = (event) => {
    // ...
  };
  // ...

Or define them inline with the object:
const dropdownHandlers = {
  mouseenter: (event) => {
    // ...
  }
  // ...


Calculator ViewController Coming from Java and Android development and Objective-C still is a bit strange to me, but I just went in head first and started writing code. Before I started getting really far into it, I just want to throw some basic code here for suggestions about correct syntax, etc. I just want to make sure that my variable and function declarations are correct as well as the way I refer to UI elements is the recommended method.

I was told to make a separate ViewController class for each UI I build as below:

@interface CalculatorViewController : UITableViewController

@property (weak, nonatomic) IBOutlet UITextField *fatTextField;
@property (weak, nonatomic) IBOutlet UITextField *carbsTextField;
@property (weak, nonatomic) IBOutlet UITextField *fiberTextField;
@property (weak, nonatomic) IBOutlet UITextField *proteinTextField;
@property (weak, nonatomic) IBOutlet UITextField *servingsTextField;
@property (weak, nonatomic) IBOutlet UITextField *foodnameTextField;
@property (weak, nonatomic) IBOutlet UILabel *pointsLabel;

- (IBAction)calculate:(id)sender;
- (IBAction)reset:(id)sender;
- (IBAction)addtotracker:(id)sender;

- (void)resetCalculator;
- (void)calculatePoints;

Then inside the CalculatorViewController.m file I ended up with the following code snippets:

// UI FUNCTIONS
- (void) resetCalculator {

    // Reset all nutrion fact fields.
    _fatTextField.text = @"";
    _carbsTextField.text = @"";
    _fiberTextField.text = @"";
    _proteinTextField.text = @"";
    _servingsTextField.text = @"";

    // Reset food name & points label
    _foodnameTextField.text = @"";
    _pointsLabel.text = @"Points: 0";

    // Set Fat as firstresponder
    [self.fatTextField becomeFirstResponder];
}

- (void) calculatePoints {
    float fatFloat = [_fatTextField.text floatValue];
    float carbsFloat = [_carbsTextField.text floatValue];
    float fiberFloat = [_fiberTextField.text floatValue];
    float proteinFloat = [_proteinTextField.text floatValue];
    float servingsFloat = [_servingsTextField.text floatValue];

    float points = proteinFloat + carbsFloat + fatFloat - fiberFloat;
    points = points / 100;
    points = roundf(points);
    points = MAX(points, 0);

    _pointsLabel.text = [NSString stringWithFormat:@"Points: %f", points];
}

- (IBAction)calculate:(id)sender {
    NSLog(@"Calculate button pressed!");

    // Close the keyboard
    [self.view endEditing:YES];
}

- (IBAction)reset:(id)sender {
    NSLog(@"Reset button pressed!");

    [self resetCalculator];
}

- (IBAction)addtotracker:(id)sender {
    NSLog(@"Tracker button pressed!");
}

There are a lot of takes on how to structure things in Objective-C, largely based on the age of the language.  It used to require you to use x, but now it lets you use y, kind of things.  I like to go with the minimalist/OOAD-purist approach.

Your Header File

Your header file (CalculatorViewController.h) is your public interface for your class.  This is what every other class sees when it compiles against it and has access to.  As such, I keep it light.  You don't want another class to set your fatTextField and if you did, you wouldn't want them to know the intimate details of how your form works so you'd pass a model object or something.  I like my header to look like this:

@class ImportantStuff; // 4
@protocol CalculatorViewControllerDelegate; // 5

@interface CalculatorViewController : UITableViewController
- (id)initWithImportantStuff:(ImportantStuff *)importantStuff; // 1
@property (nonatomic, weak) id<CalculatorViewControllerDelegate> delegate; // 2
@property (nonatomic, readonly) ImportantStuff *importantStuff; // 3
@end

@protocol CalculatorViewControllerDelegate <NSObject> // 6
- (void)calculatorViewController:(CalculatorViewController *)calculatorViewController
    didFinishWithImportantStuff:(ImportantStuff *)importantStuff;
@end

Highlighted lines:

Create an initializer with everything you need to get the class started on work.
Add a delegate or some other message passing pattern to send notifications back.
Maybe, have some exposed properties for information that interfacing classes may want.
As general practice, you should only reference the bare minimum number of headers in your header since it reduces circular dependencies and compile time. @class tells the compiler that a class with that name exists and it shouldn't worry about the details.
With delegates you often have a chicken and egg scenario.  The class references the protocol and the protocol references the class.  Like with @class you can say you'll inherit it later.
Populate your delegate with meaningful hooks.

Your Implementation File, Class Extension

Now that you've removed all those properties from your public header, you're getting compiler errors and need to stick them somewhere.  The right spot is in your class extension, the thing that looks like a category but isn't.  Xcode should have generated it as follows at the top of CalculatorViewController.m

@interface CalculatorViewController ()
@end

Just stick all your @property's there.

As far as the method declarations, you don't need them.  In C you need to declare functions ahead of time, but not with modern Objective-C.  The only reason you need to declare them is if you're trying to expose them outside of your implementation.

Your Implementation File, Implementation

Things look good here.  Just a couple things:

Directly accessing instance variables (ivars), e.g. _pointsLabel has gone out of fashion.  There's a lot of reasons for this with potential risks associated with both doing it and not doing it, but there's even a pedantic compiler warning you can turn on to catch it.  Generally, you should use ivars only in getters, setters, and initializers, places where you need direct access or want to suppress side effects.
There's no reason to have both -reset: and -resetCalculator.  There are two interesting points here: First, somewhere in the codebase exists #define IBAction void so you can turn any void returning function into an IBAction assuming it has the right arguments. Second, controller actions support three structures: -(IBAction)myAction:sender, -(IBAction)myAction:sender event:event and -(IBAction)myAction with no arguments.  You can just use -(IBAction)resetCalculator.
addToTracker, with camel case. Probably just a typo considering everywhere else.  Be aware that Interface Builder provides zero compile time checking so if you change the signature or remove a method you must edit it in Interface Builder or it will crash your app.

The most important thing

No matter what style, or principles, or philosophies you adopt as an iOS developer, every other iOS developer will disagree with you, and you with them.  Maybe it's the awesomeness/awfulness of the language or the tools or the platform or the status, but its a very opinionated group of developers.


NRPE script for monitoring load average This script converts a server load value into percentage, compares it with non desired state ($warning and $critical) and returns all information in NRPE (Nagios Remote Plugin Executor) friendly output.

Desired output:

CPU_CORES=1 load=0.30 | load_in_percent=30;95;100
(And the main thing exit code.)

Goal: Use so much less external commands how is possible. Why? I would like to have the script independent. The second thing is just training purpose of bash scripting.

#!/usr/bin/env bash

set -o errexit -o pipefail

warning=95
critical=100

# 1. Find out a load was before 15 minutes.
# citation: "Frankly, if your box spikes above 1.0 on the one-minute average, you're still fine. It's when the 15-minute average goes north of 1.0 and stays there that you need to snap to."
# source: Andre Lewis, https://scoutapm.com/blog/understanding-load-averages
read -r _ _ quarter_load_average _ _ < /proc/loadavg

# 2. Count all cores.
# citation: "How the cor es are spread out over CPUs doesn't matter. Two quad-cores == four dual-cores == eight single-cores. It's all eight cores for these purposes."
# source: Andre Lewis, https://scoutapm.com/blog/understanding-load-averages
while read -a row; do
  if [[ ${row[0]} = "cpu" ]] && [[ ${row[1]} = "cores" ]]; then
    cpu_cores=$(( cpu_cores + "${row[3]}" ))
  fi
done < /proc/cpuinfo

# 3. Convert load value into percentage.
# citation: "Sometimes it's helpful to consider load as percentage of the available resources (the load value divided by the number or cores).
# source: https://access.redhat.com/solutions/30554
load_mlt=$(echo "$quarter_load_average*100/1" | bc)
load_prc=$(( "$load_mlt" / "$cpu_cores" ))

# 4. Compare result with desired status and prepare returned value.
if [[ -z "$load_prc" ]]; then
  returned_text="LOAD UNKNOWN - check script"
  returned_code=3
else
  returned_text="CPU_CORES=$cpu_cores load=$quarter_load_average | load_in_percent=$load_prc;$warning;$critical"
  if [[ $load_prc -gt $critical ]]; then
    returned_code=2
  elif [[ $load_prc -gt $warning ]]; then
    returned_code=1
  else
    returned_code=0
  fi
fi

echo "$returned_text"
exit $returned_code

This looks pretty good. Some suggestions:

[[ is preferred over [ in Bash.
This script could benefit from set -o errexit -o nounset -o pipefail. You'll need to set cpu_cores=0 before referring to it in the loop, but that's just best practice.
Dividing by one isn't going to do much :)
The quotes are a cute feature, but I'd much rather see a summary of the point in them than the full sentences. Something like

  Treat 15 minute load average above 1 as critical.

A useful tactic I've found when dealing with comments is to think how you could name things to avoid comments altogether. Code can't lie, and names are very much like comments enshrined in code, so if you for example rename minute15 to quarter_load_average or something else human readable you should be able to remove the

  Find out a load was before 15 minutes.

comment above it.
Since you're only interested in one row from /proc/cpuinfo I would grep for it. Your goal of making the script stand-alone should be weighed against at least

maintainability, which will suffer greatly if you only use builtin commands,
speed, which is already not great for read, and
availability of grep, which is pretty much universal by now.

You can redirect a line to standard input using some_command <<< "$line" to avoid redundant echos. So echo "$quarter_load_average*100/1" | bc could instead be bc <<< "$quarter_load_average*100/1".


Blocking send and receive algorithm in Go I'm sending and receiving data over an interface (serial in this case) with the following behaviour:

The receiver sends back an Ack message if a message is delivered successfully.
If an Ack is not received within a timeout, the command should be re-sent limited a number of times.
Multiple clients may request sending at the same time, but no new commands should be sent until an Ack for the last command is received or it times out. Other sending commands shall wait until the channel is unreserved or they time out.

The algorithm to do this is fairly simple. (Feel free to say so if you think it can be improved):

The implementation in Go works as expected. I just feel there might be a smarter way to do it:
package main

import (
    "fmt"
    "math/rand"
    "time"
)

/****** Simulating the receiver behaviour ******/

var receiveChannel = make(chan string, 1)

// Used to read messages from the interface
func Read() string {
    return <-receiveChannel
}

// Used to write messages to the interface
func Write(data string) {
    // Randomly drop 50% of packets
    if n := rand.Intn(100); n < 50 {
        receiveChannel <- "ACK"
    }
}

/*******************************************/

var ackChannel = make(chan bool, 1)
var sendChannel = make(chan string, 1)

func run() {
    for {
        if Read() == "ACK" {
            ackChannel <- true
        }
    }
}

func sendWrapper(data string) error {
    // Reserve the sending channel
    timeoutTimer := time.NewTimer(1 * time.Second)
    select {
    // This should block until sendChannel has a free spot or times out
    case sendChannel <- data:
        timeoutTimer.Stop()
        fmt.Printf("Send chan reserved for command id=%v\n", data)
    case <-timeoutTimer.C:
        return fmt.Errorf("timed out while waiting for send channel to be free. id=%v", data)
    }

    attempts := 2
    err := send(data, attempts)

    // Free up the sending channel
    select {
    case x := <-sendChannel:
        fmt.Printf("Send channel cleared %v\n", x)
    default:
        fmt.Printf("Send channel is empty. This should never happen!\n")
    }

    return err
}

func send(data string, attempts int) error {
    // Send data
    Write(data)

    // Wait for an ACK to be received
    ackTimeoutTimer := time.NewTimer(time.Millisecond * 100)
    select {
    case <-ackChannel:
        ackTimeoutTimer.Stop()
    case <-ackTimeoutTimer.C:
        // Retry again
        if attempts > 1 {
            return send(data, attempts-1)
        }
        return fmt.Errorf("Timed out while waiting for ack. id=%v", data)
    }
    fmt.Printf("Frame sent and acked id=%v\n", data)
    return nil
}

/****** Testing ******/

func main() {
    go run()

    // Multiple goroutines sending data
    for i := 0; i < 7; i++ {
        go func() {
            x := i
            if err := sendWrapper(fmt.Sprint(x)); err != nil {
                fmt.Println(err.Error())
            } else {
                fmt.Printf("Sending successful. i=%v\n", x)
            }
        }()
        time.Sleep(10 * time.Millisecond)
    }
    time.Sleep(4 * time.Second)
}

You have at least one bug.

The closure from your code:

package main

import (
    "fmt"
    "time"
)

func main() {
    for i := 0; i < 7; i++ {
        go func() {
            x := i
            fmt.Printf("Sending successful. i=%v\n", x)
        }()
    }
    time.Sleep(4 * time.Second)
}

Output:

$ go run closure.go
Sending successful. i=7
Sending successful. i=7
Sending successful. i=7
Sending successful. i=7
Sending successful. i=7
Sending successful. i=7
Sending successful. i=7
$

The x := i statement is in the wrong place. It should be:

package main

import (
    "fmt"
    "time"
)

func main() {
    for i := 0; i < 7; i++ {
        x := i
        go func() {
            fmt.Printf("Sending successful. i=%v\n", x)
        }()
    }
    time.Sleep(4 * time.Second)
}

Output:

$ go run closure.go
Sending successful. i=4
Sending successful. i=5
Sending successful. i=1
Sending successful. i=3
Sending successful. i=2
Sending successful. i=0
Sending successful. i=6
$

Reference:

Go: Frequently Asked Questions (FAQ)

What happens with closures running as
goroutines?


Function for simple navigation of dataframe in console I was looking for a way to navigate through a dataframe in R using just the console that displays every n lines. There are a few functions you can find at StackOverflow that attempt to answer this but none are quite what I want. So I made my own function but it has a lot of if statements. How can I refactor my code to make it more concise and clean?

Note: I chose "." and "/" for page up and page down because code completion in RStudio automatically turns "[" into "[ ]". I'm open to any alternative symbols.

Here is the code. It is called "less" but it is obviously missing most of the functions in the Unix version of less.

less <- function(x, n = 10) {
  begin = 1
  end = n

  repeat {
    print(
      x[begin:end, ]
    )

    input <- readline("Type . to scroll up, type / to scroll down: ")

    if(input == ".") {
      begin = begin - n
      end = end - n
    }

    if(input == "/") {
      begin = begin + n
      end = end + n
    }

    if(end >= nrow(x)) break

    if(end < 1) break
  }
}

Without changing the interface too much, your code could use the following changes. Note that I am fixing three bugs regarding corner cases:

if your data has fewer than n rows to start with (see where I used min)
if your data has a single column (see where I used drop = FALSE)
make sure that the last page will always show

I avoided duplicating the statements  where begin and end are reset, by creating an increment variable. The code computing the increment (of +n or -n) has been moved to its own function. One improvement I've made is that the function will call itself again via Recall() in case the user entered something other than the expected . or /, i.e. it will prompt the user again.

less <- function(x, n = 10) {
  begin <- 1
  end   <- n

  get_increment <- function() {
    input <- readline("Type . to scroll up, type / to scroll down: ")
    if (input == ".") -n else if (input == "/") +n else Recall()
  }

  repeat {
    print(x[begin:min(end, nrow(x)), , drop = FALSE])

    increment <- get_increment()
    begin     <- begin + increment
    end       <- end   + increment

    if(begin > nrow(x) | end < 1) break
  }
}

Now regarding your design in general, I feel it would be nice to pre-split your data.frame every n rows, and let the user know that there are for example 12 pages and that he is currently looking at page 4/12. In the prompt, also allow the user to jump to a specific page in addition to the next or previous page. Finally, make it easier to exit via a specific code like "q" rather than wait for the user to go beyond the first or last page. Here is my attempt:

less <- function(x, n = 10) {

   N <- nrow(x)
   if (N == 0) {
      message("input has no rows")
      return(NULL)
   }

   row_page_idx <- (seq_len(N) - 1) %/% n
   pages <- split(x, row_page_idx)
   num_pages <- length(pages)
   page_idx <- 1

   get_new_page_idx <- function() {
      input <- readline('Type a page number, ">" for next, "<" for previous, or "q" to quit: ')
      if (input == "q") return(0)
      if (input == ">") return(min(num_pages, page_idx + 1))
      if (input == "<") return(max(1, page_idx - 1))
      integer <- suppressWarnings(as.integer(input))
      if (!is.na(integer) & integer %in% 1:num_pages) return(integer)
      Recall() # if all fails
   }

   repeat {
      print(pages[[page_idx]])
      message("Page ", page_idx, " of ", num_pages)
      page_idx <- get_new_page_idx()
      if(page_idx == 0) break
   }
}


Configuration class in C#, reading an INI file This class will load a file with INI configuration elements and cache them, it then has a method to get an item from the cache if it needs to.

internal class ConfigHandler
{
    private static readonly ILogger Logger = LogManager.GetCurrentClassLogger();

    private Dictionary<string, string> _configItems;

    public ConfigHandler()
    {
        _configItems = new Dictionary<string, string>();
        LoadConfig("resources/config/pitaya.ini");
    }

    internal void LoadConfig(string configFile)
    {
        try
        {
            if (!File.Exists(configFile))
            {
                Logger.Error("Your config file couldn't be found.");

                Console.ReadKey(true);
                Environment.Exit(0);
            }

            if (CoreUtilities.IsFileLocked(new FileInfo(configFile)))
            {
                Logger.Error("Your config file is already open somewhere.");

                Console.ReadKey(true);
                Environment.Exit(0);
            }

            _configItems = File.ReadLines(configFile).Where(IsConfigurationLine).Select(line => line.Split('=')).ToDictionary(line => line[0], line => line[1]);
        }
        catch (Exception exception)
        {
            Logger.Fatal(exception, "Error while loading config.");

            Console.ReadKey(true);
            Environment.Exit(0);
        }
    }

    private static bool IsConfigurationLine(string line)
    {
        return !line.StartsWith("#") && line.Contains("=");
    }

    internal string GetConfigValueByKey(string key)
    {
        if (!_configItems.TryGetValue(key, out var value))
        {
            Logger.Warn("Missing configuration key `" + key + "`");
        }

        return value;
    }
}

General

This class can't be reused because the path to the ini file is hardcoded. Having a parameter string path and using it to load the configuration would be a better way.

LoadConfig()

I don't like the Console.ReadKey(bool) and I don't see why the method should be able to terminate the process. In addition passing 0 to the Exit() method indicates that the process completed successfully which seems wrong.

The line where you assign the key/values to the dictionary is too long and therefor hard to read.

IMO the caller of the code should do the logging and you should let the exceptions bubble up the line.

Checking if the file is locked will pretence that it is safe to read the file, but it could happen that just after the check the file is opened and locked.

GetConfigValueByKey()

It seems strange that a not found key only will be logged. A better way would be to just use the bool TryGetXxx style.

How I would do this:

I would add an overloaded public constructor which takes a string fileName parameter. I would then make the parameterless constructor internal and let it call the public constructor with the "resources/config/pitaya.ini" to simulate the former functionality.

I would validate the fileName parameter and would throw the expected exceptions.

I would just initialize the dictionary in the constructor and if an exception occurs I would let it bubble up.

Like so
public class ConfigHandler
{
    private readonly Dictionary<string, string> _configItems = new Dictionary<string, string>();

    internal ConfigHandler()
        : this("resources/config/pitaya.ini")
    { }

    public ConfigHandler(string fileName)
    {
        if (fileName == null)
        {
            throw new ArgumentNullException(nameof(fileName));
        }

        if (string.IsNullOrWhiteSpace(fileName))
        {
            throw new ArgumentException($"The parameter {nameof(fileName)} may not be empty or whitespace", nameof(fileName));
        }

        if (File.Exists(fileName) == false)
        {
            throw new FileNotFoundException("File could not be found.", fileName);
        }

        _configItems = File.ReadLines(fileName)
                           .Where(IsConfigurationLine)
                           .Select(line => line.Split('='))
                           .ToDictionary(line => line[0], line => line[1]);
    }

    private static bool IsConfigurationLine(string line)
    {
        return !line.StartsWith("#") && line.Contains("=");
    }

    public bool TryGetValue(string key, out string value)
    {
        if (key == null)
        {
            throw new ArgumentNullException(nameof(key));
        }

        return _configItems.TryGetValue(key, out value);
    }
}


Performance difference between declaring Regex inside or outside of often-called method. I am curious as to what the performance/functional differences are between:

private bool OftenCalledMethod(string s)
{
    Regex reg = new Regex(@"^matchme$", RegexOptions.IgnoreCase);
    return (reg.IsMatch(s));
}

Versus:

readonly Regex reg = new Regex(@"^matchme$", RegexOptions.IgnoreCase);
private bool OftenCalledMethod(string s)
{
    return (reg.IsMatch(s));
}

Does the compiler optimize the former in such a way that it doesn't matter? Or is the object reg being allocated repeatedly and unnecessarily?

I would go with the former unless you profile your code and it tells you that the constructor for Regex is using a significant amount of time.

Creating the Regex inside your method makes the clear statement that this given Regex is being used by only this method.  The compiler doesn't care, but someone maintaining your code in the future might care.

Running a profiler is necessary for really optimizing your code.  There are some things that are always likely to be bad (O(n^3) algorithms for instance), but I've learned by experience that I can't tell just by looking where the time is being wasted in my code.  Profile it and see what you learn.

If your profiling results tell you that the constructor of Regex is using too much time, then definitely move it outside the method.  If you do that though, you should add comments explaining why.  (Keep thinking about the poor person maintaining your code in the future, it just might be you.)


First text-based adventure game I'm sure there are better ways to code this.  I spent a lot of time re-working the code to get it as clean as possible.  I would appreciate any input on what I could change to have the program run more efficiently and look cleaner.

outer = False
while outer == False:
    room_list = []
    room = ["This place stinks, better get moving. Door to the N or E", 1, 8, None, None]
    room_list.append(room)
    room = ["You are in a safe room. There are doors to the N,E,S and W", 2, 9, 0, 4]
    room_list.append(room)
    room = ["This room is filled with garbage. There is a door to the S", None, None, 1, None]
    room_list.append(room)
    room = ["You encounter goblins and die!"]
    room_list.append(room)
    room = ["You encounter orcs and kill them all! There is a door to the N,E and S", 3, 1, 5, None]
    room_list.append(room)
    room = ["You encounter rats, easy fight. Theres a door to the N", 4, None, 7, None]
    room_list.append(room)
    room = ["You found a companion! Theres a door to the S", None, None, 9, None]
    room_list.append(room)
    room = ["You found the treasure! Congratulations!"]
    room_list.append(room)
    room = ["Ouch, you were hit by a fireball! There's a door to the W", None, None, None, 0]
    room_list.append(room)
    room = ["You found a treasure map! It says 'Rats dwell, south it lies'. Theres a door to the N and W",6,None,None,1]
    room_list.append(room)
    current_room = 0
    print (room_list[current_room][0])
    inner = False
    new_room = 0
    while inner == False:
        user_choice = input ("What would you like to do? ")
        print ()
        if user_choice in('north','n','North','N'):
            new_room = room_list[current_room][1]
        elif user_choice in('east','e','East','E'):
            new_room = room_list[current_room][2]
        elif user_choice in('south','s','South','S'):
            new_room = room_list[current_room][3]
        elif user_choice in('west','w','West','W'):
            new_room = room_list[current_room][4]
        else:
            print ("I don't know what", user_choice, "means")

        if new_room == None:
            print ("You can't go that way.")
        else:
            current_room = new_room
            print (room_list[new_room][0])

        if current_room == 3:
            play_again = input("Would you like to play again? ")
            if play_again in("y","Y","Yes","yes"):
                print()
                break
                    else:
                        outer = True
                        break
        if current_room == 7:
            outer = True
            break

Actually, the general idea of a list of positions is really neat.  What is not so neat is that you create the list on each iteration.  That is unnecessary.  You should just create the list before the loop, and leave it there.  You also don't need those long lines of .append()s.  Just make the whole list like this:

room_list = [
    ["This place stinks, better get moving. Door to the N or E", 1, 8, None, None],
    ["You are in a safe room. There are doors to the N,E,S and W", 2, 9, 0, 4],
    ["This room is filled with garbage. There is a door to the S", None, None, 1, None],
    ["You encounter goblins and die!"],
    ["You encounter orcs and kill them all! There is a door to the N,E and S", 3, 1, 5, None],
    ["You encounter rats, easy fight. Theres a door to the N", 4, None, 7, None],
    ["You found a companion! Theres a door to the S", None, None, 9, None]
    ["You found the treasure! Congratulations!"],
    ["Ouch, you were hit by a fireball! There's a door to the W", None, None, None, 0],
    ["You found a treasure map! It says 'Rats dwell, south it lies'. Theres a door to the N and W",6,None,None,1],
]

Another thing I would suggest is to have somebody else proofread your sentences.  You have a couple cases of Theres when it should be There's.  You also in many places leave out the Oxford comma.  I will grant that the Oxford comma is controversial, and I will probably get a comment or two in strong disagreement with the use of it.

When you use if ...:, Python sees if ... is a Truthy value, and if it is, executes the suite.  Therefore, if ... == True: is redundant (and sometimes comes out with unexpected results; see this SO answer).  Consequently, if ... == False: could be simply written as if not ...:.  With all of these examples, it works the same way with a while loop.  It is in fact your while loops that bring it up.

You have some strange uses of whitespace.  In Python 3, print is a function.  Therefore, you should use print(...), not print (...).  With the space, it looks like you are using it like a statement and whatever is in the parentheses might be a tuple to be printed instead of a list of arguments that will each be printed.  Then again, you use if ... in(...).  Well, in is not a function.  Therefore, you should add the space to make that more clear that the parentheses are there for the tuple, not as a way of passing arguments.

Your if and elses could be simplified:

rooms = {'n': 1, 'e': 2, 's': 3, 'w': 4}
user_choice = user_choice.lower()
if len(user_choice) != 1 and user_choice not in ('north', 'south', 'east', 'west'):
    print("I don't know what '{}' means".format(user_choice))
    continue
new_room = room_list[current_room][rooms[user_choice[0]]]

According to PEP 8, "Comparisons to singletons like None should always be done with is or is not, never the equality operators."

I like that you check if the user wants to play again, but what if the user types Certainly!?  I can't expect you to make your program expect things like that, but it should at least show that it didn't expect it.  As it is, it exits.  It just assumes that anything not y or yes is no.  You should use a while loop that keeps asking for input until the user types a definite y, yes, n, or no.  Also, your case-insensitivity is nice, but it could be made simpler.  If you just say if plag_again.lower() in ('y', 'yes'), your program will now accept even yEs and YeS.


Building a set of all subsets recursively in C++ Here is my recursive implementation of a power set function in C++.  One thing that concerns me is that I am iterating over the set of sets while modifying its size. The code nevertheless functions as desired. Any feedback would be highly appreciated.

#include <iostream>
#include <set>
#include <vector>

template <typename T>
void power_set(int i, const std::vector<T>& set, std::set<std::set<T>>& set_of_sets)
{
    if (i == set.size())
    {
        set_of_sets.emplace(std::set<T>());  // empty set
        return;
    }

    power_set(i + 1, set, set_of_sets);

    for (auto& subset : set_of_sets)
    {
        auto augmented_subset = subset;
        augmented_subset.insert(set[i]);
        set_of_sets.insert(augmented_subset);
    }

}

int main()
{
    std::vector<std::string> set = {"a", "b", "c"};
    std::set<std::set<std::string>> set_of_sets;
    power_set(0, set, set_of_sets);

    for (auto& subset : set_of_sets)
    {
        std::cout << "{";
        for (auto& i : subset) std::cout << i << ",";
        std::cout << "}\n";
    }
    std::cout << set_of_sets.size() << "\n";
}

The recursion - even simulated via std::stack - is indeed unnecessary. Each subset has a corresponding number in \$[0, 2^n)\$ with bits indicating presence of an element in the subset, and each number in \$[0, 2^n)\$ corresponds to a subset. Consider a pseudocode

Set power_set(Set& the_set)
{
    Set result;
    Integral n = the_set.size()
    for (i = 0; i < (1 << n); i++) {
        result.insert(make_subset(the_set, i);
    }
    return result;
}

Set make_subset(Set& the_set, Integral indicator)
{
    Set result;
    Integral n = the_set.size();
    for (i = 0; i < (1 << n); i <<= 1) {
        if (indicator & i) {
            result.insert(the_set[i]);
        }
    }
    return result;
}


A simple calculator to calculate the frequency of a recessive allele It's based on the Hardy-Weinberg formula. I used the easygui module because I don't fully understand events in Tkinter yet. It's my first relatively useful script so any constructive criticism would be helpful.

from __future__ import division
import sys
from easygui import *
def calculate_frequency(recessive,total):
    q = (recessive/total)**0.5 #square root of q^2
    p = 1-q #since p + q = 1, p = 1-q
    hzDominantNum = round((p**2)*total,2) #number of homozygous dominant, not percent
    hzRecessiveNum = round((q**2)*total,2)
    heterozygousNum = round((2*p*q)*total,2)
    hzDominantFreq = round((p**2)*100,2) #frequency (percent)
    hzRecessiveFreq = round((q**2)*100,2)
    heterozygousFreq = round((2*p*q)*100,2)
    return hzDominantNum,hzRecessiveNum,heterozygousNum,hzDominantFreq,hzRecessiveFreq,heterozygousFreq #returns all calculations to be printed
while True: #loops program until terminated
    msg = "Leave Total as 100 if you want to enter the percent." #main message lets user know how to input as percent
    fieldNames = ["How many have the recessive trait","Total population"]
    fieldValues = ['',100] #first one is blank, the second is defaulted to 100 to make inputing percents easier
    test = 0 #tests if the proper information has been entered. Test will equal 1 if the conditions are met.
    while test == 0:
        fieldValues = multenterbox(msg=msg, title='Enter information about the population', fields=fieldNames, values=fieldValues)
        if fieldValues == None: sys.exit() #None is returned when the user clicks "Cancel"
        elif not fieldValues[0].isdigit() or not fieldValues[1].isdigit():
            if not ccbox("Please fill out all fields.",'ERROR',('OK','Quit')): sys.exit()
        elif int(fieldValues[0])>int(fieldValues[1]):
            if not ccbox("The amount of people with the recessive trait can't be bigger than the total population.",'ERROR',('OK','Quit')): sys.exit()
        else: test=1
    recessive = int(fieldValues[0])
    total = int(fieldValues[1])

    hzDominantNum,hzRecessiveNum,heterozygousNum,hzDominantFreq,hzRecessiveFreq,heterozygousFreq = calculate_frequency(recessive,total)
    #displays all the information
    msg = str(hzDominantNum)+'/'+str(total)+' ('+str(hzDominantFreq)+'%) are normal (homozygous dominant).\n'+'''
    '''+str(heterozygousNum)+'/'+str(total)+' ('+str(heterozygousFreq)+'%) are carriers (heterozygous).\n'+'''
    '''+str(hzRecessiveNum)+'/'+str(total)+' ('+str(hzRecessiveFreq)+'%) have the recessive trait (homozygous recessive).'
    #sees if user wants to quit or continue
    if not ccbox(msg=msg, title='RESULTS', choices=('Continue', 'Quit'), image=None): sys.exit()

Some suggestions:

call variables this_way not thisWay
use less comments
put your interaction with user into separate functions
if code will be splited properly you will be able to user return and while True: instead of test variable
return object or dict from calculate_frequency rather than long tuple
don't create strings like that:

str(hzDominantNum)+'/'+str(total)+' ('+str(hzDominantFreq)+'%) are normal (homozygous dominant).\n'

this is mutch better:

'%f/%f (%d%%) are normal (homozygous dominant).\n' % (hzDominantNum, total, hzDominantFreq)

if you have to run this as a program do it this way:

def main():
    print "test"

if __name__ == "__main__":
    main()


PRNG - Algorithm: Performance vs Readability I have a PRNG-Algorithm:

/**
* Generates an integer. Min: 0 | Max: total - 1
* @param    total The number of possibilities.
* @return The generated number.*/
public function generate_normal(total:int):int
{
    _seed = (_seed * MULTIPLIER) % MODULUS;
    return (_seed * total / MODULUS);
}

The constants are defined as follows:

/**The Multiplier.*/
private const MULTIPLIER:int = 48271;

/**The Modulus.*/
private const MODULUS:int = 2147483647;

This is readable and understandable, but pretty slow (~50% more time).

Simply using the numbers would be faster, but less readable. Will people call these numbers "magic" and kill me or will everything be fine? Should I change this part in the class description:

Modulus suggested 1988 by Stephen K. Park and Keith W. Miller.
Multiplier suggested 1993 by Park and Miller.

to this:

Modulus (2147483647) suggested 1988 by Stephen K. Park and Keith W. Miller.
Multiplier (48271) suggested 1993 by Park and Miller.

Should I keep my approach or use the numbers directly?

TL;DR What's more important, performance or readability?

There is no one answer.

Performance

How often is generate_normal called? Is it called on a program critical path or only during set up?

Could you cache a bunch of numbers before hand and only replenish the cache when you run out?

Readability

How many people will read this code over its lifetime? How often will you have to come back to the code?

Compromise

If you notice a substantial penalty using the private const construct, then do the following:

/**The Multiplier. Suggested 1993 by Park and Miller.*/
private const MULTIPLIER:int = 48271; // folded into code below

/**The Modulus. Suggested 1988 by Stephen K. Park and Keith W. Miller.*/
private const MODULUS:int = 2147483647; // folded into code below

...

public function generate_normal(total:int):int
{
    // Values folded from definitions ...
    // _seed = (_seed * MULTIPLIER) % MODULUS;
    _seed = (_seed * 48271) % 2147483647;
    // return (_seed * total / MODULUS);
    return (_seed * total / 2147483647);
}


Sequence to extend and forget After receiving some feetback about my previous attempt to create reusable, easily extendable sequence generators I try once again to create such a framework.

This time there is no inheritance. Just extension methods.

The new core is the Sequence class:

public class Sequence
{
    public Sequence(int count) { Count = count; }

    public int Count { get; }

    public static Sequence Generate(int count)
    {
        return new Sequence(count);
    }

    public IEnumerable<T> Generate<T>(Func<IEnumerable<T>> generator) => generator().Take(Count);
}

By itself it doesn't do much. You need to extend it with your extensions that hook up to it and provide it with the actual generator for your sequence.

Here are some examples for the Fibonacci sequence:

static class FibonacciSequenceExtensions
{
    public static IEnumerable<T> Fibonacci<T>(this Sequence sequence, T firstTwo, T firstStep, Func<T, T, T> sum)
    {
        return sequence.Generate(() => FibonacciGenerator(firstTwo, firstStep, sum));
    }

    public static IEnumerable<int> Fibonacci(this Sequence sequence, int firstTwo, int firstStep)
    {
        return sequence.Generate(() => FibonacciGenerator(firstTwo, firstStep, (x, y) => x + y));
    }

    public static IEnumerable<TimeSpan> Fibonacci(this Sequence sequence, TimeSpan firstTwo, TimeSpan firstStep)
    {
        return sequence.Generate(() => FibonacciGenerator(firstTwo, firstStep, (x, y) => x + y));
    }

    private static IEnumerable<T> FibonacciGenerator<T>(T firstTwo, T firstStep, Func<T, T, T> sum)
    {
        var preview = firstTwo;
        var current = sum(firstTwo, firstStep);

        yield return preview;
        yield return preview;
        yield return current;

        while (true)
        {
            var newCurrent = sum(preview, current);
            yield return newCurrent;
            preview = current;
            current = newCurrent;
        }
    }
}

Then you can use it like this:

var result = Sequence.Generate(10).Fibonacci(3, 4).ToList();

What's the purpose of this? To have only a single class for all sequences. You don't have to remember where you've implemented some sequence. You can use them all via a single class.

In case someone is interested how the final version looks like I put it here under the previous quesiton because it's the inheritance edition that I decided to keep.

The API looks ok, IMHO, except for this syntax:

Sequence.Generate(10).Generate(...); // huh?

The downside is that for every sequence, I have to write at least two methods. First, method, to generate an actual sequence (FibonacciGenerator()), second method to wire it up to sequence.Generate (Fibonacci<T>()). This looks like a lot of extra code for what essentially could have been:

SequenceExtensions.FibonacciGenerator(...).Take(count);

Some middle ground can be probably achieved if you use Range instead:

Enumerable.Range(0, count) //extend this
          .Fibonacci(3, 4);

This way one method should be enough in most cases. And you can either drop the Sequence class altogether or leave it as a pretty wrapper around Enumerable.Range or similar loop abstraction:

Sequence.Create(10)      //returns basic 0..9 sequence (IEnumerable<int>)
        .Fibonacci(3, 4) //returns Fibonacci sequence, using initial sequence as loop

I am still not convinced that all this boiler plate code is worth the effort though. But oh well. :)

P.S. I think your Fibonacci sequence implementation is a bit unconventional. There is no "step" in classical implementation:

  every number after the first two is the sum of the two preceding ones

So if I were to see Fibonacci(3, 4) in code, I would assume, that 3 and 4 are the first two numbers of the sequence, and that it will go as 3, 4, 7, 11 and not as 3, 3, 7, 10, 17 (3+3 =/= 7).


Checking the presence of certain variables in a dataframe I'm writing an Exploratory Data Analysis using R Markdown. First of all, I need to check the "sanity" of the input data set. Among the other check I perform, I need to check if at least one between the variables var_in and var_out are present in the dataset. If at least one is present, the other one can be computed from the first one. Thus, I want to check which of them is missing, and store its name in a character vector. If both are missing, the analysis is impossible and I need to exit, preferably throwing a meaningful error.

I cannot include the actual dataset on which the check is performed, because it's \$10^6\$ rows with confidential data, so I need to use fake data. The code, however, is as similar to the real code as possible.

# fake data
n <- 10^6
input_df <- data.frame(wind_speed = 10*abs(rnorm(n)), wind_direction = runif(n, 0, 2*pi),
                       var_in = NA, var_out = 3)

# nearly real code
missing_variables <- character(0)

var_in_is_missing <- all(is.na(input_df$var_in))
var_out_is_missing <- all(is.na(input_df$var_out))

if (var_in_is_missing & var_out_is_missing) {
  stop("both var_in and var_out are completely missing, so I cannot continue the EDA")
}
if (var_in_is_missing) {
  missing_variables <- c("var_in", missing_variables)
}
if (var_out_is_missing) {
  missing_variables <- c("var_out", missing_variables)
}

Note that according to the principle of early return, I put the stopping test before the other two. The code runs, but it doesn't seem that readable to me:

I test both var_in_is_missing and var_out_is_missing twice: this is definitely not going to impact notably the performance of my code, but it still feels useless
is it really necessary to use three separate if statements? Isn't there a way to do it in R in a more...compact way, without sacrificing readability?

I would do something like this:

tt <- function() {
  naI <- sapply(input_df[, c('var_in', 'var_out')], function(x) all(is.na(x)))
  if (sum(naI) == 2) {
    stop("both var_in and var_out are completely missing, so I cannot continue the EDA")
  } else {
    missing_variables <- names(naI[naI == TRUE])
  }
  missing_variables
}

I put the code into function for testing. Tests:

input_df <- data.frame(var_in = NA, var_out = 3)
tt()
#> [1] "var_in"
input_df <- data.frame(var_in = NA, var_out = NA)
tt()
#> Error in tt(): both var_in and var_out are completely missing, so I cannot continue the EDA
input_df <- data.frame(var_in = 3, var_out = NA)
tt()
#> [1] "var_out"
input_df <- data.frame(var_in = 3, var_out = 3)
tt()
#> character(0)

P.S. maybe you also want to add some kind of treatment if the one/both columns are missing in data.frame...


Stone, Paper, Scissors in Python Recently I started programming with python.
Today I was trying to make a stone, paper, scissor game. After so scratching my head long time, finally I got a working code.

from random import choice

def play_again():
        print "Do you want to play again:- Choose 'yay' or 'nay'"
        user_again = raw_input('->')
        if user_again == 'yay':
            SPCa()
        elif user_again == 'nay':
            print "Ok bye! hope you enjoyed the game. See you soon! :)"
        else:
            print "Please choose correct option."
            play_again()

def SPCa():
    computer_choice = choice( ['stone', 'paper', 'scissor'] )
    computer_choosed =  "Computer choosed %s" % computer_choice
    print "Make an choice"
    print "Choose stone, paper, scissor"
    user_choice = raw_input('->')

    if user_choice == computer_choice:
        print computer_choosed
        print "So it's a tie"
        play_again()

    elif user_choice == 'stone':
        if computer_choice == 'paper':
            print computer_choosed
            print "So, You loose"
            play_again()
        elif computer_choice == 'scissor':
            print computer_choosed
            print "So, Cheers! You won!"
            play_again()

    elif user_choice == 'paper':
        if computer_choice == 'scissor':
            print computer_choosed
            print "So, you loose."
           play_again()
        elif computer_choice == 'stone':
            print computer_choosed
            print "So, Cheers! You won!"
            play_again()

   elif user_choice == 'scissor':
        if computer_choice == 'stone':
            print computer_choosed
            print "So, you loose."
            play_again()
        elif computer_choice == 'paper':
            print computer_choosed
            print "So, Cheers! You won!"
            play_again()
    else:
         print "please choose correct option"
        SPCa()

def SPC():
    computer_choice = choice( ['stone', 'paper', 'scissor'] )
    computer_choosed =  "Computer choosed %s" % computer_choice
    print "You are playing Stone, Paper, Scissor."
    print "Make an choice"
    print "Choose stone, paper, scissor"
    user_choice = raw_input('->')

    if user_choice == computer_choice:
        print computer_choosed
        print "So it's a tie"
        play_again()

   elif user_choice == 'stone':
        if computer_choice == 'paper':
            print computer_choosed
            print "So, You loose"
            play_again()
        elif computer_choice == 'scissor':
            print computer_choosed
            print "So, Cheers! You won!"
            play_again()

    elif user_choice == 'paper':
        if computer_choice == 'scissor':
            print computer_choosed
            print "So, you loose."
            play_again()
        elif computer_choice == 'stone':
            print computer_choosed
            print "So, Cheers! You won!"
            play_again()

    elif user_choice == 'scissor':
        if computer_choice == 'stone':
            print computer_choosed
            print "So, you loose."
            play_again()
        elif computer_choice == 'paper':
            print computer_choosed
            print "So, Cheers! You won!"
            play_again()
    else:
        print "please choose correct option"
        SPCa()
SPC()

But I found my code so repetitive, it has not even a single loop. Please suggest how can I improve my code. What are things that I should learn?

Little issues

Your indentations seems wrong : too many spaces in a few places, not enough in a few other places. Whitespaces matter in Python so this is definitly something you should fix before going any further.

Style

Python has a style guide called PEP 8 which is definitly worth reading and and worth following if you do not have good reasons not to. In you case, your usage of whitespaces around parenthesis and the trailing whitespaces for instance are not compliant to PEP8. You'll find tools online to check your code compliancy to PEP8 in a automated way if you want to. This could also help you to detect and fix your indentation issues.

Don't Repeat Yourself

You've already realised that your code was repeating itself and that is was a bad thing. Let's see how this can be improved.

The only difference between SPCa and SPC is that SPC has an additional line printed at the beginning. It might be easier to replace all the calls to SPC() by a call to print and then a call to SPCa. Once this is done, we can get rid of SPC and maybe rename SPCa in play_game.

At this stage, the code looks like:

from random import choice

def play_again():
    print "Do you want to play again:- Choose 'yay' or 'nay'"
    user_again = raw_input('->')
    if user_again == 'yay':
        play_game()
    elif user_again == 'nay':
        print "Ok bye! hope you enjoyed the game. See you soon! :)"
    else:
        print "Please choose correct option."
        play_again()

def play_game():
    computer_choice = choice(['stone', 'paper', 'scissor'])
    computer_choosed = "Computer choosed %s" % computer_choice
    print "Make an choice"
    print "Choose stone, paper, scissor"
    user_choice = raw_input('->')

    if user_choice == computer_choice:
        print computer_choosed
        print "So it's a tie"
        play_again()

    elif user_choice == 'stone':
        if computer_choice == 'paper':
            print computer_choosed
            print "So, You loose"
            play_again()
        elif computer_choice == 'scissor':
            print computer_choosed
            print "So, Cheers! You won!"
            play_again()

    elif user_choice == 'paper':
        if computer_choice == 'scissor':
            print computer_choosed
            print "So, you loose."
            play_again()
        elif computer_choice == 'stone':
            print computer_choosed
            print "So, Cheers! You won!"
            play_again()

    elif user_choice == 'scissor':
        if computer_choice == 'stone':
            print computer_choosed
            print "So, you loose."
            play_again()
        elif computer_choice == 'paper':
            print computer_choosed
            print "So, Cheers! You won!"
            play_again()
    else:
        print "please choose correct option"
        play_game()

print "You are playing Stone, Paper, Scissor."
play_game()

if main guard

In Python, it is a good habit to move your code actually doing things (by opposition to merely defining things) behind an if __name__ == "__main__": guard. This is useful if you want to reuse the code : you can import the file and get all the benefits from it (the definition of values/functions/classes) without having it performing unwanted actions.

In your code, the end of the script becomes :

if __name__ == "__main__":
    # execute only if run as a script
    print "You are playing Stone, Paper, Scissor."
    play_game()

Now we can get into the actual changes in your code. One of the issue it that you have multiple functions calling each other which make things difficult to understand.

All branches in play_game end up calling play_again (except when the option is not correct). It may be easier to call it once, at the end of the function like this.

else:
    print "please choose correct option"
    play_game()
    return
play_again()

However, an even more simple option would be to check at the beginnign that the value is correct. You could define a list with the correct options and use it like this :

from random import choice

game_options = ['stone', 'paper', 'scissor']

def play_again():
    print "Do you want to play again:- Choose 'yay' or 'nay'"
    user_again = raw_input('->')
    if user_again == 'yay':
        play_game()
    elif user_again == 'nay':
        print "Ok bye! hope you enjoyed the game. See you soon! :)"
    else:
        print "Please choose correct option."
        play_again()

def play_game():
    computer_choice = choice(game_options)
    computer_choosed = "Computer choosed %s" % computer_choice
    print "Make an choice"
    print "Choose stone, paper, scissor"
    user_choice = raw_input('->')

    if user_choice not in game_options:
        print "please choose correct option"
        play_game()
        return

    if user_choice == computer_choice:
        print computer_choosed
        print "So it's a tie"

    elif user_choice == 'stone':
        if computer_choice == 'paper':
            print computer_choosed
            print "So, You loose"
        elif computer_choice == 'scissor':
            print computer_choosed
            print "So, Cheers! You won!"

    elif user_choice == 'paper':
        if computer_choice == 'scissor':
            print computer_choosed
            print "So, you loose."
        elif computer_choice == 'stone':
            print computer_choosed
            print "So, Cheers! You won!"

    elif user_choice == 'scissor':
        if computer_choice == 'stone':
            print computer_choosed
            print "So, you loose."
        elif computer_choice == 'paper':
            print computer_choosed
            print "So, Cheers! You won!"

    play_again()

if __name__ == "__main__":
    # execute only if run as a script
    print "You are playing Stone, Paper, Scissor."
    play_game()

Also, this may call for a better option. You could define a function asking a user for a value in a list. This function could be used in 2 places and make your code easier to follow and less repetitive.

from random import choice

game_options = ['stone', 'paper', 'scissor']

def get_user_input_in_list(lst):
    user_input = raw_input('->')
    while True:
        if user_input in lst:
            return user_input
        else:
            print "Please choose correct option."

def play_again():
    print "Do you want to play again:- Choose 'yay' or 'nay'"
    user_again = get_user_input_in_list(['yay', 'nay'])
    if user_again == 'yay':
        play_game()
    elif user_again == 'nay':
        print "Ok bye! hope you enjoyed the game. See you soon! :)"

def play_game():
    computer_choice = choice(game_options)
    computer_choosed = "Computer choosed %s" % computer_choice
    print "Make an choice"
    print "Choose stone, paper, scissor"
    user_choice = get_user_input_in_list(game_options)

    if user_choice == computer_choice:
        print computer_choosed
        print "So it's a tie"

    elif user_choice == 'stone':
        if computer_choice == 'paper':
            print computer_choosed
            print "So, You loose"
        elif computer_choice == 'scissor':
            print computer_choosed
            print "So, Cheers! You won!"

    elif user_choice == 'paper':
        if computer_choice == 'scissor':
            print computer_choosed
            print "So, you loose."
        elif computer_choice == 'stone':
            print computer_choosed
            print "So, Cheers! You won!"

    elif user_choice == 'scissor':
        if computer_choice == 'stone':
            print computer_choosed
            print "So, you loose."
        elif computer_choice == 'paper':
            print computer_choosed
            print "So, Cheers! You won!"

    play_again()

if __name__ == "__main__":
    # execute only if run as a script
    print "You are playing Stone, Paper, Scissor."
    play_game()

This is better but we still have play_game calling play_again and play_again calling play_game.

Maybe play_game should be used to play a single game and shouldn't call play_again at all. This can be done by removing the call to play_again in play_game and moving it after the call to play_game in play_again. That way, we'd just have play_game calling itself.

You'd have something like :

from random import choice

game_options = ['stone', 'paper', 'scissor']

def get_user_input_in_list(lst):
    user_input = raw_input('->')
    while True:
        if user_input in lst:
            return user_input
        else:
            print "Please choose correct option."

def play_again():
    print "Do you want to play again:- Choose 'yay' or 'nay'"
    user_again = get_user_input_in_list(['yay', 'nay'])
    if user_again == 'yay':
        play_game()
        play_again()
    elif user_again == 'nay':
        print "Ok bye! hope you enjoyed the game. See you soon! :)"

def play_game():
    computer_choice = choice(game_options)
    computer_choosed = "Computer choosed %s" % computer_choice
    print "Make an choice"
    print "Choose stone, paper, scissor"
    user_choice = get_user_input_in_list(game_options)

    if user_choice == computer_choice:
        print computer_choosed
        print "So it's a tie"

    elif user_choice == 'stone':
        if computer_choice == 'paper':
            print computer_choosed
            print "So, You loose"
        elif computer_choice == 'scissor':
            print computer_choosed
            print "So, Cheers! You won!"

    elif user_choice == 'paper':
        if computer_choice == 'scissor':
            print computer_choosed
            print "So, you loose."
        elif computer_choice == 'stone':
            print computer_choosed
            print "So, Cheers! You won!"

    elif user_choice == 'scissor':
        if computer_choice == 'stone':
            print computer_choosed
            print "So, you loose."
        elif computer_choice == 'paper':
            print computer_choosed
            print "So, Cheers! You won!"

if __name__ == "__main__":
    # execute only if run as a script
    print "You are playing Stone, Paper, Scissor."
    play_game()
    play_again()

This is a bit better but you can go further in the separation of concerns. It is probably a better option to have play_again to return a boolean and have a while loop ensuring we call play_game as long as required.

This would look like:

from random import choice

game_options = ['stone', 'paper', 'scissor']

def get_user_input_in_list(lst):
    user_input = raw_input('->')
    while True:
        if user_input in lst:
            return user_input
        else:
            print "Please choose correct option."

def play_again():
    print "Do you want to play again:- Choose 'yay' or 'nay'"
    user_again = get_user_input_in_list(['yay', 'nay'])
    return user_again == 'yay'

def play_game():
    computer_choice = choice(game_options)
    computer_choosed = "Computer choosed %s" % computer_choice
    print "Make an choice"
    print "Choose stone, paper, scissor"
    user_choice = get_user_input_in_list(game_options)

    if user_choice == computer_choice:
        print computer_choosed
        print "So it's a tie"

    elif user_choice == 'stone':
        if computer_choice == 'paper':
            print computer_choosed
            print "So, You loose"
        elif computer_choice == 'scissor':
            print computer_choosed
            print "So, Cheers! You won!"

    elif user_choice == 'paper':
        if computer_choice == 'scissor':
            print computer_choosed
            print "So, you loose."
        elif computer_choice == 'stone':
            print computer_choosed
            print "So, Cheers! You won!"

    elif user_choice == 'scissor':
        if computer_choice == 'stone':
            print computer_choosed
            print "So, you loose."
        elif computer_choice == 'paper':
            print computer_choosed
            print "So, Cheers! You won!"

if __name__ == "__main__":
    # execute only if run as a script
    print "You are playing Stone, Paper, Scissor."
    while True:
        play_game()
        if not play_again():
            print "Ok bye! hope you enjoyed the game. See you soon! :)"
            break

Now we can get into the internals of play_game.
First, you could get rid of the various way where printing is repeated.

def play_game():
    computer_choice = choice(game_options)
    computer_choosed = "Computer choosed %s" % computer_choice
    print "Make an choice"
    print "Choose stone, paper, scissor"
    user_choice = get_user_input_in_list(game_options)

    print computer_choosed
    if user_choice == computer_choice:
        assert result == 0
        print "So it's a tie"
    else:
        win = False  # unused value

        if user_choice == 'stone':
            win = (computer_choice == 'scissor')
        elif user_choice == 'paper':
            win = (computer_choice == 'stone')
        elif user_choice == 'scissor':
            win = (computer_choice == 'paper')

        if win:
            print "So, Cheers! You won!"
        else:
            print "So, you loose."

There are various way to define who wins in a game of player/scissors/rock. You could define a dictionnary mapping the different combinations possible to the winner. I quite like using modulo arithmetic to find the result.

Final code looks like:

from random import choice

game_options = ['stone', 'paper', 'scissor']

def get_user_input_in_list(lst):
    user_input = raw_input('->')
    while True:
        if user_input in lst:
            return user_input
        else:
            print "Please choose correct option."

def play_again():
    print "Do you want to play again:- Choose 'yay' or 'nay'"
    user_again = get_user_input_in_list(['yay', 'nay'])
    return user_again == 'yay'

def play_game():
    computer_choice = choice(game_options)
    computer_choosed = "Computer choosed %s" % computer_choice
    print "Make an choice"
    print "Choose stone, paper, scissor"
    user_choice = 'stone' # get_user_input_in_list(game_options)
    computer_idx = game_options.index(computer_choice)
    user_idx = game_options.index(user_choice)
    result = (computer_idx - user_idx) % 3

    print computer_choosed
    if result == 0:
        print "So it's a tie"
    elif result == 1:
        print "So, you loose."
    else:
       assert result == 2
       print "So, Cheers! You won!"

if __name__ == "__main__":
    # execute only if run as a script
    print "You are playing Stone, Paper, Scissor."
    while True:
        play_game()
        if not play_again():
            print "Ok bye! hope you enjoyed the game. See you soon! :)"
            break


Output splitter to handle output This class is helping to manipulate by output streams stderr, stdout, and file. Are there any cons and errors in this code?

class OutSplitter(object):
""" splitter """

def __init__(self, out_splitter):
    self.__pipes=None
    if isinstance(out_splitter, OutSplitter):
            self.__pipes = Set(out_splitter.pipes)
    elif out_splitter:
        if hasattr(out_splitter,"__iter__") : #
            for pn in out_splitter:
                self.add_pipe(pn)
        else:
            self.add_pipe(out_splitter)

@property
def pipes(self):
    return self.__pipes

def add_pipe(self, pn):
    if isinstance(pn, basestring):
        if pn == 'stderr': x = sys.stderr
        elif pn == 'stdout': x = sys.stdout
        else:
            p = os.path.dirname(pn)
            if p and not os.path.exists(p):
                os.makedirs(p)
            x = open(pn,'w+')
    else: x = pn

    if hasattr(x, 'write') and hasattr(x, 'flush'):
        if self.__pipes is None:
            self.__pipes=Set()
        self.__pipes.add(x)

def write(self,s):
    if self.__pipes:
        for p in self.__pipes:
            p.write(s)

def flush(self):
    if self.__pipes:
        for p in self.__pipes:
            p.flush()

Overall, I find the code pretty good. A couple of suggestions from my side:

self.__pipes=Set(): I think add_pipe() should not be doing this. When the object is created it should be initialized fully. So this line should be placed inside __init__() at the start of the elif out_splitter: part.
x = open(pn,'w+'): What if this call fails? Exceptions should be handled. There is a good chance that someone using this class may trigger file open failure because they may instantiate the same object from another object (constructor is allowing this). If you make low-level stuff easy to do, you should also provide for exception handling where things might go wrong.
Files opened are not closed. In fact, it might be a good idea to open/close file handles using __enter__() and __exit__() methods, which need to be defined for this class.
One of the things I find with file I/O is that users want control on the order in which messages are printed. The application may throw out stdout and stderr prints in a certain order and the user might want to preserve that order. Unfortunately, the current code flushes the queue of one pipe first before moving on to the next pipe. Users of this class might find this to be inadequate.
In addition, users might want stdout and stderr to be redirected to files. This is not possible with the current class. It maybe okay now, but sooner or later someone will want this and they will start looking for alternatives.
Just a thought: since a getter method is used for __pipes, perhaps you can consider a setter method as well.


Basic ReactJS Components routing Recently I've started learning ReactJS and I do not look forward to making one Page website so I learned about react-router and this is my way of doing to it.

App.js

import React, { Component } from "react";
import "babel-polyfill";
import Navbar from "./Components/MenuNavbar";
import Footer from "./Components/Bottom";
import Home from "./Components/Home";
import About from "./Components/About";
import Contact from "./Components/Contact";
import { BrowserRouter as Router, Route, Switch } from "react-router-dom";
import Products from "./Components/Products";
import { CircleArrow as ScrollUpButton } from "react-scroll-up-button";
// import Topbar from "./Components/Topbar";

class App extends Component {
  render() {
    return (

        <Router>
          <React.Fragment>
            <Navbar />
            <Switch>
              <Route exact path="/" component={Home} />
              <Route exact path="/products" component={Products} />
              <Route exact path="/about" component={About} />
              <Route exact path="/contact" component={Contact} />
            </Switch>
            <ScrollUpButton />

          </React.Fragment>
        </Router>

    );
  }
}

export default App;

In my other Components, I wrap parent in a div as learned in some tutorials, like this:

render() {
    return (

        <ProductsHero />
        <ProductsCashew />
        <ProductsAlmonds />

    );
  }

Is this the best practice? If not how can I improve? Shall I wrap every child in React.Fragment instead of div? If so why?

Whether to use React.Fragment or not depends on how you want the resulting DOM structured. Using React.Fragment avoids introducing additional elements in the DOM when they are not needed.

Fragments were introduced because only one React element can be returned from the render function, but it is pretty common and natural to need to return more than one element. Before Fragments, we had to wrap these elements in a div or other tag that really served no other purpose than to please React. This worked most of the time, but there were cases where it was not sufficient, such as when a component needed to return two tr elements. Wrapping tr's in a div and trying to place that in a tbody doesn't work. With Fragments, multiple elements can be returned and React does not have to render anything for the Fragment itself, only the children of the Fragment.

That being said, there are some situations where wrapping elements in a div or other tag makes sense. One that comes up frequently for me is when I have a component just like your products example, but I want to apply a margin, padding, border, or other style to the wrapper around the actual children. In that case, React.Fragment can't help me and I need a div or section.

A general rule-of-thumb, if you have a div or section that is doing absolutely nothing but letting you return multiple elements, you should switch it out with React.Fragment. If you are using that wrapper div in any way, keep it.

If you haven't yet, I would suggest reading the official Fragments guide. It demonstrates the table situation as an example for why Fragments were introduced.


Reading a config file I am relatively new to Perl and would like to check that the code I am writing docent have any major flaws in it or any bad practices.

This is a fairly simple sub but one that gets used often. Seems to work OK. Simply reads a text file that has parameters in it set by the user.

Config.txt

#cucm username and password of user associated with phones
username  = bob
password  = bobPass
#subnet details. Use /30 to start with to test 2 phones before entire subnet
subnet = 10.64.97.216/32
#HFS web server details IP and folder
webserver   = http://10.64.164.230/Desktop/
#image file names. Full and Thumbnail size
imgFullSize = screen_F.png
imgThumSize = screen_S.png

Perl sub that reads the file:

sub readConfigFile
{
print $lfh "Reading Config.txt file\n";
#path to the config file
my $configF = "config.txt";
if($DEBUG == 1){print "opening config File\n";}
open(my $configFhandle, '<', $configF) or die "Unable to open file, $!";
my @arrFileData=<$configFhandle>; #Slurp!

#go through each of the lines in the file
for (my $i = 0; $i <= $#arrFileData; ++$i)
{
local $_ = $arrFileData[$i];
if($DEBUG == 1){print "Reading line:$_";}
#check to see if the line is a comment... if it is skip it
if($_ =~ /^#/)
{
if($DEBUG == 1){print "Line is comment. Skiping:$_";}
next;
}

#split the line into the key and value
my @dataKV = split /=/, $_;
#check to see that Key and Value exist ... if not skip it
my $arrSize = @dataKV;
if($arrSize <= 1)
{
if($DEBUG == 1){print "Read Config: Error: No Key AND Value found:$_";}
next;
}
#remove all leading and end white spaces around elements
s{^\s+|\s+$}{}g foreach @dataKV;

for (my $x = 0; $x <= $#dataKV; $x++)
{
if($DEBUG == 1){print "Key:$dataKV[$x]\n";}
given($dataKV[$x])
{
when (/^username/)
{
$usr = $dataKV[++$x];
if($DEBUG == 1){print "Value Username:$dataKV[$x]\n";}
next;
}
when (/^password/)
{
$pass = $dataKV[++$x];
if($DEBUG == 1){print "Value password:$dataKV[$x]\n";}
next;
}
when (/^subnet/)
{
@arrSubnets = $dataKV[++$x];
if($DEBUG == 1){print "Value Subnet:$dataKV[$x]\n";}
next;
}
when (/^webserver/)
{
$webserver = $dataKV[++$x];
if($DEBUG == 1){print "Value webserver:$dataKV[$x]\n";}
next;
}
when (/^imgFullSize/)
{
$imgFull = $dataKV[++$x];
if($DEBUG == 1){print "Value imgFullSize:$dataKV[$x]\n";}
next
}
when (/^imgThumSize/)
{
$imgThum = $dataKV[++$x];
if($DEBUG == 1){print "Value imgThumSize:$dataKV[$x]\n";}
next
}
}#end of switch statement
}#end of for loop going through the key and value line
}#end of For loop going though the lines in the config

print "*********************************\n";
print "*Config imported\n";
print "*********************************\n";
print "*username    :$usr\n";
print "*password    :$pass\n";
print "*subnet      :@arrSubnets\n";
print "*webserver   :$webserver\n";
print "*imgFullSize :$imgFull\n";
print "*imgThumSize :$imgThum\n";
print "*********************************\n";
print "Is this above correct?[Y]:";
my $confirm = <>;
if($confirm !~ /^Y/)
{
print $lfh "Config file needs to be changed. Closing script down.\n";
close($lfh) or warn "Unable to close the file handle: $!";
close($configFhandle) or warn "Unable to close the file handle: $!";
print "Script will now terminate. Please amend \"Config.txt\" file and re-run script. Press     any key to terminate";
<>;
exit;
}
#close the file
close($configFhandle) or warn "Unable to close the file handle: $!";
print $lfh "Y pressed by User. Config all good.\n";
}

Any advise on how to improve this would be great.

Well, here is how I'd write that subroutine. I thoroughly commented the code to explain my choices.

use strict;
use warnings;
use autodie;     # automatic error handling for "open" and other builtins
use IO::Handle;  # object oriented syntax for filehandles

# DEBUG is a compile-time constant initialized by an environment variable.
# If it is false, the debug statements will get optimized away.
use constant DEBUG => $ENV{DEBUG};

# the log file handle and debug file handle
# Here, I opened both to STDERR
open my $lfh, '>&', STDERR;
open my $dfh, '>&', STDERR;

# close the $lfh file handle regardless of how the script terminates.
END {
    no autodie;
    close $lfh or warn "Unable to close the log file handle: $!";
}

# the main part of our script
{
    my $filename = "config.txt";
    # "read_config_file" is a function that takes one argument and returns one value.
    my $config = read_config_file($filename);
    # if the check fails, exit this script with error code "1".
    # the default error code (zero) is considered a success.
    ui_check_config_ok(\*STDOUT, $config, $filename) or exit 1;

    # now we can do stuff with the $config, e.g.
    my $usr = $config->{username};
}

# Name your variables in snake_case, not with camelCase.
# This function *only* parses the config file.
# It does not interact with the user.
# Such separation of concern makes your code easier to maintain:
# every function should do one thing only.
sub read_config_file {
    # Subroutines can take an argument list.
    # We unpack it like this:
    my ($filename) = @_;

    $lfh->say("Reading config file $filename");
    DEBUG and $dfh->say("Opening config file $filename");

    my %known_keys = map { $_ => 1 } qw/
        username password subnet webserver imgFullSize imgThumSize
    /;

    # we wills store the config in this hash
    my %config;

    open my $fh, "<", $filename;

    # don't slurp the file, we loop over it line by line
    LINE:
    while (<$fh>) {
        # trim the line
        s{\A\s+}{};
        s{\s+\z}{};

        DEBUG and $dfh->say("Reading line: $_");
        if (/^#/) {
            DEBUG and $dfh->say("Line is comment, skipping.");
            next LINE;
        }

        # * We use the /x flag on regexes to be able to structure the regex with spaces
        #   they won't match, they're just decoration
        # * instead of assigning to an array, we assign to a list of scalars.
        #   If the "split" fails, the last one should be "undef"
        # * We only accept one "=" per line, and produce 2 fragments max. So
        #       foo = bar = baz
        #   will produce $key="foo", $value="bar = baz".
        my ($key, $value) = split m{\s* [=] \s*}x, $_, 2;
        if (not defined $value) {
            DEBUG and $dfh->say("Ignoring config error: no key = value pair found on line: $_");
            next LINE;
        }

        DEBUG and $dfh->say("Value $key:$value");
        $config{$key} = $value;

        # a sanity check – you don't do anything on unknown keys
        if (not exists $known_keys{$key}) {
            $lfh->say("The config provided the unknown key $key, ignoring.")
        }
    }

    # another sanity check – did the user forget to specify some properties?
    for my $key (keys %known_keys) {
        if (not exists $config{$key}) {
            $lfh->say("The config didn't provide a key $key, ignoring.")
        }
    }

    return \%config;
}

# another subroutine that provides the user interface to ask whether the data is correct
# It will return a truth value whether the config was OK.
sub ui_check_config_ok {
    # Here, we see an argument list with three values
    my ($ui_fh, $config, $filename) = @_;

    $ui_fh->say($_) for
        "*********************************",
        "*Config imported",
        "*********************************",
        "*username    :$config->{username}",
        "*password    :$config->{password}",
        "*subnet      :$config->{subnet}",
        "*webserver   :$config->{webserver}",
        "*imgFullSize :$config->{imgFullSize}",
        "*imgThumSize :$config->{imgThumSize}",
        "*********************************";
    $ui_fh->print("Is this above correct?[Y]:");
    if (<STDIN> =~ /^\s*Y/) {
        $lfh->say("The user pressed Y. Config is good.");
        return 1;
    }

    $lfh->say("Config file contains false information, please correct.");
    $lfh->say("Terminating script");
    $ui_fh->say("The script will now terminate.");
    $ui_fh->say("Please amend the file $filename and rerun the script.");
    $ui_fh->say("Press any key to terminate...");
    <>;
    return 0;
}

Note how indentation made this code much easier to read than the dump you provided. Please use indentation!

I actually do not generally use IO::Handle, but it makes stuff more obvious when having more than one file handle. The autodie module is a bit inflexible, but I highly encourage it when you are still new with Perl, as it provides sensible defaults. You do not manually have to close file handles if they are variables declared with my – they will get closed automatically, and it's highly unlikely that closing a file will fail (file handles are not only used for physical files, but also for other things like pipes, where the close can fail).


Advanced formatting with argument passing I want to customise how my objects are printed using the .format() function.

I have two problems to make that happen in clear and concise way. But since the second one stems from the first one I'm presenting both here.

Formatting array

Passing options to .format() in array seems to me to be awfully messy if I want to match default behaviour with __repr__:

import binascii

def format_data(data, formatstr):
    if 'x' in formatstr:
        return binascii.b2a_hex(data).decode('ascii')
    else:
        return repr(data)

class InsideA(object):
    def __repr__(self):
        return "InsideA()"

    def __format__(self, _):
        return "InsideA()"

class InsideB(object):
    def __init__(self, data):
        self.data = data

    def __repr__(self):
        data = format_data(self.data, 'x')
        return "InsideB(data={0})".format(data)

    def __format__(self, formatstr):
        data = format_data(self.data, formatstr)
        return "InsideB(data={0})".format(data)

if __name__ == "__main__":
    a = InsideA()
    b = InsideB(bytearray(b'test\xff\xff'))

    array = [a, b]

    print("what I want:")
    print(array)
    print("")

    print("doesn't work:")
    try:
        print("{0:x}".format(array))
    except Exception as e:  # catch just to continue execution
        print(e)
    print("")

    print("Leaves ' around elements:")
    print(["{0:x}".format(elem) for elem in array])
    print("")

    print("Works but seems overly complex:")
    print("[" + ", ".join("{0:x}".format(elem) for elem in array) + "]")

Produces:

What I want:

[InsideA(), InsideB(data=74657374ffff)]

Doesn't work:

non-empty format string passed to object.__format__

Leaves ' around elements:

['InsideA()', 'InsideB(data=74657374ffff)']

Works but seems overly complex:

[InsideA(), InsideB(data=74657374ffff)]

Formatting with non-uniform objects

The problem only gets compounded if the array elements don't always implement the format function, requiring use of try except clauses:

from __future__ import print_function
import binascii

def format_data(data, formatstr):
    if 'x' in formatstr:
        return binascii.b2a_hex(data).decode('ascii')
    else:
        return repr(data)

class InsideA(object):
    def __repr__(self):
        return "InsideA()"

class InsideB(object):
    def __init__(self, data):
        self.data = data

    def __repr__(self):
        data = format_data(self.data, 'x')
        return "InsideB(data={0})".format(data)

    def __format__(self, formatstr):
        data = format_data(self.data, formatstr)
        return "InsideB(data={0})".format(data)

def format_array(array, formatstr):
    if array is None:
        return "None"
    else:
        str_array = []
        for elem in array:
            try:
                str_array += ["{0:{1}}".format(elem, formatstr)]
            except (TypeError, ValueError):  # Py2 and Py3 compat
                str_array += [repr(elem)]
        return "[" + ", ".join(str_array) + "]"

if __name__ == "__main__":
    a = InsideA()
    b = InsideB(bytearray(b'test\xff\xff'))

    array = [a, b]

    print("Automatic:")
    print(array)
    print("")

    print("With format:")
    print(format_array(array, "x"))

Prints:

Automatic:

[InsideA(), InsideB(data=74657374ffff)]

With format:

[InsideA(), InsideB(data=74657374ffff)]

This code in format_array() seems to me to be too complex for what it does. Do I don't know of some syntax that makes this a breeze or is it the best Python can do?

(Yes, I know that I can implement __repr__ as just a call to format(), leaving that in for readability of example.)

list always uses repr on it's elements.
When you str or repr it.
And internally list is doing roughly the equivalent of what you are doing in format_array.

The only way to make this work is to use something like format_array,
or subclass list.
But they will both be doing the same thing, just depends on how much sugar you want.

Your code is nice, readable and follows PEP8.
As this is tending towards example code, it's not in it's native environment, it's hard to comment distinctively on it.

But format_data can be merged with the class.

class InsideB(object):
    def __init__(self, data):
        self.data = data

    def __repr__(self):
        data = self._data(True)
        return "InsideB(data={0})".format(data)

    def __format__(self, formatstr):
        data = self._data('x' in formatstr)
        return "InsideB(data={0})".format(data)

    def _data(self, as_hex):
        if as_hex:
            return binascii.b2a_hex(self.data).decode('ascii')
        else:
            return repr(self.data)

You can then add an option to use 'hex' or repr as the output type to the class.
With the option to reverting to the original after a use.
Obviously it depends on your usage.

class InsideB(object):
    def __init__(self, data, as_hex=False, revert_hex=False):
        self.data = data
        self._as_hex = as_hex
        self.output_type = as_hex
        self._revert_hex = revert_hex

    def _data(self, as_hex):
        if as_hex or self.output_type:
            data = binascii.b2a_hex(self.data).decode('ascii')
        else:
            data = repr(self.data)

        if self._revert_hex:
            self.output_type = self._as_hex

        return data

Then you would need to make a function that maps over all the items in the list,
changes output_type to what you want, and you'll be in the same position as you are now.
But with a bit more sugar.

If you choose to subclass list, and to use str in __str__,
then you'll get output which is just wrong.

>>> print('{!s}, {!s}'.format('a', 'b'))
a, b
>>> print('{!r}, {!r}'.format('a', 'b'))
'a', 'b'

No matter how I look at it you wont be able to change the output 'nicely'.
Unless you think something more robust to map(lambda i: i.output_type = True, array) is nice.

Bottom line you have to manually implement this, as repr is hard-coded.


Chromatic aberration surface is a pygame.surface object from the Pygame graphics library. It actually runs way faster than I thought it would for a first try. Still, have I made any obvious mistakes here? Is there any way to make it go even faster?

cdef mapped_chromatic_aberration(surface, numpy.ndarray[int,ndim=2] intensitymap):
    cdef int x,y,z,maxx,maxy,intensity
    cdef numpy.ndarray[unsigned char,ndim=3] array
    cdef numpy.ndarray[unsigned char,ndim=2] r,g,b
    r=pygame.surfarray.pixels_red(surface)
    g=pygame.surfarray.pixels_green(surface)
    b=pygame.surfarray.pixels_blue(surface)
    array=pygame.surfarray.pixels3d(surface)

    maxx,maxy=surface.get_rect().bottomright

    for x in range(maxx):
        for y in range(maxy):
            try:
                pass
                intensity=intensitymap[x,y]
                array[x,y,0]=r[x+intensity,y]
                array[x,y,1]=g[x,y+intensity]
                array[x,y,2]=b[x+intensity,y-intensity]
            except IndexError:
                pass

cdef random_chromatic_aberration(surface,int intensity=5):
    mapped_chromatic_aberration(surface,numpy.random.random_integers(-intensity,intensity,surface.get_rect().bottomright))

At first glance, the double for loop seems to be very time consuming. Since you're using the numpy arrays, there is a way to get rid of it:

x = np.arange(array.shape[0])
y = np.arange(array.shape[1])
intensity = intensitymap.astype(int)
array[:, :, 0] = red[x + intensity, y]
array[:, :, 1] = green[x, y + intensity]
array[:, :, 2] = blue[x + intensity, y - intensity]

By the way, I would recommend you to use the numba JIT compiler. This will let you write pure python code. It is under active development now, and this is optimized for numpy arrays.


Ruby - Summing groups of odd and even integers in an array until no groups remain The problem my code solves is listed below. I know my code can be improved, it takes a while to run when bigger arrays are entered as input.

"Problem - Given an array of integers, sum consecutive even numbers and consecutive odd numbers. Repeat the process while it can be done and return the length of the final array."

def sum_groups(arr)

  z = arr.size
  x = []
  i = 0

  until i == z
    arr = arr.chunk{|x| x.even?}.map{|x, y| y}.map{|x| x.inject(&:+)}
    x << arr
    i += 1
  end

  x[-1].size

end

An example input -

For arr = [2, 1, 2, 2, 6, 5, 0, 2, 0, 5, 5, 7, 7, 4, 3, 3, 9]

The result should be 6.

[2, 1, 10, 5, 30, 15] - Value of numbers in final array.

The length of final array is 6

Any review and help would be appreciated to make this code more efficient.

The whole thing about sums is misdirection; you don't need to sum anything, since all you need to output is the final length of the array. So what you want to know is how many consecutive terms there are of a given parity, not what their sum is.

The trick is that only an odd number of odd terms will produce an odd sum; anything else will produce an even sum. So you can skip calculating the actual sum, because by knowing the parity and number of terms, you'll know if that sum's going to be odd or even.

Secondly, you can make it recursive instead of loop-based.

Here's my take:

def count_chunks_recursively(array)
  # get our consecutive chunks
  chunks = array.chunk(&:odd?).to_a

  # if there are as many chunks as there are elements, we're done
  return array.size if chunks.size == array.size

  # otherwise, map the chunks to 0 or 1, depending on whether they
  # sum up to something even or something odd
  mapped = chunks.map { |odd, terms| odd && terms.size.odd? ? 1 : 0 }

  # ... and repeat the process recursively
  count_chunks_recursively(mapped)
end

As for your current code:

You very, very rarely have to use plain loops in Ruby; there's almost always something in Enumerable or Array that you can use instead.
x[-1].size would be more conventionally written as x.last.size
You use the a method reference when producing the sum – inject(&:+) – but you don't use the same technique for the other parts of the same line. I.e. this

arr.chunk{|x| x.even?}.map{|x, y| y}.map{|x| x.inject(&:+)}

could just be this:

arr.chunk(&:even?).map(&:last).map { |x| x.inject(&:+) }

or in Ruby 2.4+ which has a built-in sum method:

arr.chunk(&:even?).map(&:last).map(&:sum)

Sidenote: inject (specifically) actually doesn't need the method reference, but can also take a plain symbol: inject(:+). But for consistency's sake, I'd stick to using the &.


Iterate over chunks of date range def forward_date_range(start_dt, end_dt, span_days):
    #
    # Generate tuples with intervals from given range of dates (forward)
    #
    # forward_date_range('2012-01-01', '2012-01-5', 2)
    #
    # 1st yield = ('2012-01-01', '2012-01-03')
    # 2nd yield = ('2012-01-04', '2012-01-05')
    start_dt = datetime.datetime.strptime(start_dt, '%Y-%m-%d')
    end_dt   = datetime.datetime.strptime(end_dt, '%Y-%m-%d')
    span     = datetime.timedelta(days=span_days)
    step     = datetime.timedelta(days=1)

    while start_dt + span < end_dt:
        current = start_dt + span
        yield start_dt.strftime('%Y-%m-%d'), current.strftime('%Y-%m-%d')
        start_dt = current + step
    else:
        yield start_dt.strftime('%Y-%m-%d'), end_dt.strftime('%Y-%m-%d')

def backward_date_range(start_dt, end_dt, span_days):
    #
    # Generate tuples with intervals from given range of dates (backward)
    #
    # backward_date_range('2012-01-01', '2012-01-5', 2)
    #
    # 1st yield = ('2012-01-03', '2012-01-05')
    # 2nd yield = ('2012-01-01', '2012-01-02')
    start_dt = datetime.datetime.strptime(start_dt, '%Y-%m-%d')
    end_dt   = datetime.datetime.strptime(end_dt, '%Y-%m-%d')
    span     = datetime.timedelta(days=span_days)
    step     = datetime.timedelta(days=1)

    while end_dt - span > start_dt:
        current = end_dt - span
        yield current.strftime('%Y-%m-%d'), end_dt.strftime('%Y-%m-%d')
        end_dt = current - step
    else:
        yield start_dt.strftime('%Y-%m-%d'), end_dt.strftime('%Y-%m-%d')

This is currently what I have, needless to say it looks ugly. Is there a more elegant solution, especially if it is possible to get rid of the while loop altogether? or maybe compress it into one single line?

I am currently using python3.5

I'm going to assume that the corner cases are what you want, i.e. a date
"range" of a single day is acceptable - otherwise you'd have to overhaul
the calculation a bit.

I'm not going to attempt to remove the while loops - I don't know a
better alternative; six lines self-contained logic each isn't too bad
IMO.

First things first, I'd recommend a couple of things for better
readability and interactivity:

Use proper docstrings instead of comments, so that both tools and
human readers know what the functions are about.  At the moment
help(forward_date_range) isn't too helpful, but with that change it
would be much more so.
Similarly, the _dt suffix isn't that readable either.  I'd either
drop it or expand it to _date so that it's more obvious.

Next, I'd try to remove some more duplication:

The string '%Y-%m-%d' comes up a total of twelve times.  Either put
that into a constant (if you don't ever want users to supply a
different format), or make it an optional function parameter.
The structure of both functions is also quite similar, so while the
loop might not be worth to factor out, the pre- and postprocessing
definitely is.  Also, timedelta(days=1) is another constant in the
code.
Importing the right names (e.g. datetime and timedelta) could also
cut down the number of tokens to read.
There's also an opportunity to cache the result of end - span into a
separate variable so as to not repeat it in every loop.
(From @mathias-ettinger) The loop doesn't have a break to exit it
early, so the else branch will always be executed.  In that case it
makes sense to just put it on its own line without the else which
doesn't change the meaning in any respect.

Lastly, I'd probably say that the conversion from and to datetime
objects doesn't belong into these functions and should be done
separately instead.  If you want to keep it like it is there are still
some opportunities for helper functions to cut down the noise.

The result I'm posting below can still be compressed further, but at
that point it would be generally getting more functional and less like
regular Python code.

So there we have it:

from datetime import datetime, timedelta

DATE_FORMAT = '%Y-%m-%d'
DATE_STEP = timedelta(days=1)

def _strptime(string):
    return datetime.strptime(string, DATE_FORMAT)

def _strftime(date):
    return date.strftime(DATE_FORMAT)

def _date_range_parameters(start, end, span_days):
    start = _strptime(start)
    end   = _strptime(end)
    span  = timedelta(days=span_days)
    return start, end, span

def forward_date_range(start, end, span_days):
    """
    Generate tuples with intervals from given range of dates (forward).

    forward_date_range('2012-01-01', '2012-01-5', 2)

    1st yield = ('2012-01-01', '2012-01-03')
    2nd yield = ('2012-01-04', '2012-01-05')
    """
    start, end, span = _date_range_parameters(start, end, span_days)
    stop = end - span

    while start < stop:
        current = start + span
        yield _strftime(start), _strftime(current)
        start = current + DATE_STEP

    yield _strftime(start), _strftime(end)

def backward_date_range(start, end, span_days):
    """
    Generate tuples with intervals from given range of dates (backward)

    backward_date_range('2012-01-01', '2012-01-5', 2)

    1st yield = ('2012-01-03', '2012-01-05')
    2nd yield = ('2012-01-01', '2012-01-02')
    """
    start, end, span = _date_range_parameters(start, end, span_days)
    stop = start + span

    while end > stop:
        current = end - span
        yield _strftime(current), _strftime(end)
        end = current - DATE_STEP

    yield _strftime(start), _strftime(end)