Return the shortest substring containing all the characters in the set The task:

  Given a string and a set of characters, return the shortest substring
  containing all the characters in the set.

  For example, given the string "figehaeci" and the set of characters
  {a, e, i}, you should return "aeci".

  If there is no substring containing all the characters in the set,
  return null.

My solution:

// init
const originalStr = 'figehaeci';
const set = Object.freeze(['a', 'e', 'i']);

// helpers
const isEmpty = s => s.length === 0;
const copy = s => s.slice(0);
const getNextResult = (currRes, endRes) => {
    if (!endRes || isEmpty(endRes)) {
      return currRes;
    }
    return currRes.length < endRes.length ?
           currRes : endRes;
};

// main
const contains = (str, endResult) => {
  const currStr = str;
  const currSet = copy(set);
  let pushIt;
  const currResult = currStr
    .split('')
    .reduce((acc, item) => {
      if (pushIt === false) {
        return acc;
      }
      if (currSet.includes(item)) {
        currSet.splice(currSet.indexOf(item), 1);
        pushIt = true;
      }
      if (pushIt) {
        acc.push(item);
      }
      if (isEmpty(currSet)) {
        pushIt = false;
      }
      return acc;
    }, []);

  const nextStr = str
    .split('')
    .slice(1)
    .join('');

  // exit case
  if (isEmpty(nextStr)) {
    return endResult ? endResult.join('') : null;
  }

  // recursive case
  const nextResult = isEmpty(currSet) ?
        getNextResult(currResult, endResult) :
        endResult;

  return contains(nextStr, nextResult);
}

// result
console.log(contains(originalStr, null));

I don't like the many ifs and the flag (pushIt). Do you know how to get rid of them without sacrificing readability and maintainability?

Doesn't have to be in functional programming style. Just looking for an elegant solution.

I couldn't remove the ifs and the flag , so i thought about a diffrent aproach,

Using regular expressions seem more reasonable aproach, you can have a function that returns all the possible combinations of the letters in the array, create regular expressions from the resulting combinations and match against the orignalStr,

function perm(xs) {
  let ret = [];
  for (let i = 0; i < xs.length; i = i + 1) {
    let rest = perm(xs.slice(0, i).concat(xs.slice(i + 1)));

    if (!rest.length)
      ret.push([xs[i]]);
    else
      for (let j = 0; j < rest.length; j = j + 1)
        ret.push([xs[i]].concat(rest[j]));
  }
  return ret;
}

const originalStr = 'figehaeci', letters = ['a', 'e', 'i'];
let result = originalStr;

const combinations = perm(letters).map(combo => combo.join('.*?'));

// combinations : ["a.*?e.*?i","a.*?i.*?e","e.*?a.*?i","e.*?i.*?a","i.*?a.*?e","i.*?e.*?a"]

combinations.forEach(combo => {
  const exp = new RegExp(combo, 'g');
  const matches = originalStr.match(exp) || [];

  matches.forEach(match => {
    if (match.length <= result.length)
      result = match;
  });
});

console.log(result);

The code you posted returns only one value if there's more substrings containing those letters, here's a tweak to store all possible shortest combinations in an array :

function perm(xs) {
  let ret = [];
  for (let i = 0; i < xs.length; i = i + 1) {
    let rest = perm(xs.slice(0, i).concat(xs.slice(i + 1)));

    if (!rest.length)
      ret.push([xs[i]]);
    else
      for (let j = 0; j < rest.length; j = j + 1)
        ret.push([xs[i]].concat(rest[j]));
  }
  return ret;
}

const originalStr = 'figehaecizaexi', letters = ['a', 'e', 'i'];
let shortestLength = originalStr.length, result = [];

const combinations = perm(letters).map(combo => combo.join('.*?'));

// combinations : ["a.*?e.*?i","a.*?i.*?e","e.*?a.*?i","e.*?i.*?a","i.*?a.*?e","i.*?e.*?a"]

combinations.forEach(combo => {
  const exp = new RegExp(combo, 'g');
  const matches = originalStr.match(exp) || [];

  matches.forEach(match => {
    if (match.length < shortestLength) {
      shortestLength = match.length;
      result = [match];
    }
    if (match.length === shortestLength) {
      result.push(match);
    }
  });
});

const deduped = [...new Set(result)];

console.log(deduped);


My Quick sort examination I had a test on sorting using quick sort algorithm, which I have implemented. It's working fine.

But my examiner is not satisfied. They say that the solution which I provided is not a proper quicksort, my code partially deviates from the actual quicksort algorithm, and the code is a mess in certain places.

I took the quicksort explanation from wikipedia's quick sort's graphical explanation.

The examiner said to again go through the code again, which I have done. It works fine.

My code also uses the concept of "divide and conquer" as the "partition" function in the code only works on a certain part of input array.

What's my examiner pointing at that I'm missing?

function quickSort(inputArray){
  var pivotIndex = undefined;
  var low_Index = undefined;
  var high_Index = undefined;

  pivotIndex = inputArray.length - 1;
  low_Index = 0;
  high_Index = (inputArray.length > 2) ? (inputArray.length - 2) : 0;

  partition(pivotIndex,low_Index,high_Index);

  // the function which works on partitioned array
  function partition(pivot_Index,low_Index,high_Index){
    var temp = 0;

    console.log("\nPivot Number : "+inputArray[pivot_Index]);
    console.log("input Array : "+inputArray);
    console.log("partitioned array : "+inputArray.slice(low_Index,pivot_Index+1));

    for(var low = low_Index; low < pivot_Index; low++){

        if(inputArray[low] > inputArray[pivot_Index]){

            temp = inputArray[low];
            inputArray[low] = inputArray[high_Index];
            inputArray[high_Index] = inputArray[pivot_Index];
            inputArray[pivot_Index] = temp;
            pivot_Index = high_Index;
            high_Index--;
            low--;
        }
    }// end of for loop

    if(inputArray[low_Index] > inputArray[high_Index]){
        temp = inputArray[low_Index];
        inputArray[low_Index] = inputArray[high_Index];
        inputArray[high_Index] = temp;
        pivot_Index = high_Index;
    }

    if(pivot_Index < inputArray.length - 1){

        if((pivot_Index + 1) < inputArray.length - 1){
            partition(inputArray.length - 1,(pivot_Index + 1),inputArray.length - 2);
        }

        if((pivot_Index - 2) >= 1){
            partition((pivot_Index - 1),0,(pivot_Index - 2));
        }

    }// end of if condition

  } // end of partition funtion

  return inputArray;

} // end of quick sort function

When I run the above code for input

console.log("\nFinal Sorted Array "+quickSort([5,6,1,2,7,8,3]));

the output is

rahul@rahul:~/myPractise/Algo$ node sorting.js

Pivot Number : 3
input Array : 5,6,1,2,7,8,3
partitioned array : 5,6,1,2,7,8,3

Pivot Number : 5
input Array : 1,2,3,6,7,8,5
partitioned array : 3,6,7,8,5

Pivot Number : 6
input Array : 1,2,3,5,7,8,6
partitioned array : 7,8,6

Pivot Number : 7
input Array : 1,2,3,6,5,8,7
partitioned array : 5,8,7

Pivot Number : 5
input Array : 1,2,3,6,5,7,8
partitioned array : 1,2,3,6,5

Pivot Number : 8
input Array : 1,2,3,5,6,7,8
partitioned array : 6,7,8

Pivot Number : 3
input Array : 1,2,3,5,6,7,8
partitioned array : 1,2,3

Pivot Number : 8
input Array : 1,2,3,5,6,7,8
partitioned array : 5,6,7,8

Pivot Number : 3
input Array : 1,2,3,5,6,7,8
partitioned array : 1,2,3

Pivot Number : 8
input Array : 1,2,3,5,6,7,8
partitioned array : 5,6,7,8

Pivot Number : 3
input Array : 1,2,3,5,6,7,8
partitioned array : 1,2,3

Pivot Number : 8
input Array : 1,2,3,5,6,7,8
partitioned array : 5,6,7,8

Final Sorted Array 1,2,3,5,6,7,8
rahul@rahul:~/myPractise/Algo$

Interesting question,

as far as I can tell, quicksort is recursive, your solution is not. Hence, your solution is not really quicksort.

Other than that, this makes me cringe:

  var pivotIndex = undefined;
  var low_Index = undefined;
  var high_Index = undefined;

  pivotIndex = inputArray.length - 1;
  low_Index = 0;
  high_Index = (inputArray.length > 2) ? (inputArray.length - 2) : 0;

just declaring a variable assigns the value undefined, there is no reason to assign it, so don't do it. Then right after you declared those variables, you assign values, which you could also do in the var statement. Finally, and this is truly a matter of tastes you could chain those statements:

  var pivotIndex = inputArray.length - 1,
      low_Index = 0,
      high_Index = (inputArray.length > 2) ? (inputArray.length - 2) : 0;

Even here, you could reflect on the repetition of inputArray.length and assign this to a shorter variable name, to make this more readable, and a fraction faster:

  var length = inputArray.length,
      pivotIndex = length - 1,
      low_Index = 0,
      high_Index = (length > 2) ? (length  - 2) : 0;

Then again, you are not using a consistent naming standard, always use lowerCamelCase in JavaScript. I think some professors dislike ternaries, I would avoid them in school or at least check with the professor.

  var length = inputArray.length,
      pivotIndex = length - 1,
      lowIndex = 0,
      highIndex = Math.max(length - 2, 0);

Other than that your white-spacing is far too generous, never use more than a double newline.

This probably also makes you loose points:

for(var low = low_Index; low < pivot_Index; low++){

I would use index instead of low, otherwise reading your code becomes too hard.

Finally, also check your code for repetition, you swap two entries a number of times, I would have written a dedicated function for that.


Securely upload multiple image files with PHP I am working on script to a upload image files Securely. can anyone please review my code and suggest improvements on security.
<input type="file" name="images[]" id="image" multiple="multiple">
check if request method is POST and request was made from same domain and form was submitted with submit btn
if (is_post_request() && is_request_same_domain() && isset($_POST['add-list-submit'])) {

Convert the $_FILES array to a cleaner version
$images = rearrange_files_array($_FILES['images']);

function rearrange_files_array($file)
{

    $file_array = [];
    $file_count = count($file['name']);
    $file_keys = array_keys($file);

    for ($i = 0; $i < $file_count; $i++) {
        foreach ($file_keys as $key) {
            $file_array[$i][$key] = $file[$key][$i];
        }
    }
    return $file_array;
}

Loop through the images
foreach ($images as $image) {

Check if image was upload, image mime type, extension, and size
if (!isset($image) || $image['error'] !== UPLOAD_ERR_OK) {
    $errors['form'] = 'Please upload the images.';
} else {
    if (!is_file_valid_image($image) || !has_valid_file_extension($image, ['jpeg', 'png', 'jpg'])) {
        $errors['form'] = 'Only PNG and JPG file formats are allowed.';
    }

    if (has_max_file_size($image)) {
        $errors['form'] = 'Max file size is 2.5 MB.';
    }
}

function has_valid_file_extension($file, $allowed_file_extensions)
{
  $file_extension = pathinfo($file['name'], PATHINFO_EXTENSION);
  return in_array($file_extension, $allowed_file_extensions);
}

function is_file_valid_image($file)
{
  $imageTypes = [IMAGETYPE_GIF, IMAGETYPE_JPEG, IMAGETYPE_PNG, IMAGETYPE_SWF, IMAGETYPE_PSD, IMAGETYPE_BMP, IMAGETYPE_TIFF_II, IMAGETYPE_TIFF_MM, IMAGETYPE_JPC, IMAGETYPE_JP2, IMAGETYPE_JPX, IMAGETYPE_JB2, IMAGETYPE_SWC, IMAGETYPE_IFF, IMAGETYPE_WBMP, IMAGETYPE_XBM, IMAGETYPE_ICO];
  $info = getimagesize($file['tmp_name']);
  $width = $info[0];
  $height = $info[1];
  $type = $info[2];
  $bits = $info['bits'];
  $mime = $info['mime'];

  if (!in_array($type, $imageTypes)) {
    return false; // Invalid Image Type
  } else if ($width <= 1 && $height <= 1) {
    return false; // Invalid Image Size
  } else if ($bits === 1) {
    return false; // One Bit Image
  } else if ($mime !== 'image/gif' || $mime !== 'image/jpg' || $mime !== 'image/jpeg' || $mime !== 'image/png') {
    return false; // Invalid Image Type
  } else {
    return true;
  }
}

function has_max_file_size($file)
{
  return $file['size'] > 2500000; // 2.5 MB
}

If there are no errors move the image to the uploads directory
$filename = bin2hex(random_bytes(12));
$file_ext = pathinfo($image['name'])['extension'];
$upload_path = UPLOAD_DIRECTORY . '/' . $filename . '.' . $file_ext;
move_uploaded_file($image['tmp_name'], $upload_path);

If you are talking about a "secure" version then please state what you mean with it. What are you protecting against and which security requirements do you have extracted?
if (is_post_request() && is_request_same_domain() && isset($_POST['add-list-submit'])) {

This only indicates what it is doing. But it is doing too much on a line. If this is some particular check then please implement it in a well named function.

Convert the $_FILES array to a cleaner version

What does that even mean, a "cleaner version"?
function rearrange_files_array($file)

So it is apparently a rearranged array, but why? Please document your functions if it is not clear what they are doing. Furthermore, here even the "how" is missing: how is the array rearranged?
$errors['form'] = 'Please upload the images.';

Is this something that a user could do? I just select the images. If they don't get loaded into variables then I cannot fix that myself.
$errors['form'] = 'Only PNG and JPG file formats are allowed.';
...
$errors['form'] = 'Max file size is 2.5 MB.';

You have run into a common problem: putting all possible information in the error message while keeping the parameters generic. Please fix the error message so that the extensions and max file size are dynamic. Currently you would have to adjust both the function and the content of the error message separately to achieve one thing.
You did let the extensions be the 2nd parameter of the has_valid_file_extension method. That's a good thing and you should perform the same for checking the file size. It will also bring the error and value of the file size closer together.
It seems that you fall through the if statements when error checking; I'd expect the script to perform some kind of action, right? Currently only an error is set, but the script seems to continue.
$imageTypes = [IMAGETYPE_GIF, IMAGETYPE_JPEG, IMAGETYPE_PNG, IMAGETYPE_SWF, IMAGETYPE_PSD, IMAGETYPE_BMP, IMAGETYPE_TIFF_II, IMAGETYPE_TIFF_MM, IMAGETYPE_JPC, IMAGETYPE_JP2, IMAGETYPE_JPX, IMAGETYPE_JB2, IMAGETYPE_SWC, IMAGETYPE_IFF, IMAGETYPE_WBMP, IMAGETYPE_XBM, IMAGETYPE_ICO];

That's strange, I thought only JPEG and PNG were allowed. This is probably a more generic method. But even in that case: check the file extension first before running this method. You don't want to be checking some exotic file type only to find out that it is not allowed anyway; always bring down the number of options / states of a program as soon as possible.
You may also wan to check that the extension is correct for the given image type, I don't see that happening.
As an aside: Obviously the method getimagesize is named terribly badly as it returns all kinds of information, but that's something to blame on PHP, not your script.
return false; // Invalid Image Type

This wipes out the actual error message, which means it will be impossible to inform the user correctly. You'd better return result which contains an error string or use a similar construct. I applaud you for at least including a comment, but if you have to type such comments then the code itself is often not verbose enough.
return false; // One Bit Image

Huh? No, that seems to be the bit depth. So it would be  B/W or rather monochrome image.
else {
    return true;
}

Just return true is enough here, the application has nowhere to go after all.

All in all, it's not a bad attempt to me, but a few repairs and a lot of polishing seems in order.


Setting a OnClickListener in a loop I am trying to set a OnClickListener to a image in a loop. If the params platform is "android" then use market app, instead of default browswer. Is there a better solution to my exception handling or eliminatingsome of the if-else ?

private void setupListener(ImageView image, final String platform,
        final String urlLink) {
    image.setOnClickListener(new View.OnClickListener() {

        @Override
        public void onClick(View v) {
            if (platform.equalsIgnoreCase(PLATFORM_ANDROID)) {
                // open with market app
                String packageName = extractPackageName(urlLink);
                if (packageName != null) {
                    try {
                        Intent market = new Intent(Intent.ACTION_VIEW);
                        market.setData(Uri.parse("market://details?id="
                                + packageName));
                        activity.startActivity(market);
                    } catch (Exception e) {
                        e.printStackTrace();
                        startInBroswer(urlLink);
                    }
                } else {
                    startInBroswer(urlLink);
                }

            } else {
                // open with default broswer.
                startInBroswer(urlLink);
            }

        }
    });
}

private void startInBroswer(String urlLink) {
    Intent browser = new Intent(Intent.ACTION_VIEW, Uri.parse(urlLink));
    activity.startActivity(browser);
}

Here is another version with guard clauses:

private void setupListener(final ImageView image, final String platform,
        final String urlLink) {
    image.setOnClickListener(new View.OnClickListener() {

        @Override
        public void onClick(View v) {
            if (!platform.equalsIgnoreCase(PLATFORM_ANDROID)) {
                startInBroswer();
                return;
            }

            final String packageName = extractPackageName(urlLink);
            if (packageName == null) {
                startInBroswer();
                return;
            }

            try {
                final Intent market = new Intent(Intent.ACTION_VIEW);
                market.setData(Uri.parse("market://details?id=" + packageName));
                activity.startActivity(market);
            } catch (final Exception e) {
                // TODO: log the exception
                startInBroswer();
            }
        }

        private void startInBroswer() {
            final Intent browser = new Intent(Intent.ACTION_VIEW,
                Uri.parse(urlLink));
            activity.startActivity(browser);
        }
    });
}

Two things to notice:

I've removed the urlLink parameter of the startInBroswer method.
It is a bad idea to use printStackTrace() in Android exceptions.


Is it needed to "beautify" this Stack implementation? I'm just learning OOP, and I have an assignment to create a Stack. I came up with the following. Is it "beauty" enough, or to "schooly". Where can I evolve?

This is the class:

class Stack
{
    private int _length;
    private List<int> _members = new List<int>();

    public Stack()
    {
        _length = 10;
    }

    public Stack(int length)
    {
        _length = length;
    }

    public List<int> Members
    {
        get { return _members; }
    }

    public int Length
    {
        get { return _members.Count; }

    }

    public bool IsEmpty
    {
        get { return (_members.Count == 0); }
    }

    public void Push(int member)
    {
        if (_members.Count < _length) _members.Add(member);
        else throw new Exception("The Stack is full.");
    }

    public int Pop()
    {
        if (!IsEmpty)
        {
            int last_one = _members[_members.Count - 1];
            _members.RemoveAt(_members.Count - 1);
            return last_one;
        }
        else throw new Exception("The Stack is empty.");
    }

}

And this is my Form:

public partial class Form1 : Form
{
    Stack verem = new Stack(20);
    Random rnd = new Random();

    public Form1()
    {
        InitializeComponent();
    }

    private void refreshStack()
    {
        lbStack.Items.Clear();
        foreach (int member in verem.Members)
        {
            lbStack.Items.Insert(0, member);
        }
    }
    private void btnIsEmpty_Click(object sender, EventArgs e)
    {
        MessageBox.Show(verem.IsEmpty.ToString());
    }

    private void btnLength_Click(object sender, EventArgs e)
    {
        MessageBox.Show(verem.Length.ToString());
    }

    private void btnPush_Click(object sender, EventArgs e)
    {
        verem.Push((int)rnd.Next());
        refreshStack();
    }

    private void btnPop_Click(object sender, EventArgs e)
    {
        verem.Pop();
        refreshStack();
    }

}

class Stack

Stack (or any other collection) is a prime example of type that should be generic.

private int _length;

I think length is not a good name for this. You should rename it to something like _maxLength, or even better, _maxCount (see below).

Also, I'm not completely sure if this feature is necessary, but it could make sense (for example System.Collections.Generic.Stack<T> doesn't have anything like this, but System.Collections.Concurrent.BlockingCollection<T> does).

_length = 10;

I don't think 10 is a reasonable default here. It would certainly be unexpected to me. A much better default would be infinity. Or maybe don't have a default at all.

public List<int> Members

Unless absolutely necessary, you should never expose private implementation details like this. Using this member, anyone can do anything to the underlying collection (including adding items beyond the allowed maximum number).

If you want to be able to iterate over the items in the collection, you should implement IEnumerable<T>.

public int Length

The name Length makes some sense for arrays, but not so much for other collections. For consistency with other collections, the best name would be Count.

return (_members.Count == 0);

The parentheses are not necessary here.

if (_members.Count < _length) _members.Add(member);
else throw new Exception("The Stack is full.");

You probably shouldn't write the body of if and else on the same line like this. Putting them on a line of their own will make your code clearer to read.

throw new Exception("The Stack is full.")

You shouldn't throw Exception directly, you should create your own type that inherits from Exception and throw that. Alternatively, you could use an existing exception type, like InvalidOperationException.

if (!IsEmpty)
{
    …
}
else throw new Exception("The Stack is empty.");

You could simplify this by reversing the condition and putting the exception first. This way, the main code will be less indented.

public partial class Form1 : Form

You should name your forms with more descriptive names, even if it's something as simple as MainForm.

lbStack

This form of Hungarian notation is usually frowned on. If you want to indicate that the variable is a list box, you can name is something like stackListBox.

(int)rnd.Next()

You don't need a cast here, Next() already returns an int.


Program for CodeHS 8.3.8: Word Ladder in Python 3 I have written an answer to this question on Stack Overflow. To make it easier for you, I have copy-pasted the question below.

  Your friend wants to try to make a word ladder! This is a list of
  words where each word has a one-letter difference from the word before
  it.

  Here’s an example:

cat
cot
cog
log

  Write a program to help your friend. It should do the following:

  Ask your friend for an initial word.
  Repeatedly ask them for an index and a letter.
  You should replace the letter at the index they provided with the letter they enter.
  You should then print out the new word.
  Stop asking for input when the user enters -1 for the index.

My answer to this is:

import os
get_index = 0
word = input("Enter a word: ")
for i in range(len(word)):
    while True:
        try:
            get_index = int(input("Enter an index (-1 to quit): "))
            if get_index == -1:
                os._exit(0)
            elif get_index < -1 or get_index > 2:
                raise ValueError
        except ValueError:
            print ("Invalid index!")
        else:
            while True:
                try:
                    letter = input("Enter a letter: ")
                    for c in letter:
                        if c.isupper():
                            raise ValueError
                except ValueError:
                    print ("Character must be a lowercase letter!")
                else:
                    if len(letter) > 1:
                        print ("Must be exactly one character!")
                    else:
                        word = word[:get_index] + letter + word[get_index + 1:]
                        print (word)
                        break

NOTE #1: Here is a link to the program run (output): OUTPUT

NOTE #2: I would also like to use an alternative for os._exit() as it takes time for the program to completely end.

So, I would like to know whether I could make this code shorter and more efficient.

Any help would be highly appreciated.

While your code seems to work, the next step should be separating the different responsibilities of your code. As far as I can see, there are three:

You have to ask the user for an index. It needs to be an integer in the range [-1, len(s)).
You have to ask them for a replacement character. It must be exactly one lower case character.
You need to actually run the game.

The first two can be combined into a general function asking the user for input with type and validation:

def ask_user(message, type_=str, valid=lambda: True, invalid="Invalid"):
    while True:
        try:
            user_input = type_(input(message))
        except ValueError:
            print(invalid)
            continue
        if not valid(user_input):
            print(invalid)
            continue
        return user_input

And your main program can then use this function. I would also change the internal data of the word to be a list, so you can easily replace characters. When printing the word, just use "".join(word).

def play_word_ladder():
    word = list(input("Enter a word: "))

    def valid_index(i):
        return i in range(-1, len(word))

    def valid_character(c):
        return len(c) == 1 and c.islower()

    while True:
        index = ask_user("Enter an index (-1 to quit): ",
                         int, valid_index, "Invalid index!")
        if index == -1:
            return
        char = ask_user("Enter a letter: ", str, valid_character,
                        "Character must be one lowercase letter!")
        word[index] = char
        print("".join(word))

Note that your outer for loop is completely unnecessary.

The game can be started under a if __name__ == "__main__": guard to allow importing from this script. Note that you don't need to manually exit the script, it will do so automatically at the end.

if __name__ == "__main__":
    play_word_ladder()


Hide/un-hide directory and open directory based on argument This code checks to make sure a directory is hidden and if not hides it.  Then if hidden it will prompt for password and if password matches it will un-hide it.  There is also an argument to open un-hidden directory too.

#! /bin/bash

DIR="path/untitled"
DIR2="path/.untitled"
password="Z2FsdGVkX18vy4f4QRB3qnb5vIWR0x+tlZmdAmPxX8Y="
input=""
result=""

basic()
{
    if [ -d "$DIR" ]
    then
        clear
        mv $DIR $DIR2
    elif [ -d "$DIR2" ]
    then
        echo -n ": "
        read -s input
        result=$(echo "$password"| openssl enc -aes-128-cbc -a -d -salt -pass pass:wtf)

        if [ "$result" = "$input" ]
        then
            clear
            mv $DIR2 $DIR
        else
            clear
        fi
    else
        :
    fi
exit
}

opendir()
{
    if [ -d "$DIR" ]
    then
        clear
        mv $DIR $DIR2
    elif [ -d "$DIR2" ]
    then
        echo -n ": "
        read -s input
        result=$(echo "$password"| openssl enc -aes-128-cbc -a -d -salt -pass pass:wtf)

        if [ "$result" = "$input" ]
        then
            clear
            mv $DIR2 $DIR
            open $DIR
        else
            clear
        fi
    else
        :
    fi
exit
}

#~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Main ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
if [ $# -eq 0 ]
then
    basic
elif [ $# -eq 1 ]
then
    if [ "$1" = "-o" ]
    then
       opendir
    else
        :
    fi
fi
clear
#~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ End of File ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Avoid echo -n

Avoid using any flags of echo, like this, as it's not portable:

    echo -n ": "

This is more portable, and produces the same output:

    printf ": "

Get rid of empty statements

These empty else statements are pointless,
I suggest to remove them:

else
    :

Simplify

This can be simplified:

    if [ "$result" = "$input" ]
    then
        clear
        mv $DIR2 $DIR
    else
        clear
    fi

Since in both branches of the if-else you clear, this is the same:

    clear
    if [ "$result" = "$input" ]
    then
        mv $DIR2 $DIR
    fi

Quote variables in paths

Typically you should double-quote variables,
especially when used as paths. So instead of this:

        mv $DIR2 $DIR

This is recommended and safer:

        mv "$DIR2" "$DIR"

Avoid duplicated code

The code in opendir and basic is almost the same.
The only difference I see is the content of the "$result" = "$input" conditions.
Consider using a single function with a flag to determine the behavior inside the main conditional.


returning control to caller I have built a class which validates configuration data.  Below is a very simplified version with only the code that's relevant to this question.

class configValidator
{
    private $_errors = [];

    public function validate()
    {
        $this->errorGenerator(1, 'fail');
        if ($this->_errors) return $this->_errors;

        $this->errorGenerator(2, 'pass');
        if ($this->_errors) return $this->_errors;

        $this->errorGenerator(3, 'fail');
        if ($this->_errors) return $this->_errors;

        $this->errorGenerator(4, 'fail');
        if ($this->_errors) return $this->_errors;
    }

    private function errorGenerator($id, $string)
    {
        if ($string == 'fail')  $this->_errors[] = 'an error occurred on ID ' . $id;
    }
}

$configValidator = new ConfigValidator();
$errors = $configValidator->validate();
if (!empty($errors)) {
    exit(json_encode($errors));
} else {
    echo 'config validations passed';
}

In the validate() method, it is important that errors are returned to the caller, if any exist, after each call to the errorGenerator() method.  This is because in the actual version there are arrays with keys and subsequent calls to errorGenerator() are dependent on those array keys existing.  If they don't exist, then PHP gives an "undefined index notice".

As written above, the code operates as expected - it displays ["an error occurred on ID 1"] in the browser and does not get as far as executing the other calls to the errorGenerator() method.  This is the desired behavior.

However, looking at the validate() method, it seems redundant to type this line 4 times
if ($this->_errors) return $this->_errors;
so I experimented with stripping it out of the validate() method and putting that line into the errorGenerator() method like so:

public function validate()
{
    $this->errorGenerator(1, 'fail');

    $this->errorGenerator(2, 'pass');

    $this->errorGenerator(3, 'fail');

    $this->errorGenerator(4, 'fail');
}

private function errorGenerator($id, $string)
{
    if ($string == 'fail')  $this->_errors[] = 'an error occurred on ID ' . $id;
    if ($this->_errors) return $this->_errors;
}

This results in the browser displaying config validations passed, which is not the desired outcome.  It makes sense that is what's being displayed because moving that line into the errorGenerator() method returns control to the validate() method rather than to the code that called the validate() method.

I'm wondering if there's a way to refactor this code such that it returns the errors to the caller of validate(), yet does not repeat this line
if ($this->_errors) return $this->_errors;

Honestly I have no idea what the class is supposed to do. It is not validating anything, there is no input. It just always fails on ID 1 (whatever that means), no matter what. You probably omited too much. And whatever is omited probably deserves a review on its own.

There is no reason to have private $_errors. Change your checks so that the first passes and second fails. Then call validate() twice. The second call will not behave as expected. You better keep it as the method's local variable, or maybe you dont need a variable at all...

Question is why are you returning array of errors, if you only ever return one?
But let's say you have reason, then your errorGenerator is doing too much anyway.

public function validate()
{
  if (check1()) {
    return [$this->getErrorMessage(1)];
  }
  if (check2()) {
    return [$this->getErrorMessage(2)];
  }
  return [];
}

private function getErrorMessage(int $id)
{
  return 'an error occurred on ID ' . $id;
}

The check1, check2 methods represent something that is omited in your code samples...

And actualy you can take it a step further and remove getErrorMessage and inline the messages instead. Specific message for each check. Error message "an error occured on ID xy" is a terrible one.

Another way may be using exceptions, but to me it feels like abuse of exceptions. But I know there are a lot of devs who would disagree.

And last point to say: It's not always possible to squash two lines into one line while retaining qualities like readability. If it was possible, every program (no matter the complexity) could then be written as a well readable one liner ;)


Mixing Fowlers patterns I am developing a large .NET app.  The previous developer used Martin Fowlers Transaction script design pattern.  Please see the code below:

Public Class TypeStudent
    Private _Name As String
    Private _Type As String 'Undergraduate or postgraduate
    Public Property Name() As String
        Get
            Return _Name
        End Get
        Set(ByVal value As String)
            _Name = value
        End Set
    End Property

    Public Property Type() As String
        Get
            Return _Type
        End Get
        Set(ByVal value As String)
            _Type = value
        End Set
    End Property
End Class

Public Class Students
    Public Sub Add(ByVal typeStudent As TypeStudent)

    End Sub

    Public Sub Remove(ByVal typeStudent As TypeStudent)

    End Sub

    Public Sub Enrol(ByVal typeStudent As TypeStudent)
        If typeStudent.Type = "U" Then
            'Undergraduate specific logic
        ElseIf typeStudent.Type = "P" Then
            'Postgraduate specific logic
        End If
    End Sub
End Class

Notice that there is one Student class and Student.Enrol has to check the type of student before executing the appropriate logic.  Now please see the refactored code below, which uses a domain model approach:

Public Class Student
    Private Name As String
    Private Type As String

    Public Sub Add()

    End Sub

    Public Sub Remove()

    End Sub

End Class

Public Class Undergraduate
    Inherits Student

    Public Sub Enrol()

    End Sub

End Class

Public Class Graduate
    Inherits Student

    Public Sub Enrol()

    End Sub

End Class

Notice that Undergraduate and PostGraduate has its own implementation of Enrol.  I want to refactor more code so that it is like sample B to make it more reuseable and maintainable.  My question is; is it a bad idea to mix and match domain logic patterns in the same application i.e. part of the app written using Transaction Script and another part using domain model?

I don't think that there is inherently anything "wrong" with having multiple patterns like this in a single application, especially during a refactoring exercise. I would strongly suggest documenting the patterns that you are changing from and to and the reason for the modification so that future developers understand what is happening.

I would make one substantial modification to your proposed implementation, however: define Enrol at the Student level as an Overridable method.

If there is no default implementation for all students, change Overridable to MustOverride. This will in turn force Student to be declared as MustInherit, but this is ok if you can only call Enrol on instances of Undergraduate or Graduate.


Create a two dimensional int array and find the smallest and biggest member along with the indexes From a CS Diploma course question set:

Create a two dimensional array of ints, fill it with any random integers. Now find the minimum and maximum number, also find the index of those numbers.

I was not sure what they meant by "fill it with any random integers". Should I just use my imagination? Since I wasn't sure, I opted for using rand().
My Code:
#include <stdio.h>
#include <stdlib.h>
#include <time.h>
#include <limits.h>

int main (void) {
    int array[10][10];

    srand((unsigned int)time(NULL));

    struct Nums {
        int num,
            i,
            j;
    } min, max;

    min.num = INT_MAX;
    max.num = 0;

    for (int i = 0; i < 10; i++)
        for (int j = 0; j < 10; j++) {
            array[i][j] = rand();
            if (array[i][j] > max.num) {
                max.num = array[i][j];
                max.i = i;
                max.j = j;
            }
            if (array[i][j] < min.num) {
                min.num = array[i][j];
                min.i = i;
                min.j = j;
            }
        }
    printf("Max number: %d, row: %d, column: %d\n", max.num, max.i, max.j);
    printf("Min number: %d, row: %d, column: %d\n", min.num, min.i, min.j);
    return 0;
}

I did not go through the loop twice, i.e. I checked for the max and min numbers while filling up the array, is that the wrong approach considering the question?
Also is there any easier way to achieve the same result?
Thank you.

The formulated task leaves a lot of room for interpretation. You chose to simplify several points.

Fill [the array]...now find...

I take this as "create a function that finds...". This forces you to organize the program. Combining both (2D array, function) leads to a subtle problem - I ended up with a #define COLS 10, because that number (from your imagination also) has to be known to the function at compilation.
I also think setting max to 0 and min to INT_MAX is mixing up logic and parameters.
The other problem with a function is: how to return the bunch of results. I took your structs and put them into one.
The call at the center is:
struct minmax mm = minandmaxCOLS(array, nrows);
The function takes an array and number of rows. The caller can choose this value, but the number of cols must always be COLS.
If COLS is variable, then a 2D array is the wrong approach. Array of pointers works.
Instead of
array[10][10]

I have
int nrows = 10;
int array[nrows][COLS];

illustrating that the caller (main) can choose the rows, but not the cols, if it wants to use a function.
#include <stdio.h>
#include <stdlib.h>
#include <time.h>
#include <limits.h>

#define COLS 10

/* Result type */
struct minmax {
    int min, minrow, mincol;
    int max, maxrow, maxcol;
};

/* scan for min and max in a 2D array "a[][]" with dims rows and COLS  */
struct minmax
minandmaxCOLS(int a[][COLS], int rows) {

    int r, c;
    struct minmax result;

    /* Initialize result-struct before comparing in loop */
    result.min = result.max = a[0][0];
    result.minrow = result.mincol = 0;
    result.maxrow = result.maxcol = 0;

    for (r = 0; r < rows; r++)
        for (c = 0; c < COLS; c++)
            if (a[r][c] > result.max) {
                result.max = a[r][c];
                result.maxrow = r;
                result.maxcol = c;
            }
            else
                if (a[r][c] < result.min) {
                    result.min = a[r][c];
                    result.minrow = r;
                    result.mincol = c;
                }

    return result;
}

int main (void) {
    int nrows = 10;
    int array[nrows][COLS];

    srand(time(NULL));

    for (int i = 0; i < nrows; i++)
        for (int j = 0; j < COLS; j++)
            array[i][j] = rand();

    /* A kind of test */
    //array[5][5] = -3;
    //array[6][7] = INT_MAX;

    struct minmax mm = minandmaxCOLS(array, nrows);

    printf("Max number: %d, row: %d, column: %d\n", mm.max, mm.maxrow, mm.maxcol);
    printf("Min number: %d, row: %d, column: %d\n", mm.min, mm.minrow, mm.mincol);

    return 0;
}

As a side effect you can now also search for min and max only in certain rows:
struct minmax mm = minandmaxCOLS(array+2, 1);
Output with "test" activated:
Min number: -3, row: 0, column: 5
"row" is now relative to the arg array+2. The "2" means "two rows of 10 (COLS) ints". Same as &array[2].

typedef
The function paramter list can be simplified by using a typedef. After defineing COLS as 10:
typedef int Arrcolsint_t [COLS];
This shows that an array of COLS integers is a basic unit. Now I can forget half (two thirds) of the details of "a":
minandmaxCOLS(Arrcolsint_t a[], int rows)
Same as:
minandmaxCOLS(Arrcolsint_t *a, ....
The advantage is you can use that type in several functions and only change the typedef itself. Without it is int a[][COLS] or int (*a)[COLS].
Row_t might be a better typedef name. Still there always is the compile time value 10 behind it all.


Method to check if TicTacToe board has a winner I have a class called TicTacToeBoard that has a [3][3] array of TicTacToeBoardCells.  Doc neck if board has a winner, I check if any of the 8 possible combinations have 3 of these are type.

Is there any way to make the check of the 8 states more elegant? I am not looking for optimizations like reducing number of checks using a hash table or using check of an earlier nomination, but more about ideas on how I can make my checks more elegant.

TicTacToeBoardCell.h

typedef NS_ENUM(NSInteger,TicTacToeStates)
{
    playerA=-1,
    notSet,
    playerB
};

@interface TicTacToeBoardCell : NSObject

@property TicTacToeStates state;

@end

TicTacToeBoard.h

@interface TicTacToeBoard : NSObject
{
    TicTacToeBoardCell* cells[3][3];
}

-(BOOL) checkForWinner;
-(void) changeStateOfPoint:(CGPoint)cell toState:(TicTacToeStates)state;

@end

TicTacToeBoard.m

//Can this be made more elegant?
-(BOOL) checkForWinner
{
    int sum;

    for (int x=0; x<3; x++)
    {
        sum = 0;
        for(int y=0; y<3;y++)
        {
            sum += cells[x][y].state;
        }
        if (sum == -3 || sum == 3)
        {
            return TRUE;
        }
    }

    for (int y=0; y<3; y++)
    {
        sum = 0;
        for(int x=0; x<3;x++)
        {
            sum += cells[x][y].state;
        }
        if (sum == -3 || sum == 3)
        {
            return TRUE;
        }
    }

    sum = cells[0][0].state + cells[1][1].state + cells[2][2].state;
    if (sum == -3 || sum == 3)
    {
        return TRUE;
    }

    sum = cells[2][0].state + cells[1][1].state + cells[0][2].state;
    if (sum == -3 || sum == 3)
    {
        return TRUE;
    }

    return FALSE;
}

Just in case you find the for loops inelegant, here is an alternative solution. I don't know Objective C, but the following Go code uses only simple integer arithmetic and decimal formatting, so it should be easy to translate to almost any other language. It was fun to write, so have fun solving this little puzzle.

package main

import (
    "fmt"
    "strings"
)

type Board struct {
    b [3][3]int
}

func (bo *Board) HasWinner() bool {
    b := bo.b
    st := 555555555 +
        b[0][0]*110010010 +
        b[1][0]*101010000 +
        b[2][0]*100110001 +
        b[0][1]*110001000 +
        b[1][1]*101001011 +
        b[2][1]*100101000 +
        b[0][2]*110000101 +
        b[1][2]*101000100 +
        b[2][2]*100100110
    str := fmt.Sprintf("%d", st%100000000)
    return strings.Contains(str, "2") || strings.Contains(str, "8")
}

func main() {
    b := &Board{[3][3]int{{1, 1, 1}, {0, -1, -1}, {0, -1, -1}}}
    fmt.Println(b.HasWinner())
}

Hints and spoilers:

! The leading 1s in the number block prevent the numbers from being interpreted as octal. The modulo operation later discards these digits. If performance is critical, you can remove each leading 1 and all following 0s, as well as the % operation. Alternatively you can subtract 100000000 from each of the factors, as well as remove the % operation.
! The 555555555 prevents overflow amongst the separate digits. It has to be 9 digits long because of the extra leading 1s in the number block. The st variable could become negative otherwise. If you remove the leading 1s, the leading 5 can be removed as well.
! Each column in the number block represents one winning combination. First horizontally, then vertically, then diagonally. Therefore, each column has exactly three 1s. If your programming language allows underscores in integer literals, each factor should look like 1_hhh_vvv_dd.
! Each digit of st starts at 5 and is incremented or decremented depending on the relevant cells. When there are 3 relevant cells in either direction (+1, -1), it is a winning combination. Since every column has only 3 1s, overflow cannot happen.


Check if item exists I have a service to Insert TagName , I dont want to insert duplicate tags
I used this :

   public void Insert(string tagName)
    {
        tagName = tagName.Trim();

        if (IsExists(tagName))
            throw new ArgumentException("tagName is Exists");
        Tag model = new Tag { Name=tagName};

        _uow.MarkAsAdded(model);

    }

    public bool IsExists(string tagName)
    {
        return _tags.Any(row=>row.Name==tagName.Trim());
    }

and another way is change If statement like below :

  public void Insert(string tagName)
    {
        tagName = tagName.Trim();

        if (!IsExists(tagName))
        {
            Tag model = new Tag { Name = tagName };
            _uow.MarkAsAdded(model);
        }
    }

I decided to use second one because I add another method Like this :

  public void Insert(string[] tags)
    {
        foreach (var item in tags)
        {
            Insert(item);
        }
    }

If I throw exception , Insert(string[] tags) will not completed .

Updated:

 private readonly IDbSet<Tag> _tags;

      #region ctor

    public TagService(IUnitOfWork uow)
    {
        _uow = uow;
        _tags = _uow.Set<Tag>();

    }

    #endregion

You make this harder then it needs to be.

Since IDbSet implements the IEnumerable<TEntity> interface

 public interface IDbSet<TEntity>
 : IQueryable<TEntity>, IEnumerable<TEntity>, IQueryable, IEnumerable

you can simply use the Contains extension optionally with your own comparer that can implement such things as Trim etc.


Check that input is list of lists of coordinates I have a factory method that should accept a list of coordinate lists, which are pairs of numbers, for example [[(2,4),(3,3),(2,4)],[(6,6),(7,6),(7,7),(6,7),(6,6)]]. I'd like to make sure the input is indeed in this format, but without specifying the types of sequences (might be lists, tuples, numpy arrays, and mixtures of those) or format of the numbers (int, float, etc., but no strings). So I wrote this checking method:

def _check_args(self,coords_features):
    """Check whether the input is a list of lists of pairs of numbers."""
    err = "Invalid input (not a list of coordinate lists): {}".format(coords_features)
    try:
        for coords in coords_features:
            for coord in coords:
                assert(len(coord) == 2)
                for f in coord:
                    1+f
    except (TypeError,AssertionError):
        raise ValueError(err)

Does this look "pythonic" and non-restrictive enough, or should I do anything differently?

My main problem with this method is that you don't make a useful docstring and consequently it's unclear what should or shouldn't pass.

You say that you're checking if the input is a lost of lists of pairs of numbers, but not explaining how. If I had a class with __len__ defined and could be iterated over, that would pass your test. That might sound unlikely, but consider someone passing a set of generators:

>>> coords = [xrange(2)] * 10
>>> _check_args(coords)

...no errors. But xrange most certainly does not return lists. If you only need the pairs to iterate over you'll be fine, but if you want to index them to get the two values then a generator will throw errors. This method should be testing what aspects of the data you need, such as indexing, not just using methods that indicate they're collections or collection like objects.

Once you know what does need to be tested this way, it would be beter to document it clearly in the docstring, so that if unusual data passes, the user can be sure that's because it's acceptable for your purposes and not just something you didn't account for.

Some other quick notes, assert is a statement not a function. You don't need to use brackets for it:

assert len(coord) == 2

You build the error string up front, which seems unnecessary and slightly wasteful as you may never need it. If you want to create it on its own line then you should still put it in the except block.

Your names could be clearer. I don't know what a coords_features is. arguments or args could be better names, since this function isn't really co-ordinate specific so much as it is format specific. I'd recommend either coordinates or pairs for the longer lists and coordinate or pair for the 2 item collections. The choice depends on how broad or narrow you intend this to be, but the docstring currently sounds more generic.

Last nitpick, in your error message you refer to lists only. If you want it to be broad you may want to use Collection as that refers to the range of list and dict like objects. Though if your intent is to encourage the user to use lists then it's a good error message.


Using logic to compare absolute values Is there a way to make my code simpler/more Pythonic?

  Given three int values (a, b, c), return True if one of b or c is "close"
  (differing from a by at most 1), while the other is "far", differing
  from both other values by 2 or more. Note: abs(num) computes the
  absolute value of a number.

  close_far(1, 2, 10) → True
  close_far(1, 2, 3) → False
  close_far(4, 1, 3) → True

def close_far(a, b, c):
    if abs(a - b) <= 1 or abs(a - c) <= 1:
        if abs(c - a) >= 2 <= abs(c - b) or abs(b - a) >= 2 <= abs(b - c):
            return True
    return False

Some tips:

You could replace the innermost if with return
Adding doctests would be nice
Note that abs(a - b) is the same as abs(b - a). To avoid repeated calculation of abs values, it would be more optimal to cache in local variables

Like this:

def close_far(a, b, c):
    """
    >>> close_far(1, 2, 10)
    True
    >>> close_far(1, 2, 3)
    False
    >>> close_far(4, 1, 3)
    True
    """
    diff_ab = abs(a - b)
    diff_ac = abs(a - c)
    if diff_ab <= 1 or diff_ac <= 1:
        diff_bc = abs(c - b)
        return diff_ac >= 2 <= diff_bc or diff_ab >= 2 <= diff_bc
    return False


Random numbers, IO monad, and pure code To get used to Haskell, I wrote a little program using the Diagrams package which generates 100 random dots (larger dots are more likely to be in the center).

Sample output:

My questions:

Is the program well structured with respect to the IO monad and the random values (see types for randomDot and randomDots)? Should I organize the code differently to keep more code out of the IO monad?
Other nitpicks?

module Main (main) where

import Data.Colour.SRGB
import Data.Random.Source
import Diagrams.Backend.SVG.CmdLine
import Diagrams.Prelude
import qualified Data.Random as R

data Dot = Dot { _dotCenter :: R2
               , _radius :: Double
               , _color :: Colour Double
               } deriving Show

colors :: [Colour Double]
colors = map sRGB24read [
          "bf3131",
          "f5b456",
          "a89178",
          "615b5b",
          "add274",
          "b9a1b9",
          "f0a2bc",
          "eb565c",
          "d15f69",
          "48bdbe",
          "f1ede2"]

-- |Generate a single dot with random location, radius, and color.
randomDot :: Double -> R.RVar Dot
randomDot x = do
  let mu_rad = 15 * exp (-4 * x)
      sigmaSq_rad = 0.3 * mu_rad
      sigmaSq_loc = 8 * exp (2.5*x)
  locX <- R.sample (R.normal 0 sigmaSq_loc)
  locY <- R.sample (R.normal 0 sigmaSq_loc)
  radius <- abs <$> R.sample (R.normal mu_rad sigmaSq_rad)
  color <- R.sample (R.randomElement colors)
  return $ Dot (r2 (locX, locY)) radius color

-- |Recursively generate random dots and check that they do not
-- overlap.
randomDots :: [Dot] -> [Double] -> IO [Dot]
randomDots dots [] = return dots
randomDots dots (x:xs) = do
  dot <- R.sample $ randomDot x
  if any (tooClose dot) dots
  then randomDots dots (x:xs)
  else randomDots (dot:dots) xs

tooClose :: Dot -> Dot -> Bool
tooClose x y = dist < 1.1 * radiusSum
    where
      dist = magnitude $ _dotCenter x ^-^ _dotCenter y
      radiusSum = _radius x + _radius y

dotsToDiagram :: [Dot] -> Diagram B R2
dotsToDiagram = mconcat . map fromDot

fromDot :: Dot -> Diagram B R2
fromDot c = circle (_radius c) # fc (_color c)
                               # lw none
                               # translate (_dotCenter c)

main :: IO ()
main = mainWith . dotsToDiagram =<< randomDots [] [0.01, 0.02..1.0]

I'd suggest to change the type signature of randomDots to

randomDots :: [Dot] -> [Double] -> R.RVar [Dot]

as there is nothing IO based there. And then change main to

main = mainWith . dotsToDiagram =<< R.sample (randomDots [] [0.01, 0.02..1.0])

Nitpicks: I'd put tooClose before randomDots so that the functions are in logical order, but that's just a matter of personal preferences.

Otherwise I also quite liked the code.

Update: Some further thoughts: Function randomDots is concise, but it also mixes several concepts together. Namely:

it re-runs generation of a dot (until it fits wrt the previous ones);
it checks each generated dot with all previously generated ones.

So I'd suggest to split the function into even smaller ones, each targeting one of these problems. While the resulting code is larger, I believe it's easier to comprehend and more maintainable in the long run:

-- | Runs a monadic action while its result satisfies a given predicate.
iterateWhile :: (Monad m) => (a -> Bool) -> m a -> m a
iterateWhile pred k = loop
  where
    loop = k >>= \x -> if pred x then loop else return x

This function is also available in monad-loops. Then:

-- | Generates a list of values that are independent wrt a given (commutative)
-- predicate.
-- Each monadic action is repeated until its result is independent with all the
-- previous ones.
genIndependent :: (Monad m) => (a -> a -> Bool) -> [m a] -> m [a]
genIndependent pred = foldM step []
  where
    step rs k = liftM (: rs) $ iterateWhile (\x -> any (pred x) rs) k

Both these functions are polymorphic with no coupling with RVar. The final function is then expressed just as

-- |Recursively generate random dots and check that they do not
-- overlap.
randomDots :: [Double] -> R.RVar [Dot]
randomDots = genIndependent tooClose . map (R.sample . randomDot)


Determining if current thread is executing Using C#, create a class which gets an action (parameter-less
  delegate) in its construction and has a single public method bool
  execute() which does the following:

  Executes the action only if no other thread is currently executing the action
  If another thread is already executing, wait until thread is finished before returning (but without executing action again)
  Return true if current thread executes that action, otherwise return false.

public delegate bool Action();

private static int ThreadsCount = 100; //initialize threads count

static void Main(string[] args)
{
    Action act = Execute;
    bool t = act();
    Console.WriteLine(t);
    Console.ReadLine();
}

static bool Execute()
{
    bool retVal = false;
    Thread thread = Thread.CurrentThread;
    string s = thread.Name;
    if (s == null)
    {
        test();
        retVal = true;
    }
    else
    {
        ThreadsCount = ThreadsCount - 1;

        if (ThreadsCount < 1)
        {
            //if all threads finished executing do whatever you wanna do here..
            Console.WriteLine("C# language");
            test();
            retVal = true;
        }
        else
        {
            retVal = false;
        }

    }
    return retVal;
}

private static void test()
{
    Console.WriteLine("C# language");
}

Is the above piece of code correct? Please give me your feedback.

Please don't think that this is just exercise. This question was asked by one of the interviewers.

You need to look at how you can make the Execute method both thread-safe and true to the requirements of the method. You say that it only should return true in the case where the delegate is actually called, but you need to make sure that only one thread can call it at a time. That's where lock comes in. You could potentially do something like:

public class SingleCallAction
{
  private readonly object sync = new object();
  private readonly Action action;
  private bool executed;

  public SingleCallAction(Action action)
  {
    if (action == null)
      throw new ArgumentNullException("action");

    this.action = action;
  }

  public bool Execute()
  {
    if (!executed)
    {
      lock(sync)
      {
        if (!executed)
        {
          action();
          executed = true;
          return true;
        }
      }
    }

    return false;
  }
}

Now, what this class provides is a wrapper around an Action delegate. We use Action because it supports both delegate assignment, or lambda assignment.

The Execute method will first check to see if it has previously been executed (if it has, it will jump down to the return false; as its not the thread that has executed the delegate.

If if hasn't executed, it will lock the sync object to prevent any other threads from executing the inner code block. The compiler is generating Monitor.Enter and Monitor.Exit calls here for you, worth a read up!

After the lock, we check whether the executed flag is set again. This allows us to prevent race conditions for competing threads. Don't worry about race conditions when dealing with the executed flag, as variable reads/writes are always atomic operations (mostly).

If we are the first thread to be executing the delegate, we can then execute it action(), set the flag as executed, and return true.

Any subsequent calls will always result in false.

To use in code:

var action = new SingleCallAction(() => Console.WriteLine("Executed"));

var thread1 = new Thread(() => Console.WriteLine(action.Execute()));
var thread2 = new Thread(() => Console.WriteLine(action.Execute()));

thread1.Start();
thread2.Start();

Should result in:

Executed
True
False


Getting the length of a file in characters Here's my function for getting the length of a file, in characters:

unsigned int GetFileLength(std::string FileName)
{
std::ifstream InFile(FileName.c_str());
unsigned int FileLength = 0;
while (InFile.get() != EOF) FileLength++;
InFile.close();
return FileLength;
}

How can this be improved?

Just seek to the end of the file and grab the value:

ifstream is;
is.open (FileName.c_str(), ios::binary );
is.seekg (0, ios::end);
length = is.tellg();


Query that shows similar columns between different tables on a database In my work, I often have to take a naive look at tables and then never see them again, and it can be difficult to see what lines up where for large databases with many columns. I wrote this, and while it works, it seems to require an unnecessary number of joins. I'm wondering if there is a better way to handle this issue or a way to cut down the code.

select A.COLUMN_NAME as 'Similar Column', y.TABLE_NAME as 'First Table', z.TABLE_NAME as 'Second Table'
from INFORMATION_SCHEMA.COLUMNS A
full outer join INFORMATION_SCHEMA.COLUMNS B
    on A.COLUMN_NAME = B.COLUMN_NAME
cross apply (
    SELECT TABLE_NAME FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_TYPE='BASE TABLE') as y
cross apply (
    SELECT TABLE_NAME FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_TYPE='BASE TABLE') as z
where y.TABLE_NAME <> z.TABLE_NAME
    and A.TABLE_NAME = y.TABLE_NAME
    and B.TABLE_NAME = z.TABLE_NAME
order by y.TABLE_NAME,z.TABLE_NAME, a.COLUMN_NAME

I also wonder if there is a way to prevent duplicate entries, in other words, Activity, RMA is the same as RMA, Activity.

A common-table-expression (See CTE) will help you here quite nicely..

with columns as (
    select C.COLUMN_NAME,
           C.TABLE_NAME
    from INFORMATION_SCHEMA.COLUMNS C
    inner join INFORMATION_SCHEMA.TABLES T on T.TABLE_NAME = C.TABLE_NAME
    where T.TABLE_TYPE = 'BASE TABLE')
select C.COLUMN_NAME as 'Similar Column',
       C.TABLE_NAME as 'First Table',
       D.TABLE_NAME as 'Second Table'
from columns C
inner join columns D
   on C.COLUMN_NAME = D.COLUMN_NAME
  and C.TABLE_NAME < D.TABLE_NAME
order by C.TABLE_NAME, D.TABLE_NAME, C.COLUMN_NAME

The above query reduces the complexity of the column/table table joins, and reuses the result. Further, by using < instead of <> on the table-name anti-join, it removes your duplicates.


Finding correct format? Arrays and Modulation I am fairly new to programming and I am having difficulty modulating the following code.

The program reads a file, selects certain requirements, and displays them.  I have tried to use passing arrays as arguments or functions as indicated in my textbook.  I have been using examples such as int getAges(int array[], integer).

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.IO;

namespace Project_2_practice
{
    class Program
    {
       static void Main(string[] args)
        {

            //Parsing data into memory
            string content;
            using (StreamReader reader = new StreamReader(File.Open("Data.txt", FileMode.Open)))
            {
                content = reader.ReadToEnd();
            }
            string[] rows = content.Split('\n'); //Each row is on a new line
            string[][] table = new string[rows.Length][];
            for (int i = 0; i < rows.Length; table[i] = rows[i].Split(','), i++) ;

     //selecting information

            int[] districts = new int[rows.Length];
            int[] ages = new int[rows.Length];

            for (int i = 0; i < rows.Length; i++)
            {
                districts[i] = int.Parse(table[i][3]);
                ages[i] = int.Parse(table[i][0]);
            }
                //Analyzing selected information
                foreach (int district in districts.Distinct().OrderBy(x => x))
                Console.WriteLine("District {0} has {1} resident(s)", district, districts.Count(x => x == district));
                Console.WriteLine("Ages 0-18 : {0} resident(s)", ages.Count(x => x < 18));
                Console.WriteLine("Ages 18-30 : {0} resident(s)", ages.Count(x => x >= 18 && x <= 30));
                Console.WriteLine("Ages 31-45 : {0} resident(s)", ages.Count(x => x >= 31 && x <= 45));
                Console.WriteLine("Ages 46-64 : {0} resident(s)", ages.Count(x => x >= 46 && x <= 64));
                Console.WriteLine("Ages >=65 : {0} resident(s)", ages.Count(x => x >= 65));
        }
    }
}

Here's how I would do it.

Update: I included a description of my code this time based on comments.

First, reading a file into a variable so that each line of the file is its own element in an array is super easy with File.ReadAllLines(...). But I don't feel like using a variable for that is necessary here, so I immediately start using the array by using the Select(...) method. I'm using Select here to create an anonymous type (That's the return new { ... } part).

var data = File.ReadAllLines(@"c:\temp\data.txt").Select (f =>
{
    var split = f.Split(',');
    return new {
        Age = int.Parse(split[0]),
        District = split[3]
    };
});

At this point, I've got a type-safe collection of "data" that I can work with. That's important, and very useful.  For example..

var byDistrict = data.GroupBy (d => d.District);

In the line above, I've grouped the data by District. Grouping is, in my opinion, almost always better than using Distinct.

The rest of the code is very similar to your original code, except my code avoids the need to have the age reports within a loop.

foreach (var district in byDistrict)
{
    Console.WriteLine("District {0} has {1} resident(s)", district.Key, district.Count());

}

Console.WriteLine("Ages 0-18 : {0} resident(s)", data.Count(x => x.Age < 18));
Console.WriteLine("Ages 18-30 : {0} resident(s)", data.Count(x => x.Age >= 18 && x.Age <= 30));
Console.WriteLine("Ages 31-45 : {0} resident(s)", data.Count(x => x.Age >= 31 && x.Age <= 45));
Console.WriteLine("Ages 46-64 : {0} resident(s)", data.Count(x => x.Age >= 46 && x.Age <= 64));
Console.WriteLine("Ages >=65 : {0} resident(s)", data.Count(x => x.Age >= 65));


ASP.NET caching helper I'm willing to simplify the use of the ASP.NET cache.

I wrote this helper class:

public static class CacheHelper
{
    public static T GetCached<T>(string key, Func<T> initializer, DateTime absoluteExpiration)
    {
        return GetCached(key, initializer, Cache.NoSlidingExpiration, absoluteExpiration);
    }
    public static T GetCached<T>(string key, Func<T> initializer, TimeSpan slidingExpiration)
    {
        return GetCached(key, initializer, slidingExpiration, Cache.NoAbsoluteExpiration);
    }
    public static T GetCached<T>(string key, Func<T> initializer, TimeSpan slidingExpiration, DateTime absoluteExpiration)
    {
        var httpContext = HttpContext.Current;

        if (httpContext != null)
        {
            var obj = httpContext.Cache[key];
            if (obj == null)
            {
                obj = initializer();
                httpContext.Cache.Add(key, obj, null, absoluteExpiration, slidingExpiration, System.Web.Caching.CacheItemPriority.Default, null);
            }
            return (T)obj;
        }
        else
        {

            return initializer(); // no available cache
        }
    }
}

The idea is to let the developer write its method as usual, but wrapping it like this:

public MyClass GetSomeCachedData(){
            return CacheHelper.GetCached(
                "GetSomeCachedData, () =>
                {

                        return ComputeExpensiveResult();
                }
                }, TimeSpan.FromMinutes(1), Cache.NoAbsoluteExpiration);
}

Do you see any issues?  Can it be improved?

(I'm sticking to .NET 3.5)

Several suggestions:

You could use MemoryCache class as of .NET 4, which would free you from System.Web assembly.
You should use lock in order to put the value to cache.
You should take care of value types objects

So, here is the code:

public static T GetCached<T>(string key, Func<T> initializer, TimeSpan slidingExpiration, DateTime absoluteExpiration)
{
    var httpContext = HttpContext.Current;

    if (httpContext != null)
    {
        key = string.Intern(key);
        lock(key) // locking on interned key
        {
            var obj = httpContext.Cache[key];
            if (obj == null)
            {
                obj = initializer();
                httpContext.Cache.Add(key, obj, null, absoluteExpiration, slidingExpiration, System.Web.Caching.CacheItemPriority.Default, null);
            }
            // taking care of value types
            if (obj == null && (typeof (T)).IsValueType)
            {
                return default(T);
            }
            return (T)obj;
        }
    }
    else
    {
        return initializer(); // no available cache
    }
}


Defining a Constructor In an Interface Java? I have multiple different implementations of an object, which implement this custom interface I made called Board.

Board contains a method that looks like the following

public void ConvertFromString(String formattedString);

Each object implementing Board calls ConvertFromString() in its constructor.

Looks like the following.

public void BoardImpl1 implements Board
{
    public Board(string B)
    {
        ConvertFromString(b);
    }

    public void ConvertFromString(String formattedString)
    {
        //do some parsing on string and set up the BoardImpl properties
    }
}

ConvertFromString being public causes a warning, so the best practice workaround would be to make BoardImpl final. Is there a better way to write this?

The warning you get, is because you call an overridable method from your constructor. This is bad because subclasses may change the implementation of the method to access other parts of the object, while it is not fully initialized yet.

So to get rid of the warning : don't call overridable methods from the constructor.

Then how to get around your predicament?

Well you could :

make the public method final.
make the class final (which you found yourself)
duplicate the logic (while possible, not the way to go)
have the public method delegate to a private method, and have the constructor call the private method.

Like this :

public class BoardImpl implements Board {
    public BoardImpl(String b) {
        doConvertFromString(b);
    }

    public void convertFromString(String formattedString) {
        doConvertFromString(formattedString);
    }

    private void doConvertFromString(String formattedString) {
        //do some parsing on string and set up the BoardImpl properties
    }
}

I wouldn't worry about making all implementations final. final is your friend.


Spelling bee test writer Is there any way I can cut down on code? I'm new to this.

Sub addtest()

    Dim decision As Char
    Dim fullline As String = " "

    Dim word(9), def(9), minorerror1(49), minorerror2(49), minorerror3(49), minorerror4(49), minorerror5(49) As String

    Console.Clear()
    Console.WriteLine("Please select the year of students you wish to save the test for: ")
    Console.WriteLine()
    Console.WriteLine("3 - Year 3")
    Console.WriteLine("4 - Year 4")
    Console.WriteLine("5 - Year 5")
    Console.WriteLine("6 - Year 6")

    Console.WriteLine()

    Dim testyear As Integer = Console.ReadLine()

    Select Case testyear

        Case 1
        Case 2
        Case 3
        Case 4

    End Select

    Console.WriteLine("Please enter 10 of your selected words, defenitions and selected amount minor errors to be featured in the following test")

    FileOpen(5, "F:\Computing\Spelling Bee\testtests.csv", OpenMode.Append)

    Console.Clear()

    Do

        counter = counter + 1
        Console.Write("Word: ")
        word(counter) = Console.ReadLine
        Console.Write("Defenition: ")
        def(counter) = Console.ReadLine

        Console.WriteLine("Type in a max of 5 minor errors")

        Console.Write("Minor error 1: ")
        minorerror1(counter) = Console.ReadLine
        Console.Write("Minor error 2: ")
        minorerror2(counter) = Console.ReadLine
        Console.Write("Minor error 3: ")
        minorerror3(counter) = Console.ReadLine
        Console.Write("Minor error 4: ")
        minorerror4(counter) = Console.ReadLine
        Console.Write("Minor error 5: ")
        minorerror5(counter) = Console.ReadLine

    Loop Until counter = 9
    Console.Clear()

    Console.WriteLine("Are you sure you want to save this test? (y/n) ")
    decision = Console.ReadLine
    If decision = "y" Or decision = "Y" Then
        For counter As Integer = 0 To 9
            fullline = testyear & "," & word(counter) & "," & def(counter) & "," & minorerror1(counter) & "," & minorerror2(counter) & "," & minorerror3(counter) & "," & minorerror4(counter) & "," & minorerror5(counter)
            PrintLine(5, fullline)
            Console.WriteLine("Word: " & (word(counter.ToString)) & " Defenition: " & (def(counter.ToString)) & " Minor error 1: " & (minorerror1(counter.ToString)) & " Minor error 2: " & (minorerror2(counter.ToString)) & " Minor error 3: " & (minorerror3(counter.ToString)) & " Minor error 4: " & (minorerror4(counter.ToString)) & " Minor error 5: " & (minorerror5(counter.ToString)))
        Next
        FileClose(5)
        Console.Clear()
        staffmenu()
    ElseIf decision = "n" Or decision = "N" Then
        addtest()

    End If

End Sub

You should read up on arrays :

    Console.Write("Minor error 1: ")
    minorerror1(counter) = Console.ReadLine
    Console.Write("Minor error 2: ")
    minorerror2(counter) = Console.ReadLine
    Console.Write("Minor error 3: ")
    minorerror3(counter) = Console.ReadLine
    Console.Write("Minor error 4: ")
    minorerror4(counter) = Console.ReadLine
    Console.Write("Minor error 5: ")
    minorerror5(counter) = Console.ReadLine

This should be a loop which writes to an array of strings.

Proposed reading : MSDN link on arrays


Heap implementation using pointer I know heaps are commonly implemented by an array. But, I wanted to share my pointer implementation and have your feedback(s) :)

General idea:

I convert the index to its binary value and then trace the heap (0 = leftChild & 1 = rightChild)] Note: The first 1 is for entering the root.
I do the push at the same time.

Example: for the 12th element, 12 = 1100. The first 1 is for entering the root. Now I trace the heap by following 100.

    struct Node{
         Node *leftChild;
         Node *rightChild;
        int data;
    };

    bool insertHeap(Node **heap, int data, short index){
        Node *newNode, *temp, *parent;
        int tempData;
        short mask = 0x80;

    try{
        newNode = new Node;
    }
    catch (bad_alloc& e) {
        cerr << "Memory allocation error: " << e.what() << endl;
        return false;
    }

    while (!(index & mask)) index <<= 1;

    index <<= 1;
    parent = NULL;
    temp = *heap;

    while (temp){
        if (data > temp->data){
            tempData = temp->data;
            temp->data = data;
            data = tempData;
        }
        parent = temp;

        if (!(index & mask)) temp = temp->leftChild;
        else temp = temp->rightChild;

        if (temp) index <<= 1;
    }

    newNode->leftChild = NULL;
    newNode->rightChild = NULL;
    newNode->data = data;

    if(!parent){
        *heap = newNode;
        return true;
    }

    if (!(index & mask)) parent->leftChild = newNode;
    else parent->rightChild = newNode;

    return true;
}

This is my main():

#include <iostream>
using namespace std;

int main (){
    Node *heap = NULL;
    int arr[13] = {3,6,8,1,5,9,4,2,0,7,11,14,13};

    for (int i=0; i<13; i++) insertHeap(&heap, arr[i], i+1);
    return 0;
}

You could also use a map, associative array, or dictionary to implement a heap (like the following):

class Heap(dict):

    "Heap() -> Heap instance"

    def retrieve(self, address):
        "Get the virtual value stored at the address."
        return self.get(address, 0)

    def store(self, value, address):
        "Set the virtual value meant for the address."
        if value:
            self[address] = value
        else:
            self.pop(address, None)


Generic method for obtaining enum items in an ushort object The idea is to perform a bit-wise scan of an ushort provided to find any and all matches within an enum collection.

The code I developed is as follows:

public List<T> GetEnumItemsFromUshort<T>(ushort input) where T : struct, IComparable, IConvertible, IFormattable
    {
        var output = new List<T>();
        foreach (T enumValue in Enum.GetValues(typeof(T)))
        {
            if ((input & enumValue.ToUInt16(new CultureInfo("en-US"))) == enumValue.ToUInt16(new CultureInfo("en-US")))
            {
                output.Add(enumValue);
            }
        }
        return output;
    }

As you can see, I use a generic type to define the return type.

I searched the site and found that constraining the generic type with the following struct, IComparable, IConvertible, IFormattable, makes it behave like an enum.

The code generates a list of the enum provided to contains the matches found.

Then it proceeds to scan the input value to find any and all items that masked (a bit-wise operation as stated before) do match.

If a match is found it adds the enum case to the return list.

I want to know whether this is a good or bad implementation and ways to make it better, it performs as expected but I feel like there's room for optimization.

To provide an example, suppose the following enum and input:

public ushort sample = 6;

public enum SampleCases
{
    Case1 = 0x1,
    Case2 = 0x2,
    Case3 = 0x4,
    Case4 = 0x8,
}

The output for this code would be a List of type SampleCases with 2 items, Case2 and Case 3 since both (bit-wise) are contained in the sample provided. Tell me if this works as an example for you.

Code can be greatly simplified but first I'd list few things:

1) If your enum is a bitmap then you should mark it with [Flags].

[Flags]
public enum SampleCases
{
    Case1 = 0x1,
    Case2 = 0x2,
    Case3 = 0x4,
    Case4 = 0x8,
}

2) you shouldn't convert an enum value to ushort, enums default type is int and for [Flags] you may (easily?) run out of bits. If value is out of ushort range (your input) simply drop it, Convert.ToInt16() will throw OverflowException for out of range values.

3) You shouldn't create a new instance of CultureInfo every time you need it. It may be expansive. Moreover you're using en-US culture, you already have the CultureInfo.InvariantCulture.

3) To know if a value is valid you can use Enum.IsDefined().

To put all together:

public List<T> GetEnumItemsFromUshort<T>(ushort input)
    where T : struct, IComparable, IConvertible, IFormattable
{
    const int bitsInUInt16 = sizeof(ushort) * 8;

    return Enumerable.Range(0, bitsInUInt16 - 1)
        .Select(x => 1 << x)
        .Where(x => (input & x) == x && Enum.IsDefined(typeof(T), x))
        .Select(x => (T)Convert.ChangeType(x, typeof(T)))
        .ToList();
}

It's a minor change and a micro-optimization but I'd also change return type to IEnumerable<T>. If caller will need a list then it will be able to call ToList() itself and if it does not need it then you will avoid an unnecessary copy. Also first Select() may be dropped and embedded in Where() clause.

Note that 0 is not handled, you may want to include it or not; I'd leave it out because it's always matched unless you have input == 0. Note that to do not have a 0 default value for an enum is a dangerous practice, value types are 0 initialized and an uninitialized enum will have an unknown value.

Inverting point of view you may write:

public List<T> GetEnumItemsFromUshort<T>(ushort input)
    where T : struct, IComparable, IConvertible, IFormattable
{
    return Enum.GetValues(typeof(T))
        .Cast<object>()
        .Where(x => ((ushort)x & input) == (ushort)x)
        .Cast<T>()
        .ToList();
}

This 2nd version will also handle shared bits like GuestDefault, UserDefault and AdminDefault in this example:

[Flags]
enum SampleCases {
    None = 0,
    Read = 1,
    Write = 2,
    Execute = 4,
    GuestDefault = Read,
    UserDefault = Read | Execute,
    AdminDefault = Read | Write | Execute
}

Note the hacky .Cast<object>() to use .Where() over a non-generic enumeration (also paying boxing price). Here I tried to keep code short but I wouldn't implement this with LINQ (generated enumerator with yield return will be more clear, IMO). Also this code returns duplicates and aliases, if it's not what you want then you have to handle them (checking for distinct values and for multiple bits set on the same value).

Very last point: your constraints for generic parameter reduce wrong usage of your function but it doesn't really limit usage to enums: most primitive types implement same interfaces. I'd also add an explicit check:

if (!typeof(T).IsEnum))
    throw new ArgumentException("Template argument must be an enum type.");


Axiomatic proofs of natural addition properties, in Idris I'm a seasoned programmer in a variety of languages (including Haskell and some ML dialects), but i'm a complete dilettante when it comes to formal logic. I've very briefly looked at Coq, and now i'm trying to dabble in theorem proving with Idris.

In particular, before trying to use the interactive elaborator, I'd like to understand how to create simple proofs, because I had a hard time following the elaborator section of the tutorial. So, I've written the following program, that typechecks successfully:

total
project : a = b -> S a = S b
project Refl = Refl

total
sym : a = b -> b = a
sym Refl = Refl

total
transitive : a = b -> b = c -> a = c
transitive Refl Refl = Refl

total
plus_assoc : (a: Nat) -> (b: Nat) -> (c: Nat) -> a+(b+c) = (a+b)+c
plus_assoc Z     b c = Refl
plus_assoc (S a) b c = project (plus_assoc a b c)

total
plus_succ_commute : (n: Nat) -> (m: Nat) -> S n + m = n + S m
plus_succ_commute Z     m = Refl
plus_succ_commute (S n) m = project (plus_succ_commute n m)

total
plus_commutes : (n: Nat) -> (m: Nat) -> n + m = m + n
plus_commutes Z     Z     = Refl
plus_commutes Z     (S m) = project (plus_commutes Z m)
plus_commutes (S n) m     = transitive (project (plus_commutes n m)) (plus_succ_commute m n)

I'd appreciate advice on how to make things cleaner, and in particular:

At first, I tried to write a direct proof for plus_commutes that would not involve plus_succ_commute, but alas, to no avail.
In the second clause of plus_assoc, I tried to get rid of my definition of project by writing

plus_assoc (S a) b c = let Refl = plus_assoc a b c in Refl

Which seems to me as just substituting the definition of project into the original clause, but fails with a type error:

Type mismatch between
    plus (plus a b) c = plus (plus a b) c (Type of Refl)
and
    a + (b + c) = a + b + c (Expected type)

Specifically:
    Type mismatch between
        plus (plus a b) c
    and
        plus a (plus b c)

Can anyone offer insight as to why inlining this definition breaks the proof ? perhaps I ought to pass some implicits to constraint the type of the subexpression ?

Inlining the second clause fails because you're shadowing Refl.

plus_assoc (S a) b c = let Refl = plus_assoc a b c in Refl

just reduces to

plus_assoc (S a) b c = plus_assoc a b c


AIO programming challenge - Friendlist - Who has the most friends? I attempted this sample question from the Australia Infomatics Competition. TL;DR - Each line of input contains two numbers, representing a mutual friendship relationship. The output needs to be the number representing the person who has the most friends (and if there is a tie, list them in numerical order).
Exact Words:

Input
The first line of input will contain the integer f, 1 <= f <= 1,000,000.
Each of the following f lines will be of the form a b, where a and b are different player IDs. This indicates that player #a is friends with player #b, and vice versa. All player IDs are integers between 0 and 1000 inclusive.
(Note that no friendship will ever be listed twice: for example, if "2 5" is one of the lines in the input, then neither "2 5" nor "5 2" will appear anywhere else in the input)
Output
Output should consist of all the player IDs that are tied for biggest friendlist. These IDs should be given in ascending order.
Sample Input:
6
5 6
0 1
1 4
5 4
1 2
4 0
Sample Output:
1
4

Here is my current code (in Python 2.6.6 as per the comp's requirements):
fileInput = open("listin.txt", "r")
InputList = map(int, fileInput.read().split())
fileInput.close()

input1 = InputList[1:]
friends_dict = {}

for each in xrange(int(InputList[0])+1):
    friends_dict[each] = input1.count(each)

highest = max(friends_dict.values())

biggest_nums = sorted([k for k, v in friends_dict.items() if v == highest])

Output = "\n".join([str(e) for e in biggest_nums])

fileOutput = open("listout.txt", "w")
fileOutput.write(str(Output))
fileOutput.close()

My Approach

First read in the whole file at once (is this a good idea? would it be better to do line by line?)

Count all occurrences of a friend ID and then assign it to a spot in the dictionary (thus works in theory as if A is friends with B and A is friends with C, if we were to count them, A appears 2x and that is how many friends A has, likewise with B = 1 and C = 1)

Find the maximum score(s)

The code seems to work alright (although the test machine did say one case failed - it works for the smaple input and some cases I made up, but I think that there are still flaws in the method) but the main issue seems to be that it is too slow. Over half the trial cases failed due to going over the 1 sec limit. I am open to any suggestions or even a new algorithm is general and suggestions to my coding in general.
I have been at this for a while now and I can't think of anything else to shave off. I suspect the bottleneck may be at the end which is simply finding the largest friend counts and ordering them.
I would appreciate any feedback whatsoever, be it from my coding practices to the implementation of or the actual algorithm. I would really like to be able to produce a faster and more efficient version of this code.

Take a look at my snippet which passes all the test cases.
First thing you can improve is both reading and incrementing (in the same loop) number of friendships for each id. Try to not use also count method of the list, it also searches through the whole list every time.

#!/usr/bin/python
friendlist = [0] * 1001

with open("listin.txt", "r") as listin:
    for i, line in enumerate(listin):
        if i > 0:
            a,b = line.split()
            friendlist[int(a)] += 1
            friendlist[int(b)] += 1

max_friends_num = max(friendlist)
listout = open("listout.txt", "w")
for id,val in enumerate(friendlist):
    if val == max_friends_num:
        listout.write(str(id)+'\n')
listout.close()

Best regards!


Reverse a string without the  header I'm learning C from the K.N. King book and just arrived at a question to reverse the words in a sentence.

Here is the output:

  Enter a sentence: you can cage a swallow can't you?

  Reversal of sentence: you can't swallow a cage can you?

Though I've done this and it runs correctly, I want to make my code shorter. I'm not asking for any other solution.

#include <stdio.h>

#define N 100

int main(void) {
  char ch[N], terminate;
  int i, j, k, len;
  printf("Enter a sentence: ");
  for(i = 0; i < N; i++) {
    ch[i] = getchar();
    if(ch[i] == '?' || ch[i] == '.' || ch[i] == '!' || ch[i] == '\n') {
      len = i;        //Determine the length of the string
      terminate = ch[i];
      break;
    }
  }
  int word_count = 0;
  for(j = len - 1; j >= 0; j--) {
    word_count++;
    if(ch[j] == ' ') {
      for(k = j+1; k < len; k++) {
        printf("%c", ch[k]);
      }
      printf("%c", ch[j]);  // print space character
      len = len - word_count;
      word_count = 0;
    }

  }

  for(i = 0; i < len; i++) {
    printf("%c", ch[i]);
    if(ch[i] == ' ') {
      break;
    }
  }
  printf("%c", terminate);

  return 0;
}

This can be done only with loops, arrays, getchar() and putchar() methoda using only <stdio.h> header and no extra header (restrict <string.h> header functions and methods). Also, no puts() and gets() functions. I guess this can be done only with two loops.

I have a few comments.

main takes argc/argv normally. Also start the function with the opening
brace in colum 0
Define one variable per line.
Your request for input might be better printed to stderr so that it can be
separated from program output.
Your initial loop should test for EOF. Currently it doesn't  You can test
this in the *NIX shell with (program is your executable):

/bin/echo -n "hello world" | ./program

terminate seems redundant
You can and perhaps should define loop variables in the loop (as long as
you don't need the value outside the loop):

for (int i = 0; i < n; ++i) {

The second for-loop is a little messy.  For a start it has a nested loop -
my advice is to avoid nested loops and extract the inner loop to a function.
This is not always practical, but more often than not it is useful.  In this
case, you can write a little function (eg. print_chars) that prints a
specified number of characters.
Your terminating loop can be replaced by a call to the print_chars
function.
putchar is preferable to printf("%c", ...)
while, for, if etc should be followed by a space.
Testing for ' ' might be better done using isspace, which will test for
other space-like characters too.

Here is my version, for what it is worth:

#include <stdio.h>
#include <ctype.h>

#define N 100

static inline void print_chars(const char *s, long n)
{
    for (int i = 0; i < n; ++i) {
        putchar(s[i]);
    }
}

int main(int argc, char **argv)
{
    char line[N];
    int ch; /* note that ch is an int to allow it to hold 'EOF' */
    int len = 0;

    fprintf(stderr, "Enter a sentence: ");

    while ((ch = getchar()) != EOF) {
        if (ch == '?' || ch == '.' || ch == '!' || ch == '\n' || len >= N-1) {
            break;
        }
        line[len++] = (char) ch;
    }
    line[len] = '\0';

    char *end = line + len;
    char *s = end;
    while (--s >= line) {
        if (isspace(*s)) {
            print_chars(s + 1, end - s - 1);
            putchar(*s);
            end = s;
        }
    }
    print_chars(line, end - line);
    if (ch != EOF) {
        putchar(ch);
    }
    return 0;
}


Calculating total sales from each member I am building a new system that is using some tables from an old system.

For each user on this new system, I need to go to the old system and total up their sales.  Currently it takes between 2-5 minutes to load.

public static List<DailyTeamGoal> GetListDailyTeamGoals(int teamId)
{
    string teamGoal = "";
    List<ProPit_User> lstProPit_User = new List<ProPit_User>();
    using (ProsPitEntities db = new ProsPitEntities())
    {
        // Find the team
        Team team = db.Teams.Where(x => x.teamID == teamId).FirstOrDefault();

        if (team != null)
        {
            // Grab team goal
            teamGoal = Convert.ToString(team.goal);
        }
        // Make a list of all users who are on the team
        lstProPit_User = db.ProPit_User.Where(x => x.teamID == teamId).ToList();
    }

    List<DailyTeamGoal> lstDailyTeamGoal = new List<DailyTeamGoal>();
    using (TEntities db = new TEntities())
    {
        //have to get every day of the month
        DateTime dt = DateTime.Now;
        int days = DateTime.DaysInMonth(dt.Year, dt.Month);
        decimal orderTotal = 0m;
        for (int day = 1; day <= days; day++)
        {
            // For every day in the month total the sales
            DailyTeamGoal dtg = new DailyTeamGoal();
            dtg.Date = day.ToString(); //dt.Month + "/" + day; + "/" + dt.Year;
            dtg.TeamGoal = teamGoal;
            decimal orderTotalRep = 0m;
            foreach (var propit_User in lstProPit_User)
            {
                DateTime dtStartDate = Convert.ToDateTime(dt.Month + "/" + day + "/" + dt.Year);
                DateTime dtEndDate = dtStartDate.AddDays(1);
                var lstorderTotalRep = (from o in db.Orders
                                    where o.DateCompleted >= dtStartDate
                                    where o.DateCompleted <= dtEndDate
                                    where (o.Status == 1 || o.Status == 2)
                                    where o.Kiosk != 0
                                    where o.SalesRepID == propit_User.SalesRepID
                                    orderby o.OrderTotal descending
                                    select o.OrderTotal).ToList();

                foreach (var item in lstorderTotalRep)
                {
                    //orderTotalRep =+ item;
                    orderTotalRep += item;
                }
            }

            orderTotal += orderTotalRep;
            dtg.DailyTotal = orderTotal;

            lstDailyTeamGoal.Add(dtg);
        }
    }

    return lstDailyTeamGoal;
}

The above code will use my site to find the teamID the logged in person is on.  It will then find all members who are on that team.  For each member it finds it will calculate the total sales and then spit me back a list.  Any way to speed this up?

I won't comment about the fact that this is a static method in what's possibly a static "helper" class and that it would be much prettier in a service class instance... but I just did.
The method is doing way too many things.

Thing one:
// Make a list of all users who are on the team

That's one method. But there's a twist: you not only need all users on the team, but also the teamGoal figure. I'd encapsulate that in its own class:
public class TeamGoalAndUsersResult // todo: rename this class
{
    public IEnumerable<ProPit_User> Users { get; private set; }
    public string Goal { get; private set; }

    public TeamGoalAndUsersResult(string goal, IEnumerable<ProPit_User> users)
    {
        Users = users;
        Goal = goal;
    }
}

And then you can write a method/function that will return that type:
private TeamGoalAndUsersResult GetTeamGoalAndUsers(int teamId)
{
    using (ProsPitEntities db = new ProsPitEntities())
    {
        var goal = string.Empty;
        var team = db.Teams.SingleOrDefault(x => x.teamID == teamId);
        if (team != null)
        {
            goal = Convert.ToString(team.goal);
        }

        var users = db.ProPit_User.Where(x => x.teamID == teamId).ToList();
        return new TeamGoalAndUsersResult(goal, users);
    }
}

Performance Issue:
            var lstorderTotalRep = (from o in db.Orders
                                where o.DateCompleted >= dtStartDate
                                where o.DateCompleted <= dtEndDate
                                where (o.Status == 1 || o.Status == 2)
                                where o.Kiosk != 0
                                where o.SalesRepID == propit_User.SalesRepID
                                orderby o.OrderTotal descending
                                select o.OrderTotal).ToList();

            foreach (var item in lstorderTotalRep)
            {
                //orderTotalRep =+ item;
                orderTotalRep += item;
            }

You're doing all the computation on the client. I'd take a wild guess and say that this is where your bottleneck is. Do you really need to select, sort and materialize all orders just to sum up OrderTotal?
var total = db.Orders.Where(o => o.DateCompleted >= dtStartDate
                              && o.DateCompleted <= dtEndDate
                              && (o.Status == 1 || o.Status == 2)
                              && o.Kiosk != 0
                              && o.SalesRepID == user.SalesRepID)
                     .Sum(o => o.OrderTotal);

orderTotal += total;
dtg.DailyTotal = orderTotal;
dailyTeamGoals.Add(dtg);

The above doesn't sort anything, has the same criterion and performs everything on the server side, without materializing every order in the way. Also I believe if the table has all the fields used in the WHERE, in an index (guts tell me "especially DateCompleted"), that would further help the server-side processing, but that's something you're probably better off finding out with a profiler.


Sorting an array of jobs by various criteria Below is my current code for sorting an array, "$jobs_list".

It's not very DRY and it's fugly. And there are several more similar case statements still to go in. How could I write it so my brain doesn't twitch to look at it?

switch ($sort) {
    case 'number':
        usort($jobs_list, function($a, $b){
            return ( (int) $a->number > (int) $b->number );
        });
        break;
    case 'client':
        usort($jobs_list, function($a, $b){
            return strcmp($a->client_contact_last_name, $b->client_contact_last_name);
        });
        break;
    case 'name':
        usort($jobs_list, function($a, $b){
            return strcmp($a->name, $b->name);
        });
        break;
    case 'status':
        usort($jobs_list, function($a, $b){
            return ( (int) $a->status > (int) $b->status );
        });
        break;
    case 'revenue':
        usort($jobs_list, function($a, $b){
            return ( (float) $a->base_revenue > (float) $b->base_revenue );
        });
        break;
    case 'profit':
        usort($jobs_list, function($a, $b){
            return ( (float) $a->profit > (float) $b->profit );
        });
        break;
    case 'margin':
        usort($jobs_list, function($a, $b){
            return ( (float) $a->margin > (float) $b->margin );
        });
        break;

    default:
        # do nothing...
}

First, it may make sense to make sure that the field you're checking in $sort, is the same as the propertyname of the object in $jobs_list.

Also, it looks like there's a lot of identical sort functions; the only difference being that the property you're checking is different.

You could just combine the switch statements that have an identical sorting strategy (revenue, status, profit, margin, etc..) and turn the field into a variable, such as:

switch ($sort) {
    case 'number':
    case 'status':
    case 'profit':
    case 'margin':
        usort($jobs_list, function($a, $b) use ($sort) {
            return ( (float) $a->$sort > (float) $b->$sort );
        });
        break;

}