Circular Buffer implementation in Ruby Does the implementation look correct? Did I miss something major/minor?

class CircularBuffer

  class BufferEmptyException < StandardError;  end;
  class BufferFullException < StandardError;   end;

  def initialize(size)
    @size = size
    @buffer = []
  end

  def write(data)
    raise CircularBuffer::BufferFullException if @buffer.size >= @size
    @buffer << data if data
  end

  def write!(data)
    if data
      @buffer << data
      @buffer.shift
    end
  end

  def clear
    @buffer = []
  end

  def read
    data = @buffer.shift
    raise CircularBuffer::BufferEmptyException unless data
    data
  end

end

I can pass the initializer a negative size. Or a non-numeric size. I won't get any complaints until I try to use the buffer.
If I use write! exclusively, the buffer will always be empty. You're always shifting an element off of the array - even if it's the one you just added.
It's impossible to write nil or false to the buffer, though either of those are values someone might legitimately want to write.
But even if you manage to write a false'y value, you can't get it back; you'll just get a BufferEmptyException even if the buffer's not empty. Raise if the array's truly empty, and do so before you shift; not if the value you already shifted off is false'y. And, worse yet, the value is just gone forever. You shifted it off the array, so it's just disappeared. Since you can just use the array's length to tell you the state of the buffer, it's no problem to store false'y values.
It's generally good practice to define a non-bang method in terms of its bang counterpart. Right now you have 2 separate places where things are added to the buffer, instead of having one method call the other, just with an extra check.
Speaking of, it'd be nice to have a bang version of read that doesn't raise exceptions.
It'd be nice to have an attr_reader for the size, and a to_a for the items.
It'd also be nice to have a full? method. Otherwise you'll always have to either rescue an exception from write, or exclusively use write!. Besides, you could use the same full? method within the class too.
Likewise, an empty? method would be nice to complement the full? method. While you're at it, you might even add a method that returns the number of slots left in the buffer, and/or one that that returns the number of slots occupied.

Of course, it's not quite a circular buffer since, for one, it's not a perfectly fixed size. If write! worked as intended, and only shifted off items when necessary, it'd still (in it's current form) shift them off only after the buffer's "overflowed".

For another, it's not actually circular. You don't end up overwriting previously set slots. Instead you push and shift and array that expands up to a certain size. From the outside, it might well look like a circular buffer, but none of its internals match that description.

What you have is a general (almost) fixed-length (or rather "limited-length") FIFO buffer, but I wouldn't call it circular. But then again, implementing a strictly circular buffer in Ruby isn't really that useful. A FIFO buffer has its uses, sure, but a strictly circular one seems like trying to do low-level programming in a very high-level language. It's a technique that's useful when you have to allocate memory, and it's cool when you can do pointer arithmetic. But you don't allocate memory in Ruby, and you can't do pointer manipulation, so the arguments and tools for creating efficient buffers aren't really there.

It's like camping in you own back yard. It's fun to try, but it's not survivalism (and it'd probably be kinda dangerous to make a campfire).

But hey, it is fun

class FixedLengthFIFOBuffer
  class BufferEmptyException < StandardError;  end;
  class BufferFullException < StandardError;   end;

  attr_reader :size

  def initialize(size)
    @size = size.to_i # still lacking a check for values below 1
    @buffer = []
  end

  def full?
    @buffer.size >= size
  end

  def empty?
    @buffer.empty?
  end

  def write(data)
    raise CircularBuffer::BufferFullException if full?
    write! data
  end

  def write!(data)
    @buffer.shift if full? # maybe use read! instead and return the value?
    @buffer << data
  end

  def clear
    @buffer.clear
  end

  def read
    raise CircularBuffer::BufferEmptyException if empty?
    read!
  end

  def read!
    @buffer.shift
  end

  def to_a
    @buffer.dup
  end
end


Best practice instantiating generic delegates and accessing property getters I want to create delegates to access properties of different objects without knowing them in advance.

I have the following definitions

public delegate T MyMethod<K, T>(K data);

public static MyMethod<K, T> CreatePropertyGetter<K, T>(PropertyInfo property)
{
   MethodInfo mi = property.DeclaringType.GetMethod("get_" + property.Name);
   return (MyMethod<K, T>)Delegate.CreateDelegate(typeof(MyMethod<K, T>), mi);
}

where T can be decimal, string, datetime or int

I have some initializing code that will create MyMethod delegates, based on the reflected properties of my object as follows:

foreach (PropertyInfo property in entityType.GetProperties())
{
    switch (property.PropertyType.Name)
    {
        case "System.Decimal":
            return CreatePropertyGetter<T, decimal>(property);
        case "System.DateTime":
            return CreatePropertyGetter<T, DateTime>(property);
        case "System.String":
            return CreatePropertyGetter<T, DateTime>(property);
    }
}

Is there a better way to

create property getters?
enumerate through the supported property types hard-coded as strings?

However, performance is an issue, since these delegates will be called frequently (ticking scenario), so any casting will slow it down. While a more elegant solution is desirable, performance is still my main concern

As my answer on Stack Overflow states, there is a solution which allows you to create a getter delegate for any type, so the entire switch block can be dropped. Personally, I find that the cleanest solution. The only overhead of this approach is the cast from object to the specific type where necessary. (For a getter this would happen only once on the return value.) I'm guessing if you would benchmark this, the result would be negligible.

When you are certain that the only types required are those specified, or that no new types are expected very often, you could still opt to use your current approach. I would apply a few changes however:

Use PropertyInfo.GetGetMethod() to retrieve your MethodInfo for the getter.
Use the type of PropertyType directly to determine which CreatePropertyGetter to call instead of the name of the type.
Use Func<K, T> instead of MyMethod.
Use a Dictionary holding anonymous functions which create the correct delegate for the specified type of the key. This can replace the switch statement.

The last point is possible as follows:

Dictionary<Type, Func<PropertyInfo, object>> getterCreators =
    new Dictionary<Type, Func<PropertyInfo, object>>
    {
        { typeof(string), p => CreatePropertyGetter<T, string>(p) },
        { typeof(DateTime), p => CreatePropertyGetter<T, DateTime>(p) },
        { typeof(decimal), p => CreatePropertyGetter<T, decimal>(p) }
    };

Ofcourse you still have to cast from object to the desired type.

UPDATE:

As Timwi pointed out in the comments, p => CreatePropertyGetter<T, ...>(p) can be further reduced to: CreatePropertyGetter<T, ...>.


Simple web based file browser with Flask I made a simple web-based file browser with Flask. It also shows some metadata of a file. I have a few questions regarding security though. Is it possible for an attacker to escape out of the preset root directory? Are there any ways to make my code more readable and/or more efficient? Would love to hear some feedback to improve my program! :)

app.py:

from flask import Flask, render_template
import os
import stat

app = Flask(__name__)

# Make the WSGI interface available at the top level so wfastcgi can get it.
wsgi_app = app.wsgi_app

FILE_SYSTEM_ROOT = "D:\Test123"

@app.route('/')
def index():
    return render_template('index.html')

@app.route('/browser')
def browse():
    itemList = os.listdir(FILE_SYSTEM_ROOT)
    return render_template('browse.html', itemList=itemList)

@app.route('/browser/<path:urlFilePath>')
def browser(urlFilePath):
    nestedFilePath = os.path.join(FILE_SYSTEM_ROOT, urlFilePath)
    if os.path.isdir(nestedFilePath):
        itemList = os.listdir(nestedFilePath)
        fileProperties = {"filepath": nestedFilePath}
        if not urlFilePath.startswith("/"):
            urlFilePath = "/" + urlFilePath
        return render_template('browse.html', urlFilePath=urlFilePath, itemList=itemList)
    if os.path.isfile(nestedFilePath):
        fileProperties = {"filepath": nestedFilePath}
        sbuf = os.fstat(os.open(nestedFilePath, os.O_RDONLY)) #Opening the file and getting metadata
        fileProperties['type'] = stat.S_IFMT(sbuf.st_mode)
        fileProperties['mode'] = stat.S_IMODE(sbuf.st_mode)
        fileProperties['mtime'] = sbuf.st_mtime
        fileProperties['size'] = sbuf.st_size
        if not urlFilePath.startswith("/"):
            urlFilePath = "/" + urlFilePath
        return render_template('file.html', currentFile=nestedFilePath, fileProperties=fileProperties)
    return 'something bad happened'

if __name__ == '__main__':
    app.run(host='0.0.0.0', port=80)

browse.html:

<!DOCTYPE html>

    <meta charset="utf-8" />
    <title></title>

        {% for item in itemList %}

{{ item }}

        {% endfor %}

file.html:

<!DOCTYPE html>

    <meta charset="utf-8" />
    <title></title>

Current file: {{ currentFile }}

            {% for key, value in fileProperties.items() %}

                {{ key }}
                {{ value }}

            {% endfor %}

The output when browsing looks something like this:

The output with the metadata of a file looks like this:

@Matthias has already mentioned this in the comments, and he is correct.

The code suffers from a Directory Traversal vulnerability

You can filter this in 2 ways:

Use os.path.realpath and check if the resolved path is different

def check_lfi(file_path):
    return os.path.realpath(file_path) != file_path

Use regex to filter out bad (../) Paths

import re
DIR_TRAVERSAL = re.compile(r"\.\.\/")
def check_lfi(file_path):
    return bool(DIR_TRAVERSAL.search(file_path))

My advice would be to use the first example, since blacklisting can be error prone (Unicode for instance)


ForEach Methods Implementation for Multidimensional Array in C# I am working with System.Array and I found that the input parameter of ForEach method public static void ForEach<T>(T[] array, Action<T> action); is specified on one dimensional array case. I am trying to generalize this to multidimensional array.
The experimental implementation
Here's the experimental implementation of generalized ForEach method.
class ArrayHelpers
{
    public static void ForEach<T>(Array array, in Action<T> action)
        where T : unmanaged
    {
        if (ReferenceEquals(array, null))
        {
            throw new ArgumentNullException(nameof(array));
        }

        if (ReferenceEquals(action, null))
        {
            throw new ArgumentNullException(nameof(action));
        }

        foreach (T item in array)
        {
            action.Invoke(item);
        }
        return;
    }
}

Test cases
The Test cases for one dimensional case, two dimensional case and three dimensional case are as below.
//    One dimensional case
Console.WriteLine("One dimensional case");
int[] test = new int[] { 1, 2, 3, 4 };
ArrayHelpers.ForEach<int>(test, ShowSquares);

//    two dimensional case
Console.WriteLine("Two dimensional case");
int[,] test2 = { { 0, 1 }, { 2, 3 } };
ArrayHelpers.ForEach<int>(test2, ShowSquares);

//    three dimensional case
Console.WriteLine("Three dimensional case");
int[,,] test3 = { { { 0, 1 }, { 2, 3 } }, { { 0, 1 }, { 2, 3 } } };
ArrayHelpers.ForEach<int>(test3, ShowSquares);

/// <summary>
/// Reference: https://docs.microsoft.com/en-us/dotnet/api/system.array.foreach?view=net-5.0
/// </summary>
/// <param name="val">The input value for displaying and calculating square result</param>
private static void ShowSquares(int val)
{
    Console.WriteLine("{0:d} squared = {1:d}", val, val * val);
}

The output of the above tests:
One dimensional case
1 squared = 1
2 squared = 4
3 squared = 9
4 squared = 16
Two dimensional case
0 squared = 0
1 squared = 1
2 squared = 4
3 squared = 9
Three dimensional case
0 squared = 0
1 squared = 1
2 squared = 4
3 squared = 9
0 squared = 0
1 squared = 1
2 squared = 4
3 squared = 9

I am wondering the things about the potential drawback or risks of this implementation.
All suggestions are welcome.

The solution is fine as it easy as it could be but there's a better way to make the access to it - extension method.
public static class ArrayHelpers
{
    public static void ForEach<T>(this Array array, in Action<T> action)
    {
        if (ReferenceEquals(array, null))
        {
            throw new ArgumentNullException(nameof(array));
        }

        if (ReferenceEquals(action, null))
        {
            throw new ArgumentNullException(nameof(action));
        }

        foreach (T item in array)
        {
            action.Invoke(item);
        }
        return;
    }
}

Then the test cases can look like this
Console.WriteLine("One dimensional case");
int[] test = new int[] { 1, 2, 3, 4 };
test.ForEach<int>(ShowSquares);

Console.WriteLine("Two dimensional case");
int[,] test2 = { { 0, 1 }, { 2, 3 } };
test2.ForEach<int>(ShowSquares);

Console.WriteLine("Three dimensional case");
int[,,] test3 = { { { 0, 1 }, { 2, 3 } }, { { 0, 1 }, { 2, 3 } } };
test3.ForEach<int>(ShowSquares);


Blackjack / 21 in Python3 I have created a blackjack game in Python3 but I am worried that it is not very efficient and also there are some problems with it.
Problem 1) The player almost always wins unless he goes bust as the program allows the player to take multiple cards but the computer only chooses 2 and I can't think of any solutions.
Problem 2) Because of how I have done the choosing of the suits it is entirely possible to get e.g. 2 Kings of spades which is impossible with a regular deck of cards.
Since I am new to python any help would be much appreciated.

import random

cards = {
    "Ace": 1,
    "two": 2,
    "three": 3,
    "four": 4,
    "five": 5,
    "six": 6,
    "seven": 7,
    "eight": 8,
    "nine": 9,
    "ten": 10,
    "Jack": 10,
    "Queen": 10,
    "King": 10
}

suits = [" of spades", " of clubs", " of hearts", " of diamonds", " of spades", " of clubs", " of hearts",
         " of diamonds", " of spades", " of clubs", " of hearts", " of diamonds", " of spades", " of clubs",
         " of hearts", " of diamonds", " of spades", " of clubs", " of hearts", " of diamonds", ]

again = "yes"

while again == "yes":

    def firstcards():
        global first_card
        global second_card
        global total
        total = 0
        first_card = random.choice(list(cards))
        second_card = random.choice(list(cards))
        total += cards.get(first_card)
        total += cards.get(second_card)

    firstcards()

    def gameplay():
        global total
        another_card = "yes"
        print("Your first card is the " + first_card + random.choice(suits))
        print("Your second card is the " + second_card + random.choice(suits))
        print("Your total is: " + str(total))

        while another_card == "yes" and total < 21:
            print("Would you like another card?")
            another_card = input("Y/N:  ")
            if another_card == "Y":
                card = random.choice(list(cards))
                total += cards.get(card)
                print("Your next card is the " + card + random.choice(suits))
                print("Your total is now: " + str(total))
                another_card = "yes"
            elif another_card == "N":
                num1 = random.randint(1, 10)
                num2 = random.randint(1, 10)
                computerTotal = num1 + num2
                print("The computer got a total of " + str(computerTotal))
                if computerTotal > total:
                    print("The computer wins!")
                else:
                    print("You win!")
                another_card = "no"
            else:
                print("That is not a valid option.")
                another_card = "yes"
        if total > 21:
            print("You have gone bust!")
            print("The computer wins!")

        def play_again():
            global again
            print("")
            print("Would you like to play again?")
            again = input("yes/no: ")
            if again != "yes" and again != "no":
                print("That is not a valid option")
                play_again()
            elif again == "yes":
                print("")
                print("")
            elif again == "no":
                again = "no"
        play_again()

    gameplay()

Once again any help would be much appreciated.

It's hard to make small tweaks to this code to make it do what you want, so I'm going to give some pointers on how to do a wholesale rewrite.
Before you can write a card game, you need to write a data model for a deck of cards.  Picking the right data model makes everything else easy (part of why your program was hard to write and is hard to fix that the cards were modeled in a way that didn't make the rest of the program easy).  Here's how I'd do it:
from enum import auto, Enum
from random import shuffle
from typing import List, NamedTuple

class Rank(Enum):
    ACE = 1
    TWO = 2
    THREE = 3
    FOUR = 4
    FIVE = 5
    SIX = 6
    SEVEN = 7
    EIGHT = 8
    NINE = 9
    TEN = 10
    JACK = 10
    QUEEN = 10
    KING = 10

class Suit(Enum):
    SPADES = auto()
    CLUBS = auto()
    DIAMONDS = auto()
    HEARTS = auto()

class Card(NamedTuple):
    rank: Rank
    suit: Suit
    def __repr__(self) -> str:
        """Pretty-print the name of the card, e.g. 'Queen of Hearts'"""
        return f"{self.rank.name.title()} of {self.suit.name.title()}"

There's my Card class -- every card has a rank and suit.  Enums have the convenient built-in functions name and value that I can use for pretty-printing and also for scoring (I set the value of my Rank enum to correspond to standard scoring numbers).
A NamedTuple is an easy way to build a class that contains multiple other values that never change; it automatically implements a constructor to set its member variables and in many cases you don't need to do anything other than declare what those variables are, but you can also add other methods to it.  My Card implements a magic __repr__ function so that anywhere we print one (including inside a list) it will get formatted as something like Queen of Hearts rather than the default NamedTuple formatting of Card(rank=Rank.QUEEN, suit=Suit.HEARTS).
And here's a really quick example of how you could use this class to implement a game like blackjack.  This isn't an actual blackjack game, I've just implemented the dealer part to demonstrate how building a deck, shuffling, and dealing would work, but hopefully you can play with this code and see how simple everything is when you have a nice data model.  In particular something like the best_value function is going to be really important to a blackjack game (and it's a lot easier to define it as its own function and then be able to call it in a bunch of places).
deck = [Card(rank, suit) for suit in Suit for rank in Rank]
print("Here's our deck fresh from the factory: ", deck)
shuffle(deck)
print("Here it is all shuffled: ", deck)

def best_value(hand: List[Card]) -> int:
    """Get the best blackjack value for a hand of cards (highest without busting)"""
    value = sum([card.rank.value for card in hand])
    if value < 12 and any([card.rank == Rank.ACE for card in hand]):
        # Count one of our aces as 11, adding 10 to the total.
        value += 10
    return value

print()
print("Deal me two!")
hand = [deck.pop(), deck.pop()]
print(f"My hand is {hand}, which is worth {best_value(hand)}")

while best_value(hand) < 17:
    print("Hit me!")
    card = deck.pop()
    hand.append(card)
    print("I got ", card)

if best_value(hand) > 21:
    print("Bust!")
print(f"{hand} = {best_value(hand)}")

Run this code, read through it, understand how it works, then take another crack at writing a blackjack program and hopefully it'll be easier this time around.  :)


Three-layer parallax effect I want to make it lightweight (fastest, best performance) as it possible for browser, but I have no idea - I haven't huge knowledge about JavaScript.

That code run parallax effect on my web page. I have three parallax containers, which are different position background image. Script call on scroll event and when some container exist it's making it parallax-y. The first container ('.parallax-top') exists always and hasn't any offset of image - it is always top-positioned. Second and third have offsets (1700 and 1400).

My page have a lot of content, scripts, images and I want to make all of them lightweight as it is possible.

if ($(window).width() > 1200) {
    (function() {
        var parallaxTop = document.querySelectorAll(".parallax-top");
        if ($(".parallax-center").length) {
            var parallaxCenter = document.querySelectorAll(".parallax-center");
            var centerPosition = $(".parallax-center").position().top;
        }

        if ($(".parallax-bottom").length) {
            var parallaxBottom = document.querySelectorAll(".parallax-bottom");
            var bottomPosition = $(".parallax-bottom").position().top;
        }
        var speed = 0.4;
        window.onscroll = function() {

            [].slice.call(parallaxTop).forEach(function(el, i) {
                var windowYOffset = window.pageYOffset,
                    elBackgrounPos = "center " + "-" + (windowYOffset * speed) + "px";
                el.style.backgroundPosition = elBackgrounPos;
            });
            if ($(".parallax-center").length)
                [].slice.call(parallaxCenter).forEach(function(el, i) {
                    var windowYOffset = window.pageYOffset,
                        elBackgrounPos = "center " + "" + ((windowYOffset - centerPosition) * speed + 1700) + "px";
                    el.style.backgroundPosition = elBackgrounPos;
                });
            if ($(".parallax-bottom").length)
                [].slice.call(parallaxBottom).forEach(function(el, i) {
                    var windowYOffset = window.pageYOffset,
                        elBackgrounPos = "center " + "" + ((windowYOffset - bottomPosition) * speed + 1400) + "px";
                    el.style.backgroundPosition = elBackgrounPos;
                });
        };
    })();
}

First, why are you using querySelectorAll when you use jQuery? Use jQuery's $(SELECTOR_STRING_HERE) instead.
I believe you're using querySelectorAll because you're trying to use forEach. Unfortunately, forEach is slower than your regular loops. Internally, jQuery uses a for-in and for in their each implementation, which may be faster than forEach.
Again, you use jQuery but you use window.onscroll. Use jQuery's event mechanism instead.

I believe you're trying to go native in your code assuming things would go faster. But you're trying to solve the wrong problem. The real problem is the scroll event. Just scrolling a notch of your mouse wheel will fire the scroll event a dozen times, more or less. Now in your handler, you have this:

[].slice.call(parallaxTop)

What this code does is:

Create an array (the [] part)
Create a copy of that array (the call to slice)
It appears 3 times in your handler

That means if the handler fired a dozen times, this means you created 6 arrays per fire. A dozen would mean 72 arrays. Note that when the garbage collector runs, it "stops the world" (stops everything, scripting and rendering), causing freezes every now and then.

Additionally, forEach is synchronous. Nothing else happens when it's running, and doing something that takes too long inside each iteration will hurt performance. That and the GC, you'll eventually leave a very small gap for rendering.

What I suggest is to move your logic in a separate function and throttle your handler. That is, you only call your logic once the scroll has fired for at least some time, or at least a number of times. Only then will you update your element positions.

Now you'd wonder if you scrolled for a second and you scrolled 500px, wouldn't the background just jump? Yes, but that's where CSS transitions come in. You transition the background-position property so that the background will slide (depending on the easing function) to the updated position instead of jumping to it.

But, if I were you, I'd save my time and effort and instead, use a plugin for parallax effects.


Php Login Class - Security and Logic check I write a simple php login class that only operate on php sessions to login user in the system.

My Question is that this class is secure enough to be used on production environment ?

NOTE : Die Statements are used only for testing of scripts

class.login.inc.php

class Login
{

//setSessionFunction sets a login session with user id and all user details array

private function setSession($user_id,$user_details)
{
  $_SESSION['user_id'] = $user_id;
  $_SESSION['user_details'] = $user_details;
  $_SESSION['key'] = $this->hashSession();
}

//GetIp function is used to get ip address of client
private function hashSession()
{
return sha1($_SERVER['HTTP_USER_AGENT'] . getIP());
}

//get sessions and verify them
public function getSession(){

if(isset($_SESSION['user_id']) && ($_SESSION['user_id']!== '') )
 if($this->verifySessionUser($_SESSION['user_id']) === true )
  if($this->hashSession() == $_SESSION['key'])
return true;
else
 return false;

}

//Logout Function
public function logout(){
   $_SESSION['user_id'] = null;
   $_SESSION['key'] = null;
   $_SESSION = array();
   session_unset();
   session_destroy();
}
public function verifyUser($username,$password){
  $password = sha1($password);
  $conn = new mysqli(DB_HOST,DB_USERNAME,DB_PASSWORD,DB_DATABASE);
  if(!$conn) die("Connection Error To DATABASE" . mysqli_connect_errno());
  $sql = "SELECT * FROM user WHERE username = ?";
  $mysqli = $conn->prepare($sql);
  if(!$conn) die("Query Error To DATABASE In Class Session" . mysqli_errno($conn));
  $mysqli->bind_param("s",$username);
  $mysqli->execute();
  $result = $mysqli->get_result();
  $mysqli->close();
  $conn->close();
   if($result->num_rows > 0)
      {
           $r = $result->fetch_assoc();
          if($r['password'] === $password)
          {
     $this->setSession($r['user_id'],$r);
     return true;
          }else return false;
     }
     else
        return false;
        return false;

}

//Private DataBase Verification of User Credentials
private function verifySessionUser($user_id){
$conn = new mysqli(DB_HOST,DB_USERNAME,DB_PASSWORD,DB_DATABASE);
  if(!$conn) die("Connection Error To DATABASE" . mysqli_errno());
  $sql = "SELECT * FROM user WHERE user_id = ?";
  $mysqli = $conn->prepare($sql);
  if(!$conn) die("Query Error To DATABASE In Class Session" . mysqli_errno());
  $mysqli->bind_param("d",$user_id);
  $mysqli->execute();
  $result = $mysqli->get_result();
  $mysqli->close();
  $conn->close();
   if($result->num_rows > 0)
      {
     return true;
     }
     else
        return false;
  }
}

securepage.php

$user = new Session();
   if($user->getSession() == true)
      {
         header("location: admin.php");
         exit();
       }

Is there any security measures i should consider in future ?

Security

sha1 is not secure enough for password hashing as it's way too fast. This is also noted in the documentation. Use bcrypt instead.
=== is not timing safe, use a timing safe string compare instead (bcrypt does this for you).
You should really use salts when hashing passwords (bcrypt does this for you).
You shouldn't safe the password in the session data.

Misc

Your formatting is all messed up, leading to less readable code. Your indentation is off quite often, your usage of newlines and curly brackets is inconsistent, and you should really always use curly brackets, even for one-line statements (to reduce bugs and increase readability; return false; return false; eg is hard to understand, especially with your indentation).
if (cond) return true; else return false; can be written as return cond.
Binding the session to the ip increases security, but decreases usability (eg for mobile users).
Having three nested ifs instead of one if with & can sometimes reduce readability.
You seem to use == and === interchangeably. In most cases, === would be the correct choice, as it can avoid bugs.
You should only create one db object and pass that around, instead of creating a new one each time.
I'm not sure what verifySessionUser is actually used for. The user didn't set the user id, you did. So it should always be correct, and the check should be unnecessary.
getSession: From the name, I would expect that this returns a session. But it doesn't. isLogedIn seems more fitting. And then I would rename verifyUser to logIn to make it fit.


Prime Number Finder in Python I am creating a simple program that will do three things: add up the prime numbers found within the range set by the user, print out the entire list of prime numbers, and print how many prime numbers were found.

# Starting number
lower = int(input("Enter lower range: "))
# Final number
upper = int(input("Enter upper range: "))
print "Running..."
# Variables to keep track of progress
total = 0
list_of_primes = []

# Loop to see whether the number is prime or not
for num in range(lower, upper + 1):
    if num > 1:
        for i in range(2, num):
            if (num % i) == 0:
                break
        else:
            total += num
            list_of_primes.append(num)
    if num == 2:
        total += num
        list_of_primes.append(num)

# Print out the results
print "Finished..."
print "Sum of prime numbers:", total
print "List of prime numbers:", list_of_primes
print "Number of prime numbers:", len(list_of_primes)

The time waiting can be very long, especially with large numbers (max is around 3,000,000,000). Is there any way to shorten the time needed to run the program (thus improve efficiency) and make the code more elegant and readable?

Note: Negative numbers will work. No errors will be thrown.

EDIT: I have added three more lines of code, which helps to reduce time spent by removing all even numbers immediately except 2, which actually is prime.

Your coding style is clean enough, although you could add longer more descriptive names, and use functions instead of doing everything at the top level. However your main issues are related to choice of algorithm for finding the prime numbers.

Your algorithm is currently to test whether it is dividable by all numbers below it self. And this you repeat for each new number. This is by far the most ineffective algorithm for finding primes, so here are some basic stuff to do better algorithms:

No need to check even numbers – Your loop could skip every even number since the division by 2 should eliminate all other even numbers...
No need to check beyond square root of \$n\$ – If none of the numbers below the square root divides it, then none above will divide it either. This makes sense as if it's not prime you'll have that \$a*b = n\$, and if \$a\$ is larger than the square root, \$b\$ has to be lower.
Avoid multiplies of 3's (as well as 2's) – Another optimization of the brute force is to change the incrementation from 2, as in the sequence: 5, 7, 9, 11, 13, 15, 17, 19, 21, ... One can see that to avoid the 3's (in bold) one adds +2, +4, +2, +4. This can be simplified to an initial value of increment = 2 and then let increment = 6 - increment.
Keep track of earlier primes – Finally, still using a naive approach, you should keep track of earlier primes, as for lower values testing for division against non-primes is also a waste of time.

However you should probably look into more efficient algorithms (see Wikipedia's Generating primes. One good candidate is the Sieve of Erasthones, i.e. like in this answer which has a cool animation as well.


Using Javascript FileWriter with minimal use of closures I'm currently writing a WebGL HTML5 game, and large part of the game initialization involved loading assets streamed over a websocket, and saving them to disk using the FileSystem API.

As part of optimizing the code, we're trying to eliminate as many memory allocations as we can, which means cutting down on the use of closures, object/array creation during the main game loop and so on.

One of the biggest uses of closures left is our interaction with the FileSystem API, namely the part where we write the assets to disk:

//Write file to disk
directory.getFile(
    filename,
    CREATE_TRUE,
    function(fileEntry) {
        fileEntry.createWriter(
            function(fileWriter) {
                fileWriter.onwriteend=function(e) {
                    finishedRequest(filename);
                    if (onSuccess!==undefined) onSuccess(filename);
                };
                fileWriter.onerror=function(e) {
                    finishedRequest(filename);
                    if (onError!==undefined) onError(filename, "Write error: "+directoryRoot+"/"+filename);
                };
                var dataBlob=new Blob([new Uint8Array(data)]);
                if (append) fileWriter.seek(fileWriter.length);
                fileWriter.write(dataBlob,"application/octet-stream");
             },
             function(e) {
                 finishedRequest(filename);
                 if (onError!==undefined) onError(filename,"File writer creation error: "+directoryRoot+"/"+filename:"");
             }
        );
    },
    function(e) {
        finishedRequest(filename);
        if (onError!==undefined) onError(filename,DEVELOPMENT_MODE?"File not found: "+directoryRoot+"/"+filename:"");
    }
);

As you can see, this code involves the creation of something like six new closures for every file write, which isn't the greatest when we're trying to cut down on memory allocation.

I initially tried to pull each of the closures out into standalone functions, but obviously I then needed to be able to get references to things like the filename and onSuccess callback function from those standalone functions (whereas previously they would have been able to have access to them from their outer closures):

...
fileWriter.onwriteend=onFileWrite;
...

function onFileWrite=function(e) {
    finishedRequest(filename);  //filename needs to be retrieved from somewhere...
    if (onSuccess!==undefined) onSuccess(filename);
}

The problem is that the onFileWrite callback is only supplied with the event object, which contains a reference to the target fileWriter, but nothing higher.  Crucially, there seems to be no way to link the fileEntry with the fileWriter that it creates, as the createWriter callback only gets given the FileWriter object - with no meta data about what created it.

My question is this: is it possible to rewrite this without using quite some many closures - or ideally none at all?

Edit: I'm convinced that it is (unfortunately) impossible to do this without creating some kind of container for the details of the request, be it a new function whose closure includes the request details, or a new object whose properties have the details.  So, this question really then becomes about the best way to achieve the file write with the smallest amount of memory churn.

I think that you will always need some closures, but you can limit what they carry.
This was your example:

...
fileWriter.onwriteend=onFileWrite;
...

function onFileWrite=function(e) {
    finishedRequest(filename);  //filename needs to be retrieved from somewhere...
    if (onSuccess!==undefined) onSuccess(filename);
}

I guess that your intention was to define a function for each case and then assign it.
So your example transforms into this (I'm just reorganizing your code and fixing a function declaration).

var onFileWrite = function(e) {
    finishedRequest(filename);  //filename needs to be retrieved from somewhere...
    if (onSuccess!==undefined) onSuccess(filename);
}

...
fileWriter.onwriteend=onFileWrite;
...

This is actually an optimization because you can reuse things, the problem now is how do we get the filename. Well you can write a function that gets the filename and return a new function that only has the filename in its closure and nothing else. Like this:

var onFileWrite = function(filename) {
    return function(e) {
        finishedRequest(filename);
        if (onSuccess!==undefined) onSuccess(filename);
    }
}

...
fileWriter.onwriteend=onFileWrite(filename);
...

Note that this way you just pass what you need, variables, functions but always controlling the closure "size".

Hope it helps you, good luck with the game!


Network byte order to host byte order While doing a little project involving sniffing packets, I found myself programming a cross platform network to host byte order and host to network byte order, this is my first time developing a project using networking.

ntboc.h

#ifndef NTBOC_H_INCLUDED
#define NTBOC_H_INCLUDED

#define LITTLE_EDIAN    1
#define BIG_EDIAN       2

// convert from host to network byte order
void ntboc_tont(void *data, size_t len);
//convert from network to host byte order
#define ntboc_toht(data, len)   (ntboc_tont(data,len))
#endif

ntboc.c

#include <stdlib.h>
#include "ntboc.h"

static unsigned int ntbo = 0;

static void ntboc_init()
{
    int t = 1;
    if(*((char *)&t))
        ntbo = LITTLE_EDIAN;
    else
        ntbo = BIG_EDIAN;
}

static void ntboc_swap(void *data, size_t len)
{
    size_t off = len-1;

    unsigned char *ddata = data;
    unsigned char *sdata = data + off;
    unsigned char tmp;

    while((void *)sdata > (data + (off)/2)) {
        tmp = *ddata;
        *ddata = *sdata;
        *sdata = tmp;

        sdata--, ddata++;
    }

}

void ntboc_tont(void *data, size_t len)
{
    if(ntbo == 0)
        ntboc_init();

    if(ntbo == BIG_EDIAN)
        return;

    ntboc_swap(data, len);
}

main()
{
    short len = 0x3e;
    ntboc_toht(&len, 2);
    printf("len : %x", len);
}

The problem with your approach is that your are trying to simultaneously satisfy two contradictory requirements: produce a robust cross-platform implementation, and explicitly deal with different hosts.

A better approach is to avoid any knowledge of the host's endianness by using shifts and or's. It is, however, perfectly fine to rely on network byte order being big-endian as that is an invariant.

Consider the conversion to network order first. This is the simpler direction as it is always fixed but, in C at least, you will need to write routines to deal with the different sizes of int. For now, we will deal with unsigned int only.

void uint_to_network(unsigned int value, unsigned char * buffer, size_t buffer_size)
{
    size_t nw_length = sizeof(value);

    if (buffer_size < nw_length) return; // Avoid potential overflow

    // Note that this loops until all possible locations in the
    // output stream are filled, automatically handling leading zeroes.
    while(nw_length != 0)
    {
        // Pre-decrementing nw_length deals with the zero-based nature
        // of C arrays and avoids us having to worry about the representation
        // of -1 as an unsigned value.
        nw_length--;
        buffer[nw_length] = value & 0xff;
        value >>= 8;
    }
}

Note that this code is written in terms of the network order, with no knowledge of the host's endianness. Thus, it is inherently cross-platform.

Converting from network to host follows a similar pattern.

unsigned int network_to_uint(unsigned char * buffer, size_t input_length)
{
    unsigned int result = 0;
    size_t decode_length;

    decode_length = (input_length > sizeof(unsigned int))
                      ? sizeof(unsigned int)
                      : input_length;

    for (size_t i = 0; i < decode_length; i++)
    {
        result <<= 8;
        result |= (unsigned int) buffer[i];
    }

    return result;
}

Again, the code is written in terms of the known network order and makes no reliance on the host order.

Also note that, whenever dealing with network side data, I always use the unsigned char format. This is because network data is basically a 'bit stream', and you should never try to deal with it as a different native data type. That way lies madness .. or at least very subtle bugs.

Now, in a production implementation where execution speed is all important, you would actually have two implementations of the routines. On a big-endian host they would be no-ops, or at worst a macro performing a cast. On a little-endian host, they would be heavily optimized based on the knowledge that conversion was always required. Then your build-script would link in the set of routines that made sense for the target: and you would have some intelligence in the build script if only to select the correct target-processor for the cross compiler.

But for a true cross-platform single-implementation solution, avoiding knowledge of the host's endianness is the only sure route to avoiding problems.

By the way, if you change the return type of network_to_uint to unsigned long long (or unsigned long if your compiler is too old for long long) it can deal with all of the unsigned integer types in the network data by passing in the correct value for input_length. But you will still need a different routine to deal with signed data. And remember that, for an unsigned int in the network data you need to specify the correct input length. A network int may not be the same size as your host's.

For signed data, you have to be a little more careful. There are hosts out there which store data with a separate sign bit (i.e. they can represent both positive and negative zero) and C is designed to run on those as well.

And really, my uint_to_network has the fault that it assumes sizeof(unsigned int) on the host is the same as its representation in a network data stream. It really ought to take an output_length argument instead of setting nw_length to sizeof(unsigned int). Then it can also be made to work for all unsigned data, because it only extracts the least significant bytes of the data.


Recommended improvements on beginner Linked List I know this implementation is very bare-bones, but are there any improvements I can make to it? (E.g. more pythonic, style, speed, comments).

I was considering using @property decorators a bit more, but I thought that was overkill for this example.

class Node:
    def __init__(self, value=None, next=None):
        self.value = value
        self.next = next

    def __str__(self):
        return "Node Object : val {}".format(self.value)

class LinkedList:
    def __init__(self):
        self.head = None

    @property
    def length(self):
        if self.head is None:
            return 0
        length = 1
        current = self.head
        while current.next is not None:
            current = current.next
            length += 1
        return length

    def append(self, node):
        if self.head is None:
            self.head = node
            return
        current = self.head
        while current.next is not None:
            current = current.next
        current.next = node

    def prepend(self, node):
        if self.head is None:
            self.head = node
            return
        node.next = self.head
        self.head = node

    def get(self, index):
        current = self.head
        for i in range(0, index):
            if current.next is not None:
                current = current.next
            else:
                return None
        return current

    def delete(self, val):
        if self.head.value is val:
            self.head = None
            return
        current = self.head
        while current.next is not None:
            if current.next.value is val:
                current.next = current.next.next
                return
            current = current.next

There are a couple of things to consider.

Pythonic Attributes

Using built-in functions

You have defined your own version of list.length, but Python provides a built-in function __len__(). So you could change:

@property
def length(self):
    # some code

to:

def __len__(self):
    # some code

Also, you could override the built-in __str__() function to visualize your list:

def __str__(self):
     nodes = []
     temp = self.head
     for i in range(self.length):
         nodes.append(str(temp.value))
         temp = temp.next
     return ' --> '.join(nodes)

Also, you can implement an iterator:

def __init__(self):
    self.head = None
    self.iter_ref = self.head
    self.itered = False

def __iter__(self):
    return self

def __next__(self):
    if self.iter_ref is None and self.itered:
        self.itered = False
        raise StopIteration
    else:
        if self.iter_ref is None and not self.itered:
            self.iter_ref = self.head
            self.itered = True
        ret_val = self.iter_ref.value
        self.iter_ref = self.iter_ref.next
        return ret_val

Also, there is a built-in function for get() called __getitem()__. It takes, the same parameters as your function.

Convenience

One more thing, instead of taking in a Node as a parameter for your LinkedList functions, consider just taking in the raw data type. That means you just need to change the first two lines of the prepend() and append() to look like (for example):

def append(self, data):
    node = Node(data)
    # rest of the implementation

So now, to use your code, you can do the following:

ll = LinkedList()
ll.append(1)
ll.prepend(3)
print(len(ll))  # prints "2"
print(str(ll))  # prints "3 --> 1"
print(ll[0])    # prints "3"
print(list(ll)) # prints "[3, 1]"

Optimizations

If you decided to take in raw data types as mentioned above, you can now perform these optimizations:

Computation of Length

Rather than iterate through the entire list every time you need the length, just keep the length as a member variable and increment/decrement it accordingly. Make sure to initialize self.length = 0 in your constructor, then in your append() and prepend() methods, make sure to increment the length, and in your delete() method, make sure to decrement the length when you actually delete a Node. Then, your __len__() method can just look like:

def __len__(self):
    return self.length

Append

Also, your append() function could be optimized by keeping a pointer to the tail rather than iterating to the tail each time:

def append(self, data):
    node = Node(data)
    if self.head is None:
        self.head = self.tail = node
    else:
        self.tail.next = node
        self.tail = node
    self.length += 1

def prepend(self, data):
    node = Node(data)
    if self.head is None:
        self.head = self.tail = node
    else:
        node.next = self.head
        self.head = node
    self.length += 1

Make sure to add self.tail = None in your constructor as well.

Here is the code that I suggested:

class Node:
    def __init__(self, value=None, next=None):
        self.value = value
        self.next = next

    def __str__(self):
        return "Node Object : val {}".format(self.value)

class LinkedList:
    def __init__(self):
        self.head = self.tail = None
        self.length = 0
        self.iter_ref = self.head
        self.itered = False

    def __len__(self):
        return self.length

    def __str__(self):
        nodes = []
        temp = self.head
        for i in range(self.length):
            nodes.append(str(temp.value))
            temp = temp.next
        return ' --> '.join(nodes)

    def __iter__(self):
        return self

    def __next__(self):
        if self.iter_ref is None and self.itered:
            self.itered = False
            raise StopIteration
        else:
            if self.iter_ref is None and not self.itered:
                self.iter_ref = self.head
                self.itered = True
            ret_val = self.iter_ref.value
            self.iter_ref = self.iter_ref.next
            return ret_val

    def append(self, data):
        node = Node(data)
        if self.head is None:
            self.head = self.tail = node
        else:
            self.tail.next = node
            self.tail = node
        self.length += 1

    def prepend(self, data):
        node = Node(data)
        if self.head is None:
            self.head = self.tail = node
        else:
            node.next = self.head
            self.head = node
        self.length += 1

    def __getitem__(self, index):
        current = self.head
        for i in range(index):
            if current.next is not None:
                current = current.next
            else:
                return None
        return current.value

    def delete(self, val):
        if self.head.value is val:
            self.head = None
            self.length -= 1
            return
        current = self.head
        while current.next is not None:
            if current.next.value is val:
                current.next = current.next.next
                self.length -= 1
                return
            current = current.next


Sentence building DSL Cardshifter is an online trading card game, made by a gang of Code Review community members. Cards can have a bunch of different, and sometimes complex effects, and descriptions for these are automatically generated if no custom one is given.

  EIGHT IMMORTALS:
  Choose 1 at random: Summon 1 Life Tool to your Battlefield or Summon 1 Destruction Tool to your Battlefield

The effect description code is quite messy and hard to adapt, which is why I've started a refactoring effort to improve it. The class up for review here is part of this effort.

Design aims

The ability to act as a Strangler Vine and gradually replace what already exists. This is achieved through the text method that simply proxies its input straight to the string builder (which is what was earlier used).
I wanted the class to be very robust and still produce consistent output no matter what is thrown at it. This is largely because of the legacy code, which is inconsistent in some areas.

The code

/**
 * Build sentences using a convenient DSL. Sentences are capitalized and end with a period.
 */
class SentenceBuilder {

    private StringBuilder stringBuilder = new StringBuilder()

    static String build(@DelegatesTo(strategy = Closure.DELEGATE_FIRST, value = SentenceBuilder) Closure closure) {
        def builder = new SentenceBuilder()
        closure.delegate = builder
        closure.resolveStrategy = Closure.DELEGATE_FIRST
        closure.call()

        return builder.toString()
    }

    String toString() {
        String sentence = stringBuilder.toString()

        sentence = sentence.trim()
        if (sentence.length() > 0 && !sentence.endsWith('.')) {
            sentence = sentence + '.'
        }
        sentence = sentence.capitalize()

        return sentence
    }

    /**
     * Add some text.
     */
    void text(String string) {
        stringBuilder.append(string)
    }

    /**
     * Add a separator if not already present and it's not the start of the sentence.
     * @param sep The separator, e.g. ' ' or ', '.
     */
    void separator(String sep) {
        if (stringBuilder.length() > 0 && !stringBuilder.toString().endsWith(sep)) {
            stringBuilder.append(sep)
        }
    }

    /**
     * Add a list like 'egg, bacon and ham'.
     * @param conjunction Conjunction between the last two elements, e.g. 'and'.
     * @param elements A list of the elements, e.g. ['egg', 'bacon', 'ham'].
     */
    void list(String conjunction, List<String> elements) {
        if (elements.size() > 0) {
            stringBuilder.append(elements[0])

            if (elements.size() > 1) {
                for (int i = 1; i < elements.size() - 1; i++) {
                    separator(', ')
                    stringBuilder.append(elements[i])
                }

                separator(' ')
                stringBuilder.append(conjunction)
                separator(' ')
                stringBuilder.append(elements[elements.size() - 1])
            }
        }
    }

}

I don't really like the implementation of list - it doesn't look very groovy at all, but I didn't come up with a better way to handle all cases.

Usage example

def things = ['cats', 'crackers', 'computers']
def feeling = 'like '

SentenceBuilder.build {
    text 'i'
    separator ' '
    text feeling
    separator ' '
    list 'and', things
}

// I like cats, crackers and computers.

Here's another way to implement the list() method:

switch(elements.size()) {
    case 0:
        null
        break
    case 1:
        elements[0]
        break
    case 2:
        elements.join(" $conjunction ")
        break
    default:
        new StringBuilder().with {
            append elements.take(elements.size() - 1).join(', ')
            append " $conjunction "
            append elements.last()
        }.toString()
        break
}

This makes use of join() to handle most of the String building, instead of looping and appending to StringBuilder. The StringBuilder only comes in during the case of 3 or more items.

Revised

It's possible to knock out one of the cases like this:

switch(elements.size()) {
    case 0:
        ''
        break
    case 1:
        elements[0]
        break
    default:
        new StringBuilder().with {
            append elements.take(Math.max(elements.size() - 1, 1)).join(', ')
            append " $conjunction "
            append elements.last()
        }.toString()
        break
}

So now there are cases for an empty list, a one-item list, and more-than one-item list.


Clustering using k-medoids This is the program function code for clustering using k-medoids

def kMedoids(D, k, tmax=100):
    # determine dimensions of distance matrix D
    m, n = D.shape
    # randomly initialize an array of k medoid indices
    M = np.sort(np.random.choice(n, k)
    # create a copy of the array of medoid indices
    Mnew = np.copy(M)
    # initialize a dictionary to represent clusters
    C = {}
    for t in range(tmax):
    # determine clusters, i.e. arrays of data indices
        J = np.argmin(D[:,M], axis=1)
        for kappa in range(k):
            C[kappa] = np.where(J==kappa)[0]
        # update cluster medoids
        for kappa in range(k):
            J = np.mean(D[np.ix_(C[kappa],C[kappa])],axis=1)
            j = np.argmin(J)
            Mnew[kappa] = C[kappa][j]
            np.sort(Mnew)
            # check for convergence
            if np.array_equal(M, Mnew):
                break
                M = np.copy(Mnew)
            else:
                # final update of cluster memberships
                J = np.argmin(D[:,M], axis=1)
                for kappa in range(k):
                    C[kappa] = np.where(J==kappa)[0]
                    # return results
            return M, C

and the I will call The function KMedoids with this program, I think my program run slowly in line D = Pairwise_distances(arraydata, metric='euclidean')

D = pairwise_distances(arraydata,metric='euclidean')

# split into 2 clusters
M, C = kMedoids(D, 2)

print('medoids:')
for point_idx in M:
    print(arraydata[point_idx] )

print('')
# array for get label
temp = []
indeks = []
print('clustering result:')
for label in C:
    for point_idx in C[label]:
        print('label {0}:　{1}'.format(label, arraydata[point_idx]))
        temp.append(label)
        indeks.append(point_idx)

This is the result from this program

clustering result:
label 0:　[0.00000000e+00 0.00000000e+00 1.00000000e+00 1.00000000e+00
1.00000000e+00 1.00000000e+00 1.00000000e+00 1.00000000e+00
1.00000000e+00 1.00000000e+00 1.00000000e+00 1.00000000e+00
1.00000000e+00 1.00000000e+00 1.00000000e+00 1.00000000e+00
1.00000000e+00 1.00000000e+00 1.00000000e+00 1.00000000e+00
1.00000000e+00 1.00000000e+00 1.00000000e+00 1.00000000e+00
1.00000000e+00 0.00000000e+00 1.00000000e+00 1.00000000e+00

Why my result of my program is slow for large data and almost have a result "Memory Error"?  I hope someone can help me to review this code to improve its performance to get the result and process large amounts of data.

This is exactly the implementation found in NumPy and SciPy Recipes for Data Science on k-Medoids Clustering but with some indentation mistakes (probably due to the copy & paste process). To understand the indentation being used in the algorithm is might be worth to read the answers on the for...else syntax:
https://stackoverflow.com/questions/9979970/why-does-python-use-else-after-for-and-while-loops#9980752
A working script with the correct indentation and some improvement is found at
https://github.com/letiantian/kmedoids/blob/master/kmedoids.py :
import numpy as np
import random

def kMedoids(D, k, tmax=100):
   # determine dimensions of distance matrix D
   m, n = D.shape

   if k > n:
       raise Exception('too many medoids')

   # find a set of valid initial cluster medoid indices since we
   # can't seed different clusters with two points at the same location
   valid_medoid_inds = set(range(n))
   invalid_medoid_inds = set([])
   rs,cs = np.where(D==0)
   # the rows, cols must be shuffled because we will keep the first duplicate below
   index_shuf = list(range(len(rs)))
   np.random.shuffle(index_shuf)
   rs = rs[index_shuf]
   cs = cs[index_shuf]
   for r,c in zip(rs,cs):
       # if there are two points with a distance of 0...
       # keep the first one for cluster init
       if r < c and r not in invalid_medoid_inds:
           invalid_medoid_inds.add(c)
   valid_medoid_inds = list(valid_medoid_inds - invalid_medoid_inds)

   if k > len(valid_medoid_inds):
       raise Exception('too many medoids (after removing {} duplicate points)'.format(
        len(invalid_medoid_inds)))

   # randomly initialize an array of k medoid indices
   M = np.array(valid_medoid_inds)
   np.random.shuffle(M)
   M = np.sort(M[:k])

   # create a copy of the array of medoid indices
   Mnew = np.copy(M)

   # initialize a dictionary to represent clusters
   C = {}
   for t in xrange(tmax):
       # determine clusters, i. e. arrays of data indices
       J = np.argmin(D[:,M], axis=1)
       for kappa in range(k):
           C[kappa] = np.where(J==kappa)[0]
       # update cluster medoids
       for kappa in range(k):
           J = np.mean(D[np.ix_(C[kappa],C[kappa])],axis=1)
           j = np.argmin(J)
           Mnew[kappa] = C[kappa][j]
       np.sort(Mnew)
       # check for convergence
       if np.array_equal(M, Mnew):
           break
       M = np.copy(Mnew)
   else:
       # final update of cluster memberships
       J = np.argmin(D[:,M], axis=1)
       for kappa in range(k):
           C[kappa] = np.where(J==kappa)[0]

   # return results
   return M, C

Note that the else as well as the return are at the same indentation as the for-loop.
However, I recommend to change the second inner loop
# update cluster medoids
for kappa in range(k):
       J = np.mean(D[np.ix_(C[kappa],C[kappa])],axis=1)
       j = np.argmin(J)
       Mnew[kappa] = C[kappa][j]

to
# update cluster medoids
for kappa in range(k):
       J = np.mean(D[np.ix_(C[kappa],C[kappa])],axis=1)
       # Fix for the low-idx bias by J.Nagele (2019):
       shuffled_idx = np.arange(len(J))
       np.random.shuffle(shuffled_idx)
       j = shuffled_idx[np.argmin(J[shuffled_idx])]
       Mnew[kappa] = C[kappa][j]

as otherwise the resulting medoids depend on the order of the input. That is because the argmin command returns always the lowest index in case multiple potential medoids have equal distances to the cluster members.


Leftmost bisect search to find valid perfect square I am working on Valid Perfect Square - LeetCode
and wrote a standard leftmost bisect search to solve the problem:

class Solution:
    def isPerfectSquare(self, num: int) -> bool:
        #base case 1
        if num == 1: return True
        lo = 1
        hi = num
        #recur case
        while lo < hi:
            mid = (lo + hi) // 2
            if num > mid ** 2:
                lo = mid + 1
            else:
                hi = mid
        return True if lo ** 2 == num else False

However, the score is annoying:

  Runtime: 48 ms, faster than 24.22% of Python3 online submissions for Valid Perfect Square.
  Memory Usage: 13.2 MB, less than 5.17% of Python3 online submissions for Valid Perfect Square.

I am not able to improve a standard template a little bit.

Could you please provide any advice?

Some small points:

ternary expression

No need for return True if lo ** 2 == num else False. you can use just return lo ** 2 == num

early return

If mid**2 == num, you can return early:

sq = mid ** 2
if num == sq:
    return True
if num > sq:
    lo = mid + 1
else:
    hi = mid -1

returns early in this case. This reduced the time for me to 36ms.

unpacking

If you want, you can use a ternary expression and tuple unpacking to redefine hi and lo

lo, hi = (mid + 1, hi) if num > sq else (lo, mid - 1)

but this is a matter of taste, and doesn't seem to save any time or memory.

memory

I don't see a lot of ways in which to reduce this method's memory usage. If you look at the distribution of all submissions, almost all use this bisect method, so memoty reduction will be based on the wims of the VM.


Flood fill algorithm in Java Doing homework, implementation of the algorithm flood fills. I'm writing a program to this guide: http://en.wikipedia.org/wiki/Flood_fill. And I have some questions:

Is it normal to specify the parameters in function replaces the color of any character bucketFill.fill (0, 0, '*', 'O');, I do not know what kind of color will be initially to these coordinates?
An algorithm is correct? I wrote it for example in Wikipedia, but the result of my program is as follows:

@@@@@
@@@@@
@@@@@
@@@@@
@@@@@
@@@@@
@@@@@

I think must be this:

*@@@@
@@@@@
@@#@@
@@@@@
@@@@@
@@@##
@@@@@

My code:

class BucketFill {

    private char[][] pixels;

    public BucketFill(char[][] pixels) {
        this.pixels = pixels;
    }

    public void fill(int x, int y, char newColor, char oldColor) {
        if (x < 0) return;
        if (y < 0) return;
        if (x >= pixels.length) return;
        if (y >= pixels[x].length) return;

        oldColor = pixels[x][y];

        if (newColor == pixels[x][y]) return;
        if (oldColor != pixels[x][y]) return;

        pixels[x][y] = newColor;

        fill(x - 1, y, newColor, oldColor);
        fill(x + 1, y, newColor, oldColor);
        fill(x, y - 1, newColor, oldColor);
        fill(x, y + 1, newColor, oldColor);
    }

    public void inspect() {
        for (int y = 0; y < pixels.length; y++) {
            for (int x = 0; x < pixels[y].length; x++) {
                System.out.print(pixels[y][x]);
            }
            System.out.print("\n");
        }
    }

    public static void main(String argv[]) {
        char pixels[][] =
        {
            { 'O', 'X', 'X', 'X', 'X' },
            { 'X', 'O', 'O', 'O', 'X' },
            { 'X', 'O', '#', 'O', 'X' },
            { 'X', 'O', 'O', 'O', 'X' },
            { 'X', 'X', 'X', 'X', 'X' },
            { 'X', 'X', 'X', '#', '#' },
            { 'X', 'X', 'X', 'X', 'X' }
        };
        BucketFill bucketFill = new BucketFill(pixels);
        bucketFill.fill(0, 0, '*', 'O');
        bucketFill.fill(3, 0, 'O', 'O');
        bucketFill.fill(2, 1, '@', 'O');
        bucketFill.inspect();
    }
}

Normally there's no need to specify the color to change from. You only need to specify the coordinates and leave it up to the flood fill routine to find out what color is at that location. I would define another method that is public, and make the recursive method a private implementation method. The public method would look up the color at the location and then call the private method.

public void fill(int x, int y, char newColor) {
    fill(x, y, newColor, pixels[x][y]);
}

private void fill(int x, int y, char newColor, char oldColor) {
    ...
}

You are not using oldColor correctly; you're overwriting the parameter value. The line:

oldColor = pixels[x][y];

should either be deleted or (better) amended to:

char color = pixels[x][y];

And the lines:

if (newColor == pixels[x][y]) return;
if (oldColor != pixels[x][y]) return;

should then be changed to use the color variable:

if (newColor == color) return;
if (oldColor != color) return;

Other than that, you have implemented the algorithm correctly.

It's worth noting that the way this algorithm uses recursion could lead to a stack overflow for a large block of color. Also, by dint of the +1 and -1, will end up calling fill() multiple times for each cell.


Read/write a pipe-delimited file line by line with some simple text manipulation This code that I wrote is supposed to read/write a pipe-delimited file line by line to a new file with some simple text manipulation. (It also adds two new columns) and publishes a "Status Update" ever 100,000 lines to keep me updated on how close it is to completion.

I previously posted this code on StackOverflow to get help with incrementing, and someone mentioned that it would be faster if I did not open the second text file, but being extremely new at Python, I do not understand how to do that without potentially breaking the code.

counter=1
for line in open(r"C:\Path\name.txt"):
    spline = line.split("|")
    if counter==1:
        with open(r"C:\PATH\2019.txt",'a') as NewFile:
            spline.insert(23,"Column A")
            spline.insert(23,"Column B")
            s="|"
            newline=s.join(spline)
            NewFile.write(newline)

    elif counter > 1 and not spline[22]=="0.00":
        spline.insert(23,"")
        spline.insert(23,"")
        gl=spline[0]
        gl=gl.strip()
        if gl[0]=="-": gl="000" + gl
        gl=gl.upper()
        spline[0]=gl

        if gl[:3]=="000": spline[24]="Incorrect"

        s="|"
        newline=s.join(spline)

        with open(r"C:\PATH\PythonWrittenData.txt",'a') as NewFile:
            NewFile.write(newline)

    counter+=1
    if counter%100000==0: print("Status Update: \n", "{:,}".format(counter))

Here is another way to organize your code. Instead of an if within the loop, use iterators more explicitly. Concretely:

with open(r"C:\Path\name.txt") as source:
    lines = iter(source)

    # first line
    first_line =  next(lines)
    with open(r"C:\PATH\2019.txt") as summary:
        # ... omitted ...

    # remaining lines
    with open(r"C:\PATH\PythonWrittenData.txt", 'a') as dest:
        for counter, line in enumerate(lines, start=1):
            # ... omitted ...

I have also used enumerate to update counter and line simultaneously.

The other answer has some more tips on writing good python code. But as far as structuring the opening and closing of files, as well as the main loop, this approach should get you started.


"Favorite food" program This is my first program written in C++.  I chose to make this public in order to receive suggestions on further improvements and to know if I'm using any "forbidden" code.

I'm asking because I initially put a lot of gotos in the program, but later found out that using goto was "evil."  Therefore, after a lot of trial and error, I removed them all and replaced them with a couple of do-while loops. I must say using do-while makes the final code much easier to read.

Please let me know if you still see any "forbidden" code.

#include <iostream>
#include <string>

using namespace std;

int main () {

    string name, favfood, age, y_n_decision;

    do {

        cout << "welcome to my program in c++" << endl;
        cout << "Please enter your name" << endl;
        getline (cin, name);

        cout << "enter your favourite food" << endl;
        getline (cin, favfood);
        cout << "awesome! " << favfood << " is my favourite food too" << endl;

        cout << "enter your age" << endl;
        getline (cin, age);

        cout << " so your name is " << name << " and your fav food is "
        << favfood << " and your age is " << age << endl;

        cout << "please press Y to restart, or Q to quit" << endl;

        do {

            getline (cin, y_n_decision);

            if (y_n_decision == "q") {
                cout << "quitting now" << endl;
                return 0;
            }

            else if (y_n_decision == "y") {
                cout << "nice! you typed y. Exiting current loop and returning to int main() aka the beginning." << endl;
                break;
            }

            else {
                cout << "you didn't type either y or q. YOU MUST PRESS Y OR Q OR I WILL PROMPT U FOREVER!!" << endl;
            }

        } while (y_n_decision == "y" || "n");

    } while (y_n_decision == "y" || "n");

    return 0;
}

EDITED:

#include <iostream>
#include <string>

using std::cout;
using std::endl;
using std::string;
using std::cin;

int main () {

    string name, favfood, age;
    string y_n_decision;

    while (y_n_decision != "y" || y_n_decision != "q") {
        cout << "welcome to my program in c++" << endl;
        cout << "Please enter your name" << endl;
        getline (cin, name);

        cout << "enter your favourite food" << endl;
        getline (cin, favfood);
        cout << "awesome! " << favfood << " is my favourite food too" << endl;

        cout << "enter your age" << endl;
        getline (cin, age);

        cout << " so your name is " << name << " and your fav food is "
        << favfood << " and your age is " << age << endl;

        cout << "please press y to restart, or q to quit" << endl;

        while (y_n_decision != "y" || y_n_decision != "q") {

            getline (cin, y_n_decision);

            if (y_n_decision == "q") {
                cout << "quitting now" << endl;
                return 0;
            }

            else if (y_n_decision == "y") {
                cout << "nice! you typed y. Exiting current loop and returning to int main() aka the beginning." << endl;
                break;
            }

            else {
                cout << "you didn't type either y or q. YOU MUST PRESS y OR q OR I WILL PROMPT U FOREVER!!" << endl;
            }
        }

    }
    return 0;
}

Armond, the main thing you should do is to learn about functions.  Putting everything in main is not practicable beyond very small programs.   And nested loops are generally a bad idea too.  So for example you might extract the content of the first loop into a function and extract the second loop into another function.

So you would get something like:

int main(int argc, char **argv)
{
    do {
        user_interaction();
    } while (user_says_exit() == false);
    return 0;
}

Also note that while (true) {...} is more commonly used than do {... } while (true); although do{} while() loops clearly have their uses.

EDIT

To be honest, I like the edited version less than the original.  You are now testing the y_n_decision as the condition in both loops. And you are relying on the variable having been default-initialized. Moreover the loops are now while instead of do-while and are arguably more wrong now than they were before.  A while (true) loop is preferable to your original do ... while (true)  because the condition (true) is immediately visible at the top of the loop.  But the new loops using while (condition) rely upon the initial value of the condition - and in your case you have to force that to be false by using an initializer that is not y/q. This is at best inelegant.

Here is how I might have coded what you are doing:

#include <iostream>
#include <string>
#include <ctype.h>

using std::cout;
using std::cin;
using std::endl;

static bool user_says_exit(void)
{
    std::string decision;
    do {
        cout << "Please press y to restart, or q to quit" << endl;
        getline(cin, decision);
    } while (decision != "y" && decision != "q");
    return (decision != "y");
}

static void user_interaction(void)
{
    std::string age;
    std::string favfood;
    std::string name;
    cout << "Welcome to my program in c++" << endl;
    cout << "Please enter your name" << endl;
    getline (cin, name);
    cout << "Enter your favourite food" << endl;
    getline (cin, favfood);
    cout << "Awesome! " << favfood << " is my favourite food too" << endl;
    cout << "Enter your age" << endl;
    getline (cin, age);
    cout << "So your name is " << name << ", your fav food is "
         << favfood << " and your age is " << age << endl;
}

int main(int argc, char **argv)
{
    do {
        user_interaction();
    } while (user_says_exit() == false);
    cout << "Quitting now" << endl;
    return 0;
}


Recursive Brute-Force I have to recursively solve the following problem, using a brute force approach:

Suppose two people, A and B, have an even number of ordered boxes, each with a given value. For example, boxes = {5, 3, 7, 10}. They need to split the boxes between them, in this way: person A chooses either the first or the last box in the set, then person B does the same, and so on until there are no boxes left.

Person A wants to know, what's the maximum value he can get, in total, bearing in mind that at each turn person B can make two choices as well. In other words, the problem is to come up with an algorithm that simulates all the choices of both people, considering they're all aiming to have the maximum value in the long term.

So, for now I have this:

public static int maxValue(ArrayList <Integer> boxes, int choice, int person){
    int value;

    //Stop condition - if there are no more boxes, return 0
    if (boxes.isEmpty())
        return 0;

    if (choice == 0) //Person chose the first box in the sequence
        value = boxes.remove(0);
    else //Person chose the last box in the sequence
        value = boxes.remove(boxes.size() - 1);

    //Person A makes a choice, checking which one works best in the long run
    if (person == 1)
        return (value + max(maxValue(boxes, 0, 2), maxValue(boxes, 1, 2)));

    //Person B makes a choice, checking which one works best in the long run
    else
        return (value + max(maxValue(boxes, 0, 1), maxValue(boxes, 1, 1)));
}

For an input of boxes = {5, 3, 7, 10}, the code is supposed to produce 15, yet the one I wrote above gives me 25. After placing some debugging prints, I saw it goes:

Person A chooses '10'
Person B chooses '7'
Person A chooses '3'
Person B chooses '5'

And then just adds all the values. I'm figuring it's because of the way the function is called by person A with reference to person B (in max(maxValue(boxes, 0, 2), maxValue(boxes, 1, 2))), and vice-versa, and also because of that stop condition (if I change it slightly the value returned is different).

if it really has to be brute force solution, then you have to check all possible combinations of take first/take last item for both players

you know that there are 4 items, so every player takes two of them, combinations are (for each player):

take first (0), take first (0)
take first (0), take last  (1)
take last  (1), take first (0)
take last  (1), take last  (1)

as you can see, numbers are similar to binary numbers equal to 0-3(dec)

second player should do the same in an inner loop for every iteration of first player (outer loop)

pseudo code would be:

ArrayList <Integer> boxes = {5, 3, 7, 10}

boolean shouldTakeFirst(int combination, int iteration) {
    return (combination & (1<<iteration)) > 0; // checks if i-th bit of combination is "1"
}

int player1Score(int combinationP1, int combination P2) {
    int score = 0;
    for (int i=0; i<boxes.size()/2; i++) {
        score += shouldTakeFirst(combinationP1, i) ? boxes.popFirst() : boxes.popLast(); // assuming that .popFirst() and .popLast() remove elements after taking them
        shouldTakeFirst(combinationP2, i) ? boxes.first() : boxes.last();
    }
    return score;
}

for (int i=0; i < Math.pow(2, boxes.size()/2); i++) {
    for (int j=0; j < Math.pow(2, boxes.size()/2); j++) {
        int p1Score = player1Score(i, j);
        int p2Score = sum(boxes) - p1Score;

        // check if scores better than max and remember/print it
    }
}


Bash function to generate colored output I like to user colored output in my interactive shell scripts.

I've been using Ubuntu and bash for a long time, but haven't done much bash scripting.

I'd appreciate a review of this function for generating colored output.

#!/usr/bin/env bash
#
# Function to generate colored output.
#
# The output is specified using a format string and arguments, similar to printf.
#
# All special format specifiers are enclosed in parenthesis.
# Colors start with either "f" for foreground or "b" for background with the
# remainder of the specifier being the color name, starting with a capital
# letter (e.g., fRed or bCyan).
#
# There are a few other specifiers in addition to the colors:
# - %s%         Placeholder; consumes the next argument from the command line
# - %u%         Underline
# - %r$         Reverse video
# - %default%   Default foreground and background color
# - %reset%     Reset all attributes
#
# Example:
#   echoc '%fRed%The value "%r%%s%%fRed%" is not valid for the --trace option' extra
# Result:
#   The value "extra" is not valid for the --trace option
#   with everything except the word extra being red on the default background
#   color and the word extra being reverse-video.
#
unset -f echoc
function echoc()
{
  local       char
  local       code
  local       fmt="${1}"
  local       fmtLen=${#fmt}
  local       i=0
  local       output=''
  local       str=''
  local       tagActive=0

  shift       # Remove the format string.

  #
  # The ANSI color and format codes.
  # Keys that start with "f" are foreground colors.
  # Keys that start with "b" are background colors.
  # Other keys are eight resets or other formatting codes.
  # The "r" and "u" are reverse and underscore, respectively.
  #
  declare -A colors
  colors=(['fBlack']='30' ['fBlue']='34' ['fBrown']='33' ['fCyan']='36')
  colors+=(['fDarkgray']='1;30' ['fDefault']='39' ['fGreen']='32' ['fLavender']='1;34')
  colors+=(['fLightblue']='1;36' ['fLightgray']='37' ['fLightgreen']='1;32')
  colors+=(['fLightpurple']='1;35' ['fMagenta']='35' ['fPink']='1;31' ['fPurple']='35')
  colors+=(['fRed']='31' ['fWhite']='1;37' ['fYellow']='1;33')
  colors+=(['bBlack']='40' ['bBlue']='44' ['bBrown']='43' ['bCyan']='46' ['bDefault']='49')
  colors+=(['bGreen']='42' ['bMagenta']='45' ['bPurple']='45' ['bRed']='41')
  colors+=(['default']='39;49' ['reset']='0')
  colors+=(['r']='7' ['u']='4')

  #
  # Parse the format string, building the output string as we go.
  #
  while [ $i -lt $fmtLen ]
  do
    char="${fmt:i:1}"
    case $char in
      %)
        if [ 0 -eq $tagActive ]
        then
          if [ -n "${str}" ]
          then
            output="${output}${str}"
            str=''
          fi
          tagActive=1
        else # tagActive is true.
          if [ -z "${str}" ]
          then
            output="${output}%" # %% inserts a percent sign.
          else
            if [ 's' == "${str}" ]
            then
              output="${output}${1}\033[0m"
              shift
            else
              code="${colors[$str]}"
              if [ -z "${code}" ]
              then
                echo -e "\033[31mInvalid color name \"${str}\"\033[0m"
              else
                output="${output}\033[${code}m"
              fi
            fi
            str=''
          fi
          tagActive=0
        fi
        ;;

      *)
        str="${str}${char}"
        ;;
    esac
    ((i++))
  done
  if [ -n "${str}" ]
  then
    output="${output}${str}"
  fi
  output="${output}\033[0m"
  echo -e "${output}"

}
export -f echoc

Three things strike me immediately:

You are treating foreground and background colors equally, and giving preference to reverse video and underline(?). Why is red foreground %fRed% while reverse video is %r%? Aren't you more likely to use red,yellow,green foreground than reverse video? Aren't you more likely to use foreground colors than background colors?

I think you should give some thought to the "Huffmanization" of your color names. Maybe you need %r% %g% %y% as shortcuts for foreground red, green, and yellow. (Or, maybe not. It depends on what your use case is going to be. If you're writing a rogue-like in bash, maybe you have a different color vocabulary...)

I would suggest that foreground colors just be color names, while background colors have a bg prefix:  %cyan% versus %bgCyan% for example. This makes background colors more expensive than they currently are (1 extra character) but makes foreground colors less expensive, and I suspect they'll be more common.
Next, I think you're doing too much work inside your loop. You are actually processing the format string character-by-character! Given that the beginning delimiter and ending delimiter are the same - % - you could use IFS to split the string into an array and process the segments of the array. Given a string like "before % during % after", or even "%during%", you know that the alternate members of the array are going to be tags: even indices text, odd indices tag.

See this answer on SO for how to split a string.

This would make change your loop from processing characters into processing "strings", and should speed up your function.
Finally, there are a lot of "magic strings" in your code. Like magic numbers, you should replace these with constants (variables) and reference them just once. In particular, I'm thinking of ${pfx} ("\033[") and maybe ${reset} ("${pfx}0m").


GetNeighborhood function for 3D cells structure in MATLAB I am attempting to implement a GetNeighborhood function in order to get a specific region from inputCells by sizeInput and centralLocation parameters in 3D cells structure.
The experimental implementation
The experimental implementation of GetNeighborhood function is as below.
function [output] = GetNeighborhood(inputCells, sizeInput, centralLocation)
    %   Get neighborhood of fixed size and centered at centralLocation
    output = cell( sizeInput ,sizeInput ,sizeInput );
    X = centralLocation(1);
    Y = centralLocation(2);
    Z = centralLocation(3);
    for x = -sizeInput:sizeInput
        for y = -sizeInput:sizeInput
            for z = -sizeInput:sizeInput
                if X + x < 1
                    xLocation = 1;
                elseif X + x > size(inputCells, 1)
                    xLocation = size(inputCells, 1);
                else
                    xLocation = X + x;
                end

                if Y + y < 1
                    yLocation = 1;
                elseif Y + y > size(inputCells, 2)
                    yLocation = size(inputCells, 2);
                else
                    yLocation = Y + y;
                end

                if Z + z < 1
                    zLocation = 1;
                elseif Z + z > size(inputCells, 3)
                    zLocation = size(inputCells, 3);
                else
                    zLocation = Z + z;
                end

                output{sizeInput + x + 1, sizeInput + y + 1, sizeInput + z + 1} = ...
                    inputCells{xLocation, yLocation, zLocation};
            end
        end
    end
end

Test cases
For testing purpose, a simple test script is created as below.
clear all;
clc;
close all;
set(groot,'defaultFigureVisible','on');

%% Create test cells

testCellsSize = 10;

test = cell(testCellsSize, testCellsSize, testCellsSize);
for x = 1:size(test, 1)
    for y = 1:size(test, 2)
        for z = 1:size(test, 3)
            test{x, y, z} = [x * 100 + y * 10 + z];
        end
    end
end

%% Specify test parameters

NeighborhoodDist = 1;
centralLocation = [5 2 2];

%% Perform test

result = GetNeighborhood(test, NeighborhoodDist, centralLocation);
result

The output of the above testing code:

  3×3×3 cell array

result(:,:,1) =

    {[411]}    {[421]}    {[431]}
    {[511]}    {[521]}    {[531]}
    {[611]}    {[621]}    {[631]}

result(:,:,2) =

    {[412]}    {[422]}    {[432]}
    {[512]}    {[522]}    {[532]}
    {[612]}    {[622]}    {[632]}

result(:,:,3) =

    {[413]}    {[423]}    {[433]}
    {[513]}    {[523]}    {[533]}
    {[613]}    {[623]}    {[633]}

If there is any possible improvement about potential drawback or unnecessary overhead, please let me know.

I think the biggest gain in efficiency you can get is to  not use a cell array. Each element of a cell array is an array. Each array has a header data structure that takes up 104 bytes (if I remember that number correctly). Storing only one number in an array means that there is a memory overhead of 104/(104+8)=92%. Access is also slower because of the double indirection. To store individual numbers you should always use a numeric array.
This code:
if X + x < 1
   xLocation = 1;
elseif X + x > size(inputCells, 1)
   xLocation = size(inputCells, 1);
else
   xLocation = X + x;
end

has quite some redundancy. Every time you write the same expression twice, you should think about storing the result of the expression. Even if the computation is trivial and likely not to slow down your code, duplicated code still leads to maintenance issues. You could do instead:
xLocation = X + x;
maxLocation = size(inputCells, 1);
if xLocation < 1
   xLocation = 1;
elseif xLocation > maxLocation
   xLocation = maxLocation
end

This can be further simplified using max and min:
xLocation = X + x;
xLocation = max(xLocation, 1);
xLocation = min(xLocation, size(inputCells, 1));

You could maybe even simplify this to a single line of code, though that doesn’t improve readability:
xLocation = min(max(X + x, 1), size(inputCells, 1));

Furthermore, this code is repeated three times, so you should make it into a function you can call three times.
Once your input and output are numeric arrays, you can likely vectorize the operation, removing the loops and further simplifying the code.
clear all is not recommended. clear by itself clears the workspace, deleting all defined variables. Adding the all argument causes it to additionally remove from memory all parsed and pre-compiled functions, meaning that the next time you call those functions they’ll take longer to run. clear all is hardly ever needed, and should not be a part of your normal workflow.


Find the duplicate in a sorted array In less-than O(n) Assumptions:

The array is sorted.
There is only one duplicate.
The array is only populated with numbers [0, n], where n is the length of the array.

Example array: [0,1,2,3,4,5,6,7,8,8,9]

Here is my implementation:

public static int DuplicateBinaryFind(int[] arr, int left, int right)
{
   int dup =0;

   if(left==right)
   {
      dup = left;
   }
   else
   {
        int middle = (left+right)/2;
        if(arr[middle]<middle)
        {
          dup = DuplicateBinaryFind(arr,left, middle-1);

        }
        else
        {
           dup = DuplicateBinaryFind(arr, middle+1, right);
        }
   }

   return dup;

}

Is this considered as good solution to the problem as it finds the duplicate in \$O(\log n)\$?

Don't use (left + right) / 2 in a binary search, as (left + right) is vulnerable to overflow.  Java's binary search implementation had such a bug for some time.

Instead, do

int middle = left + (right - left) / 2;

Alternatively, you could do

int middle = (int)(unchecked((uint)left + (uint)right) >> 1);


Basic calculator that takes 2 numbers and does an operation with them The code works perfectly fine for me (compiled it in BlueJ and Eclipse) but I was wondering what other more experienced programmers thought of it. I'd specifically like to know if it could be shorter or be made more resource efficient.

    /**
* This program is just a simple addition calculator that can use whole numbers or decimals.
*
* @author Christopher Goodburn
* @version 6/4/2016
*/
import java.util.Scanner;
import java.math.*;

public class Calculator
{
private static final Scanner askScanner = new Scanner(System.in);
public static double answer;
public static double firstNumber;
public static double secondNumber; //makes variables for the whole class

public static void main(String[] args) {
    calculator();
}

    public static void calculator() {

    System.out.println("Basic calculator");
    System.out.println("Pick one:");
    System.out.println("1. Addition");
    System.out.println("2. Subtraction");
    System.out.println("3. Multiplication");
    System.out.println("4. Division");
    int pick = askScanner.nextInt();

    if(pick == 1) {
        addition();
    }
    else if(pick == 2) {
        subtraction();
    }
    else if(pick == 3) {
        multiplication();
    }
    else if(pick == 4) {
        division();
    }
    else {
        System.out.println("You need to choose 1, 2, 3, or 4");
        calculator();
    }
    askScanner.close();
}

    private static double getNumbers() {
        System.out.println("First number?");
        firstNumber = askScanner.nextDouble();
        System.out.println("Second Number?");
        secondNumber = askScanner.nextDouble();
        return firstNumber;
    }

    public static void subtraction() {
        System.out.println("Subtraction");
        getNumbers();
        answer = firstNumber - secondNumber;
        System.out.println("This is the difference of the two numbers:  " + answer);
        calculator();
    }

    public static void addition() {
       System.out.println("Addition");
       getNumbers();
       answer = firstNumber + secondNumber;
       System.out.println("This is the sum of the two numbers:  " + answer);
       calculator();
    }

    public static void multiplication() {
        System.out.println("Multiplication");
        getNumbers();
        answer = firstNumber * secondNumber;
        System.out.println("This is the product of the two numbers  " + answer);
        calculator();
    }

    public static void division() {
        System.out.println("Division");
        getNumbers();
        answer = firstNumber / secondNumber;
        System.out.println("This is the quotient of the two numbers:    " + answer);
        calculator();
    }

}

The main issue with your code is repetition. The 4 methods calculating the result based on user input are sketched in the same way:

public static void operation() {
    // get numbers
    // perform operation and print result
    // return to main loop
}

As a result, there is copy-pasted code inside those 4 methods.

The first remark is that all of them will need to get numbers from the user. Instead of having each of addition, subtraction, etc. to get the numbers, retrieve them before hand from the user. In the same way, all of them invoke calculator(); to ask the user numbers again.

A first possiblity is therefore to have:

int pick = askScanner.nextInt();

getNumbers();
// perform operation based on input
calculator();

with a sample operation being:

public static void addition() {
    System.out.println("Addition");
    answer = firstNumber + secondNumber;
    System.out.println("This is the sum of the two numbers:  " + answer);
}

That said, this still leaves a design issue:

private static final Scanner askScanner = new Scanner(System.in);
public static double answer;
public static double firstNumber;
public static double secondNumber; //makes variables for the whole class

Having static global variables to keep the numbers input by the user isn't a good idea. What you want instead is to have local variables with the minimum possible scope.

You'll need to drop the method getNumbers() completely: it needs to have global variables (because it only returns one value and not the two). Also, the operation methods won't operate on the global variables anymore; instead, they will be given the two operands as parameter. And, lastly, they won't set the result global variable but return the result.

As such, the code would be:

if (pick == 1) {
    addition(firstNumber, secondNumber);
} else if (pick == 2) {
    subtraction(firstNumber, secondNumber);
} else if (pick == 3) {
    multiplication(firstNumber, secondNumber);
} else if (pick == 4) {
    division(firstNumber, secondNumber);
} else {
    System.out.println("You need to choose 1, 2, 3, or 4");
}

with a sample operation being:

public static double addition(double firstNumber, double secondNumber) {
    System.out.println("Addition");
    double answer = firstNumber + secondNumber;
    System.out.println("This is the sum of the two numbers:  " + answer);
    return answer;
}

You could also consider using a switch instead of the if/else blocks:

switch (pick) {
case 1: addition(firstNumber, secondNumber);
case 2: subtraction(firstNumber, secondNumber);
case 3: multiplication(firstNumber, secondNumber);
case 4: division(firstNumber, secondNumber);
default: System.out.println("You need to choose 1, 2, 3, or 4");
}

which makes the code a bit shorter.

As a final note: your code is an infinite loop right now, the user has no way to exit. It would be nice to introduce an option giving the user the possibility to quit the program.


The fastest "find"er of them all? For part of a project, I was given this directive:

  The find command will use the directory (dirent) function calls, but
  will require you to start at a directory and then determine if a file
  is a directory or a file and if it is a directory you will need to
  walk down that directory as well. You will need a method/algorithm to
  note the new directories and build a logical graph or thread out every
  directory to find if the filename is in that directory and display the
  "absolute" path to that file.

I thought the nftw() function would be optimal for this, and I believe it makes my application pretty robust and fast.  On my Mac (ommiting 196 lines of filename output each):

$ time find ../../ -name "CMakeLists.txt"
real    0m0.337s
user    0m0.025s
sys 0m0.292s

$ time ./my_find ../../ -name "CMakeLists.txt"
real    0m0.326s
user    0m0.022s
sys 0m0.278s

find.c:

#define _XOPEN_SOURCE 1
#define _XOPEN_SOURCE_EXTENDED 1

#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/types.h>
#include <dirent.h>
#include <ftw.h>
#include <unistd.h>

#ifndef streq
#define streq(x, y) (strcmp((x), (y)) == 0)
#endif

typedef enum {SUCCESS, INVALD_DIR, OPENDIR_ERR, NFTW_ERR} Return;

const char* str = NULL;

int check_path(const char* path)
{
    DIR* dir = opendir(path);
    if (dir)  // directory exists
    {
        closedir(dir);
        return 0;
    }
    else if (ENOENT == errno) // directory doesn't exist
        return INVALD_DIR;
    else // opendir failed for some other reason
        return OPENDIR_ERR;
}

int callback(const char *file, const struct stat *sb, int flag, struct FTW *s)
{
    int err = 0;
    const char *name = file + s->base;

    switch (flag)
    {
        case FTW_F:
            if (streq(name, str))
                puts(file);
            break;
        case FTW_D:
        case FTW_DNR:
        case FTW_SL:
        case FTW_NS:
            break; // continue as normal
        case FTW_DP:
        case FTW_SLN:
        default:
            err = NFTW_ERR; // error occured
            break;
    }

    return err;
}

int process(const char* path)
{
    int err = nftw(path, callback, getdtablesize(), 0);
    return err;
}

void usage(const char* prog_name)
{
    fprintf(stdout, "usage: %s [path] -name <name>\n", prog_name);
    exit(EXIT_SUCCESS);
}

int main(int argc, char **argv)
{
    if(argc < 4)
        usage(argv[0]);

    const char* path = argv[1];
    str = argv[3];

    int err = check_path(path);
    if (err)
    {
        fprintf(stderr, "%s: %s: %s\n", argv[0], path, strerror(errno));
        return err;
    }
    else
        process(path);

    return err;
}

Any suggestions?

I like your solution, it's clever & fast, however I can't help but feel like it goes against the spirit of the assignment.  You've found a built in system call that greatly simplifies what it is your need to do, which is great, but in doing so you've delegated the algorithm, which may not be what is expected from the assignment.

The assignment reads as if it's expecting your to use things like opendir and readdir to iterate through the folders yourself.  You would then be responsible for maintaining state, and writing the iteration algorithm.

-name or not -name

Your argument handling is also a little bit on the naive side.  Should I really be able to run it like this:

./my_find ../../ -ThisArgumentIsNeverCheckedSoICanPutPrettyMuchAnything "CMakeLists.txt"

Why both having the second argument if you're not going to check it, it's just extra typing...

Wildcards

It's not really clear from the spec, however the built in 'find' command supports wildcards.  Your current implementation doesn't appear to (you're only doing an exact match).