pancake sorting using python pancake sorting is implemented below. Algorithm is below:

Find the largest out of order pancake and flip it to the bottom (you may need to flip it to the top of the stack first).Repeat step one until the stack is ordered.

from random import randint

def pancake_sort(cakes):
  if not cakes or len(cakes) == 1:
        return cakes
  cake_index = 0
  while cake_index < len(cakes)-1:
    prev_cake_size, current_cake_size = cakes[cake_index], cakes[cake_index+1]
    if prev_cake_size > current_cake_size:
      '''move the highest element to the beginning by reversing
      the cakes from 0 to highest element(including) and then move
      the highest element to the end by reversing the subarray'''
      cakes[0:cake_index+2] = (cakes[0:cake_index+1][::-1] + [cakes[cake_index+1]])[::-1]
      cake_index = 0
      continue
    cake_index += 1
  return cakes

for i in range(randint(0, 5)):
  data = [randint(0, 100) for _ in range(randint(0, 40))]
  if pancake_sort(data[:]) != sorted(data[:]):
    print("failed for", data[:])

Naming

The name cake_index is very long and makes things hard to read - maybe i would be enough without making things confusing.

The names  prev_cake_size and current_cake_size are very misleading are we are not really handling sizes.

More generally, I think we could use more generic terms. My preference is to use the following names:

cakes becomes lst
prev_cake_size and current_cake_size become prev_elt and curr_elt
cake_index becomes i

Style

A few things are unusual in your code regarding style. I suggest reading PEP 8, the style guide for Python code.

Among the things I'd change:

use the # comment syntax for comments (instead of literal strings)
use 4-space indentation

Also, it may be clearer to replace your continue by a simple else to make the 2 different situations more explicit.

Useless code

The special case for small lists (size equal to 0 or 1 is not required).

Flipping pancake

The code used to flip pancakes is very complicated and does not really show the kind of operation we perform on your pancake stack.
I'd rewrite this:

        lst[:i+1] = reversed(lst[:i+1])
        lst[:i+2] = reversed(lst[:i+2])

Then, the code would look like:

def pancake_sort(lst):
    leng = len(lst)
    i = 0
    while i < leng-1:
        prev_elt, curr_elt = lst[i], lst[i+1]
        if prev_elt > curr_elt:
            # move the highest element to the beginning by reversing
            # the list from 0 to highest element(including) and then move
            # the highest element to the end by reversing the subarray
            lst[:i+1] = reversed(lst[:i+1])
            lst[:i+2] = reversed(lst[:i+2])
            i = 0
        else:
            i += 1
    return lst

Tests

Your tests are complicated because of the amound of calls to random, in particular testing a random number of times.

I'd also suggest using a real testing framework.

It would be a nice youch to also test explicitly edge cases : empty list, list with one element, list with the same elements multiple times.

Finally, instead of calling randint multiple times, you could use random.shuffle(range(50)).


Computing divisibility and multiplication Please check my Java program to see if I use the global variables, nested loops and two functions with parameters correctly. I would also like any suggestions to improve my program.

import java.util.*;
public class javaProgram {

    public static int loopCount = 0;
    static Scanner scanner = new Scanner(System.in);

    public static void div(int n) {
        for (int i = 1; i <= n; i++) {
                System.out.print("The number " + i + " is divisible by:");
                for (int j = 1; j <= i; j++) {
                    if ((i % j) == 0)
                        System.out.print(" " + j);
                    loopCount++;
                }
                System.out.println();
            }
       }

     private static void mul(int n) {
                System.out.println();
                System.out.println("Multiplication table for "+n);
                System.out.println("---------------------------");
                for(int i = 1; i<=10;i++) {
                    System.out.format("%2d x %d = %3d\n", i,n,i*n);
                    loopCount++;
                }
     }

     public static void main(String[] args) {
            System.out.println("Let's compute for divisible and multiplication!");
            System.out.print("Enter number of integers: ");
            int number = scanner.nextInt();
            div(number);
            mul(number);
            System.out.println();
            System.out.println("Loop count " + loopCount);
        }

}

Review

  see if I use the global variables, nested loops and two functions with parameters correctly

loopCount is a magic variable, its meaning beats me. Is it some kind of debugging assistant?
The indentations seem random picks between 4 and 8 spaces. Stick to 4.
The div method is public, the mul private. Why introduce this inconsistency?
Give full names to your methods for readabiliy divide, multiply.
The methods are void, printing the results to the console. This limits usability to zero. Separate interaction with the console from the method flow. Use return values to yield results. Let main log these results to the console.
As suggested in the comments, you can optimize division by probing the smallest prime divisor to minimize cycles in the loops.
Be a bit more consistent in aligning brackets with method declarations.
For readability and consistency, add a blank line after the imports and remove that blank line after the last method.


Is this a decent way of writing a start-stop-daemon script? This is the first time I've written a start-stop-daemon script, so I would love some feedback. :) The goal is to have it start late, when the network and everything is ready. I also have 2 other start-stop-daemons that depend on this once being started before they launch and are nearly identical.

#! /bin/sh
# chkconfig 345 85 60
# description: startup script for foo
# processname: foo

NAME=foo
DIR=/etc/foo/services
EXEC=foo.py
PID_FILE=/var/run/foo.pid
IEXE=/etc/init.d/foo
RUN_AS=root

### BEGIN INIT INFO
# Provides:          foo
# Required-Start:    $remote_fs $syslog
# Required-Stop:     $remote_fs $syslog
# Default-Start:     5
# Default-Stop:      0 1 2 3 6
# Description:       Starts the foo service
### END INIT INFO

if [ ! -f $DIR/$EXEC ]
then
        echo "$DIR/$EXEC not found."
        exit
fi

case "$1" in
  start)
        echo -n "Starting $NAME"
    cd $DIR
    start-stop-daemon -d $DIR --start --background --pidfile $PID_FILE --make-pidfile --exec $EXEC --quiet
        echo "$NAME are now running."
        ;;
  stop)
    echo -n "Stopping $NAME"
        kill -TERM `cat $PID_FILE`
    rm $PID_FILE
        echo "$NAME."
        ;;
  force-reload|restart)
        $0 stop
        $0 start
        ;;
  *)
        echo "Use: /etc/init.d/$NAME {start|stop|restart|force-reload}"
        exit 1
    ;;
esac
exit 0

Probably, you also may implement condrestart:

  The condrestart (conditional restart) option only starts the daemon if it is currently running.
  This option is useful for scripts, because it does not start the daemon if it is not running.

Quote from question about condrestart at LinuxQuestions


Given an array of Strings , check if they are anagrams are not Two strings are anagrams if they are permutations of each other. Assuming we have an ASCII alphabet we can take 128 integer array to count the characters in the first string. Then decrement the count for the corresponding character in the second string. If all the character counts are zero then the strings are anagrams else they are not.

public class anagrams {
        public static boolean anagramsHelper(String[] words)
        {
            for(int i = 1 ; i < words.length ; i++)
            {
                if(!areAnagrams(words[0], words[i]))
                {
                    return false;
                }
            }
            return true;
        }
        public static boolean areAnagrams(String word1, String word2)
        {
            if(word1.length() != word2.length())
            {
                return false; // anagrams are strings with same length
            }
            int[] charCount = new int[128]; // 128 unique chars in ASCII
            //count the chars in word1
            for(int i = 0 ; i < word1.length() ; i++)
            {
                charCount[(int)word1.charAt(i)]++;
            }
            //decrement the char count for chars in word2
            for(int i = 0 ; i < word2.length() ; i++)
            {
                if(charCount[(int)word2.charAt(i)] == 0)
                {
                    return false;
                }
                charCount[(int)word2.charAt(i)]--;
            }
            //verify if any char count is non zero
            //for anagrams it should be zero else they are not anagrams
            for(int i  = 0 ; i < 128 ; i++)
            {
                if(charCount[i] != 0)
                {
                    return false;
                }
            }
            return true;
        }
        public static void main(String[] args) {
            String[] words = {"abc", "bca", "cab" , "myd"};
            System.out.println(anagramsHelper(words));
        }
    }

1

class anagrams: in Java, class names must begin with a capital letter.

2

You restrict yourself to ASCII. However, you could use a HashMap<Character, Integer>. Of course, it implies larger constant factor, yet it matches the linear time complexity of your implementation.

Summa summarum

I thought about this:

public static boolean areAnagrams(String word1, String word2) {
    if (word1.length() != word2.length()) {
        return false;
    }

    Map<Character, Integer> map = new HashMap<>();

    for (char c : word1.toCharArray()) {
        map.put(c, map.getOrDefault(c, 0) + 1);
    }

    for (char c : word2.toCharArray()) {
        int count = map.getOrDefault(c, 0) - 1;

        if (count < 0) {
            return false;
        }

        map.put(c, count);
    }

    return true;
}

Hope that helps.


Decoding 32-bit MIPS instructions For a homework assignment I've been given the task of parsing out information from an 32-bit MIPS instruction. (For more information on the instruction formats, see here). The instructor has provided us with a header file enumerating all of the functions required, and it's my job to implement them.

Currently, this is what the implementation for one of my functions looks like:

void instruction_partition(unsigned instruction, unsigned *op, unsigned *r1, unsigned *r2, unsigned *r3, unsigned *funct, unsigned *offset, unsigned *jsec)
{
    assert(op && r1 && r2 && r3 && funct && offset && jsec);

    *op = (instruction >> 26) & 0x3F;
    *r1 = (instruction >> 21) & 0x1F;
    *r2 = (instruction >> 16) & 0x1F;
    *r3 = (instruction >> 11) & 0x1F;
    *offset = (instruction >> 6) & 0x1F;
    *funct = instruction & 0x3F;
    *jsec = instruction & 0x3FFFFFF;
}

In a scenario such as this, I'd almost be tempted to hard-code the shift values and bitmasks, despite it being ingrained within me to avoid doing so at all costs.

I tried a few other configurations, but neither resonated with me. Any thoughts?

void instruction_partition(unsigned instruction, unsigned *op, unsigned *r1, unsigned *r2, unsigned *r3, unsigned *funct, unsigned *offset, unsigned *jsec)
{
    assert(op && r1 && r2 && r3 && funct && offset && jsec);

    struct { unsigned offset; unsigned bitMask; }
        opMaskFormat = { 26, 0x3F },
        r1MaskFormat = { 21, 0x1F },
        r2MaskFormat = { 16, 0x1F },
        r3MaskFormat = { 11, 0x1F },
        functMaskFormat = { 6, 0x1F },
        offsetMaskFormat = { 0, 0x3F },
        jsecMaskFormat = { 0, 0x3FFFFFF };

    *op = (instruction >> opMaskFormat.offset) & opMaskFormat.bitMask;
    *r1 = (instruction >> r1MaskFormat.offset) & opMaskFormat.bitMask;
    *r2 = (instruction >> r2MaskFormat.offset) & r2MaskFormat.bitMask;
    *r3 = (instruction >> r3MaskFormat.offset) & r3MaskFormat.bitMask;
    *offset = (instruction >> offsetMaskFormat.offset) & offsetMaskFormat.bitMask;
    *funct = (instruction >> functMaskFormat.shift) & functMaskFormat.bitMask;
    *jsec = (instruction >> jsecMaskFormat.offset) & jsecMaskFormat.bitMask;
}

void instruction_partition(unsigned instruction, unsigned *op, unsigned *r1, unsigned *r2, unsigned *r3, unsigned *funct, unsigned *offset, unsigned *jsec)
{
    assert(op && r1 && r2 && r3 && funct && offset && jsec);

    unsigned SIX_BIT_MASK = 0x3F,
        FIVE_BIT_MASK = 0x1F,
        TWENTY_SIX_BIT_MASK = 0x3FFFFFF;

    *op = (instruction >> 26) & SIX_BIT_MASK;
    *r1 = (instruction >> 21) & FIVE_BIT_MASK;
    *r2 = (instruction >> 16) & FIVE_BIT_MASK;
    *r3 = (instruction >> 11) & FIVE_BIT_MASK;
    *offset = (instruction >> 6) & FIVE_BIT_MASK;
    *funct = instruction & SIX_BIT_MASK;
    *jsec = instruction & TWENTY_SIX_BIT_MASK;
}

Since the instruction is a 32-bit number, you should use uint32_t, rather than assuming that unsigned is 32 bits on the host.

The first version is fine, I think, because it has the least clutter.  An experienced programmer will have no problem figuring out what it does.  If you are concerned about the mental arithmetic required to figure out the width of the bitmasks, you could write it like this:

void instruction_partition(uint32_t instruction, unsigned *op, unsigned *r1, unsigned *r2, unsigned *r3, unsigned *funct, unsigned *offset, unsigned *jsec)
{
    assert(op && r1 && r2 && r3 && funct && offset && jsec);

    *op     = (instruction >> 26) & ((1 << 6) - 1); /* 6 most significant bits */
    *r1     = (instruction >> 21) & ((1 << 5) - 1);
    *r2     = (instruction >> 16) & ((1 << 5) - 1);
    *r3     = (instruction >> 11) & ((1 << 5) - 1);
    *offset = (instruction >>  6) & ((1 << 5) - 1);
    *funct  =  instruction        & ((1 << 6) - 1); /* 6 least significant bits */
    *jsec   =  instruction        & ((1 << 26) - 1);
}

This should compile to identical code as your first implementation.  Technically, the opcode doesn't need to be masked:

    *op = instruction >> 26;


Preventing XSS in Input $_POST Form I tried to prevent XSS attacks by adding

// prevent XSS
$_GET   = filter_input_array(INPUT_GET, FILTER_SANITIZE_STRING);
$_POST  = filter_input_array(INPUT_POST, FILTER_SANITIZE_STRING);

to the top of the .PHP File. It works, but I don't think that this is the best practice.

What I need to do is improve the way this code handles the data:

    if (empty($_POST['telefonnummer']) || !isset($_POST['telefonnummer'])) {
        $this->setStatusMessage($translator->translate('Bitte geben Sie Ihre Telefonnummer ein.'));
        $this->setStatus(KDatabase::STATUS_FAILED);

        return false;
    }else{
      $this->telefonnummer = $_POST['telefonnummer'];
    }

Find purpose-built libraries which are actively maintained. For example, anti-xss:

$harm_string = "Hello, i try to  your site";
$harmless_string = $antiXss->xss_clean($harm_string);

$harm_string = "";
$harmless_string = $antiXss->xss_clean($harm_string);

$harm_string = "CLICK";
$harmless_string = $antiXss->xss_clean($harm_string);

$harm_string = "CLICK";
$harmless_string = $antiXss->xss_clean($harm_string);

$harm_string = '
';
$harmless_string = $antiXss->xss_clean($harm_string);

$harm_string = '
';
$antiXss->removeEvilAttributes(array('style')); // allow style-attributes
$harmless_string = $antiXss->xss_clean($harm_string);

$harm_string = "\x3cscript src=http://www.example.com/malicious-code.js\x3e\x3c/script\x3e";
$harmless_string = $antiXss->xss_clean($harm_string);

$harm_string = "";

$antiXss->removeEvilHtmlTags(array('iframe'));

And a UTF8 library for data you are sending:

UTF8::json_enocde(array(1, '¥', 'ä')); // '[1,"\u00a5","\u00e4"]'

UTF8::html_encode('中文空白'); // '&#20013;&#25991;&#31354;&#30333;'

UTF8::utf8_decode(UTF8::utf8_encode('-ABC-中文空白-')); // '-ABC-中文空白-'

Implement Content Security Policy(CSP) on the web server as well. For example:

default-src 'self' https:; connect-src 'self' https: http:; font-src 'self' https:; frame-src *; img-src
 'self' https: http: data:; media-src 'self' https:; object-src 'self' https:; script-src 'sha256-q590j1fW
+aERb666H10h55ePy0sxRjUYCiOmJPftXDs=' 'self' https: 'unsafe-eval' 'unsafe-inline' http:; style-src 'self'
 https: 'unsafe-inline' http:; report-uri /tracking/csp?action=listing_frame&controller=embed&req_uuid
=cff37d5d-4c12-4c8b-b288-1ce0d103a25c&version=c7fc601874a5350c79eceb33ba6d4c09a433035f;

  default-src is set to 'self' which means it's setting all CSP rules to only allow src attribute from a same-origin. In short, you should only be able to load src from a relative endpoint.
  frame-src is set to wildcard (*) so we can load external src links in frames (iframe, frame, frameset). Because we're injecting HTML past the body element, we cannot use frame or frameset. The WAF has made it next to impossible to use iframe.
  script-src has 'self' supplied after the sha256 hashed script for unsafe-inline and unsafe-eval, but https does not have 'self' supplied meaning we can load external scripts for execution.

References

anti-xss repo: public function testXssClean()
PayloadsAllTheThings/XSS injection at master · swisskyrepo/PayloadsAllTheThings
UTF8 can be tricky – especially with PHP – DerEuroMark
voku/portable-utf8:  Portable UTF-8 library - performance optimized (unicode) string functions for php.
Airbnb – When Bypassing JSON Encoding, XSS Filter, WAF, CSP, and Auditor turns into Eight Vulnerabilities | Brett Buerhaus
How to protect PHP application from XSS attacks: CSP 3 nonce | PHP & Symfony Tips


Getting a serial number from shared table I have a table that keeps track of a global serial number shared between several tables. This is not the PK or unique id for each table. It is like a case identifier. Every time users create a new entity we need to give them a unique serial number from this row. My worries is what happens if 2 users hit the method for incrementing this counter at the exact same time. Would this solution be enough?

Also this code would be run from 2 servers using the same database.

public string GetIdentifier()
{
    int retries = 0;
    bool success = false;

    string newId;

    do
    {
        try
        {
            var tracker = Trackers.Find(1);
            newId = tracker.Increment();
            SaveChanges();
            success = true;

        }
        catch (DbUpdateConcurrencyException)
        {
            Thread.Sleep(500);
            if (++retries == 5)
                throw;
        }
        catch (Exception)
        {
            //do some logging
            throw;
        }

    } while (!success);

    return newId;
}

You propably won't ever get the effect that 2 people create the same unique identifier at the same time. Still, it could possibly happen, most databases will handle this. If you create your db with unique values only in your table, it will automatically give you an error when 2 values are identic.


Update column based on input variable in stored procedure The purpose of the below code is to update a column based on the name of the field name sent from the .NET code. This allows one piece of code to handle multiple rows when a user is only adding/updating one at a time.

I have been using Stored Procedures for a while but normally just Add/Update but not using variable field names. All tips appreciated.

USE DB
GO
/****** Object:  StoredProcedure [dbo].[spActionUpdateOldestDate]    Script Date: 04/02/2014 14:24:09 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
ALTER PROCEDURE [dbo].[spUpdateViaFieldName]

-- spActionUpdateOldestDate '1234','date','field'
-- Add the parameters for the stored procedure here
@AlphaNumbericalRef nvarchar(50)
,@vValue nvarchar(MAX)
 ,@vFieldName varchar(MAX)

AS
BEGIN

-- add selection for courseID etc.. here
Execute ('UPDATE [TblActionsOldest] SET ' + @vFieldName + ' = ''' + @vValue + ''' WHERE RefID = ''' + @AlphaNumbericalRef+ '''')

END

I think your stored procedure looks great!
The only line that had me scratching my head is this one:
Execute ('UPDATE [TblActionsOldest] SET ' + @vFieldName + ' = ''' + @vValue + ''' WHERE RefID = ''' + @AlphaNumbericalRef+ '''')

I'm guessing those apostrophes are escape characters for asp.net but if that's not the case I would discard them.
Nice work!


Project Euler Question #2: Sum of even Fibonacci numbers under 4 million The question is:

  Each new term in the Fibonacci sequence is generated by adding the previous two terms.
    By starting with 1 and 2, the first 10 terms will be:

  1, 2, 3, 5, 8, 13, 21, 34, 55, 89, ...

  By considering the terms in the Fibonacci sequence whose values do not exceed four
   million, find the sum of the even-valued terms.

#Problem 2
P2 = 0
fib= 0
f1 = 1
f2 = 0
debugP2 = []
while fib < 4000000:
    fib = f1 + f2
    f2 = f1
    f1 = fib
    if fib % 2 == 0:
        P2 += fib
        debugP2.append(fib)
print(debugP2)
print(P2)

This script can

Give me all Fibonacci numbers up to 4000000
Give the sum of all even numbers up to 4000000
It satisfies Project Euler Question #2.

Is there a way to make this shorter or more efficient?

Your current implementation, and the suggested improvements are all brute force implementations, i.e. enumerating all fibonacci numbers, and they will all run in O(n).

By using some mathematical tricks, you can turn this into an O(1) implementation. Since this is a project Euler problem, I won't spell out the answer, but here are some pointers:

When starting at F(0) = 1 (instead of starting at F(1) as in the problem description), every third number is an even fibonacci number
Because the fibonacci numbers are by definition based on the addition of the previous two numbers, the sum of all even fibonacci numbers up to n is equal to the sum of all fibonacci numbers up to n divided by two.
There are cool formula's to calculate the sum of fibonacci numbers and the index of the highest fibonacci number up to n. Refer to wolframalpha or wikipedia to find them.


Polymorphic event notification model in Rails I have a polymorphic notification model. When user gets a notification he can get on the page of the notifiable to check  out what happened exactly. For instance invited you to join product or commented on a post. On the top of this I also have a polymorphic comment model. So user can get a notification not only for commented on a post but also for commented on a product customeror commented on a product, which of course just makes the whole situation more complex.

So the flow is the  following:

If the user clicks on the notification, the link redirects to the custom checking_decreasing action to check the notification and decrease the notification number if necessary, and then it redirects to the desired page with anchor.

As you see my notification_redirection_path action looks terrible and I'm not even sure if it should be in application controller. Any ideas how I could refactor this code?

notifications index page (there is also a json version for dropdown notifications)

<%= link_to checking_decreasing_user_notifications_path(
      current_user,
      notifiable_type: notification.notifiable_type,
      notifiable_id: notification.notifiable_id,
      notification_action: notification.action
     ) do %>

notifications_controller

def checking_decreasing
  current_user.decreasing_comment_notification_number(
    params[:notifiable_type],
    params[:notifiable_id]
  )
  redirect_to notification_redirection_path(
    params[:notifiable_type],
    params[:notifiable_id],
    params[:notification_action]
  )
end

application controller

def notification_redirection_path(notifiable_type, notifiable_id, action)
  if action == "commented"
    if notifiable_type == "ProductCustomer"
      product_customer = ProductCustomer.find(notifiable_id)
      product_id = product_customer.product_id
    elsif notifiable_type == "ProductLead"
      product_lead = ProductLead.find(notifiable_id)
      product_id = product_lead.product_id
    end
    route = case notifiable_type
            when "Post"
              posts_path(anchor: "post_#{notifiable_id}")#{}"/posts#post_#{notifiable_id}"
            when "Product"
              product_path(notifiable_id, anchor: "comment-panel")#/products/#{notifiable_id}#comment-panel"
            when "ProductLead"
              product_product_lead_path(product_id, notifiable_id, anchor: "comment-panel")#{}"/products/#{product_id}/#{notifiable_type}/#{notifiable_id}#comment-panel"
            when "ProductCustomer"
              product_product_customer_path(product_id, notifiable_id, anchor: "comment-panel") #/products/#{product_id}/#{notifiable_type}/#{notifiable_id}#comment-panel"
            end
  elsif action == "invited"
    product_path(notifiable_id, anchor: "product-invitation-well")
  elsif action == "accepted"
    product_product_users_path(notifiable_id)
  end
end

Here's a really basic refactor of notification_redirection_path:

def notification_redirection_path(notifiable_type:, notifiable_id:, notification_action:, **)
  case notification_action
  when "commented"
    case notifiable_type
    when "Post"
      posts_path(anchor: "post_#{notifiable_id}")
    when "Product"
      product_path(notifiable_id, anchor: "comment-panel")
    when "ProductLead"
      product_lead = ProductLead.find(notifiable_id)
      product_product_lead_path(product_lead.product_id, notifiable_id, anchor: "comment-panel")
    when "ProductCustomer"
      product_customer = ProductCustomer.find(notifiable_id)
      product_product_customer_path(product_customer.product_id, notifiable_id, anchor: "comment-panel")
    end
  when "invited"
    product_path(notifiable_id, anchor: "product-invitation-well")
  when "accepted"
    product_product_users_path(notifiable_id)
  end
end

A few things to notice:

I changed the method arguments to keyword arguments. This way instead of doing this:

redirect_to notification_redirection_path(
  params[:notifiable_type],
  params[:notifiable_id],
  params[:notification_action]
)

...we can do this:

redirect_to notification_redirection_path(params)

I turned the outer if expression into a case expression. I also changed the indentation so the whens line up with their case. Usually I prefer the other style (like you wrote it) but I find with nested cases this is a bit easier to read.
I removed your initial if notifier_type == ... expression and moved that logic into the when "ProductLead" and when "ProductCustomer" blocks below. No need to have those in two separate places.

This is still pretty messy, though. A quick win for readability, maintainability, and testability would be to split it into separate methods:

def notification_redirection_path(notifiable_type:, notifiable_id:, notification_action: nil, **)
  case notifiable_type
  when "commented"
    commented_notification_redirection_path(notifiable_type, notifiable_id)
  when "invited"
    product_path(notifiable_id, anchor: "product-invitation-well")
  when "accepted"
    product_product_users_path(notifiable_id)
  end
end

private
def commented_notification_redirection_path(type, id)
  case type
  when "Post"
    posts_path(anchor: "post_#{id}")
  when "Product"
    product_path(id, anchor: "comment-panel")
  when "ProductLead"
    product_lead = ProductLead.find(id)
    product_product_lead_path(product_lead.product_id, id, anchor: "comment-panel")
  when "ProductCustomer"
    product_customer = ProductCustomer.find(id)
    product_product_customer_path(product_customer.product_id, id, anchor: "comment-panel")
  end
end

You could employ polymorphic_path to take this to its extreme conclusion, which might look something like this:

ACTION_ANCHORS = {
  "commented" => "comment-panel",
  "invited" => "product-invitation-well"
}

def notification_redirection_path(notifiable_type:, notifiable_id:, notification_action:, **)
  if notification_action == "commented" && notifiable_type == "Product"
    return posts_path(anchor: "post_#{notifiable_id}")
  end

  notifiable = notifiable_type.constantize.find(notifiable_id)

  path =
    if notification_action == "commented" &&
        (ProductLead === notifiable || ProductCustomer === notifiable)
      [ notifiable.product, notifiable ]
    elsif notification_action == "accepted"
      [ notifiable, :product_user ]
    else
      [ notifiable ]
    end

  polymorphic_path(path,
    anchor: ACTION_ANCHORS[notification_action])
end

...but it's probably not worth it, since you lose a lot in readability and maintainability.


Simple converter: Is it better to use a function within a function? Below is an example of a simple converter. In the first example temperature I have used a function within a function. In the second example milli I have used a more direct approach. Which would you consider is the better method to handling calculations in functions and why? Also feedback on my approach and examples of other methods is appreciated.

def temperature():
  while True:
    try:
      user_in = float(input('Number to convert:> '))
      t = user_in
      def cel_to_far(x):
        return 9 * x / 5 + 32
      result = 'is: {} degrees Fahrenheit'.format(cel_to_far(t))
      print('Input: {} degrees Celcius'.format(t), result)
      break
    except ValueError:
      print('Enter numbers only!')

def milli():
  while True:
    try:
      user_in = float(input('Number to convert:> '))
      n = 1 * user_in / 25.4
      print('Input: {} millimeters is: {} inches'.format(user_in, n))
      break
    except ValueError:
      print('Enter numbers only!')

I think it depends on how self-evident the code is. If it's clear from context what the intent of a calculation or function call is, then I agree with @Sam that the code will be more concise without giving it a name.

On the other end though, depending on the context, 9 * x / 5 + 32 might not be obvious in what it's doing. You may find that giving that equation a name helps readability in general, which may trump conciseness.

In this exact case though, I don't think there's a ton of benefit to giving it a name unless you strongly suspected that you'd need it elsewhere in the future, or you personally like the way having a name reads. I think that that formula is well-known enough that most people would be able to tell what it's doing; especially when it's inside of a function called temperature.

In this case though, there's no good reason that the function should be inside of temperature. Defining one function inside of another suggests that the inner function has no meaning outside of enclosing function, which may be the case if you're forming a closure over a local variable or something similar. In this case though, cel_to_far is just simply taking a parameter and returning an output. It isn't using anything local to temperature. If you wanted to have a standalone function, I would definitely move cel_to_far outside of temperature. If you want it to be "private" to the module, prefix the name with an _:

def _cel_to_far(x):
    return 9 * x / 5 + 32

def temperature():
    . . .

You're only using two spaces of indentation. Please use four-spaces, as per PEP8.

t = user_in isn't useful. You're taking a line to rename an already short variable name to something obscure. If you need to reduce line lengths, there are other more readable ways of achieving that. Here though, the longest line is only 76 characters long after fixing the indentation and getting rid of t. That's not very long, and user_in is a much more descriptive name.

In both your main functions, you're getting float input from the user manually with a while True and try. In an effort to reduce duplication and centralize how you're handling input, I'd make a helper function:

def _ask_for_float():
    while True:
        try:
            return float(input('Number to convert:> '))

        except ValueError:
            print('Enter numbers only!')

Instead of repeating the while True: try . . . part, make that a function, then call the function. Notice how much neater this makes each of the functions:

def _ask_for_float():
    while True:
        try:
            return float(input('Number to convert:> '))

        except ValueError:
            print('Enter numbers only!')

def _cel_to_far(x):
    return 9 * x / 5 + 32

def temperature():
    user_in = _ask_for_float()
    result = 'is: {} degrees Fahrenheit'.format(_cel_to_far(user_in))

    print('Input: {} degrees Celcius'.format(user_in), result)

def milli():
    user_in = _ask_for_float()
    n = 1 * user_in / 25.4

    print('Input: {} millimeters is: {} inches'.format(user_in, n))

That immediately fixes any line-length issues by removing two levels of indentation, and makes each function trivial.

Finally, lines like:

print('Input: {} degrees Celcius'.format(user_in), result)

Can be written inline using f-strings. I'd collapse result back into the literal for readability as well:

def temperature():
    user_in = _ask_for_float()

    print(f'Input: {user_in} degrees Celcius is: {_cel_to_far(user_in)} degrees Fahrenheit')


Palindromic game in Python This is just a little palindromic game that searches all palindromic numbers in ur random generated list. Feel free to give me some advice.

import random

numbers = []
total = []
dice = random.randint(1000, 50000)

while len(numbers) < dice:
        x = random.randint(100000, 1000000)
        numbers.append(x)

def palindrome(n):
    check_palin = []
    for i in n[::-1]:
        i = str(i)
        check_palin.append(i)
        for k in check_palin:
            k = k[::-1]
            check_palin.clear()
            if k == i:
                total.append(k)
    print(total)
    numbers.clear()

palindrome(numbers)

length = len(total)

if length > 58:
    print("\nHolly Shit!!! You broke my record!!! with %d." % (length))

elif length == 58:
    print("\nWow, Nice!!! %d is my highest Palindromic number (too).\nThat's a draw" % (length))

Instead of saving len(total), just use:

if len(total) > 58:
     print("\nHolly Shit!!! You broke my record!!! with %d." % (length))

elif len(total) == 58:
    print("\nWow, Nice!!! 58 is my highest Palindromic number (too).\nThat's a draw")
    # Since you hardcoded your -highscore-, there's no need to use a variable to print the highscore.

Use whitespace more efficiently, especially in your palindrome() function.
Instead of converting n to str() in your palindrome() function, use numbers.append(str(x)). This way you can reuse palindrome() and have it take n as a string by default (or use an assertion / try: except: statement).

Rewritten:

from random import randint

numbers = []
palindromes = []
dice = randint(1000, 50000)

while len(numbers) < dice:
    x = randint(100000, 1000000)
    numbers.append(str(x))

def palindrome(n):
    for i in n[::-1]:
        if i == i[::-1]:
            palindromes.append(i)
    return palindromes

palindromes = palindrome(numbers)
print(palindromes) # To check this works

if len(palindromes) > 58:
    print("\nHoly shit! You broke my record with %d!\n" % (length))

elif len(palindromes) == 58:
    print("\nWow, nice! 58 is my highest palindromic number too!\n")


Promises in mongoose I've implemented a promises in mongoose to reduce the amount of callback hell but I am not sure whether my technique is correct.

The scenario:

Before I can save my schema I want to run some functions in specific order. These functions update other collections in mongodb. So I have used promises to implement this.

Code: (using promises)

// before saving do this
return incrementSessionTypesTotal(req, res)
.then(
    // before saving do this
    incrementSessionsTypes(req, res, newSurvey))
   //  now save my schema!
.then(newSurvey.save(function(err, data) {
    if (err) {
        sendJsonResponse(res, 400, err)
    } else {
        sendJsonResponse(res, 200, data);
    }
}));

Code:

function incrementSessionTypesTotal(req, res) {

return Total.findOne({dP: "xyz"}, function(err, doc) {
    if (err) {
        sendJsonResponse(res, 401, err)
    } else if (doc) {

            if (req.body.q1 === "poor") {
                doc.poorSessions += 1;
            } else if (req.body.q1 === "ok") {
                doc.okSessions += 1;
            } else if (req.body.q1 === "well") {
                doc.wellSessions += 1;
            } else if (req.body.q1 === "very well") {
                doc.veryWellSessions += 1;
            }
         }

     doc.save(function(err, data) {
        if (err) {
            sendJsonResponse(res, 400, err)
        } else {
            return data;
        }
    });

    }).exec();
 }

Is this implementation okay? I have not really used promises with mongoose before so I am not sure whether this is correct. I have tested the code and it works but I would like some feedback.

What I don't like in your solution

In the latest version of Mongoose, save can also return a Promise. The "problem" with your code is that when you are saving you are using a callback, which defeats the purpose of Promises overall.

How I would do it

This is how I would implement using Promises:

function incrementSessionTypesTotal(req, res) {
    return Total.findOne({dP: "xyz"})
        .then(doc => {
            incrementSessionsTypes(req, res, doc));
            return doc.save();
        })
        .then(() => {
            sendJsonResponse(res, 200, data);
        })
        .catch(error => {
            sendJsonResponse(res, 400, err);
        });
}

Following the code flow

Following the code flow, without the callbacks it is clear what you do.

First you findOne
After finding an object, we increment it. If doc===null then the increment function will throw an exception, which will be caught later on.
Then we save it. Remember I said save can also return a Promise? Well, because Promises are monads (sort of) if you return a Promise  within a then, its result will follow the chain!
Save was successful, send message!

Problems with my solution

As you may have guessed ... this solution catches any error and sends the same response for all of them (400).

With this structure, if you want error differentiation, the only thing you can do is to detect an error occurred, build an object with that information and then throw it:

.then(doc => {
    if(doc === null)
        throw {reason: "ObjectNotFound"};

    incrementSessionsTypes(req, res, doc));
    return doc.save();
})
.catch(error => {
    if(error.reason === "ObjectNotFound")
        sendJsonResponse(res, 404, err);
    else if(error.reason === "Something else ")
        sendJsonResponse(/*you get the idea*/);
    else //if we don't know the reasons, its because the server likely crashed
        sendJsonResponse(res, 500, err);
});

Another solution would be to have an ErrorFactory and/or to subclass your errors.

I personally don't find any of these solutions particularly attractive because it means you will have a huge if then else chain of statements in your catch block, if you want error differentiation.

This is an issue I am still fighting with, but its up to you.

Hope it helps!

EDIT EDIT EDIT

In another question I posted what I believe to be the solution to complete error differentiation while using promises (see solution 2):

https://codereview.stackexchange.com/a/159593/59328

That post also has a good example of classical error building and differentiation, alongside an analysis of its limitation (solution 1).

I hope you enjoy!


Converting an input of space delimited integers to a list tablou = input("Enter values delimited by space: ")
b = tablou.split()

t = [] # initial  list with int() contents
for l in b:
    r = int(l)
    t.append(r)

I have to do multiple assignments which require working with lists of integers/floats.  Above is what I'm using for converting a string to a proper list of integers.  I was wondering if there's any way to optimize the code in a such way that it won't need to use memory to keep tablou, b and the final list.  I would be actually working with t

I'm not sure if I used the right terms to describe the issue, I'm still learning.

Releasing memory

If you assign None to your variables, their contents is lost and the memory will (eventually) be reclaimed.

tablou = input("Enter values delimited by space: ")

b = tablou.split()
tablou = None

t = [] # initial  list with int() contents

for l in b:
    r = int(l)
    t.append(r)
b = None
l = None
r = None

Forgetting the variables

The variables tablou, l, b, and r will still exist; they will just contain the value None.  If you accidentally use the variable tablou again, you won't get a NameError: name 'tablou' is not defined, although you may get an error stating the operation cannot be performed on the value None.

Better would be to delete the variables when they are no longer needed:

del tablou
del l, b, r

After deleting the variables, it is an error to refer to them again, without first storing a value under that name.

List Comprehension

Repeatedly appending values to a list is an expensive operation.  It is usually faster to create the list using "list comprehension".

First, note that instead of this ...

t = []

for l in b:
    r = int(l)
    t.append(r)

... you could write this, eliminating the r variable:

t = []

for l in b:
    t.append(int(l))

Then, you can replace the list creation, loop, append with the following:

t = [ int(l) for l in b ]

This does the same thing as the former code, but without the repeated append call.  As a bonus, the list comprehension variable, l does not escape into the local scope; there is no need to del l.

Mapping

Applying the same operation to every element of a list is called a "mapping", and Python comes with a built in function map() to do just that.

In your specific case, you are calling int( ) on every item in the list b, so instead of:

t = [ int(l) for l in b ]

you could write:

t = list(map(int, b))

Advance topic: The map() function actually returns an iterator, rather than a list.  In many cases, you can avoid the call to list(), which iterates over all the items the iterator can produce and builds a list of out them, and simply use the returned iterator directly.

One-liner

You can reduce your code to one line, removing all temporary variables, using nested calls:

t = list(map(int, input("Enter values delimited by space: ").split()))

I don't recommend it, however.  It does not result in easy-to-understand code.  It is better to separate "user input" from "processing".

Variable Names

t, r, l and b are horrible variable names.  Name the variables with longer, descriptive names.


forEach and NodeList This is loosely based on one of W3_School samples:

<!DOCTYPE html>

Hello World!

The DOM is very useful!

This example demonstrates forEach() function

Please let me know if any improvements need (could) be done to the script.

The only purpose of this script appears to be to illustrate the use the Array.prototype.slice.call "idiom" and the forEach function. So it's hard to make "improvements" when the purpose of the code isn't its actual result (which is trivial).

What the code actually does can be done in a number of ways, some of which won't (directly) use forEach and/or NodeList and/or slice.call. In that case, is it still an improvement? Conversely, sticking to forEach etc. doesn't leave much room for change, since the code seems designed as a minimal example of those things.

I'm hesitant to write any code, because I'm not sure what the point would be.

With that said,

Don't rely on W3 Schools (this site lists several reasons and better alternatives)
Avoid document.write() - or use it with open eyes (see why here)
I'd also avoid forEach in favor of a standard for loop, since forEach is not universally supported in browsers (see caniuse.com)
The page's text appears to betray some confusion. Yes, the DOM is useful, and yes, the code demonstrates forEach. But forEach isn't part of the DOM. The whole point is that a NodeList object is a DOM object and not a normal array. It must therefore be converted to one by way of Array.prototype.slice.call. Then one can use forEach which is an array function but which, in and of itself, has nothing to do with the DOM.

Update after reading @PM77-1's comment:

There's nothing inherently wrong with this code. It is true, that
A) for...in doesn't behave like it does in Java and shouldn't be used here, and that
B) getElementsByTagName doesn't return a "real" array. The slice.call trick is indeed the "standard" way of converting an "array-like" object (which you find a lot of, unfortunately) to a real array. A very useful trick to know.

So all in all, the code is basically fine. Yes, improvements can be made, as @Palacsint points out in his answer, but there's nothing terrible going on.

However, while a NodeList object isn't a real array, one can still use a regular old for loop:

var nodes, i;

nodes = document.getElementsByTagName("p");
for(i = 0 ; i < nodes.length ; i++) {
  document.write(nodes[i].innerHTML);
  document.write("
");
}

This removes the need for the slice.call trick, and it works on old browsers (where forEach doesn't exist).

In general, though, when working with the DOM, it's useful to use a library to iron out the otherwise frustrating differences between browsers. jQuery is the king of the hill these days, and with that, you'd do something like this to find and iterate over some elements:

$("p").each(function () {
  document.write(this.innerHTML);
  document.write("
");
});

(Of course, jQuery also makes it easy to do "proper" DOM insertion without resorting to document.write, but this is just for illustration)

However, I do think it very wise to learn the nitty-gritty of JS, and not just run straight for jQuery or its ilk. And dealing with those "array-like" is one of those nitty-gritty things indeed.


Add user profile fields I have this PHP function that adds custom user profile fields to WordPress. It saves the field through the WooCommerce form-edit-my-account.php file. I have a lot of repeated code, and would like to clean it up.

function save_sfirst_account_details( $user_ID ){
    $sfirst = ! empty( $_POST['sfirst'] ) ? wc_clean( $_POST['sfirst'] ) : '';
        update_user_meta( $user_ID, 'sfirst', $sfirst );
    }

function save_slast_account_details( $user_ID ){
    $slast = ! empty( $_POST['slast'] ) ? wc_clean( $_POST['slast'] ) : '';
    update_user_meta( $user_ID, 'slast', $slast );
}

function save_grade_account_details( $user_ID ){
    $grade = ! empty( $_POST['grade'] ) ? wc_clean( $_POST['grade'] ) : '';
    update_user_meta( $user_ID, 'grade', $grade);
}

function save_city_account_details( $user_ID ){
    $city = ! empty( $_POST['city'] ) ? wc_clean( $_POST['city'] ) : '';
    update_user_meta( $user_ID, 'city', $city );
}

function save_state_account_details( $user_ID ){
    $state = ! empty( $_POST['state'] ) ? wc_clean( $_POST['state'] ) : '';
    update_user_meta( $user_ID, 'state', $state);
}

function save_degree_account_details( $user_ID ){
    $degree = ! empty( $_POST['degree'] ) ? wc_clean( $_POST['degree'] ) : '';
    update_user_meta( $user_ID, 'degree', $degree );
}

function save_semail_account_details( $user_ID ){
    $semail = ! empty( $_POST['semail'] ) ? wc_clean( $_POST['semail'] ) : '';
    update_user_meta( $user_ID, 'semail', $semail );
}
add_action( 'woocommerce_save_account_details', 'save_student_account_details' );

Can anyone help me out with this?

I would make a single function, since all your functions do the same thing. It just needs an extra argument.

    /// one all-purpose function
    function saveData ($user_ID, $data_field) {
        $data = !empty($_POST[$data_field]) ? wc_clean($_POST[$data_field]) : '';
        update_user_meta( $user_ID, $data_field, $data );
    }

    /// define actions
    $actions=array("sfirst","slast",...);

    /// run actions
    foreach ($actions as $action) {
        add_action( 'woocommerce_save_account_details','saveData',$action);
    } unset($action);


Product of all but one number in a sequence I was given the following prompt in a coding interview:

Given an array of integers, return a new array such that each element at index i of the new array is the product of all the numbers in the original array except the one at i.
For example, if our input was [1, 2, 3, 4, 5], the expected output would be [120, 60, 40, 30, 24]

I solved this in two ways:

fun multiplies all elements together in the first iteration, and then loops again and divides by the number at that position
fun2 does not use division, and instead iteratively builds up the sum in each index

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

int fun(int* nums, int arr_size)
{
    int sum;
    int i;

    for(i=0, sum=1; i<arr_size; i++)
        sum*=nums[i];
    for(i=0; i<arr_size; i++)
        nums[i]=sum/nums[i];
    return 0;
}

int fun2(int* nums, int arr_size)
{
    int i,j;
    int sum=1;
    int new_arr[arr_size];

    for(i=0; i<arr_size; i++) {
        for(j=0; j<arr_size; j++) {
            if(i!=j)
                sum*=nums[j]; //skip member same index in the loop
        }
        new_arr[i]=sum;
        sum=1;
    }
    memcpy(nums, new_arr, arr_size*sizeof(int));
    return 0;
}

int main(void)
{
    /*Given an array of integers, return a new array such that each element at index i of the
    new array is the product of all the numbers in the original array except the one at i.
    For example, if our input was [1, 2, 3, 4, 5], the expected output would be
    [120, 60, 40, 30, 24] */
    int nums[] = {1, 2, 2, 4, 6};
    int size = sizeof(nums)/sizeof(nums[0]);
    int i;
    fun(nums, size);
    for (i = 0; i < size; i++)
        printf("%d ", nums[i]);

    //what if you can't use division?
    printf("\n");
    int nums2[] = {1, 2, 2, 4, 6};
    fun2(nums2, size);
    for (i = 0; i < size; i++)
        printf("%d ", nums2[i]);

    return 0;
}
```

Here are some things that may help you improve your code.
Use all required #includes
The code uses memcpy, so it should #include <string.h>.  It might still compile on your machine, with your compiler, but it's not portable.
Think about potential errors
As one of the comments correctly notes, if one of the entries has the value of zero, this line will have a problem:
nums[i]=sum/nums[i];

Also, what happens if the passed arr_size is zero or negative?  What should the function return if there is exactly one item in the array? What if the passed pointer is NULL?
Follow directions exactly
The problem says to "return a new array" but that is not really what this code is doing.  This code is overwriting the input array.  One of the problems with that is that it's not possible to call this with a const pointer as mentioned in the next suggestion.  It also means that rather than returning a meaningless constant value in all cases, the function should probably return a pointer.
Use const where practical
As mentioned above, the code should return a new array rather than overwriting the passed one.  I would suggest that the function should be something like this:
int* exclusive_product(const int* nums, size_t nums_size)

Note that first, we use const and second, we use size_t rather than int for the second argument to more clearly indicate the type of variable we are expecting.
Use better variable names
I would say that nums, size and i are good variable names, but that fun and fun2 and definitely sum are not.  The problem is that fun doesn't tell the reader anything about what the code is supposed to do and sum is actually misleading (it's a product, not a sum).
Think about an efficient way to solve this
The \$O(n^2)\$ code you have in fun2 is not a terrible way to solve the problem and has the advantage of being obviously correct.  When I interview people, I typically like such answers because it's much easier to make slow correct code fast than it is to make fast incorrect code correct.  However, in a good interview, I like to ask the candidate to make comments on his or her own code, including any limitations, assumptions or potential improvements that might be made.  In this case, it helps if we think mathematically about the final values in the resulting array \$B\$ from input array \$A\$.  For example, we know that every value \$B_j\$ can be expressed as the product $$\displaystyle B_j = \prod_{i=0}^{j-1} A_i \prod_{i=j+1}^{n-1} A_i$$ if \$n\$ is the length of the array.  This suggests a more efficient approach I'll leave for you to figure out.


Simple terminal graphics line drawing The program starts at the center of the screen and starts drawing a line in 50ms steps. When the line hits the edge of the terminal it will be reflected and continue drawing. steps takes the number of drawing steps to perform before finishing while until-key finishes on a key press.

Apart from the lack of comments, what would I need to do to make this "good" Forth and how would I change it to fit in one screen (16 lines of 64 chars)?

variable h
variable w
: height h @ ;
: width  w @ ;

variable point-var
cell allot
: point ( offset -- addr ) cells point-var + ;

variable direction-var
cell allot
: dir ( offset -- addr ) cells direction-var + ;

: init ( -- )
    rows h !
    cols w !
    width  2 / 0 point !
    height 2 / 1 point !
    1 0 dir !
    1 1 dir ! ;

: reflect-top ( -- t/f )
    1 point @ 0= dup if 1 1 dir ! then invert ;
: reflect-bot ( -- t/f )
    1 point @ height 1 - = if -1 1 dir ! then ;
: reflect-left ( -- t/f )
    0 point @ width 1 - = if -1 0 dir ! then ;
: reflect-right ( -- t/f )
    0 point @ 0= dup if 1 0 dir ! then invert ;
: reflect ( -- )
    reflect-top if reflect-bot then
    reflect-right if reflect-left then ;

: draw-point ( -- ) [char] # emit ;

: move-x ( -- n ) 0 point @ 0 dir @ + dup 0 point ! ;
: move-y ( -- n ) 1 point @ 1 dir @ + dup 1 point ! ;
: move-xy ( -- ) move-x move-y at-xy ;

: try ;

: step ( -- )
    try reflect
    move-xy draw-point ;

: steps ( n -- )
    init
    page
    0 do step 50 ms loop ;

: until-key ( n -- )
    init
    page
    begin step 50 ms key? until ;

Magic numbers

    0 constant X
    1 constant Y

looks more attractive than naked 0 and 1. I suppose (sorry can't test it right now) it may let you unify move-x with move-y, as well as reflects.
Height and width

    : height= h @ 1 - = ;
    : width=  w @ 1 - = ;

allow more symmetric definition of reflects:

    : reflect-top Y point @ 0=      dup  if  1 1 dir ! then invert ;
    : reflect-bot Y point @ height=      if -1 1 dir ! then ;

Now it looks like they have different stack effects, which is strange. Are you sure reflect-bot doesn't miss dup?
move-*

I'd rearrange pushes, e.g.

    : move-x 0 0 0 point @ dir @ + dup point ! ;
    : move-y 1 1 1 point @ dir @ + dup point ! ;

which really prompts for a point @ dir @ + dup point ! word. move-in-dir perhaps?
One screen

Do modern Forthers still care about screens?


A generic REST API consuming Python library I wrote this, which essentially acts as a single library for all possible REST services.

Here is the main file, as of alpha version 1:

import requests, json

class RestConsumer(object):

    def __init__(self,base_url,append_json=False,**kwargs):
        self.base_url = base_url
        self.uriparts = []
        self.append_json = append_json
        self.kwargs = kwargs

    def __getattr__(self,k):
        if k=='spc':
            return self.spc
        if not k[0] == '_':
            self.uriparts.append(k)
        else:
            if k[1:3]=='in':
                self.uriparts.append(k[3:])
            elif k[1:3] == 'uh':
                self.uriparts.append(k.replace('_','-'))
        return self

    def spc(self,k):
        self.uriparts.append(k)
        return self

    def __call__(self, **kwargs):
        uri_constructed = '/'.join(self.uriparts)
        self.uriparts = []
        self.uri_constructed = "%s%s"%(uri_constructed,'.json') if self.append_json else uri_constructed
        self.url = '/'.join([self.base_url,self.uri_constructed])
        return self.get(self.url,**kwargs)

    def get(self,url,**kwargs):
        print url
        r = requests.get(url,**kwargs)
        return json.loads(r.content)

    def post(self,**kwargs):
        r = requests.post(**kwargs)
        return json.loads(r.content)

if __name__=='__main__':
    from pprint import pprint
    t = RestConsumer(base_url='https://api.twitter.com/1',append_json=True)
    public_timeline = t.statuses.public_timeline()
    pprint(public_timeline)

    g = RestConsumer(base_url='https://api.github.com')
    repos = g.users.kennethreitz.repos()
    pprint(repos)

    s = RestConsumer(base_url='http://api.stackoverflow.com/1.1')
    sr = s.users.spc('55562').questions.unanswered()
    pprint(sr)

    sr2 = s.tags.python.spc('top-answerers').spc('all-time')()
    pprint(sr2)

The rationale and usage is all explained in the README.

Please critique the library, the idea, implementation and shed light on alternatives.

import requests, json

class RestConsumer(object):

    def __init__(self,base_url,append_json=False,**kwargs):
        self.base_url = base_url
        self.uriparts = []
        self.append_json = append_json
        self.kwargs = kwargs

Since you treat attributes "special", I recommend putting _ in front of these attributes so that are clearly offset.

    def __getattr__(self,k):

I recommend not using the random letter k for the name.

        if k=='spc':
            return self.spc

__getattr__ only gets invoked if the attribute cannot be found. Since spc will be found this won't ever get used.

        if not k[0] == '_':
            self.uriparts.append(k)
        else:
            if k[1:3]=='in':
                self.uriparts.append(k[3:])

What's the point of this rule? _infoo means foo?

            elif k[1:3] == 'uh':
                self.uriparts.append(k.replace('_','-'))

uh?

        return self

I don't like the way you've implemented this. Accessing attributes modifying the object's state is deeply counter-intuitive.

    def spc(self,k):
        self.uriparts.append(k)
        return self

spc is not a clear name. Its hard to guess what it means.

    def __call__(self, **kwargs):
        uri_constructed = '/'.join(self.uriparts)
        self.uriparts = []
        self.uri_constructed = "%s%s"%(uri_constructed,'.json') if self.append_json else uri_constructed
        self.url = '/'.join([self.base_url,self.uri_constructed])
        return self.get(self.url,**kwargs)

    def get(self,url,**kwargs):
        print url
        r = requests.get(url,**kwargs)
        return json.loads(r.content)

    def post(self,**kwargs):
        r = requests.post(**kwargs)
        return json.loads(r.content)

get prints while post doesn't. Probably neither should be printing, a log facility might be useful.

The way you've designed this, some useful things won't work:

public_timeline = twitter.statuses.public_timeline
while True:
      print public_timeline()

or

user = s.users.spc(55562)
print user.questions.unanswered()
print user.questions.answered()

I suggest that instead of modifying the object, you return new objects. That way it will work in an intuitive manner.

Instead of spc I suggest that you provide a __getitem__. That way you can do:

 s.users[55562].questions.unanswered()

Which I think will make it cleaner.


Remove duplication in Ruby map with conditional entries I have a nasty (IMO) piece of code for a datatable configuration, how do i best remove the duplication here?

I've tried the obvious things I know (collapsing into one statement and trying inline conditionals) but it isn't working and I can't help thinking there must be a more elegant Ruby way here.

if show_status == "true"
  if show_requested_by == "true"
    records.map do |record|
      {
        warning: as_warning(record),
        updated_at: as_time_ago(record.updated_at),
        status: as_status(record.status),
        user: record.user.nil? ? "" : record.user.formal_name,
        manager: record.manager.nil? ? "" : record.manager.formal_name,
        first_date: as_date(record.first_date),
        last_date: as_date(record.last_date),
        days: as_number(record.total_days),
        hours: as_number(record.total_hours),
        units: as_number(record.total_units),
        value: as_number(record.total_value),
        comment: as_comment(record.comment),
        actions: as_actions(record)
      }
    end
  else
    records.map do |record|
      {
        warning: as_warning(record),
        updated_at: as_time_ago(record.updated_at),
        status: as_status(record.status),
        manager: record.manager.nil? ? "" : record.manager.formal_name,
        first_date: as_date(record.first_date),
        last_date: as_date(record.last_date),
        days: as_number(record.total_days),
        hours: as_number(record.total_hours),
        units: as_number(record.total_units),
        value: as_number(record.total_value),
        comment: as_comment(record.comment),
        actions: as_actions(record)
      }
    end
  end
else
  if show_requested_by == "true"
    records.map do |record|
      {
        warning: as_warning(record),
        updated_at: as_time_ago(record.updated_at),
        user: record.user.nil? ? "" : record.user.formal_name,
        manager: record.manager.nil? ? "" : record.manager.formal_name,
        first_date: as_date(record.first_date),
        last_date: as_date(record.last_date),
        days: as_number(record.total_days),
        hours: as_number(record.total_hours),
        units: as_number(record.total_units),
        value: as_number(record.total_value),
        comment: as_comment(record.comment),
        actions: as_actions(record)
      }
    end
  else
    records.map do |record|
      {
        warning: as_warning(record),
        updated_at: as_time_ago(record.updated_at),
        manager: record.manager.nil? ? "" : record.manager.formal_name,
        first_date: as_date(record.first_date),
        last_date: as_date(record.last_date),
        days: as_number(record.total_days),
        hours: as_number(record.total_hours),
        units: as_number(record.total_units),
        value: as_number(record.total_value),
        comment: as_comment(record.comment),
        actions: as_actions(record)
      }
    end
  end
end

This is just a rough thought, so it's untested, but here goes:

A vast majority of the hash is the same every time. So, that vast majority can be stuck into a method. There are two fields that are conditionally added to that hash based on their values ('true' or... not). So, we can create a method which takes a record as an argument and the two 'true'/not options and generates a hash accordingly:

def record_for(record, status: false, requested_by: false)
  record_hash = {
    warning: as_warning(record),
    updated_at: as_time_ago(record.updated_at),
    manager: record.manager.nil? ? "" : record.manager.formal_name,
    first_date: as_date(record.first_date),
    last_date: as_date(record.last_date),
    days: as_number(record.total_days),
    hours: as_number(record.total_hours),
    units: as_number(record.total_units),
    value: as_number(record.total_value),
    comment: as_comment(record.comment),
    actions: as_actions(record)
  }
  record_hash[:status] = as_status(record.status) if status
  record_hash[:user] = record.user.nil? ? "" : record.user.formal_name if requested_by
  record_hash
end

Then, all that remains is to convert the strings to booleans:

def to_bool(str)
  return true if str == 'true'
  false
end

and then iterate over the records:

status = to_bool(show_status)
requested_by = to_bool(show_requested_by)

records.map { |r| record_for(r, status: status, requested_by: requested_by) }

I'd be happy to go more in depth and test the code if you provided a set of test records that I could use or some larger context this code would run in.


writing a generic object pool in C++ For a hobby project, My aim is to design and implement a generic system for pooling an arbitrary class of object. I tried implementing it using templates.
This pool should support 4 actions:
"Create a pool"
"Allocate an object from this pool"
"Deallocate an object from this pool"
"Destroy this pool"
For sake of simplicity, there is no multi-threading involved.

If I have to pool classes like this example, how do I do that?

typedef char            ByteType;
typedef void*           PointerType;
typedef char            FixedStringType[256];

// A basic struct
struct Point
{
    int x, y, z;
};

Can someone please provide your thoughts/feedback on bugs and how to improve this code?

//ObjectPool.h
#pragma once
#include <vector>
#include <iostream>

const int g_MaxNumberOfObjectsInPool = 2;

template<typename T>
class DefaultAllocator {
public:
    T* operator()() {
        return new T{};
    }
    void operator()(T* p) {
        delete p;
    }
    void reset() {
        std::cout << "reset function called from default allocator" << std::endl;
    }
};

template<typename T, typename AllocatorT=DefaultAllocator<T>>
class ObjectPool
{
    struct ObjectInfo {
        bool isInUse{};
        T* ptrObject{};
    };
    static inline std::vector<ObjectInfo> poolObjects{};
    static inline AllocatorT allocator{};
public:
    static T* getObject() {
        for (auto& currObj : poolObjects) {
            if (!currObj.isInUse) {
                currObj.isInUse = true;
                std::cout << "Existing Object returned" << std::endl;
                return currObj.ptrObject;
            }
        }

        if (poolObjects.size() == g_MaxNumberOfObjectsInPool) {
            std::cout << "Pool is full " << std::endl;
            return nullptr;
        }

        std::cout << "Creating a new object" << std::endl;
        //auto newObj = new T{};
        auto newObj = allocator();
        poolObjects.push_back({ true, newObj });
        return newObj;
    }

    static void releaseObject(T* ptrSo) {
        for (auto& currObj : poolObjects) {
            if (currObj.ptrObject == ptrSo) {
                currObj.isInUse = false;
                return;
            }
        }
    }

    static void destroy() {
        for (auto& currObj : poolObjects) {
            if (currObj.isInUse) {
                std::cout << "WARNING! this object is still in use" << std::endl;
            }
            allocator(currObj.ptrObject);
        }
        allocator.reset();
        poolObjects.clear();
    }
};

The main file looks like this:
#include "ObjectPool.h"
#include <iostream>

class PrivateClass {
    PrivateClass() {

    }
public:
    void func() {

    }
    friend class PrivateAllocator;
};

class PrivateAllocator {
public:
    PrivateClass* operator()() {
        return new PrivateClass{};
    }
    void operator()(PrivateClass* p) {
        delete p;
    }
    void reset() {
        std::cout << "reset function called" << std::endl;
    }
};

int main() {
    using integer = ObjectPool<int>;
    auto int1 = integer::getObject();
    auto int2 = integer::getObject();
    auto int3 = integer::getObject();

    integer::releaseObject(int1);
    auto int4 = integer::getObject();
    integer::destroy();
    return 0;
}

#pragma once

Don’t use #pragma once. It’s non-standard, it breaks silently and unexpectedly and when it does all hell breaks loose, and the standard solution (include guards) are just as efficient in practice.
const int g_MaxNumberOfObjectsInPool = 2;

There doesn’t seem any good reason why this should be a global variable. It would be better as a static data member of ObjectPool.
It should probably also be constexpr.
But the most important issue is that you are using (signed) int for a size type. That will cause problems if you’re not very careful, because most sizes in C++ use unsigned integers. Yes, this was a mistake, but you can’t fix it by stubbornly ignoring it and insisting on using signed types for sizes instead. “When in Rome” and all.
The correct type to use here is probably std::size_t.
template<typename T>
class DefaultAllocator {
public:
    T* operator()() {
        return new T{};
    }
    void operator()(T* p) {
        delete p;
    }
    void reset() {
        std::cout << "reset function called from default allocator" << std::endl;
    }
};

… why?
There is a standard allocator interface in C++, and it already does everything you need it to do (and more! it’s actually possible to build what is functionally an object pool using any container type(s) with the right allocator… and the right allocator comes built-in with the standard). It makes zero sense to just ignore that and reinvent your own, incompatible allocator interface. (Especially one with a nonsensical, hobbled interface. What does reset() even do?)
Not to mention that you’re already using the standard allocator interface anyway! std::vector<T> is actually std::vector<T, std::allocator<T>>. So when you do:
auto newObj = allocator();
poolObjects.push_back({ true, newObj });

You are using two allocators: your allocator in the first line, and the vector’s standard allocator in the second.
All-in-all, making your own, incompatible allocator regime when a perfectly good standard regime exists (and you even use it!) just seems remarkably silly. And it will get worse….
class ObjectPool

You have made everything in this class static, presumably to hack a singleton-like interface. That’s a bad idea for a number of reasons.
First, singletons are an anti-pattern. In rare circumstances, they are genuinely useful, but even then it should be opt-in… not forced by the type… because that makes testing extremely difficult. And you are testing your code, aren’t you? Hm?
In addition to making testing difficult, forcing a singleon design makes your object pool less useful. What if I want multiple object pools? What if, for example, I’m making a engineering modelling program, and I want to put all the points for the current model in a pool… but then I want to allow a split-screen view of two models side-by-side, so I now need two pools? Why would you punish users of your object pool by denying them the ability to create more than one? What’s the sense in that?
If you made your object pool in the normal, sensible way—that is, not making everything static—then if you really wanted a singleton object pool later, you could always make a trivial wrapper:
template <typename T, typename Allocator>
class SingletonObjectPool
{
    static ObjectPool<T, Allocator> _pool;

public:
    static auto getObject() { return _pool.getObject(); }

    static auto releaseObject(T* p) { return _pool.releaseObject(); }

    static auto destroy() { return _pool.destroy(); }
};

But you can’t go the other way; if you make ObjectPool with everything static, you can’t make a non-static object pool.
struct ObjectInfo {
    bool isInUse{};
    T* ptrObject{};
};
static inline std::vector<ObjectInfo> poolObjects{};

This is really kinda defeating the whole purpose of std::vector.
First of all, ObjectInfo is a dangerous type, because that T* in there is an owning pointer… but it’s not a smart pointer. For that sin alone I’d refuse the whole thing from any project I was in charge of.
The result of this sin is that your object pool leaks all kinds of memory if you don’t manually call destroy()… which not only defeats the purpose of std::vector, it defeats the whole purpose of C++!
The standard first fix to this kind of problem is to replace pointers with the objects themselves; C++ is a value-semantic language, after all:
struct ObjectInfo
{
    bool isInUse = false;
    T object;
};

But if you do this, then you run into problems due to the fact that you’re using a std::vector as your actual pool. Every time you add a new object, it may trigger an internal reallocation of the vector’s array, which would change the addresses of all the objects… and you’re using object addresses as handles, so it would completely break your object pool.
There are two options to fix this problem: don’t use vector as your pool, or don’t store the pooled objects directly in the vector. Your solution is option 2, which, with smart pointers gives:
struct ObjectInfo
{
    bool isInUse = false;
    std::unique_ptr<T> ptrObject = nullptr;
};

Is that the best solution? Eeeh. But we’ll get back to that.
static T* getObject() {
    for (auto& currObj : poolObjects) {
        if (!currObj.isInUse) {
            currObj.isInUse = true;
            std::cout << "Existing Object returned" << std::endl;
            return currObj.ptrObject;
        }
    }

    if (poolObjects.size() == g_MaxNumberOfObjectsInPool) {
        std::cout << "Pool is full " << std::endl;
        return nullptr;
    }

    std::cout << "Creating a new object" << std::endl;
    //auto newObj = new T{};
    auto newObj = allocator();
    poolObjects.push_back({ true, newObj });
    return newObj;
}

This function is way too complex. It does way too much work for a single function:

First it finds the first not-in-use object, if any.
Then it checks whether the pool is full.
Finally, it creates a new object in the pool.

Each one of those steps could be its own function. And some of them could even be public functions, because they might be useful to users. For example, the first step, finding the first not-in-use object could be handy for an algorithm that would like a T, but if it can’t get one, can do something else instead.
So you might do:
template<typename T, typename Allocator>
class ObjectPool
{
    // ... [snip] ...

public:
    // tries to get an available object, and if that fails, tries to create one
    auto getObject() -> T*
    {
        if (auto p = getAvailableObject(); p)
            return p;

        return newObject();
    }

    // gets an object only if one is available, never creates one, never fails
    auto getAvailableObject() noexcept -> T*
    {
        if (auto p_info = _findAvailableObjectInfo(); p_info)
        {
            p_info->isInUse = true;
            std::cout << "Existing Object returned\n";
            return (p_info->ptrObject).get();
        }
        else
        {
            return nullptr;
        }
    }

    // always creates an object (unless the pool is full)
    auto newObject() -> T*
    {
        if (poolObjects.size() == g_MaxNumberOfObjectsInPool)
            throw std::runtime_error{"pool is full"};

        std::cout << "Creating a new object\n";

        auto newObj = std::unique_ptr{allocator()};
        poolObjects.emplace_back(true, std::move(newObj));
        return newObj;
    }

private:
    auto _findAvailableObjectInfo() noexcept -> ObjectInfo*
    {
        auto is_in_use = [](auto&& p_info) { return p_info->isInUse; };

        if (auto p = std::find_if_not(poolObjects.begin(), poolObjects.end(), is_in_use); p != poolObjects.end())
            return *p;
        else
            return nullptr;
    }

    // ... [snip] ...
};

Some additional notes:

Don’t write naked loops. Use standard algorithms. In this case, that loop is obviously a std::find_if(_not)().
Don’t use std::endl. Every time you use it, it flushes the output stream, which can slow things down tremendously.
Don’t return nullptr when an object can’t be gotten. That just gives users of the object pool headaches, because now they have to check every single call to getObject(). If the point is to get an object, and an object can’t be gotten, then throw an exception to indicate failure. (OR, if you really want to avoid relying on exceptions, look in to an “expected” type, like the proposed std::expected, or boost::outcome.)

static void releaseObject(T* ptrSo) {
    for (auto& currObj : poolObjects) {
        if (currObj.ptrObject == ptrSo) {
            currObj.isInUse = false;
            return;
        }
    }
}

This loop is another std::find_if().
static void destroy() {
    for (auto& currObj : poolObjects) {
        if (currObj.isInUse) {
            std::cout << "WARNING! this object is still in use" << std::endl;
        }
        allocator(currObj.ptrObject);
    }
    allocator.reset();
    poolObjects.clear();
}

There is no single standard algorithm that could cleanly replace this loop. Part of the problem is that it does multiple things… which is bad. You could replace it with a single std::for_each(), of course, but it might make more sense to break it into two steps—one to check for still-in-use, and one to do the deallocation.
This becomes especially important if you use smart pointers, in which case the deallocation step becomes completely unnecessary. And, given that allocator.reset() seems to serve no purpose, the whole thing could boil down to:
static void destroy()
{
    auto is_in_use = [](auto&& p_info) { return p_info->isInUse; };

    std::for_each(
        std::remove_if(poolObjects.begin(), poolObjects.end(), is_in_use),
        poolObjects.end(),
        [] (auto&&) { std::cout << "WARNING! this object is still in use\n"; }
    );

    poolObjects.clear();
}

But frankly, a destroy() function is silly in C++. We have built-in destroy() functions; they’re called destructors, and they’re awesome precisely because you don’t have to call them… they work automatically. If the object pool weren’t a hacky-singleton due to everything being static, then the whole function (except for the warning about unreleased objects part) would become obsolete due to the destructor.
An alternative design
Let me propose an alternative design, that doesn’t use std::vector.
The primary purpose of using std::vector is that it packs its data altogether in a tight space, which is fantastic for performance. However, you lose all the benefits of that when you store pointers in a vector, because even though the pointers are all nicely packed together, your data can be all the hell over the place. A vector of pointers to stuff has more-or-less no real benefit over any other container type… and in fact is usually worse than other containers because you have to do this two-stage allocation dance: first allocate the actual stuff, then allocate the pointer to the stuff in the vector.
Instead of using a vector, how about using std::forward_list. And let’s throw away ObjectInfo, and just store the Ts directly in the forward list. So how do we know which ones are in use and which aren’t? Simple: two forward lists; one for in-use objects, one for not-in-use objects.
And of course, let’s use standard allocators rather than reinventing the wheel.
So:
template <typename T, typename Allocator = std::allocator<T>> // or std::pmr::allocator<T>, if you prefer
class ObjectPool
{
    std::forward_list<T> _objects;
    std::forward_list<T> _available_list;

public:

    ObjectPool() = default;

    // this constructor allows you to provide a stateful allocator
    explicit ObjectPool(Allocator const& alloc) :
        _objects(alloc),
        _available_list(alloc)
    {}

    // moveable
    ObjectPool(ObjectPool&&) noexcept = default;
    auto operator=(ObjectPool&&) noexcept -> ObjectPool& = default;

    // non-copyable
    ObjectPool(ObjectPool const&) = delete;
    auto operator=(ObjectPool const&) -> ObjectPool& = delete;

    // need to declare this for sanity, because the copy ops are deleted
    ~ObjectPool() = default;

    // ... [snip] ...
};

Now how would we implement getObject()? Well, let’s take it one step at a time. First, how would we implement newObject()? That’s easy:
template <typename... Args>
auto newObject(Args&&... args) -> T*
{
    if (/* check whether there is capacity left - depends on how you want to handle capacity */)
        throw std::runtime_error{"pool is full"};

    std::cout << "Creating a new object\n";
    return &(_objects.emplace_front(std::forward<Args>(args)...));
}

What about getAvailableObject()? The first part—checking whether anything is available—is not hard:
auto getAvailableObject() noexcept -> T*
{
    if (_available_list.empty())
        return nullptr;

    // ???
}

If the available list isn’t empty, we want to take an object out of it, and put it into the active objects list. We’ll put it at the very front of the active objects list, because it’s easiest to access it there:
auto getAvailableObject() noexcept -> T*
{
    if (_available_list.empty())
        return nullptr;

    // take the first object in available list (_available_list.begin()), pull
    // it out of the available list, and splice it into the active list before
    // the first object (_objects.before_begin())
    _objects.splice_after(_objects.before_begin(), _available_list, _available_list.begin()).

    return &(_objects.front());
}

And that makes getObject() the same as before:
auto getObject() -> T*
{
    if (auto p = getAvailableObject(); p)
        return p;

    return newObject();
}

What about releaseObject()? Also simple. All we have to do is find the object in the list, then splice it out of _objects and into _available_list:
auto releaseObject(T const* p) -> void
{
    auto it = std::find_if(_objects.begin(), _objects.end(), [p](auto&& o) { return &o == p; });
    if (it != _objects.end())
    {
        _available_list.splice_after(_available_list.before_begin(), _objects, it);
    }
    else
    {
        // tried to release an object that wasn't active in the pool
        //
        // report an error?
    }
}

As for destroy(), it’s completely unnecessary. If you really want to check for any in-use objects when destroying or clearing the pool, you just need to see if _objects is not empty.
This isn’t the only alternative design, and may not even be the best. (The best, if you want a fixed-size object pool, might be to allocate a raw buffer and construct objects in-place as needed, with a bitset to keep track of which positions are available. If you want a growable pool, then maybe the same but with multiple buffers, added as needed… but a growable pool generally makes little sense.) But this design is simple, efficient, and easy to extend. For example, you could pre-allocate a bunch of objects (just create them in _available_objects). You could compact the pool (just clear _available_objects). And if you wanted locality, you could use a std::pmr::monotonic_buffer_resource.
This satisfies your 4 pool requirements:

“Create a pool”: auto pool = ObjectPool<T>{};
“Allocate an object from this pool”: auto* p_obj = pool.getObject();
“Deallocate an object from this pool”: pool.releaseObject(p_obj);
“Destroy this pool”: /* nothing */ (automatic)

Your main() becomes:
int main() {
    auto pool = ObjectPool<int>{};

    auto int1 = pool.getObject();
    auto int2 = pool.getObject();
    auto int3 = pool.getObject();

    pool.releaseObject(int1);

    auto int4 = pool.getObject();

    // pool will now contain 3 objects: int2, int3, and int4
    //
    // int1 will have been released, and replaced with int4

    // the rest is no longer necessary
    // integer::destroy();
    // return 0;
}

As for the question… I don’t understand the confusion. You could already pool all the types you listed even with your original code.


Dictionary algorithm, computing all case options I am building this algorithm as part of a larger project for a network security class. The gist of this part is that I have been given a dictionary, and I need to find every possible option for a word in both upper and lowercase.

My teacher selected a word out of this dictionary, and messed with the case of a few random letters in it, then created the SHA-256 hash value for it. We have been given the hash value and now have to find the word.

I figured I would create two new dictionaries, use the first one to create all possible options of the upper lowercase combinations, then use that second one to create a third with all the hash values.

Example:

to
tO
To
TO

My first thought was that it looked a lot like counting in binary, and that's what I have based this algorithm off of. The only problem is that it's pretty slow.

   public class DictionaryBuilder {

private Scanner in;
private PrintWriter out;

public DictionaryBuilder() {

}

public File createHashDictionary(File dictionary) throws IOException {
    File hashDict = new File("HashDictionary.txt");
    in = new Scanner(dictionary);
    out = new PrintWriter(hashDict);
    String word;
    String wordList[];

    while (in.hasNext()) {
        wordList = getSmallWordList(in.nextLine());
        for (int i = 0; i < wordList.length; i++) {
            out.println(wordList[i]);
        }

    }
    in.close();
    out.close();
    return hashDict;
}

public String[] getSmallWordList(String _word) {
    System.out.println(_word);
    char[] word = _word.toCharArray();

    int length = (int) (Math.pow(2, _word.length()));

    char[][] binaryList = new char[word.length][length];

    int weirdCount = length / 2;

    for (int i = 0; i < word.length; i++) {
        //System.out.println();
        int count = 0;
        int onOff = 1;

        for (int n = 0; n < length; n++) {
            if (count == weirdCount) {
                onOff++;
                count = 0;
            }
            if ((onOff % 2 == 0)) {
                binaryList[i][n] = Character.toLowerCase(word[i]);
                System.out.print(binaryList[i][n]);
                count++;
            } else {
                binaryList[i][n] = Character.toUpperCase(word[i]);
                //System.out.print(binaryList[i][n]);
                count++;
            }
        }
        weirdCount = weirdCount / 2;
    }

    //System.out.println();
    //System.out.println();
    String wordList[] = new String[length];

    for (int i = 0; i < wordList.length; i++) {
        wordList[i] = "";
        for (int n = 0; n < word.length; n++) {
            wordList[i] = wordList[i] + "" + binaryList[n][i];

        }
        if(i%10==0){
        System.out.println(wordList[i]);}
    }

    return wordList;
}

Main method:

    public class Launch {

/**
 * @param args the command line arguments
 */
public static void main(String[] args) throws NoSuchAlgorithmException, IOException {

    File dictionary = new File("Dictionary");

    DictionaryBuilder build = new DictionaryBuilder();
    File x = build.createHashDictionary(dictionary);

   Scanner in = new Scanner(x);
   System.out.println(in.nextLine());

}

It's stuck on "Antidisestablishmentarian." Can I streamline this?

You are absolutely correct - it is binary counting. In the case of antidiseastablismnetarianism you'd need to produce \$2^{28}\$ words, that is quite a few. No surprise you are stuck, but unfortunately there is no real way around.

You may streamline the word generation a bit (most likely it wouldn't affect the bottom line) by doing a more natural binary counting. For example,

    int length = 1 << word.length;

    for (int counter = 0; counter < length; ++counter) {
        for (int pos = 0; pos < word.length; ++pos) {
            binaryList[pos][counter] = (counter & (1 << pos))?
                toUpper(word[pos]): toLower(word[pos]);
        }
    }


What would be preferred aesthetically and performance wise? Which one of these two would you prefer writing in your code?

This:

tweet = tweet.replaceAll("@\\w+|#\\w+|\\bRT\\b", "");

tweet = tweet.replaceAll("\n", " ");

tweet = tweet.replaceAll("[^\\p{L}\\p{N} ]+", " ");

tweet = tweet.replaceAll(" +", " ").trim();

Or this:

tweet = tweet.replaceAll("@\\w+|#\\w+|\\bRT\\b", "")
                .replaceAll("\n", " ")
                .replaceAll("[^\\p{L}\\p{N} ]+", " ")
                .replaceAll(" +", " ")
                .trim();

Which one looks cleaner (in my opinion the second one but it obstructs you from commenting every line if you wanted to) and which one should perform better? (my guess is that there is no difference since regex doesn't create a new string everytime and it does everything internally if I am correct)

Additional Information:

Using one .replaceAll() is not possible because if the content that I want to be removed is not removed in this order then the output will be wrong.

Here is a short compilable example:

public class StringTest {

    public static void main(String args[]) {
        String text = "RT @AshStewart09: Vote for Lady Gaga for \"Best Fans\""
                + " at iHeart Awards\n"
                + "\n"
                + "RT!!\n"
                + "\n"
                + "My vote for #FanArmy goes to #LittleMonsters #iHeartAwards\n"
                + "for ART!"
                + "htt… è, é, ê, ë asdf324 ah_";
        System.out.println("Before: " + text + "\n");

        text = text.replaceAll("@\\w+|#\\w+|\\bRT\\b", "")
                .replaceAll("\n", " ")
                .replaceAll("[^\\p{L}\\p{N} ]+", " ")
                .replaceAll(" +", " ").trim();

        System.out.println("After: " + text + "\n");
    }
}

I have tried merging some of the .replaceAll() but the output was always wrong if I did the operations in any other order than this. In the end I want to be left with just the words of the tweet and nothing else. Bare in mind that this is the first time I'm using regex so I am far from a pro at it so if there actually is a way to merge them then do tell.

This is the closest I'm currently at merging the replaceAll():

text = text
    .replaceAll("@\\w+|#\\w+|\\bRT\\b|\n|[^\\p{L}\\p{N} ]+", " ")
    .replaceAll(" +", " ")
    .trim();

I basically made everything a space and then removed all the extra spaces. Is that better than before? Is it even possible to merge the last replaceAll as well?

I agree mostly with @Vogel612's answer, the second version is more concise and there is no need to litter variables all over the place as is done in the first version.

Another point is that code should be self-documenting, which @Vogel612 also suggests, but else you can still add a comment behind the code lines.

My review

I wanted to talk about a more interesting point, as this question seems to be a subproblem of a bigger question of yours, which involves creating a high throughput program that does something with tweets.
Perhaps this is a premature optimization given that I do not know how well your program performs, but your program may very well hit a threshold limit because of the way you handle regexes.

You should consider what a String.replaceAll does:

public String replaceAll(String regex, String replacement) {
    return Pattern.compile(regex).matcher(this).replaceAll(replacement);
}

For every provided regex it creates a pattern and matches and replaces the occurences with the replacement.
It is also important to look into Pattern.replaceAll now:

public String replaceAll(String replacement) {
    reset();
    boolean result = find();
    if (result) {
        StringBuffer sb = new StringBuffer();
        do {
            appendReplacement(sb, replacement);
            result = find();
        } while (result);
        appendTail(sb);
        return sb.toString();
    }
    return text.toString();
}

Here you can see, that because of four regex replaces, you create four times a StringBuffer, which is synchronized, and a String object, which can count up if you are processing tons of tweets.

There are two things you can do:

Create a single regex that captures what you want to do, thus needing only one replaceAll call.
Go really into micro-optimizing and get rid of the regexes in a whole, operate on a char[] and only store the resulting character array.


JavaScript Graphing Calculator I have a pretty functional JavaScript based graphing calculator.

Functions like \$\space tan(x)\$, \$\space\frac{1}{tan(x)}\space\$ and \$\space x^{high\space number}\space\$ look weird, however because the interval it graphs isn't small enough. If I make that interval any smaller to fix the gaps though, it severely impacts its performance.

Is there a more efficient way to do the plotting?

Here is the pertinent code. It draws rectangles on an HTML canvas. The page of the complete version can be found here.

var mathjs = require('mathjs');
//JavaScript parser that evaluates the user input
var canvas = document.getElementById("graph");
var ctx = canvas.getContext("2d");

var width = canvas.width;
var height = canvas.height;
var center = width/2;

function plot(){
    var f = mathjs.eval('f(x) = '+($("#function").val()));
    try{
        for(var x = -250;x<250;x+=0.001){
            //"x+=0.001" is the interval in question
            ctx.fillRect(center+x,center-f(x),0.1,0.1);
        }
    }
    catch(e){
        throw new Error("Syntax error or invalid function sent to mathjs.");
    }
}

Increase the interval but shrink the view

Because you're plotting points you can't just increase the number of points you have, as you know, that will take up much more processing power. To mitigate this processing power issue, you need to shrink the view of your graph to display a smaller area. This has the effect of zooming in.

If you were using a more advanced way to draw the functions (i.e. estimating the area between points and drawing curves) you would most likely not have this issue, but using only points and a for loop limits you.

Another problem I barely noticed, you're overdrawing. It would seem that you're plotting past the borders of your canvas/graph. This causes unnecessary drawing that slows down everything. You should be basing the size of the area you're drawing on the width and height of the canvas.


Non brute force algorithm for Birthday Chocolate Problem: https://www.hackerrank.com/challenges/the-birthday-bar/problem

Lily has a chocolate bar that she wants to share it with Ron for his birthday. Each of the squares has an integer on it. She decides to share a contiguous segment of the bar selected such that the length of the segment matches Ron's birth month and the sum of the integers on the squares is equal to his birth day. You must determine how many ways she can divide the chocolate.

Function Description

It should return an integer denoting the number of ways Lily can divide the chocolate bar.

s: an array of integers, the numbers on each of the squares of chocolate
d: an integer, Ron's birth day
m: an integer, Ron's birth month

Print an integer denoting the total number of ways that Lily can portion her chocolate bar to share with Ron.

Example: Lily wants to give Ron  m =2 squares summing to d = 3. The following two segments meet the criteria:

Here is my brute force solution that enumerates all possibilities:

// Complete the birthday function below.
static int birthday(List<int> s, int d, int m) {
    int n = 0;
    for(int i = 0; i < s.Count; i++) {
        if(s.Skip(i).Take(m).Sum() == d)
            n++;
    }
    return n;
}

Question: any non brute-force solutions to this problem?

You basically always need to use brute force here. There is no other way. However, you can do it a bit more efficiently.

You're redoing the whole summation in every step of the loop. You don't need to. You can compute the total with one subtraction and one addition in each step of the loop. Like this:

static int birthday(List<int> s, int d, int m) {
    int n = 0;
    int t = s.Take(m).Sum();
    for(int i = 1; i < s.Count - m; i++) {
        if (t == d) n++;
        t = t - s[i - 1] + s[i + m];
    }
    return n;
}

I hope this makes sense, I never write C#. I got here because of your 'javascript' tag. The code might not work correctly, it is untested, but I hope you understand the intention.

However, I do think there's a bug in your code. You loop over the whole list, all the way to the end, where you cannot take m squares anymore. The sum() there might result in unexpected matches.


Find the middle of the string The follow code should correctly run the test cases at the bottom.

It works correctly.
I just don't feel like this is clearest way I could write it.

A lot of the complexity comes from having to return the middle two characters if the length of the string is even.

Here is a link to the Programing Challenge.

(defn convert-to-zero-based-list [index]
  (- index 1))

(defn convert-to-one-based-list [index]
  (+ index 1))

(defn get-middle-with-offset [s]
    (/ (convert-to-one-based-list (count s)) 2))

(defn get-start-of-middle [s]
    (convert-to-zero-based-list (Math/floor (get-middle-with-offset s))))

(defn get-end-of-middle [s]
    (Math/ceil (get-middle-with-offset s)))

(defn get-middle [s]
    (subs s (get-start-of-middle s) (get-end-of-middle s)))

(= (get-middle "a") "a")
(= (get-middle "aa") "aa")
(= (get-middle "aba") "b")
(= (get-middle "abba") "bb")
(= (get-middle "abcba") "c")

An alternative would be to recursively/iteratively trim off the first and last elements of the string until you reached the middle one or two characters.

(defn trim-first-and-last [s]
  (subs s 1 (dec (count s)))
)

(defn get-middle-recur [s]
  (if (<= (count s) 2)
      s
      (get-middle-recur (trim-first-and-last s))
  )
)

(= (get-middle-recur "a") "a")
(= (get-middle-recur "aa") "aa")
(= (get-middle-recur "aba") "b")
(= (get-middle-recur "abba") "bb")
(= (get-middle-recur "abcba") "c")
(= (get-middle-recur "abccba") "cc")