Dynamic array of structs in C Based on some code on internet, I implemented a dynamic array of structures in C.

I am really interested in some feedback on this.  Maybe there are some places where it can cause memory leaks or other pitfalls? (I am also concerned if there can be situation that I free a pointer twice in my case, and how to avoid that)

#include "stdafx.h"
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

typedef struct
{
    int ID;
    char * name;
} Student;

// array of structs
typedef struct
{
    Student *array;
    size_t used;
    size_t size;
} Array;

void initArray(Array *a, size_t initialSize)
{
    // Allocate initial space
    a->array = (Student *)malloc(initialSize * sizeof(Student));

    a->used = 0;           // no elements used
    a->size = initialSize; // available nr of elements

    // Initialize all values of the array to 0
    for(unsigned int i = 0; i<initialSize; i++)
    {
        memset(&a->array[i],0,sizeof(Student));
    }
}

// Add element to array
void insertArray(Array *a, Student element)
{
    if (a->used == a->size)
    {
        a->size *= 2;
        a->array = (Student *)realloc(a->array, a->size * sizeof(Student));
    }

    // Copy name
    a->array[a->used].name = (char*)malloc(strlen(element.name) + 1);
    strcpy(a->array[a->used].name, element.name);

    // Copy ID
    a->array[a->used].ID=element.ID;

    a->used++;
}

void freeArray(Array *a)
{
    // Free all name variables of each array element first
    for(int i=0; i<a->used; i++)
    {
        free(a->array[0].name);
        a->array[0].name=NULL;
    }

    // Now free the array
    free(a->array);
    a->array = NULL;

    a->used = 0;
    a->size = 0;
}

int main(int argc, const char * argv[])
{

    Array a;
    Student x,y,z;

    x.ID = 20;
    x.name=(char*)malloc(strlen("stud1") + 1);
    strcpy(x.name,"stud1");

    y.ID = 30;
    y.name=(char*)malloc(strlen("student2") + 1);
    strcpy(y.name,"student2");

    z.ID = 40;
    z.name=(char*)malloc(strlen("student3") + 1);
    strcpy(z.name,"student3");

    // Init array
    initArray(&a, 5);

    // Add elements
    insertArray(&a, x);
    insertArray(&a, y);
    insertArray(&a, z);

    // Print elements
    printf("%d\n", a.array[0].ID);
    printf("%s\n", a.array[0].name);
    printf("%d\n", a.array[1].ID);
    printf("%s\n", a.array[1].name);
    printf("%d\n", a.array[2].ID);
    printf("%s\n", a.array[2].name);
    printf("%d\n", a.array[3].ID);
    printf("%s\n", a.array[3].name);

    // Free array
    freeArray(&a);

    free(x.name);
    free(y.name);
    free(z.name);

    return 0;
}

I am also concerned if there can be situation that I free a pointer twice in my case

... and ...

  Yes just I am more interested in cases when/if there can be for example freeing unallocated memory or freeing already freed memory ...

After a quick inspection it doesn't appear that you free memory more than once:

free statements are only in the freeArray and main methods
Each free(a->array[0].name); is different because each name is allocated using its own malloc
free(a->array) is only called once
freeArray is only called once
free(x.name); doesn't free the same memory as free(a->array[0].name); because insertArray allocates new memory for each name

  and how to avoid that

Something which can help (though not guarantee) is to assign NULL to the pointer after you pass it to free.

It can help, because calling free on a previously-nulled pointer will harmlessly do nothing
It's not a guarantee, because you might have more than one pointer pointing to the same memory

dmcr_code's comment below points out a bug. You wrote,

for(int i=0; i<a->used; i++)
{
    free(a->array[0].name);
    a->array[0].name=NULL;
}

This should be,

for(int i=0; i<a->used; i++)
{
    free(a->array[i].name);
    a->array[i].name=NULL;
}

Because you set a->array[0].name=NULL; after freeing it, you don't free it twice.

But, you did fail to free the memory associated with a->array[i].name for values of i larger than 0.

  But then how do I protect against that - when array[i].name can contain random value and I try to free it?

To protect yourself:

Either, don't let it contain a random value (e.g. ensure that it's either a valid pointer, or zero)
Or, don't use it (e.g. ensure that your a->used logic is correct so that you don't touch elements which you haven't used/initialized).

  is memset in the initArray method fine for that?

memset is good:

You could use calloc instead of malloc to avoid having to use memset as well
You could use memset on the whole array at once instead of using memset on each element of the array

memset in initArray isn't enough. It's enough to begin with, but there's a realloc in insertArray. So to be good enough, you'd also need to use memset after realloc (to memset the as-yet-unused end of the newly-reallocated array; without using memset on the beginning of the reallocated array, which already contains valid/initialized/used elements).

  the only unclear part that remains from your response is how to memset realloced array

Your current code in initArray says,

// Initialize all values of the array to 0
for(unsigned int i = 0; i<initialSize; i++)
{
    memset(&a->array[i],0,sizeof(Student));
}

Another way to do that would be:

// Initialize all elements of the array at once: they are contiguous
memset(&a->array[0], 0, sizeof(Student) * initialSize);

The memset statement to add to insertArray would be:

if (a->used == a->size)
{
    a->size *= 2;
    a->array = (Student *)realloc(a->array, a->size * sizeof(Student));
    // Initialize the last/new elements of the reallocated array
    for(unsigned int i = a->used; i<a->size; i++)
    {
        memset(&a->array[i],0,sizeof(Student));
    }
}

Or:

if (a->used == a->size)
{
    a->size *= 2;
    a->array = (Student *)realloc(a->array, a->size * sizeof(Student));
    // Initialize the last/new elements of the reallocated array
    memset(&a->array[a->used],0,sizeof(Student) * (a->size - a->used));
}

  and this comment: "It's not a guarantee, because you might have more than one pointer pointing to the same memory " would be nice if you can address that too

This is safe:

void* foo = malloc(10);
free(foo);
// protect against freeing twice
foo = NULL;
// this is useless and strange, but harmless
free(foo);

This is not safe:

void* foo = malloc(10);
void* bar = foo;
free(foo);
// protect against freeing twice
foo = NULL;
// this is useless and strange, but harmless
free(foo);
// but this is dangerous, illegal, undefined, etc.
// because bar is now pointing to memory that has already been freed
free(bar);


Web scraper for a webpage article I am a beginner in Python and have just coded a simple web scraper for a webpage article, output to a text file, using BeautifulSoup and List.

The code is working fine, but I'm wondering if anybody would know a more efficient way to achieve the same.

import requests
page = requests.get('https://www.msn.com/en-sg/money/topstories/10-top-stocks-of-2017/ar-BBGgEyA?li=AA54rX&ocid=spartandhp')

# 2. Parsing the page using BeautifulSoup
import pandas as pd
from bs4 import BeautifulSoup
soup = BeautifulSoup(page.content, 'html.parser')

# 3. Write the context to a text file
all_p_tags = soup.findAll('p') # Put all <p> and their text into a list
number_of_tags = len(all_p_tags) # No of <p>?

x=0
with open('filename.txt', mode='wt', encoding='utf-8') as file:
    title = soup.find('h1').text.strip() # Write the <header>
    file.write(title)
    file.write('\n')
    for x in range(number_of_tags):
        word = all_p_tags[x].get_text() # Write the content by referencing each item in the list
        file.write(word)
        file.write('\n')
    file.close()

There are at least three things that may help to make the code more efficient:

switch to lxml instead of html.parser (requires lxml to be installed)
use a SoupStrainer to parse only the relevant part of the document
you can switch to http instead of https. While this would bring the security aspect down, you would avoid overhead of SSL handshaking, encryption etc - I've noticed the execution time difference locally, try it out

Improved code:
import requests
from bs4 import BeautifulSoup, SoupStrainer

page = requests.get('http://www.msn.com/en-sg/money/topstories/10-top-stocks-of-2017/ar-BBGgEyA?li=AA54rX&ocid=spartandhp')

parse_only = SoupStrainer("body")
soup = BeautifulSoup(page.content, 'lxml', parse_only=parse_only)

with open('filename.txt', mode='wt', encoding='utf-8') as file:
    title = soup.find('h1').text.strip()
    file.write(title + ' \n')

    for p_tag in soup.select('p') :
        file.write(p_tag.get_text() + '\n')

Note that I've also removed the unused variables and imports.
Btw, if it weren't for the title, we could've pinpointed SoupStrainer to p elements only - might've improved performance even more.


Script that fixes YML SIDs to be simplified for production I work with YML files that include SIDs and the following structure:

    title:
  "2": "content a" # key: comment
  "3": "content b" # key: comment
  "4": "content c" # key: comment
  "5": "content d" # key: comment
  "6": "content e" # key: comment

Usually, I have to remove some strings (note I never remove the number 1 or 2) so my new file looks like this:

    title:
  "2": "content a" # key: comment
  "3": "content b" # key: comment
  "5": "content d" # key: comment
  "6": "content e" # key: comment

I need to rearrange the SIDs in order to have a sequence without any gap (in this case 2, 3, 4, 5, 6) independently on the content. For that reason I have written the following script. It works properly but I need to bring it into production so I need your help to reduce its complexity, make it clear and simpler or any advice you may have for a beginner (in both, Python and Stack Exchange).

import re, os

file=input ('YML file name: ')

#read the file and store its content as a list
os.chdir('/home/balaclava/Desktop/Scripts/YML fixer/')
rdfile= open(file)
cont=rdfile.readlines()
rdfile.close()

#list to store the reviewed strings
newfile=[]
newfile.append(cont[0]+cont[1])

#Get the second string SID as reference
numRegex = re.compile(r'\d+')
act=numRegex.search(cont[1])
global refnum
refnum=int(act.group())

#Loop for each string (-2 due to the two first string are excluded)
for i in range(len(cont)-2):
    act=numRegex.search(str(cont[i+2]))
    temp=int(act.group())
#If the SID is correct, add item to newlist, else, fix it and add the item to the list.
    if temp == (refnum+1):
        newfile.append(cont[i+2])
    else:
        temp= (refnum+1)
        change=numRegex.sub(str(temp), cont[i+2])
        newfile.append(change)
    refnum += 1

#overwrite the file with the newlist content
with open (file,'w') as finalfile:
    finalfile.write(''.join(newfile))
    finalfile.close()

You could use rumael.yaml, it can preserve comments. https://stackoverflow.com/questions/7255885/save-dump-a-yaml-file-with-comments-in-pyyaml#27103244

Moreover, you want to be a better python developer (or maybe pythonist?) I can give you some tips:

Content duplication
You are storing the file content inside cont and after closing the file you are duplicating that info in a new variable newfile I think that is an unnecessary process in this situation. You could store all the data in cont and just modify the lines needed. You can replace the entire if-else by:

    if temp != (refnum+1):
        temp= (refnum+1)
        change=numRegex.sub(str(temp), cont[i])
        cont[i] = change

For loop's range
Change your range call in the for loop to range(2, len(cont)):
Now inside the loop you can access the current line with simply cont[i] it's more readable and efficient.

As with i's range you are accessing refnum always with a +1. By initializing it as refnum=int(act.group()) +1 your code saves that operations inside the loop. Another thing that you can do is do the +=1 increment at the beginning of the loop.

File management
You don't need to manually close files when using with statement you can remove finalfile.close(). Another thing, you are using with in the when writing but not when reading, think about always use the same method.

More things can be changed but I think that's enough for now.


ffmpeg batch script for video resizing - improved I posted a question here ffmpeg batch script for video resizing but I forgot the issue of the rotation and the video files were scaled with wrong dimensions.

Below is the improved version of the script

#!/bin/bash

set -eu

status=true
fail() {
    echo "$@" >&2
    status=false
}

# Resize a single file
resize() {
    exiftool -p 'Filename:$filename Megapixel:$megapixels Dimension:$imagesize Rotation:$rotation' $1
    if exiftool -if '$imageheight == 1920 && $imageheight == 1080' $1 >     /dev/null
    then
        fail "-> No need to convert"
        return
    fi

    filename="$(date +%s)".mp4
    rotation=$(exiftool -p '$rotation' $1)

    if [ "${rotation}" -eq 0 ] || [ "${rotation}" -eq 180 ]
    then
        scaling="-1:720"
    elif [ "${rotation}" -eq 90 ] || [ "${rotation}" -eq 270 ]
    then
        scaling="720:-1"
    else
        fail "-> Unhandled rotation"
        return
    fi

    if
        ffmpeg -v error -stats -i "$1" -map_metadata 0 \
               -vf scale="$scaling" -c:v libx264 -crf 23 \
               -c:a copy "$filename" < /dev/null &&
            exiftool -TagsFromFile "$1" '-all:all>all:all' \
                     -overwrite_original "$filename"
    then
        # success
        rename 's/.mp4/.success.mp4/' $1
        mv "$filename" $1
        true
    else
        rename 's/.mp4/.error.mp4/' $1
        # failed; destroy the evidence
        rm -f "$filename" 2>/dev/null
        fail "-> Failed to convert"
    fi
}

[ $# -gt 0 ] || fail "Usage: $0 FILE FILE..."

for arg
do
    if [ -f "$arg" ]
    then
        resize "$arg"
        echo
    elif [ -e "$arg" ]
    then
        fail "$arg: not a plain file or directory"
    else
        fail "$arg: file not found"
    fi
done

exec $status # true or false

Is it intended that date is executed separately for each input, or should all converted files have consistent names (for that use of the script)?  It's certainly worth a comment explaining your choice.

I'm not a fan of the big space in the redirection >     /dev/null - that makes it harder to parse.

This if/else chain looks a little clumsy:

if [ "${rotation}" -eq 0 ] || [ "${rotation}" -eq 180 ]
then
    scaling="-1:720"
elif [ "${rotation}" -eq 90 ] || [ "${rotation}" -eq 270 ]
then
    scaling="720:-1"
else
    fail "-> Unhandled rotation"
    return
fi

When testing a single value against various possibilities, it tends to be neater using case:

case "$rotation" in
    0|180) scaling="-1:720" ;;
    90|270) scaling="720:-1" ;;
    *) fail "-> Unhandled rotation '$rotation'"; return ;;
esac

(I also added extra information in the failure case, that may help diagnose problem input files).


More idiomatic way to handle simple file writing in C#? Is there anything you would change about the following function? I welcome any feedback on style, correctness, etc. Thank you for reading.

Some of my questions are:

Obviously, I catch the exception and convert it to a return value because I don't want to take down the application in case of an error. Would it be better to allow the exception to propagate up and handle it there (in the function that is coordinating the creation of the set of import files)?
Is there another, "better-in-general" way of writing to files?

private static bool CreateImportFile(string path, string content)
{
    try
    {
        using (var fileStreamWriter = File.CreateText(path))
        {
            fileStreamWriter.Write(content);
            fileStreamWriter.Close();
        }
    }
    catch (IOException e)
    {
        Console.WriteLine("Failed to write import file due to IOException. " + e.Message);
        return false;
    }

    return true;
}

I think this is more idiomatic but I think this style does not really add much benefit over calling File.WriteAllText directly. Consider what your client code will need to do to handle a single bool result with no other information as to why it failed.

private static bool CreateImportFile(string path, string content)
{
    try
    {
        File.WriteAllText(path, content);
    }
    catch (IOException e)
    {
        Console.WriteLine("Failed to write import file due to IOException. " + e.Message);
        return false;
    }

    return true;
}


Intercept read access to a certain object attribute I have a library that accepts a file-like object but assumes that it has a valid .name unless that name is None. This causes it to break when I pass e.g. someproc.stdout as it has a name of <fdopen>. Unfortunately the attribute cannot be deleted or overwritten.

So I've written this simple wrapper to return None in case the name attribute is requested.

class InterceptAttrWrapper(object):
    def __init__(self, obj, attr, value):
        self.obj = obj
        self.attr = attr
        self.value = value

    def __getattribute__(self, name):
        ga = lambda attr: object.__getattribute__(self, attr)
        if name == ga('attr'):
            return ga('value')
        return getattr(ga('obj'), name)

This seems to work fine, I'm using it like this:

proc = subprocess.Popen(..., stdout=subprocess.PIPE)
some_func(InterceptAttrWrapper(proc.stdout, 'name', None))

However, I wonder if:

My InterceptAttrWrapper object is missing something that could cause problems.
There is a better way to access attributes within __getattribute__. That lambda I'm using right now looks pretty ugly.

Something to watch out for with your approach is that Python calls certain special methods without looking them up via __getattribute__. For example, the __iter__ method:

>>> w = InterceptAttrWrapper(open('test'), 'attr', 'value')
>>> for line in w: print line
...
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: 'InterceptAttrWrapper' object is not iterable

See the documentation on "Special method lookup for new-style classes". If this affects you, then you'll need to add to your wrapper class all the special methods that you need to support (in my example, __iter__, but when wrapping other objects one might need to implement __getitem__, __repr__, __hash__, or others).

Here's a slightly more readable way to get at the wrapper object's own data, together with support for intercepting multiple attributes:

class InterceptAttrWrapper(object):
    """
    Wrap an object and override some of its attributes.

    >>> import sys
    >>> stdin = InterceptAttrWrapper(sys.stdin, name = 'input')
    >>> stdin.mode # Pass through to original object.
    'r'
    >>> stdin.name # Intercept attribute and substitute value.
    'input'
    """
    def __init__(self, obj, **attrs):
        self._data = obj, attrs

    def __getattribute__(self, name):
        obj, attrs = object.__getattribute__(self, '_data')
        try:
            return attrs[name]
        except KeyError:
            return getattr(obj, name)


Encrypting a payload for transmission over HTTP. AES256 with PBKDF2 I've done some reading about implementing AES256 and deriving a key from a password. If I understand correctly:

I want to generate a new salt (for the key) and a new IV (for the encrypted message) for every new message.
It should also not be a problem sending the salt and the IV together with the message.

I decided to pack everything together in one byte array where the first 16 bytes is the salt, the next 16 bytes is the IV and the rest is the encrypted message. This with a key length of 256 bits and 20000 iterations for generating the key. I also encode the whole thing in Base64 for transmission.

Can this approach be improved? Knowing that I'm limited to 256 bytes for the complete message (salt+iv+message).

public class app {

    public static void main(String[] args) throws Exception {
        int iterations = 20000;
        int keyLength = 32;
        byte[] salt = getRandomBytes(16);
        byte[] iv = getRandomBytes(16);
        char[] password = "password_here".toCharArray();
        byte[] payload = "payload_here".getBytes();

        byte[] key = deriveKey(iterations, keyLength, salt, iv, password);
        byte[] encrypted = encrypt(iv, key, payload);
        byte[] bytes = concatBytes(salt, iv, encrypted);

        String output = Base64.encodeBytes(bytes);
        int outputLength = output.getBytes().length;
        System.out.println(outputLength + "\n" + output);
    }

    private static byte[] deriveKey(int iterations, int length, byte[] salt, byte[] iv, char[] password)
            throws InvalidKeySpecException, NoSuchAlgorithmException {
        PBEKeySpec spec = new PBEKeySpec(password, salt, iterations, length * 8);
        SecretKeyFactory skf = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1");
        byte[] key = skf.generateSecret(spec).getEncoded();
        return key;
    }

    private static byte[] getRandomBytes(int length) throws NoSuchAlgorithmException {
        SecureRandom sr = SecureRandom.getInstance("SHA1PRNG");
        byte[] salt = new byte[length];
        sr.nextBytes(salt);
        return salt;
    }

    private static byte[] concatBytes(byte[]... arrays) throws IOException {
        ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
        for (byte[] array : arrays)
            outputStream.write(array);
        return outputStream.toByteArray();
    }

    private static byte[] encrypt(byte[] iv, byte[] key, byte[] text) throws NoSuchAlgorithmException,
            NoSuchPaddingException, InvalidKeyException, InvalidAlgorithmParameterException, IllegalBlockSizeException,
            BadPaddingException {
        AlgorithmParameterSpec ivSpec = new IvParameterSpec(iv);
        SecretKeySpec newKey = new SecretKeySpec(key, "AES");
        Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5Padding");
        cipher.init(Cipher.ENCRYPT_MODE, newKey, ivSpec);
        return cipher.doFinal(text);
    }
}

A few, mostly minor notes:

"payload_here".getBytes();

getBytes() should have been called with a Charset. Otherwise it uses the platform's default charset which can vary from platform to platform and could result to data loss and/or other bugs.
You have local variable called salt inside the getRandomBytes method. It's a little bit confusing since the same method is called for iv too:

byte[] salt = new byte[length];

The encrypt method could throw six different types of exceptions. It's a little bit too much. All of them is a subclass of GeneralSecurityException. I'd use that instead, I don't think that clients want to handle those six different types differently.
According to the Code Conventions for the Java Programming Language class names should start with uppercase letters.

Code Conventions for the Java Programming Language, 9 - Naming Conventions
Effective Java, 2nd edition, Item 56: Adhere to generally accepted naming conventions


Creating custom week start and week number import org.joda.time.DateTime;
import org.joda.time.DateTimeConstants;
import org.joda.time.MutableDateTime;
import org.joda.time.ReadWritableDateTime;
import org.joda.time.Weeks;

import android.content.Context;
import android.content.SharedPreferences;
import android.preference.PreferenceManager;

public class WorkDateUtils {

    private static Context mContext;
    public WorkDateUtils(Context context) {
        mContext = context;
    }

    /**
     *  Get the date of the first work date of the first week of the year.
     *  Set it to 01/01 and count forward.
     *  setting DayOfWeek(FIRST_DAY_OF_WEEK) can get us into last year, and I want
     *  that to be -1 with the first whole week of the current year as 0.
     */
    public static DateTime getFirstWorkDateOfYear(int year, int first_day) {
        ReadWritableDateTime readWritableDateTime = new MutableDateTime(year, 1, 1, 0, 0, 0, 0);
        while (readWritableDateTime.getDayOfWeek() != first_day) {
            readWritableDateTime.addDays(1);
        }
        return readWritableDateTime.toDateTime();
    }
    /**
     * Get the date of the beginning of the week containing the current day.
     *
     */
    public static DateTime getStartOfWorkWeek(DateTime dateTime, int first_day) {
        ReadWritableDateTime readWritableDateTime = dateTime.toMutableDateTime();
        readWritableDateTime.setMillisOfDay(0);
        while (readWritableDateTime.getDayOfWeek() != first_day) {
            readWritableDateTime.addDays(-1);
        }

        // Make sure we have the right week.
        while (readWritableDateTime.isAfter(dateTime)) {
            readWritableDateTime.addWeeks(-1);
        }

        return readWritableDateTime.toDateTime();
    }

    public static Week workWeekNumber(DateTime workDate) {
        SharedPreferences preferences = PreferenceManager.getDefaultSharedPreferences(mContext);
        final Integer FIRST_DAY_OF_WEEK = preferences.getInt("workweek_start", DateTimeConstants.SUNDAY);

        Week workWeek = new Week();
        int week;

        DateTime firstWorkDate = getFirstWorkDateOfYear(workDate.getYear(), FIRST_DAY_OF_WEEK);

        DateTime firstOfWeek = getStartOfWorkWeek(workDate, FIRST_DAY_OF_WEEK);

        workWeek.setWeekStart(firstOfWeek.withTime(0, 0, 0, 0).toString());
        workWeek.setWeekEnd(firstOfWeek.plusDays(6).withTime(23, 59, 59, 999).toString());

        week = Weeks.weeksBetween(firstWorkDate, firstOfWeek).getWeeks();
        workWeek.setWeekNum(week);

        return workWeek;
    }
}

I've run this through tests for entire years and not seen any issues, but this will get called heavily, and hoping to optimize it as much as I can.

the Weeks class is just a holder to pass everything around.

You really don't need to iterate, just do the (mod 7) math to get the correct date directly:

public static DateTime getFirstWorkDateOfYear(int year, int firstDay) {
    ReadWritableDateTime readWritableDateTime = new MutableDateTime(year, 1, 1, 0, 0, 0, 0);
    int doW = readWritableDateTime.getDayOfWeek();
    readWritableDateTime.addDays((7 + firstDay - doW) % 7);
    return readWritableDateTime.toDateTime();
}

public static DateTime getStartOfWorkWeek2(DateTime dateTime, int firstDay) {
    ReadWritableDateTime readWritableDateTime = dateTime.toMutableDateTime();
    readWritableDateTime.setMillisOfDay(0);
    int doW = readWritableDateTime.getDayOfWeek();
    readWritableDateTime.addDays(((firstDay - doW - 7) % 7));
    return readWritableDateTime.toDateTime();
}

The technique proposed is not particularly dependent on which date library you use; with minor variations it can be applied to normal Date or Calendar types.  While I like joda, in general, I have no opinion on its performance on Android.


Shortest distance from point to line segment in 3D This function returns the shortest distance from a 3D point P to a line segment defined by two 3D points A and B. It projects P onto the line, then checks if this projected point lies between A and B. If it does, then the distance is calculated using this projected point.

If the projection is outside the line segment, then the distance is calculated to either point A or B, depending on which is closer.

I wanted to get feedback on if this is a clean solution to the problem.

function d = point_seg_dist(P, A, B)
% Returns distance from a 3D point to a line segment
%
% Inputs
%
%   P : vector
%       Position in space
%
%   A : vector
%       Position of line segment endpoint
%
%   B : vector
%       Position of other line segment endpoint
%
% Outputs
%
%   d : double
%       Distance from point to line segment

AP = P - A; % Vector from A to point
AB = B - A; % Vector from A to B

% Project point onto line
P_line = A + dot(AP, AB) / dot(AB, AB) * AB;

if all(A < P_line) && all(P_line < B)
    % The point projected onto the line is in between A and B

    % Projection of point onto segment is the same
    % as projection of point onto line
    P_seg = P_line;
else
    % The point projected onto the line is outside of A and B

    if all(P_line <= A)
        % The projected point is closer to A
        P_seg = A;
    else
        % The projected point is closer to B
        P_seg = B;
    end
end

d = norm(P - P_seg); % Distance to line segment

end

Here is the implementation of the solution provided by Wolfram Alpha, as was suggested in the comments of the OP.

The implemented equation is:

$$ d=\frac{|(\mathbf{x}_2-\mathbf{x}_1)\times(\mathbf{x}_2-\mathbf{x}_1)|}{|\mathbf{x}_1-\mathbf{x}_0|} $$

where

$$x_1=(x_1,y_1,z_1)$$
$$x_2=(x_2,y_2,z_2)$$

are two points on the line and \$x_0\$ is a third point.

function d = point_seg_dist(x0, x1, x2)

    % Make sure we are in 3D
    if length(x0) == 2
        x1(3) = 0;
        x2(3) = 0;
        x0(3) = 0;
    end

    % We want column arrays
    if size(x0,1) < size(x0,2)
         x0 = P';
         x1 = x1';
         x2 = x2';
    end

    d = norm(cross(x2-x1 , x1-x0)) / norm(x2-x1);

Apart from a couple of checks on the input, the calculation is on one line and it does not need projections or checks on the projected point's position.


Create a directed graph and compute in degrees in Python I have implemented a couple of functions in python as part of an online course. Although the functions seem to work fine I feel that the code is inefficient with a lot of nested loops. Are there any improvements I can make? I am new to the language so any advice is welcome.

Here is the code and some simple tests:

def make_complete_graph(num_nodes):
    """
    Takes the number of nodes num_nodes
    and returns a dictionary corresponding to a complete directed graph
    with the specified number of nodes
    """

    result = dict()
    for i in range(num_nodes):
        set_per_node = set()
        for num in range(num_nodes):
            if num != i:
                set_per_node.add(num)
        result[i] = set_per_node

    return result

def compute_in_degrees(digraph):
    """
    Takes a directed graph digraph (represented as a dictionary) and computes
    the in-degrees for the nodes in the graph.
    Returns a dictionary with the same set of keys (nodes) as digraph
    whose corresponding values are the number of edges
    whose head matches a particular node.
    """
    result = dict()
    count = 0
    for key in digraph:
        count = 0
        for key2, value in digraph.iteritems():
            if key != key2:
                for val in value:
                    if val == key:
                        count += 1
        result[key] = count
        count = 0

    return result

class TestStringMethods(unittest.TestCase):

    def test_make_complete_graph(self):
        expected_dict_one = {0: set([1, 2]), 1: set([0, 2]), 2: set([0, 1])}
        expected_dict_two = {}

        actual_dict_one = at_project1.make_complete_graph(3)
        actual_dict_two = at_project1.make_complete_graph(0)

        self.assertEquals(actual_dict_one, expected_dict_one)
        self.assertEquals(actual_dict_two, expected_dict_two)

    def test_compute_in_degrees(self):
        given_dict_one = {0: set([1, 2]), 1: set([0, 2]), 2: set([0, 1])}
        given_dict_two = {0: set([1, 2]), 1: set([]), 2: set([])}

        expected_dict_one = {0: 2, 1: 2, 2: 2}
        expected_dict_two = {0: 0, 1: 1, 2: 1}

        actual_dict_one = at_project1.compute_in_degrees(given_dict_one)
        actual_dict_two = at_project1.compute_in_degrees(given_dict_two)

        self.assertEquals(actual_dict_one, expected_dict_one)
        self.assertEquals(actual_dict_two, expected_dict_two)

if __name__ == '__main__':
    unittest.main()

In make_complete_graph, the inner loop could be written with a list comprehension:

result[i] = set([num for num in range(num_nodes) if num != i])

In compute_in_degrees, instead of iterating over all nodes for all nodes x to find links going to x, you could build a dictionary of counts easier:

counts = dict([(x, 0) for x in digraph])
for targets in digraph.values():
    for target in targets:
        counts[target] += 1


Is this a good implementation of a thread-safe singleton using the observer pattern? I need a singleton that can safely operate in a multi-thread environment. Threading and concurrency is new to me, so I'm not sure if this implementation holds. Take a look:

public class ObserverSingleton {

    private static ObserverSingleton _instance;
    private ArrayList<OnRequestFinishedListener> _listeners;

    private ObserverSingleton() {
        _listeners = new  ArrayList<OnRequestFinishedListener>();
    }

    public static synchronized ObserverSingleton getInstance() {
        if(_instance == null) {
            _instance = new ObserverSingleton();
        }

        return _instance;
    }

    public synchronized void addOnRequestFinishedListener(OnRequestFinishedListener listener) {
        // Check if listener already exists in list, skip if it does
        for (OnRequestFinishedListener listListener : _listeners) {
            if(listListener.equals(listener)) {
                return;
            }
        }

        _listeners.add(listener);
    }

    public synchronized void removeOnRequestFinishedListener(OnRequestFinishedListener listener) {
        // Check if listener exists in list, remove if it does
        for (OnRequestFinishedListener listListener : _listeners) {
            if(listListener.equals(listener)) {
                _listeners.remove(listListener);
            }
        }
    }

    private void notifyListeners() {
        for (OnRequestFinishedListener listener : _listeners) {
            listener.onRequestFinished();
        }
    }

    public Object clone() throws CloneNotSupportedException {
        throw new CloneNotSupportedException();
    }

    public interface OnRequestFinishedListener {

        public void onRequestFinished();

    }

}

Any feedback?

The notifyListeners() method is private, and is never called anywhere. It should be synchronized on the same lock than the other methods using the list of listeners.

Unless lazy instantiation is really needed, I wouldn't use it. Just use the following code:

private static final ObserverSingleton INSTANCE = new ObserverSingleton();

public static ObserverSingleton getInstance() {
    return INSTANCE;
}

Also, I would use a Set<OnRequestFinishedListener> rather than a list, since your add method makes sure there is only one copy of a listener inside the list, and Set is precisely used for that. If order of insertion matters, use a LinkedHashSet<OnRequestFinishedListener>.

Declaring the List as ArrayList is bad practice. You should declare it as List, since the concrete type doesn't matter.


Dependency injection thread safety Is it OK to mix different lifetime of ISerializer and ICacheProvider in this case? I am using Unity DI. In this case, whenever ICacheProvider is needed, Unity will always give the same instance (singleton) but ISerializer will always be a new instance. I am not sure if it would be OK to have ISerializer as a singleton as well in an ASP.NET Web API environment.

public class MyCacheProvider : ICacheProvider
{
   private readonly ISerializer _serializer;

   public MyCacheProvider(ISerializer serializer)
   {
       _serializer = serializer;
   }
}

Serializer implementation:

public class JilSerializer : ISerializer
    {
        public string Serialize<T>(T instance)
        {
            return JSON.Serialize(instance);
        }

        public T Deserialize<T>(string input)
        {
            return JSON.Deserialize<T>(input);
        }
    }

Configuring DI Container to provide instances:

container.RegisterType<ISerializer, JilSerializer>();
            container.RegisterType<ICacheProvider, MyCacheProvider>(new ContainerControlledLifetimeManager());

Your MyCacheProvider will hold an instance of JilSerializer and since your MyCacheProvider is a singleton - there's going to be an instance of JilSerializer around forever as well.

As it's apparently fine for your ICacheProvider to use the same serializer throughout the lifetime of the app, and the implementation looks completely fine for sharing an instance, I don't see any reason why you can't just make ISerializer a singleton too.

I think that this isn't quite a captive dependency as the default lifetime of ISerializer is transient but it's certainly a related topic that you can read about on Mark Seemann's blog.


How to rewrite recursion in a more ruby way def get_all_friends(uid, client)
  friends = client.friendships.friends(uid: uid)
  total = friends.total_number
  get_friends(total, 0, uid, client)
end

def get_friends(total, cursor, uid, client)
  left = total
  count = [200, left].min

  friends = client.friendships.friends(uid: uid, count: count, cursor: cursor)
  left -= count
  next_cursor = friends.next_cursor

  return friends if (left == 0 || next_cursor == 0)

  more_firends = get_friends(left, next_cursor, uid, client)
  more_firends.users << friends.users
  return more_firends
end

get_all_friends(uid, observer.client).users.map do |info|
  user = User.find_or_create_by_uid_and_name(uid: info.id, name: info.name)
  follow(user)
  user
end

This code is written in a functional style. Maybe there is a more natural way to structure it with Block.

And how can I reuse the code when client.friendships.friends(uid: uid, count: count, cursor: cursor) differs?

Update:

I reconstruct the code. It now looks more clean and general.

def get_all_friends(client, uid)
  fetch_all do |count, cursor|
    client.friendships.friends(uid: uid, count: count, cursor: cursor)
  end
end

def fetch_all(&fetch_proc)
  res = fetch_proc.call(10, 0)
  total = res.total_number
  return res if total <= 10
  fetch(total, 0, &fetch_proc)
end

def fetch(total, cursor, &fetch_proc)
  left = total
  count = [200, left].min

  res = fetch_proc.call(count, cursor)
  left -= count
  next_cursor = res.next_cursor

  if (left == 0 || next_cursor == 0)
    return res
  end

  more_res = fetch(left, next_cursor, &fetch_proc)
  more_res.users << res.users
  more_res
end

It looks pretty good to me. Some notes:

I'd avoid return on the last expression
I'd avoid the "-=" (not very functional)
I'd avoid inline returns (write a full-fledged conditional instead)
I have my doubts about this more_friends.users <<, is this a side-effect you are doing behind the caller's back?

As a general note, Ruby is an OOP language, you can strive for functional style (I do whenever possible), but still, use also an OOP structure or you'll end up with weird undiomatic code.


Generate mask for non zero (or inverse) nibbles in integer Apologies for the cryptic title, but the problem really is rather simple.
I have a 64 bit integer which I interpret as 16 4 bit nibbles, or a nibble array if you wish, and I needed generate a mask covering the nibbles which are zero, or the inverse.
I have already come up with a solution, but it seems to me that there has to be a better way.
uint64_t getNonZeroMask( uint64_t b )
{
    return ( ( ( b >> 2 | b ) >> 1 | ( b >> 2 | b ) ) & 0x1111111111111111 ) * 15;
}

The above is the most efficient I've found. I've tried numerous approaches, but they pretty much all use the same approach.
Just in case there should be any doubt what exactly I'm frying to do, a give value:
ex.
0x010203456000abc0

Should give the mask:
0x0f0f0ffff000fff0

or the inverse.
What I have works, of course, but I do believe it can be done better, and I'd very much like to know how. Preferably it would only involve bitwise operations. Intrinsics from bmi and similar are okay, but I haven't found anything that seems useful. It may be that I'm entirely in the wrong, and this is indeed the way of doing it, in which case I'd very much like to know that too.

Intrinsics from bmi and similar are okay, but I haven't found anything that seems useful.

They're not very useful for this. pdep and pext let you do this in a different way, but it's not better (especially not on AMD Zen1), you shouldn't use this (but maybe it's interesting to think about):
uint64_t m = 0x1111111111111111;
uint64_t stacked = _pext_u64(b, m) | _pext_u64(b, m << 1) |
                   _pext_u64(b, m << 2) | _pext_u64(b, m << 3);
return _pdep_u64(stacked, m) * 15;

There is a slightly different way to write the original logic in a way that may be easier to read, of course that's subjective but it will be as a straight-line composition of common bitwise idioms, it shouldn't make any difference for a compiler. It's the same thing really, but instead of being a single expression it's linearized into discrete steps, which also creates the opportunity for in-line comments to explain the steps:
uint64_t getNonZeroMask( uint64_t b )
{
    // horizontal OR of nibbles in log(bits) steps
    b |= b >> 2;
    b |= b >> 1;
    // remove polluted bits, leaving the results in the lsb of every nibble
    b &= 0x1111111111111111;
    // spread single bit flags into full nibbles
    return b * 15;
}


Metal King: A JavaScript Game I started to learn programming just a few days ago. And I've written code for a simple original game for practice, using some knowledge of JavaScript that I have gained so far.

Is the following code properly written? I'm especially concerned about the use of "Math.floor", "Math.random", "function", and "if / else".

Incidentally, "Metal King" is the name of a monster in the game, and "Angelo" is a character.

alert("Metal Kings (A, B, and C) have appeared!");

// Which Metal King (A, B, or C) will Angelo decide to attack? \
// 33% probability for each.

var MetalKingABC = 1 + Math.floor(Math.random() * 3);

if (MetalKingABC === 1) {
    MetalKingABC = "Metal King A";
}
else if (MetalKingABC === 2) {
    MetalKingABC = "Metal King B";
}
else {
    MetalKingABC = "Metal King C";
}

var WhichOneToAttack = function(MonsterName) {

    alert("Angelo is about to attack " + MonsterName + ".");

    // Metal King has a 50% chance of fleeing the battle.

    var FleeOrFight = 1 + Math.floor(Math.random() * 2);

    if (FleeOrFight === 1) {
        alert(MonsterName + " flees like lightning!");
        // The battle ends here before Angelo attacks.
    }

    else // Metal King stays, and Angelo attacks.
    {
        // 70% chance of MISS, 20% of ONE damage, 10% of Critical.

        var MissOneCritical = 1 + Math.floor(Math.random() * 10);

        if (MissOneCritical <= 7) {
            alert(MonsterName + " swiftly evades Angelo's attack.");
        }

        else if (MissOneCritical <= 9) {
            alert("Angelo inflicts 1 point of damage on " + MonsterName + ".");
        }

        else {
            // The damage dealt by a critical hit can range from 197 to 256 points.

            var CriticalRange = 197 + Math.floor(Math.random() * 60);

            alert("Angelo lands a critical hit on " + MonsterName + ", dealing massive \
            damage of " + CriticalRange + " points! Angelo defeats " + MonsterName + ".");

            // When defeated, Metal King drops a shield with an 80% probability.

            var ShieldDrop = 1 + Math.floor(Math.random() * 10);

            if (ShieldDrop <= 8) {
                alert(MonsterName + " drops a shield. Angelo obtains a Diamond Shield!");
            }
            else {
            // No code is written in this else portion, since Metal King didn't drop \
            // any item.
            }
        }
    }
};

// Finally, call the function.

WhichOneToAttack(MetalKingABC);

First, your game doesn't have a name! (Which is why I renamed the question like that...)

A few direct things to focus on:

if (MetalKingABC === 1) {
    MetalKingABC = "Metal King A";
}
else if (MetalKingABC === 2) {
    MetalKingABC = "Metal King B";
}
else {
    MetalKingABC = "Metal King C";
}

This is not how you do that.

Instead of using 1 + Math.floor get rid of the 1 as 0 is a valid array index, and is helpful in many situations.

Instead of using * 2, alongside an array, use * array.length.

Instead of using an if-else loop with duplicate logic (string assignment), use an array instead.

var metalKingTypes = ['A', 'B', 'C'];
var metalKing = metalKingTypes[Math.floor(Math.random() * metalKingTypes.length)];

Instead of using MissOneCritical: use a switch

After you decide who to attack, and then attack them, the game is over! Even if you miss or only do 1 damage!

Set a isActive variable to keep the game going.

A few indirect things:

WhichOneToAttack: could be better named: DecideOpponent
FleeOrFight: You naming structure should be camelCase where the first word is entirely lowercase.
You can use this to get a random true or false value: Math.floor(Math.random() * 2)

Your structure:

You have a bunch of loose functions that aren't linked to one another in any other way than the fact that they call each other.

Use a class structure, or a prototype chain, that way you can avoid passing parameters around:

Taking those changes into consideration would give you something like this:

    function Game(){
        alert("Metal Kings (A, B, and C) have appeared!");

        /* Which Metal King (A, B, or C) will Angelo decide to attack?
         * 33% probability for each.
         */

        var metalKingTypes = ['A', 'B', 'C'];
        this.monster = "Metal King " + metalKingTypes[Math.floor(Math.random() * metalKingTypes.length)];
        this.isActive = true;
        while (this.isActive) {
            this.DecideOpponent();
        }
    }
    Game.prototype.DecideOpponent = function(){
        alert("Angelo is about to attack " + this.monster + ".");

        // Metal King has a 50% chance of fleeing the battle.

        var fleeOrFight = Math.floor(Math.random() * 2);

        if (fleeOrFight) {
            alert(this.monster + " flees like lightning!");
            // The battle ends here before Angelo attacks.
            return false;
        }
        // 70% chance of MISS, 20% of ONE damage, 10% of Critical.
        var attackOptions = ['evade', 'normal', 'critical'];

        var attack = attackOptions[Math.floor(Math.random() * attackOptions.length)];
        switch (attack) {
            case 'evade':
                alert(this.monster + " swiftly evades Angelo's attack.");
                break;
            case 'normal':
                alert("Angelo inflicts 1 point of damage on " + this.monster + ".");
                break;
            case 'critical':
                var CriticalRange = 197 + Math.floor(Math.random() * 60);

                alert("Angelo lands a critical hit on " + this.monster + ", dealing massive " +
                      "damage of " + CriticalRange + " points! Angelo defeats " + this.monster + ".");

                // When defeated, Metal King drops a shield with an 80% probability.

                var shieldDrop = Math.floor(Math.random() * 2);

                if (shieldDrop) {
                    alert(this.monster + " drops a shield. Angelo obtains a Diamond Shield!");
                }
                this.isActive = false;
                break;
        }
    };

    // Finally, call the function.

    var g = new Game();

Additionally, instead of using alert consider building each line of the game to the console or to a HTML element.

Something like:

function buildToHTML(text){
    var field = document.getElementById('gameField');
    field.text += text;
}


Check two stacks for equality Write a method equals that takes as parameters two stacks of integers
  and returns true if the two stacks are equal and that returns false
  otherwise. To be considered equal, the two stacks would have to store
  the same sequence of integer values in the same order. Your method is
  to examine the two stacks but must return them to their original state
  before terminating. You may use one stack as auxiliary storage.

I'm pretty new to stacks and queues, and data structures in general. I'd like to know if there are other ways I can do this that are easier or better for when I'm dealing with stacks in the future. Also, is my code readable? Does everything "make sense"? And can my code be easily optimized?

public boolean equals(Stack<Integer> s1, Stack<Integer> s2) {
    Queue<Integer> q1 = new LinkedList<Integer>();
    Queue<Integer> q2 = new LinkedList<Integer>();
    boolean equal = true;

    while (!s1.isEmpty() && !s2.isEmpty()) {
        int firstNum = s1.pop();
        int secondNum = s2.pop();
        q1.offer(firstNum);
        q2.offer(secondNum);
        if (firstNum != secondNum) {
            equal = false;
        }
    }

    // used to preserve stack structure
    if (s1.isEmpty() && !s2.isEmpty()) {
        while(!s2.isEmpty()) {
            q2.offer(s2.pop());
        }
        equal = false;
    }

    if (s2.isEmpty() && !s1.isEmpty()) {
        while(!s1.isEmpty()) {
            q1.offer(s1.pop());
        }
        equal = false;
    }

    // add back to stack using s -> q -> s
    // for both stacks
    while (!q1.isEmpty()) {
        s1.push(q1.remove());
    }
    while (!s1.isEmpty()) {
        q1.offer(s1.pop());
    }
    while (!q1.isEmpty()) {
        s1.push(q1.remove());
    }

    while (!q2.isEmpty()) {
        s2.push(q2.remove());
    }
    while (!s2.isEmpty()) {
        q2.offer(s2.pop());
    }
    while (!q2.isEmpty()) {
        s2.push(q2.remove());
    }

    return equal;
}

Your code breaks the rules slightly by using two queues as auxiliary storage - I've done it with the allowed single stack. As you can see the first part of the code is very similar to yours, but the reconstruction and equals checking are simpler. By knowing that I always put the elements from s1 first, I need to reconstruct the other way around so s2 first later on from aux.

The code works nicely with empty inputs and puts them back in the correct order

    public boolean equals(Stack<Integer> s1, Stack<Integer> s2){
    Stack<Integer> aux = new Stack<>();
    boolean equal = true;
    while(!s1.isEmpty() && !s2.isEmpty()){
        int first = s1.pop();
        int second = s2.pop();
        aux.push(first);
        aux.push(second);
        if(first != second){
            equal = false;
            break;
        }
    }
    // Equal if both stacks are empty
     if(!(s1.isEmpty() || s2.isEmpty())){
        equal = false;
     }

    // Reconstruct
    while(!aux.isEmpty()){
        s2.push(aux.pop());
        s1.push(aux.pop());
    }
    return equal;
}


Meta-class to allow inspection of all __slots__of a class If classes provide __slots__ and at the same time inherit from another class also providing __slots__, there is no straightforward way to access and list each and every slot of the class. Especially when you consider chaining of inheritance.

I've written this meta-class that adds the __all_slots__ property, listing all unique slots:

class MetaSlotMachine(type):
    """Meta-class that adds the attribute `__all_slots__` to a class.

    `__all_slots__`  is a set that contains all unique slots of a class,
    including the ones that are inherited from parents.

    """
    def __init__(cls, name, bases, dictionary):
        super(MetaSlotMachine, cls).__init__(name, bases, dictionary)
        slots_iterator = (getattr(c, '__slots__', ()) for c in cls.__mro__)
        # `__slots__` might only be a single string,
        # so we need to put the strings into a tuple.
        # `basestring` becomes just `str` in Python 3
        slots_converted = ((slots,) if isinstance(slots, basestring) else slots
                                    for slots in slots_iterator)
        cls.__all_slots__ = set()
        cls.__all_slots__.update(*slots_converted)

Let's consider classes that inherit slots, like the following:

class HasSlots1(object):
    __metaclass__ = MetaSlotMachine   # Python 2 Syntax
    __slots__ = ['x', 'y']

class HasSlots2(HasSlots1):
    __slots__ = 'zz'

class HasSlots3(HasSlots2):
    __slots__ = ()

If we create and instance via

myslots = HasSlots3()

and ask for

myslots.__slots__

Python will return the empty tuple:

 ()

However, now we can get hold of all slots via

myslots.__all_slots__

which returns all inherited slots:

{'y', 'x', 'zz'}

Is there anything flawed with the design of the meta-class? What about listing all slots based on the __mro__ in _get_all_slots function?

EDIT: Revised version with gathering slots within the init function. Moreover, special care is taken about __slots__ that only consist of a single string, like __slots__ = '__weakref__'.

This is a clever use of metaclasses - in a good way. The only thing I would change is to use set().union over update (this is debatable).


Sum numbers input from terminal The following Golang code accepts a sequence of numbers (any integer and float type) in the terminal, adds them, and prints the result (sum) to the terminal.

Actually in Golang you can't add different integer types together, because of which this script accepts all the digits as a string representation of space-separated digits, then splits it on the spaces into int64 digits. It then sums all of the digits and writes the result to the terminal.

Is there any way to improve this code?

package main

import (
    "bufio"
    "fmt"
    "os"
    "strconv"
    "strings"
)

func main() {
    a := ReadInput()
    fmt.Println("Here is the result:", AddAll(a))
}

func ReadInput() []float64 {
    reader := bufio.NewReader(os.Stdin)
    fmt.Println("Enter digits to be added (space separated): ")
    text, err := reader.ReadString('\n')
    if err != nil {
        fmt.Println(err)
    }

    textSlice := strings.Fields(text)
    floatsSlice := make([]float64, 0)
    for _, elem := range textSlice {
        i, err := strconv.ParseFloat(elem, 64)
        if err != nil {
            fmt.Println(err)
        }
        floatsSlice = append(floatsSlice, i)
    }

    return floatsSlice
}

func AddAll(floatsSlice []float64) float64 {
    var sum float64
    for _, v := range floatsSlice {
        sum += v
    }
    return sum
}

For example if we enter the following,

1 2.3345 2.5 3.3 8.9 0.5 23

The output will be 41.5345

No sense send slice of numbers into the summation function. Would be better pass the string and work with it. This will avoid necessary  enumeration of the elements of the slice and return result.
No need to create a slice with 0 capacity,because it will increase costly. In your case, it was possible to write so:

floatsSlice := make([]float64, len(textSlice))
 ...
 for index, elem := range textSlice {
     ...
    floatsSlice[index] = i
 }

No need to create extra variable

fmt.Println("Here is the result:", AddAll(ReadInput()))

Putting it all together and the result is:

func getline() (string, error) {
    return bufio.NewReader(os.Stdin).ReadString('\n')
}

func sumNumbers(str string) float64 {
    var sum float64
    for _, v := range strings.Fields(str) {
        i, err := strconv.ParseFloat(v, 64)
        if err != nil {
            fmt.Println(err)
        } else {
            sum += i
        }
    }
    return sum
}

Call:

str, err := getline()
if err == nil {
    fmt.Println("Here is the result:", sumNumbers(str))
}


Python palindrome program I have written a palindrome program. And I was just wondering if it correct? I checked using the words racecar and madam, and it says they are palindromes. I have also tested nonpalindrome words and it worked correctly.

Is my implementation correct? If it's not could you correct it for me. It it works correct, is there an easier way to implement this?

def is_palindrome(x):

    for i in range(0,len(x)):
        x = x.lower()
        if x[i::1] == x[::-1]:
            return("%s Is a palindrome" %(x))
        else:
            return("%s Is not a palindrome" %(x))

print(is_palindrome('racecar'))

I would expect a function that has a "predicate" name (is_x()) to return a boolean value rather than a string.  The boolean is much more useful to the caller.
We should add a main-guard to top-level code, so the file could be useful as a module.
Consider adding some unit tests.  A convenient way to do this is using the doctest module, which finds tests in the function's docstring; any line beginning with >>> is a test, and is immediately followed by the expected result.
Building on the version shown by C.Nivs, that would give this improved version:
def is_palindrome(x):
    '''
    True if the letters of x are the same when
    reversed, ignoring differences of case.
    >>> is_palindrome('')
    True
    >>> is_palindrome('a')
    True
    >>> is_palindrome('a0')
    False
    >>> is_palindrome('ab')
    False
    >>> is_palindrome('ab a')
    False
    >>> is_palindrome('Aba')
    True
    >>> is_palindrome('Àbà')
    True
    '''
    x = x.lower()
    # Alternative (letters only):
    #     x = ''.join(filter(str.isalpha, x.lower()))
    # Alternative (letters and digits only):
    #     x = ''.join(filter(str.isalnum x.lower()))
    return x[::-1] == x

if __name__ == '__main__':
    import sys
    for string in sys.argv[1:]:
        # check each command-line argument
        message = 'is a palindrome' if is_palindrome(string) else 'is not a palindrome'
        print(f'{string} {message}')
    else:
        # no arguments, so run self-test
        import doctest
        doctest.testmod()

It may well make sense to separate the pre-processing from the palindrome checking, so that is_palindrome() simply returns x[::-1] == x, and the caller is responsible for deciding which characters are significant.  We could call it with a specific canonicalisation:
is_palindrome(string.lower())

More advanced would be to pass a normalisation function and/or a character filter, as optional arguments:
def is_palindrome(s, normalise=None, char_filter=None):
    '''
    True if the letters of x are the same when
    reversed, ignoring differences of case.
    >>> is_palindrome('')
    True
    >>> is_palindrome('a')
    True
    >>> is_palindrome('a0')
    False
    >>> is_palindrome('a0', char_filter=str.isalpha)
    True
    >>> is_palindrome('ab')
    False
    >>> is_palindrome('ab a')
    False
    >>> is_palindrome('Aba')
    False
    >>> is_palindrome('Aba', normalise=str.lower)
    True
    >>> is_palindrome('Àbà', normalise=str.lower)
    True
    '''
    if normalise:
        s = normalise(s)
    if char_filter:
        s = ''.join(filter(char_filter, s))
    return s == ''.join(reversed(s))

Usage of these optional arguments should be apparent from the extra tests I've added.

Bug
Whether we use [::-1] or reversed() to reverse the input string, we have a bug when the input contains characters that are formed using combining accents, as demonstrated by these additional tests:
>>> is_palindrome("o̅o") # o+overline, o
False
>>> is_palindrome("o̅o̅") # o+overline, o+overline
True

Both of these fail, because we reverse the string by treating each Unicode character independently, rather than considering them as (possibly multi-character) letters.  To fix that, we'll need to find or create a combining-character-aware function to reverse a string - I'm not aware of such a thing in the Python standard library.


Given a Haskell list, return all sub-lists obtained by removing one element I am trying to write a Haskell function which takes a list ls and returns all sub-lists obtained by removing one element from ls. An additional constraint is that the returned list of lists must be in the order of the missing element.

Here is what I have. I know there must be a simpler solution.

subOneLists :: [a] -> [[a]]
subOneLists ls = let  helper :: [a] -> [a] -> [[a]] -> [[a]]
                      helper _ [] ss = ss
                      helper ps (x:xs) ss = helper ps' xs ss'
                        where ps' = ps ++ [x]
                              ss' = ss ++ [ps ++ xs]
                 in helper [] ls []

λ> subOneLists [1, 2, 3, 4]
[[2,3,4],[1,3,4],[1,2,4],[1,2,3]]

Here's a simpler way to implement it:

subOneLists :: [a] -> [[a]]
subOneLists [] = []
subOneLists (x:xs) = xs : map (x :) (subOneLists xs)


Repeatedly partitioning an array equally as far as possible I solved HackerRank Nikita and the Game.  The implementation is correct as program passes all test cases.

  Nikita just came up with a new array game. The rules are as follows:

  Initially, there is an array, A, containing N integers (1 ≤ N ≤ 109), with 0 ≤ Ai ≤ 109.
  In each move, Nikita must partition the array into 2 non-empty parts
  such that the sum of the elements in the left partition is equal to
  the sum of the elements in the right partition. If Nikita can make
  such a move, she gets 1 point; otherwise, the game ends.
  After each successful move, Nikita discards either the left partition
  or the right partition and continues playing by using the remaining
  partition as array A.

  Nikita loves this game and wants your help getting the best score
  possible. Given A, can you find and print the maximum number of points
  she can score?

import java.io.*;
import java.util.*;

public class NikitaAndTheGame
{

public static void main(String[] args)
{
    Scanner sc=new Scanner(System.in);
    int cases=sc.nextInt();
    for(int i=0;i<cases;i++)
    {
        int size=sc.nextInt();
        int[] array=new int[size];

        for(int j=0;j<size;j++)
        {
            array[j]=sc.nextInt();
        }

    System.out.println(splitPossible(array,0,size-1));

    }
}
public static int splitPossible(int[] array,int start,int end)
//a recursive fuction that returns 1 if equal sum split possible otherwise returns 0
{
    int size=end-start+1;
    int sum=0;
    int count=0;
    if(sumOfArray(array,0,array.length-1)==0&&start==0&&end==array.length-1) //handles the case when all elements of array are 0.

   {
       return array.length-1;
   }
    if(size==0)
    {
        return 0;
    }
    for(int i=0;i<size;i++)
    {
        if(sumOfArray(array,start,start+i)!=sumOfArray(array,start+i+1,end))
        {
            continue;
        }
        else
        {
            count+=1;
            return count+Math.max(splitPossible(array,start,start+i),splitPossible(array,start+i+1,end));
        }
    }

    return count;
}
public static int sumOfArray(int[] array,int start,int end)
//returns sum of array from start to end.
{
    int sum=0;
    for(int j=start;j<=end;j++)
    {
        sum+=array[j];
    }
    return sum;
}
}

I am new to recursion, so any input in designing of recursive method/assigning or simplifying variables/simpler coding styles during recursion are welcomed.

The implementation of the algorithm, using a helper recursive method, is very clear. It is easy to see what the code is doing:

There are 2 base cases: there are only 0s, in which case the size of the array is the score, and the size is zero, in which case the score is 0.
Then, for all possible sizes, we keep the maximum of the score of the left array versus the right array when they have equal sum, adding 1.

There are simplifications possible, and improvements in terms of style and namings.

Namings

The first point to consider is naming: splitPossible is a terrible name for this method. It doesn't convey what it's doing. Rename it to getMaxScore, since this is what it does: gets the maximum score for the given array in the given bounds.

Also, consider "hiding" the recursive implementation inside a new method by creating a method passing the initial values:

public static int getMaxScore(int[] array) {
    return getMaxScore(array, 0, array.length - 1);
}

private static int getMaxScore(int[] array, int start, int end) {
    // rest of code
}

This way, the caller just needs to invoke getMaxScore(array), without worrying about the initial value of the recursive algorithm; and it gives you the possibility to update the code in the future (to an iterative method for example) without changing the calling code.

Style

You should always reformat your code in order to have proper indentation. Right now, it isn't the case and makes the code harder to read for no reason.

int sum = 0;

is unused in the program, so you should remove it.

if (sumOfArray(array, start, start + i) != sumOfArray(array, start + i + 1, end)) {
  continue;
} else {
  count += 1;
  return count + Math.max(getMaxScore(array, start, start + i), getMaxScore(array, start + i + 1, end));
}

is the logic for the recursive calls. In this case, there is no need to use a continue statement: those can sometimes help understanding the logic when there would be a lot of code in the else branch. They introduce a sort of early-return inside the for loop. But in this case, it clutters up a bit, and you can simply have:

if (sumOfArray(array, start, start + i) == sumOfArray(array, start + i + 1, end)) {
    count += 1;
    return count + Math.max(getMaxScore(array, start, start + i), getMaxScore(array, start + i + 1, end));
}

A couple of more comments here: count += 1; is more generally written count++ (or ++count), but in fact, there is no need for this as you can just add 1 to the result of the next line:

return 1 + count + Math.max(getMaxScore(array, start, start + i), getMaxScore(array, start + i + 1, end));

Arguably, this line is starting to become a bit crowded, so it is possible to store the two intermediate recursive results in temporary variables:

int leftScore = getMaxScore(array, start, start + i);
int rightScore = getMaxScore(array, start + i + 1, end);
return 1 + count + Math.max(leftScore, rightScore);

Improvements

You have an early return in case the size is 0, to return 0. While that can make the code easier to follow in certain situations, in this case, it's not needed at the rest of the code handles size = 0 just fine. Actually, the for loop is not entered, and we directly return the count, which is 0. Consider removing this special case (it also doesn't lead to performance improvements).

But why is there a count variable in the first place? It's initialized to 0, and in fact never changes. It's actually really unneeded: when making the recursive call, the total score is always the maximum score of the left and right split, plus 1. You can safely remove this count variable.

The first if, in case the array only contains 0, can be written more simply as well: start == 0 && end == array.length - 1 is equivalent to size == array.length.

The recursive method now looks like:

public static int getMaxScore(int[] array, int start, int end) {
    int size = end - start + 1;
    if (sumOfArray(array, 0, array.length - 1) == 0 && size == array.length) {
        return array.length - 1;
    }
    for (int i = 0; i < size; i++) {
        if (sumOfArray(array, start, start + i) == sumOfArray(array, start + i + 1, end)) {
            int leftScore = getMaxScore(array, start, start + i);
            int rightScore = getMaxScore(array, start + i + 1, end);
            return 1 + Math.max(leftScore, rightScore);
        }
    }
    return 0;
}

Finally, something can be done to simplify this first early-return. When all the elements in the array are 0, the recursive calls inside the for loop handle the case fine as well: they will increment the count by 1 each time, finally ending up with array.length - 1. But we can't get total rid of it: the way the algorithm is build, splitting each array into smaller chunks, the special case is when size == 1, and in this case, it is impossible to split it evenly, so the answer is 0. So we could have

if (size == 1) {
    return 0;
}

which avoids traversing the array entirely.

Further improvements

It might sound a bit strange to you, as it probably is to new developers, but a lot of APIs, for all programming languages, very often deal with ranges in an open-half manner: when asked for something in the range from start to end, it will generally be implied start inclusive to end exclusive (like Python slice, C++ APIs, and in Java, look at Random.nextInt, IntStream.range, List.subList or String.substring). The current code implies that the end is inclusive, which, actually, makes it that bit harder to follow for those accostumed to those practices. And switching to an exclusive end will make the code even easier:

// start inclusive, end exclusive
public static int getMaxScore(int[] array, int start, int end) {
    for (int i = 1; i < end - start; i++) {
        int split = start + i;
        if (sumOfArray(array, start, split) == sumOfArray(array, split, end)) {
            return 1 + Math.max(getMaxScore(array, start, split), getMaxScore(array, split, end));
        }
    }
    return 0;
}

// start inclusive, end exclusive
public static int sumOfArray(int[] array, int start, int end) {
    int sum = 0;
    for (int j = start; j < end; j++) {
        sum += array[j];
    }
    return sum;
}

There is no need to have special cases for a size of 0 or 1; the code handles them already.
There is a clear split index, which corresponds to where the left and right arrays are split.

All the test cases still pass after making those changes.


Simple generic cartesian product in C++ The idea is to take a sequence of sequences (of the same type) and use tuples of [iterator, current index, sequence size] to keep track of the current state, such that the cartesian_product object can return one sequence at a time.
General questions:

bugs? (would hope not!)
code quality ok? anything i've missed?
is there a simpler way to keep track of the object state (iterators, counts, etc)
would it make sense to make this into an iterator (it could probably be bidirectional?)

Improvement ideas:

when the number of sequences is known at compile time, maybe it would be better for next() to return a tuple/static array instead of a std::vector<T>?
use a simple aggregate type instead of a tuple to avoid all the std::get ugliness?

Thanks a lot!
#include <iostream>
#include <tuple>
#include <type_traits>
#include <utility>
#include <vector>

template <typename T>
class cartesian_product {
public:
    template <typename... Args, typename = typename std::enable_if<(sizeof...(Args) > 1)>::type>
    cartesian_product(Args&&... args)
        : count(1)
    {
        std::cout << "constructor 1\n";
        static_assert(std::is_arithmetic<T>::value, "T must be an arithmetic type.");
        append(args...);
    }

    cartesian_product(std::vector<std::vector<T>> const& sequences)
        : count(1)
    {
        std::cout << "constructor 2\n";
        tuples.reserve(sequences.size());
        for(auto&& s : sequences) {
            tuples.emplace_back(s.cbegin(), 0ul, s.size());
            count *= s.size();
        }
    }

    cartesian_product(std::vector<std::vector<T>>&& sequences)
        : cartesian_product(sequences)
    {
        std::cout << "constructor 3\n";
    }

    bool has_next() { return count > 0; }

    // next combination
    std::vector<T> next()
    {
        std::vector<T> res(tuples.size());
        auto s = res.rbegin();

        for (auto p = tuples.rbegin(); p < tuples.rend(); ++p, ++s) {
            *s = *std::get<0>(*p);

            if (p > tuples.rbegin()) {
                auto q = p - 1;
                if (std::get<1>(*q) == std::get<2>(*q)) {
                    std::get<0>(*q) -= std::get<2>(*q);
                    std::get<1>(*q) = 0;

                    ++std::get<0>(*p);
                    ++std::get<1>(*p);
                }
            } else {
                ++std::get<0>(*p);
                ++std::get<1>(*p);
            }
        }

        --count;
        return res;
    }

private:
    using I = typename std::vector<T>::const_iterator;
    using E = std::tuple<I, std::size_t, std::size_t>;
    std::vector<E> tuples;
    int count;

    template <typename... Args>
    void append(std::vector<T> const& vec, Args&&... args)
    {
        append(vec);
        append(args...);
    }

    void append(std::vector<T> const& vec)
    {
        count *= vec.size();
        tuples.emplace_back(std::cbegin(vec), 0ul, vec.size());
    }
};

int main(int argc, char** argv)
{
    std::vector<int> a { 1, 2 };
    std::vector<int> b { 3, 4 };
    std::vector<int> c { 5, 6 };
    std::vector<int> d { 7, 8, 9 };

    std::vector<std::vector<int>> s{ a, b, c, d };

    cartesian_product<int> p(a, b, c);
    cartesian_product<int> q(s);
    cartesian_product<int> r(std::vector<std::vector<int>>{ a, b, c, d });

    auto print = [](auto&& vec) { for (auto v : vec) std::cout << v << " "; std::cout << "\n"; };

    while (p.has_next())
        print(p.next());

    return 0;
}

Don't limit Cartesian products to arithmetic types
You shouldn't limit your class to only allow products of vectors of a type that satisfies std::is_arithmetic. Why shouldn't I be able to do a Cartesian product of a std::string and a std::chrono::time_point for example? There is no actual multiplication involved in a Cartesian product after all.
Your class will likely force copies of vectors to be made
In this constructor:
cartesian_product(std::vector<std::vector<T>> const& sequences) {...}

The parameter sequences is a reference, so you might think that this doesn't cause any copies to be made. However, that is only true if the caller already has a vector of vectors lying around. In your own example usage in main(), you have four vectors a, b, c and d, but then you create the vector of vectors s which causes a copy to be made of the four original vectors. And the following line constructs a temporary that also makes copies:
cartesian_product<int> r(std::vector<std::vector<int>>{ a, b, c, d });

Support more containers besides std::vector
You only support products of std::vector<T>, but what if I have my data in a regular array, a std::array, a std::list or any other container? Again, it is an unnecessary restriction. Try to rewrite it so any type of container works. Either have it accept a reference to a container object directly, or have it accept iterators like the STL algorithms do.
Return a tuple instead of a vector
A std::vector will allocate memory on the heap, which will impact performance. And since you know the size of the result, you can return a std::tuple instead, which has the advantage that it can be unpacked using C++17's structured bindings. Apart from that, with a std::tuple the value types of each input vector/array/etc can be different, whereas your solution requires all of them to be the same,
Add iterators to your class
Indeed, it makes sense to add iterators to your class. In fact, it already basically keeps track of everything that an iterator would do, and next() is the equivalent of an iterator's operator++(). Ideally, you want to be able to write something like:
std::vector<std::string> a_vector{"foo", "bar", "baz"};
std::list<int> b_vector{4, 5, 6, 7};

for (auto [a_element, b_element]: cartesian_product(a_vector, b_vector))
    std::cout << a_element << ", " << b_element << "\n";

And then expect the following result:
foo, 4
foo, 5
foo, 6
foo, 7
bar, 4
bar, 5
...

This also avoids the need for a has_next() function.


Leetcode: Longest substring without repeating characters I saw the editorial solutions for this problem, but they are written in an unfamiliar language. I'd like to post my code, which worked, and understand the timing of my solution. I don't understand how to judge Big-O time yet.

def length_of_longest_substring(s)

    longest_sub,current_sub = "",""
    s.each_char do |c|
      if current_sub.include?(c)
        longest_sub = current_sub if current_sub.length > longest_sub.length
        current_sub=current_sub[current_sub.index(c)+1..-1]+c
      else
        current_sub+=c
      end
    end
    longest_sub = current_sub if current_sub.length > longest_sub.length
    longest_sub.length
end

I assume that since I'm creating strings, my space complexity suffers somewhat. I'm not really sure how I would speed things up with time.

You have exploited the space complexity.

def findLongestSubstring(inputString)
    hashMap = Hash.new
    longestSubstringLength = 0
    jIndex = 0
    iIndex = 0
    while(jIndex < inputString.length)
        if(hashMap[inputString[jIndex]])
            iIndex = [iIndex, hashMap[inputString[jIndex]]].max
        end
        longestSubstringLength = [longestSubstringLength, jIndex-iIndex+1].max
        hashMap[inputString[jIndex]] = jIndex + 1
        jIndex = jIndex + 1
    end
    longestSubstringLength
end

This method makes use of HashMap which works efficiently in terms of complexity. Searching the element in HashMap works in O(1) and same goes for insertion. This way  you can reduce the complexity of your program


Filetype backup script This was one of my assignments in which I had to create a backup script that will individually compress all files (of an arbitrary number) of file-types (indicated by their .extension).

Example: backup a b c will take all files ending in .a, .b and .c and compress them using tar, keeping the original file, and naming the new compressed file the same as before except with .tar.gz at the end.

The -t flag can be used to specify the target directory.

The -d flag can be used to specify the destination directory.

Example: backup -t ~/Desktop/stuff -d goesHere gif where goesHere is a directory in your current path and all files ending with .gif come from ~/Desktop/stuff.

$ ls
.  ..  backup  cat.gif  dog.gif  funny_video.mov  media
$ ./backup -d media gif mov
backup: compressed .gif files
backup: compressed .mov files
backup: files saved to "media"
$ cd media
$ ls
.  ..  cat.gif.tar.gz  dog.gif.tar.gz  funny_video.mov.tar.gz
$

Both flags are optional, allowing you to specify either one or both. If you specify both flags you must use the order -t <directory> -d <directory>.

Granted I really don't like the specifications of the assignment, but that is what they are. The fact we're asked to compress all files by their .extension is kind of anti-*nix anyways (everyone knows cat.png could be a plain text file).

#!/bin/bash

list_begin=1 # where the filetype arguments begins
list_end=$ # where the filetype arguments end

target_directory=$(pwd)
destination_directory=$(pwd)

if [[ $# < 1 ]]; then
    echo "backup: not enough arguments"
    exit
fi

# determine if any flags are used and if so update directories and $list_begin
if [[ $1 == "-t" ]]; then # -t
    target_directory=$2
    list_begin=3
    if [[ $3 == "-d" ]]; then # -t, -td
        destination_directory=$4
        list_begin=5
    fi
elif [[ $1 == "-d" ]]; then # -d
    destination_directory=$2
    list_begin=3
fi

# confirm directories are real
if [[ ! -d "${target_directory}" ]]; then # invalid -t
    echo "backup: \"${target_directory}\" is not valid"
    exit
elif [[ ! -d "${destination_directory}" ]]; then # invalid -d
    echo "backup: \"${destination_directory}\" is not valid"
    exit
fi

declare -a filetypes # array to store filetypes

for ((i=${list_begin}, j=0; i<=${list_end}; i++, j++)); do # fetch all filetype arguments and add to array
    eval dir=\${$i}
    filetypes[${j}]=${dir}
done

for i in "${filetypes[@]}"; do # for each filetype
    f_list="${target_directory}/*.${i}" # directory to iterate through
    for file in ${f_list}; do # for each file of that filetype in the target directory
        if [[ -f "${file}" ]]; then
            tar -czf ${destination_directory}/$(basename "$file").tar.gz -P ${file} # compress the file in the destination directory
        fi
    done
    echo "backup: finished .${i} files"
done

echo "backup: files saved to \"${destination_directory}\""

As you say, the requirements are, let's say, unconventional.  But in some ways, that's good preparation for crazy requirements you'll get in working life!  Except that as a student, you might not get to ask the "five whys" to determine whether the requirements are actually grounded, and to get them changed.
There are still some clarifications of the requirements:

Should only regular files be archived, or should we include directories and devices, too?
Should the archive contain full pathnames of the contained files, or relative names, or just basenames?  If only basenames, then how should we deal with conflicts?
Should the destination directory be created if it doesn't already exist?

As this is a homework assignment, you may not be in a position to ask these questions - in that case, you should assume a position and state it clearly in your answer.  If possible, localise the effects of a wrong assumption, to make it easy to change when you get more information (again, that's a good reflection of working life).

Working through the code from the beginning:
list_begin=1 # where the filetype arguments begins
list_end=$ # where the filetype arguments end

target_directory=$(pwd)
destination_directory=$(pwd)

It looks like you can save two invocations of pwd by using . as the default directory name - you never need an absolute path.
A more common approach is to remove option arguments using shift as they are parsed; then all of $* are your inputs.  I'm assuming $  there is a typo for $#?
Here's the outline:
while test -n "${1+:}"
do
    case "$1" in
        -t|--target)
            target="$2"
            shift 2
            ;;
        -d|--destination)
            destination="$2"
            shift 2
            ;;
        *)
            process_file "$1"
            shift
            ;;
    esac
done

I've included long-name alternatives just to show how it's done, not because that's a requirement.  This approach allows the user to specify -t and -d more than once, to change those settings during a run.  It's not clear whether you are required to enforce the ordering constraints you were given, or if that's just limiting the problem space to make it easier.  If the former, you can just process them in turn:
if [ "-t" = "$1" ]
then
    target="$2"
    shift 2
fi
if [ "-d" = "$1" ]
then
    destination="$2"
    shift 2
fi

and so on.
I've also fixed the quoting above - you'll find that if you miss it out, then people will want at target or destination with whitespace in the name!

# confirm directories are real
if [[ ! -d "${target_directory}" ]]; then # invalid -t
    echo "backup: \"${target_directory}\" is not valid"
    exit
elif [[ ! -d "${destination_directory}" ]]; then # invalid -d
    echo "backup: \"${destination_directory}\" is not valid"
    exit
fi

You don't need to use Bash [[ here - it's more portable to stick to plain old [ (aka test).  You'll want your error messages to go to the error stream (>&2), and you'll want to exit with a non-zero value, so that anything executing your program will know it didn't succeed.  I like to define a small function to do that:
die() {
    echo "$@" >&2
    exit 1
}

Then I can write:
test -d "$target_directory" || die "$0: $target_directory is not a directory"
test -r "$target_directory" && test -x "$target_directory" || die "$0: $target_directory cannot be read"
test -d "$destination_directory" || die "$0: $destination_directory is not a directory"
test -w "$destination_directory" || die "$0: $destination_directory is not writable"

declare -a filetypes # array to store filetypes

for ((i=${list_begin}, j=0; i<=${list_end}; i++, j++)); do # fetch all filetype arguments and add to array
    eval dir=\${$i}
    filetypes[${j}]=${dir}
done

I don't think it's necessary to do all this copying.  You can simply process each argument as you come to it.

for i in "${filetypes[@]}"; do # for each filetype
    f_list="${target_directory}/*.${i}" # directory to iterate through

You shouldn't be quoting the * there, because you want the shell to expand it.  If you expand it later, you may well end up splitting $i.

My version
#!/bin/sh
set -e

die() {
    echo "$@" >&2
    exit 1
}

process_suffix() {
    for file in "${source-.}"/*."$1"
    do
        # Warning: may overwrite existing archive
        test -e "$file" \
             && tar cfz "${destination:-.}/${file##*/}.tar.gz" -C "${source-.}" -- "${file#${source-.}/}"
    done
    echo "$0: finished *.$1 files"
}

while test -n "${1+:}"
do
    case "$1" in
        -t|--source)
            test -d "$2" || die "$0: $2 is not a directory"
            test -r "$2" && test -x "$2" || die "$0: $2 cannot be read"
            source="$2"
            shift 2
            ;;
        -d|--destination)
            test -d "$2" || die "$0: $2 is not a directory"
            test -w "$2" || die "$0: $2 is not writable"
            destination="$2"
            shift 2
            ;;
        *)
            process_suffix "$1"
            shift
            ;;
    esac
done

echo "$0: files saved to $destination"

Additional notes:

Everything is now POSIX shell, so we can use /bin/sh which may be faster, more available, etc (bash, ksh, dash etc. are all sufficiently POSIX-compliant).
I've given -- to tar to indicate that subsequent arguments are filenames, even if they begin with -.
I default uninitialised source and destination directories - see the Parameter Expansion section of the shell man page.
Archive filenames are all relative to the source directory.
I use "$0" in messages to show the actual name the program was invoked as.
Test cases should include file and directory names containing spaces or beginning with - (that's a good general rule for testing shell scripts).
Remember that (by default) a non-matching glob is treated literally.  So if there are no matching files, we'll want to ignore *.suffix when archiving (but still archive a file that really does have * in the name).


(Leetcode) Freedom Trail I would like to know whether I could make my solution to this challenge shorter and more efficient. Below is the description of the challenge (this is a Leetcode problem) -

  In the video game Fallout 4, the quest "Road to Freedom" requires
  players to reach a metal dial called the "Freedom Trail Ring", and use
  the dial to spell a specific keyword in order to open the door.

  Given a string ring, which represents the code engraved on the
  outer ring and another string key, which represents the keyword
  needs to be spelled. You need to find the minimum number of steps
  in order to spell all the characters in the keyword.

  Initially, the first character of the ring is aligned at the 12:00
  direction. You need to spell all the characters in the string key
  one by one by rotating the ring clockwise or anticlockwise to make
  each character of the string key aligned at the 12:00 direction
  and then by pressing the center button.

  At the stage of rotating the ring to spell the key character
  key[i] -

  You can rotate the ring clockwise or anticlockwise one place, which counts as 1 step. The final purpose of the rotation is
  to align one of the string ring's characters at the 12:00
  direction, where this character must equal to the character
  key[i].
  If the character key[i] has been aligned at the 12:00 direction, you need to press the center button to spell, which also
  counts as 1 step. After the pressing, you could begin to spell the
  next character in the key (next stage), otherwise, you've finished all
  the spelling.

  Example -

Input: ring = "godding", key = "gd"
Output: 4

# Explanation:
# For the first key character 'g', since it is already in place, we just need 1 step to spell this character.
# For the second key character 'd', we need to rotate the ring "godding" anticlockwise by two steps to make it become "ddinggo".
# Also, we need 1 more step for spelling.
# So the final output is 4.

  Note -

  Length of both ring and key will be in range 1 to 100.
  There are only lowercase letters in both strings and might be some duplicate characters in both strings.
  It's guaranteed that string key could always be spelled by rotating the string ring.

Here is my solution to this challenge -

# Uses dynamic programming

def find_rotate_steps(ring, key):
    """
    :type ring: str
    :type key: str
    :rtype: int
    """
    dp = [[min(i, len(ring) - i) for i in range(len(ring))]]
    dp.extend([[float('inf')] * len(ring) for _ in range(len(key))])
    for i in range(1, len(key) + 1):
        for j in range(len(ring)):
            if ring[j] != key[i - 1]:
                continue
            min_step = float('inf')
            for k in range(len(ring)):
                if dp[i - 1][k] == float('inf'):
                    continue
                step = abs(k - j)
                step = min(step, len(ring) - step) + 1 + dp[i - 1][k]
                min_step = min(min_step, step)
            dp[i][j] = min_step
    return min(dp[-1])

Here is the time taken for the example output -

# %timeit find_rotate_steps("godding", "gd")

36.2 µs ± 1.33 µs per loop (mean ± std. dev. of 7 runs, 10000 loops each)

Here is my Leetcode result for 302 test cases -

NOTE - Here is what dynamic programming means (according to freeCodeCamp) -

  Dynamic programming amounts to breaking down an optimization problem
  into simpler sub-problems and storing the solution to each sub-problem
  so that each sub-problem is only solved once.

It was hard to understand, how your code works, so I started by changing variable names to more meaningful form, also unnecessary +1, -1 operation were fixed (i - 1, 1, len(key) + 1, etc):

class Solution:
    def findRotateSteps(self, ring, key):
        cumulative_lens = [[min(i, len(ring) - i) for i in range(len(ring))]]
        cumulative_lens.extend([[float('inf')] * len(ring) for _ in range(len(key))])

        for key_num in range(len(key)):
            for new_ring_pos in range(len(ring)):
                if ring[new_ring_pos] != key[key_num]:
                    continue

                min_sum_len = float('inf')

                for prev_ring_pos in range(len(ring)):
                    prev_ltr_sum_len = cumulative_lens[key_num][prev_ring_pos]

                    if prev_ltr_sum_len == float('inf'):
                        continue

                    clk_w_len = abs(prev_ring_pos - new_ring_pos)
                    a_clk_len = len(ring) - clk_w_len

                    new_sum_len = min(clk_w_len, a_clk_len) + prev_ltr_sum_len + 1
                    min_sum_len = min(min_sum_len, new_sum_len)

                cumulative_lens[key_num + 1][new_ring_pos] = min_sum_len

        return min(cumulative_lens[-1])

Now, it is possible to read the code like a story. The cause of low performance is how you store and search visited letter lens (cumulative_lens). You keep them in the list, thus you need to iterate through all items to find not inf ones for each key value:

for prev_ring_pos in range(len(ring)):
    prev_ltr_sum_len = cumulative_lens[key_num][prev_ring_pos]

    if prev_ltr_sum_len == float('inf'):
        continue

Example: you have 100 letters in the ring and 10 letters in the key. You found all possible lengths in the ring (there are 3 only) to the first key's letter and move to the second. It will be good to take just needed 3 in the next key's letter processing, not checking others 97 items, which are 'infs'. It can be done by storing ring letters in  the dictionary - the Python's hash table structure, like this:

letter_indexes = {
    'a' : [first a's index, second a's index],
    'c' : [first c's index, second c's index, third c's index],
    etc
}

In this case, we can get the list of the needed letter positions by doing letter_indexes[letter], not messing with the others. I wrote my own solution, see it in the end.

Also, I did some refactoring of your code:

changed multiple len(ring) calls to the ln_ring variable - no performance gain, but more readable.
changed

for key_num in range(len(key)):
    for new_ring_pos in range(len(ring)):
        if ring[new_ring_pos] != key[key_num]:

like constructions to more pythonic:

for key_num, key_ltr in enumerate(key):
    for new_ring_pos, ring_ltr in enumerate(ring):
        if key_ltr != ring_ltr:

Result:

class Solution:
    def findRotateSteps(self, ring, key):
        ln_ring = len(ring)

        cumulative_lens = [[min(i, ln_ring - i) for i in range(ln_ring)]]
        cumulative_lens.extend([[float('inf')] * ln_ring for _ in range(len(key))])

        for key_num, key_ltr in enumerate(key):
            for new_ring_pos, ring_ltr in enumerate(ring):
                if key_ltr != ring_ltr:
                    continue

                min_sum_len = float('inf')

                for prev_ring_pos, prev_ltr_sum_len in enumerate(cumulative_lens[key_num]):

                    if prev_ltr_sum_len == float('inf'):
                        continue

                    clk_w_len = abs(prev_ring_pos - new_ring_pos)
                    a_clk_len = ln_ring - clk_w_len

                    new_sum_len = min(clk_w_len, a_clk_len) + prev_ltr_sum_len + 1
                    min_sum_len = min(min_sum_len, new_sum_len)

                cumulative_lens[key_num + 1][new_ring_pos] = min_sum_len

        return min(cumulative_lens[-1])

My solution:

It works similar to your, but is much faster due to dictionary usage.

class Solution:
    def findRotateSteps(self, ring, key):
        # the 'prev_ltr' variable should have the start value in the
        # beginning, so the '#' character is choosed
        # It can be changed to any character different from key's content
        # In other words, it is like '-1' item with 0 index.
        ltr_indexes = {'#' : [0]}

        for idx, ltr in enumerate(ring):
            ltr_indexes.setdefault(ltr, []).append(idx)

        ln = len(ring)
        l_lens = [0] * ln

        prev_ltr = '#'
        for ltr in key:
            for pos in ltr_indexes[ltr]:
                all_variants = []

                for prev_pos in ltr_indexes[prev_ltr]:
                    clk_w = abs(prev_pos - pos)
                    a_clk = ln - clk_w
                    all_variants.append(min(clk_w, a_clk) + l_lens[prev_pos])

                l_lens[pos] = min(all_variants)

            prev_ltr = ltr

        return min(l_lens[pos] for pos in ltr_indexes[ltr]) + len(key)


Finding inorder successor of a BST in C Please review this code
I have written a function to find the successor of BST in c. It takes three arguments, i.e root pointer to the binary search tree, node value of which we have to find a successor and parent pointer (for first call it is also root pointer. It will return an integer that is the successor of the node value give.
/**
 * find Successor of BST, complex: o(h)
 * @param rootPtr root pointer of BST
 * @param parent_node parent node of the subtree (root node should be passed by default)
 * @param nodeValue node val to find successor
 * @return succsor
 */
int find_successor_h(BSTNode * rootPtr, int data, BSTNode * parent_node) {
  if (rootPtr == NULL) return data;
  int rt;

  if (data < rootPtr -> data)
    rt = find_successor_h(rootPtr -> left_child, data, rootPtr);
  else if (data > rootPtr -> data)
    rt = find_successor_h(rootPtr -> right_child, data, rootPtr);
  else {
    if (rootPtr -> right_child != NULL)
      return minBST(rootPtr -> right_child);
    else if (rootPtr != parent_node)
      return INT_MIN;
    else
      return data;
  }

  if (rt == INT_MIN)
    return INT_MAX;

  if (rt == INT_MAX)
    return parent_node -> data;
}

Missing return
Code may even fail the last rt == INT_MAX and then no return is specified.
  if (rt == INT_MAX)
    return parent_node -> data;

  // Missing return.
  }

Use const
Using const, as able, provides more information as to what the code does and does not do.  This also allows greater usage and potential optimization.
// int find_successor_h(BSTNode * rootPtr, int data, BSTNode * parent_node) {
int find_successor_h(const BSTNode * rootPtr, int data, const BSTNode * parent_node) {

Mis-matched comments
Comment does not match parameter names nor order: node val vs.  data.
* @param rootPtr root pointer of BST
* @param parent_node parent node of the subtree (root node should be passed by default)
* @param nodeValue node val to find successor

BSTNode * rootPtr, int data, BSTNode * parent_node

Missing include
NULL not defined.
Spelling
* @return succsor vs. * @return successor
Name error?
Why h in int find_successor_h()?
{ block }
Consider using {} even for simple blocks.  Little need for else
//if (rootPtr -> right_child != NULL)
//  return minBST(rootPtr -> right_child);
//else if (rootPtr != parent_node)
//  return INT_MIN;
//else
// return data;

if (rootPtr -> right_child != NULL) {
  return minBST(rootPtr -> right_child);
}
if (rootPtr != parent_node) {
  return INT_MIN;
}
return data;

Deeper review
More support and test drive code needed for a deeper review.