Directory tree representation with double linked list var root = this;
root.node = function(path, type) {
  root.type = type;
  this.type = type;
   this.parents = [];
   this.children = [];
   var components = path.split('/');
   this.parents = parseParents(components);
   this.name = components[components.length - 1];
   this.addChild = function(path, type){
              if(this.type === 'file'){
                throw new Error('A file node cannot have any children');
              }
              this.children.push(new root.node(path, type));
  }

  function parseParents(components){

        var parents = [];
        if(components.length > 0){
           for(var i = (components.length - 1); i > 0; i--){
              if(components[i] !== "")
                if(parents.length === 0){
                  var currentParent = new node(components[i], 'dir');
                  currentParent.children.push(new node(components[0], root.type));
                  parents.push(currentParent);
                }
                else{
                  var newParent = new node(components[i], 'dir');
                  var curretnChild = new node(components[i - 1], 'dir');
                  currentParent.parents.push(newParent);
                  currentParent = newParent;
                }
           }
           return parents;
         }
         return [];

   }
   return this;
}

var a = new node('/usr/local/foo', 'dir');
a.addChild('test', 'file');
console.log(a);

2 way linked list, works great besides that the the 'leaf' items are not populated currently, since in the current structure it will result in an infinite loop.

You thoughts please.

Not sure this question is valid, but there is reviewable code:

node is a constructor, it should be named with an uppercase N
curretnChild is not properly spelled, and not used anywhere, drop it
Since you set parents to [], you might as well return parents when if(components.length > 0){ is not true
You should declare your vars on top in parseParents, the scope of newParent and currentParent is the whole function anyway
Your reliance on root.type is a bug in waiting, find a better way to figure out what the type should be.
addChild should be part of Node.prototype, it does not make sense for every instance of Node to have it's own version of this function
Your code in addChild does not take care of dupes, that should be addressed

For fun, I  wrote this, it creates a double linked list and takes most of my comments in to consideration, except for the last one ( does not take care of dupes ).

function Node( path , type )
{
  var parts = this.cleanPathParts( path.split("/") ),
      part  = parts.pop();
  //Set the basics
  this.name = part;
  this.type = type;
  //Do we still have parts left ?
  if( parts.length ){
    this.parent = new Node( parts.join('/'), this.DIR );
    this.parent.children = [ this ];
  }
}
//Ignore blanks and `..` strings
Node.prototype.cleanPathParts = function( parts )
{
  return parts.filter( function(part){
    return !!part && part != "..";
  });
};
//Get the ultimate parent, usefull for adding path children
Node.prototype.getRoot = function()
{
  var root = this;
  while( root.parent )
    root = root.parent;
  return root;
};
//Add a child, either a simple node or a path
Node.prototype.addChild = function( part , type )
{
  if( this.type != this.DIR ){
      throw new Error('A file node cannot have any children');
  }
  var node = (new Node( part, type )).getRoot();
  node.parent = this;
  this.children = this.children || [];
  this.children.push( node );
};
//Constants
Node.prototype.DIR  = 'dir';
Node.prototype.FILE = 'file';

var a = new Node('/usr/local/foo', 'dir');
a.addChild('1/2/test', 'file');
a.addChild('test', 'file');
console.log(a);


More efficient columnar transposition decryption Below, I have created a rudimentary script to decipher a columnar transposition. I am wondering if there is a more efficient way to determine the number of the rows in a column? Also, is there a way to build the resulting string without creating a matrix in the interim?

Overview

Quick refresher for columnar transpositions.

Method 1

Given a key of 3,

the text 'Attack at dawn' is transposed into three columns...

1   2   3
A   t   t
a   c   k
a   t   D
a   w   n

...joining each column; left->right, top->bottom; will result in the following text:

Aaaa tctw tkDn

Method 2

Given a key 'table',

the text 'Attack at dawn' is transposed into five columns (length of the key)...

t   a   b   l   e
A   t   t   a   c
k   a   t   D   a
w   n

...joining the columns, as described above, result in the following text:

tAkw atan btt laD eca

Code

Is there a better way to determine how many rows will be present for each column?

function decrypt(ciphertext, key) {
    var chars   = normalizeText(ciphertext);
    var len     = chars.length;
    var isStr   = typeof key === 'string';
    var cols    = isStr ? key.length : key;
    var rows    = Math.ceil(len / cols);
    var mod     = divMod(len, cols)[1];
    var matrix  = Matrix(rows, cols, '');
    var counter = 0;
    for (var col = 0; col < cols; col++) {
        var remainder = mod !== 0 && col >= mod ? 1 : 0;
        for (var row = 0; row < rows - remainder; row++) {
            matrix[row][col] = chars[counter++];
        }
    }
    return (function(plaintext, startIndex) {
        return plaintext.substr(startIndex);
    }(matrix.reduce(function(result, row) {
        return result + row.join('');
    }, ''), isStr ? cols : 0));
}

Demo

[ [ 'Aaaa tctw tkDn'       , 3       ],
  [ 'tAkw atan btt laD eca', 'table' ]
].forEach(function(cipher) {
    println(decrypt(cipher[0], cipher[1]));
});

function decrypt(ciphertext, key) {
    var chars   = normalizeText(ciphertext);
    var len     = chars.length;
    var isStr   = typeof key === 'string';
    var cols    = isStr ? key.length : key;
    var rows    = Math.ceil(len / cols);
    var mod     = divMod(len, cols)[1];
    var matrix  = Matrix(rows, cols, '');
    var counter = 0;
    for (var col = 0; col < cols; col++) {
        var remainder = mod !== 0 && col >= mod ? 1 : 0;
        for (var row = 0; row < rows - remainder; row++) {
            matrix[row][col] = chars[counter++];
        }
    }
    return (function(plaintext, startIndex) {
        return plaintext.substr(startIndex);
    }(matrix.reduce(function(result, row) {
        return result + row.join('');
    }, ''), isStr ? cols : 0));
}
function divMod(y, x) {
    return [~~(y / x), y % x];
}
function normalizeText(ciphertext) {
    return ciphertext.replace(/ +/g, "").split('');
}
function Matrix(rows, cols, defaultVal) {
    defaultVal = defaultVal !== undefined ? defaultVal : 0;
    var arr = [];
    for (var i = 0; i < rows; i++) {
        arr.push([]);
        arr[i].push(new Array(cols));
        for (var j = 0; j < cols; j++) {
            arr[i][j] = defaultVal;
        }
    }
    return arr;
}
function print(str, targetEl) {
    return (function (el) {
        return el.html(el.html() + (str || ''));
    }(targetEl || $('#out')));
}
function println(str, targetEl) {
    print((str || '') + '
', targetEl)
}

Allow me to provide a bit of a different approach. I know it does not exactly answer your questions, but I believe it makes them somewhat obsolete.

This is how I would do the decrypt:

function decrypt(sentence) {
    var result = '';
    var pos = 0;

    var words = sentence.split(' ');
    words = words.map(function(word) {
       return word.split('');
    });

    while(true) {
       char = words[pos].shift();
        if (! char) {
            break;
        }
        result += char;

        pos = pos > words.length -2 ? 0 : pos + 1;
    }

    return result;
}

For a working example: http://jsfiddle.net/35jyyv8g/1/

Note that I left the key out, since it is not really necessary with the spaces in the input.


Converting a string into an array and returning the values spelled in reverse I must create a function that accepts a string of names all separated by a comma.
Example: "Jack,Matthew,John,Sam"
And return the string of names in the same order but spelled in reverse.
Expected output: "kcaJ,wehttaM,nhoJ,maS"
I completed the function with an inner for loop but I would like to know if maybe running a single for loop and inside using a Linq function or something similar, would have been better?
public string Reverse(string s)
{
    var names = s.Split(",");
    string temp = "";

    for (int i = 0; i < names.Length; i++)
    {
        for (int j = names[i].Length - 1; j >= 0; j--)
        {
            temp += names[i].ElementAt(j);
        }
        temp += i == names.Length - 1 ? "" : ",";
    }

    return temp;
}

Anyway I found this problem very simple even though I tried taking my time on it, I found this solution almost immediately so I went with it. I am thinking maybe if I spent more time on it then I could have implemented this with some sort of Linq function to reverse the spelling of the names and removed the inner loop.
Really any suggestions for improving this small function is appreciated.

Good code, few suggestions.

Instead of having to check each and every time to make sure you can add a "," or not, add it for all the cases, use String.TrimEnd to remove the extra leading ","
            var names = s.Split(",");
            string temp = "";

            for (int i = 0; i < names.Length; i++)
            {
                for (int j = names[i].Length - 1; j >= 0; j--)
                {
                    temp += names[i].ElementAt(j);
                }
                temp += ",";
            }
            return temp.TrimEnd(',');

With that change, we have the ability to foreach loop to further improve the readability of the code.
            string temp = "";
            foreach (var name in s.Split(','))
            {
                for (int j = name.Length - 1; j >= 0; j--)
                {
                    temp += name.ElementAt(j);
                }
                temp += ",";
            }
            return temp.TrimEnd(',');

That's good. Although we can improve this further by using a better (faster and cleaner) method to reverse the individual names, and that is to use String.ToCharArray and Array.Reverse
The idea is to convert the string into a character array, use the inbuilt Reverse method to reverse it, add it to temp.
            string temp = "";
            foreach (var name in s.Split(','))
            {
                char[] NameArray = name.ToCharArray();
                Array.Reverse(NameArray);

                temp = temp + new string(NameArray) + ",";

            }
            return temp.TrimEnd(',');

You can also use StringBuilder which is slightly faster
            StringBuilder reversed = new StringBuilder();

            foreach (var word in str.Split(','))
            {
                char[] singlesentence = word.ToCharArray();
                Array.Reverse(singlesentence);
                reversed.Append(singlesentence);
                reversed.Append(",");
            }
            return reversed.ToString().TrimEnd(',');

Nitpick:

Prefer using a better name like reversed compared to temp, it tells more to the reader.

Use string.Empty and not ""

Benchmarks
Your original code
        static public string Reverse(string s)
        {
            var names = s.Split(",");
            string temp = "";

            for (int i = 0; i < names.Length; i++)
            {
                for (int j = names[i].Length - 1; j >= 0; j--)
                {
                    temp += names[i].ElementAt(j);
                }
                temp += i == names.Length - 1 ? "" : ",";
            }
            return temp;
        }

New
        static public string Reverse2(string s)
        {

            string reversed = string.Empty;
            foreach (var name in s.Split(','))
            {
                char[] NameArray = name.ToCharArray();
                Array.Reverse(NameArray);

                reversed += new string(NameArray) + ",";

            }
            return reversed.TrimEnd(',');
        }

New2
        static public string Reverse3(string str)
        {
            StringBuilder reversed = new StringBuilder();

            foreach (var word in str.Split(','))
            {
                char[] singlesentence = word.ToCharArray();
                Array.Reverse(singlesentence);
                reversed.Append(singlesentence);
                reversed.Append(",");
            }
            return reversed.ToString().TrimEnd(',');
        }

Note that I have static to use the function without an object reference.
I will use the following method to call the two functions multiple times, and benchmark it.
static void Perform(Func<string, string> Function, string names, int iterations, string type)
{

    var watch = System.Diagnostics.Stopwatch.StartNew();
    for (int i = 0;i < iterations;i++)
        Function(names);

    watch.Stop();

    var elapsed = watch.ElapsedMilliseconds;
    Console.WriteLine($"Time taken for {type}: {elapsed} ms");
}

        static void Main()
        {
            string names = "Jack,Dylan,Bobby,Peter";
            const int iterations = 1000000;

            Perform(Reverse,  names, iterations, "Original");
            Perform(Reverse2, names, iterations, "New");
            Perform(Reverse3,  names, iterations, "New2");

        }

Here are the results
/*
" Release " Configuration

 Iterations  |   Original  |   New     |   New2
-------------+-------------+-----------+--------
 10 000      |    29   ms  |    5   ms |   5   ms
-------------+-------------+-----------+--------
 100 000     |    226  ms  |    54  ms |   43  ms
-------------+-------------+-----------+--------
 1 000 000   |    1763 ms  |    442 ms |   405 ms

*/

Try it here


Avoiding use of an initialized variable that will either be changed or ignored Follow on from this question

This is a follow on question from one I asked a few days ago. I am making (or rather have made) a recursive method that takes a list of ArrayList objects as its parameters. There can be any number of them, but let's pretend there are five. They are numbered from 1 to 5, so the Array contains: [1 2 3 4 5].

The Array is then passed through a recursive method which returns only the numbers at odd indices in the Array, i.e. [2 4] (as they are at indices 1 and 3).

This is the code for the recursive method:

public static ArrayList<Integer> oddList (ArrayList<Integer> tList) {

    ArrayList<Integer> oddList = ListMethods.deepClone(tList);
    int tempStorage = -1;

    if (oddList.size() <= 0)
        return oddList;
    else
    {
        if (oddList.size()%2==0)
            tempStorage = oddList.remove(oddList.size()-1);

        oddList.remove(oddList.size()-1);
        oddList = ListMethods.oddList(oddList);

        if (tempStorage >= 0)
            oddList.add(tempStorage);
    }
    return oddList;
}

There are two things I am curious about:

Firstly, is there an alternative to that if statement at the very end, to check that tempStorage >= 0.
Secondly, and probably more importantly, should I care? In other words, is what I've done there a cheap, easy fix, or is that common coding practice? It just seems strange having an initialised variable that will always be either changed or ignored.

Any feedback would be fantastic, thanks (and to the guys who posted on my previous post, I'm still reviewing all the information - not allowed to use most of it in this assignment, but thanks anyway!)

I already explained in my other post, not to use this tempStoragevariable in this way. At least (compared to the other version) you do not change the meaning of it in this version.
If you rename it to something like removedItemFromEvenIndex it could be fine. If and really if the requirement is that all values inside the lists are >= 0.

If all values are allowed, I would suggest to stick to the solution with the boolean I suggested in the other post. Because if all values are allowed, You can not find any "check" value to see if you removed or something or not.

From where comes this problem? You try to do 2 things together with the tempStorage:

Transport the removed value
Status flag if you removed something or not

You can not address both things with the same Integer, because you already need everything from your Integer for the first part.

Rest is the same as for the other question.

To handle this one, same approach as other question:

What we want to do here? Add every second element to the new list, starting with the second.

So a simplified description could be:

function oddList(list)
  if list is empty or has only 1 element
    return empty list
  return new list(second element of list, oddList(all elements from the third to the end))

translate to algorithm in Java:

public static List<Integer> oddList(final List<Integer> list) {
    if (list.size() <= 1)
        return new ArrayList<>();

    final List<Integer> newList = new ArrayList<>(Arrays.asList(list.get(1)));
    newList.addAll(oddList(list.subList(2, list.size())));
    return newList;
}

If we take into account the other question, this could be a clever way (you should always try to reuse existing and working solutions):

public static List<Integer> oddList(final List<Integer> list) {
    final List<Integer> newList = new ArrayList<>(list); //we create a copy, otherwise list is modified for the caller
    newList.remove(0); //all indices shifted one to the left
    return even(newList);
}


Optimizing join statement I have this piece of code:

List<myItem> items = (from x in db.myItems
                        join y in db.tblParents on x.item_parent equals y.id
                        where !y.hidden
                        orderby x.name
                        select x).ToList();

And it is really slow. How can I optimize it? I need it to be faster. I have no idea how to do this.

I honestly don't see anything particularly wrong with that Linq query, but there are a number of other things you should consider:

Converting the query to a List<T> forces immediate evaluation of the query and requires the allocation of an single, contiguous portion of memory to store the results. If there are several thousand records, this can be fairly costly. If you later filter / re-order / group this list, it must be done in memory, which is usually much slower than allowing the database to filter / re-order / group the results. Generally, you should try to put off evaluating results until as late as possible.
Even if you're not doing any further processing on the list, it may not be necessary to actually store the result set as a List<T>. You really should only use a List<T> when you need to be able to add or remove items from the result set dynamically. If you only need to be able to access elements in the result set by index (particularly in O(n) time), I'd recommend using an array instead (with ToArray). If all you need to do is iterate through the result set one item at a time, I'd strongly recommend using an IEnumerable<T> instead (with AsEnumerable). This way, your application doesn't need to pull all items in at once, and it doesn't have to allocate a single, contiguous portion of memory to store them.
If you've done all you can to speed up the C# code, and it still runs slowly, you should look at optimizing the database—your code can't run any faster than the back-end database, after all. Perhaps you need to add an index to the item_parent column (and probably a foreign key as well).

If you've setup a proper navigation property you could write your query a bit more cleanly, although I doubt this would run any faster, since this will still translate to an INNER JOIN in SQL:

List<myItem> items =
    (from x in db.myItems
     where !x.tblParent.hidden
     orderby x.name
     select x)
    .ToList();

Or in fluent syntax:

List<myItem> items = db.myItems
    .Where(x => !x.tblParent.hidden)
    .OrderBy(x => x.name)
    .ToList();


Optimize bash script that concatenates output I am trying to optimize my script that loops through a folder and extracts only the part of a file name before a date and the header of the file and outputs it into a different file using a delimiter. I feel the script is robust and I want to refactor it. If there is also a better way, please tell me.

#!/bin/bash
# script variables
FOLDER=path/to/folder
LOG_FILE=path/to/logfile.csv

# Getting the pattern and  header of files from FOLDER
for file in `ls $FOLDER/*.csv`
do
    echo -n $(basename "$file") 2>&1 | sed -r 's/(.*)_[0-9]{8}_[0-9][0-9]-[0-9][0-9].[0-9][0-9].csv/\1/'  | tee -a $LOG_FILE
    echo -n "|" | tee -a $LOG_FILE
    cat $file | head -1 | tee -a $LOG_FILE

done #> $LOG_FILE

do ... done is a compound command; every subcommand shares the file descriptors; so teeing the the loop has the same effect as teeing each subcommand.
Two subsequent invocations of echo can be combined together.
cat $file is a dreaded UUOC.
A basename invocation can be avoided by changing directory to $FOLDER.
ls is absolutely unnecessary. The shell already globbed the *.csv.

Summing up,

    chdir "$FOLDER"
    for file in *.csv; do
        echo -n "$file" "|" | sed -r ...
        head -1 "$file"
    done | tee $LOGFILE

does the same job.


fractions data type -- when to reduce? Here is my Python implementation of a simple fractions class:

class frac:
    def __init__(self, a,b):
        self.a = a
        self.b = b

    def __add__(self, x):
        return frac(self.a*x.b + self.b*x.a, self.b*x.b)

    def __mul__(self, x):
        return frac(self.a*x.a, self.b*x.b)

    def __sub__(self, x):
        return frac(self.a*x.b - self.b*x.a, self.b*x.b)

    def __div__(self, x):
        return frac(self.a*x.b, self.b*x.a)

    def __repr__(self):
        return "%s/%s"%(self.a,self.b)

It does addition OK:

x = frac(1,3)
y = frac(1,2)
print x+ y

>> 5/6

However, it can miss the mark:

x = frac(1,3)
y = frac(2,3)
print x+ y

>> 9/9

It seems if I run this library enough, the numerators and denominators will get very large.  When should I start simplifying and where should I put that code?

There seems to be a fractions class already in Python, it this was instructive to put together.

My original goal was to allow for square roots of integers as well, but I will ask this in a separate question.

I would probably put it right in your __init__ method, which simplifies it every time a fraction is created (which includes after any operation).

I disagree with Jaime's answer that you should do the simplification as part of the operation, as you'll have to copy/paste that code for every new operation.

I’d write something like:

def gcd(x, y):
    while y:
        x, y = y, x % y
    return x

class Fraction(object):
    def __init__(self, numer, denom):
        cancel_factor = gcd(numer, denom)
        self.numer = numer / cancel_factor
        self.denom = denom / cancel_factor

A few other comments:

Per PEP 8 on class names, I'm using Fraction instead of frac.

Also from PEP 8, I would put spaces around your multiplication operators.
New style classes should inherit from object (as I've done above).
I think numer and denom are better attribute names than a and b.
The convention is to use other as the variable name in a method that takes two inputs, rather than x. For example:

def __add__(self, other):
    new_numer = self.numer * other.denom + self.denom * other.numer
    new_denom = self.denom * other.denom
    return Fraction(new_numer, new_denom)

I disagree with your use of __repr__. As a rule of thumb, __repr__ should be something you can eval() to get back to that object; __str__ is the human-readable object. See difference between __str__ and __repr__ for more.

Here's what I'd write:

def __repr__(self):
    return "Fraction(%s, %s)" % (self.numer, self.denom)

def __str__(self):
    return "%s / %s" % (self.numer, self.denom)


LeetCode: Greatest Common Divisor of Strings C# https://leetcode.com/problems/greatest-common-divisor-of-strings/

  For strings S and T, we say "T divides S" if and only if S = T + ... +
  T  (T concatenated with itself 1 or more times)

  Return the largest string X such that X divides str1 and X divides
  str2.

Example 1:

Input: str1 = "ABCABC", str2 = "ABC"
Output: "ABC"
Example 2:

Input: str1 = "ABABAB", str2 = "ABAB"
Output: "AB"
Example 3:

Input: str1 = "LEET", str2 = "CODE"
Output: ""

Note:

1 <= str1.length <= 1000
1 <= str2.length <= 1000
str1[i] and str2[i] are English uppercase letters.

I had 30 minutes to solve this.
please review this as a real interview.

what is a red flag, remember code will not be prefect.

using System;
using System.Text;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace StringQuestions
{
    /// <summary>
    /// https://leetcode.com/problems/greatest-common-divisor-of-strings/
    /// </summary>
    [TestClass]
    public class GreatestCommonDivisorOfStringsTest
    {
        [TestMethod]
        public void TestMethod1()
        {
            string str1 = "ABABAB";
            string str2 = "ABAB";
            string res = GCDOfStringsclass.GcdOfStrings(str1, str2);
            Assert.AreEqual("AB", res);
        }

        [TestMethod]
        public void TestMethod2()
        {
            string str1 = "ABCDEF";
            string str2 = "ABC";
            string res = GCDOfStringsclass.GcdOfStrings(str1, str2);
            Assert.AreEqual(string.Empty, res);
        }
    }

    public class GCDOfStringsclass
    {
        public static string GcdOfStrings(string str1, string str2)
        {
            int min = Math.Min(str1.Length, str2.Length);

            StringBuilder build = new StringBuilder();
            //build largest prefix
            for (int i = 0; i < min; i++)
            {
                if (str1[i] != str2[i])
                {
                    break;
                }

                build.Append(str1[i]);
            }

            // no common prefix
            string prefix = build.ToString();
            if (prefix.Length == 0)
            {
                return string.Empty;
            }

            while (str1.Length % prefix.Length != 0 || str2.Length % prefix.Length != 0)
            {
                prefix = prefix.Remove(prefix.Length - 1);
                if (prefix.Length == 0)
                {
                    return string.Empty;
                }
            }

            for (int i = 0; i < str1.Length / prefix.Length; i++)
            {
                for (int j = 0; j < prefix.Length; j++)
                {
                    if (str1[j + prefix.Length * i] != prefix[j])
                    {
                        return string.Empty;
                    }
                }
            }

            for (int i = 0; i < str2.Length / prefix.Length; i++)
            {
                for (int j = 0; j < prefix.Length; j++)
                {
                    if (str2[j + prefix.Length * i] != prefix[j])
                    {
                        return string.Empty;
                    }
                }
            }

            return prefix;
        }
    }
}

The first step in your method is to determine the longest common prefix. Here it would be sufficient to determine the length of the longest common prefix. Building the prefix string itself is not necessary at this point.

The next step is to decrease that length until it divides both string lengths. An explaining comment would be helpful here. We still don't need the prefix string itself, it suffices to decrease the previously determined prefix length.

Finally you determine if the prefix “divides” both strings. At this point we can build the actual prefix string. Again a short comment could be helpful. Instead of the inner loop I would compare substrings (unless that turns out to be too slow):

var prefix = str1.Substring(0, prefixLength);

// Verify that `str1` is a multiple of `prefix`:
for (int i = 1; i < str1.Length / prefixLength; i++)
{
    if (prefix != str1.Substring(i * prefixLength, prefixLength))
    {
        return string.Empty;
    }
}

// Same for `str2` ...

Note that the loop can start with i = 1 because we already now that both strings start with prefix. Since the same logic is applied to both strings I would also consider to move it to a separate IsMultipleOf function.

Performance: Consider the following example:

str1 = "AAA......A"  //  999 characters
str2 = "AAA......AA" // 1000 characters

Your method builds a prefix string of 999 characters first, and then removes successively all but one character, because 1 is the only length that divides both string lengths.

As mentioned above, this can be improved by updating the prefix length only, but one can still do better:

It is not difficult to see that the length of X must be exactly the greatest common divisor of the lengths of str1 and str2 – and that can be determined efficiently with the Euclidean algorithm.

The implementation would then look like this:

private static int gcd(int a, int b)
{
    // Your favorite implementation of the Euclidean algorithm...
}

public static string GcdOfStrings(string str1, string str2)
{
    var prefixLength = gcd(str1.Length, str2.Length);
    var prefix = str1.Substring(0, prefixLength);

    // Is it a common prefix?
    if (prefix != str2.Substring(0, prefixLength)) {
        return "";
    }

    // Verify that `str1` is a multiple of `prefix` ...
    // Verify that `str2` is a multiple of `prefix` ...

    return prefix;
}


Search arrays for values I implemented a search function like this:

private static void GetResults(ref ObservableCollection<string> resultTitles, ref string[] query, ref ObservableCollection<int> weight)
{
    int position = -1;

    foreach (string[] r in SearchKeys.Keys)
    {
        position++;
        foreach (string s in query)
        {
            int m = r.Length/2;
            int min = r[m][0] < s[0] ? m : 0;
            int max = r[m][0] <= s[0] ? r.Length : m+1;

            for (int i = min; i < max; i++)
            {
                weight.Add(0);

                if (r[i] == s)
                {
                    if (weight[position] == 0)
                    {
                        resultTitles.Add(r[0]);
                    }
                    weight[position]++;

                }   // end if
            }   // end foreach(s)
        }   // end foreach(t)
    }   // end foreach(r)
}

In the main program, the use inputs a query, the program splits the query at the spaces, and passes the query and two empty ObservableCollections to this function.

SearchKeys.Keys is implemented like this:

public static string[][] Keys = { Array1, Array2, Array3 };

The arrays are implemented like this:

private static string[] Array1 = { "Title", "val1", "val2", "val3", "val4", "val5", "val6" };

As always, all comments are welcome; particularly, I am interested in which ways could this code's performance be improved.  The data in SearchKeys is sorted, of course.

This is a previous version of my code, which may also be of interest.  This version should run about twice as slow as the above, according to my estimates:

private void getResults(ref ObservableCollection<string> tmp, ref string[] query, ref ObservableCollection<int> weight)
{
    int position = -1;

    foreach (string[] r in SearchKeys.Keys)
    {
        position++;
        foreach (string t in r)
        {
            weight.Add(0);

            foreach (string s in query)
            {
                if (t == s)
                {
                    if (weight[position] == 0)
                    {
                        tmp.Add(r[0]);
                    }
                    weight[position]++;

                }   // end if
            }   // end foreach(s)
        }   // end foreach(t)
    }   // end foreach(r)
}

This is how I call this function:

static void Main(string[] args)
{
    ObservableCollection<int> weight = new ObservableCollection<int>();
    ObservableCollection<string> resultTitles = new ObservableCollection<string>();
    string[] query = Console.ReadLine().Split(' ');

    GetResults(ref resultTitles, ref query, ref weight);
    StableSort(ref resultTitles, ref weight);

    foreach (string s in resultTitles)
        Console.WriteLine(resultTitles.IndexOf(s)+": "+s);
}

This is StableSort (see Stable Sort in C#):

static void StableSort(ref ObservableCollection<string> values, ref ObservableCollection<int> weights)
{
    while(weights.Contains(0))
    {
        weights.Remove(0);
    }

    if (values == null) { throw new ArgumentNullException("values"); }
    if (weights == null) { throw new ArgumentNullException("weights"); }

    if (values.Count != weights.Count) { throw new ArgumentOutOfRangeException("collections count not equal", (Exception)null); }

    ObservableCollection<string> localValues = new ObservableCollection<string>();
    ObservableCollection<int> localWeights = new ObservableCollection<int>();

    int index = -1;
    var weightsWithIndex = weights.Select(p => new { Value = p, Index = ++index }).OrderByDescending(p => p.Value);

    foreach (var w in weightsWithIndex)
    {
        localWeights.Add(w.Value);
        localValues.Add(values[w.Index]);
    }

    values = localValues;
    weights = localWeights;
}

You're doing a good job at indenting your code, well done! But then why would you need these?

            }   // end if
        }   // end foreach(s)
    }   // end foreach(t)
}   // end foreach(r)

Now, the signature:

private static void GetResults(ref ObservableCollection<string> resultTitles, ref string[] query, ref ObservableCollection<int> weight)

"void GetResults"... "void Get". And then the two ref parameters...

  and passes the query and two empty ObservableCollections to this function.

query doesn't need to be passed by reference, doesn't need to be specifically an array, and if I had to use ref parameters I'd put it first in the signature:

private static void GetResults(IEnumerable<string> query, ref ObservableCollection<string> resultTitles, ref ObservableCollection<int> weight)

Now, the method doesn't care that our collections are observable - it only needs to know it's some kind of ICollection<T>... but there's a smell there nonetheless: passing ref parameters instead of returning a collection, doesn't feel quite natural.

I believe, because it's a private method, that whatever you're passing in as weight could be promoted to a private field if it's used anywhere else in the class, or moved to the GetResults scope where it's used.


Python file overwriting script I am fairly new to Python and I'm trying to make a script that does three basic things: prints the contents of a file, overwrites it with your new lines, and prints the new file. I rewrote the script with functions and was looking for critiques. Is this a good use of functions? What would you do different?

print("Type the name of the file for overwriting.")
file = input(">")
x = open(file)
print("The contents are currently: \n", x.read())
print("Enter in the new lines")
l1 = input(">")
l2 = input(">")
l3 = input(">")
x.close()

y = open(file, 'w')

print("Entering new lines.")

y.write(l1 + '\n' + l2 + '\n' + l3)
y.close()

z = open(file).read()
print("The new contents are:")
print(z)

Script with functions:

def open_and_read(x):
    print(open(x).read())

def nl(x):
    z.write(x + '\n')

temp = ">"
print("Type the name of the file for overwriting.")
file = input(temp)
print(f"The current content of {file} is:")
open_and_read(file)

z = open(file, 'w')

print("Enter in three new lines of text:")
l1 = input(temp)
l2 = input(temp)
l3 = input(temp)

nl(l1)
nl(l2)
nl(l3)

z.close()

print("The new contents are:")
open_and_read(file)

First of all, you should definitely switch to using with context manager which is gonna safely and automatically close the opened file handler.

Also, you need to improve your variable names, l1, l2, l3 or z are not descriptive.

And, you may use .writelines() to write a list of strings each of them on a separate line.

In the end, you might get something along these lines:

print("Type the name of the file for overwriting.")
file_name = input(">")

with open(file_name) as input_file:
    print("The contents are currently: \n", input_file.read())
    print("Enter in the new lines")

lines = [input(">") for _ in range(3)]

print("Entering new lines.")
with open(file_name, 'w') as output_file:
    output_file.writelines(lines)

with open(file_name) as input_file:
    print("The new contents are:")
    print(input_file.read())


Project Euler #1: Multiples of 3 and 5 Challenge Description:

  If we list all the natural numbers below 10 that are multiples of 3 or 5, we get 3, 5, 6 and 9. The sum of these multiples is 23.

  Find the sum of all the multiples of 3 or 5 below 1000.
  Source

I want to receive advice on my code.

total_sum = 0
for i in range(1000):
    if (i%3 == 0 or i%5 == 0):
        total_sum = total_sum+i
print total_sum

You don't need iteration at all for this problem.

Consider; the sum of all numbers from 1 to n is equal to n*(n+1)/2.  Also the sum of all numbers less than n that divides d equals d times the sum of all numbers less than n/d.

So the sum of all numbers less than 1000 that divides 3 is

3*floor(999/3)*(floor(999/3)+1)/2

Likewise the sum of all numbers less than 1000 that divides 5 is

5*floor(999/5)*(floor(999/5)+1)/2

Adding the two numbers would overcount though.  Since the numbers that divides both 3 and 5 would get counted twice.  The numbers that divides both 3 and 5 is precisely the numbers that divides 3*5/gcd(3,5)=15/1=15.

The sum of all numbers less than 1000 that divides 15 is

15*floor(999/15)*(floor(999/15)+1)/2

So the final result is that the sum of all numbers less than 1000 that divides either 3 or 5 equals:

  3 * (floor(999/3)  *  (floor(999/3)+1))/2
+ 5 * (floor(999/5)  *  (floor(999/5)+1))/2
-15 * (floor(999/15) * (floor(999/15)+1))/2


Creating String from a bitmask In my Android app I store days of week in an int variable by using bit masking.

I'm using the following SparseIntArray to get the day flags for the corresponding days:

public static final SparseIntArray DAY_FLAGS = new SparseIntArray(7);
static {
    DAYS.put(Calendar.MONDAY, 1);
    DAYS.put(Calendar.TUESDAY, 2);
    DAYS.put(Calendar.WEDNESDAY, 4);
    DAYS.put(Calendar.THURSDAY, 8);
    DAYS.put(Calendar.FRIDAY, 16);
    DAYS.put(Calendar.SATURDAY, 32);
    DAYS.put(Calendar.SUNDAY, 64);
}

I'd like to be able to create String from a mask, for example:

  int value - String

  1 - "Mon"

  42 - "Tue Thu Sat"

  127 - "Mon Tue Wed Thu Fri Sat Sun"

For this I'm using the following method:

public static String maskToString(Context context, int mask, int firstDayOfWeek) {
    List<Integer> dayList = new ArrayList<>(7);

    // The idea is to iterate through the SparseIntArray and check
    // if the bit in the mask is set for the particular day.
    // If it is, add that day to the dayList.
    for (int i = 0; i < DAY_FLAGS.size(); i++) {
        if (isFlagSet(DAY_FLAGS.valueAt(i), mask)) {
            dayList.add(DAY_FLAGS.keyAt(i));
        }
    }

    // Sort the days by taking the first day of the week into account
    Collections.sort(dayList, new DayComparator(firstDayOfWeek));

    // This List will contain the individual Strings for the days
    List<String> dayStringList = new ArrayList<>(dayList.size());

    // dayToString() simply returns a String resource for a given day
    for (Integer day : dayList) {
        dayStringList.add(dayToString(context, day));
    }

    // join the individual Strings into a single String
    return TextUtils.join(" ", dayStringList);
}

The isFlagSet() method simply checks if a given flag is set in a given mask:

private static boolean isFlagSet(int flag, int mask) {
    return ((mask & flag) == flag);
}

My maskToString() method seems clumsy (mostly in terms of unnecessary(?) object creation). I believe it can be optimized, I just don't know how.

(I can use only Java 7 features, no Java 8)

I'd appreciate any advice.

EDIT:

One thing I came up with regarding optimization is to create a static final DayComparator instance and pass the firstDayOfWeek variable in a setter (instead of the constructor):

private static final DayComparator DAY_COMPARATOR = new DayComparator();

public static String maskToString(Context context, int mask, int firstDayOfWeek) {
    List<Integer> dayList = new ArrayList<>(7);

    for (int i = 0; i < DAY_FLAGS.size(); i++) {
        if (isFlagSet(DAY_FLAGS.valueAt(i), mask)) {
            dayList.add(DAY_FLAGS.keyAt(i));
        }
    }

    // so we do not create a new object here every time the method is called
    DAY_COMPARATOR.setFirstDayOfWeek(firstDayOfWeek);
    Collections.sort(days, DAY_COMPARATOR);

    List<String> dayStringList = new ArrayList<>(dayList.size());

    for (Integer day : dayList) {
        dayStringList.add(dayToString(context, day));
    }

    return TextUtils.join(" ", dayStringList);
}

What do you think? Any other suggestions?

Current approach

You can determine the exact count of days in the mask beforehand -> you can use a better initial size than 7 for the array list (or directly an array as the size is fixed).
You don't have to sort the list (and thus don't need the comparator), rotating based on the firstDayOfTheWeek is sufficient as the order of the days does not change.
While this is very ugly: You could replace the new ArrayList<>(dayList.size()) with (List<String>)(List<?>)dayList.

Regarding your edit: This seems like a very bad idea if you ever want to make your app multithreaded.

Alternative implementation

You can avoid rotating the resulting list if you start interpreting the mask at the provided firstDayOfWeek instead of Monday.

public static final int MONDAY    = 0x01;
public static final int TUESDAY   = 0x02;
public static final int WEDNESDAY = 0x04;
public static final int THURSDAY  = 0x08;
public static final int FRIDAY    = 0x10;
public static final int SATURDAY  = 0x20;
public static final int SUNDAY    = 0x40;

public static String maskToString(int mask, int firstDayOfWeek) {
    assert (mask & ~127 | firstDayOfWeek & ~127
            | firstDayOfWeek & firstDayOfWeek - 1) == 0;
    String[] values = new String[bitCount(mask)];
    int i = 0, d = firstDayOfWeek;
    do {
        if ((mask & d) != 0)
            values[i++] = dayToString(d);
    } while ((d = d << 1 & 127 | d >> 6) != firstDayOfWeek);
    return Strings.join(" ", values, true);
}

Which gives the following result:

maskToString(MONDAY, MONDAY);                              // Mon
maskToString(TUESDAY | THURSDAY | SATURDAY, MONDAY);       // Tue Thu Sat
maskToString(TUESDAY | THURSDAY | SATURDAY, SUNDAY);       // Tue Thu Sat
maskToString(TUESDAY | THURSDAY | SUNDAY, MONDAY);         // Tue Thu Sun
maskToString(TUESDAY | THURSDAY | SUNDAY, SUNDAY);         // Sun Tue Thu
maskToString(127, MONDAY);      // Mon Tue Wed Thu Fri Sat Sun
maskToString(127, SUNDAY);      // Sun Mon Tue Wed Thu Fri Sat
maskToString(127, WEDNESDAY);   // Wed Thu Fri Sat Sun Mon Tue


Coffee machine that adds condiments dynamically I want to make a coffee machine class which can produce coffee with condiments I like, and the price of the final coffee can be easily calculated.

I chose decorator pattern in my design. I've tried making the fields private to follow the principle of making scope as small as possible, but when I implemented the concrete decorator, the getName() caused trouble, I can not use getName() of the beverage in getName() of Milk(ConcreteDecorator). So finally I chose protected.

I've considered this post about using private over protected, but I don't know which one should be better in my implementation. This is a small program but I need some advice about the big picture which I should keep in mind.

So, any advice about any layer of the design of my code is appreciated.

public abstract class Beverage {
    protected final String name;

    public Beverage(String name) {
        this.name = name;
    }

    public String getName() {
        return name;
    }

    public abstract Double getCost();
}

public class HouseBlend extends Beverage {
    public HouseBlend() {
        super("HouseBlend");
    }

    @Override
    public Double getCost() {
        return 1.5;
    }
}

public abstract class Dairy extends Beverage {
    protected final Beverage beverage;

    public Dairy(String name, Beverage beverage) {
        super(name);
        this.beverage = beverage;
    }

    public Beverage getBeverage() {
        return beverage;
    }

    public abstract String getName();
}

public class Milk extends Dairy {
    public Milk(Beverage beverage) {
        super("Milk", beverage);
    }

    @Override
    public String getName() {
        return name + "-flavored " + beverage.getName();
    }

    @Override
    public Double getCost() {
        return 1.0 + beverage.getCost();
    }
}

public class Butter extends Dairy {
    public Butter(Beverage beverage) {
        super("Butter", beverage);
    }

    @Override
    public String getName() {
        return name + "-above " + beverage.getName();
    }

    @Override
    public Double getCost() {
        return 0.5 + beverage.getCost();
    }
}

public abstract class Sweet extends Beverage {
    protected final Beverage beverage;

    public Sweet(String name, Beverage beverage) {
        super(name);
        this.beverage = beverage;
    }

    public Beverage getBeverage() {
        return beverage;
    }

    public abstract String getName();
}

public class Sugar extends Sweet {

    public Sugar(Beverage beverage) {
        super("Sugar", beverage);
    }

    @Override
    public String getName() {
        return "Strong-" + name + "ed " + beverage.getName();
    }

    @Override
    public Double getCost() {
        return 0.2 + beverage.getCost();
    }
}

UML:

I would remove the name field entirely, and instead inline it into the getName() method.
That way you can implement the getName() similar to how you implemented getCost().

Concretely this means your getName() method from Beverage becomes abstract just like the other abstract classes.

public abstract String getName();

The base beverage (HouseBlend) then implements the method as:

public String getName() {
    return "HouseBlend";
}

And similarly the decorator classes implement it as such:

public String getName() {
    return "Milk-flavored " + beverage.getName();
}

You can fill in the implementation for the other decorators.


Put ages within a supplied range into a HashMap I'm looking for general considerations to make for the next time I'm writing code. I'm wondering if in this case it's better to have another auxiliary storage (another map?) in this case (and how to do so?). I haven't yet learned about complexity, so I'm not sure if I'm completing that part of the question (running in O(N log N) time).

Write a method byAge that accepts three parameters: 1) a Map where each key is a person's name (a string) and the associated value is that person's age (an integer); 2) an integer for a minimum age; and 3) an integer for a maximum age. Your method should return a new Map with information about people with ages between the min and max, inclusive.

In your result map, each key is an integer age, and the value for that key is a string with the names of all people at that age, separated by "and" if there is more than one person of that age. Your result map's pairs should be sorted in ascending order by age (keys). Include only ages between the min and max inclusive, where there is at least one person of that age in the original map. If the map passed is empty, or if there are no people in the map between the min/max ages, return an empty map.

For example, if a Map named ages stores the following key=value pairs:

{Paul=28, David=20, Janette=18, Marty=35, Stuart=98, Jessica=35, Helene=40,
 Allison=18, Sara=15, Grace=25, Zack=20, Galen=15, Erik=20, Tyler=6, Benson=48}

The call of byAge(ages, 16, 25) should return the following map (the contents can be in any order):

{18=Janette and Allison, 20=David and Zack and Erik, 25=Grace}

For the same map, the call of byAge(ages, 20, 40) should return the following map:

{20=David and Zack and Erik, 25=Grace, 28=Paul, 35=Marty and Jessica, 40=Helene}

Obey the following restrictions in your solution:

You will need to construct a map to store your results, but you may not use any other structures (arrays, lists, etc.) as auxiliary storage. (You can have as many simple variables as you like.)
You should not modify the contents of the map passed to your method.
Your solution should run in no worse than O(N log N) time, where N is the number of pairs in the map.

public Map<Integer, String> byAge(Map<String, Integer> namesAges, int min, int max) {
    Map<Integer, String> range = new HashMap<Integer, String>();
    for (String name : namesAges.keySet()) {
        int age = namesAges.get(name);
        if (range.containsKey(age)) {
            range.put(age, range.get(age) + " and " + name);
        } else if (min <= age && age <= max) {
            range.put(age, name);
        }
    }
    return range;
}

range.put(age, range.get(age) + " and " + name);

This is expensive.

Specifically, string construction that is done over and over via creating longer and longer strings. You're basically building the same string over and over and over. And strings are basically arrays of char, so that's a lot of copying.

And there's nothing you can do about it because of this rule:

  You will need to construct a map to store your results, but you may not use any other structures (arrays, lists, etc.) as auxiliary storage. (You can have as many simple variables as you like.)

If you were to ignore this one rule for a moment, what you could do is make a Map<Integer, List<String>>. And then put the names of people in the list. Then, when you're done, as per this stackoverflow answer... https://stackoverflow.com/a/22577565/540837

You can use a stream and Collectors.joining.

But you can't, because you're not allowed any other lists or arrays or whatnot.

I'd personally consider the problem unsolvable because the strings could get very long and become a larger part of the problem than the actual pairs would.

What I'd change about your code is this bit:

for (String name : namesAges.keySet()) {
    int age = namesAges.get(name);

You're first getting the keys, and then you're using the keys to get the value. Maps have .entrySet to get a set of entries, and then you can have the key and the value in one go.

Also, "Your result map's pairs should be sorted in ascending order by age (keys)." sorting already takes O(N log N) time, so basically your instructors are expecting that you'll find a way to get N log N string concatenation which doesn't exist when you're not allowed to create arrays.


Make file paths relative to the configuration file path I have a configuration file which holds several file paths.

If these paths are relative, I want them to be relative to the path of the configuration file (and not to the path of the user working directory ("user.dir")).

Since it's not possible to change the working directory with Java, I've written this method:

public static File newFile(String parent, String child) {
  File file = new File(child);

  if(file.isAbsolute()) {
    return file;
  }

  return new File(parent, child);
}

The folder of my config file would be parent here.

Is there a better solution than this?

A very raw and simple approach.
 I believe you would be loading the properties from the configuration file.

Properties prop = new Properties();
prop.load(new FileInputStream("somepath/config.properties"));

Hence I assume you know the path of your config file.

Now simple iterate over your properties and set them again.

for(String name : getProperty.getNames()) {
   String propValue = prop.getProperty(name);
   //TODO if(propValue == relative)
   setProperty(name, somepath + propValue);
}


Timed requests and parsing JSON in Python I wrote this tiny script to pull the JSON feed from the CitiBike website:

import requests
import time

def executeCiti():
    r = requests.get("http://citibikenyc.com/stations/json")
    print r.json()

    time.sleep(62)

while True:
    executeCiti()

exit()

Then I just run the script in terminal and output it to a .txt file like so: python citi_bike.py > output.txt

The end goal of what I would like by the end of this exercise is well formatted JSON data (with only select few of the pairs from each request), separated by each request.  I want it to be manageable so I can create visualizations from it.

Is this an okay way to start what I'm trying to accomplish?
Is there a better way to begin, so that the data came out like I want it as my end goal? In regards to this question, I feel like I have already started taking a very roundabout way to getting to a cleaned up data set, even though this is only the first step.

While this is a good start, there are several things that should be noted.

  r = requests.get("http://citibikenyc.com/stations/json")

This is code snippet is network-based, and so errors may occur. You will want to handle these errors in some way, with a try-except block like so:

try:
    r = requests.get("http://citibikenyc.com/stations/json")
except ConnectionError:
    pass # handle the error
except TimeoutError:
    pass # handle the error

and so on, as per the requests documentation.

Additionally, do not sleep and do in the same function. These are two responsibilities and so should be separated as per the Single Responsibility Principle.

I would suggest adding the sleep(62) to the while True: block.

Also, there is no point to this:

  exit()

as it will never be executed. This leads me to my next point, you should probably do the file writing in Python instead of the command line, so you can open the file, append some data, and then close it to make sure it is safe between network errors and power outages.


Java user roles modeling I have the following code where I try to manage user roles in teams:

Teams have entries, an entry has a creator. This creator is member of a team and has a specified role. Moderators may delete all entries, users may only delete their own entries. Here is how I implemented this:

public class TeamMember implements TeamRole {
    public String rank = "rank: Member";
    @Override
    public boolean deleteEntry(AbstractUser requester, Team team, Entry entry) throws IllegalAccessException {
        if(requester.getUid().equals(entry.getCreator().getUid())) {
            team.delete(entry);
            System.out.println("Member "  + requester.getName() +  " deleted own entry: " + entry.toString() + "from " + team.toString());
            return true;
        }
            throw new IllegalAccessException("Team members can only delete their own entries!");
    }

Moderator:

public class TeamModerator implements TeamRole {
    public String rank = "rank: Moderator";
    @Override
    public boolean deleteEntry(AbstractUser requester, Team team, Entry entry) throws IllegalAccessException {
        team.delete(entry);
        System.out.println("Moderator " +  requester.getName() + " deleted entry: "  + entry.toString() +  "from " + team.toString());
        return true;
    }

Team:

public class Team {
    private List<Entry> entries;
    private Map<AbstractUser, TeamRole> memberPerms;
    private String id;
    private String name;

public TeamRole findPermissionsForUser(AbstractUser user) {
    return memberPerms.get(user);
}
}

And the method that handles the request:

@RequestMapping(value = "/rest/entries/delete/userid/{userId}/teamid/{teamId}/entryid/{entryId}")
    public ResponseEntity<Team> deleteEntry(@PathVariable String userId, @PathVariable String teamId, @PathVariable String entryId) {
        Optional<Team> team = Repository.findTeamById(teamId);
        Optional<AbstractUser> user = Repository.findUserById(userId);
        Optional<Entry> entry = Repository.findEntryById(teamId, entryId);
        if (!(team.isPresent())) {
            return new ResponseEntity("team not found", HttpStatus.BAD_REQUEST);
        }
        if (!(user.isPresent())) {
            return new ResponseEntity("user not found", HttpStatus.BAD_REQUEST);
        }
        if (!(entry.isPresent())) {
            return new ResponseEntity("entry not found", HttpStatus.BAD_REQUEST);
        }
        return EntryService.deleteEntry(user.get(), team.get(), entry.get());

    }

The service that does the delete:

public class EntryService {
    public static ResponseEntity<Team> deleteEntry(AbstractUser requester, Team team, Entry e) {
        try {
            team.findPermissionsForUser(requester).deleteEntry(requester, team, e);
            return new ResponseEntity(team, HttpStatus.OK);
        } catch (IllegalAccessException e1) {
            e1.printStackTrace();
            return new ResponseEntity(e1.getMessage(), HttpStatus.INTERNAL_SERVER_ERROR);
        }
    }
}

This is just a MVCE which does not contain user authentication, it trustst that the passed userid is correct.
I think that this is not a good implementation, how would you model this?

I think that implementations of TeamRole should not know how to do things.

Based on the name part: ...Role it should only know if a certain user can perform a certain action on a particular resource.

I'm expecting the TeamRole to have a method like:

public interface TeamRole {
    boolean canDelete(Entry entry);
}

And then change the TeamMember:

public class TeamMember implements TeamRole {
    // need the user to be a part of the class here
    AbstractUser user;
    public TeamMember(AbstractUser user) {
        this.user = user;
    }

    @Override
    public boolean canDelete(Entry entry) {
        if (entry == null) return false;
        return user.getUid().equals(entry.getCreator().getUid());
    }
}

You can have an alternative with a static method in TeamMember, so you can avoid the instantiation of the class, and just pass the user (requester) as a parameter:

public interface TeamRole {
    // Java 8+
    static boolean canDelete(AbstractUser requester, Entry entry);
}

So your service method will become:

public static ResponseEntity<Team> deleteEntry(AbstractUser requester, Team team, Entry e) {
    TeamRole role = team.findPermissionsForUser(requester);
    if (role == null)
         return new ResponseEntity("no role found...", HttpStatus.INTERNAL_SERVER_ERROR);

    if (role.canDelete(entry)) {
        try {
            team.delete(e);
            return new ResponseEntity(team, HttpStatus.OK);
        } catch (Exception error) {
            // here you have an actual error to report to
            // the user, as something that should be done
            // failed.
            return new ResponseEntity(error.getMessage(), HttpStatus.INTERNAL_SERVER_ERROR);
        }
    }
    // So you can say to the user that nothing is wrong
    // with the request there is just a permission
    // issue here.
    return new ResponseEntity("user cannot delete entity", HttpStatus.METHOD_NOT_ALLOWED);
}

Profiling users is always an hard job, and each implementation should deal with the system requirements.

A good design should try to separate as much as possible the authorization part of the logic from the business part.

Authorization, like authentication and accounting should be aspect that can be changed in you system without adding any issue on the business logic.

Please have a look at Spring Security.

I don't mean you should use it, but I think could be a good inspiration on how things could be done cleaner.

Now a couple of things I noticed on your code.

The deleteEnytry method of TeamRole return a boolean, but this is always true!

So why returning such a value?

In this case just put void as the return value.

Have void as a return value from command method is considered a best practice. Of course is not mandatory and don't cover all the case, it is just usual that a coomand should not return something.

In your EntryService you have a NullPointerException:

public class EntryService {
    public static ResponseEntity<Team> deleteEntry(AbstractUser requester, Team team, Entry e) {
        try {
            // NullPointer here!
            team.findPermissionsForUser(requester)
                .deleteEntry(requester, team, e);
            return new ResponseEntity(team, HttpStatus.OK);
        } catch (IllegalAccessException e1) {
            e1.printStackTrace();
            return new ResponseEntity(e1.getMessage(), HttpStatus.INTERNAL_SERVER_ERROR);
        }
    }
}

the team.findPermissionsForUser(requester) didn't return always a value, as it returns just the value found in the Map, that is an actual TeamRole or null if no entry was found in the map.

And as you catch just the IllegalAccessException you will got an error that could be complicated to debug.


Is Lock necessary to make operation thread safe for this scenario I have the following method that is responsible for retrieving an Item from the database. There will be multiple threads instantiating this class, so I want to make sure that the operation will be thread-safe. As a result, I have added a lock around it, however since class only contains instance variables (and no static variables), is adding a lock necessary for this scenario?

using System;
using System.Collections.Generic;
using System.Data;
using System.Data.SqlClient;

namespace My.Library
{
    public class MyDataProvider : IMyDataProvider
    {
        private object sync = new object();
        private string connectionString;

        public MyDataProvider(string connString)
        {
            connectionString = connString;
        }
        public Item GetItem(Guid itemId)
        {
            lock (sync)
            {
                Item item = new Item();
                string sqlQuery = @"SELECT ItemId, ItemName FROM Item WHERE ItemId = @itemId";

                using SqlConnection connection = new SqlConnection(connectionString);
                SqlCommand command = new SqlCommand(sqlQuery, connection);
                command.Parameters.AddWithValue("@itemId", itemId);
                connection.Open();

                SqlDataReader reader = command.ExecuteReader();

                while (reader.Read())
                {
                    item.ItemId = reader["ItemId"] == DBNull.Value ? Guid.Empty : (Guid)reader["ItemId"];
                    item.ItemName = reader["ItemName"] == DBNull.Value ? null : reader["ItemName"];
                }

                return item;
            }
        }
    }
}

According to information you provided it's not necessary to wrap it with lock.
Let's consider situation you described. You wrote:

There will be multiple threads instantiating this class

So there will be one instance for each thread. There's no synchronisation needed, because only one thread will be accessing your class at a time.
Other scenarios:

Single instance of MyDataProvider class is shared between multiple threads. What does it mean? GetItem() method could be invoked by multiple threads at the same time. In that case you'll need synchronization IF your class have any state that (private field for example) could be accessed and modified from GetItem() method. You don't have such situation, so synchronization is not needed. Also you get connection from '[POOL]' each time anyone is invoking GetItem() instance method.
Single instance of MyDataProvider class is shared between multiple threads AND sqlconnection is a state in it. Then you would need to synchronize it.

Your implementation should be fine.


Rock-Paper-Scissors game for learning purposes I just want your opinions about my code. How could I improve it? Is there a better method to do it?

from sys import exit
from random import randint

print "Welcome to the PIN Game 1!"
print "Tell me your name"

user = raw_input('> ')

#asks the user for an entry and prints what he has chosen
def start():
    print "%s, choose one of the options: " % user

    print """
    1. Rock
    2. Paper
    3. Scissors
    """

    choice = 0
    cpu_choice = randint(1,3)

    while choice <= 0 or choice >= 4:

        choice = int(raw_input('> '))
        if choice == 1:
            print "You chose ROCK!"
        elif choice == 2:
            print "You chose PAPER"
        elif choice == 3:
            print "You chose SCISSORS"
        else:
            print "What is your problem? Try again, this time choose a number between 1 and 3!"
    cpu_decision(cpu_choice)
    result(choice, cpu_choice)

#random cpu input
def cpu_decision(cpu):
    if cpu == 1:
        print "Your opponent chose ROCK!"
    elif cpu == 2:
        print "Your opponent chose PAPER!"
    else:
        print "Your opponent chose SCISSORS"

#if user loses
def lose():
    print "Sorry, you lost the match"

#if user wins
def win():
    print "Congratulations! You beat your opponent!!"

#prints the result
def result(user, comp):
    if user == comp:
        print "Gee! You tied!"
    elif user == 1 and comp == 2:
        lose()
    elif user == 1 and comp == 3:
        win()
    elif user == 2 and comp == 1:
        win()
    elif user == 2 and comp == 3:
        lose()
    elif user == 3 and comp == 1:
        lose()
    elif user == 3 and comp == 2:
        win()

    again()

#ask if the user wants to play again
def again():
    print """
    Wanna play again?
    1 for YES
    Anything else to quit
    """

    again1 = int(raw_input('> '))

    if again1 == 1:
        start()
    else:
        exit()

start()

Choice representation

You're using numbers to represent choices (rock/paper/scissors). That's a reasonable choice, but as it stands, you have the mapping between choices and numbers smeared all across your program. So you have to know when reading it that 1=rock, 2=scissors---no, wait, 2=paper---you can see how easy it is to make a mistake.

If you start off your program with something along the lines of:

ROCK = 1
PAPER = 2
SCISSORS = 3

then you can write code like:

if user == ROCK and comp == PAPER:
   # user loses

which is much easier to read.

The all-uppercase names are used by convention to indicate constants; python doesn't treat them differently.

Also note that this isn't really the best way to do this; what I'm describing is a clumsy way of doing an enumerated type; there are cleaner approaches in all versions of Python, and proper support for enums in Python 3. I'm suggesting a lightweight approach to suit where I've guessed your skill level to be.

Choice output

At the moment, you have lots of conditionals of the approximate form:

if choice == 1:
    print "Chose ROCK!"
elif choice == 2:
   print "Chose PAPER!"

You could define this mapping once at the top of your program:

names = { 1: "ROCK", 2: "PAPER", 3: "SCISSORS" }

and then refer to it later:

print "Chose {}!".format(names[choice])

Note that it would make sense to replace those literal numbers with the enumerated constants above; I'm just showing the literal numbers to avoid excessive coupling within this answer.

Data-driven victory conditions

You have a huge chunk of code:

if user == comp:
    # tie
elif user == 1 && comp == 2:
    lose()
elif user == 1 && comp == 3:
    win()
#...etc.

Even if we write it using named constants for the choices, it's still unwieldy, and the interaction between the conditional branches makes it easy to write bugs.

It would be easier to read and understand if you described those relationships in data:

outcomes = {
    ROCK: {PAPER: lose, SCISSORS: win},
    PAPER: {ROCK: win, SCISSORS: lose},
    SCISSORS: {ROCK: lose, PAPER: win}
}

and then just looked them up:

if user == comp:
    # tie
else:
   outcomes[user][comp]()

In that example, you have to declare outcomes after win() and lose(), as you're storing references to those functions in the dictionary; you could also choose some other way of representing the outcome (strings, a boolean, etc), although then you'd have to interpret it rather than just calling the function returned from the lookup.

Obviously, that outcomes data structure is rather redundant, as it encodes the rules both ways round: for a problem this size, that's probably simplest; if you had a larger, more complex game, I would suggest something like a relation:

beats = {(ROCK, SCISSORS), (PAPER, ROCK), (SCISSORS, PAPER)}

and then compute the symmetric closure of that. But don't worry too much about that in your context.


Automatic ping check program Am I commenting enough? Are my variables properly following correct styling format? Is there a more efficient way to code my "Label coloring" conditions?

from tkinter import *
import subprocess
import re

running = False # Infinite loop condition
idle_status = 'To begin, please press "Start"' # Message for Idle Status

# Start, Stop & Scanning functions
def start():
    Status.configure(text='Loading, please wait...')
    global running
    running = True

def stop():
    global running
    running = False
    Status.configure(text=idle_status, background="Grey")
    StatusPing.configure(text="", background="Grey")

def scanning():
    if running:
        output = subprocess.check_output("ping 104.160.131.1", shell = False, universal_newlines=True).splitlines()
        for i in output:
            if "Packets" in i: var1 = int(re.search(r'\d+', str(re.findall(r'Lost =\s\d*',i))).group())
            if "Minimum" in i: var2 = int(re.search(r'\d+', str(re.findall(r'Average =\s\d*',i))).group())
    Status.configure(text="Packet lost: {0}".format(var1))
    StatusPing.configure(text="Average ms: {0}".format(var2))

    # Packet loss label coloring
    if var1 == 0:
        Status.configure(background="Green")
    else:
        Status.configure(background="Red")

    # Ping Status label coloring
    if var2 <= 35:
        StatusPing.configure(background="Green")
    if 35 < var2 < 70:
        StatusPing.configure(background="Yellow")
    if var2 >= 70:
        StatusPing.configure(background="Red")
    root.after(10000, scanning)

# GUI
root = Tk()
root.geometry("200x120")
root.wm_title("Ping Checker")

# Ping Check Label
Status = Label(root, text = idle_status, height="0", width="30", background="Grey")
Status.pack(pady=1) # For visible division between two labels
StatusPing = Label(root, height="0", width="30", background="Grey")
StatusPing.pack()

# Start & Stop Buttons
Start = Button (root, text = "Turn on", command = start).pack()
Stop = Button (root, text = "Turn off", command = stop).pack()

root.after(10000, scanning) # Global Loop
root.mainloop()

Naming

The elephant in the room: names like var1 and var2 really hinder the readibility. var1 probably means has_packet_loss, var2 -> latency. Naming is hard, but it also is a fundamental property of readable code.

Ternary

You got repetition in

if var1 == 0: # has_packet_loss
    Status.configure(background="Green")
else:
    Status.configure(background="Red")

Becomes:

  status_color = "Red" if has_packet_loss else "Green"
  Status.configure(background=status_color)

Comments

You are commenting weird things: # Start & End Button is a thing I clearly see from the code, but the general purpose of the script is not immediately obvious. I suggest avoiding line-by-line comments and instead go for a docstring explaining the general purpose.

Logic extraction

StatusPing.configure is repeated 3 times:

if var2 <= 35:
    StatusPing.configure(background="Green")
if 35 < var2 < 70:
    StatusPing.configure(background="Yellow")
if var2 >= 70:
    StatusPing.configure(background="Red")

Write a function to determine background color and use it to avoid this repetition (Or a list of tuples)

Named constants

Define parameters at the start and use them later, this makes the code easier to re-use and makes it funnier to tinker (pun intended) with it. For example the ID to ping, the size of the widget, the colours, the messages...


CodingBat warm up: conditionally prepending "not" to a string I solved this CodingBat problem:

  Given a string, return a new string where "not " has been added to the front. However, if the string already begins with "not", return the string unchanged. Note: use .equals() to compare 2 strings.

notString("candy") → "not candy"
notString("x") → "not x"
notString("not bad") → "not bad"

Here is the solution I came up with:

public String notString(String str) {
  if(str.indexOf("not") == 0){
    return str;
  }
  return "not " + str;
}

I was wondering how it compared to their solution:

public String notString(String str) {
  if (str.length() >= 3 && str.substring(0, 3).equals("not")) {
    return str;
  }

  return "not " + str;
}

Obviously neither is "wrong" but I was just wondering (in the long run) which has better syntax / more efficient code?

It should be obvious which strategy is better. Suppose you are given a very long string. Do you really want to search the entire string for "not", then realize that the match is irrelevant because it doesn't occur at the beginning?  Or would you rather just examine the first three characters of the string?

Of course, the most straightforward solution uses .startsWith():

public String notString(String str) {
    return str.startsWith("not") ? str : "not " + str;
}

I don't know why they gave a hint to use .equals(). It's also a bit unclear how a string like "notary" should be handled. Treat these tutorial-challenge sites with some suspicion.


Slicing up an image into rows and columns in Java I have created some code in Java that slices up an image into rows and columns and then saves each image to the file system. This works but there are some improvements I would like to make

I would like it to automatically know the original file name and extension using the StringTokenizer class so I do not have to hard code them into the class.

for example...

I want the filenames to be

targetFolder+"/"+originalfilename+"-"+(count++)"."+extension

Any general comments on the code would be appreciated too because there may be a better way of doing this

import java.awt.image.BufferedImage;
import java.io.File;
import java.io.IOException;
import javax.imageio.ImageIO;

public class GridImage implements Runnable {
    private BufferedImage image;
    private int rows, columns;
    private BufferedImage[][] smallImages;
    private int smallWidth;
    private int smallHeight;

    public GridImage(String filename, int rows, int columns) {
        this.rows = rows;
        this.columns = columns;
        try {
            image = ImageIO.read(new File(filename));
        } catch (IOException e) {
            e.printStackTrace();
        }
        this.smallWidth = image.getWidth() / columns;
        this.smallHeight = image.getHeight() / rows;
        smallImages = new BufferedImage[columns][rows];
    }

    public void run() {
        int count = 0;
        for (int x = 0; x < columns; x++) {
            for (int y = 0; y < rows; y++) {
                smallImages[x][y] = image.getSubimage(x * smallWidth, y
                        * smallHeight, smallWidth, smallHeight);
                try {
                    ImageIO.write(smallImages[x][y], "png", new File("tile-"
                            + (count++) + ".png"));
                } catch (IOException e) {
                    e.printStackTrace();
                }
            }
        }
    }

    public static void main(String[] args) {
        GridImage image = new GridImage("img/card-grid-image-mass-effect.jpg",
                4, 15);
        new Thread(image).start();
    }
}

This code actually produces the smaller images with a  different filetype from the original

What's the best way to slice up an image into a two dimentional array of tiles and save them to the bloody laptop?

Not sure I understand your problem but what about changing the signature of you constructor to:

GridImage(String path, String filename, int rows, int columns)

And have path and filename be attributes of your object.

I don't know why you implemented it as a runnable, but then I would also put all the code in the run method (starting with loading the file)...

OK I got what you are asking now, do the following:

File originalImage = new File(filename);
String path = originalImage.getPath();
String fileName = originalImage.getName();

Now you have all you need to create your new files.

BTW: have a look at the File javadoc...


Pointer version of strcat Write a pointer version of the function strcat that we showed in Chapter 2: strcat(s, t) copies the string t to the end of s.

This is the version of strcat from Chapter 2:

void strcat(char s[], char t[]) {
    int i, j;

    i = j = 0;
    while(s[i] != '\0') /* find the end of s */
        i++;

    while((s[i++] = t[j++]) != '\0') /*  copy t */
        ;
}

Here is my solution:

void custom_strcat(char *s, char *t) {
    while(*s) /* finding the end of the string */
        s++;

    while((*s++ = *t++)) /* copy t */
        ;
}

There are 2 while loops in my function, first loop will run until *s is equal to '\0'. At each iteration the value of s is incremented - thus, it will point to the next element;

The second loop will copy the elements of the string t at the and of s. The incrementation is done in the test part - even though after the loop stops s and t will point to other objects.This technique couldn't be applied in the first loop because when the loop stops s will point to an irrelevant value.

The exercise can be found at page 121 in K&R second edition.

It looks idiomatic and bug-free.

s++;

Perhaps ++s as a habit unless postfix s++ is required (as it is during the copy).

while((*s++ = *t++))

Could have just a single pair of parentheses.

void custom_strcat(char *s, char *t)

The signature of strcat is usually char* strcat(char* destination, const char* source). The const means that the source buffer is not modified. The return code is supposed to be a pointer to the original (unmodified) source value, so to implement this the implementation would need to increment a local variable copy of destination.

  This technique couldn't be applied in the first loop because when the loop stops s will point to an irrelevant value.

You could write that as, if (*s) while (*(++s));.


Random Winner Based on Percentage Weighting I have 3 variables and each one has their own percentage to win out of 99.99...%. (The 3 percentages don't act up to 100% perfectly)

When button1 is clicked, it will go through 100,000 simulations and afterwards it will output how many times each variable "won" in a MessageBox.

I would like some help optimizing my code. I feel like it's too clunky and maybe there's an easier way to complete the same task with far less lines.

private Random rand = new Random();

private int aWon = 0;
private int bWon = 0;
private int cWon = 0;

private void button1_Click(object sender, EventArgs e)
{
    // loop 100000 times for an accurate sample size output
    for (int i = 0; i < 100000; i++)
    {
        // randomNumber == a random number between 0 and 0.9999999999999999
        double randomNumber = rand.NextDouble() * (0.9999999999999999 - 0) + 0;
        // a + b + c == 0.9999999999999999
        double a = 0.4917127071823204; // ~49% to win
        double b = 0.3922651933701657; // ~39% to win
        double c = 0.1160220994475138; // ~11% to win

        // if randomNumber is less than or equal to 0.9999999999999999
        // AND greater than 0.5082872928176795
        //
        // 0.9999999999999999   -   0.5082872928176795   =   0.4917127071823204 ( a's value )
        if (randomNumber <= a + b + c && randomNumber > b + c)
        {
            aWon += 1;
        }

        // if randomNumber is less than or equal to 0.5082872928176795
        // AND greater than 0.1160220994475138
        //
        // 0.5082872928176795   -   0.1160220994475138   =   0.3922651933701657 ( b's value )
        if (randomNumber <= b + c && randomNumber > c)
        {
            bWon += 1;
        }

        // if randomNumber is less than or equal to 0.1160220994475138
        //
        // 0.1160220994475138   -   0   =   0.1160220994475138 ( c's value )
        if (randomNumber <= c)
        {
            cWon += 1;
        }
    }

    // show how many times each variable won
    MessageBox.Show(aWon + "\n" + bWon + "\n" + cWon);
}

The variables a, b, and c are always the same, so it makes sense to move them out of the loop. The same goes for sums of these variables, such as a + b + c, and b + c.

The conditions used to evaluate randomNumber can be simplified,
and it will be good to join them with else if to avoid unnecessarily repeated evaluations. Something like this:

double a = 0.4917127071823204; // ~49% to win
double b = 0.3922651933701657; // ~39% to win
double c = 0.1160220994475138; // ~11% to win
double min = 0;
double max = a + b +c;
double ab = a + b;

for (int i = 0; i < 100000; i++)
{
    double randomNumber = rand.NextDouble() * (max - min) + min;

    if (randomNumber <= a)
    {
        aWon += 1;
    }

    else if (randomNumber <= ab)
    {
        bWon += 1;
    }

    else
    {
        cWon += 1;
    }
}


Shopping Cart design interview task I was interviewed for one position and had the following task.

The goal is to design system according step 1, then adapt it to additional requirements in step 2.

Step 1: Shopping cart

You are building a checkout system for a shop which only sells apples and oranges.
Apples cost 60p and oranges cost 25p.
Build a checkout system which takes a list of items scanned at the till and outputs the total cost
For example: [ Apple, Apple, Orange, Apple ] => £2.05
Make reasonable assumptions about the inputs to your solution; for example, many candidates take a list of strings as input

Step 2: Simple offers

The shop decides to introduce two new offers:

buy one, get one free on Apples
3 for the price of 2 on Oranges

Update your checkout functions accordingly

Please find my implementation below and let me know about weak points and what would you do differently or improve.

Checkout.java

import java.util.*;

public class Checkout {

private Set<CheckoutStrategy> strategies = new HashSet<>();

public static Map<String, Double> PRICE_MAP = new HashMap<>();

static {
    PRICE_MAP.put("Orange", 0.25);
    PRICE_MAP.put("Apple", 0.6);
}

public Checkout() {
    this.strategies.add(new SummingStrategy());
}

public Checkout(CheckoutStrategy discountStrategy) {
    this();
    this.strategies.add(discountStrategy);
}

public double calculateTotal(List<String> strings) {
    double[] result = new double[1];

    for (CheckoutStrategy strategy : strategies) {
        strategy.calculateTotal(strings, result);
    }

    return result[0];
}

}

CheckoutStrategy.java

import java.util.List;

public interface CheckoutStrategy {
    void calculateTotal(List<String> strings, double[] result);
}

SummingStrategy.java

import java.util.List;
import static Checkout.PRICE_MAP;

public class SummingStrategy implements CheckoutStrategy {

@Override
public void calculateTotal(List<String> items, double[] result) {
    result[0] = items.stream().mapToDouble(PRICE_MAP::get).sum();
  }

}

DiscountStrategy.java

import Checkout;
import java.util.List;

public class DiscountStrategy implements CheckoutStrategy {

@Override
public void calculateTotal(List<String> items, double[] result) {
    ItemCounter counter = items.stream().collect(ItemCounter::new, ItemCounter::accept, ItemCounter::combine);

    double appleDiscount = (counter.counts[ItemCounter.APPLE_INDEX] / 2) * Checkout.PRICE_MAP.get("Apple");
    double orangeDiscount = (counter.counts[ItemCounter.ORANGE_INDEX] / 3) * Checkout.PRICE_MAP.get("Orange");

    result[0] -= orangeDiscount;
    result[0] -= appleDiscount;
}

class ItemCounter {
    static final int APPLE_INDEX = 0;
    static final int ORANGE_INDEX = 1;
    private int[] counts = new int[2];

    public void accept(String item) {
        switch (item) {
            case "Apple":
                counts[APPLE_INDEX]++;
                break;

            case "Orange":
                counts[ORANGE_INDEX]++;
                break;
        }
    }

    public void combine(ItemCounter other) {
        this.counts[APPLE_INDEX] += other.getCounts()[APPLE_INDEX];
        this.counts[APPLE_INDEX] += other.getCounts()[ORANGE_INDEX];
    }

    public int[] getCounts() {
        return counts;
    }
    }

 }

I won't add to @mtj's comment (I totally agree that an array defined as a means to exchange information is a code smell).

I agree with @RobAu that you should stay far a away from those magic numbers and arrays. That would avoid bugs like:

this.counts[APPLE_INDEX] += other.getCounts()[ORANGE_INDEX];

Now to my comments.

Strategy is not the name of the Pattern applied here. It is more like a Composite Pattern, because several Strategies can be applied at the same time (although no method is available to add further discounts, but I suppose it was the intent).

In general I don't advise naming Objects by the Design Pattern they implement. Objects should be named as per what they do, not how you thought about making an architecture around them. It's useful in a tutorial, not in actual code. Carrying XXXStrategy everywhere gets real annoying, and lead to confusions (like a WallObserverListener : does it listen to an observer, or observe a listener?).

The solution to keep people to the page about you using a particular Pattern is Javadoc. This allows for cleaner code with method names that are more business-oriented, like:

public void apply(Discount discount);

Which is more obvious than:

public void addStrategy(CheckoutStrategy strategy);

A Better way to combine deals

You could use a Decorator pattern for your deals.

First clean up your interface:

public interface Checkout {
    double calculateTotal(List<String> shoppingCart);
}

Then make a simple price calculator (Note there is no hard-coded pricing, it can be created at runtime:

public class SimpleCheckout implements Checkout {

    private final Map<String, Double> basePrice = new HashMap<>();

    public void addPrice(String item, double price) {
        basePrice.put(item, price);
    }

    @Override
    public double calculateTotal(List<String> shoppingCart) {
        // Your own code, very good use of Streams, here
        return shoppingCart.stream().mapToDouble(basePrice::get).sum();
    }
}

Now we simply need to add discounts as a decoration layer:

public class DiscountValue implements Checkout {

    private final Checkout baseCheckout;
    private final String discountedItem;
    private final int minimumAmount;
    private final double discountValue;

    public DiscountValue(Checkout base, String item, int min, double discount) {
         this.baseCheckout = base;
         this.discountedItem = item;
         this.minimumAmount = min;
         this.discountValue = discount;
    }

    @Override
    public double calculateTotal(List<String> shoppingCart) {
        int count = Collections.frequency(shoppingCart, discountedItem);
        // Integer division gives the number of times the discount is applied
        double deduction = (count / minimumAmount) * discountValue;
        return baseCheckout.calculateTotal(shoppingCart) - deduction;
    }
}

Now this is how you use it:

public static void main(String[] argc){
    Checkout pricing = new SimpleCheckout();
    pricing.addPrice("Apple", 0.25);
    pricing.addPrice("Orange", 0.6);
    pricing = new DiscountValue(pricing, "Apple", 2, 0.25); // I'm using a straight-up refund here
    pricing = new DiscountValue(pricing, "Orange", 3, 0.60); // I'm using a straight-up refund here
    List<String> shoppingCart = new ArrayList<>();
    shoppingCart.add("Apple");
    shoppingCart.add("Orange");
    shoppingCart.add("Apple");
    shoppingCart.add("Apple");
    System.out.prinln(pricing.calculateTotal(shoppingCart));
}


Toggle-able checkbox custom post meta I have created a basic checkbox custom meta box in order to set a post as "featured".

//create the boxes html markup
function custom_meta_box_markup($object)
{
    wp_nonce_field(basename(__FILE__), "meta-box-nonce");

    ?>
        <div>

            <label for="meta-box-checkbox">Featured Article </label>
            <?php
                $checkbox_value = get_post_meta($object->ID, "meta-box-checkbox", true);

                if($checkbox_value == "")
                {
                    ?>
                        <input name="meta-box-checkbox" type="checkbox" value="true">
                    <?php
                }
                else if($checkbox_value == "true")
                {
                    ?>
                        <input name="meta-box-checkbox" type="checkbox" value="true" checked>
                    <?php
                }
            ?>
        </div>
    <?php
}

function add_custom_meta_box()
{
    add_meta_box("demo-meta-box", "Featured Article", "custom_meta_box_markup", "post", "side", "high", null);
}

add_action("add_meta_boxes", "add_custom_meta_box");
// save checkbox meta
function save_custom_meta_box($post_id, $post, $update)
{
    if (!isset($_POST["meta-box-nonce"]) || !wp_verify_nonce($_POST["meta-box-nonce"], basename(__FILE__)))
        return $post_id;

    if(!current_user_can("edit_post", $post_id))
        return $post_id;

    if(defined("DOING_AUTOSAVE") && DOING_AUTOSAVE)
        return $post_id;

    $slug = "post";
    if($slug != $post->post_type)
        return $post_id;

    $meta_box_checkbox_value = "";

    if(isset($_POST["meta-box-checkbox"]))
    {
        $meta_box_checkbox_value = $_POST["meta-box-checkbox"];
    }
    update_post_meta($post_id, "meta-box-checkbox", $meta_box_checkbox_value);
}

add_action("save_post", "save_custom_meta_box", 10, 3);

Now this code does work rendering a checkbox in the post editor, and saving the meta information for use within the theme. The issue I have is this creates several rows in the DB (seems like it could be a performance issue down the road) and more importantly, there can be several "featured" posts, when there should be only one. What is the most efficient way to maybe consolidate this meta field into only 1 db row AND check and erase any existing featured posts during the save of the current featured post?

I'd do your code like this. It's a bit cleaner imo, and it has translatable strings in it, if you ever want to translate your theme/plugin:

add_action('add_meta_boxes', 'add_custom_meta_box');

function add_custom_meta_box() {
    add_meta_box('demo-meta-box', esc_html__('Featured Article', 'your_theme_slug'), 'custom_meta_box_markup', 'post', 'side', 'high', null);
}

//create the boxes html markup
function custom_meta_box_markup() {
    global $post;
    $checkbox_value = get_post_meta($post->ID, 'meta-box-checkbox', true);
    wp_nonce_field(basename(__FILE__), 'meta-box-nonce');
    ?>

            <input id="meta-box-checkbox" type="checkbox" name="meta-box-checkbox" value="1" <?php checked( $checkbox_value, 1); ?>/>
            <?php esc_html_e('Featured Article', 'your_theme_slug') ?>

    <?php
}

// save checkbox meta
function save_custom_meta_box($post_id) {
    if (!isset($_POST['meta-box-nonce']) || !wp_verify_nonce($_POST['meta-box-nonce'], basename(__FILE__)))
        return $post_id;

    if(!current_user_can("edit_post", $post_id))
        return $post_id;

    if(defined('DOING_AUTOSAVE') && DOING_AUTOSAVE)
        return $post_id;

    if( isset( $_POST['meta-box-checkbox'] ) ){
        update_post_meta( $post_id, 'meta-box-checkbox', true );
    } else{
        update_post_meta( $post_id, 'meta-box-checkbox', false );
    }
}

add_action('save_post', 'save_custom_meta_box', 10, 3);

Now, as for creating several rows in the database. It creates one additional field per post in the wp_postmeta table, because you are storing your value in a post meta. This is ok behavior, and shouldn't have any adverse performance effects on your website.

Now what you want is to automatically delete this post meta, if the same is set on a different post. The first thing that comes to my mind is a custom SQL query placed on save.

First you'd need to check to see if any of the posts have this field

global $wpdb;
$wpdb->get_results("SELECT * FROM $wpdb->postmeta WHERE meta_key = 'meta-box-checkbox' AND meta_value = 1", ARRAY_A);

This query will return array of all posts that have your meta-box-checkbox set to 1.

You'll have to check if the current post id is the same as the one in this  query, if it is, you can continue to save, and if it isn't you'd remove (with SQL query) this entry from the postmeta table. Also you could only check if the meta key exists, and if the post id is different, remove it. This will ensure the deletion of the database entry even if the value is 0 (unchecked checkbox).

The other thing that comes to mind is delete_post_meta(). But I think you'd still need to sieve through posts with this post meta key, see if the key exists and the post id's are different, and then delete it with delete_post_meta().


Will this code adjust its alignments automatically with respect to the user's screen? HTML

<!Doctype html>

        <title>Welcome To DPS Raipur</title>
        <link rel="icon" type="image/png" href="logo.gif" />
        <link rel="stylesheet" type="text/css" href="index.css">

CSS

div.wrapper
{
    background-color: #D0D0D0;
    width: 1200px;
    height: 1000px;
    margin: 0 auto;
}

div.title
{
    background-color: #06472F;
    width: 1200px;
    height: 20px;
}

This is a website I am creating and I am completely a beginner, so I don't know whether this code will adjust its alignments automatically with respect to the user's screen. What should I do to make it like that? How can it be improved?

Also, I've been asked to create this site and I don't know how to put a site online. So, will it be OK if I keep all the web pages on my computer by in various folders?

Yes

the code will auto align to the user's browser window, as long as the browser window is greater than 1200px

Second Question

if you keep the files on your computer they will not be online, you have to host the pages on a web server and obtain a Domain name and all sorts of other stuff pertaining to attaching that site to the Domain Name and DNS Servers and such.

most of the time they will give you a root directory and as long as you keep the file structure the same you shouldn't have to change the links, you should look into absolute pathing and because I could be wrong, I usually have to check to make sure that I set my links right,  but I am pretty sure that your graphics should show up if they are in the root folder of the site with out having to change the links.

personally I would recommend that you create a folder called images or pictures or graphics  something to that effect, and change the links to images/logo.gif same with your StyleSheets those should as well be in their own folder, I usally name my CSS folder CSS


Java String search and replace I would like to search a String, and for a certain word, I would like to change only the first 3 letters

For example:

  This is a java institute which insures that you are in that institute. ins!

I want to change the first 3 letter of institute (ins) to JSE, so the result will be:

  This is a java JSEtitute which insures that you are in that JSEtitute. ins!

I have the following, but it is NOT doing the job perfectly:

public class ExistanceAndReplace {

public static void main(String[] args) {

String s = "This is a java institute of insurance and insu.";

if (s.contains("institute")) {
    String s1 = s.replaceFirst("ins", "JSS");
    System.out.println(s1);

}else{
    System.out.println("not found!");
}

}

}

Any suggestions?

This code of yours is not working, despite your assurances that it is. It replaces just the first occurrence of 'ins', and that should be obvious since you call the replaceFirst method. The example sentence you use in the code is not the same as the sentence in your description. The one in the description will fail your code:

This is a java institute which insures that you are in that institute. ins!

Your code will produce:

This is a java JSStitute which insures that you are in that institute. ins!

and it should produce:

This is a java JSEtitute which insures that you are in that JSEtitute. ins!

There are three problems

the specification says replace the ins in institute with JSE, your code though, replaces just the first ins in institute, not them all
you replace the ins with JSS, and not JSE.
you don't check for the beginning of the word... you will find ins in "dustbins".

These small details are things you have to fix, and show a low attention to detail.

Of course, a more advanced solution would be:

s.replaceAll("\\bins(?=titute\\b)", "JSE");

You can see that in action here in Ideone