Binary tree (S-Expression) pretty printer I'm just trying to create a binary tree, and a pretty printer for it:

#include <stdio.h>

typedef unsigned long Leaf;

struct NodeStruct {
    struct TreeStruct * left;
    struct TreeStruct * right;
};
typedef struct NodeStruct Node;

struct TreeStruct {
    char isNode;
    union {
        Node node;
        Leaf leaf;
    } value;
};
typedef struct TreeStruct Tree;

Tree node(Tree* left, Tree* right){
    Tree result;
    result.isNode = 1;
    result.value.node.left = left;
    result.value.node.right = right;
    return result;
};

Tree leaf(unsigned long x){
    Tree result;
    result.isNode = 0;
    result.value.leaf = x;
    return result;
};

void print(Tree* tree, int isRoot){
  if (!tree->isNode){
        printf("%lu",tree->value.leaf);
        return;
    };
    printf("(");
    print(tree->value.node.left,0);
    printf(" ");
    print(tree->value.node.right,0);
    printf(")");
};

int main(){
    Tree a = leaf(1);
    Tree b = leaf(2);
    Tree c = leaf(3);
    Tree d = leaf(4);
    Tree ab = node(&a,&b);
    Tree cd = node(&c,&d);
    Tree abcd = node(&ab,&cd);
    print(&abcd,1);
};

There are so many things that sound wrong to me, yet I don't know how I'm supposed to improve this in C. Ideas?

Also, what if I wanted to return the string instead of printing it?

Disclaimer: I am a C++ coder, but I researched to make sure none of that made it into this advise.

It looks alright, there are a couple little things.

First, explicitly state that main() takes no arguments.  This is applicable to all functions in C:

int main(void)

leaf should take an argument of Leaf rather than unsigned long, this means that you will only have one line to change if you ever want your tree to hold something else.

Tree leaf(Leaf x)

Either use <stdbool.h> if on a C99 compiler, or explicitly define true and false rather than just using 1 and 0. This will make your code more self documenting.

#include <stdbool.h>

#define true 1
#define false 0

Again, if using <stdbool.h>, use the bool type or roll your own:

typedef int bool

If you are using C11 then you can use anonymous unions to simplify your TreeStruct.  It will also mean you can remove the now superfluous .value throughout your code.

struct TreeStruct
{
    bool isNode;
    union
    {
        Node node;
        Leaf leaf;
    };
};

You don't actually use the isRoot value passed to print, but it can be used to print a newline at the end.

void print(Tree * tree, bool isRoot)
{
    if (!tree->isNode)
    {
        printf("%lu", tree->leaf);
        return;
    };

    printf("(");
    print(tree->node.left, false);
    printf(" ");
    print(tree->node.right, false);
    printf(")");

    if (isRoot)
    {
        printf("\n");
    }
};

As a matter of personal preference, I like to space things out a bit more, as you can see above.


PS1 for Bash prompt showing last exit status, written mostly in Python I had most of this functionality accomplished in BASH, but I was having trouble getting the last process's return value (even though I had a minimal working example). Plus, the escape codes made everything unreadable and I wasn't sure how to factor them out appropriately.

Since I know Python pretty well, I decided to rewrite it in Python, since I have now figured out enough shell that I felt confident embedding some Python into it.

The __future__ import should allow this to work in Python 2 or 3.

I do not know if this is cross-compatible with other shells, or if this is only likely to work in BASH, but another driver that pushed me in this direction was that desire. I am aware of the PROMPT_COMMAND for BASH.

Perhaps I could use a  similar technique along with a BASH function to keep it all in shell and avoid the Python process altogether. However, I find this to be incredibly easy to read, work on, and modify without fear of breaking it - especially relative to shell scripting.

Another perhaps: maybe these escape codes are already available in the Python standard library. I looked at ncurses - I don't think that's the one I want to focus on.

I'm not sure if I factored the escape codes properly - perhaps I should have the constants point to the formatted escapes instead. But theoretically I could embed multiple codes into a single escape, instead of doing it multiple times for every tweak. Maybe there's a drawback to doing that my way, too.

PS1="\$(export LAST_RETURN_VALUE="\$?" WHOAMI="\$\(whoami\)" HOSTNAME ; python -c \"
from __future__ import print_function
import os
import datetime

ESC = '\033[{0}m'
esc = ESC.format

OFF, BOLD, UNDERSCORE, BLINK = 0, 1, 4, 5
BLACK, BLACKBG = 30, 40
RED, REDBG = 31, 41
GREEN, GREENBG = 32, 42
YELLOW, YELLOWBG = 33, 43
BLUE, BLUEBG = 34, 44
MAGENTA, MAGENTABG = 35, 45
CYAN, CYANBG = 36, 46
WHITE, WHITEBG = 37, 47

LAST_RET = os.environ['LAST_RETURN_VALUE']

print(esc(YELLOW) + datetime.datetime.now().strftime('%Y-%m-%d %H:%M:%S'),
      esc(GREEN) + os.environ['WHOAMI'] + esc(RED) + '@' +
      esc(CYAN) + os.environ['HOSTNAME'],
      esc(BLUE) + esc(BOLD) + os.getcwd() +
      esc(OFF))
if LAST_RET != '0':
    print(esc(MAGENTABG) + LAST_RET + esc(OFF), end='')
print('$ ', end='')
\" "

I'm getting impatient and want to go ahead and post this. I usually do more due diligence on code I write, but I'm up way past my bedtime already, and I think this is too clever not to share.

Here's the result:

The typo

If we look at the beginning and the of the posted code:

PS1="\$(export LAST_RETURN_VALUE="\$?" WHOAMI="\$\(whoami\)" HOSTNAME ; python -c \"
from __future__ import print_function
... more python code
\" "

The end part is incorrect.
The \" closes the python -c \",
and the final " closes the PS1=",
but a ) is missing to close the \$(export.
(Pro tip: if you edit the script in vim, and press % on an opening (, it shows where's the closer, or emits an error beep if missing.)

I guess the last line was meant to be:

\")"

An easier way to escape stuff

Escaping things correctly within "..." can be quite painful in very long strings as in this example.
In this particular example there's easy alternative using literal here documents:

PS1=$(cat << "EOF"
...
EOF
)

All the code you previously had within the "..." could go there in place of the ..., and it will be taken literally as if enclosed within single-quotes, but no need to escape ", or ' or $.

PS1=$(cat << "EOF"
$(export LAST_RETURN_VALUE="$?" WHOAMI="$(whoami)" HOSTNAME ; python -c "
from __future__ import print_function
import os
import datetime

ESC = '\033[{0}m'
esc = ESC.format

OFF, BOLD, UNDERSCORE, BLINK = 0, 1, 4, 5
BLACK, BLACKBG = 30, 40
RED, REDBG = 31, 41
GREEN, GREENBG = 32, 42
YELLOW, YELLOWBG = 33, 43
BLUE, BLUEBG = 34, 44
MAGENTA, MAGENTABG = 35, 45
CYAN, CYANBG = 36, 46
WHITE, WHITEBG = 37, 47

LAST_RET = os.environ['LAST_RETURN_VALUE']

print(esc(YELLOW) + datetime.datetime.now().strftime('%Y-%m-%d %H:%M:%S'),
      esc(GREEN) + os.environ['WHOAMI'] + esc(RED) + '@' +
      esc(CYAN) + os.environ['HOSTNAME'],
      esc(BLUE) + esc(BOLD) + os.getcwd())
if LAST_RET != '0':
    print(esc(OFF) + esc(MAGENTABG) + LAST_RET + esc(OFF), end='')
print(esc(OFF) + '$ ', end='')
"
)
EOF
)

Safety

If PROMPT_COMMAND is set, PS1 will be ignored.
To avoid confusion, I would add unset PROMPT_COMMAND.

What's wrong with doing this in Python?

It's an additional dependency to worry about.
If your friend asks you how to do that cool prompt,
it gets less cool when you explain how he needs Python to get it.
And running a Python script for every single command is a bit of a waste.

Why not do it in Bash?

The script basically sets variables, formats the date and prints stuff.
That's easy enough to do in Bash.
The script gets all its critical elements from Bash.
It uses the Bash variable $? to get the exit code of the last command,
the whoami command to get the current user,
and the HOSTNAME variable that's already defined in the environment.

In other words,
you already know everything it takes to make this work in Bash,
and Python doesn't bring real benefits,
the only thing left to do is to translate it to Bash.

This is pretty much a word-for-word translation of Python to Bash,
with some minor improvements, such as \u instead of calling whoami,
and unused variables dropped.
I don't see the readability suffering significantly compared to the Python solution.

PS1=$(cat << "EOF"
$(
LAST_RET=$?
WHOAMI='\u'
HOSTNAME='\h'
DATE=$(date +'%Y-%m-%d %H:%M:%S')

OFF='\033[0m'
BOLD='\033[1m'
RED='\033[31m'
GREEN='\033[32m'
YELLOW='\033[33m'
BLUE='\033[34m'
MAGENTABG='\033[45m'
CYAN='\033[36m'

echo "$YELLOW$DATE $GREEN$WHOAMI$RED@$CYAN$HOSTNAME $BLUE$BOLD$PWD"
test $LAST_RET = 0 || printf "$OFF$MAGENTABG$LAST_RET$OFF"
printf "$OFF\$ "
)
EOF
)


Redirecting to the appropriate URL for some type of notification I have a polymorphic notification model with action attribute. When user clicks on a notification he/she should be redirected to given page the notification refers to through notifiable.

What is the best way to refactor the following method?

_notification.html.erb

<%= link_to checking_decreasing_user_notifications_path(current_user, notification: notification) do %>

notifications controller

def checking_decreasing
  notification = Notification.find(params[:notification])
  current_user.decreasing_comment_notification_number(notification)
  redirect_to notification_redirection_path(notification)
end

def notification_redirection_path(notification)
  notifiable_type = notification.notifiable_type
  notifiable_id = notification.notifiable_id
  action = notification.action
  if action == "commented"
    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"
              product_lead = ProductLead.find(notifiable_id)
              product_id = product_lead.product_id
            when "ProductCustomer"
              product_customer = ProductCustomer.find(notifiable_id)
              product_id = product_customer.product_id
              product_product_customer_path(product_id, notifiable_id, anchor: "comment-panel") #/products/#{product_id}/#{notifiable_type}/#{notifiable_id}#comment-panel"
            end
  elsif action == "invited"
    if notifiable_type == "Product"
      product_path(notifiable_id, anchor: "product-invitation-well")
    elsif notifiable_type == "ProductCustomer"
      product_customer = ProductCustomer.find(notifiable_id)
      product_id = product_customer.product_id
      product_product_customer_path(product_id, notifiable_id)
    end
  elsif action == "accepted"
    if notifiable_type == "Product" #team member invitation
      product_product_owner_panels_path(notifiable_id)
    elsif notifiable_type == "ProductCustomer" #referencer invitation
      product_customer = ProductCustomer.find(notifiable_id)
      product_id = product_customer.product_id
      product_product_owner_panels_path(product_id)
    end
  elsif action == "wrote"
    product_customer = ProductCustomer.find(notifiable_id)
    product_id = product_customer.product_id
    product_product_customer_path(product_id, notifiable_id)
  end
end

Suggestions:

Break long methods into simple parts;
Avoid unnecessary local variables;
Use single quotes for strings without interpolation;
Move repeated parts of code into separate methods.

Code:

def notification_redirection_path(notification)
  notifiable_type = notification.notifiable_type
  notifiable_id = notification.notifiable_id

  case notification.action
  when 'commented'
    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(*lead_params(notifiable_id), anchor: 'comment-panel')#{}"/products/#{product_id}/#{notifiable_type}/#{notifiable_id}#comment-panel"
    when 'ProductCustomer'
      product_product_customer_path(*customer_params(notifiable_id), anchor: 'comment-panel') #/products/#{product_id}/#{notifiable_type}/#{notifiable_id}#comment-panel"
    end
  when 'invited'
    if notifiable_type == 'Product'
      product_path(notifiable_id, anchor: 'product-invitation-well')
    elsif notifiable_type == 'ProductCustomer'
      product_product_customer_path(*customer_params(notifiable_id))
    end
  when 'accepted'
    if notifiable_type == 'Product' #team member invitation
      product_product_owner_panels_path(notifiable_id)
    elsif notifiable_type == 'ProductCustomer' #referencer invitation
      product_product_owner_panels_path(*customer_params(notifiable_id).first)
    end
  when 'wrote'
    product_product_customer_path(*customer_params(notifiable_id))
  end
end

private

def customer_params(notifiable_id)
  [ProductCustomer.find(notifiable_id).product_id, notifiable_id]
end

def lead_params(notifiable_id)
  [ProductLead.find(notifiable_id).product_id, notifiable_id]
end


Total beginner Tic Tac Toe game in Python I've decided to learn Python about 2 weeks ago, been going through various books and videos, and I've decided to try my hand at programming a Tic Tac Toe game. I was somewhat successful (it doesn't recognize if there's already a mark in a certain spot and allows overwriting of already placed marks) and I was wondering if any more experienced programmers could give me some general feedback about how I could do things better, and how to prevent overwriting of previously placed mark.

board = ['-'] * 9

def print_board():
    print (board[0] + '|' + board[1] + '|' + board[2])
    print (board[3] + '|' + board[4] + '|' + board[5])
    print (board[6] + '|' + board[7] + '|' + board[8])

legalMoves = [1,2,3,4,5,6,7,8,9]
print_board()

turnCount = 0
def move():

    move = int(input('Pick a number 1-9:'))
    while move not in legalMoves:
        print('Illegal move')
        move = int(input('Pick a number 1-9:'))

    marks = ['X','O']

    if turnCount % 2 == 0:
        board[move - 1] = marks[1]
    else:
        board[move - 1] = marks[0]

while True:
    if board[0] == board[1] == board[2] == 'X'\
    or board[3] == board[4] == board[5] == 'X'\
    or board[6] == board[7] == board[8] == 'X'\
    or board[0] == board[3] == board[6] == 'X'\
    or board[1] == board[4] == board[7] == 'X'\
    or board[2] == board[5] == board[8] == 'X'\
    or board[0] == board[4] == board[8] == 'X'\
    or board[2] == board[4] == board[6] == 'X' \
    or board[0] == board[1] == board[2] == 'O' \
    or board[3] == board[4] == board[5] == 'O' \
    or board[6] == board[7] == board[8] == 'O' \
    or board[0] == board[3] == board[6] == 'O' \
    or board[1] == board[4] == board[7] == 'O' \
    or board[2] == board[5] == board[8] == 'O' \
    or board[0] == board[4] == board[8] == 'O':
        print('Victory')
        break

    else:
        move()
        print_board()
        turnCount = turnCount + 1

Here are my notes:

your printBoard function has code repetition, you can see there is a pattern 0 1 2, 3 4 5, 6 7 8, so  you can do a for loop, example:

for i in range(3):
  print (board[i * 3] + '|' + board[i * 3 + 1] + '|' + board[i * 3 + 2])

your legalMoves list is not required, because it's just numbers from 1 to 9, so you can check against that example: if n >= 1 and n <= 9: #accepted

"move = int(input('Pick a number 1-9:'))", you need to check for the input first if it can be converted to int or not to prevent the user from breaking your program if non int string value was typed, and to do that you can store the input in a string then check against it to be an int or not example:

def isInt(strInt):
  for c in strInt:
    if c not in "0123456789": return False
  return True

n = input()
if isInt(n): move = int(n)

and your checks inside the while loop can be shortened to using for loop instead, you can see a pattern in those cells positions, so if you want to check horizontally, yoou need to do, example:

for i in range(3):
  if board[i * 3] == board[i * 3 + 1] == board[i * 3 + 2] and board[i * 3] != "-":
    # do something

and vertically

for i in range(3):
  if board[i] == board[i + 3] == board[i + 6] and board[i] != "-":
    # do something

and diagonally

  if (board[0] == board[4] == board[8] or board[2] == board[4] == board[6]) and board[4] != "-":

without checking for "X" or "O", you just need to check for "-"

you need also to prevent the user from overwriting an already filled cell, example

if board[move - 1] != "-":
  print("Pls choose an empty cell")

you need also to check for tie, because not all time you get a winner example if your main loop I mean the one for each user correct move which are 9 moves, if it reaches the end and there is no winner then it's a tie, and you should let the user know.

That's it, you can improve your code now, I hope you like my feedBack, also I have written a full solution, you can find it in your question in Stack Overflow, and if you have any question, I will be happy to answer.


Produce a nearly sorted (or K sorted) array Given an array of n elements, where each element is at most k away from its target position, devise an algorithm that sorts in O(n log k) time.
For example, let us consider k is 2, an element at index 7 in the sorted array, can be at indexes 5, 6, 7, 8, 9 in the given array. I'm looking for code review, best practices, optimizations etc. Also for some reason I could not get assertarrayequals hooked up so tested arrays unconventionally. Please ignore that as part of feedback

public final class KSortedArray {

    private KSortedArray() { }

    /**
     * Returns the sorted array provided the input array is k-sorted.
     * If input array is not k-sorted, then results are unpredictable.
     *
     * @param arr   The k-sorted array
     * @param k     the value of k, the deviation of placement.
     * @return      the sorted array
     */
    public static int[] kSortDontModifyInput(int[] arr, int k) {
        int[] n = new int[arr.length];

        final Queue<Integer> queue = new PriorityQueue<Integer>(k + 1);
        for (int  i = 0; i <= k; i++) {
            queue.add(arr[i]);
        }

        int ctr = 0;
        for (int i = k + 1; i < arr.length; i++) {
            n[ctr++] = queue.poll();
            queue.add(arr[i]);
        }
        while (!queue.isEmpty()) {
            n[ctr++] = queue.poll();
        }
        return n;
    }

    /**
     * Sorted array provided the input array is k-sorted.
     * If input array is not k-sorted, then results are unpredictable.
     *
     * @param arr   The k-sorted array
     * @param k     the value of k, the deviation of placement.
     */
    public static void kSortMoidifyInput(int[] arr, int k) {
        Queue<Integer> queue = new PriorityQueue<Integer>(k + 1);
        for (int  i = 0; i <= k; i++) {
            queue.add(arr[i]);
        }
        int ctr = 0;
        for (int i = k + 1; i < arr.length; i++) {
            arr[ctr++] = queue.poll();
            queue.add(arr[i]);
        }

        while (!queue.isEmpty()) {
            arr[ctr++] = queue.poll();
        }
    }

    public static void main(String[] args) {
        int arr[] = {2, 6, 3, 12, 56, 8};

        int[] expected = {2, 3, 6, 8, 12, 56};
        int[] actual = kSortDontModifyInput(arr, 3);
        kSortMoidifyInput(arr, 3);
        for (int i = 0; i < expected.length; i++) {
            Assert.assertEquals(expected[i], actual[i]);
            Assert.assertEquals(expected[i], arr[i]);
        }
    }
}

When you use a class in a 'hacky' way, like you do by using a PriorityQueue as a TreeSet, you should make sure that you document why the class is used, and what properties of the class are being leveraged.

Your code does not work in O(n log(k) ) time because it uses a PriorityQueue, which has O( log(n) ) time-complexity for add():

  Implementation note: this implementation provides O(log(n)) time for the enqueing and dequeing methods (offer, poll, remove() and add); linear time for the remove(Object) and contains(Object) methods; and constant time for the retrieval methods (peek, element, and size)

Your algorithm is not correct for the requirements given:

For a start, it will fail for input where the input array is smaller than k. It will throw an ArrayIndexOutOfBoundsException.
Secondly, you are working in k+1 space instead of k. Why? Where is the comment?

Further, because you auto-box all your values to Integer, from int, you have a significant performance penalty. If you keep your data as primitives (and use an array of primitives rather than a PriorityQueue), you will have better results.

The algorithm you need you use is strongly hinted at by the complexity requirement...

O( n log(k) ) strongly implies that you need to iterate over each value once, and, with that element, there is an O(log(k)) way to sort it.

For the loop, think a for-loop. For the log(k), think a binary search....

for (int i = 0; i < data.length; i++) {
    int from = i > k ? i - k : 0;
    int val = data[i];
    int pos = Arrays.binarySearch(data, from, i, val);
    if (pos < 0) {
        pos = -pos - 1;
    }
    System.arraycopy(data, pos, data, pos+1, i - pos - 1);
    data[pos] = val;
}


Function that abbreviates a number (e.g. 1202 => 1.2K) Like Twitter and Instagram and others I wanted to display numbers like 1.2K and 3.8M etc. My function works well enough but I would appreciate any input you may have.

function abbrNum (num) {
    if(typeof num !== 'number') {
        throw new TypeError('Expected a number');
    }

    var shortNumber;
    var exponent;
    var suffixes = ['K', 'M', 'B', 'T'];
    var size = (num + '').length;

    exponent = size % 3 === 0 ? size - 3 : size - (size % 3);

    if(num < 1000) {
        return num;
    } else {
        shortNumber = Math.round(10 * (num / Math.pow(10, exponent))) / 10;
    }

    if(exponent < 6) {
        shortNumber += suffixes[0];
    } else if(exponent < 9) {
        shortNumber += suffixes[1];
    } else if(exponent < 12) {
        shortNumber += suffixes[2];
    } else if(exponent < 16) {
        shortNumber += suffixes[3];
    }

    return shortNumber;
}

Update: This code (updated according to feedback) is available on npm.

converting to string just to get the length won't work if there is a decimal or it's large enough that the string conversion uses scientific notation.

var size = floor(log(num)/log(10))+1;

This will give the place of the highest significant digit (1 to 9 results in 1, 10 to 99 results in 2, 100 to 999 results in 3, etc...)


Different Constructors, Same Implementation I have this class containing two constructors with different signatures but they do the same thing:

public Person(Dictionary dictionary, string someString)
    : base(dictionary, someString)
{
    base.GetProperty("FirstName");
    base.GetProperty("LastName");
}

public Person(Dictionary dictionary, string[] someStringArray)
    : base(dictionary, someStringArray)
{
    base.GetProperty("FirstName");
    base.GetProperty("LastName");
}

The type of the second parameter in each of these constructors determines the behavior of the 'GetProperty' method.

I've read about casting and the bad behaviors of it, but would it be appropriate to do something like  this:

public Person(Dictionary dictionary, Object something)
{
}

and then cast something to either a string or string[] depending on whichever is appropriate?

Two things your could do:

Refactor the common code into a method (like Initialize) and call that:

private void Initialize()
{
    base.GetProperty("FirstName");
    base.GetProperty("LastName");
}

public Person(Dictionary dictionary, string someString)
    : base(dictionary, someString)
{
    Initialize();
}

public Person(Dictionary dictionary, string[] someStringArray)
    : base(dictionary, someStringArray)
{
    Initialize();
}

Not sure whether that's an option as I don't know how the behaviour changes but you could reduce one case to the other:

public Person(Dictionary dictionary, string someString)
    : this(dictionary, new [] { someString })
{
}

public Person(Dictionary dictionary, string[] someStringArray)
    : base(dictionary, someStringArray)
{
    base.GetProperty("FirstName");
    base.GetProperty("LastName");
}

Apart from that:

I'd avoid the casting. Apparently only strings or arrays of strings make sense to be passed in. If you change the parameter to object then it is no longer clear to the caller what he can and cannot pass in and would have to write a test to make sure that what he is passing in will be accepted. It reduces the clarity of the interface.
GetProperty seems to be a strange thing to call in a constructor. I'd expect it to return a property value yet you do nothing with the return value.
Consider making "FirstName" and "LastName" (and any other property string you use) string constants rather than literals - especially if you use them in more than one place. Lambda expressions might be an option if they are actual properties on the object.


Replacing occurrences in NSString and NSMutableString I ran it and it's working, but I just want someone to double check that I followed the directions. Any suggestions or corrections will be appreciated.

  Assume you have a NSString *str object containing some text. Write a code fragment to create a new string with all substrings "aaa" deleted.

My answer:

NSString *str = @"aaa 123 yes there will be aaaa even more a y a y aaa, ";

NSString *str2 = [str stringByReplacingOccurrencesOfString:@"aaa" withString:@""];

NSLog(@"its %@", str2);

  Redo when this is NSMutableString *str, without creating a new object.

My answer:

NSMutableString *string1 = [NSMutableString stringWithString: @"aaa 123 yes there aaaa even more a y a y aaa,"];

[string1 replaceOccurrencesOfString:@"aaa" withString:@"" options:0 range:NSMakeRange(0, string1.length)];
NSLog (@"string1 = %@", string1);
    }

In both cases, this is exactly the most efficient way for removing a substring from a string.

In practice, I'm not sure how practical or how frequently you'll truly need to be using the second example, and for most of us, simply doing this:

string1 = [[string1 stringByReplacingOccurrencesOfString:@"aaa" withString:@""] mutableCopy];

is a bit easier to remember.  And you generally want to work with immutable objects whenever you can anyway.  But given the requirements, the method you've used does remove the substring without creating a new object.

One thing that may or may not be of concern is the white space that's left in these strings at the beginning/end.  So you may or may not want to put some effort into trimming the white space.

Also, by replacing @"aaa" with @"", that means you're also effectively replacing @"aaaa" with @"a".  Be sure this is what is intended, otherwise this problem is a bit more complicated.

You can also use regex.  Regex will be slightly less efficient, but can give you some more control over what's being replaced:

NSError *error;
NSRegularExpression *regex = [NSRegularExpression regularExpressionWithPattern:@"/\baaa\b/"
                                                                       options:nil
                                                                         error:&error];
NSString *str2 = [regex stringByReplacingMatchesInString:string
                                                 options:nil
                                                   range:NSMakeRange(0, [str length])
                                            withTemplate:@""];
NSLog(@"%@", str2);

I don't know at this moment whether there's anything similar to this available for mutating a NSMutableString object rather than creating a new object.


Number guess game program in python 3 I started learning Python just yesterday, and so I am aware of my lack of knowledge regarding the language. Below is a program I wrote to simulate a guessing game. I ended up creating a lot of ad hoc patches for some possible errors, and suspect that there are more elegant ways to write the program. I'd like pointers/assistance/an example of a "best practices" version of the program I write below:

import random, sys

sys.setrecursionlimit(10000)    #I expect any troll who's trying to fuck with the program to give up before they reach 10,000.
control = True    #global variable to control the behaviour of the 'guessNumber()' function.
errors = 0    #To store the number of errors a user triggers.

def guessNumber(maxx):
    global errors
    global control
    try:
        maxx = float(maxx)    #Incase a cheeky user decided to submit floating point numbers.
        if int(maxx) != maxx:     #If they submitted a floating point number instead of an integer.
            print("Input whole numbers only.")
            errors += 1
            guessNumber(input('Input the Maximum number: \t'))  #Call a new instance of 'guessNumber()'
        else:   #If they indeed submitted a whole number.
            maxx = int(maxx)
    except ValueError:  #They didn't submit a whole number.
        print("Input whole numbers only.")
        errors += 1
        guessNumber(input('Input the Maximum number: \t'))  #Call a new instance of 'guessNumber()'

    if not control:    #control variable. If the program has already properly evaluated, it should terminate.
        return None

    print('I am thinking of a whole number between 1 and ' + str(maxx) + ' (both inclusive).')
    num = random.randint(1, maxx)
    count = 0
    check = True
    var = None

    while check:
        print("Guess the number.")
        try:
            var = float(input())    #Incase a cheeky user decided to submit floating point numbers.
            if int(var) != var:     #If they submitted a floating point number instead of an integer.
                print("Input whole numbers only.")
                errors += 1
                continue
            else:   #If they indeed submitted a whole number.
                var = int(var)
        except ValueError:  #They didn't submit a whole number.
            print("Input whole numbers only.")
            errors += 1
            continue
        count +=  1
        if var == num:  #If they guess the number right.
            check = False
        elif var > num:
            print('Too high! \nTry a little lower.')
        elif var < num:
            print('Too low! \nTry a little higher.')

    print('Congratulations!!!\nYou guessed the number after ' + str(count) + " tries and triggering " + str(errors) + " errors.")
    control = False     #After every completed execution, we set control to false so that if the function was called from another instance of itself it terminates and doesn't cause errors.
    return None

guessNumber(input('Input the Maximum number (integers only): \t'))

I am particularly worried about the memory cost, due to using recursion to deal with errors.

Your comments describe what the code is doing, not why the code's doing what it is. And so are generally unhelpful.
Please don't use profanity in comments, or anywhere. It makes you look unprofessional, and is not really that helpful.
To hell with sys.setrecursionlimit. It just delays the inevitable, rather than fixes your problem.
Rather than using global variables, you could instead use a closure.
Your recursion should be in a separate function. That only gets user input. But it'd be better if it were a while loop.

For example you could use:

def get_user_input(fn, prompt='', exception_message=''):
    while True:
        try:
            return fn(input(prompt))
        except ValueError:
            print(exception_message)

Don't repeat yourself. You write get_user_input out twice. In different ways.
The while loop for your higher or lower game seems pretty good. Nice job on that.

And so ignoring your error part I'd use:

import random

def get_user_input(fn, prompt='', exception_message=''):
    while True:
        try:
            return fn(input(prompt))
        except ValueError:
            print(exception_message)

def main():
    limit = get_user_input(int, prompt='Input the Maximum number: \t',
                           exception_message='Input whole numbers only.')

    print('I am thinking of a whole number between 1 and {limit} (both inclusive).'.format(limit=limit))
    number = random.randint(1, limit)
    count = 0
    while True:
        print("Guess the number.")
        guess = get_user_input(int, exception_message='Input whole numbers only.')
        count += 1
        if guess == number:
            break

        if guess > number:
            print('Too high!\nTry a little lower.')
        else guess < number:
            print('Too low!\nTry a little higher.')

    print('Congratulations!!!\nYou guessed the number after {count} tries.'.format(count=count))

if __name__ == '__main__':
    main()

If errors is super important to you, you could return the amount of errors from get_user_input and add it to a local errors variable.
Say:

errors = 0
while True:
    limit, input_errors = get_user_input(...)
    errors += input_errors


Getting JSON data from an HTTP response using conditional binding I am new to Swift and I am trying to use the optional binding feature in as efficient way as possible but I am a bit confused on how to use it (if I can) in this scenario:

var jsonData:AnyObject? = nil

jsonData = try? NSJSONSerialization.JSONObjectWithData(data!, options: [])
if(jsonData == nil) {
    print("Could not parse data")
}

if(httpresponse.statusCode == 200 || httpresponse.statusCode == 201) {
    if (jsonData as? ReturnType) != nil {
        return jsonData as? ReturnType
    } else { print("Request Successful") }
} else { // Not HTTP 200 or 201
    if let dic = jsonData as? NSDictionary {
        if dic["Message"] != nil && (dic["Message"] as? String) != nil {
            let message = dic["Message"]! as! String
            print("Server returned status code '\(httpresponse.statusCode)': '\(message.trim())'")
        }
    } else {
        let message = NSHTTPURLResponse.localizedStringForStatusCode(httpresponse.statusCode)
        print("Server returned status code '\(httpresponse.statusCode)': \"\(message)\"")
    }
}

The key statements here are:

jsonData = try? NSJSONSerialization.JSONObjectWithData(data!, options: [])
if(jsonData == nil) {
    print("Could not parse data")
}

There are potentially several places where all I need to do is a print or a user message (basically run some conditional code) after an important assign/unwrap without any major redirections of the rest of the code.

I was wondering if there was a smarter way of doing what I want to do without having to follow up each of those assignments with its own if statement.

I was under the impression that if let and guard did that very thing by merging conditional statements like if and else with the assignment, thus compacting the code, but they also seem to add some restrictive scope limitations like scope of the constant in the if let and having to 'return' or 'continue' in the guard scope.

What do you think?

To answer your explicit question with what is probably an obvious answer... Wrap it up in a function.

func debugSerialize(data: NSData) -> AnyObject? {
    let jsonData = try? NSJSONSerialization.JSONObjectWithData(data, options: [])
    if(jsonData == nil) {
        print("Could not parse data")
    }
    return jsonData
}

Another option would be to let NSJSONSerialization.JSONObjectWithDatas thrown error propagate up the stack and have a centralized place to catch them and put the print there... Unless you are married to the idea that the rest of your code must run even if jsonData is nil...

Keep in mind that most of the code you have posted does nothing but print debug output. Once the debugging is complete all of that code can, and should, be removed (don't leave a bunch of debugging print statements in production code.) Once you do that, you will see that all your code really does is check to see of the JSON deserialization created a ReturnType object.

Likely, you need this code to do more than produce debug output on bad server responses, but I'm not able to review hypothetical code that you could have written.


How to optimize this majority vote method I have the following code to do a majority vote for data in a dataframe:
def vote(df, systems):
    test = df.drop_duplicates(subset=['begin', 'end', 'case', 'system'])
    n = int(len(systems)/2)
    data = []

    for row in test.itertuples():
        # get all matches
        fx = test.loc[(test.begin == row.begin) & (test.end == row.end) & (test.case == row.case)]
        fx = fx.loc[fx.system.isin(systems)]

        # keep if in a majority of systems
        if len(set(fx.system.tolist())) > n:
            data.append(fx)

    out = pd.concat(data, axis=0, ignore_index=True)
    out = out.drop_duplicates(subset=['begin', 'end', 'case'])

    return out[['begin', 'end', 'case']]

The data look like:
systems = ['A', 'B', 'C', 'D', 'E']

df = begin,end,system,case
0,9,A,0365
10,14,A,0365
10,14,B,0365
10,14,C,0365
28,37,A,0366
38,42,A,0366
38,42,B,0366
53,69,C,0366
56,60,B,0366
56,60,C,0366
56,69,D,0366
64,69,E,0366
83,86,B,0367

The expected output should be:
out = begin,end,case
    10,14,0365
    56,69,0366

IOW, if desired elements begin, end, case appear in a majority of systems, we accumulate them and return them as a dataframe.
The algorithm works perfectly fine, but since there are hundreds of thousands of rows in it, this is taking quite a while to process.
One optimization I can think of, but am unsure of how to implement is in the itertuples iteration: If, for the first instance of a filter set begin, end, case there are matches in
fx = test.loc[(test.begin == row.begin) & (test.end == row.end) & (test.case == df.case) & (fx.system.isin(systems))]

then, it would be beneficial to not iterate over the other rows in the itertuples iterable that match on this filter. For example, for the first instance of 10,14,A,0365 there is no need to check the next two rows, since they've already been evaluated. However, since the iterable is already fixed, there is no way to skip these of which I can think.

This can be solved with a simple groupby operation, which can tell you how often each combination appears. Then you just need to compare this with your n and filter the data:
key = ["begin", "end", "case"]
n = len(systems) // 2

mask = df.groupby(key)["system"].count() > n

df.set_index(key)[mask] \
  .reset_index() \
  .drop(columns="system") \
  .drop_duplicates()

#    begin  end  case
# 0     10   14  0365

This outputs the same as your code on my machine, but not the same as the example output you gave in your question (which I think is wrong).
Note that I used integer division // instead of float division / and manually casting to int.
In general, if you find yourself using itertuples in pandas, you should think very hard if you cannot achieve your goal in another way, as that is one of the slowest ways to do something with all rows of a dataframe.


Leetcode: Merging 2 sorted lists https://leetcode.com/problems/merge-two-sorted-lists/

Merge two sorted linked lists and return it as a new list.
The new list should be made by splicing together the nodes of the first two lists.

Example:

Input: 1->2->4, 1->3->4
Output: 1->1->2->3->4->4

using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace SortingQuestions
{
    [TestClass]
    public class Merge2SortedLists
    {
        [TestMethod]
        public void TwoListsMerge()
        {
            ListNode l1 = new ListNode(1);
            l1.next = new ListNode(2);
            l1.next.next = new ListNode(4);
            ListNode l2 = new ListNode(1);
            l2.next = new ListNode(3);
            l2.next.next = new ListNode(4);
            var res = MergeTwoLists(l1, l2);
            Assert.AreEqual(1, res.val);
            Assert.AreEqual(1, res.next.val);
            Assert.AreEqual(2, res.next.next.val);
            Assert.AreEqual(3, res.next.next.next.val);
            Assert.AreEqual(4, res.next.next.next.next.val);
            Assert.AreEqual(4, res.next.next.next.next.next.val);
        }
        [TestMethod]
        public void OneEmptyList()
        {
            ListNode l1 = new ListNode(1);
            l1.next = new ListNode(2);
            l1.next.next = new ListNode(4);
            ListNode l2 = null;
            var res = MergeTwoLists(l1, l2);
            Assert.AreEqual(1, res.val);
            Assert.AreEqual(2, res.next.val);
            Assert.AreEqual(4, res.next.next.val);
        }

        public ListNode MergeTwoLists(ListNode l1, ListNode l2)
        {
            ListNode result = new ListNode(0);
            if (l1 == null && l2 == null)
            {
                return null;
            }

            ListNode currRes = result;
            while (l1 != null && l2 != null)
            {
                if (l1.val < l2.val)
                {
                    currRes.next = l1;
                    l1 = l1.next;
                    currRes = currRes.next;
                }
                else
                {
                    currRes.next = l2;
                    l2 = l2.next;
                    currRes = currRes.next;
                }
            }
            if (l1 != null)
            {
                currRes.next = l1; //concat all the rest
            }
            else
            {
                currRes.next = l2;
            }
            return result.next; // we don't pass the first value
        }
    }

    public class ListNode
    {
        public int val;
        public ListNode next;
        public ListNode(int x) { val = x; }
    }
}

if you could please review this code from testing point of view, in an interview. what edge cases would you expected to be tested here. please remember this is about 30 minutes. also please comment on style.

Style wise, the assignment currRes = currRes.next; happens no matter what, so lift it out of if-else clause:

            if (l1.val < l2.val)
            {
                currRes.next = l1;
                l1 = l1.next;
            }
            else
            {
                currRes.next = l2;
                l2 = l2.next;
            }
            currRes = currRes.next;

Correctness wise, the condition if (l1.val < l2.val) loses the stability: if the elements compare equal, the one from l2 is merged first. Consider if (l1.val <= l2.val) (or even if (l2.val < l2.val)) instead. It is not required by the problem statement, and it doesn't matter for ints. In real life however the stability is of utmost importance, and as an interviewer I'd definitely mark it.

On the bright side, a dummy head for the merged list is a right way to go.


Appending various elements to SVG This is the code I currently have for adding various elements inside the svg and then inside a circle. On the text element I added 3 attributes that are the same. But I haven't found a way to remove this to minimize code.

var node = svg.selectAll("circle")
  .data(nodes)
  .enter()
  .append('g')
  .append("circle")
  .style("fill", function (d) {
    return 'rgb(108,181,205)';
  }).on("click", function (d, i) {
    if (i < 5) {
      d.radius *= 1.1;
      d3.select(this).attr("r", d.radius);
      force.resume();
    }
  }).call(force.drag);

// Adding the text
svg.selectAll('g')
  .append('text')
  .attr("text-anchor", "middle")
  .attr("alignment-baseline", "before-edge")
  .attr('font-family', 'FontAwesome')
  .attr('font-size', function(d) { return '1.9em'})
  .append('tspan')
  .attr('x', '0')
  .attr('dy', '-0.2em')
  .style('fill', 'white')
  .style('display', function(d, i) {
    if (i < 5) {
      return 'block';
    }
    else {
      return 'none';
    }
  })
  .text(function(d, i) {
    return circlesText[i].icon;
  });

svg.selectAll('g')
  .append('text')
  .attr("text-anchor", "middle")
  .attr("alignment-baseline", "before-edge")
  .attr('font-family', 'FontAwesome')
  .attr('font-size', function(d) { return '0.9em'})
  .append('tspan')
  .attr('x', '0')
  .attr('dy', '0.9em')
  .style('fill', 'white')
  .style('display', function(d, i) {
    if (i < 5) {
      return 'block';
    }
    else {
      return 'none';
    }
  })
  .text(function(d, i) {
      return circlesText[i].text[0];
  });

svg.selectAll('g')
  .append('text')
  .attr("text-anchor", "middle")
  .attr("alignment-baseline", "before-edge")
  .attr('font-family', 'FontAwesome')
  .attr('font-size', function(d) { return '0.9em'})
  .append('tspan')
  .attr('x', '0')
  .attr('dy', '1.8em')
  .style('fill', 'white')
  .style('display', function(d, i) {
    if (i < 5) {
      return 'block';
    }
    else {
      return 'none';
    }
  })
  .text(function(d, i) {
      return circlesText[i].text[1];
  });

svg.selectAll('g')
  .append('text')
  .attr("text-anchor", "middle")
  .attr("alignment-baseline", "before-edge")
  .attr('font-family', 'FontAwesome')
  .attr('font-size', function(d) { return '1.9em'})
  .append('tspan')
  .attr('x', '0')
  .attr('dy', '1.8em')
  .style('fill', 'white')
  .style('display', function(d, i) {
    if (i < 5) {
      return 'block';
    }
    else {
      return 'none';
    }
  })
  .text(function(d, i) {
      return '\uF056';
  });

The simpler option is to create a local function or two.

var appendText = function(fontSize) {
  return svg.selectAll('g')
    .append('text')
    .attr("text-anchor", "middle")
    .attr("alignment-baseline", "before-edge")
    .attr('font-family', 'FontAwesome')
    .attr('font-size', function(d) { return fontSize });
}

var appendTextSpan = function(fontSize) {
  return appendText(fontSize).
    .append('tspan')
    .style('fill', 'white')
    .style('display', function(d, i) {
      return (i < 5) ? 'block' : 'none';
    });
}

appendTextSpan('1.9em')
  .attr('x', '0')
  .attr('dy', '-0.2em')
  .text(function(d, i) {
    return circlesText[i].icon;
  });

appendTextSpan('0.9em')
  .attr('x', '0')
  .attr('dy', '0.9em')
  .text(function(d, i) {
      return circlesText[i].text[0];
  });

appendTextSpan('0.9em')
  .attr('x', '0')
  .attr('dy', '1.8em')
  .text(function(d, i) {
      return circlesText[i].text[1];
  });

appendTextSpan('1.9em')
  .attr('x', '0')
  .attr('dy', '1.8em')
  .text(function(d, i) {
      return '\uF056';
  });

The more complex one, but arguably better, would be to inject some CSS into the SVG such that you can just use

svg.selectAll('g')
  .append('text')
  .attr('class', 'label')
  .append('tspan')
  ...

How easy that is to do depends on how much control you have of the enclosing context.


Reduce the number of elements I hover The level 3 menu is broken in 2 parts if there are more than 10 items. The issue is with hovering over the newly created list.

At this moment I have 3 functions that apply the shown-sub class. I want to reduce it to only one. It should work when hovering over cotton and both ul lists.

 !function(){
 $("#nav ul.level0 ul.level1").each(function(){
  var menuLength = $(this).find("li.level2").length;
  if (menuLength > 10){
   var detachedElements = $(this).find("li.level2").slice(10);
   detachedElements.detach();
   $(this).parent().append("

");
   detachedElements.appendTo($(this).siblings(".second-menu"));
  }
  $(this).siblings("a").hover(
   function(){
    $(this).siblings(".second-menu").addClass("shown-sub");
   },
   function(){
    $(this).siblings(".second-menu").removeClass("shown-sub");
   }
  );
  $(this).children(".level2").hover(
   function(){
    $(this).parent().siblings(".second-menu").addClass("shown-sub");
   },
   function(){
    $(this).parent().siblings(".second-menu").removeClass("shown-sub");
   }
  );
  $(this).siblings(".second-menu").children(".level2").hover(
   function(){
    $(this).parent().addClass("shown-sub");
   },
   function(){
    $(this).parent().removeClass("shown-sub");
   }
  );
 })
 }();
.second-menu{
  color:red;
}
.shown-sub{
 color:blue;
}

        link text

            Cotton

has to be black

has to be black

has to be black

has to be black

has to be black

has to be black

has to be black

has to be black

has to be black

has to be black

has to be blue on hover

has to be blue on hover

has to be blue on hover

has to be blue on hover

has to be blue on hover

has to be blue on hover

has to be blue on hover

has to be blue on hover

has to be blue on hover

has to be blue on hover

Particularly with your example, I'm not sure if is the kind of functionality what you want, but you can use pure html + css for get a similar approach, using :hover selector.

http://jsfiddle.net/4cywb3yg/4/

If doesn't work for you, and want to keep your code, I refactored a couple of things.

I think you are add/remove the same class. I extracted in a generic function, and apply independently to each element.

    function setToggles($mainElement, $subElement) {

    var hoverIn = function () {
       $subElement.addClass("shown-sub");
    };

    var hoverOut = function () {
       $subElement.removeClass("shown-sub");
    };

    $mainElement.hover(hoverIn,hoverOut);
}

Also I extract a couple of variables with the convention $variable for jQuery elements.

Particularly in this example you are using a lot of jQuery selectors,that can be re-use extracting in a variables. (that was the way what i found the extract code)

Also you chain a lot of jQuery calls, Is not bad, but in excess is easy lose who is receive the last message.And can be a mess for human-reading.

http://jsfiddle.net/4cywb3yg/3/


Checking whether a given timestamp is more than two weeks old I am trying to see whether a given timestamp is more than two weeks old or not. If it is two weeks old, then print out "Hello" and if it is not more than two weeks old then print out "World".

I have a variable declared as uint64_t which contains the timestamp in milliseconds.

uint64_t data_timestamp;

How do I check whether this timestamp is more than two weeks old and if it is two weeks old, then print out "Hello" otherwise prints out "World".

This is the code I have which gives me the timestamp in milliseconds for two weeks back from the current date, which I then compare to see whether it is two weeks old or not.

#include <ctime>
#include <chrono>
#include <iostream>

int main()
{

    uint64_t data_timestamp = 1406066507000;

    const auto now = std::chrono::system_clock::now();
    auto twoWeeks = std::chrono::hours(24 * 14);
    auto lastTwoWeeks = now - twoWeeks;

    auto millis = std::chrono::duration_cast<std::chrono::milliseconds>\
                    (lastTwoWeeks.time_since_epoch()).count();

    std::cout << "Time stamp in milliseconds since UNIX epoch start: "\
              << millis << std::endl;

    if (data_timestamp < millis) {
        std::cout << "Hello";
    } else {
        std::cout << "World";
    }

    return 0;
}

I will be running this code on Ubuntu 12.04. What is a better way of doing this? Are there any improvements which I can do it here? In general, I need to check whether data_timestamp is more than two weeks old or not. If it is two weeks old, then print "hello" otherwise prints "world."

Do I even need to use chrono time here? There are 2 * 7 * 24 * 60 * 60 * 1000 = 1,209,600,000 milliseconds in two weeks, so a simple subtraction of that from the current should tell me what is more than 2 weeks old? right?

You're already using <chrono> (and just that), so you don't need <ctime>.  Including it anyway won't automatically allow compilation in pre-C++11 (if that was your intent here).

Also, since you're using uint64_t, you must include <cstdint> as well.
Each variable can be const since they're not being modified.  This would still be a good idea for ensuring const-correctness, even though the entire scope is within main().
The \ characters for line-continuations are useless here; the wrapping still works.
Since you're not needing to flush the buffer, std::endl should be replaced with "\n".  This may also give you a slight performance boost.
The "hello world" output can be shortened to a single-line ternary statement:

std::cout << ((data_timestamp < millis) ? "Hello" : "World");

Regarding this comparison: my tests give a signed/unsigned mismatch warning.  You have already given data_timestamp an unsigned type, so auto must be deducing a signed type for millis based on the function's return type.

One way to fix this could be by changing data_timestamp to a signed type such as int64_t, though <chrono> may offer more appropriate alternatives to help avoid uncertainty for this.
You don't need your own return 0 at the end of main().  The C++ standard should guarantee that the compiler will do this return for you since success is implied.


Random number generation in Dice class I don't intend on implementing this into an application yet as I'm going to test it first.  As such, it doesn't do much beyond rolling.  It also overloads some "typical" operators such as == and !=.

Most of all, I'm still very unsure about my random number generator.  I've read about <random>'s std::uniform_int_distribution and that it has advantages over std::rand, but I'm also not sure if I've put it in a good place.  Client code will be forced to use my seed if my source file is used, but my header file is free from this.

What could be said about my RNG use?  Anything else that could be done?

Dice.h

#ifndef DICE_H
#define DICE_H

#include <ostream>

class Dice
{
private:
    int topValue; // same default type as std::uniform_int_distribution<>

public:
    void roll();
    int value() const { return topValue; }
};

// the C++ FAQ on SO suggests that these overloads are best as non-members

inline bool operator==(Dice const& lhs, Dice const& rhs) { return lhs.value() == rhs.value(); }
inline bool operator!=(Dice const& lhs, Dice const& rhs) { return !operator==(lhs, rhs); }
inline std::ostream& operator<<(std::ostream& out, Dice const& obj) { return out << obj.value(); }

#endif

Dice.cpp

#include "Dice.h"
#include <random>

std::random_device rd;
std::mt19937 gen(rd());
std::uniform_int_distribution<> dis(1, 6);

void Dice::roll()
{
    topValue = dis(gen);
}

Just based on the code, and in no particular order:

Bikeshedding: as Dice represents a single die, I would call it such.
You're passing Dice by reference to const everywhere.  In your current design it is just an int, and I don't see anything happening that would increase the cost of passing it significantly, or cause it to become non-copyable.  As per Optimizing Emergent Structures in C++ you should prefer to pass by value here.
Your rd, gen, and dis should all be in an anonymous namespace to prevent linker errors when people define things of the same name in other files.
Does equality and inequality really make sense here?  It bothers me that x == y is not preserved after x.roll(); y.roll();.  I think I'd stick to accessing value explicitly, and not allow equality on Dice.

As for design: the implicit sharing of state between dice bothers me.  It isn't detectable as-is, but it means that there is no way of specifying that a particular die will return particular results (for debugging).  If this is used in a multithread application, the results of roll may be surprising; I'm fairly sure uniform_int_distribution<>::operator() should not be called from multiple threads at once.

I think I'd go for the more explicit approach of having an updateRandom and getRandom pair, or ensure thread-safety of roll.

You could also make this more generic:

// die.hpp
extern std::random_device rd;
extern std::mt19937 gen;

template<typename T>
struct Die {
    T value() const {
        return topValue;
    }

    void roll() {
        topValue = dis(gen);
    }

private:
    T topValue;
    static std::uniform_int_distribution<T> dis;
};

template<typename T>
std::uniform_int_distribution<T> Die<T>::dis(1, 6);

// die.cpp
std::random_device rd;
std::mt19937 gen(rd());

However, this would have the obvious drawback of the implementation being exposed.


Bash code to install bootstrap in a Ruby on Rails repository Wrote some code to install bootstrap (latest) in a newly created rails application.

update: fixed a typo

yarn add bootstrap jquery popper.js expose-loader
yarn install --check-files
perl -pi.bak -e "s/extract_css: false/extract_css: true/" config/webpacker.yml
s1=$'\n\nimport "bootstrap/dist/js/bootstrap";'
echo "$s1" >> app/javascript/packs/application.js
echo '@import "~bootstrap/scss/bootstrap";' > app/javascript/packs/styles.scss
(gsed $'/module.exports = environment/{e cat expose-loader.txt\n}' config/webpack/environment.js) > config/webpack/environment.js.new
mv config/webpack/environment.js.new config/webpack/environment.js
(gsed $'/javascript_pack_tag .application/{e cat stylesheet_pack_tag.txt\n}' app/views/layouts/application.html.erb) > app/views/layouts/application.html.erb.new
mv app/views/layouts/application.html.erb.new app/views/layouts/application.html.erb

It's worth starting with a shebang; I'd also advise setting shell flags -e (exit on error) and -u (error when using unset variables):
#!/bin/bash

set -eu

Please use more whitespace and comments to make it easier to follow.
Perl is overkill for the simple substitution here - if we stick to sed, then that's one fewer dependency:
sed -e '/^extract_css:/:.*/ true/' \
    -i config/webpacker.yml

I omitted creating the backup because I don't think there's any value in keeping the original.  If your later processes depend on having it around, then use -i.bak instead of -i, as in Perl (in fact, that's where Perl got that option).
The two transform-and-move pairs can also use sed -i, making the script more consistent, and thus easier to follow.  We can use the (standard sed) r command to read a file, instead of the non-standard e command:
sed -e '/module.exports = environment/rexpose-loader.txt' \
    -i config/webpack/environment.js
sed -e '/javascript_pack_tag .application/rstylesheet_pack_tag.txt' \
    -i app/views/layouts/application.html.erb

$s1 is only used once, and doesn't add any value (such as a meaningful name), so just inline it.
If we can eliminate the one use of C-escape quoting, then we can use plain POSIX shell instead of Bash.

Modified code
#!/bin/sh

set -eu

yarn add bootstrap jquery popper.js expose-loader
yarn install --check-files

sed -e '/^extract_css:/:.*/ true/' \
    -i config/webpacker.yml

printf '\n\n%s' 'import "bootstrap/dist/js/bootstrap";' \
       >> app/javascript/packs/application.js
echo '@import "~bootstrap/scss/bootstrap";' \
     > app/javascript/packs/styles.scss

sed -e '/module.exports = environment/rexpose-loader.txt' \
    -i config/webpack/environment.js
sed -e '/javascript_pack_tag .application/rstylesheet_pack_tag.txt' \
    -i app/views/layouts/application.html.erb


Summing the size/count of associations for multiple records I'm working on a plain Ruby class within a Rails application.

For the total_leg_count method---is that the best way to go about achieving what I want? I feel like there's potentially a better way.

class AircraftTimeReport

  ...

  def total_leg_count
    count = 0
    active_flights_within_timeframe.includes(:legs).each do |flight|
      count += flight.legs.size
    end
    count
  end

  private

  def active_flights_within_timeframe
    @aircraft.flights.within_timeframe(from_datetime, to_datetime).not_cancelled
  end
end

report = AircraftTimeReport.new(@aircraft)
puts report.total_leg_count

An Aircraft has_many Flights and a Flight belongs_to Aircraft.

A Flight has_many Legs and a Leg belongs_to a Flight.

I've seen other solutions that use Arel or plain SQL but I can't seem to wrap my head around it.

Also, I'm finding it difficult to unit test this method, which is why I'm wondering if there's a better way!

Equivalent, but functional and idiomatic:

def total_leg_count
  active_flights_within_timeframe.includes(:legs).map { |fl| fl.legs.size }.sum
end

This pure SQL query should be equivalent and more performant:

def total_leg_count
  active_flights_within_timeframe.includes(:legs).count(:legs)
end


My first program in C. A simple command-line calculator I've recently started programming in C and as my first serious program I thought I'd create a simple calculator. To make it a bit more complex I decided not to use the functions included in math.h (like pow) but to create these functions by myself.
Let me know if there is something I can do to improve this code.
#include<stdio.h>

double power(double x, double y, double r)                                                       // This function calculates the power and takes numer1, number2 and the results
{
    r = 1;                                                                           // We need to initialize the results otherwise we can't store the partial result on line 11
    int c;
    for (c = y; c > 0; c--)                                  // putting c equal to y we can decrement c and obtain the precise number of operations needed to complete the power
    {
        int pow = y;                                  // that's a bit tricky: we can't apply the modulus operand in line 12 because y is a double, so we must convert y in an int
        r *= x;                                                             // this way result = 1 * number1, then number1 * number1, then number1 * number1 * number1 and so on
        if ((x < 0) && (pow % 2 == 1))    /* If the first number is negative, and we calculate the power of this number to 2 or multiples, the number resulting will be negative
                                                                to obtain the correct result (positive) we must multiply the result to -1, if the previous conditions are met */
        {
            x *= -1;
        }
    }
    printf("%0.1lf", r);
}

double root(double x, double y, double r)
{
    int c;
      if (x < 0)
      {
         puts("is impossible to calcultate the root of a negative number");
      }
    for (c = 1; c < x; c++)
    {
        int partRes = x;
        partRes /= c;
        if (partRes * partRes == x)
        {
            printf("The root of %0.1lf is: %0.1d\n", x, partRes);
            break;
        }
    }
}

int main()
{
    double num1, num2, res;
    char op;
    puts("Input number, operator and number");
    puts("operands permitted: + (addition), - (subtraction), * (multiplication), / (division), ^ (power), $ (square root)");
    scanf("%lf %c %lf", &num1, &op, &num2);
    if (op == '+')
    {
        res = num1 + num2;
        printf("%0.1lf", res);
    }
    else if (op == '-')
    {
        res = num1 - num2;
        printf("%0.1lf", res);
    }
    else if (op == '*')
    {
        res = num1 * num2;
        printf("%0.1lf", res);
    }
    else if (op == '/')
    {
        res = num1 / num2;
        printf("%0.1lf", res);
    }
    else if (op == '^')
    {
        power(num1, num2, res);
    }
    else if (op == '$')
    {
        root(num1, num2, res);
    }
    else
    printf("error, use a valid operand");
}

power() problems
double power() does not return a value.
Incorrect result when y is not a value in the int range.
Code attempts a linear power calculation: O(y).  Research Exponentiation by squaring.  It is faster and less precision loss.
double r parameter serves no purpose. Instead make a local double r variable.
A floating point power function is one of the most changeling functions to code well.  Rough cut below.

root() problems
double root() does not return a value.
Other problems like power().

It floating point, not fixed point
More informative to use exponential notation for large and small value than a fix-point format.
// printf("%0.1lf", res);
printf("%g", res);

Sample improved my_power() - unchecked
// Very rough code
double my_power(double x, double y) {
  double whole;
  double fraction = modf(y, &whole);
  if (fraction == 0.0) {
    double r = 1.0;
    double p = fabs(whole);
    while (p > 0) {
      if (fmod(p, 2.0)) {
        r *= x;
      }
      x *= x;
      p /= 2.0;
    }
    if (whole < 0) {
      r = 1.0 / r;
    }
    return r;
  }
  if (x == 0.0) {
    return 0.0;
  }
  if (x < 0.0) {
    return NAN;
  }
  return exp(log(x) * y);
}


Creating all possible combinations of points I want to create all possible combinations of points (0,0),(0,1)...(12345,6789) and then all segments from these points.

The code I have written is simple and with no optimization.  Is there any algorithm to generate it in less time?

public static void main(String[] args) {
    int m=12345;
    int n=6789;

    for(int x1=0;x1<=m;x1++)
    {
        for(int y1=0;y1<=n;y1++)
        {
            for(int x2=0;x2<=m;x2++)
            {
                for(int y2=0;y2<=n;y2++)
                {
                    for(int x3=0;x3<=m;x3++)
                    {
                        for(int y3=0;y3<=n;y3++)
                        {
                            for(int x4=0;x4<=m;x4++)
                            {
                                for(int y4=0;y4<=n;y4++)
                                {
                                    Point p1 = new Point(x1, y1);
                                    Point p2 = new Point(x2, y2);
                                    Point p3 = new Point(x3, y3);
                                    Point p4 = new Point(x4, y4);

                                    Segment s1 = new Segment(p1, p2);
                                    Segment s2 = new Segment(p2, p3);
                                    Segment s3 = new Segment(p3, p4);
                                    Segment s4 = new Segment(p4, p1);

                                    //operate on those segements

                                }
                            }
                        }
                    }
                }
            }
        }
    }
}

Google Guava has a cartesianProduct helper method, I would try to use it instead of the nested loops. I don't think that it will be faster but it would be easier to read an maintain.


itoa base function in C This is a function that converts an integer value to a null-terminated string using the specified base and stores the result in a char array that I must allocate (with malloc). I have decided not to use malloc. Allowed functions: malloc. Loops: while Please review my code.

char *itoa_base(int value, int base)
{
    static  char rep[] = "0123456789abcdef";
    static  char buf[50];
    char    *ptr;
    int     num;

    ptr = &buf[49];
    *ptr = '\0';
    num = value;
    if (value < 0 && base == 10)
        value *= -1;
    if (value == 0)
        *--ptr = rep[value % base];
    while (value != 0)
    {
        *--ptr = rep[value % base];
        value /= base;
    }
    if (num < 0 && base == 10)
        *--ptr = '-';
    return (ptr);

Input checking

If the caller passes in a base greater than 17, you will read past the end of rep.
If the caller passes in a base of 0, you will divide by zero.
If the caller passes in a base of 1, your code will infinite loop.
If the caller passes in a negative value but the base is not 10, you will read past the front of rep because your modulus will be negative.  You should always make value positive before proceeding with your loop.

Insufficient buffer size
On a system with 64-bit ints and a base of 2, you will need a buffer of size 65 instead of 50.  (Or 66 if you allow negative values for all bases).
Code simplification
This code:

if (value == 0)
    *--ptr = rep[value % base];
while (value != 0)
{
    *--ptr = rep[value % base];
    value /= base;
}

could be simplified to:
do {
    *--ptr = rep[value % base];
    value /= base;
} while (value != 0);

I'm not sure from your question whether you were allowed to use a do loop or not.  Also, your question says you are supposed to return an allocated buffer but you intentionally didn't, so I'm not sure how to review that part of it.


Reading columns of a delimited text file This class reads a delimited text file into String arrays on demand. Delimiter can be any regex.

Features include:

Read a single column
Read a range of columns
Read arbitrary columns by providing an array of indices, e.g. for columns 1, 3, 5, use new int[]{1, 3, 5}

import java.io.IOException;
import java.nio.charset.Charset;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.Arrays;
import java.util.List;
import java.util.function.Supplier;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.Stream;

/**
 * Created by IDEA on 17/11/14.
 */
public class Txt {
    final private String sep;
    final private Pattern pattern;
    final private String filename;
    private Supplier<Stream<String>> fileStream;
    Txt(String _filename, String _sep) throws IOException {
        sep = _sep;
        filename = _filename;
        pattern = Pattern.compile(sep);
        fileStream = () -> {
            try {
                return Files.lines(Paths.get(filename), Charset.defaultCharset());
            } catch (IOException e) {
                e.printStackTrace();
                return null;
            }
        };
    }
    Txt(String _filename) throws IOException {
        this(_filename, "\\s+");
    }

    public String[] nthCol(int n) throws IOException {
        if(n < 1) n = 1;
        final int finalN = n;
        List<String> colList =  fileStream.get()
                .map(line -> pattern.split(line, finalN + 1)[finalN - 1])
                .collect(Collectors.toList());
        return colList.toArray(new String[colList.size()]);
    }

    public String[][] cols() throws IOException {
        List<String[]> colsList =  fileStream.get()
                .map(line -> pattern.split(line))
                .collect(Collectors.toList());
        return colsList.toArray(new String[colsList.size()][]);
    }

    public String[][] cols(final int start, final int end) {
        if(start > end || start < 1) throw new IllegalArgumentException("Starting index must <= ending index AND >= 1");
        List<String[]> colsList =  fileStream.get()
                .map(line -> {
                    String[] fields = pattern.split(line, end + 1);
                    // indices of col 2-4          2 3 4
                    // fields after splitting    0 1 2 3 4
                    // range of array              1     4
                    // java ending index is              exclusive
                    String[] demandedFields = Arrays.copyOfRange(fields, start-1, end);
                    return demandedFields;
                })
                .collect(Collectors.toList());
        return colsList.toArray(new String[colsList.size()][]);
    }

    public String[][] cols(int[] index) {
        Arrays.sort(index);
        final int[] finalIndex = index;
        final int start = finalIndex[0];
        final int end = finalIndex[finalIndex.length - 1];
        if(start > end || start < 1) throw new IllegalArgumentException("Starting index must <= ending index AND >= 1");
        List<String[]> colsList =  fileStream.get()
                .map(line -> {
                    String[] fields = pattern.split(line, end + 1);
                    String[] demandedFields = new String[finalIndex.length];
                    for(int i=0; i<finalIndex.length; i++) {
                        demandedFields[i] = fields[finalIndex[i]-1];
                    }
                    return demandedFields;
                })
                .collect(Collectors.toList());

        return colsList.toArray(new String[colsList.size()][]);
    }
}

General Remarks
The code checks incoming parameters, and throws an IllegalArgumentException when they are out of bounds (except nthCol; see later).  This is a good thing.  It would be even better if you could include the faulty indices in the error message so that I don't have to pop out the debugger. ;-)
The class seems to work with 1-based indices rather than 0-based, the latter being more common in Java[1].  As you commented about this within your code (but not in your public documentation), you've felt there's a mismatch; it may be better to use 0-based indices for consistency with the rest of the JDK.
You're going through hoops to work with arrays in the public interface while using streams and collections behind the scenes.  Consider returning lists instead of arrays, or maybe even streams if you're so inclined.
Constructors
Character encodings are important, too important to force the default encoding on your users, for any definition of 'default encoding'.  Many of the classes and methods deprecated in java.io were so because they assumed about encodings, which I've seen lead to subtle but no less spectacular breakage.[2]  Allow/force your users to specify the encoding.  If they make it be this fangled default encoding, then on their heads be it.  (A simular argument could be made for Paths.get.)
For your class, it doesn't matter how it gets its stream of strings, as long as it gets it.  Make this clear in your constructor:
Txt(Supplier<? extends Stream<String>> stream, Pattern delimiter) {
    this.stream = stream;
    this.delimiter = delimiter;
}

No fuss about encodings, file paths, IOExceptions, invalid patterns...  Just the bare needs to function.  Any other constructors or static factory methods you desire are then for convenience, and should delegate to this one.
Specifics
nthCol(int) breaks with the other methods in that it tries to fix an erroneous input parameter.  This leads to the oddness of nthCol(0) succeeding but cols(0, 0) failing.  Consistency is king in library usage: nthCol should fail, or cols should succeed.[3]
[!] cols(int[]) changes the contents of the incoming array.  You don't really need to sort this array, anyway: I'd consider it a feature if the columns returned are in the same order that I requested them.
Txt is not a very descriptive name. ;-)  Consider names such as LineSplitter or TextTable.

[1] Oh, how sweetly I loathe you, java.sql, for your 1-based indexing.
[2] Fixed record length + multi-byte encoding + JNI.  It was glorious.
[3] Personally, I lean towards "fail early, fail hard," (i.e. throw) but this depends on your intended audience.


Aho-Corasick algorithm to scan through a list of strings This is a follow-up to my previous question about finding min and max values of an iterable.
Aho-Corasick algorithm was suggested to solve the problem. Below is my solution with the use of ahocorapy library.

Short re-cap of the problem:

  You are given 2 arrays (genes and health), one of
  which have a 'gene' name, and the other - 'gene' weight (aka
  health). You then given a bunch of strings, each containing values m and n, which denote the start and end of the slice to be applied
  to the genes and health arrays, and the 'gene'-string, for which
  we need to determine healthiness. Then we need to return health-values
  for the most and the least healthy strings.

I think there might be something off with the code, but not sure what. It works quite fine for small testcases, giving more or less same timing as previous versions of the solution showed, but when it comes to large testcases, my PC basically hangs.

Example of a small testcase:

genes = ['a', 'b', 'c', 'aa', 'd', 'b']
health = [1, 2, 3, 4, 5, 6]
gene1 = "1 5 caaab" (result = 19 = max)
gene2 = "0 4 xyz" (result = 0 = min)
gene3 = "2 4 bcdybc" (result = 11)

Large testcase (2 lists 100K elements each; testcase 41K+ elements): txt in my dropbox (2,80 MB) (too large for pastebin)

So, I have 2 questions: 1) What is wrong with my code, how can I impore its performace
2) How do I apply the Aho-Corasick without turning to any non-standard library (because, most likely, it cannot be installed on HackerRank server)

def geneshealth(genes, health, testcase):
    from ahocorapy.keywordtree import KeywordTree
    import math

    min_weight = math.inf
    max_weight = -math.inf

    for case in testcase:
        #construct the keyword tree from appropriately sliced "genes" list
        kwtree = KeywordTree(case_insensitive=True)
        fl, ceil, g = case.split()
        for i in genes[int(fl):int(ceil)+1]:
            kwtree.add(i)
        kwtree.finalize()
        #search the testcase list for matches
        result = list(kwtree.search_all(g))

        hea = 0
        for gn, _ in result:
            for idx, val in enumerate(genes):
                if val == gn:
                    hea += health[idx]

        if hea < min_weight:
            min_weight = hea
        if hea > max_weight:
            max_weight = hea
    return(min_weight, max_weight)

This code is slow because:

It builds a new keyword tree for each test case. Just build it once, using all the genes.
It builds a list of all the matching keywords. KeywordTree.search_all() is a generator, just loop over it directly. And,
It loops over the list of genes to find the gene index, so that it can find the health.
Instead, build a dict with the genes as keys and an (index, health) tuple for the value.

Something like this (untested):

import math
from collections import defaultdict
from ahocorapy.keywordtree import KeywordTree

def geneshealth(genes, health, testcases):

    # build the kwtree using all the genes
    kwtree = KeywordTree(case_insensitive=True)
    for gene in genes:
        kwtree.add(gene)
    kwtree.finalize()

    # build a dict that maps a gene to a list of (index, health) tuples
    index_and_health = defaultdict(list)
    for gene, data in zip(genes, enumerate(health)):
        index_and_health[gene].append(data)

    min_dna_health = math.inf
    max_dna_health = -math.inf

    for case in testcases:
        start, end, dna = case.split()
        start = int(start)
        end = int(end)

        dna_health = 0

        # search the dna for any genes in the kwtree
        # note: we don't care where the gene is in the dna
        for gene, _ in kwtree.search_all(dna):

            for gene_index, gene_health in index_and_health[gene]:

                # only genes that are within the testcase limits
                # contribute dna_health
                if start <= gene_index <= end:
                    dna_health += gene_health

        # keep the min/max weight
        if dna_health < min_dna_health:
            min_dna_health = dna_health

        if dna_health > max_dna_health:
            max_dna_health = dna_health

    return(min_dna_health, max_dna_health)


BOOL×BOOL→ENUM mapping Does this part look clean enough? Any suggestions on how to make it cleaner?

if (isCheck)
{
    if (isStuck)
    {
        return GameState.Mate;
    }
    else
    {
        return GameState.Check;
    }
}
else
{
    if (isStuck)
    {
        return GameState.Stalemate;
    }
    else
    {
        return GameState.Ok;
    }
}

I'd use a decision table here:

GameState[,] decisionTable = new GameState[2,2];
decisionTable[0,0] = GameState.Ok;
decisionTable[0,1] = GameState.Stalemate;
decisionTable[1,0] = GameState.Check;
decisionTable[1,1] = GameState.Mate;
return decisionTable[Convert.ToInt32(isCheck), Convert.ToInt32(isStuck)];

From Code Complete 2nd Edition, Chapter 19: General Control Issues, page 431:

  Use decision tables to replace complicated conditions

  Sometimes you have a complicated test involving several variables. It can be helpful to use a decision table to perform the test rather than using ifs or cases. A decision-table lookup is easier to code initially, having only a couple of lines of code and no tricky control structures. This minimization of complexity minimizes the opportunity for mistakes. If your data changes, you can change a decision table without changing the code; you only need to update the contents of the data structure.


Teamwork challenge for Timus Online Judge Question is here:

  Input

  The first line contains an integer \$N\$, which is the quantity of numbers written down by Vasya (\$1 \le N \le 1000\$). The second line contains these numbers separated by spaces. All the numbers are positive integers not greater than 10.

  Output

  Output the numbers Petya would write down separated by space.

For instance, the sequence "1 1 2 3 3 3 10 10" will be told by Vasya as "two ones, one two, three threes, two tens". Petya also wants to be concise, so he writes down the numbers pronounced by Vasya instead of the whole words. For the example above, Petya would write: "2 1 1 2 3 3 2 10".

public class TestClass {

    static String firstLine = "8";
    static String secondLine = "1 1 2 3 3 3 10 10";

    public static void main(String[] args) {
        int countOfIntegersToBeProcessed = Integer.parseInt(firstLine);
        String[] integersAsStringArray = secondLine.split(" ");
        int localCounterForASpecificInteger = 1;
        for (int integersToBeProcessedCounter = 0; integersToBeProcessedCounter < countOfIntegersToBeProcessed; integersToBeProcessedCounter++) {
            String integerBeingProcessed = integersAsStringArray[integersToBeProcessedCounter];
            if (integersToBeProcessedCounter == countOfIntegersToBeProcessed - 1) {
                System.out.print(localCounterForASpecificInteger + " " + integerBeingProcessed);
                return;
            }
            if (integerBeingProcessed.equals(integersAsStringArray[integersToBeProcessedCounter + 1])) {
                localCounterForASpecificInteger++;
            } else {
                System.out.print(localCounterForASpecificInteger + " " + integerBeingProcessed + " ");
                localCounterForASpecificInteger = 1;
            }
        }
    }

}

And the output will be:

  2 1 1 2 3 3 2 10

Ow, my eyes.  Your variable names all look so long and similar.  I can't even see all of the code at once without line wrapping.  I'd rather read for (int i = 0; i < n; i++) than

for (int integersToBeProcessedCounter = 0; integersToBeProcessedCounter < countOfIntegersToBeProcessed; integersToBeProcessedCounter++) {

In cases like this, short variable names are fine, because it is an established convention that i is an array index, and n is the array size.

Furthermore, the verbose naming isn't really that accurate anyway.  integerBeingProcessed isn't really an integer, but a string.

A further improvement in naming would come from breaking out the array analysis into a separate function, so that main() is just responsible for parsing the input.  Then, integersAsStringArray and countOfIntegersToBeProcessed can become seq and seq.length, respectively.

Just applying some renaming and other minor transformations…

public class TestClass {

    static String firstLine = "8";
    static String secondLine = "1 1 2 3 3 3 10 10";

    public static void summarize(String[] seq) {
        int groupSize = 1;
        for (int i = 0; i < seq.length; i++) {
            if (i == seq.length - 1) {
                System.out.println(groupSize + " " + seq[i]);
            } else if (seq[i].equals(seq[i + 1])) {
                groupSize++;
            } else {
                System.out.print(groupSize + " " + seq[i] + " ");
                groupSize = 1;
            }
        }
    }

    public static void main(String[] args) {
        int arrayLen = Integer.parseInt(firstLine);
        String[] array = secondLine.split(" ");
        if (arrayLen != array.length) {   // Optional validation
            throw new IllegalArgumentException("Expected " + arrayLen + " numbers, got " + array.length);
        }
        summarize(array);
    }
}

I also propose the following algorithm, which does not need as much of a special case for handling the last element.

    public static void summarize(String[] seq) {
        int groupStart, groupEnd;
        for (groupStart = 0; groupStart < seq.length; groupStart = groupEnd) {
            String leader = seq[groupStart];
            for (groupEnd = groupStart + 1; groupEnd < seq.length && seq[groupEnd].equals(leader); groupEnd++);
            System.out.print((groupEnd - groupStart) + " " + leader +
                             (groupEnd < seq.length - 1 ? " " : "\n"));
        }
    }


What is the optimal way to write this small Todo List app Hello I don't have any specific problem but I'm just interested how someone who is experienced in JavaScript would write this small toy program. it's a simple Todo List app, there are many approaches to writing something like this but I'm not sure if the way I wrote it is the most optimal.
I want to see how others would write this, and maybe learn something new.
I'm not very experienced programmer so any criticism  is welcome :)

const TodoList = document.getElementById("TodoList");
const AddTodo = document.getElementById("AddTodo");
let ToggleBtns = []; let RemoveBtns = [];

let MyTodos = [
    {todoText: "hello world", Completed: true},
    {todoText: "hello world", Completed: false},
    {todoText: "hello world", Completed: false}
];

function CreateItems(text, index){
    const TodoItem = document.createElement("li");
    const ToggleButton = document.createElement("button");
    const TodoText = document.createElement("p");
    const RemoveButton = document.createElement("button");

    TodoItem.className = "TodoItem";
    ToggleButton.className = "ToggleButton";
    TodoText.ClassName = "TodoText";
    RemoveButton.className = "RemoveButton";
    TodoText.innerText = text;
    TodoText.style.textDecoration = (MyTodos[index].Completed === true ? "line-through" : "none");
    TodoItem.appendChild(ToggleButton);
    TodoItem.appendChild(TodoText);
    TodoItem.appendChild(RemoveButton);
    TodoList.appendChild(TodoItem);

    ToggleBtns.push(ToggleButton);
    RemoveBtns.push(RemoveButton);
};
for (i in MyTodos) { CreateItems(MyTodos[i].todoText, i); }

AddTodo.addEventListener("keypress", function(e){
    if(e.keyCode === 13){
            MyTodos.push({todoText: AddTodo.value, Completed: false});
            CreateItems(MyTodos[MyTodos.length -1].todoText, (MyTodos.length -1))
     }
});
document.addEventListener("click", function(e){
        if(e.target.className === "ToggleButton"){
            ToggleIndex = ToggleBtns.indexOf(e.target);
            MyTodos[ToggleIndex].Completed =
            (MyTodos[ToggleIndex].Completed === true ? false : true);
            e.target.parentElement.children[1].style.textDecoration = (MyTodos[ToggleIndex].Completed === true ?  "line-through" : "none");
        }
        else if (e.target.className === "RemoveButton"){
            RemoveIndex = RemoveBtns.indexOf(e.target);
            if(RemoveIndex !== 0){
                MyTodos.splice(RemoveIndex, RemoveIndex);
                ToggleBtns.splice(RemoveIndex, RemoveIndex);
                RemoveBtns.splice(RemoveIndex, RemoveIndex);
            } else {
                MyTodos.splice(0, 1);
                ToggleBtns.splice(0, 1);
                RemoveBtns.splice(0, 1);
            }
            e.target.parentElement.remove();
        }
    });
 #TodoList {
            display: flex;
            flex-direction: column;
            padding: 0  ;
        }
        .TodoItem {
            display: flex;
            flex-direction: row;
        }
        .TodoItem button {
            width: 50px;
        }
        .TodoItem p {
            width: 300px;
            text-align: center;
        }
        #Todos {
            display: table;
            margin: 0 auto;
        }
        #AddTodo {
            display: table;
            height:35px;
            width: 70%;
            padding: 0 20px;
            margin: 0 auto;
        }

    <input type="text" name="" id="AddTodo">

If you had not written this code but were trying to use it, how would you know that the square on either side was a button or what the function of that button was? Buttons need labels and they are generally oval shaped.
When you design or write code for the web you need to consider the user and how they will be using the tool. Even a toy application should follow some basic standards.


HarckerRank 2D Arrays Hourglass challenge About this 2D Arrays HackerRank problem.

The task is to find the maximum sum of a region in a 2D array. Specifically, it is to find the maximum sum of an "hourglass" region, defined as a 3x3 square without the middle entries on the left and right sides, as shown by this mask.

1 1 1
0 1 0
1 1 1

Here's my solution.

I'd appreciate some review about code quality.

# Begin of HackerRank code: data entries for testing the user code
# ----------------------------------------------------------------
arr = []

for _ in range(6):
    arr.append(list(map(int, input().rstrip().split())))
# ----------------------------------------------------------------
# End of HackerRank code

my_hourglasses = list()

for i in range(0,4):
    for j in range(0,4):
        hourglass = list()
        hourglass += arr[i][j:j+3]
        hourglass.append(arr[i+1][j+1])
        hourglass += arr[i+2][j:j+3]
        my_hourglasses.append(hourglass)

hourglasses_sums = [sum(item) for item in my_hourglasses]

print(max(hourglasses_sums))

One thing I like about this code is the clear division of role.

You have a section for identifying and extracting the hourglasses. Then you sum the individual hourglasses. Then you find the maximum.

There's a quote by Computer Scientist Tony Hoare that "There are two ways to write code: write code so simple there are obviously no bugs in it, or write code so complex that there are no obvious bugs in it." The clear structure helps make this into the first category. This is the sort of code that I'd like to see, for example, in a reference implementation or test case.

On the other hand, that simplicity comes at a cost. Your implementation requires allocating a bunch of extra memory to hold all the temporary hourglasses, and a bit more still to hold all the sums.

In principle, you could add them up as soon as you identify the elements in an hourglass, and then only store hourglasses_sums instead of my_hourglasses. Furthermore, instead of storing a list of all the sums you have seen, you could just keep track of the highest score you have seen thus far. You'd then have something like this:

max_hourglass_score = -63  # Minimum possible score = -9 * 7
for i in range(0,4):
    for j in range(0,4):
        hourglass = list()
        hourglass += arr[i][j:j+3]
        hourglass.append(arr[i+1][j+1])
        hourglass += arr[i+2][j:j+3]
        hourglass_sum = sum(hourglass)
        max_hourglass_score = max(max_hourglass_score, hourglass_sum)

print(max_hourglass_score)

Of course, the same pattern could be applied to do the sum as you go rather than picking it into an intermediate hourglass list.

max_hourglass_score = -63  # Minimum possible score
for i in range(0,4):
    for j in range(0,4):
        hourglass_sum = 0
        hourglass_sum += sum(arr[i][j:j+3])
        hourglass_sum += arr[i+1][j+1]
        hourglass_sum += sum(arr[i+2][j:j+3])
        max_hourglass_score = max(max_hourglass_score, hourglass_sum)

print(max_hourglass_score)

This only allocates a few primitive variables rather than all the extra arrays you had before.

There are in fact still cleverer algorithms which require fewer arithmetic operations. The key idea is easier to explain with squares rather than hourglasses.
If you look at the square in the top left corner and compare it with the square one place across, many of the entries are the same. Specifically, of the nine entries in a square, six are overlapped. The potential for speeding up the program come from not looking that the overlap squares any more, but starting from the last sum, taking away the values that are no longer in it, and adding the values that are new to it.

That algorithm, however clever, is complicated. It's hard to get right, and even hard to see whether it is right. It's therefore a judgement call how much you value making your code run as fast as possible, and how much you prefer writing it in a way that is easy to understand. There isn't a best answer. It depends on what you want to use it for. If this were running in real time on video frames and were (somewhat appropriately) the biggest bottleneck in your program, then you'd probably prefer faster. If it's trundling along on a small grid, you would want clear code.

A couple of other general comments:

Variable naming is important. Although "hourglass" would usually seem like a fairly specific and helpful word to identify what's going on, when three of your main variables are built around the same word one should wonder whether it actually gets confusing. (Also, "my_" is usually unnecessary.)
I appreciate the use of Python comprehensions. If you only want to use it once, a generator comprehension is often better. In terms of how it's written, just swap the square brackets around sum(item) for item in my_hourglasses for parentheses. In terms of what it does, it avoids actually doing the calculation until you need it, giving space and sometimes time savings.
Beware of magic numbers. I realise that 4 is the width of the big grid, minus 2, which you need because the width of the hourglass is 3 and the overlap is 1. But that's not immediately obvious, so it's worth spelling out what 4 means.


Divisions using Arraylists Exercise: Using Arraylists create a program that makes divisions between two very large numbers (numbers greater than 10 digits).

I did it (the code works), but could my code be improved?

public class Main {

    static void division(List<Integer> lista, List<Integer> listb) {

        List<Integer> listResult = new ArrayList<>();

        if (lista.size() <= 10 || listb.size() <= 10) {
            System.out.println("The numbers must be bigger than 10");
        } else {double a = 0, j = 0;
            String Stringnumber = "";
            String Stringnumber2 = "";

            for (int number : lista) {
                Stringnumber += String.valueOf(number);
            }

            for (int number : listb) {
                Stringnumber2 += String.valueOf(number);
            }

            double test = Double.parseDouble(Stringnumber2);
            double test2 = Double.parseDouble(Stringnumber);

            double dividend = 0, divisor = 0;

            if (test >= test2) {
                dividend = test;
                divisor = test2;
            } else {
                dividend = test2;
                divisor = test;
            }

            for (int i = 9; i > 0 && j != 1; i--) {
                if (i * divisor <= dividend) {
                    listResult.add(i);

                    a = dividend - (i * divisor);
                    if (a >= divisor) {
                        i = 9;
                    } else {
                        j = 1;
                    }
                }
            }

            System.out.println("The result is:");
            for (int i = 0; i < listResult.size(); i++) {
                System.out.println(listResult.get(i));
            }

        }
    }

    public static void main(String[] args) {

        Scanner x = new Scanner(System.in);

        List<Integer> lista = new ArrayList<>();
        List<Integer> listb = new ArrayList<>();

        System.out.println("Give a number");
        for (char digit : x.nextLine().toCharArray()) {
            lista.add(digit - '0');
        }

        System.out.println("Give another number");
        for (char digit : x.nextLine().toCharArray()) {
            listb.add(digit - '0');
        }
        x.nextLine();

        division(lista, listb);
    }

}

At first, I thought that you were going to implement division of two integers, each represented as a list of digits.  But then, you simply concatenate all of the elements of each operand to be parsed as a double.  If you're going to cheat on the arithmetic using the built-in floating-point support, then what's the point of the exercise?  In fact, it doesn't even care whether you pass in an ArrayList of individual digits {1, 2, 3, 4, 5} or an ArrayList of one element {12345} — either way, it all gets stringified and parsed as 12345.0d.


Fill a list of 2 lists from split string This might be too trivial of a question for Code Review, but here goes:

Assume I have text file with content:

A 1
B 2
C 3
D 4

And I want a list like [[1,2,3,4],["A","B","C","D"]] created by Python's list comprehensions.

My code accomplishes this, but it seems like I jump through way too many loops:

with open('textFile.txt') as f:
    lines = f.readlines()

z = [y for y in map(list, list(zip(*[x.split() for x in lines])))]

Is there an easier way to achieve this using list comprehension?

If you're going to use map, then it's simpler to surround it in a list() call, then a list comprehension.
Rather than using list(map(list, it)), you can instead use a list comprehension.

[list(i) for i in it]

You don't need to use list(zip()), it only hinders performance and readability.
You don't have to use a list comprehension with *, instead you could use a generator comprehension.
You don't have to use f.readlines(), if you perform the comprehension in the with.

Merging this all together gets:

with open('textFile.txt') as f:
    z = [list(i) for i in zip(*(j.split() for j in f))]