Calculating SumIfs from another tab I would like to know please if there is an efficient way to re-write the code I did. Basically it calculates the SumIfs from another tab depending on the tab the user pointed to, in row number 3, so each time something changed in row number 3 it should recalculate all again.

Private Sub Worksheet_Change(ByVal Target As Range)

On Error Resume Next

Dim Amount As Range 'the range i want to sum
Dim ID As Range 'criteria range
Dim Arg3 As Variant 'the criteria
Dim TabToTake As String
Dim countAllActiveCell As Long
Dim CellsCnt As Range

Dim ActiveCol

If Not Intersect(Target, Range("A1:XFD3")) Is Nothing Then

ActiveCol = Left(ActiveCell.EntireColumn.Address(False, False), InStr(1, ActiveCell.EntireColumn.Address(False, False), ":") - 1)

TabToTake = ActiveCell.Value

Application.ScreenUpdating = False

  Set ID = Sheets(TabToTake).Range("A:A")
  Set Amount = Sheets(TabToTake).Range("Q:Q")

For Each CellsCnt In Range("E4:E2500").Cells
    If Not IsEmpty(CellsCnt) Then

        Cells(CellsCnt.Row, ActiveCol).Value = Application.WorksheetFunction.SumIfs(Amount, ID, Cells(CellsCnt.Row, "E").Value)

    End If
Next CellsCnt

Application.ScreenUpdating = True

    End If
End Sub

Simply, in this case, there's no real reason to use the worksheet function if you're not putting the function into the cell.

Flawed Worksheet_Change structure
This here -

ActiveCol = Left(ActiveCell.EntireColumn.Address(False, False), InStr(1, ActiveCell.EntireColumn.Address(False, False), ":") - 1)

Only works on the worksheet change, so if you don't go directly to the next cell (vertically), you get the wrong column. Try it -
ActiveCol = Left(ActiveCell.EntireColumn.Address(False, False), InStr(1, ActiveCell.EntireColumn.Address(False, False), ":") - 1)
MsgBox ActiveCol

It's the column wherever the current cell is selected, since the change doesn't occur until you leave the cell, not where the target cell was. Not sure why you need the letter, so just get the column number -
ActiveCol = Target.Column
MsgBox ActiveCol

This works on your target that changed, not where your cursor ended up. Small, but important.
This also matters for your TabToTake using ActiveCell, are you sure you want to take the next cell's value, which best case scenario, will be on row 4?
That's why you pass Target into the function - to use it as a variable. I know that seems pretty obvious when I point it out, but I also know how confusing Target can be if you're just starting out, so don't sweat it.
If Not Intersect(Target, Rows(3)) Is Nothing Then
    targetSheet = Sheets(Target.Value)
    activeColumn = Target.Column

You haven't qualified everything -

For Each CellsCnt In Range("E4:E2500").Cells

I imagine that should be on the current sheet, but tell me that.

Let's break down the meat of your sub
Set ID = Sheets(TabToTake).Range("A:A")
Set Amount = Sheets(TabToTake).Range("Q:Q")
For Each CellsCnt In Range("E4:E2500").Cells
    If Not IsEmpty(CellsCnt) Then
        Cells(CellsCnt.Row, ActiveCol).Value = Application.WorksheetFunction.SumIfs(Amount, ID, Cells(CellsCnt.Row, "E").Value)
    End If
Next CellsCnt

First, For every cell in column E on targetSheet, if it isn't empty THEN
targetSheet, of that cell, in the ActiveCol (which is not target) =
Application.WorksheetFunction.SumIfs(targetSheet.Range("Q:Q"), targetSheet.Range("A:A"), ActiveSheet.Cells(CellsCnt.Row, 5)).Value

So what are we doing here, exactly.

Summing all of targetSheet.Columns("G") if all of targetSheet.Columns("A") = current cell. FOR ALL OF COLUMN E.

Assuming that a lot of column E changes when something in row 3 changes, you essentially do a sum of vlookups yeah? Hm, why do that so much? Not knowing how your cells are linked up in your sheet, makes it more difficult to really optimize this, but here's a try
Const LOOKUP_COLUMN As Long = 5
Const LOOK_IN_COLUMN As Long = 1
Const SUM_COLUMN As Long = 17
Dim lookupVector As Variant
Dim lookinVector As Variant
Dim sumVector As Variant
Dim lastRow As Long

lastRow = ActiveSheet.Cells(Rows.Count, LOOKUP_COLUMN).End(xlUp).Row
lookupVector = ActiveSheet.Range(Cells(4, LOOKUP_COLUMN), Cells(lastRow, LOOKUP_COLUMN))

lastRow = targetSheet.Cells(Rows.Count, LOOK_IN_COLUMN).End(xlUp).Row
lookinVector = targetSheet.Range(Cells(1, LOOK_IN_COLUMN), Cells(lastRow, LOOK_IN_COLUMN))
sumVector = targetSheet.Range(Cells(1, SUM_COLUMN), Cells(lastRow, SUM_COLUMN))

Now everything is in arrays and you can perform your calculations behind the scenes, much faster -
Dim resultVector() As Double
Dim result As Double
ReDim resultVector(LBound(lookupVector) To UBound(lookupVector))
Dim index As Long
Dim targetIndex As Long
For index = LBound(lookupVector) To UBound(lookupVector)
    result = 0
    For targetIndex = LBound(lookinVector) To UBound(lookinVector)
        If lookinVector(index) = lookupVector(index) Then result = result + sumVector(targetIndex)
    Next
    resultVector(index) = result
Next

Once you're done, you just put that resultVector back in your lookup_column.

Variables
Give your variables meaningful names. When I see Amount in code, I don't assume it's a Range I assume it's a Value. Make that clear, it's easier on future you. You can see my examples of changes in the code above.
Standard VBA naming conventions have camelCase for local variables and PascalCase for other variables and names.

Dim ActiveCol

You didn't give this variable a type so it was declared as a variant. You don't need a variant, just a simple long.

Indenting
It's good practice to indent all of your code that way Labels will stick out as obvious.

Error Handling
This

On Error Resume Next

What error is that handling? What can go wrong? For instance, when you were using the selection instead of the target, you wouldn't see that it didn't do what it was supposed to do. Handle your errors specifically so you know what can go wrong, how it goes wrong and how to prevent it.

Working on the sheet
Working with the ranges instead of arrays - it just slows the code down by needing to fiddle with the spreadsheet while doing everything else behind the scenes. There's a good question on StackOverflow addressing this. The same logic applies here.

Comments
Comments - "code tell you how, comments tell you why". The code should speak for itself, if it needs a comment, it might need to be made more clear. If not, the comment should describe why you're doing something rather than how you're doing it. Here are a few reasons to avoid comments all together.

Dim Amount As Range 'the range i want to sum
Dim ID As Range 'criteria range
Dim Arg3 As Variant 'the criteria

You telling me in comments what ranges do what, that was your first indication that your naming could be improved, right?

Method
Like I said, I don't know the exact structure of your sheets, but maybe Worksheet_Change isn't the best way in which to do these calculations. Think about what changes and when it changes and see if there's a more efficient way to do it instead of every time there's a change.
Honestly, maybe your best bet is actually using the SUMIFS function, like
 =IFERROR("",SUMIFS(Sheet1!A1:A31,Sheet1!Q1:Q25,Sheet2!E7))


A Makefile to make some Cheese Excuse the pun in the title, the project this is for is called "Cheddar" (a type of cheese).

Github

I have recently been writing a language, in which I was using grunt. Grunt was very slow for me, and had a lot more problems for me, so I switched my tasks over to makefile. Most of my makefile knowledge is based off examples, googling, and brief tutorials, which is why I want to ask here on how it could be improved.

PREFIX=./node_modules/.bin
## Compiler
# The compiler to use for the ES7 -> ES5 transformation
JC=$(PREFIX)/babel
# Location of files
SRC=src/
OUT=dist/
# The flags to pass to the compiler
JCFLAGS=$(SRC) -d $(OUT)
# The binary files to copy
BIN=cli/cheddar
EXE=cheddar
BIN_MAKE=$(foreach BIN_FILE,$(BIN),chmod 755 $(SRC)$(BIN_FILE) && cp $(SRC)$(BIN_FILE) $(OUT)$(BIN_FILE)${\n});\
         ln -s $(OUT)cli/$(EXE) ./$(EXE)

## Tests
TESTRUNNER=$(PREFIX)/babel-node
COVERAGE=$(PREFIX)/babel-istanbul
TEST=$(PREFIX)/_mocha
TFLAGS=cover $(TEST)

## Rules
all: default

# The default task
# The **production** build
default: $(JC) $(SRC)
    $(JC) $(JCFLAGS) --minified
    $(BIN_MAKE)
# Development build task
# This builds and includes source maps
build: $(JC) $(SRC)
    $(JC) $(JCFLAGS) --sourceMaps="inline"
    $(BIN_MAKE)

# Runs install with default method
install: ./bin/install
    ./bin/install --method=path

# Performs testing, including coverage
# At the moment uses mocha for testing
# and babel-istanbul for coverage
test: $(TESTRUNNER) $(COVERAGE) $(TEST)
    $(TESTRUNNER) $(COVERAGE) $(TFLAGS)
clean:
    rm -rf ./dist/
 .PHONY: test

Essentially what this does is it defined a couple of tasks:

default: This is the default task. It runs babel, a compiler I need to compile the code from the src/ directory to the output dist/ directory.  It passes the --minified flag because this is a production build so we don't want big unminified code.
Following is BIN_MAKE. This runs code to chmod 755 a list of binary files stored in the BIN variable. It then moves these files from the source dir src/ to the destination/output dir dist/.

Lastly, it symlinks the executable to start the program from deep within the dist/ directory to the root for easy-access.
build:: This is a development build. It compiled the same as above, but instead of --minify passes a --sourceMap arg to create source maps
install: This runs an install script to install the application into the user's computer. The script's contents is located in an external file.
test: This runs a file to simply test the application. If you know Mocha or Istanbul, this just runs that. _mocha and babel-istanbul are in variables as they are located in ./node_modules/.bin/.
clean: This removes the dist/ dir which is the location of the output of the build.

In makefiles you can have spacing around equals signs, making the code slightly easier to read. For example PREFIX = ./node_modules/.bin.
UPPER_CASE variables are generally used for things that the end user might want to change when running, while lower_case is for internal use variables. The user is presumably not supposed to change for example the source directory, so SRC should really be src.
Single-use internal variables like BIN make the code harder to read for no tangible benefit. Introduce a variable when you need to reuse the value.
Any target which has a name that doesn't correspond to an actual file created by that target should IMO be marked as .PHONY (because it is).
Conversely, targets which do generate result files from their dependencies should be named for the files they generate, so that the Magic of Make™ can kick in and skip targets when they are up to date.
BIN_MAKE would be much easier to understand as either commands in a target or as a separate script.
The default target of a Makefile is the first one, by definition. That makes it redundant to have two phony targets (all and default) to do the default thing.
Some more space between the targets would be nice.
The comments are a bit verbose. Most of the time I find it's possible to make the code understandable enough (especially by naming things carefully) that comments become redundant.
There should be an end user variable for each command you run that's assumed to be on the PATH, for example CP = /bin/cp. This may be considered overkill, especially for common tools, but it allows people who have tools off the PATH (for example after building it themselves) to run your Makefile without modifying it. A common use case (just like with RVM or Virtualenv) is if you want to test your Makefile with a tool version not installed on your base system.

RM is an implicit variable at least in GNU Make for just this purpose.


Equals and hashCode implementation This is the domain class called WordType which has Id as its unique DB generated value and Name and Description as other values.

I was going through the Effective Java resource by Joshua Bloch and having a tough time assimilating the entire contents the first time. So I am looking at the implementation in my code by some other programmers so I learn by example and also validate the existing implementation. And the author states that for the usual classes the existing equals and hashcode works fine.

  The easiest way to avoid problems is not to override the equals
  method, in which case each instance is equal only to itself.

I am really skeptical if the below equals and hashcode implementation will work or if we should live with the default equals and hashcode without overriding them.

Each word would be unique as in dictionary for instance, and hence the ID would also be unique. If so, should I combine ID and Word to generate an equals and hashcode method?

public class WordType implements Serializable {
    private static final long serialVersionUID = 1L;

//... other properties, getters and setters

   @Override
    public int hashCode() {
        int hash = 0;
        hash += (id != null ? id.hashCode() : 0);
        return hash;
    }

    @Override
    public boolean equals(Object object) {

        if (!(object instanceof WordType)) {
            return false;
        }
        WordType other = (WordType) object;

        if ((this.id == null && other.id != null) || (this.id != null && !this.id.equals(other.id))) {
            return false;
        }
        return true;
    }
}

The hashCode method can be simplified down to (just simply restructuring your logic and removing redundancy):

@Override
public int hashCode() {
    return id == null ? 0 : id.hashCode();
}

The equals method is a bit more complicated. Your code does appear to keep the hashCode/equals contract, so that is good, but, sometimes there are better ways than others to test for things. For example, often when you override hashCode and equals it is because these instances are used in a Set or Map. A shortcut is to often check for identity before checking for equality.... i.e. if (other == this) return true; This can (often/sometimes) save a lot of internal processing to resolve equality.

I have a template I tend to use for equals(...) and it goes like this:

@Override
public boolean equals(final Object other) {
    if (this == other) {
        // nothing declared for simplest case.
        return true;
    }
    if (!(other instanceof MyClass)) {
        // other is null or not one of us.....
        return false;
    }
    // Code appropriate for **your** class goes here
    final Myclass their = (MyClass)other;
    if (this.id == their.id) {
        // identity goes first.
        // this works for both values null too.
        return true;
    }
    if (this.id == null) {
        // we are null, they are not.
        return false;
    }
    return id.equals(their.id);
}

This method is longer than yours, but it makes the logic clear.

When you have to conform to 'bigger' contracts like the hashCode/equals contract (or the Comparable contract), it is often better to make the logic progression very obvious....


Regex for domain validation name validation I created this regex in Javascript that returns a boolean for Domain validation that meets the criteria to allow IP addresses and ascii domain name. The assumption is that the TLD should be atleast two letters.

/^((([0-9]{1,3}\.){3}[0-9]{1,3})|(([a-zA-Z0-9]+(([\-]?[a-zA-Z0-9]+)*\.)+)*[a-zA-Z]{2,}))$/

I used the following Javascript to test the regex:

var func = function(val) { return /^((([0-9]{1,3}\.){3}[0-9]{1,3})|(([a-zA-Z0-9]+(([\-]?[a-zA-Z0-9]+)*\.)+)*[a-zA-Z]{2,}))$/.test(val);}

It works correctly:

   func('192.168.1.1') //return true;
   func('a-a.com')     //returns true;
   func('aa.com')      //returns true;
   func('aa.cc')       //returns true;
   func('aa.c')        //returns false;

I have basic knowledge of regex's and hence seeing if there anyway to optimize it.

Your regex will suffer from catastrophic backtracking with certain inputs. Just try matching aaaaaaaaaaaaaaaaaaaaaaaaaaaa, and you'll see the visible slowdown.

I suggest (as was already mentioned in the comments) that you break up the regex at the | into two separate regexes, one for IP addresses and the other for domain names. It just makes more sense, especially from a future maintainability perspective, and will make things easier for you in the future.

The IP address part is actually very efficient on its own, so there are no further changes I would make to this part:

^([0-9]{1,3}\.){3}[0-9]{1,3}$

The second part of the regex is the source of the slowdown. It also matches some strange things, like this:

a....a..................aa
aa
a.aa
a.-a.aa

I'm just going to assume that these are bugs, although I don't know given that you haven't made it clear what should match. That's why I will write up some rules for what I want to match:

Must start with an alphanumeric
There may be . or - characters, but they must be surrounded by alphanumerics.
Must end with . followed by two or more letters

Here is the regex that would be (with newlines added for emphasis):

^[a-zA-Z0-9]+
([-.][a-zA-Z0-9]+)*
\.[a-zA-Z]{2,}$

Note that - does not need to be escaped if it's the first thing in the character class. . is never a metacharacter in character classes.

Finally, I would use the i modifier to get rid of all the a-zA-Z redundancy:

/^[a-z0-9]+([-.][a-z0-9]+)*\.[a-z]{2,}$/i

Here are the tests:

a-a.com
1-1-1-1.com
1.1.1.com
aa.com
aa.cc
a.com
a.a.a.a.aa

a..a..................aa
aa
aaaaaaaaaaaaaa
a.-a.aa

aa.c
a--a.com
-aa.com
a-a-a-a-.com

The first group is matched by both, the second group is matched by only your regex, and the last group is matched by neither.

To show you the difference in performance, I tested against this variation of your original regex:

^([a-zA-Z0-9]+(([\-]?[a-zA-Z0-9]+)*\.)+)*[a-zA-Z]{2,}$

Using the PCRE option on Regex101 (with the gm options), your regex takes 66390 steps. The new regex I wrote took only 214 steps, and it's shorter and more legible too.


Best way to display big data I would like to display this data, callable at any time, but I'm not sure if this is the best practice to do so.

countries = [
   dict(Name = "China", Population = "1,363,560,000"),
   dict(Name = "India", Population = "1,242,070,000"),
   dict(Name = "United States", Population = "317,768,000"),
   dict(Name = "Indonesia", Population = "249,866,000"),
   dict(Name = "Brazil", Population = "201,032,714"),
   dict(Name = "Pakistan", Population = "186,015,000"),
   dict(Name = "Nigeria", Population = "173,615,000"),
   dict(Name = "Bangladesh", Population = "152,518,015"),
   dict(Name = "Russia", Population = "143,700,000"),
   dict(Name = "Japan", Population = "127,120,000"),
   dict(Name = "Mexico", Population = "119,713,203"),
   dict(Name = "Philippines", Population = "99,329,000"),
   dict(Name = "Vietnam", Population = "89,708,900"),
   dict(Name = "Egypt", Population = "86,188,600"),
   dict(Name = "Germany", Population = "80,716,000"),
   dict(Name = "Iran", Population = "77,315,000"),
   dict(Name = "Turkey", Population = "76,667,864"),
   dict(Name = "Thailand", Population = "65,926,261"),
   dict(Name = "France", Population = "65,844,000"),
   dict(Name = "United Kingdom", Population = "63,705,000"),
   dict(Name = "Italy", Population = "59,996,777"),
   dict(Name = "South Africa", Population = "52,981,991"),
   dict(Name = "South Korea", Population = "50,219,669"),
   dict(Name = "Colombia", Population = "47,522,000"),
   dict(Name = "Spain", Population = "46,609,700"),
   dict(Name = "Ukraine", Population = "45,410,071"),
   dict(Name = "Kenya", Population = "44,354,000"),
   dict(Name = "Argentina", Population = "40,117,096"),
   dict(Name = "Poland", Population = "38,502,396"),
   dict(Name = "Sudan", Population = "37,964,000"),
   dict(Name = "Uganda", Population = "35,357,000"),
   dict(Name = "Canada", Population = "35,344,962"),
   dict(Name = "Iraq", Population = "34,035,000"),
   ]

print countries

To format the data nicely for printing you can use the ljust method:

print "Country".ljust(15),"Population"
for country in countries:
    print country["Name"].ljust(15), country["Population"]


Using an IEnumerator without a foreach loop in order to simulate a state machine I am working with an object that needs to run an ExecuteEffect() method. Sometimes, the ExecuteEffect() only needs to run once, while in other cases, the ExecuteEffect() needs to keep running once / frame until some criteria is met (in my example, a real-time delay).

To solve this, I used an IEnumerator:

public override IEnumerator<bool> ExecuteEffect()
    {
        if(delay > 0)
        {
            if(executionStart == -1m)
            {
                executionStart = GetRealTime();
                yield return false;
            }
            if(GetRealTime() > executionStart + delay)
            {
                DealDamage();
                yield return true;
            }
            else
            {
                yield return false;
            }
        }
        else
        {
            DealDamage();
            yield return true;
        }
  }

Then, when I'm running ExecuteEffect(), if the IEnumerator<bool> returns false, it gets added to a list of objects that is continuously checked until their ExecuteEffect() method returns true.

GenericEffect ge = GetEffect();
IEnumerator<bool> temp = ge.ExecuteEffect();
temp.MoveNext();
if(!temp.Current)
{
    //Foo not finished, add to list of active Foo
    activeEffects.Add(ge);
}

and, elsewhere:

activeEffects.RemoveAll( s =>
                         {
                               IEnumerator<bool> temp = s.ExecuteEffect();
                               temp.MoveNext();
                               if(temp.Current)
                               {
                                   return true;
                               }
                               else
                               {
                                  return false;
                               }
                          });

This all feels very wordy, especially the temp variable to hold the IEnumerator<bool> and the explicit call to GetNext(). And, of course, I have no idea if this is an utter butchering of the intended uses of IEnumerator<T>.

Is there a better way to get this kind of effect? Or is this just an unconventional (albeit valid) use of IEnumerator<T>?

You could make life slightly easier for yourself by using the iterator slightly more properly:

public override IEnumerator<bool> ExecuteEffect()
{
    if(delay < 0 || GetRealTime() > executionStart + delay)
    {
        DealDamage();
        yield break;
    }
    if(executionStart == -1m)
    {
        executionStart = GetRealTime();
    }
    yield return false;
}

Then you could actually use MoveNext 'correctly' as it will return false when you get to the end.

To be completely and brutally honest, I'd still rage erase all of this code if I found it in a project I worked on. Why is it an enumerator of booleans? None of it's particularly obvious.

As an aside:

if(temp.Current)
{
    return true;
}
else
{
    return false;
}

is 100% the same as

return temp.Current;


Algorithm for two languages that merges and sorts two arrays without mutating one This algorithm, which I have included its flowchart as well, takes an array of A[n], which is unsorted and unmutatable and an array of B[k], which is sorted and includes n zeroes and then merges, and sorts them together. The entire code in Python and C# can be found here and on this very post.

In C#:

class Program
{
    static void Main(string[] args)
    {
        var inputA = Console.ReadLine();
        var inputB = Console.ReadLine();

        char[] delimiter = { ' ', ',', '-' };
        string[] inputASplit = inputA.Split(delimiter, StringSplitOptions.RemoveEmptyEntries);
        string[] inputBSplit = inputB.Split(delimiter, StringSplitOptions.RemoveEmptyEntries);

        int[] listA = new int[inputASplit.Length];
        int[] listB = new int[inputBSplit.Length];

        for (int i = 0; i < inputASplit.Length; i++)
        {
            listA[i] = int.Parse(inputASplit[i]);
        }

        for (int i = 0; i < inputBSplit.Length; i++)
        {
            listB[i] = int.Parse(inputBSplit[i]);
        }

        int[] concatinatedList = MergeSort(listA, listB);

        foreach (var number in concatinatedList)
        {
            Console.WriteLine(number);
        }

        Console.ReadKey();
    }

    static int[] ParseListA(int[] ListA)
    {
        List<int> newListA = ListA.ToList();
        newListA.Sort();

        return newListA.ToArray();

    }

    static int CountZeros(int[] ListB)
    {
        int zeroCount = 0;

        for (int i = 0; i < ListB.Length; i++)
        {
            if (ListB[i] == 0)
            {
                zeroCount += 1;
            }
        }

        if (zeroCount == 0)
        {
            throw new Exception("List B doesn't contain any Zeroes.");
        }

        return zeroCount;
    }

    static int[] MergeSort(int[] listA, int[] listB)
    {
        int[] newListA = ParseListA(listA);
        int zeroCount = CountZeros(listB);
        int[] newListB = new int[listB.Length - zeroCount];

        if (newListA.Length != zeroCount)
        {
            throw new Exception("There weren't enough buffers!");
        }

        for (int i = 0; i < listB.Length; i++)
        {
            if (listB[i] != 0)
            {
                for (int j = 0; j < newListB.Length; j++)
                {
                    newListB[j] = listB[i];
                }
            }
        }

        int[] concatenatedList = newListA.Concat(newListB).ToArray();
        Array.Sort(concatenatedList);

        return concatenatedList;
    }

In Python:

def ParseListA(listA):
    newListA = sorted(listA);

    return newListA;

def MergeSort(listA, listB):
    newListA = ParseListA(listA);
    zeroCount = 0;

    for number in listB:
        if number == 0:
            zeroCount += 1;

    if zeroCount == 0:
        raise ValueError("Needs zeros!");
        return;

    newlistB = [i for i in listB if i != 0];

    return sorted(newListA+newlistB);

def Main():
    listAInput = input("Enter List A: ").split(' ');
    listBInput = input("Enter List B: ").split(' ');

    listA = [int(i) for i in listAInput];
    listB = [int(i) for i in listBInput];

    print(MergeSort(listA, listB));

Main()

Python doesn't use semicolons at the end of a line.
The function ParseListA does nothing that the built-in function sorted doesn't do already.  Remove it.
In MergeSort, you can use the built-in function any to make sure you have at least one zero in ListB, like this:

if not any(i==0 for i in listB):
    raise ValueError("Needs zeros")

These two lines replace all the code beginning with zeroCount=0 and ending with return;.  The return statement is, incidentally, unreachable code since it follows a raise statement.
At the end of MergeSort, you concatenate the two lists and then sort them.  If you are going to do that, the initial sort of listA is redundant.

When all is said and done, the function ParseListA is gone and MergeSort is only four lines long.  Only one sort operation is required:

def MergeSort(listA, listB):
    if not any(i==0 for i in listB):
        raise ValueError("Needs zeros")
    newB = [i for i in listB if i != 0]
    return sorted(listA + newB)


Sort a list of objects to put parents first I have a class which contains a reference to a parent of the same type( or null if it has no parent).

class MyObject{
    private MyObject parent;
    public MyObject getParent(){ return parent};
    …
}

I have a list of MyObject, and need to order it so that every object comes after its parent in the list (the parent will always be in the list).

void orderList(List<MyObject> toSort){
    List<MyObject> tmp = new ArrayList<>();
    while(!toSort.isEmpty()) {
        for(MyObject obj: toSort){
            if(obj.getParent() == null) {
                tmp.add(obj);
            }else {
                if(tmp.contains(obj.getParent())) {
                    tmp.add(obj);
                }
            }
        }
        toSort.removeAll(tmp);
    }
    toSort.addAll(tmp);
}

Is there a way to make this function more efficient and readable ?

You can make this faster quite a bit (in complexity terms) by precomputing which objects have which parent and then successively building your list instead of iterating all objects with pure chance. Your complexity in the worst case right now is \$O(n^3)\$. (n^2 for the nested loops, another n for the contains)

Consider the following:

void orderList(List<MyObject> toSort) {
    final MyObject nullPlaceholder = new MyObject();
    Map<MyObject, List<MyObject>> byParent = toSort.stream()
         .collect(Collectors.groupingBy(obj ->
           (obj.getParent() == null ? nullPlaceholder : obj.getParent())
           , Collectors.toList()));

    List<MyObject> ordered = new ArrayList<>();
    Queue<MyObject> processor = new LinkedList<>();
    byParent.get(nullPlaceholder).forEach(processor::add);
    while (!processor.isEmpty()) {
        MyObject tmp = processor.poll();
        byParent.getOrDefault(tmp, Collections.emptyList())
          .forEach(processor::add);
        ordered.add(tmp);
    }
    toSort.clear();
    toSort.addAll(ordered);
}

Notes:

You menitioned in comments, that null-keys are not supported in the groupingBy collector. This code works around by creating a "null object". more info at wikipedia
getOrDefault works around the fact that the last MyObjects won't have "children", meaning that they don't have elements in the map. With standard get that would return null. You could manually check, but that breaks the flow IMO, and I prefer it like this :)

This should boil down to a runtime complexity of \$O(n)\$, which should give you significant performance boosts for larger lists.

In addition to that I dislike reusing references you receive as input. I personally prefer to use value-returning methods instead of modifying references. As such I'd adjust your method to return a List<MyObject> and return ordered; at the end.


Tracking hours worked for the days of the week I am working through a udemy course on Python, this is the first real program I've written. I am new to Python and to programming.

I'd like to replace the main function below with something more efficient and less repetitive but I'm not sure what or how. The most I could think to do is write a function for adding hours and call it under each day.

Most relevant code:

def main(day):

    if day == "Monday":
            print "Today is %s, How many hours would you like to add today?" % (day)
            hours = int(raw_input())
            DOTW['Monday'] = DOTW['Monday'] + hours
            print "Cool your hours for %s are %d" %(day, DOTW['Monday'])
            want = raw_input("Do you want to add more time? y/n \n > ")
            overtime()
            leave(want)

    if day == "Tuesday":
            print "Ah, Today is %s, How many hours have you worked?" %(day)
            hours = int(raw_input())
            DOTW['Tuesday'] = DOTW['Tuesday'] + hours
            print "alright, yours hours for %s are %d" %(day, DOTW['Tuesday'])
            want = raw_input("Do you want to add more time? y/n \n > ")
            overtime()
            leave(want)

    if day == "Wednesday":
            print "Ah, Today is %s, How many hours have you worked?" %(day)
            hours = int(raw_input())
            DOTW['Wednesday'] = DOTW['Wednesday'] + hours
            print "Cool for %s I've got %d hours" %(day, DOTW['Wednesday'])
            want = raw_input("Do you want to add more time? y/n \n > ")
            overtime()
            leave(want)

    if day == "Thursday":
            print "Ah, Today is %s, How many hours have you worked?" %(day)
            hours = int(raw_input())
            DOTW['Thursday'] = DOTW['Thursday'] + hours
            print "Your hours for %s are %d" %(day, DOTW['Thursday'])
            want = raw_input("Do you want to add more time? y/n \n > ")
            overtime()
            leave(want)

    if day == "Friday":
            print "FINALLY IT'S FRIDAY!!! How many hours have you worked?"
            hours = int(raw_input())
            DOTW['Friday'] = DOTW['Firday'] + hours
            print "Cool. for %s I've got %d hours" %(day, DOTW['Friday'])
            want = raw_input("Do you want to add more time? y/n \n > ")
            overtime()
            leave(want)

    if day == "Saturday":
            print "Ah, Today is %s, How many hours have you worked?" %(day)
            hours = int(raw_input())
            DOTW['Saturday'] = DOTW['Saturday'] + hours
            print "alright, yours hours for %s are %d" %(day, DOTW['Saturday'])
            want = raw_input("Do you want to add more time? y/n \n > ")
            overtime()
            leave(want)

    if day =="Sunday":
            print "Ah, Today is %s, How many hours have you worked?" %(day)
            hours = int(raw_input())
            DOTW['Sunday'] = DOTW['Sunday'] + hours
            print "Cool your hours for %s are %d" %(day, DOTW['Monday'])
            want = raw_input("Do you want to add more time? y/n \n > ")
            overtime()
            leave(want)

The entire program: http://pastebin.com/8hLVcGMr

The first step to reducing repetition is to use variables instead of literals. Why call DOTW['Monday'] when you could just use day as your key. Importantly, this will avoid errors like the one you created in the Sunday block, where you actually display Monday's hours instead of Sunday's.

The only difference between days is in the messages that you print to the user. You've given custom messages, but you could just put those in a dictionary and access them the same way you access DOTW. Here's an example of how the greeting could look:

today_messages = {
                "Monday": "Today is %s, How many hours would you like to add today?",
                "Tuesday": "Ah, Today is %s, How many hours have you worked?",
                "Wednesday": "Ah, Today is %s, How many hours have you worked?",
                "Thursday": "Ah, Today is %s, How many hours have you worked?",
                "Friday": "FINALLY IT'S FRIDAY!!! How many hours have you worked?",
                "Saturday": "Ah, Today is %s, How many hours have you worked?",
                "Sunday": "Ah, Today is %s, How many hours have you worked?",
                 }

With those changes you could make it all one block with just one test because Friday's message is an exception that doesn't need the string formatting argument.

def main(day):
    if day == "Friday":
        print today_messages[day]
    else:
        print today_messages[day] % day

    hours = int(raw_input())
    DOTW[day] = DOTW[day] + hours
    print hours_worked[day] %(day, DOTW[day])
    want = raw_input("Do you want to add more time? y/n \n > ")
    overtime()
    leave(want)

Also you should use the newer str.format method. Using % is the old way, and str.format has a lot of useful formatting options. You don't particularly need them in your case but it's clearer and cleaner syntax. The basic way to use it is like this:

                "Sunday": "Ah, Today is {}, How many hours have you worked?",

...

print today_messages[day].format(day)

The particular advantage for you here is that formatting a message that takes no arguments won't raise errors, unlike with %.

>>> string = "Ignore me"
>>> "Hello World!" % string

Traceback (most recent call last):
  File "<pyshell#2>", line 1, in <module>
    "Hello World!" % string
TypeError: not all arguments converted during string formatting
>>> "Hello World!".format(string)
'Hello World!'

This means you no longer need to test for "Friday", since the format call will just be ignore for that string:

def main(day):
    print today_messages[day].format(day)
    hours = int(raw_input())
    DOTW[day] = DOTW[day] + hours
    print hours_worked[day].format(day, DOTW[day])

    want = raw_input("Do you want to add more time? y/n \n > ")
    overtime()
    leave(want)


Reading a data file for performing a FFT I'm a bit concerned about function usage. There are so many different libraries/possible ways to do something that I'm not sure if the way I'm working at the moment is reasonable, or what I could modify to have a better code.

Also, I'm searching on how to optimize this code, since for a small 25MB file the memory use goes to 800MB, and a bigger file goes OOM. What makes the memory to go so high?

Before I had strict IO writing, and was expecting to be it that caused to evaluate everything on the code, but going lazy didn't solve anything.

{-# LANGUAGE OverloadedStrings,BangPatterns #-}
import qualified Data.Attoparsec.Char8 as Ap
import Data.Attoparsec

import Control.Monad

import System.IO
import System.Environment
import Data.Maybe
import Data.List (unzip4,zip4,foldl')
import Data.List.Split
import Data.Bits

import Numeric.FFT
--import Data.Vector.Storable (fromList)

import Data.Complex
import Data.String (fromString)
import Data.ByteString.Internal
import qualified Data.ByteString.Char8 as B
import qualified Data.ByteString.Lazy.Char8 as Bl

newtype FourD b a = FourD  { getFourD  :: (a,a,a,b) }
instance (Show b, Show a) => Show (FourD b a) where
    show (FourD (a,b,c,d)) = "( "++show a++" , "++show b++" , "++show c++" , "++show d++" )"
instance Functor (FourD c) where
    fmap f (FourD  (x,y,z,d)) = FourD  (f x,f y,f z,d)

mgrav_per_bit = [ 18, 36, 71, 143, 286, 571, 1142 ]

aToG a = sign * uresult
    where   twocomp = if a>128
                    then 256-a
                    else a
            uresult = foldr (+) 0  $ zipWith (*)   mgrav_per_bit (map ( g . testBit  twocomp) [0..7] )
            sign = if a>128
                    then -1
                    else 1
            g True = 1; g False = 0

amap f g = map FourD . uncurry4 zip4 . map4 f g . unzip4 . map getFourD
    where map4 f g (a,b,c,d) = ( f a,f  b,f  c, g d)
          uncurry4 f (a,b,c,d) = f a b c d

splitN n x =  helper x 0
    where
    l = length x
    helper x c = if (c+n)>l
                    then []
                    else (Prelude.take n x):(helper  (drop 1 x) (c + 1))

quare = foldr (\a b-> b+(magnitude a)) 0

getTime :: (FourD String a)-> String
getTime (FourD (_,_,_,t)) = t

filterAcc (FourD (x,_,_,t)) = if x > 50
                                then  (Bl.pack . B.unpack) $ "yes" `B.append` t
                                else  Bl.pack ""

parseAcc :: Parser (FourD B.ByteString Int)
parseAcc = do   satisfy (== 40)
                x <- Ap.decimal
                string ","
                y <- Ap.decimal
                string ","
                z <- Ap.decimal
                string ","
                time <-  takeTill (== 41)
                return $ FourD (x,y,z,time)
                where isDigit w = w >= 48 && w <= 57
                      isNot w = w/=29

readExpr input = case parse parseAcc  input of
     Done b val -> val
     Partial p-> undefined
     Fail a b c -> undefined

fftAcross = map ( map (/32) . fft ) . splitN 32

main = do

    filename <- liftM head getArgs
    filehandle <- openFile "lol.txt" WriteMode
    contents <- liftM B.lines $ B.readFile filename
    Bl.hPutStr (filehandle) . Bl.concat  . map (filterAcc) . splitAndApply . convertComplex $ contents where
        convertComplex = map ( fmap( fromIntegral . aToG). readExpr )
        splitAndApply  = (amap (map (floor .quare .(drop 1) ). fftAcross) ((map  head) . splitN 32))

   -- B.hPutStrLn (filehandle)  . B.unlines . map (B.pack . show ) .  amap (map (floor .quare) .  (filter (/=[])) . map ( (drop 1) . (map (/32)) . fft ) . splitN 32) . map ( fmap(fromIntegral . aToG)) . map readExpr $ contents

You can use HLint tool to get rid of few extra parentheses, and use <$> instead of liftM:

contents <- B.lines <$> B.readFile filename

Other than that I don't see anything to be improved in your code at low level. To help with high level design there's not enough info in your question. Can you explain what do you want to achieve with your code?


golang rabbitmq message consumer I need to process rabbitmq messages with golang with worker style,
is this correct way to process rabbitmq messages with golang?

package main

import (
    "log"
    "github.com/streadway/amqp"
)

func main() {
    conn, err := amqp.Dial("amqp://guest:guest@localhost:5672/")
    if err != nil {
        panic(err)
    }
    defer conn.Close()

    for _, q := range []string{"q1", "q2", "q3"} {
        go RunFetcher(q, conn, 3, 50)

    }

    select {}
}

func RunFetcher(queueName string, conn *amqp.Connection, workers, qos int) {
    ch, err := conn.Channel()
    if err != nil {
        log.Println(err.Error())
        return
    }

    ch.Qos(qos, 0, false)
    defer ch.Close()
    msgs, err := ch.Consume(queueName, "", false, false, false, false, nil)
    if err != nil {
        log.Println(err.Error())
        return
    }

    for index := 0; index < workers; index++ {
        go func() {
            for d := range msgs {
                // process message
                d.Ack(false)
            }
        }()
    }

    select {}
}

There's not really a one "right" way of doing things, and all of the others are wrong. However, there's a number of things that are considered good practice, and WRT to those, your code can do with a bit of TLC.

To that end, I'll just run through your code line by line, leaving comments on both style, and things I deem to be missing, hopefully ending up with something that is more "idiomatic" go.

package main

import (
    "log"
    "github.com/streadway/amqp"
)

Yup, I'm going to comment on your imports. The standard way to organise the imports in golang is to have multiple sections (separated by a blank line). The order is: first standard packages, second group have packages local to the project, then a third group with your external packages. In this case, the amqp package most certainly is an external one, so I'd write:

import (
    "context"
    "log"

    "github.com/streadway/amqp"
)

As you may have notices, I've also added the context package. It's a good idea to look into this one, especially once you start playing around with multiple routines. More on that later

func main() {
    conn, err := amqp.Dial("amqp://guest:guest@localhost:5672/")
    if err != nil {
        panic(err)
    }
    defer conn.Close()

Ok, something you've probably heard before is "Don't panic". Maybe you know the phrase from the hitchhikers guide to the galaxy, but it's also one of the go proverbs. Just do a log.Fatalf("failed to connect to AMQP: %+v", err) or something. Fatal log is basically logging the output, and calling os.Exit(1).

    for _, q := range []string{"q1", "q2", "q3"} {
        go RunFetcher(q, conn, 3, 50)

    }

    select {}
}

The remainder of your main function just looks really weird. You seem to know what queues you want to consume from, but you've not given yourself any way to control the execution of the routines. Something you really would want is a way to stop the routines dead in their tracks. For example: the worker receives a kill signal, what you really want to do is cleanly terminate the workers. That's why I added the context package, because this allows you to do just that:

ctx, cfunc := context.WithCancel(context.Background())
defer cfunc() // cancel execution context when the main function returns
for _, q := range queueNames {
    go runFetcher(ctx, q, 3, 50) // no need to export this func
}

Now let's add the bit that listens for a kill/interrupt signal:

import (
    "os"
    "os/signal"
)

func main() {
    // add this:
    sch := make(chan os.Signal, 1)
    // listen for interrupt and kill signals
    signal.Notify(sch, os.Interrupt, os.Kill)

    // after you've done everything you needed to do:
    <-sch // this is blocking, so after the read, the context will be automatically cancelled thanks to the defer cfunc() we added
}

Now your worker can run indefinitely, but cleans up after itself once it receives an interrupt or kill signal. Also: we don't need the empty select {}, which looks really nasty to my eye...

func RunFetcher(queueName string, conn *amqp.Connection, workers, qos int) {
    ch, err := conn.Channel()
    if err != nil {
        log.Println(err.Error())
        return
    }

    ch.Qos(qos, 0, false)
    defer ch.Close()
    msgs, err := ch.Consume(queueName, "", false, false, false, false, nil)
    if err != nil {
        log.Println(err.Error())
        return
    }

    for index := 0; index < workers; index++ {
        go func() {
            for d := range msgs {
                // process message
                d.Ack(false)
            }
        }()
    }

    select {}
}

Ok, this is a big one to unpack. First off, let's not export this function (there's no reason to), and add the context argument as first argument to the function. Something I can't quite wrap my head around is why this function is kicked off in a routine, and doesn't even have a method of communicating any of the errors that it may encounter (for example in conn.Channel()). An error here causes an early return. To me, that error looks like something that needs to be handled. What's more, within this routine, you're actually spawning 3 subroutines that actually read from the same channel. That's fine, but it's actually this part you want to have running asynchronously. Why not change the runFetcher function to a normal function that returns an error, so you can handle it in main, and have it spawn as many routines as needed? You could also simply call conn.Channel() in the main function, and pass the Channel to the fetcher.

I'd also consider adding a loop so your fetchers can run indefinitely:

func runFetcher(ctx context.Context, queueName string, conn *amqp.Connection, workers, qos int) error {
    ch, err := conn.Channel()
    if err != nil {
        return err
    }
    // the usual stuff, but:
    msgs, err := ch.Consume(queueName, uuid, false, false, false, false, nil)

I would use a UUID package to create a unique consumer name, so we can actually stop our consumers correctly:

    // create your workers like so:
    for i := 0; i < workers; i++ {
        go func() {
            select {
            case <-ctx.Done():
                return // the context was cancelled, stop working
            case msg := <- msgs:
               msg.Ack(false) // acknowledge (or not)
            }
        }()
    }
    // now let's add this to stop the consumer
    go func() {
        <-ctx.Done()
        ch.Cancel(uuid, false) // stop consumer quickly
    }()
}

Now we're able to stop the consumers from actually getting messages from the queue, we can stop all our routines cleanly, and errors when establishing a consumer channel are actually propagated to the main function, so we can decide whether or not we want to proceed consuming messages from other queues or not.

These are some initial thoughts after quickly reading through the code. I might revisit this later on with more comments.


Count the word frequencies within a given directory I have built a tiny script count the word frequencies of all text files with a specified directory. I would be glad to receive any suggestions and feedback.

import re, glob,os
from collections import OrderedDict, Counter

class CountWords:
    def __init__(self,dir,ext):
        self.dir = dir
        self.ext = ext

    @property
    def files(self):
        #collect all the files with `.txt` extension
        os.chdir(self.dir)
        return glob.glob(f"**/*.{self.ext}", recursive=True)
    @property
    def contents(self):
        #concatenate files
        cat_content = []
        for file in self.files:
            with open(file, 'r') as f:
                content = f.read()
                cat_content.append(content)
        contents = ",".join(cat_content)
        return contents
    @property
    def words(self):
        words = re.findall(r"[a-zA-z_]+", self.contents)
        words = [word.lower() for word in words]
        return words

    def count(self):
        words_counter = Counter(self.words)
        words_counter = OrderedDict(sorted(words_counter.items(),key=lambda t:t[1], reverse=True))
        return words_counter

The function version:

def count_words(dir,ext):
    os.chdir(dir)
    #collect all the files with `.txt` extension
    files = glob.glob(f"**/*.ext", recursive=True)
    #concatenate files
    cat_content = []
    for file in files:
        with open(file, 'r') as f:
            content = f.read()
            cat_content.append(content)
    contents = ",".join(cat_content)
    #extract the words
    words = re.findall(r"[a-zA-z_]+", contents)
    words = [word.lower() for word in words]
    words_counter = Counter(words)
    words_counter = OrderedDict(sorted(words_counter.items(),key=lambda t:t[1], reverse=True))
    return words_counter

#!/usr/bin/python3
import os
import re
from os.path import join
from collections import Counter, OrderedDict

def count_words(directory, ext): # don't use the name dir, it's a builtin function
    """Counts word frequencies in a directory of files.

    Keyword arguments:
    directory -- count_words will search this directory recursively
    ext -- the extension of files that you wish to count

    Returns an OrderedDict, from most to least frequent.

    (You don't strictly need a docstring, but it's a good habit to get into.
    See PEP 257.)"""

    # Initialize the counter
    word_counter = Counter()

    # Personally I like to break my code into small, simple functions
    # This code could be inline in the loop below,
    # but I think it's a bit clearer this way.
    def update_counter(word_counter, filename):
        '''updates word_counter with all the words in open(filename)'''
        with open(filename, 'r') as f:
            try:
                # Two things:
                # First, I'm updating counter here,
                # instead of creating an enormous string.
                # Each file string only exists long enough for me to search it.

                # Second and less important,
                # I chose to lower the full text first, to make the regex simpler.
                word_counter.update(re.findall('[a-z_]+', f.read().lower()))
            except UnicodeDecodeError:
                # In python3 if your file isn't UTF-8
                # then read() throws an exception.
                # Maybe you want this behavior,
                # but I'd rather just warn and continue.
                print("Warning: couldn't decode", filename)

    # Using os.walk instead of glob
    for root, dirs, files in os.walk(directory):
        for fname in files:
            if fname.endswith(ext):
                update_counter(word_counter, join(root, fname))

    # words_counter.most_common() does exactly the sort you are looking for
    return OrderedDict(word_counter.most_common())


Optimizing a search for list of artists in iTunes library I'm trying to get a list of all artists on an iPhone / iPod touch music library. The best way I've found has been to group by artists (the convenience constructor on MPMediaQuery) and to iterate over the collections, being one for each artist. That way I've optimized out having to use an NSSet to ensure uniqueness in the artist list. Still, I need to make each collection into a representative item and get it's artist, which according to Time Profiler takes a very good part of this code snippet.

NSString       *artistKey    = [MPMediaItem titlePropertyForGroupingType:MPMediaGroupingArtist];
MPMediaQuery   *artistsQuery = [MPMediaQuery artistsQuery];
NSMutableArray *artists      = [NSMutableArray arrayWithCapacity:artistsQuery.collections.count];

for (MPMediaItemCollection *album in artistsQuery.collections) {
    MPMediaItem *albumItem = [album representativeItem];
    [artists addObject:[albumItem valueForProperty:artistKey]];
}

Wrapping it in some timing blocks, such as below, I've gotten it to run in 0.315 seconds on my iPhone 4 with a library of ~2600 songs and 88 artists, but I feel like that long for only 88 items is way too long.

NSTimeInterval start  = [[NSDate date] timeIntervalSince1970];
// [snip]
NSTimeInterval finish = [[NSDate date] timeIntervalSince1970];
NSLog(@"Execution took %f seconds.", finish - start]);

Does anyone know of a way to make this a bit faster?

Is it actually necessary to add just the artist to the array?  We could simply do this:

NSMutableArray *artists = artistsQuery.collections;

Now we don't have to iterate through an entire array.  And later, when we need to access the artist at a particular array index, we can still get to that value as such:

[[artists[someIndex] representativeItem] valueForProperty:artistKey];

Also, this SO answer seems to suggest that valueForProperty: is just slow in general.  Despite only needing the artist, it might be better to enumerate through all the properties:

[artistsQuery.collections enumerateObjectsUsingBlock:^(id obj, NSUInteger idx, BOOL *stop) {
    MPMediaItem *item = (MPMediaItem *)obj;
    [item enumerateValuesForProperties:properties usingBlock:^(NSString *property, id value, BOOL *stop) {
        if ([property isEqualToString:artistKey]) {
            [artists addObject:value];
        }
    }];
}];

At first glance it may seem like this will automatically be slower since you're iterating through every property instead of just accessing the one you need, and in this case, since you only need a single property, this may actually be slower.  But if you need multiple properties, this should definitely be faster--enumerations like this (as well as forin loops) are handled in batches so they run quite fast.


Bash script to temporarily run a shell without certain group memberships I have here a script called rwog (run without groups) that allow a user to run a shell without particular group memberships.  rwog, in more detail...

Is primarily for a support staffer to pretend to be an ordinary user without  su-ing to anybody.
Must be run as root, but those allowed to run it will be given an entry in /etc/sudoers.
Should be executed as sudo rwog groups to drop.
Will have a uid of root, a gid of the_support_team, and permissions 0550 (i.e. user and group can read and execute, world can do nothing, and no setgid/setuid bits).
Cannot change your gid, change your uid, or add yourself to supplementary groups.
Revokes pretty much all capabilities, though this may be a tad zealous.
Does not modify /etc/group (so id will be changed, but id $USER will not).
Is primarily (but not exclusively) intended to run on a Centos 7 environment..

My main concern: Can this script be exploited to gain escalated privileges?
 I won't ignore suggestions not related to security.

The Script

#!/bin/bash

function help(){
    echo "rwog - run without groups";
    echo "Runs a shell as if you weren't in certain supplementary groups."
    echo "Good for pretending that you're not a support user."
    echo "Usage example:";
    echo -e "\trwog [(-h|--help)] [group ...]";
    echo "Options:";
    echo -e "\t-h or --help: Displays this information.";
    exit 1;
}

# Declare vars. Flags initalizing to 0.

# Execute getopt
ARGS=$(getopt -o "h" -l "help" -n "rwog" -- "$@");

#Bad arguments
if [ $? -ne 0 ];
then
    help;
fi

eval set -- "$ARGS";

while true; do
    case "$1" in
        -h|--help)
            shift;
            help;
            ;;

        --)
            shift;
            break;
            ;;
    esac
done

if [[ $(id -u) != 0 ]]; then
    # If you're not root...
    echo "You must be root to use this script. Run it with sudo."
    exit 1
fi

for group in $@; do
    # For each group we want to drop...

    if [[ "$group" == "$SUDO_USER" ]]; then
        # If you're trying to drop your own gid...
        echo "You cannot drop your own gid."
        exit 1
    elif ! getent group "$group" &>/dev/null ; then
        #If this group doesn't exist...
        echo "$group is not a valid group."
        exit 1
    elif  ! groups "$SUDO_USER" | grep -E &>/dev/null "\b$group\b"; then
        # Else if you're not actually a member of this group...
        echo "Not a member of $group, cannot drop it"
        exit 1
    fi
done

my_groups=$(id -Gn "$SUDO_USER" | xargs -n1 | sort -u)
groups_to_drop=$(echo $@ | xargs -n1 | sort -u)
# Put the groups that you want to drop on multiple lines, then sort them

if [[ -z "$groups_to_drop" ]]; then
    # If you didn't pick a group to drop...
    echo "Please specify at least one supplementary group to drop."
    help
fi

reduced_groups=$(comm -13 <(echo "$groups_to_drop") <(echo "$my_groups") | paste -s -d,)
# Subtract the groups we want to drop from the groups we're in, then merge them onto one line

export USERNAME="$SUDO_USER"
export USER="$SUDO_USER"
export LOGNAME="$SUDO_USER"
export HOME=$( getent passwd "$SUDO_USER" | cut -d: -f6 )

dropped_capabilities=$(capsh --print | grep -E "Bounding set =(.+)" | sed "s/Bounding set =//g")

capsh --secbits=0xf --drop="$dropped_capabilities" --groups="$reduced_groups" --gid="$SUDO_GID" --uid="$SUDO_UID" -- --login

It's not an error to ask for help

When -h is passed, the help() function shouldn't exit with non-zero status, because it's done what was asked for.

Conversely, when we pass no arguments, we should exit with non-zero, and we should write the message to the standard error stream.

I'd write that as

help() {
    local tab=$'\t'
    cat <<END
$0 - run without groups
Runs a shell as if you weren't in certain supplementary groups.
Good for pretending that you're not a support user.
Usage example:
${tab}$0 [(-h|--help)] [group ...]
Options:
${tab}-h or --help: Displays this information.
END
}

And then use it as

# Just use the command directly instead of testing $?
if ! ARGS=$(getopt -o "h" -l "help" -n "rwog" -- "$@");
then
    help >&2
    exit 1
fi

and

    case "$1" in
        -h|--help)
            help
            exit 0
            ;;

Elsewhere in the code, diagnostic messages are printed to standard output; these should all be redirected to &2.

Checking more exit statuses

We depend on tools such as xargs and sort that might not be present.  Consider letting Bash check some statuses, but make sure you know which commands this will not check:

set -e

Also consider getting Bash to check that you're not using unset variables:

set -u

You'll want to use expansions such as ${foo:-} when you need to avoid this checking.

Quote your variable expansions

Just because you've checked that this is being run by root (perhaps overkill - is it sufficient to be any user that can run capsh?), doesn't mean that they can't do something unexpected.  So write "$@", not $@:

for group in "$@"; do

Don't compare a group against a user

if [[ "$group" == "$SUDO_USER" ]]; then

Did you mean "$SUDO_GID" there?  Also, there's little point using both [[...]] and quotes - I'd stick to the portable [...] here.

If you did mean to compare against the user name (e.g. you know your configuration always creates a per-user group sharing the same name), then it's a good idea to add a comment documenting that assumption, as it might not be valid on another system you copy this to.

No need to grep and sed in a pipeline

Sed is perfectly capable of selecting lines:

dropped_capabilities=$(capsh --print | sed -n -e 's/Bounding set =//gp')

Take care with $PATH

Even though your site's sudo policy should constrain PATH, it may be a good idea to set a specific PATH at the start of the script and/or use fully-qualified pathnames for executables.


Improving efficiency for finding longest contiguous non-decreasing substring I was working on a problem set and I came across this problem:

Assume s is a string of lower case characters.
Write a program that prints the longest substring of s in which the letters occur in alphabetical order. For example, if s = 'azcbobobegghakl', then your program should print:
Longest substring in alphabetical order is: beggh

In the case of ties, print the first substring. For example, if s = 'abcbcd', then your program should print
Longest substring in alphabetical order is: abc

I actually was able to get a solution, but it was the most inefficient ever! And here's what I came up with:
def test():
    index = 1
    prev_index = 0
    count = 0
    global largest
    largest = ''
    test = s[prev_index]

    while count < len(s):
        if ord(s[index]) > ord(s[prev_index]):
            test += s[index]
            index += 1
            prev_index += 1
        elif ord(s[index]) == ord(s[prev_index]):
            test += s[index]
            index += 1
            prev_index += 1
        else:
            if len(largest) < len(test):
                largest = test[:]
            test = s[index]
            prev_index += 1
            index += 1
        count += 1
    return largest
try:
     test()
except IndexError:
     pass
finally:
     print largest

Can anyone give me an example of better code that produces the same output?

Your algorithm is basically fine, but it is expressed in the code clumsily.

The biggest problem is the interface of the function: it should accept a string parameter and return a string.  Instead, your function takes s from its environment and uses the global variable largest to store the answer.  Although there is a return largest statement, it is very misleading.  What actually happens is that s[index] raises an IndexError, so that the return largest is never reached, and the answer is passed back using the global variable instead.

The root cause for the IndexError is that there are too many variables, causing confusion.  With each iteration through the loop, you increment index, prev_index, and count.  That means that those three variables could be reduced to one, which could just be called i.  (If you think about it, you'll find that count is always one less than index.  Since the loop condition is count < len(s), s[index] will go one position beyond the end of the string.)

The next observation to make is that your if and elif blocks are almost identical.  You can combine them by using >= for the test.

Python strings are immutable, so largest = test[:] could just be largest = test.

Putting everything together…

def longest_nondecreasing_substring(s):
    if s is None or len(s) <= 1:
        return s

    longest = test = s[0]

    for i in range(1, len(s)):
        if ord(s[i]) >= ord(s[i - 1]):
            test += s[i]
        else:
            if len(longest) < len(test):
                longest = test
            test = s[i]
    if len(longest) < len(test):
        longest = test
    return longest


Simple Spring Service code I used Spring MVC arch, and I need to be informed about correct names of methods and exceptions throwing.

/**
 * @author Eugene G. Ustimenko
 * @date Jan 19, 2015
 */
@Service
@Qualifier ("entityService")
public class BaseEntityService < T extends BaseEntity > implements     IBaseEntityService < T > {

  @Autowired
  private IBaseEntityDao<T> dao;

  @Transactional
  @Override
  public T create (T entity) throws ServiceException {

    try {
      return dao.create(entity);
    }
    catch (final Exception ex) {
      throw new ServiceException("Create entity operation error " +     ex.getMessage(), ex);
    }
  }

  @Transactional
  @Override
  public T update (T entity) throws ServiceException {

    try {
      return dao.update(entity);
    }
    catch (final Exception ex) {
      throw new ServiceException("Update entity operation error " +     ex.getMessage(), ex);
    }
  }

  @Transactional
  @Override
  public void remove (Long id) throws ServiceException {

    try {
      dao.remove(id);
    }
    catch (final Exception ex) {
      throw new ServiceException("Remove entity operation error " +     ex.getMessage(), ex);
    }
  }

  @Override
  public List < T > getAll () throws ServiceException {

    try {
      return dao.getAll();
    }
    catch (final Exception ex) {
      throw new ServiceException("Get list of entities operation error "     + ex.getMessage(), ex);
    }
  }

  @Override
  public T get (Long id) throws ServiceException {

    try {
      return dao.get(id);
    }
    catch (final Exception ex) {
      throw new ServiceException("Get entity by identifier operation error" + ex.getMessage(), ex);
    }
  }

}

If you really, really want to catch an unchecked exception that might be thrown when using a database, you can catch DataAccessException which is the higher exception on Spring's data translation exception hierarchy.

For correct method names, there no such thing on Spring, just use a name which can clearly describe what your method does, your current names are pretty good too. One more note, use the Service layer to aggregate data or do complicated business logic, and place simple logics on the domain, to avoid the Anemic Domain Model

You can find more info here about the last statement around Spring


Function outcome: consistent approach and easy logging My goal is to make a consistent and easy way to determine how a function fails/succeeds. I'm looking for code review of my Status class for any bad practices, code that isn't thread-safe and general improvements and changes.

For logging and debugging I always needed to know how a function failed and print this out as readable text. The solution I usually employed was using enumerations - enum ERROR_CODE {..}; - which is fine except they cannot be expanded/overridden - I'm making an API so enums don't allow the user to expand/add custom error codes. They also don't print out nicely to describe what error actually occurred - they just print out as numbers. So I made a status class.

Main features:

Allows creation of error/states wherever & whenever (runtime, compile time):

static const Status S_SUCCESS         = CStatus::registerState("Success");
static const Status S_EMPTY_CONTAINER = CStatus::registerState("Empty rectangle vectors");

General usage:

Status unionRectangles(const vector<Rect>& left, vector<Rect>& right)
{
  if (left.size() <= 0 || right.size() <= 0)
     return S_EMPTY_CONTAINER;

  ...

  return S_SUCCESS;
}

Print out descriptive errors without requiring conversion:

vector<Rect> v1, v2;
Status myStatus = unionRects(v1, v2)
cout << myStatus << end; // "Empty rectangle vectors"

Easily and efficiently compare objects without conversion:

if (myStatus == S_SUCCESS) // for eg; resolves to if (1 == 9), ie, not string comparison
    // do something

Easy to inline check error:

if (!unionRects(v1, v2))
    // do something

Full implementation:

#ifndef STATUS_H
#define STATUS_H

#include <iostream>
#include <hash_map>

typedef int STATE;
typedef std::hash_map<STATE, tstring>::iterator StateIterator;
typedef class CStatus;
typedef const CStatus Status;

#ifndef S_SUCCESS
#define S_SUCCESS CStatus::CS_SUCCESS
#endif // S_SUCCESS

#ifndef S_UNDEFINED_ERROR
#define S_UNDEFINED_ERROR CStatus::CS_UNDEFINED_ERROR
#endif // S_SUCCESS

class CStatus
{
public:
    // Static Properties//
    static Status CS_SUCCESS;
    static Status CS_UNDEFINED_ERROR;

    // Static Functions //
    static Status registerState(const tstring &stateMsg)
    {
        // Post:

        int nextStateTmp = nextState + 1;
        std::pair<StateIterator, BOOL> res = states.emplace(std::make_pair(nextStateTmp, stateMsg));

        // if state has been inserted
        if (res.second)
            return Status(++nextState);

        // return existing state id
        return Status(res.first->first);
    }

    // Properties //
    const STATE state;

    // Functions //
    CStatus(const STATE &state)  : state(state) {}
    virtual ~CStatus() {}

    const tstring& toString() const
    {
        return (states.find(state) == states.end()) ? states[state] : _T("Undefined");
    }

    operator tstring() const
    {
        return toString();
    }

    operator int() const
    {
        return state;
    }

    bool operator==(const CStatus& obj) const
    {
        return state == obj.state;
    }

    bool operator!=(const CStatus& obj) const
    {
        return state != obj.state;
    }

    bool operator!() const
    {
        return *this != S_SUCCESS;
    }

    std::ostream& operator <<(std::ostream &stream, const CStatus& obj)
    {
        return stream << obj.toString().c_str();
    }

    friend std::ostream& operator<<(std::ostream& stream, const CStatus& obj);

private:
    static std::hash_map<STATE, tstring> states;
    static STATE nextState;

protected:

};

//static Status S_SUCCESS         = CStatus::registerState(_T("Success"));
//static Status S_UNDEFINED_ERROR = CStatus::registerState(_T("Undefined Error"));

#endif // STATUS_H

Is this a working program? In other words, have you tried to compile this? I'd be very surprised it compiled.

#include <hash_map>

This header does not exist in the C++ Standard Library. If you want a hash map, you need to use #include <unordered_map> and use the C++11 standard.

typedef const CStatus Status;

I'm pretty sure that something like: const Status S_SUCCESS doesn't compile as that resolves to: const const CStatus S_SUCCESS.

#ifndef S_SUCCESS
#define S_SUCCESS CStatus::CS_SUCCESS
#endif // S_SUCCESS

#ifndef S_UNDEFINED_ERROR
#define S_UNDEFINED_ERROR CStatus::CS_UNDEFINED_ERROR
#endif // S_SUCCESS

You have an error in the comment for the S_UNDEFINED_ERROR macro.

Your RegisterState function could also be slightly simplified:

// Static Functions
static Status registerState(const tstring &stateMsg)
{
    // Post:
    auto res = states.emplace({nextState + 1, stateMsg});

    // if state has been inserted
    return res.second ? Status(++nextState) : Status(res.first->first);
}

const tstring& toString() const
{
    return (states.find(state) == states.end()) ? states[state] : _T("Undefined");
}

I'm not familiar with what a tstring is but this function should be marked static (as only static members can access static member variables) and you shouldn't be returning the string by const value. Let the caller decide whether or not they want to make it const.

operator tstring() const
{
    return toString();
}

operator int() const
{
    return state;
}

What is the point of these and what do they return? These should be removed as they aren't going to be useful to your API. The entire point of your API is to allow the user to easily create new States without having them know too many of the internal implementation details.

bool operator!=(const CStatus& obj) const
{
    return state != obj.state;
}

Implement this in terms of operator==():

bool operator!=(const CStatus &ob) const
{
    return !(state == obj.state);
}

Finally, with this setup, your user will not be able to add any states during compile time. They will only be able to add checks during runtime (most likely during program startup). You should change your description to reflect this.


Customizing color for the navbar of my Android app I got my navigation bar to be the color I want of Blue background with white text.

Here is my code which works but this feels totally hacky. Please suggest improvements
styles.xml
<resources>
    <style name="AppTheme" parent="@android:style/Theme.Holo.Light">
        <item name="android:actionBarStyle">@style/MyActionBar</item>
    </style>

    <!-- ActionBar styles -->
    <style name="MyActionBar" parent="@android:style/Widget.Holo.Light.ActionBar">
        <item name="android:background">#6a9abd</item>
        <item name="android:titleTextStyle">@style/TitleBarTextColor</item>
    </style>

    <style name="TitleBarTextColor" parent="@android:style/Widget.Holo.Light.ActionBar">
        <item name="android:textColor">#ffffff</item>
    </style>
</resources>

You could actually name (i.e. wrap in a proper resource) the color you used there, and thus reference it easier in the future. Not only that, but properly defining resources is recommended for Android development, and depending on the tools you use, you might see them complaining if you don't.

Inside

/res/values/colors.xml

you could define yours as

<?xml version="1.0" encoding="utf-8"?>
<resources>
    <color name="my_amazing_blue">#6a9abd</color>
</resources>

and then reference it as

@color/my_amazing_blue

in the XML, or

Resources res = getResources();
int myAmazingBlue = res.getColor(R.color.my_amazing_blue);

in your Java code.

Also note that for Android, resource names are usually cased_like_this.


Max Increase to Keep City Skyline in Java In a 2-dimensional array grid, each value grid[i][j] represents the height of a building located there. We are allowed to increase the height of any number of buildings, by any amount (the amounts can be different for different buildings). Height 0 is considered to be a building as well.

In the end, the "skyline" when viewed from all four directions of the grid, i.e. top, bottom, left, and right, must be the same as the skyline of the original grid. A city's skyline is the outer contour of the rectangles formed by all the buildings when viewed from a distance. See the following example.

What is the maximum total sum that the height of the buildings can be increased?

Example:
Input: grid = [[3,0,8,4],[2,4,5,7],[9,2,6,3],[0,3,1,0]]
Output: 35
Explanation:
The grid is:
[ [3, 0, 8, 4],
  [2, 4, 5, 7],
  [9, 2, 6, 3],
  [0, 3, 1, 0] ]

The skyline viewed from top or bottom is: [9, 4, 8, 7]
The skyline viewed from left or right is: [8, 7, 9, 3]

The grid after increasing the height of buildings without affecting skylines is:

gridNew = [ [8, 4, 8, 7],
            [7, 4, 7, 7],
            [9, 4, 8, 7],
            [3, 3, 3, 3] ]

Notes:

1 < grid.length = grid[0].length <= 50.
All heights grid[i][j] are in the range [0, 100].
All buildings in grid[i][j] occupy the entire grid cell: that is, they are a 1 x
   1 x grid[i][j] rectangular prism.

My approach:

class Solution {
    public int maxIncreaseKeepingSkyline(int[][] grid) {

        int nrows = grid.length;
        int ncols = grid[0].length;

        int [] leftRig = new int[nrows];
        int [] topBot = new int[ncols];

        //Filling topBot
        for( int i = 0; i < ncols; i++ )
        {
               int max = Integer.MIN_VALUE;

                for( int j = 0; j < nrows; j++ )
               {
                   if( max <= grid[j][i] )
                       max = grid[j][i];
               }
            topBot[i] = max;
        }

        //Filling leftRig
        for( int k = 0; k < nrows; k++ )
        {
               int max = Integer.MIN_VALUE;

                for( int l = 0; l < ncols; l++ )
               {
                   if( max <= grid[k][l] )
                       max = grid[k][l];
               }
            leftRig[k] = max;
        }

        int count = 0;
        int min = 0;

        //Enumerating the minimum height to be added
        for( int i = 0; i < nrows; i++ )
        {
          for( int j = 0; j < ncols; j++ )
            {
                  min = Math.min(leftRig[i],topBot[j]);
                  count += min - grid[i][j];
            }
        }

        return count;
    }
}

Time complexity: O(n^2)

Space complexity: O(n)

Time complexity: O(n) Space complexity: O(n)

I have the following questions regarding the above code snippets:

1) How can I improve the time and space complexity of my code?

2) Is there a better way(lesser lines of code, better data structures) that can be used to improve the code?

3) Is there any better approach to solve this question?

Reference

Here:

for( int i = 0; i < ncols; i++ )
{
    int max = Integer.MIN_VALUE;

    for( int j = 0; j < nrows; j++ )
    {
        if( max <= grid[j][i] )
            max = grid[j][i];
    }
    topBot[i] = max;
}

You don't need an extra variable max as a reference. You can simply use topBot[i] as a reference instead (the elements of an int array will be initialized to the default value 0 upon the array's creation, and since the height of a building cannot be less than 0, 0 is an adequate initial value). Also, it would suffice to check if(max < grid[j][i]) (i.e. using < instead of <=), because you don't need to assign the variable if the two values are equal. In fact, there's an even more concise way to write this:

topBot[i] = Math.max(topBot[i], grid[j][i]);

And with the local variable max eliminated, you can fill both topBot and leftRig in one loop:

for (int i = 0; i < ncols; i++) {
    for (int j = 0; j < nrows; j++) {
        topBot[i] = Math.max(topBot[i], grid[j][i]);
        leftRig[j] = Math.max(leftRig[j], grid[j][i]);
    }
}

In your final loop, min is not needed outside the inner for loop, so it would suffice to declare it inside this inner loop. Reducing the scope of variables to the smallest extent necessary makes code easier to read, because, in this case, it would make it instantly obvious that the variable is not needed outside the loop.


Sending data to a database in size-limited chunks I have a method which takes a parameter which is Partition enum. This method will be called by multiple background threads around same time period by passing different value of partition. Here dataHoldersByPartition is a map of Partition and ConcurrentLinkedQueue<DataHolder>.

  private void validateAndSend(final Partition partition) {
    ConcurrentLinkedQueue<DataHolder> dataHolders = dataHoldersByPartition.get(partition);
    Map<byte[], byte[]> clientKeyBytesAndProcessBytesHolder = new HashMap<>();
    int totalSize = 0;
    DataHolder dataHolder;
    while ((dataHolder = dataHolders.poll())  != null) {
      byte[] clientKeyBytes = dataHolder.getClientKey().getBytes(StandardCharsets.UTF_8);
      if (clientKeyBytes.length > 255)
        continue;
      byte[] processBytes = dataHolder.getProcessBytes();

      int clientKeyLength = clientKeyBytes.length;
      int processBytesLength = processBytes.length;

      int additionalLength = clientKeyLength + processBytesLength;
      if (totalSize + additionalLength > 50000) {
        Message message = new Message(partition, clientKeyBytesAndProcessBytesHolder);
        sendToDatabase(message.getAddress(), message.getLocation());
        clientKeyBytesAndProcessBytesHolder.clear(); // watch out for gc
        totalSize = 0;
      }
      clientKeyBytesAndProcessBytesHolder.put(clientKeyBytes, processBytes);
      totalSize += additionalLength;
    }
    // calling again with remaining values only if clientKeyBytesAndProcessBytesHolder is not empty
    if(!clientKeyBytesAndProcessBytesHolder.isEmpty()) {
        Message message = new Message(partition, clientKeyBytesAndProcessBytesHolder);
        sendToDatabase(message.getAddress(), message.getLocation());
    }
  }

In the method, I will iterate dataHolders CLQ and I will extract clientKeyBytes and processBytes from it. Here is the validation that I am supposed to do:

If the clientKeyBytes length is greater than 255 then I will skip it and continue iterating.
I will keep incrementing the totalSize variable which will be the sum of clientKeyLength and processBytesLength, and this totalSize length should always be less than 50000 bytes.
As soon as it reaches the 50000 limit, I will send the clientKeyBytesAndProcessBytesHolder map to the sendToDatabase method and clear out the map, reset totalSize to 0 and start populating again.
If it doesn't reaches that limit and dataHolders got empty, then it will send whatever it has.

Basically, what I have to make sure is whenever the sendToDatabase method is called, the clientKeyBytesAndProcessBytesHolder map should have size less than 50000 (sum of all keys and values length). It should never be called with the size greater than 50000 (50k).

Question:

My code works fine, but as you can see, I am calling the sendToDatabase method at two places: one is as soon as it is reaching the 50k limit, and the other is it didn't reach because we ran out of elements.

Is there any way by which I can call the sendToDatabase method just once and still fulfill both my conditions by rewriting it in a better way? I am working with Java 7.

I waited a few days to get other answers to your questions. now I feel I can add my 2 cents.

First off, the obvious: This is one single method. It does:

Polling a queue
Serialising a message into a byte[]
Validating the messages
Packaging in 50Kb packages
Sending chunks to a Database

The obvious thing to do is split those actions into individual methods, and move the right methods into the right objects which allow you to use a nicer syntax (right now, too much is in the current single method of the current objec, so your hands are tied).
Our goal is to make your code more readable, like this:
public void batchSend(final Partition partition) {
    Queue<DataHolder> dataHolderQueue = dataHoldersByPartition.get(partition);
    DataHolder data = dataHolderQueue.poll();
    Message message = new Message();
    while (data != null) {
        if(data.isValid()){
            if(message.hasRoomFor(data)){
                message.append(dataHolder);
            } else {
                // Message is at maximum capacity, send it
                database.send(message);
                message = new Message();
            }
        }
        data = dataHolderQueue.poll();
    }
    if(!message.isEmpty()) {
        database.send(message); // Queue is empty, sending remaining message
    }
}

I simply modified the Message Object, which now holds a bunch of DataHolder's serialized data and tells you when you reach a certain size limit. I don't really understand how you concatenate the messages together (it's all hidden in the Message class, which was not included) so most of the below is just a guess:
public class Message {
    public static final int MAX_BYTES_PER_MESSAGE = 50000;
    private final Map<byte[], byte[]> data = new HashMap<>();
    private int size = 0;
    ... Other fields I don't know about ...
    public boolean hasRoomFor(DataHolder holder){
        return size + holder.getSizeIncludingHeader() <= 50000;
    }
    public void append(DataHolder holder){
        byte[] clientKeyBytes = dataHolder.getClientKey().getBytes(StandardCharsets.UTF_8);
        byte[] processBytes = dataHolder.getProcessBytes();
        size += clientKeyBytes.length + processBytes.length;
        // Maybe throw InvalidOperationException is size > 50000 ?
        data.put(clientKeyBytes, processBytes);
    }
    public boolean isEmpty(){
        return map.isEmpty();
    }
    // provide getters on the data field for sendTodatabase() method
    ... Other methods I don't know about ...
}

Also modified the DataHolder Object to add:
public boolean dataHolder.isValid(){
    return dataHolder.getClientKey().getBytes(StandardCharsets.UTF_8) <= 255;
}

Indeed, its validity seem to be an intrinsic property of a DataHolder. By this I mean a DataHolder either is valid, or isn't, but it does not depend on external factors. This is a perfect candidate to be made a method of the DataHolder Object.

Weird message usage
This is weird:

   Message message = new Message(partition, clientKeyBytesAndProcessBytesHolder);
   sendToDatabase(message.getAddress(), message.getLocation());

You're creating an instance of message, but not using the instance, only some of its fields, via getters. The point of using a Message Object is to keep its constitutive elements consistent inside, so those elements are only consistent if using the instance.
By allowing sendToDatabase to have two split parameter, you're allowing the caller to pass two inconsistent parameters:
sendToDatabase(anAddress, aTotallyUnrelatedLocation);

The obvious fix is to pass in the Message Object entirely, the method will just extract the right data:
boolean sendToDatabase(Message message){
    Address address = message.getAddress();
    Location location = message.getLocation();
    ... do Stuff
    return successStatus;
}

It is additionally syntaxically more pleasing to send a Message rather than an address and a location.
By the way, why would the message body not be sent as well? This is confusing. I would have expected sendToDatabase(Message message, Address address) at least. Maybe the Location is the Message body? It does not make sense to me.

GC Handling
This should never be written:

clientKeyBytesAndProcessBytesHolder.clear(); // watch out for gc

The GC knows his stuff. Don't 'help' him. It knows if the collection needs to be GC'ed, or if the data in it does. This call doesn't even make the memory available!
If the collection must be cleared, it must be for business reasons, not because of GC.
Also be careful, if you clear() it, but the sendToDatabase() spawned a Thread which holds onto the reference a bit longer, you've just deleted all data before it was sent.
The best would be to overwrite the reference to the collection with a new one:
clientKeyBytesAndProcessBytesHolder = new HashMap<>();

With this, you signal to the GC that it can get rid of the Collection (you no longer keep a handle), so when no one else holds a reference either, it will be recycled.

Late validation
I wonder why you validate() your messages so late. You only validate them just before sending them. You could do this check earlier, for example when the message is sent to the queues.
This is the "Fail Fast" principle: Don't keep corrupt data in your system, flush them as soon as you can.

Edit:
One shortcoming of this code is I might perform several dataholder-to-byte conversions. If those are not complex, it's fine. If they are, here is a Workaround:
Create a ValidatedRawData Object wich contains just the raw byte data.
It would be created by the DataHolder itself in a re-write of validate() method: so:
public class DataHolder {
    ... stuff ...
    /** Now returns null if fails to validate */
    public ValidatedRawData validate(){
        byte[] clientKeyBytes = dataHolder.getClientKey().getBytes(StandardCharsets.UTF_8);
        if (clientKeyBytes.length > 255)
            return null;
        byte[] processBytes = dataHolder.getProcessBytes();
        return new ValidatedRawData(clientKeyBytes, processBytes);
    }
}

ValidatedRawData is just a holder, with Getters.


Search for a multiword string in a list of strings Unlike Perl, you can't to my knowledge match a regular expression inside an if statement in Python and assign the result to a varaible at the same moment. This leads to typical constructs like this:

match = re.search(REGEX, STRING)
if match:
    # do something

So far, so Python. But what if I want to iterate through a file / array of lines, check each line for a few regexes, and fire a catch-all when none has matched? I can't think my way around a rather unwieldy and deeply nested if-else-if-else...-construction:

import re
strings = ["abc zzz", "y", "#comment"]
for s in strings:
    match = re.search("(\S+) (\S+)", s)
    if match:
        print "Multiword: %s+%s" % (match.group(1), match.group(2))
    else:
        match = re.match("y$", s)
        if match:
            print "Positive"
        else:
            match = re.match("n$", s)
            if match:
                print "Negative"
            else:
                # a few more matches possible in real life script,
                # and then the last catch-all:
                print "No match found, line skipped"

Isn't there any way to put this in a much nicer looking elif-construction or something? The following doesn't work in Python, because if-clauses take only expressions, not statements. However, something along these lines would strike me as pythonic, or am I blind to something obvious here?

if match = re.search(" ", s):
    print "Multiword: %s+%s" % (match.group(1), match.group(2))
elif match = re.match("y$", s):
    print "Positive"
else:
    print "No match found, line skipped"

Just use the continue keyword to force the advancement to evaluate the next string.
Code after the if statement will only execute if the previous statement was false.

import re
strings = ["abc zzz", "y", "#comment"]
for s in strings:
    match = re.search("(\S+) (\S+)", s)
    if match:
        print "Multiword: %s+%s" % (match.group(1), match.group(2))
        continue
    match = re.match("y$", s)
    if match:
        print "Positive"
        continue
    match = re.match("n$", s)
    if match:
        print "Negative"
        continue

    # a few more matches possible in real life script,
    # and then the last catch-all:
    print "No match found, line skipped"


OOP Dependency Inversion Principle I made an example for learning Dependency inversion principle in JavaScript OOP programming .

String.prototype.ucfirst = function() {
    return this.charAt(0).toUpperCase() + this.slice(1);
}

function textHandler(){

    this.printRedbox = function( ){
        return ' Red Box text ';
    }

    this.printBluebox = function( ){
        return ' Blue Box text ';
    }
};

function htmlHandler( handler ){

    var block = [],
        box  = document.getElementById('container');

    this.boxContentHandler =  handler ;

    this.setType = function( handler ){
        this.boxContentHandler = handler
    };

    this.appendBlock = function( html ){

        block.push( html );
        box.innerHTML = block.join('');
    };

    this.printBox = function ( action ){

        var title = this.printTitle() ,
            text  = this.boxContentHandler[ 'print'+ action.ucfirst() ](),
            html  = '
'
                        +   title + text
                    +'
' ;

        this.appendBlock( html );
    };

    this.printTitle = function( ) {
            return 'Title ' + block.length + '
';
    };
};

Here I am doing the action handling part

var htmlHandle = new htmlHandler(  new textHandler()  );

document.getElementById('bluebox').addEventListener( 'click', function(){
    htmlHandle.printBox(  this.id );
},false );

document.getElementById('redbox').addEventListener( 'click', function(){
    htmlHandle.printBox(  this.id );
},false );

I would like to know, is there a way to make it better. Did I miss anything here ?

This is a decent way to implement Dependency Inversion, from the standpoint of your implementing classes. The only improvements I would recommend is to find a good class library that does the lookup of dependencies for you.

Really Inversion of Control/Dependency Inversion involves several parts:

Writing your classes so that they receive their dependencies via one of three methods:

Constructor injection (like you have) new Foo(x)
Property injection a.foo = b
Setter injection a.setFoo(b)

An object factory that can turn a class name in the form of a String into a new object
An dependency resolver that does the lookup of dependencies and injects them into a new object
A "container" object allowing you to configure your dependencies and wire your classes together

I've created Hypodermic for this purpose, though I think it should be refactored to separate the "container" from the "dependency resolver" functionality. I would be interested in finding out if there are other Dependency Injection libraries out there for JavaScript.


A possibly-terrible alternative to conditionals full of null checks I am always super annoyed when I have to write a bunch of if statements whenever I have to traverse an object graph of possibly null objects. As there is not yet a ?. operator I resorted to creating this wonderful piece of work: TryGetProperty. Please tear it apart. This was written for LINQPad.

public bool TryGetProperty<TInstance, TFinal>(TInstance instance, Expression<Func<TInstance, TFinal>> propertyAccessor, out TFinal output)
{

    var currentExpression = propertyAccessor.Body as MemberExpression;
    var stack = new Stack<PropertyInfo>();
    do
    {
       stack.Push(currentExpression.Member as PropertyInfo);
    } while((currentExpression = currentExpression.Expression as MemberExpression) != null);

    var failed = false;

    object currentObject = instance;
    while(!failed && stack.Count > 0 )
    {
        currentObject = stack.Pop().GetMethod.Invoke(currentObject, null);
        failed = currentObject == null && stack.Count != 0;
    }

    output = failed ? default(TFinal) : (TFinal) currentObject;
    return !failed;
}

Example call from the same file.

void Main()
{
    List<Class1> instances = new List<Class1>
    {
        new Class1(),
        new Class1
        {
            Class2 = new Class2
            {
                Class3 = new Class3
                {
                    AProperty = "not null"
                }
            }
        },
        new Class1
        {
            Class2 = new Class2
            {
                Class3 = new Class3()
            }
        }
    };

    string propertyToGet;

    foreach(var instance in instances)
    {
       if(TryGetProperty(instance, c => c.Class2.Class3.AProperty, out propertyToGet) && propertyToGet != null)
       {
            "Yay!".Dump();
       }
       else
       {
            "Boo!".Dump();
       }
    }
}

public class Class1
{
    public Class2 Class2{ get; set; }
}

public class Class2
{
    public Class3 Class3 { get; set; }
}
public class Class3
{
    public string AProperty { get; set; }
}

I tend to use monads in such situations:

public static TResult With<TSource, TResult>(this TSource source, Func<TSource, TResult> action)
    where TSource : class
{
    if (source == null) return default(TResult);
    return action(source);
}

public static TSource Do<TSource>(this TSource source, Action<TSource> action)
    where TSource : class
{
    if (source != null)
    {
        action(source);
    }
    return source;
}

And then:

c.With(x => x.Class2).With(x => x.Class3).With(x => x.AProperty).Do(x => "Yay!".Dump());


Implementing a rectangle class First I've to indicate that this is my homework.

An example of a given input is:

s1 = {'p1': (x1, y1), 'p2': (x2, y2)}

where all names and coordinations are user input, only the input format is predefined; also p1 is the upper leftmost point and p2 is the lower rightmost point as shown below:

I can go on to get area, surrounding, midpoint, height and width based on this input.

I wrote a code that works as demanded but I don't like it. it doesn't seem I followed the Pythonic approach here; how can I improve this?

class rectangle:
    def __init__(self, dct):
        # ['name', [x, y]]
        names = list(dct.keys())
        coords = list(dct.values())
        start = [names[0], coords[0]]
        end = [names[1], coords[1]]
        self.start = start
        self.end = end

        # int
        start_coords = self.start[1]
        end_coords = self.end[1]
        width = end_coords[0] - start_coords[0]
        height = end_coords[1] - start_coords[1]
        self.width = width
        self.height = height

        # [x, y]
        midpoint = [self.width / 2, self.height / 2]
        self.midpoint = midpoint

        # int
        area = (self.width + self.height) * 2
        surr = self.width * self.height
        self.area = area
        self.surr = surr

s1 = rectangle({'p1': (1,1), 'p2': (2,2)})
print('point one: ', s1.start)
print('point two: ', s1.end)
print('width: ', s1.width)
print('height: ', s1.height)
print('midpoint: ', s1.midpoint)
print('area: ', s1.area)
print('surrounding: ', s1.surr)

PEP-8

Class names should be CapWords, so instead of rectangle you should have Rectangle.
Commas should be followed by 1 space.  You've mostly followed this, except in s1 = rectangle({'p1': (1,1), 'p2': (2,2)})

Bugs

The formula for "area" is not twice the sum of width & height.
I don't know what "surrounding" is, but the formula for perimeter is not width times height.
The midpoint (centre?) of a rectangle should be within the bounds of the rectangle.  Consider the rectangles with corners (10, 10) and (12, 12).  The centre would be (11, 11), not (1, 1) as calculated.

Awkward initialization

This code:

        names = list(dct.keys())
        coords = list(dct.values())
        start = [names[0], coords[0]]
        end = [names[1], coords[1]]
        self.start = start
        self.end = end

relies on the dictionary's ordering of keys.  It can break in Python 3.6 and earlier (CPython 3.5 and earlier).  It does not enforce the key names p1 and p2; any two keys will work.  And self.start[0] and self.end[0] are never used, so storing the key names in these entries is unnecessary.

The code could simply and safely read:

         self.start = dct['p1']
         self.end = dct['p2']

with suitable modifications of the usage of self.start and self.end.

Class with no methods

A class should have methods.  Without any methods, you'd be better off with a namedtuple for constant data, or a dict for mutable data.

So let's give your class some methods:

    def width(self):
        return self.end[0] - self.start[0]

    def height(self):
        return self.end[1] - self.start[1]

As mentioned by Peilonrayz, you may wish to use abs(...) here.

You can use these methods externally:

print('width: ', s1.width())
print('height: ', s1.height())

as well as in other members of this class:

    def area(self):
        return (self.width() + self.height()) * 2    # Note: Formula is still incorrect

An Over-Engineered Solution

Do not submit this as your home-work solution!  You would likely fail or be expelled!  This illustrates some advanced concepts like the @dataclass and the NamedTuple, type hints and the typing module, as well as read-only @property attributes, a @classmethod, and """docstrings""".  You may find these interesting to study in your free time.

from typing import NamedTuple
from dataclasses import dataclass

class Point(NamedTuple):
    x: float
    y: float

@dataclass
class Rectangle:
    """A class for calculations on a Rectangle"""

    p1: Point
    p2: Point

    @classmethod
    def from_dict(cls, dct):
        """
        Constructs a Rectangle from a dictionary with "p1" and "p2" keys.
        These keys must contain a tuple or list of two numeric values.
        """
        return Rectangle(Point._make(dct['p1']), Point._make(dct['p2']))

    @property
    def width(self):
        """
        Computes the width of the rectangle.
        """
        return abs(self.p2.x - self.p1.x)

    @property
    def height(self):
        """
        Computes the height of the rectangle.
        """
        return abs(self.p2.y - self.p1.y)

    @property
    def area(self):
        """
        Incorrectly computes the area of the rectangle.
        """
        return (self.width + self.height) * 2     # Note: still the incorrect formula

s1 = Rectangle.from_dict({'p1': (1,1), 'p2': (2,2)})
print('point one: ', s1.p1)
print('point two: ', s1.p2)
print('width: ', s1.width)
print('height: ', s1.height)
print('area: ', s1.area)

Output:

point one:  Point(x=1, y=1)
point two:  Point(x=2, y=2)
width:  1
height:  1
area:  4