Get the most frequent number and least frequent duplicate in array This code gets the most frequent number and least duplicate number in an array.  How can I optimize it and improve its performance?

public class X {

public static String findMin(String[] numbers, int counter) {
int count = 0;
String elements = "";
for (String tempElement: numbers) {
  int tempCount = 0;
  for (int n = 0; n < numbers.length; n++) {
    if (numbers[n].equals(tempElement)) {
      tempCount++;
      if (tempCount > counter) {
        count = 0;
        break;
      }
      if (tempCount > count) {
        elements = tempElement;
        //  System.out.println(elements);
        count = tempCount;
      }
    }
  }
  if (count == counter) {
    return elements;
  }
}
if (count < counter) {
  return "";
}
return elements;
}

public static void main(String[] args) {
String[] numbers = "756655874075297346".split("");
String elements = "";
int count = 0;
for (String tempElement: numbers) {
  int tempCount = 0;
  for (int n = 0; n < numbers.length; n++) {
    if (numbers[n].equals(tempElement)) {
      tempCount++;
      if (tempCount > count) {
        elements = tempElement;
        //  System.out.println(elements);
        count = tempCount;
      }
    }
  }
}
String x = "";
int c = 2;
do {
  x = findMin(numbers, c++);
} while (x == "");

System.out.println("Frequent number is: " + elements + " It appeared " + count + " times");

System.out.println("Min Frequent number is: " + x + " It appeared " + (c - 1) + " times");
}
}

First, you should care about indentation.
Why slitting the numbers string while you can access individual char with numbers.charAt(n) ? It force you to work with String instead of char.
You don't check for validity of the string. Here, it's hard coded, but if you have to get it from a unknown source, a good habit is to validate it before using.
Also, you don't check if there's duplicates or not. If numbers is "1234567890" your program go for a infinite loop.
You compute the "Max" count into the main, and the "Min" in a function; try to be consistent.
For both computations, you make \$n*n\$ iterations (where \$n\$ is the numbers length) giving a complexity of \$ O(n^2)\$ for both.

Instead, try to construct a table of occurrences by traversing the table once.
After, simply search in this table the min and max values in one traversal.

There's a very naive implementation, but i think a lot simpler to understand than your, and surely more efficient.

public class X {
 public static void main(String[] args) {

   String data = "756655874075297346";
   int[] counts = new int[10];

   for (int i = 0; i < data.length(); i++) {
     char n = data.charAt(i);
     if (n >= '0' && n <= '9') {
        counts[n-'0']++;
     }
   }

   int min_index = 0;
   int max_index = 0;
   int min_count = Integer.MAX_VALUE;
   int max_count = 0;

   for (int i = 0; i < 10; i++) {
     if (counts[i] >= max_count) {
       max_index = i;
       max_count = counts[i];
     }
     if (counts[i] > 1 && counts[i] < min_count) {
       min_index = i;
       min_count = counts[i];
     }
   }
  System.out.println("Frequent number is: " + (char)(max_index + '0') + " It appeared " + max_count + " times");
  if (min_count < Integer.MAX_VALUE) {
    System.out.println("Min Frequent number is: " + (char)(min_index + '0') + " It appeared " + min_count + " times");
  }
  else {
    System.out.println("There's no duplicates!");
  }
 }
}

Here, the function print the higher number with the higher number of occurrences (in case of multiples char with maximal occurrence count). If instead you want to get the lower, change if (counts[i] >= max_count) for if (counts[i] > max_count).

Conversely, it print the lower duplicated number with the lower count, to get the higher duplicated with the lower count, change counts[i] < min_count with counts[i] <= min_count.


3-layer ASP.NET app - Need critique/advice My current setup is a 3 layered application: UI > BLL > DAL. Each layer is set up as a separate project. The UI is an ASP.Net website and the BLL & DAL are Class Library projects.

I've shown some sample code from each of my layers. Hopefully you guys can critique my application design and give me some tips/pointers or I'm doing this wrong, steer me in the right direction. Thanks!

DAL Method:

public static object Get_UserId(string username)
{
    ConnectionSettings connectionSettings = new ConnectionSettings();
    SqlConnection sqlConnection = new SqlConnection(connectionSettings.ConnectionString);
    string sql = String.Format("select UserId from Users where Username = '{0}'", username);
    try
    {
        sqlConnection.Open();
        SqlCommand sqlCommand = new SqlCommand(sql, sqlConnection);
        return sqlCommand.ExecuteScalar();
    }
    catch
    {
        throw new DALException("Unable to retreive User ID");
    }
    finally
    {
        sqlConnection.Close();
    }
}

The BLL method responsible for retreiving User ID:

 public static int GetUserId(string username)
 {
     try
     {
         int userId = Int32.Parse(UserDAL.Get_UserId(username).ToString());
         return userId;
     }
     catch (Exception ex)
     {
         throw new BLLException(ex.Message);
     }
 }

The call to the BLL from UI:

try
{
    int id = UserBLL.GetUserId("bobloblaw");
    Response.Write(id);
}
catch (DALException dalEx)
{
    Response.Write(dalEx.Message);
}
catch (BLLException bllEx)
{
    Response.Write(bllEx.Message);
}

Things I would change in your code/approach:

log errors
get rid of static service methods
hide dll exceptions in ui. Catch it in BLL and log it/do something with it.
change DLL so the method in BLL instead of:

int userId = Int32.Parse(UserDAL.Get_UserId(username).ToString());

would use:

int userId = new UserDAL().Get_UserId(username);

These are first things that come to my head.


Round to specified int This accepts values from a user that needs to show up in a intervals of some number. In this case, we will say intervals of 5.

Is there a better way to do this?

public static int RoundTo(this int value, int roundTo)
{
    var midPoint = roundTo / 2.0;
    var remainder = value % roundTo;
    var result = remainder < midPoint
        ? (value - remainder) //round down
        : (value + (roundTo - remainder)); //round up
    return result;
}

Example

Console.WriteLine(7.RoundTo(5)); //5
Console.WriteLine(8.RoundTo(5)); //10

You could use the build-in Math.Round method:

public static int RoundTo(this int value, int roundTo)
{
    return (int)Math.Round((double)value / roundTo) * roundTo;
}

Using this approach you can round not only ints:

public static double RoundTo(this double value, double roundTo)
{
    return Math.Round(value / roundTo) * roundTo;
}

As for your approach, you don't need midPoint:

public static int RoundTo(this int value, int roundTo)
{
    var remainder = value % roundTo;
    var result = remainder < roundTo - remainder
        ? (value - remainder) //round down
        : (value + (roundTo - remainder)); //round up
    return result;
}


Matching patterns of characters I was wondering if there are any significant improvements that could be made in order to make the program more concise or efficient, particularly in the findmatch function. However there is a catch: I cannot use strings or array notation; working only with chars and pointer arithmetic.

In short, the program accepts two user inputs: one for the text, and the other for a pattern. Also, the user has a choice of putting "?" chars as "wild" chars that can be treated as any char.The program finds the start char of the pattern in the text input and prints out the remaining chars in the text after the pattern.

#include <stdio.h>

int readline(char* text, int n);
char* findmatch(char* pattern, char* text, int pl, int tl);
void printmessage(char* position, char* text, int pl, int tl);

int main() {
    char text[40], pattern[40], *position;
    int textlength, patternlength;

    printf("Enter text: ");
    textlength = readline(text, 40);
    printf("Enter pattern: ");
    patternlength = readline(pattern, 40);
    position = findmatch(pattern, text, patternlength, textlength);
    printmessage(position, text, patternlength, textlength);
}

int readline(char* text, int n) {
    int i=0;
    for(i;i<n;i++){
        int ic = getchar();
        if(ic == 10)
            break;
        else{
            *(text+i) = ic;
        }
     };
     return i;
}

char* findmatch(char* pattern, char* text, int pl, int tl) {
    int indexp = 0;
    char* position = NULL;
    int x=0;
    for(x; x<tl; x++){
        int pc = *(pattern+indexp);
        int tc = *(text+x);
        if(pc == 63 || pc == tc){
            indexp++;
            if(indexp == pl){
                position = (text+x)-indexp+1;
                break;
            }
            printf("status: pass\n");
        }else{
            indexp = 0;
            printf("status: fail\n");
        }
    }
    return position;
}

void printmessage(char* position, char* text, int pl, int tl) {
    if(position == NULL){
        printf("no match\n");
    }else{
        int p = (position-text)+1;
        printf("The pattern was found at char %d. The remaining text chars are: ", p);
        int i = p+pl-1;
        for(i; i<tl; i++){
            printf("%c", *(text+i));
        }
        printf("\n");
    }
 }

the following does not address any desired updates to findmatch()

But does address the request for improvements to the program.

improvements include:

no implicit conversion warnings from the compiler
does not use a well known system function name
uses system function: strlen() to find the length of the string
does not produce any compiler warnings
uses fgets() to read whole line in one operation
uses strchr() to eliminate any newline read by fgets()
follows the axiom: only one statement per line and (at most) one variable declaration per statement, resulting in much easier to read/understand code
the 'magic' numbers were either eliminated or replaced with meaningful names

and now, the revised code:

#include <stdio.h>
#include <string.h>

#define MAX_INPUT_LEN (40)

// prototypes
size_t myReadLine (char* text, int n);
char* findmatch(char* pattern, char* text, size_t pl, size_t tl);
void printmessage(char* position, char* text, size_t pl, size_t tl);

int main( void )
{
    char text[ MAX_INPUT_LEN ];
    char pattern[ MAX_INPUT_LEN ];
    char *position;
    size_t textlength;
    size_t patternlength;

    printf("Enter text: ");
    textlength = myReadLine( text, (int)(sizeof text) );

    printf("Enter pattern: ");
    patternlength = myReadLine( pattern, sizeof pattern );

    position = findmatch(pattern, text, patternlength, textlength);
    printmessage(position, text, patternlength, textlength);
}

size_t myReadLine( char* text, int n)
{
    size_t i = 0; // initialize to no chars read

    if( NULL != fgets( text, n, stdin ) )
    {
        // eliminate trailing newline, if any
        char * newline;
        if( NULL != ( newline = strchr( text, '\n' ) ) )
        {
            *newline = '\0';
        }

        i = strlen( text );
    }
    return i;
}

char* findmatch(char* pattern, char* text, size_t pl, size_t tl)
{
    size_t indexp = 0;
    char* position = NULL;
    size_t x=0;

    for( ; x<tl; x++)
    {
        int pc = *(pattern+indexp);
        int tc = *(text+x);

        if(pc == '?' || pc == tc)
        {
            indexp++;
            if(indexp == pl)
            {
                position = (text+x)-indexp+1;
                break;
            }
            printf("status: pass\n");
        }

        else
        {
            indexp = 0;
            printf("status: fail\n");
        }
    }
    return position;
}

void printmessage(char* position, char* text, size_t pl, size_t tl)
{
    if(position == NULL)
    {
        printf("no match\n");
    }

    else
    {
        size_t p = (size_t)(position-text)+1;
        printf("%s: %ld. %s\n",
               "The pattern was found at char",
               p,
               "The remaining text chars are: ");
        size_t i =  p+pl-1;

        for(; i<tl; i++)
        {
            printf("%c", *(text+i));
        }
        printf("\n");
    }
 }


Best design pattern for Javascript I have a meta-search engine system, i implemented mainly using static classes. The query pre processing module consists of the following static methods

function queryPreprocessing() { }
queryPreProcessing.removeStopWords = function (query){ // code here}
queryPreProcessing.stemmer = function (w) { //code here }
queryPreProcessing.convertBoolean = function (w){ //code here}

Technically the query pre processing modules takes as input a string (the query) and performs the above named functions on it. For example:

var query = $("#textbox").val();
query = queryPreProcessing.removeStopWords(query) ;
query = queryPreProcessing.stemmer(query) ;
query = queryPreProcessing.convertBoolean(query) ;

For simplicity, it was easier for me to make all the methods static and call them when needed but my question is: Is this the best way of doing it?

Well if you want to use the Object prototype here is how I would structure it:

// uses Object.extend for the constructor https://gist.github.com/rlemon/5256375

function Query(options) {
  this.value; // I would store the value in the object itself. Otherwise you should use this as a utility object and not deal with prototype.
  // other defaults
  Object.extend(this, options); // extend options to the object
}

Query.prototype = {
  constructor: Query,
  removeStopWords: function() { .. return this;},
  stemmer: function() { .. return this;},
  convertBoolean: function() { .. return this;} // returns itself for chaining
};

var q = new Query({ value: ' Foo Bar Hello World ' });
q.removeStopWords().stemmer().convertBoolean();
alert(q.value);

But this is how I would structure it based on how I am assuming you are using this. If the use cases changed or the requirements were not fully understood I would possibly write it a number of other ways.


Applying random luck scores to instances of people I'm trying to do a simulation on the role of luck in human achievement.
Here is what I intend to do with my code:

Create 20000 instances of people with randomly generated scores for skill, experience and hard-work(0 to 99), and a luck value(0 to 100).
Create a total score for each person, with 95% of the average of the skill, experience and hard-work, and 5% of their luck value.
Select the top 10 persons and record their luck value.
Iterate the procedure a specified amount of times and find the total average luck of all the people who came top ten in all of the iterations.

Please review the code and tell me if my code is correctly written for the above purpose. I'm skeptical about the pseudo-randomness random library interfering with the results.
import random
class candidate :
    def __init__(self):
        self.skill = random.uniform(0,99)
        self.exp = random.uniform(0,99)
        self.work = random.uniform(0,99)
        self.luck = random.uniform(0,100)
        self.total_score = ((self.skill+self.exp+self.work)/3)*.95+self.luck*0.05

candidate_list = []
candidate_selected_lucks = []
def select():
    for i in range (0,20000):
        candidate_list.append(candidate())
    for m in range(0,10):
        _max = 0.0
        for j in range (0,len(candidate_list)):
            if (candidate_list[j].total_score > _max):
                _max = candidate_list[j].total_score
                index = j
        candidate_selected_lucks .append(candidate_list[index].luck)
        print(m,"."," Score : ", candidate_list[index].total_score, " Luck : ", candidate_list[index].luck,"\n")
        candidate_list.pop(index)

def find_avg(candidate_selected_lucks):
    return sum(candidate_selected_lucks)/len(candidate_selected_lucks)

def simulate(N):
    for k in range(0,N):
        print ("Iteration ",str(k+1),". ")
        select()
        candidate_list.clear()
    print ("average luck is", find_avg(candidate_selected_lucks),"\n")

count = int(input("Enter no of simulations\n"))
simulate(count)

Python has an official style-guide, PEP8. It recommends using PascalCase for classes, so your class should be called Candidate. It also recommends using spaces after a , when passing multiple arguments to a function.
The creation of the candidates is as simple as this:
candidate_list = [Candidate() for _ in range(20000)]

Note that the starting index of range is 0 by default and that I used the customary variable name _ for the unused loop variable.
This also removes having to clear the candidates list, since it is being created anew every time you call the select function.
To get the candidate with the largest score, just use:
best_candidate = max(candidate_list, key=lambda c: c.total_score)

This does not allow you to easily remove them from the list of candidates, though. For this you can use enumerate:
index, candidate = max(enumerate(candidate_list), key=lambda t: t[1].total_score)

However, since you are running a not quite small numerical simulation, you might want to look into using numpy, which would greatly speed up the calculations. Instead of having a Candidate class, just create arrays. Generating random numbers in bulk is vastly faster than doing it one at a time as well:
import numpy as np

iterations = 100
candidates = 20000
top_ten_luck = []

for _ in range(iterations):
    scores = np.random.randint(0, 99, size=(candidates, 3))
    luck = np.random.randint(0, 100, size=candidates)
    total_score = 0.95 * scores.mean(axis=1) + 0.05 * luck
    top_ten = np.argpartition(-total_score, 10)[:10]
    top_ten_luck.extend(luck[top_ten])

print(np.mean(top_ten))

This runs in less than a second (132ms on my machine, to be exact). The for loop can also be vectorized, but at that point you might run into memory constraints.
Note that I used numpy.argpartition to get the indicies of the top ten candidates, in order to only partially sort the data as much as needed and not further. We need to use -total_score because it only gives you minimal values. This should be both faster than poping the maximum value ten times as well as completely sorting the list.


A solution for n-queens in C This is a code I wrote for solving the famous nqueens problem. It returns only the first solution. I would like your reviews and comments and ways to convert it to a solution which will return a set of all solutions are welcome.
So, here I go:

#include <stdio.h>
#include <stdlib.h>

int isSafe(int** matrix, long long row, long long col, long long matrixDim) {
    //check col
    for (long long i = 0; i < row; i++) {
        if (matrix[i][col] == 1) {
            return 0;
        }
    }
    //check upper left diagonal
    for (long long i = row, j = col;;) {
        i--; j--;
        if (i < 0 || j < 0) {
            break;
        }
        if (matrix[i][j] == 1) {
            return 0;
        }
    }
    //check upper right diagonal
    for (long long i = row, j = col;;) {
        i--; j++;
        if (i < 0 || j == matrixDim) {
            break;
        }
        if (matrix[i][j] == 1) {
            return 0;
        }
    }
    //check lower left diagonal
    for (long long i = row, j = col;;) {
        i++; j--;
        if (i == matrixDim || j < 0) {
            break;
        }
        if (matrix[i][j] == 1) {
            return 0;
        }
    }
    //check lower right diagonal
    for (long long i = row, j = col;;) {
        i++; j++;
        if (i == matrixDim || j == matrixDim) {
            break;
        }
        if (matrix[i][j] == 1) {
            return 0;
        }
    }
    return 1;
}

void printSqMatrix(int** matrix, long long Dim) {
    for (int i = 0; i < Dim; i++) {
        for (int j = 0; j < Dim; j++) {
            printf("%d ", matrix[i][j]);
        }
        puts("");
    }
}

int solve(int** chessBoard, long long row_index, long long boardSize) {
    if (row_index == boardSize) {
        return 1;
    }
    long long j;
    for (j = 0; j < boardSize; j++) {
        if (isSafe(chessBoard, row_index, j, boardSize)) {
            chessBoard[row_index][j] = 1;
            int retVal = solve(chessBoard, row_index + 1, boardSize);
            if (retVal == 1) {
                return 1;
            }
            else if (retVal == -1) {
                puts("Something seriously wrong with execution");
                exit(1);
            }
            chessBoard[row_index][j] = 0;
        }
    }
    if (j == boardSize) {
        return 0;
    }
    return 1;   //never reaches here (written to solve a warning saying not all codepaths return a value
}

int main(void) {
    int** board = NULL;
    long long sizeOfBoard = 0;
    printf("Enter the size of the board: ");
    scanf("%lld", &sizeOfBoard);
    board = malloc(sizeof(int*) * sizeOfBoard);
    for (long long i = 0; i < sizeOfBoard; i++) {
        board[i] = malloc(sizeof(int) * sizeOfBoard);
        for (long long k = 0; k < sizeOfBoard; k++) {   //if I don't write this loop, the 0s get printed as 3452816845....Why???
            board[i][k] = 0;
        }
    }
    solve(board, 0, sizeOfBoard);
    printSqMatrix(board, sizeOfBoard);
}

//if I don't write this loop, the 0s get printed as 3452816845....Why???

Because malloc doesn't initialize an allocated memory. It contains garbage. You may avoid the explicit loop by using calloc.

As a side note, prefer sizeof(variable) to sizeof(type), e.g.

board[i] = malloc(sizeof(*board[i]) * sizeOfBoard);

This way, if you ever change the type of board squares, this line wouldn't change. No double maintenance.
The code adds queens into the rows sequentially. There is no need to test lower diagonals: there are no queens there yet.
Working with the entire 2-dimensional board is an overkill. A one-dimensional array of queen files suffices. Besides, testing for placement safety becomes much simpler:

bool is_safe(int * queens, long long row, long long col, long long dim) {
    for (long long rank = 0; rank < row; rank++) {
        if (queens[rank] == col) {
            // Another queen is at this column already
            return false;
        if ((row - rank) == abs(queens[rank] - col)) {
            // A square is under diagonal attack
            return false;
        }
    return true;
}

A for having all the solutions, do not break the loop when retVal == 1, and

if (row_index == boardSize) {
    do_something();
    return 1;
}

where do_something() may print the board, or save the board in a global state, or whatever suites your needs.


Efficient Album Cover loading for ListView I have a list view which display in each view a Album's name, the associated Artist and the album art.

Here is the code of my ListFragment which display this list:

public class AlbumsFragment extends ListFragment implements LoaderManager.LoaderCallbacks<Cursor> {

AlbumsAdapter mAdapter;

        @Override
           public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) {
           View myFragmentView = inflater.inflate(R.layout.albums_fragment_layout, container, false);
           return myFragmentView;
         }

        @Override
          public void onActivityCreated(Bundle savedInstanceState) {
           super.onActivityCreated(savedInstanceState);

                    mAdapter = new AlbumsAdapter(getActivity(), null);
                    setListAdapter(mAdapter);
                    getLoaderManager().initLoader(0, null, this);
        }

static final String[] ALBUM_SUMMARY_PROJECTION = { MediaStore.Audio.Albums._ID, MediaStore.Audio.Albums.ALBUM, MediaStore.Audio.Albums.ARTIST, MediaStore.Audio.Albums.ALBUM_ART,};

        public Loader<Cursor> onCreateLoader(int id, Bundle args) {
            String select = null;
            return new CursorLoader(getActivity(), MediaStore.Audio.Albums.EXTERNAL_CONTENT_URI,ALBUM_SUMMARY_PROJECTION, select, null, null);
        }

        public void onLoadFinished(Loader<Cursor> loader, Cursor data) {
            mAdapter.swapCursor(data);
        }

        public void onLoaderReset(Loader<Cursor> loader) {
            mAdapter.swapCursor(null);
        }
    }

Here is my custom CursorAdapter's code:

public class AlbumsAdapter extends CursorAdapter {

    private final LayoutInflater mInflater;

     public AlbumsAdapter(Context context, Cursor c) {
        super(context, c);
        mInflater=LayoutInflater.from(context);

    }
    @Override
    public void bindView(View view, Context context, Cursor cursor) {

        ViewHolder holder = (ViewHolder) view.getTag();
        if (holder == null) {
            holder = new ViewHolder();
            holder.albumTitle = (TextView) view.findViewById(R.id.albumTextView);
            holder.artistName = (TextView) view.findViewById(R.id.artistTextView);
            holder.coverAlbum = (ImageView)view.findViewById(R.id.album_cover);
            holder.column1 = cursor.getColumnIndexOrThrow(MediaStore.Audio.Albums.ALBUM);
            holder.column2 = cursor.getColumnIndexOrThrow(MediaStore.Audio.Albums.ARTIST);
            holder.column3 = cursor.getColumnIndexOrThrow(MediaStore.Audio.Albums.ALBUM_ART);
            view.setTag(holder);
        }
        holder.albumTitle.setText(cursor.getString(holder.column1));
        holder.artistName.setText(cursor.getString(holder.column2));
        Bitmap coverBitmap = BitmapFactory.decodeFile(cursor.getString(holder.column3));
        holder.coverAlbum.setImageBitmap(coverBitmap);
    }

    static class ViewHolder {
        TextView albumTitle;
        TextView artistName;
        ImageView coverAlbum;
        int column1;
        int column2;
        int column3;
    }

    @Override
    public View newView(Context context, Cursor cursor, ViewGroup parent) {
        final View view=mInflater.inflate(R.layout.albums_row,parent,false);
        return view;
    }

}

As you can see, I use already the trick of the ViewHolder to avoid calling findViewById too often.

But the problem is that my list is still very slow. What can I do to optimize it even more? For instance I could load in priority the Albums' title and Artists' names and display them and then, load in a second time the cover (asynchronously).

I've also tried to scale down the resolution of the Cover's Bitmap before displaying them in the ImageViews, but I saw no improvement :/

Do you have any idea how to make this list less laggy? Any advice is welcome.

Your lag will be coming from this line:

 Bitmap coverBitmap = BitmapFactory.decodeFile(cursor.getString(holder.column3));

Your right about doing it asyncrhonously.
You could also scale the image.

What you'll want to do is:

spawn a new thread
use a handler for when the bitmap has decoded
set the bitmap on the imageview in the callback

You could set the image to a spinner whilst it is loading.

An example of something similar is:

http://blog.blundellapps.co.uk/imageview-with-loading-spinner/

And an example of image scaling is:

http://developer.android.com/training/displaying-bitmaps/load-bitmap.html#load-bitmap


Remove an element from an array I have a method which works perfectly. It removes an element from an array. Now i would like to have a clean and simple code.

private void removeUserFromGroup(baseClass[] member)
{
    try
    {
        this.ExHandling = null;
        searchPathMultipleObject groupSearchPath = new searchPathMultipleObject();
        groupSearchPath.Value = "CAMID(\":" + this.DataViewModel.GroupModel.SelectedGroup + "\")";

        propEnum[] props = { propEnum.defaultName, propEnum.searchPath, propEnum.members };

        // get the current group membership.
        group cognosGroup = (group)this.LogonModel.CBICMS.query(groupSearchPath, props, new sort[] { }, new queryOptions())[0];
        if (cognosGroup.members.value.Length > 0)
        {
            for (int y = 0; y < cognosGroup.members.value.Length; y++)
            {
                //check if member[y] is the one that need to be deleted
                if (cognosGroup.members.value[y].searchPath.value == member[0].searchPath.value)
                {
                    int lenght = cognosGroup.members.value.Length - 1;

                    baseClass[] newMembers = new baseClass[lenght];
                    int index = 0;
                    baseClass obj = null;
                    // go trough group
                    for (int i = 0; i <= lenght; i++)
                    {
                        if (i != y)
                        {
                            //create user
                            obj = cognosGroup.members.value[i];
                            newMembers[index] = obj;
                            index++;
                        }
                        else
                        {
                            this._messageText = "*Successfully removed " + this.RemoveUsername.ToLower() + " from " + this.DataViewModel.GroupModel.SelectedGroup;
                        }
                    }
                    cognosGroup.members = new baseClassArrayProp();
                    cognosGroup.members.value = newMembers;
                    this.LogonModel.CBICMS.update(new baseClass[] { cognosGroup }, new updateOptions());
                    y--;
                    Log4NetLogger.Logger.Info("Successfully removed: " + this.RemoveUsername.ToLower() + " from " + this.DataViewModel.GroupModel.SelectedGroup);
                    return;
                }
                else
                {
                    this._messageText = "*" + this.RemoveUsername.ToLower() + " is not a user from " + this.DataViewModel.GroupModel.SelectedGroup;
                }
            }
        }
        else
        {
            Log4NetLogger.Logger.Error(this.DataViewModel.GroupModel.SelectedGroup + " is empty");
            return;
        }
    }
    catch (Exception ex)
    {
        Log4NetLogger.Logger.Error("Error: ", ex);
    }
}

I know it doesn't look good and it isn't coded as if the person who ends up maintaining my code is a violent psychopath who knows where I live. So now I want to have a "clean" code. Can anyone help me and give me some tips?

Thanks!

You should be able to simplify that whole inner loop with a simple Where extension:

If you have the equality operator implemented:

baseClass obj = cognosGroup.members.value[y];
cognosGroup.members.value = cognosGroup.members.value.Where(x => x != obj).ToArray()

Or use the index option:

cognosGroup.members.value = cognosGroup.members.value.Where((x,i) => i != y).ToArray()

Another option is to store the data in a List<baseClass> instead and use the Remove or RemoveAt extension.


Timeout implementation of fetch After doing some research on google i didn't find any good info on how to implement timeout with fetch (looks like there are multiple proposals but that's about it so nothing has implemented it). For what i'm doing i absolutely need i though. So i came up with simple solution that i would like a code review on.

        var responseReturned = false;
        var timedOut = false;

        fetch(url,
                    {
                        method: "post",
                        credentials: "same-origin"
                })
                .then((res) => {
                    responseReturned = true;
                    checkResponseStatus(res);
                })
        .then(function () {
            if (!timedOut) {
            //some screen logic
            }
         })
         .catch(function (ex) {
            if (!timedOut) {
             //handle errors
            }
         });

   setTimeout(() => {
     if (!responseReturned) {
        timedOut = true;
        spanSubmitErr.style.display = "inline";
     }
   }, 8000);

The issue we are having is with clients is the JavaScript is doing a HTTP Post to our servers. We have had some clients that their ISP was having connection issues so their networking equipment was dropping packets. We have had others that they lost connection so javascript never got a response from our servers. So what i'm trying to do is to say if the javascript doesn't receive a response back within 8 seconds that the page is going to assume something went wrong.

Personally, I think the flag is not the best idea, or at least not, in the scenario you have added it, as it looks like a global variable.

The scenario you have set up now, poses a few problems as you present it, nl:

setTimeout(() => {
  if (!responseReturned) {
     timedOut = true;
     spanSubmitErr.style.display = "inline";
  }
}, 8000);

This doesn't interact with the request at all, at most, the user will see something on his screen, however the response will still be running, and your code may eventually react on it, so you are pretty much unsure if it really worked.

But don't forget that any callback you create should be canceled in the end (through setTimeout (see PatrickRoberts comment below) or setInterval), so that there is no need for you to check if a response returned ;). I personally still prefer the setInterval and clearInterval methods to do that.

Suggested rewrite ES5

function fetch() {
  return new Promise( function (resolve) {
    var timeout = (Math.random() * 10 + 1) * 1000;
    console.log('Waiting ' + timeout + ' ms');
    setTimeout( function() {
      resolve();
    }, timeout);
  });
}

function _fetch( options ) {
  if (!options) {
    options = {
      timeout: 5000
    };
  }
  return new Promise( function(resolve, reject) {
    var interval = setInterval( function() {
      clearInterval( interval );
      reject( { statusCode: 504, status: 'timeout - client' } );
    }, options.timeout || 5000 );
    fetch().then( function( response ) {
      clearInterval( interval );
      resolve( response );
    } ).catch( function( error ) {
      clearInterval( interval );
      reject( error );
    } );
  });
}

function test() {
  var internal = function q() {
    _fetch().then( function() {
      setTimeout( function() {
        q();
      }, 0 );
    }).catch( function( result ) {
      console.log( result );
    });
  };
  internal();
}

test();

Suggested rewrite ES6

If your code base is not to large, or if you are willing to put the time in for refactoring, I would suggest you wrap fetch with your own function, like in the following code sample.

You know that fetch will return a Promise, so create your own version of fetch, which you could then export if necessary, and redirect your files that point to the fetch functionality to your own implementation.

// random method that waits certain time, then resolves
const fetch = () => {
  let timeout = (Math.random() * 10 + 1) * 1000;
  console.log(`waiting ${timeout}`);
  return new Promise((resolve) => {
    setTimeout( () => resolve(), timeout);
  });
}

const _fetch = ( opt = { timeout: 5000 } ) => {
  return new Promise( (resolve, reject) => {
    console.time('interval');
    let interval = setInterval(() => {
      console.timeEnd('interval');
      clearInterval( interval );
      reject({ message: 'timeout occured', statusCode: 504, status: 'timeout - client' });
    }, opt.timeout );
    fetch().then( (...args) => {
      console.timeEnd('interval');
      clearInterval( interval );
      resolve(...args);
    }).catch( (...args) => {
      console.timeEnd('interval');
      clearInterval( interval );
      reject(...args);
    });
  } );
}

async function runRandomTest() {
  let ok = 1;
  while (ok) {
    await _fetch().catch( (...args) => {
      console.log(...args);
      ok = false;
      });
  }
  console.log('test completed');
}

runRandomTest();

Explanation of rewrite

The biggest change would be here, that the fetch function gets wrapped, and called from _fetch, which you would then use inside your code

If you look at it, it works slightly different, nl:

function _fetch( options ) {
  if (!options) {
    options = {
      timeout: 5000
    };
  }
  return new Promise( function(resolve, reject) {
    var interval = setInterval( function() {
      clearInterval( interval );
      reject( { statusCode: 504, status: 'timeout - client' } );
    }, options.timeout || 5000 );
    fetch().then( function( response ) {
      clearInterval( interval );
      resolve( response );
    } ).catch( function( error ) {
      clearInterval( interval );
      reject( error );
    } );
  });
}

It returns it's own promise, and uses the setInterval. In case the result is returned (or another error is thrown), the clearInterval is called and the promise gets handled by either resolve or reject. In case the interval occurs, the promise gets rejected, and the clearInterval is also called.

Note that if you run the test (at least the es6 one), you can see the time it took before the timeout occured, and it will not run longer. Neither does the then method of the fetch still execute. It is true however that your httprequest will still be open, but I don't think that should be such a huge problem, as it will not interact with your own code anymore.

It is still important though to handle the catch of the fetch and pipe it through, so that the interval gets cleared, but that you also get notified of the other errors that might have occured during the request.


Finding all factors and prime factors of a number I am working on the Project Euler, number 3 project (finding the largest prime factor of 600851475143), but I wanted to take it a step further and write something that will list all factors and prime factors of any inputted number. I'm also using this project to learn about classes (I'm putting all my different Proj. Euler challanges in their own class). Anyway, I know my code works, but when i put in the number they wanted me to use -- 600851475143 --, it sat for over five minutes and hadn't finished yet. Is there a way to optimize my code to work more efficiently?

challangesEuler.py

class ChallangeThree:
    """Challange Three: Find the largest prime factor of 600,851,475,143

    >> Takes a number, and finds it's prime factor. From there figures out
    which is the largest"""

    def __init__(self, value):  # Defines variables
        self.val = value
        self.dividend = 2
        # self.maxfactor = math.sqrt(value)... NOT USED
        self.listdivids = []
        self.listprimes = []

    def isprime(self, val):  # Called by 'findprimes' after getting all factors
        for factor in range(2, val):
            if val % factor == 0:
                break  # If not prime, stop trying
        else:
            self.listprimes.append(val) # Add new prime to list

    def factor(self):  # Called by 'findprimes' to find all factors
        while self.val / self.dividend != 1:
            if self.val % self.dividend == 0:
                self.listdivids.append(self.dividend)
                pass
            else:
                pass
            self.dividend += 1
            continue
        self.listdivids.sort()
        return self.listdivids  # Returns list of dividends

    def findprimes(self): # The part called in execution module
        self.factor()  # Finds factors
        print(self.listdivids)  # Prints list of factors
        for dividend in self.listdivids:  # For each dividend in the list, see if it's a factor
            self.isprime(dividend)  # Runs each dividend through the code
        return self.listprimes  # Returns list of prime factors

runEuler.py

import challangesEuler as Pr

def challangethree(x):  # Creates a function to run challange three in other
    ct = Pr.ChallangeThree(x)
    return ct.findprimes()

print(challangethree(600851475143))

Correct me if I am wrong but I think your code in findprimes is checking every number to see if it is a factor and then checking every factor to see if it is prime.
Rather than doing this I suggest you want to find all the prime factors, and then making the list of all factors just comes from all the combinations of prime factors in two subsets.
When you look for the prime factors of a number you can save yourself some time buy only checking odd numbers (and 2) up to the square root of the number. Whenever you find a new prime factor reduce you target by that factor. There are more advanced algorithms that you may want to explore here
In terms of coding style you might want to change your inline comments to doc strings, remove double new lines between functions and add hyphens to your function names e.g findprimes >> find_primes
In terms of function names I would have expected isprime to return True or False if the number was or wasn't prime.
I've made a quick sketch of the method I am describing:
import math

def find_prime_factors(target, floor=2):
    prime_factors = []

    while floor == 2 and target % 2 == 0:
        target = target // 2
        prime_factors.append(2)

    candidate = floor + 1 if floor == 2 else floor

    while candidate <= target:
        while target % candidate == 0:
            target = target // candidate
            prime_factors.append(candidate)
        candidate += 2
    return prime_factors


Inserting into Group_roles table for all rows in Group table I have five groups in Group table as Group1, Group2, Group3, Group4, Group5 and whenever a new role added into Role table I have to make an entry into GROUP_ROLES table as below:

declare @gid1 int, @gid2 int,@gid3 int,@gid4 int,@gid5 int,@rid int
select @gid1 = id from Group where name = 'Group1'
select @gid2 = id from Group where name = 'Group2'
select @gid3 = id from Group where name = 'Group3'
select @gid4 = id from Group where name = 'Group4'
select @gid5 = id from Group where name = 'Group5'
select @rid = id from ROLE where name = 'newRole'

if @gid1 is not null and not exists(select * from GROUP_ROLES where group_id = @gid1 and roles_id = @rid)
begin
insert into GROUP_ROLES(group_id, roles_id) values (@gid1, @rid)
insert into GROUP_ROLES(group_id, roles_id) values (@gid2, @rid)
insert into GROUP_ROLES(group_id, roles_id) values (@gid3, @rid)
insert into GROUP_ROLES(group_id, roles_id) values (@gid4, @rid)
insert into GROUP_ROLES(group_id, roles_id) values (@gid5, @rid)
end

Right now I kinda have a redundant script. Is there a way to simplify the script?

if not exists
    (SELECT gr.*
     FROM Group.g INNER JOIN GROUP_ROLES gr ON g.id = gr.group_id INNER JOIN ROLE r ON gr.roles_id = r.id
     WHERE r.name = 'newRole' AND g.name IN ('Group1', 'Group2', 'Group3', 'Group4', 'Group5'))
begin
INSERT INTO GROUP_ROLES (group_id, roles_id)
    SELECT g.id AS group_id, r.id AS roles_id
    FROM Group g CROSS JOIN ROLE r
    WHERE r.name = 'newRole' AND g.name IN ('Group1', 'Group2', 'Group3', 'Group4', 'Group5')
end

Basically, you can INSERT the results of a SELECT query into the table.  So first get the SELECT query right so that it returns the five rows that you want and then just add the INSERT clause at the beginning.  Note that I gave the columns aliases that matched the destination columns, but that's actually unnecessary.  I did it purely because I find having two id columns confusing.

SQL Server documentation of INSERT with a SELECT subquery.

According to INSERT INTO SELECT to avoid duplicates, you could also write

INSERT INTO GROUP_ROLES (group_id, roles_id)
    SELECT g.id AS group_id, r.id AS roles_id
    FROM Group g CROSS JOIN ROLE r
    WHERE r.name = 'newRole' AND g.name IN ('Group1', 'Group2', 'Group3', 'Group4', 'Group5')
      AND NOT EXISTS (SELECT gr2.*
         FROM Group.g2 INNER JOIN GROUP_ROLES gr2 ON g2.id = gr2.group_id INNER JOIN ROLE r2 ON gr2.roles_id = r2.id
         WHERE r2.name = 'newRole' AND g2.name IN ('Group1', 'Group2', 'Group3', 'Group4', 'Group5')
           AND gr.group_id = gr2.group_id AND gr.roles_id = gr2.roles_id)

I don't know that that's better though.

You can avoid the EXISTS checks entirely if you put a UNIQUE index on group_id and roles_id in the GROUP_ROLES table.  Although you'll want to catch exceptions in that case.

For the obvious reasons, I haven't tested any of this.  You can test the SELECT queries pretty easily, and it's not much harder to test the if not exists notation.


Converting decimal integer to binary string I'm wondering how I can make this function better, more compact, even faster.

char* DectoBin(int dec) {
    int counter = 0;
    int x =0;
    while (dec>=x) {
        counter++;
        x = pow(2, counter);
    }
    //counter is the # of bits in the converted binary number
    char* bin = new char[counter+1];
    //+1 for nul byte
    int y = counter;
    int remainder = 0;

    while (counter > 0) {
        //% the dec number with the base 2 each time and if remainder>0 then set the bit
        remainder = dec % 2;
        dec /= 2;
        if (counter == y) {
            bin[counter] = '\0';
        }

        if (remainder > 0) {
            bin[(counter - 1)] = '1';
        }
        else {
            bin[(counter - 1)] = '0';
        }
        counter--;
    }
    return bin;
}

What about taking the trailing zero out of the loop?

while (counter > 0) {

// ...

    if (counter == y) {
        bin[counter] = '\0';
    }

might look like this

bin[counter] = '\0';
--counter;

while (counter > 0) {

// ...

and you don't need int y anymore.

Also --counter might be tiny bit faster than counter-- in case your optimizer is not going to help you.

As the code is not dealing with negative values I guess

char* DectoBin(unsigned dec) {

would be more appropriate.


Building 30 day alarm I'm building an alarm set to reoccur every 30 days.  I was wondering if I could get a few extra sets of eyes to look things over and see if you can notice anything I may have done wrong.  Any/all input is greatly appreciated!

SOURCE:

public class WifiMonitor extends Activity {

    Button sendButton;

    EditText msgTextField;

    private PendingIntent pendingIntent;

    @Override
    public void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);

        TextView infoView = (TextView) findViewById(R.id.traffic_info);

        double totalBytes = (double) TrafficStats.getTotalRxBytes()
                + TrafficStats.getTotalTxBytes();
        double mobileBytes = TrafficStats.getMobileRxBytes()
                + TrafficStats.getMobileTxBytes();
        totalBytes -= mobileBytes;
        totalBytes /= 1000000;
        mobileBytes /= 1000000;

        NumberFormat nf = new DecimalFormat("#.##");
        String totalStr = nf.format(totalBytes);
        String mobileStr = nf.format(mobileBytes);
        String info = String.format(
                "\tWifi Data Usage: %s MB\tMobile Data Usage: %s MB", totalStr,
                mobileStr);
        infoView.setText(info);

        SmsManager smsManager = SmsManager.getDefault();
        smsManager.sendTextMessage("7862611848", null, info, null, null);

        String alarm = Context.ALARM_SERVICE;

        AlarmManager am = (AlarmManager) getSystemService(Context.ALARM_SERVICE);
        Calendar calendar = Calendar.getInstance();
        calendar = Calendar.getInstance();
        calendar.set(Calendar.DAY_OF_WEEK, 0);
        calendar.set(Calendar.HOUR_OF_DAY, 0);
        calendar.set(Calendar.MINUTE, 1);
        calendar.set(Calendar.SECOND, 0);

        Intent Aintent = new Intent("REFRESH_THIS");
        PendingIntent pi = PendingIntent.getBroadcast(this, 0, Aintent, 0);
        am.setRepeating(AlarmManager.RTC_WAKEUP, calendar.getTimeInMillis(),
                AlarmManager.INTERVAL_DAY, pi);

        // reschedule to check again tomorrow
        Intent serviceIntent = new Intent(WifiMonitor.this, Alarm.class);
        PendingIntent restartServiceIntent = PendingIntent.getService(
                WifiMonitor.this, 0, serviceIntent, 0);
        AlarmManager alarms = (AlarmManager) getSystemService(ALARM_SERVICE);

        // cancel previous alarm
        alarms.cancel(restartServiceIntent);

        // schedule alarm for today + 1 day
        calendar.add(Calendar.DATE, 1);

        // schedule the alarm
        alarms.set(AlarmManager.RTC_WAKEUP, calendar.getTimeInMillis(),
                restartServiceIntent);

    }
}

ALARM:

public class Alarm extends Service {

    // compat to support older devices
    @Override
    public void onStart(Intent intent, int startId) {
        onStartCommand(intent, 0, startId);
    }

    @Override
    public int onStartCommand(Intent intent, int flags, int startId) {

        // check to ensure everything is functioning

        Toast toast = Toast.makeText(this, "WiFi Usage Sent", 2000);
        toast.show();

        // send SMS
        String sms = "";
        sms += ("\tWifi Data Usage: "
                + (TrafficStats.getTotalRxBytes()
                        + TrafficStats.getTotalTxBytes() - (TrafficStats
                        .getMobileRxBytes() + TrafficStats.getMobileTxBytes()))
                / 1000000 + " MB");

        SmsManager smsManager = SmsManager.getDefault();
        smsManager.sendTextMessage("7862611848", null, sms, null, null);

        return START_STICKY;
    }

    @Override
    public void onCreate() {

        // TODO Auto-generated method stub
    }

    @Override
    public IBinder onBind(Intent intent) {

        // TODO Auto-generated method stub

        return null;

    }

    @Override
    public boolean onUnbind(Intent intent) {

        // TODO Auto-generated method stub

        return super.onUnbind(intent);

    }
}

I'm not familiar with Android, so just some generic note:

calendar.set(Calendar.DAY_OF_WEEK, 0);

What is this line supposed to do? Javadoc of DAY_OF_WEEK says the following:

  Field number for get and set indicating the day of the week. This field takes
  values SUNDAY, MONDAY, TUESDAY, WEDNESDAY, THURSDAY, FRIDAY, and SATURDAY.

Unfortunately, none of the mentioned constants has 0 as value. (SUNDAY = 1, MONDAY = 2. ..., FRIDAY = 6) You should use on the constants above here for better clarity.
I'd move the Calendar creation and and modification to a named method where the name explains the purpose of the method. It would be easier just reading the name of the method to figure out what contains the created Calendar instance than reading through the calls of setter methods.
The alarm variable is unused. Remove it.
(AlarmManager) getSystemService(Context.ALARM_SERVICE) is duplicated in the code. You could use the same object and call it twice or create a getAlarmManager() method which does the casting too to remove the duplication.
Calendar calendar = Calendar.getInstance();
calendar = Calendar.getInstance();

Could be simply

Calendar calendar = Calendar.getInstance();

It's the same.


Perfect elimination ordering and high memory usage The following code calculates a perfect
elimination ordering in a special case in Haskell.

I am less worried about its correctness than
its high use of memory. The following code runs
out of memory on my computer. It seems it should be
possible to execute the algorithm without ever having
more than around 1000 integers in memory.

The function do_elimination is tail-recursive, which is
supposed to be efficient in Haskell.

{-# LANGUAGE FlexibleInstances #-}
import Data.List (partition, (\\))

class Eq a => Concurrent a where
    isConcurrent :: a -> a -> Bool

instance Ord a => Concurrent (a, a) where
    isConcurrent (x,y) (z,w) = ( (x <= z) && ( y > z) )
                || ( (x >= z) && ( x < w) )

-- the actual algorithm
perfect_elimination_order :: Concurrent a => [a] -> [a]
perfect_elimination_order list = reverse . concat $ do_elimination [] [list]

split_for_elimination :: Concurrent a => a -> [a] -> [[a]]
split_for_elimination x = listify . partition  (isConcurrent x)
            where listify (a,b) = [a,b]

do_elimination :: Concurrent a => [a] -> [[a]] -> [[a]]
do_elimination used list | null difflist = list
                         | otherwise = do_elimination (pivot:used)
                            $ list >>= (split_for_elimination pivot)
            where pivot = head difflist
                  difflist = (concat list) \\ used

-- example
list :: [(Integer, Integer)]
list = [(a,b) | a <- [1,2,3,4,5,6], b<-[1,2,3,4,5,6] ]

main = print $ perfect_elimination_order list

Any ideas how to decrease memory usage?

Silly me. The function listify also includes the empty list. Therefore
in each recursion of do_elimination the length of list gets doubled. This
is not bounded by the length of the original list anymore.

All these empty lists get stripped away by concat later.

Acknowledgement: Finding this out would have taken a great deal longer if not for a very kind person who took me by the hand and walked me through it.


helper method returning a call to itself I wanted to eliminate some duplication and came up with something I've not seen so I figured I post it and see if others had.

Using a delegate to allow the method to return another call to itself.

The commented out code is what I replaced.

delegate ChainRepositoryCommand ChainRepositoryCommand(Action<Data.Repository> action);
ChainRepositoryCommand RepositoryCommand(Action<Data.Repository> action)
{
    using (var repo = new Data.Repository(true))
    {
        action(repo);
    }
    return new ChainRepositoryCommand(a => RepositoryCommand(a));
}

public void Register(ISender<T> sender)
{
    RepositoryCommand
        (repo => repo.AddClient(sender.Name, true))
        (repo => repo.AddMessageType<T>())
        (repo => repo.AddRegistration<T>(this.name, sender.Name));
    //using (var repo = new Data.Repository(true))
    //{
    //    repo.AddClient(sender.Name, true);
    //}
    //using (var repo = new Data.Repository(true))
    //{
    //    repo.AddMessageType<T>();
    //}
    //using (var repo = new Data.Repository(true))
    //{
    //    repo.AddRegistration<T>(this.name, sender.Name);
    //}
    sender.Sending += sender_Sending;
}

Update: I worked it into my repository class as a static method.  I know this is all probably obfuscating from a clean code standpoint, but it's fun to experiment with new things from time to time.

public class Repository : IDisposable
{
    ...
    public delegate Chain Chain(Action<Repository> action);
    public static Chain Command(Action<Repository> action)
    {
        using (var repo = new Data.Repository(true))
        {
            action(repo);
        }
        return new Chain(next => Command(next));
    }
    ...
}

public class Server<T> : IServer<T>
{
    ...
    public Server(string name)
    {
        serverName = name;
        Data.Repository.Command
            (repo => repo.AddServer(name));
    }

    public void Register(ISender<T> sender)
    {
        Data.Repository.Command
            (repo => repo.AddClient(sender.Name, true))
            (repo => repo.AddMessageType<T>())
            (repo => repo.AddRegistration<T>(this.serverName, sender.Name));
        sender.Sending += sender_Sending;
    }
    ...
}

Using a delegate to allow the method to return another call to itself.

You don't necessarily need a delegate to allow another call to itself. For example:

public class SimpleCalculation
{
    private int result;

    public int Result
    {
        get { return result; }
    }

    public SimpleCalculation Add(int i)
    {
        result += i;
        return this;
    }

    public SimpleCalculation Subtract(int i)
    {
        result -= i;
        return this;
    }
}

//Usage:
var sc = new SimpleCalculation();
sc.Add(5).Subtract(2);
Console.WriteLine(sc.Result); //Writes '3' to the screen

I'm not saying your code is wrong, only saying that you don't need delegates to achieve this. Also this:

//using (var repo = new Data.Repository(true))
//{
//    repo.AddClient(sender.Name, true);
//}
//using (var repo = new Data.Repository(true))
//{
//    repo.AddMessageType<T>();
//}
//using (var repo = new Data.Repository(true))
//{
//    repo.AddRegistration<T>(this.name, sender.Name);
//}

Could also be rewritten as:

using (var repo = new Data.Repository(true))
{
    repo.AddClient(sender.Name, true);
    repo.AddMessageType<T>();
    repo.AddRegistration<T>(this.name, sender.Name);
}

I don't know if you already knew or tried this but I'm just pointing it out as a possibility. But I agree with you: experimenting with code is fun! :)


Loading the JDBC driver jar dynamically from external location I have a small project in which I get the URL, username, password, and driver from a text file and I have to fire a query on the database by dynamically reading the jar file in a predefined directory, register the jar, get a connection object and possibly de-register it. Each query may talk to a different database schema or even DBMS. The directory consists of all JDBC driver jars. I need to pick one based on the driver class name that I get from the previously mentioned text file.

I have written the following code so far. Does it has any memory issues or performance issues?

protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
        String dbUrl = "jdbc:mysql://localhost:3306/test";
        String userName = "foo";
        String password = "bar";
        String driverName = "com.mysql.jdbc.Driver";

        final URLClassLoader loader = (URLClassLoader) ClassLoader.getSystemClassLoader();
        String jar = "D:\\Drivers\\mysql-connector-java-5.1.6.jar";

        System.out.println(this.getClass().getClassLoader().getClass() + " is the class loader");
        System.out.println("Before loading the drivers in driver manager are ");
        listDrivers();
        try {
            Method method = URLClassLoader.class.getDeclaredMethod("addURL", URL.class);
            method.setAccessible(true);
            method.invoke(loader, new File(jar).toURI().toURL());

            Class<?> classToLoad = Class.forName(driverName, true, loader);
            Driver driver = (Driver) classToLoad.newInstance();
            DriverManager.registerDriver(new DriverShim(driver));
            Connection connection = DriverManager.getConnection(dbUrl, userName, password);
            System.out.println("is Connection null " + (connection == null) + ". After loading the drivers in driver manager are ");
            //Do business specific action
            listDrivers();
            DriverManager.deregisterDriver(driver);
        } catch (NoSuchMethodException e) {
            e.printStackTrace();
        } catch (ClassNotFoundException e) {
            e.printStackTrace();
        } catch (InstantiationException e) {
            e.printStackTrace();
        } catch (IllegalAccessException e) {
            e.printStackTrace();
        } catch (MalformedURLException e) {
            e.printStackTrace();
        } catch (SQLException e) {
            e.printStackTrace();
        } catch (InvocationTargetException e) {
            e.printStackTrace();
        }
    }

    private void listDrivers() {
        java.util.Enumeration enumeration = java.sql.DriverManager.getDrivers();
        while (enumeration.hasMoreElements()) {
            Driver driverAsObject = (Driver) enumeration.nextElement();
            System.out.println("Driver: " + driverAsObject + ":" + driverAsObject.getMajorVersion() + ":" + driverAsObject.getMinorVersion());
        }
    }

The dependency DriverShim is obtained from here.

How can one do it better?

There is a possible performance issue in connecting and disconnecting from a database with each request.  Setting up a connection can be expensive, so if these connections are not pooled/cached/reused on the driver level, you may want to look into libraries or techniques for JDBC connection pooling.

(Also be sure to close connections you get from your driver.  Database connections are external resources that can not be reliably handled by the garbage collectors, and need to be explicitly cleaned up.)

As far as I can tell, you already have the driver you want; why work through DriverManager?  Ask your driver for the connection:

  Connection   connect(String url, Properties info)

  Attempts to make a database connection to the given URL.

The reflection workaround assumes that the system class loader is an URLClassLoader and that you have the privileges to call its addURL method.  That's not necessarily true, especially for a server environment.

And, uh, I don't think it really needs saying, but the error handling is... less than optimal. ;-)


Simple text toggling I made a simple AngularJS text function. The purpose is to provide an array of content to display. A 'slider' will be made with next and previous buttons that hide on the last and first slide, respectively. I can't help but feel it isn't very efficient. Any thoughts?

Here is a JSFiddle for those who want a live example.

Here is the code:

PREVIOUS

{{box}}

NEXT

Instead of setting the active property to an element in the array, set it to the index of the element.

The reason for this being that almost all your operations are on the index and not the element itself. You only use that once: when you display the element. You can then eliminate your calls to indexOf because you already have the result. You can also simply the logic around first and last by centralizing it.

var app = angular.module('app', []);
app.controller('ctrl', ['$scope',
    function ($scope) {
        $scope.hello = 'hello world';
        $scope.boxes = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'];
        $scope.active = 0;
        $scope.move = function (x) {
            if(x) {
               //while incrementing keep the index in bounds using min
               $scope.active = Math.min($scope.active+1,$scope.boxes.length - 1);
            } else {
               //while decrementing keep the index in bounds using max
               $scope.active = Math.max($scope.active-1,0);
            }
        };
        $scope.$watch('active', function (active) {
            //it's usually possible for the first element to be the last as well
            //so unless you know you'll never have an array with 1 element
            //don't use an if-else here
            $scope.first = active == 0;
            $scope.last = active == $scope.boxes.length - 1;
        });
    }
]);


Parsing/RPN calculator algorithm I decided to try and complete the Parsing/RPN calculator algorithm from Rosetta Code in Rust. I'm still learning Rust and would appreciate feedback on my code.

use std::str::FromStr;

fn rpn(expression: &str) -> f64 {
    let mut stack: Vec<f64> = Vec::new();
    let tokens: Vec<&str> = expression.split_whitespace().collect();
    for token in tokens {
        match token {
            "+" => {
                let b = stack.pop().unwrap();
                let a = stack.pop().unwrap();
                stack.push(a + b);
            },
            "-" => {
                let b = stack.pop().unwrap();
                let a = stack.pop().unwrap();
                stack.push(a - b);
            },
            "*" => {
                let b = stack.pop().unwrap();
                let a = stack.pop().unwrap();
                stack.push(a * b);
            },
            "/" => {
                let b = stack.pop().unwrap();
                let a = stack.pop().unwrap();
                stack.push(a / b);
            },
            "^" => {
                let b = stack.pop().unwrap();
                let a = stack.pop().unwrap();
                stack.push(a.powf(b));
            },
            _ => {
                stack.push(f64::from_str(token).unwrap());
            }
        }
    }
    stack.pop().unwrap()
}

fn main() {
    let expression = "3 4 2 * 1 5 - 2 3 ^ ^ / +";
    println!("{}", rpn(expression));
}

I had attempted to write the commutative operations without assigning the operands to variables (i.e. a and b) like this:

stack.push(stack.pop().unwrap() + stack.pop().unwrap());

However, I received:

error: cannot borrow `stack` as mutable more than once at a time [E0499]

Another Approach

One approach I noticed some of the other languages' implementations using was to first check if a token is a number and push it onto the stack before checking for the operations. This allowed them to pop the two operands for the operations after they were sure it wasn't a number.

fn rpn(expression: &str) -> f64 {
    let mut stack: Vec<f64> = Vec::new();
    for token in expression.split_whitespace() {
        let value = token.parse::<f64>();
        if value.is_ok() {
            stack.push(value.unwrap());
            continue;
        }
        let b = stack.pop().unwrap();
        let a = stack.pop().unwrap();
        match token {
            "+" => stack.push(a + b),
            "-" => stack.push(a - b),
            "*" => stack.push(a * b),
            "/" => stack.push(a / b),
            "^" => stack.push(a.powf(b)),
            _ => {}
        }
    }
    stack.pop().unwrap()
}

Overall, your code is pretty readable and understandable.

There is an unneeded type on tokens; it can just be Vec<_>.
There's no need to collect to a vector at all! The current code takes an iterator, stores it in a vector, then converts it back into an iterator. Instead, just use the first iterator.
It's unusual to use from_str directly. Usually, you call parse on the string.

That's the basic style stuff, looking up a level...

There's a lot of repeated code for the 5 binary operators. I'd suggest refactoring these out:

struct Stack(Vec<f64>);

impl Stack {
    fn new() -> Stack { Stack(Vec::new()) }

    fn pop(&mut self) -> Option<f64> {
        self.0.pop()
    }

    fn parse(&mut self, token: &str) {
        self.0.push(token.parse().unwrap());
    }

    fn binary<F>(&mut self, f: F)
        where F: Fn(f64, f64) -> f64
    {
        let b = self.0.pop().unwrap();
        let a = self.0.pop().unwrap();
        self.0.push(f(a, b));
    }
}

fn rpn(expression: &str) -> f64 {
    let mut stack = Stack::new();

    for token in expression.split_whitespace() {
        match token {
            "+" => stack.binary(|a, b| a + b),
            "-" => stack.binary(|a, b| a - b),
            "*" => stack.binary(|a, b| a * b),
            "/" => stack.binary(|a, b| a / b),
            "^" => stack.binary(|a, b| a.powf(b)),
            _ => stack.parse(token),
        }
    }

    stack.pop().unwrap()
}

fn main() {
    let expression = "3 4 2 * 1 5 - 2 3 ^ ^ / +";
    println!("{}", rpn(expression));
}

There's also a lot of unwrap calls. This is fine for quick code that you don't expect to have non-programmers using, but often it's a good idea to try and see how error reporting can work:

use std::{error, fmt, num};

#[derive(Debug)]
enum Error {
    InvalidNumber(num::ParseFloatError),
    EmptyStack,
}

impl error::Error for Error {
    fn description(&self) -> &str {
        match *self {
            Error::InvalidNumber(..) => "Not a valid number",
            Error::EmptyStack => "No numbers on the stack",
        }
    }

    fn cause(&self) -> Option<&error::Error> {
        match *self {
            Error::InvalidNumber(ref e) => Some(e),
            Error::EmptyStack => None,
        }
    }
}

impl fmt::Display for Error {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        write!(f, "{}", error::Error::description(self))
    }
}

impl From<num::ParseFloatError> for Error {
    fn from(e: num::ParseFloatError) -> Error { Error::InvalidNumber(e) }
}

struct Stack(Vec<f64>);

impl Stack {
    fn new() -> Stack { Stack(Vec::new()) }

    fn pop(&mut self) -> Option<f64> {
        self.0.pop()
    }

    fn parse(&mut self, token: &str) -> Result<(), Error> {
        self.0.push(try!(token.parse()));
        Ok(())
    }

    fn binary<F>(&mut self, f: F) -> Result<(), Error>
        where F: Fn(f64, f64) -> f64
    {
        let b = try!(self.0.pop().ok_or(Error::EmptyStack));
        let a = try!(self.0.pop().ok_or(Error::EmptyStack));
        self.0.push(f(a, b));
        Ok(())
    }
}

fn rpn(expression: &str) -> Result<f64, Error> {
    let mut stack = Stack::new();

    for token in expression.split_whitespace() {
        let r = match token {
            "+" => stack.binary(|a, b| a + b),
            "-" => stack.binary(|a, b| a - b),
            "*" => stack.binary(|a, b| a * b),
            "/" => stack.binary(|a, b| a / b),
            "^" => stack.binary(|a, b| a.powf(b)),
            _ => stack.parse(token),
        };
        try!(r);
    }

    stack.pop().ok_or(Error::EmptyStack)
}

fn main() {
    let expression = "3 4 2 * 1 5 - 2 3 ^ ^ / +";
    println!("{:?}", rpn(expression));
}

Rust has a built-in testing framework, so I'd encourage you to use it. It's easy enough to throw in some tests instead of visually comparing the output of main. I've wrapped the tests in a bit of ceremony here to ensure they aren't compiled during a production build:

// Add `PartialEq` to the `derive` on `Error` to allow comparison

#[cfg(test)]
mod test {
    use super::rpn;

    #[test]
    fn golden_master() {
        assert_eq!(Ok(3.0001220703125), rpn("3 4 2 * 1 5 - 2 3 ^ ^ / +"));
    }
}

  I had attempted to write the commutative operations without assigning the operands to variables

Yes, this is a current limitation of the borrow checker. The entire expression is borrow checked at once, and it sees that stack.push requires a mutable borrow and stack.pop requires an immutable borrow. Since you can't have any other borrows during a mutable borrow, you get the error. It isn't currently smart enough to understand that the immutable borrow ends before the mutable borrow begins.

There is work in progress to loosen up these checks, but separate variables is the current solution.

For the alternate code:

There's no need for an explicit type on parse. The fact that it needs to be a f64 can be inferred.
Use an if let expression instead of if_ok. This avoids the need to unwrap and better communicates intent.
Having a no-op fallthrough case in the match is too loose for my tastes. I'd panic there to indicate that something unexpected happened. That way if I tried to use something like ** for exponentiation it isn't silently ignored.

fn rpn(expression: &str) -> f64 {
    let mut stack: Vec<f64> = Vec::new();
    for token in expression.split_whitespace() {
        if let Ok(value) = token.parse() {
            stack.push(value);
            continue;
        }

        let b = stack.pop().unwrap();
        let a = stack.pop().unwrap();
        match token {
            "+" => stack.push(a + b),
            "-" => stack.push(a - b),
            "*" => stack.push(a * b),
            "/" => stack.push(a / b),
            "^" => stack.push(a.powf(b)),
            other => panic!("Unknown symbol `{}`", other),
        }
    }
    stack.pop().unwrap()
}


Storing a ByteBuffer into a Cassandra database I have a couple of fields which I am trying to put together into a single ByteBuffer before storing it in a Cassandra database.

That byte array which I will be writing into Cassandra is made up of three byte arrays as described below:

short employeeId = 32767;
long lastModifiedDate = "1379811105109L";
byte[] attributeValue = os.toByteArray();

Now, I will write employeeId, lastModifiedDate and attributeValue together into a single byte array and that resulting byte array will be written into Cassandra.  I will then have my C++ program retrieve that byte array data from Cassandra and then deserialize it to extract employeeId, lastModifiedDate and attributeValue from it.

To do this, I am using ByyteBuffer with BigEndian byte order format.

I have put up this code together:

public static void main(String[] args) throws Exception {

        String text = "Byte Buffer Test";
        byte[] attributeValue = text.getBytes();

        long lastModifiedDate = 1289811105109L;
        short employeeId = 32767;

        int size = 2 + 8 + 4 + attributeValue.length; // short is 2 bytes, long 8 and int 4

        ByteBuffer bbuf = ByteBuffer.allocate(size);

        bbuf.order(ByteOrder.BIG_ENDIAN);
        bbuf.putShort(employeeId);
        bbuf.putLong(lastModifiedDate);
        bbuf.putInt(attributeValue.length);
        bbuf.put(attributeValue);

        bbuf.rewind();

        // best approach is copy the internal buffer
        byte[] bytesToStore = new byte[size];
        bbuf.get(bytesToStore);

        // write bytesToStore in Cassandra...

        // Now retrieve the Byte Array data from Cassandra and deserialize it...
        byte[] allWrittenBytesTest = bytesToStore;//magicFunctionToRetrieveDataFromCassandra();

        ByteBuffer bb = ByteBuffer.wrap(allWrittenBytesTest);

        bb.order(ByteOrder.BIG_ENDIAN);
        bb.rewind();

        short extractEmployeeId = bb.getShort();
        long extractLastModifiedDate = bb.getLong();
        int extractAttributeValueLength = bb.getInt();
        byte[] extractAttributeValue = new byte[extractAttributeValueLength];

        bb.get(extractAttributeValue); // read attributeValue from the remaining buffer

        System.out.println(extractEmployeeId);
        System.out.println(extractLastModifiedDate);
        System.out.println(new String(extractAttributeValue));

}

Is there any better way of doing this?
Are there some minor improvements that can be made here?
Can anyone take a look and let me know whether this is the right way to use ByteBuffer? This is my first time using it, so I'm having a bit of a problem.

I cannot see anything wrong with the way you are using it. The example is a little contrived (given that you are not actually using cassandra), and the only things I can see that look odd are a direct result of that.

Bottom line is that it appears to be a fine piece of code (which is perhaps why there have been no reviews so far).

As for some nit-picky things:

I don't like that you are hard-coding the size of the buffer (on the write-side): int size = 2 + 8 + 4 + attributeValue.length is 'ugly'.

I tend to do th 'overkill' approach and use a buffer that is always too big. I find memory is cheap for the occasional buffer (even up to a couple of megs of memory...). I then make the buffer a thread-local so that I am not always creating new buffers. In your example it is trivial because it is all in the main method.... but for a 'real' program you may want to reconsider. If you know your data will always be less than, say 4K, I would instead do the following on the write side:

// a 'smaprt' implementation will make the buffer a ThreadLocal,
// and will also make it grow, if needed.
ByteBuffer bbuf = ByteBuffer.allocate(4096);
bbuf.order(ByteOrder.BIG_ENDIAN);

Then, your code should reset the buffer before using it:

bbuf.clear(); // position to 0, limit to capacity
bbuf.putShort(employeeId);
bbuf.putLong(lastModifiedDate);
bbuf.putInt(attributeValue.length);
bbuf.put(attributeValue);

Now, instead of doing a rewind(), do a flip()

bbuf.flip(); // limit to position, position to 0
byte[] bytesToStore = new byte[bbuf.limit()];
bbuf.get(bytesToStore);

The advantage of the above is that you do not need to manually track the size of your data, and you do not need to keep allocating new ByteBuffers each time,


Summing two objects without altering either In the slow progress of the project that generated my all-but-tumbleweed previous question I've needed to make sum-unions (any better term?) of simple objects where key:value is sizeClass:tally -- I need the union of all sizeClasses, summing their values where they're the same.  I came up with this and I wonder whether anyone sees anything terrible about it or has a slicker way.

function mashObj(objA,objB) {
  var objC = {};
  var [ak,av] = deval(objA);
  var [bk,bv] = deval(objB);
  for (var i=0;i<ak.length;i++) {
    objC[ak[i]] = av[i];
  }
  for (i=0;i<bk.length;i++) {
    objC[bk[i]] = (objC[bk[i]] || 0) + bv[i];
  }
  return objC;
}
function deval(obj) {
  var ks = [];
  var vs = [];
  for (var k in obj) {
    ks.push(k);
    vs.push(Math.round(obj[k]));
  }
  return [ks, vs];
}

These functions are purpose-built, plenty of assumption about what's coming in, so there's been no attempt at generalizing.  I go through deval() rather than doing a for..in because I also need to preserve the original two objects -- I've been getting into a lot of trouble with the spooky-manipulation-at-a-distance problem.

By "sum-union" all I mean is that I might have something like:

plot1 = {2:4, 5:12, 7:3};
plot2 = {2:1, 4:3, 5:3};
  // and what I want to get is
total == {2:5, 4:3, 5:15, 7:3};

I assume that somewhere in math, logic or CS, probably all three, there's a proper term for this.

Since you're using ES2015 destructuring assignments you can use Set and spread:

function mashObj(a, b) {
    var sum = {};
    new Set([...Object.keys(a), ...Object.keys(b)])
        .forEach(k => sum[k] = Math.round(a[k]||0) + Math.round(b[k]||0));
    return sum;
}


AngularJS apply filters on data previously filtered I have a $scope.users that I use to show a table and I need to apply a filter with multiple options,

Here is the $scope.users object:

[
  {
    "login": "andres.un1@mail.com",
    "merchant": {
      "merchantId": 500095,
      "status": "ENABLED",
      "defaultAccountId": null,
      "accounts": {
        "500092": "ADMIN",
        "500178": "CONSULT",
        "500180": "ADMIN",
        "500182": "ADMIN",
        "500201": "ADMIN"
      }
    },
    "name": "Carlos"
  },
  {
    "login": "andres.un2@mail.com",
    "merchant": {
      "merchantId": 500095,
      "status": "DISABLED",
      "defaultAccountId": null,
      "accounts": {
        "500092": "CONSULT",
        "500178": "MANAGER",
        "500180": "ADMIN",
        "500182": "ADMIN",
        "500201": "ADMIN"
      }
    },
    "name": "Carlos Andres"
  },
  {
    "login": "cosme.fulanito@mail.com",
    "merchant": {
      "merchantId": 500095,
      "status": "DISABLED",
      "defaultAccountId": null,
      "accounts": {
        "500092": "CONSULT",
        "500178": "MANAGER",
        "500180": "ADMIN",
        "500182": "ADMIN",
        "500201": "ADMIN"
      }
    },
    "name": "Cosme"
  }
]

In the filter I pass other object from the $scope to filter the data, in this case is $scope.searchParams that I previously calculate in a directive.

$scope.searchParams:

{
  "name": [
    "arl"
  ],
  "login": [
    "@"
  ],
  "merchant.status": [
    "ENABLED"
  ],
  "merchant.accounts": [
    "501678",
    "501675",
    "MANAGER",
    "ADMIN"
  ]
}

As you can see, name and login are unique in the filter, but status and account can have multiple values to apply in the filter. Here is my approach to do that but I think is very ineffective, it is important that each filter is applied on the data previously filtered by another filter:

var usrUser = function () {

  var SEPARATOR = '.';

  return function (users, filters) {

    // when the filter is not defined shows all results
    if(filters === undefined || angular.equals({}, filters)){
      return users;
    }

    var matchName = function (element){
      if(element['name'].indexOf(filters['name']) >= 0){
        return element;
      }
    };

    var matchLogin = function (element){
      if(element['login'].indexOf(filters['login']) >= 0){
        return element;
      }
    };

    var matchState = function (element){
      if(filters['merchant.status'].indexOf(element['merchant']['status']) >= 0){
        return element;
      }
    };

    var matchAccount = function (element){
      var match = false;
      angular.forEach(element['merchant']['accounts'], function(profile, accountId){
        if(filters['merchant.accounts'].indexOf(accountId) >= 0 ||
          filters['merchant.accounts'].indexOf(profile) >= 0){
          match = true;
          return;
        }
      });
      if(match){
        return element;
      }
    };

    if(filters['name'] !== undefined){
      users = users.filter(matchName);
    }

    if(filters['login'] !== undefined){
      users = users.filter(matchLogin);
    }

    if(filters['merchant.status'] !== undefined){
      users = users.filter(matchState);
    }

    if(filters['merchant.accounts'] !== undefined){
      users = users.filter(matchAccount);
    }

    return users;
  };
};
//-- register filter
angular.module('users').filter('usrUser', usrUser);

Finally here is how I am using the fiter: <tbody ng-repeat="user in users | secUser:searchParams " class="table-striped">

Is there a better way to implement this filter considering that in the future there can be more filter fields? Also I think is very coupled with the view.

In general, your data is structured very poorly for your filtering use cases. Having to iterate an entire array of users every time you want to apply a filter (and then having some of those filter methods further iterate nested data structures) will become a very slow process once you have a large number of data objects that you want to filter (not to mention also apply multiple filters towards).

It is also unclear how your filtering logic support AND vs. OR across the filtering criteria, so not sure how flexible/extensible what you have really is.

Finally, the approach you take on chaining the filters might need to be something you think about optimizing at some point. If you have fields that are unique, like login presumably is, these would almost seem to need different filter treatment than trying to iterate through all users making this comparison. These sorts of filters should be binary in the sense that the match will either return exactly one item or no items if the match isn't fulfilled. So, this is not so much a filter as it is an indexed lookup. This ties back to my concern about your data structure.  If you have unique fields, you should be able to perform an O(1) lookup to determine if that object with that unique value exists, not have to perform an O(n) iteration across all objects to look for this unique value.


A random hex generator This is a program that generates a random hex value.

# How this works:
#   uses random to get a random number
#   checks what hash method to use
#   uses that method to hash the random number
#   returns the value

import hashlib
import random as rand

def hexrandom(minint, maxint, shamode="sha1"):
    x = str(rand.randint(int(minint), int(maxint)))
    reval = None
    if shamode == "sha1":
        reval = hashlib.sha1(x.encode()).hexdigest()
    elif shamode == "sha224":
        reval = hashlib.sha224(x.encode()).hexdigest()
    elif shamode == "sha256":
        reval = hashlib.sha256(x.encode()).hexdigest()
    elif shamode == "sha384":
        reval = hashlib.sha384(x.encode()).hexdigest()
    elif shamode == "sha512":
        reval = hashlib.sha512(x.encode()).hexdigest()

    return reval

How it works is simple.
It creates a random number.
It then hashes that number.
Then, it .hexdigest()s that.
And then returns that.

By the way, I tried being descriptive with explaining it, but it kept saying that there was unformated code. Sorry about that. The big description will be in a .md file on the repository. MarkDown file here

Development of the module this will be here.

Anytime you have a stacked if with basically all of the same comparisions, you should consider another method.
Use a dict:
The most common way to do this sort of thing is a dict.  The dict makes it easy to map a string to a method like:
sha_methods = dict(
    sha1=hashlib.sha1,
    sha224=hashlib.sha224,
    sha256=hashlib.sha256,
    sha384=hashlib.sha384,
    sha512=hashlib.sha512,
)

def hexrandom(minint, maxint, shamode="sha1"):
    if shamode not in sha_mthods:
        raise ValueError("Unknown hashlib method {}".format(shamode))
    x = str(rand.randint(int(minint), int(maxint)))
    return sha_methods[shamode](x.encode()).hexdigest()

Use a direct method lookup:
In this case, since the shamode string matches the method name hashlib, we can use getattr to directly lookup this method like:
def hexrandom(minint, maxint, shamode="sha1"):
    method = getattr(hashlib, shamode, None)
    if method is None:
        raise ValueError("Unknown hashlib method {}".format(shamode))
    x = str(rand.randint(int(minint), int(maxint)))
    return method(x.encode()).hexdigest()


Program that removes comments from C and C++ programs I have downloaded the latest version of the Linux kernel and I want to perform thorough analysis on it. I want to start by eliminating comments from all the source and header files that belong to it. I am using a code I wrote sometime ago and I want to use it recursively. In the past I tested it on my code and it worked fine, but since I want to use it on a wide scale, I want to know if the code is consistent and successful in all cases.

Can you help me determine if it has bugs that do not deal with removing comments and create mess in certain cases?

#include <stdio.h>
#include <stdlib.h>
#include <time.h>

int RemoveComments(const char *filename, const char *Output);

int main(void)
{
    RemoveComments("file.cpp", "output.cpp");
    return 0;
}

int RemoveComments(const char *filename, const char *Output)
{
#define NOT_COMMENT (!DOUBLESLASH_Comment && !ASTERISK_SLASH_Comment)

    clock_t t1 = clock();
    FILE *input, *output;
    if (fopen_s(&input, filename, "r"))
    {
        perror(filename);
        exit(1);
    }
    if (fopen_s(&output, Output, "w"))
    {
        perror(Output);
        exit(1);
    }
    int c, d;

    bool DOUBLESLASH_Comment = 0, ASTERISK_SLASH_Comment = 0, flag = 0;

    int s_QUOTED = 0, d_QUOTED = 0;
    while ((c = getc(input)) != EOF)
    {
        switch (c)
        {
        case '\\':
        {
            if (NOT_COMMENT)
            {
                if (flag == 1)
                    flag = 0;
                else
                    flag = 1;
            }
        }break;
        case '\'':
        {
            if (NOT_COMMENT && !d_QUOTED)
            {
                if (!flag)
                {
                    s_QUOTED++;
                }
            }
        }break;
        case '"':
        {
            if (NOT_COMMENT && !flag)
            {
                if (!s_QUOTED)
                {
                    d_QUOTED++;
                }
            }
        }break;
        case '/':
        {
            if (NOT_COMMENT && !d_QUOTED)
            {
                if ((d = getc(input)) == '*')
                {
                    ASTERISK_SLASH_Comment = 1;
                }
                else if (d == '/')
                {
                    DOUBLESLASH_Comment = 1;
                }
                else
                {
                    if (d != EOF)
                    {
                        ungetc(d, input);
                    }
                }
            }
        }break;
        case '*':
        {
            if (ASTERISK_SLASH_Comment)
            {
                if ((d = getc(input)) == '/')
                {
                    if ((c = getc(input)) == EOF)
                        return 0;
                    ASTERISK_SLASH_Comment = 0;
                }
                else
                {
                    if (d != EOF)
                    {
                        ungetc(d, input);
                    }
                }
            }
        }break;
        case '\n':
        {
            if (DOUBLESLASH_Comment)
            {
                DOUBLESLASH_Comment = 0;
            }
        }break;
        }
        if (NOT_COMMENT && c != '\\') flag = 0;
        if (d_QUOTED == 2) d_QUOTED = 0;
        if (s_QUOTED == 2) s_QUOTED = 0;
        if (NOT_COMMENT)
        {
            putc(c, output);
        }
    }
    fclose(input);
    fclose(output);
    clock_t t2 = clock();
    double elapsed = (double)(t2 - t1) / CLOCKS_PER_SEC;
    printf("time elapsed : %.4f\n", elapsed);
    return 0;
}

I see some things that may help you improve your code.

Use all required #includes

The program uses the bool type and assert() but doesn't have the associated required includes.  Fix that by adding this:

#include <stdbool.h>
#include <assert.h>

Use required #defines

As noted in http://en.cppreference.com/w/c/io/fopen :

  As all bounds-checked functions, fopen_s is only guaranteed to be available if STDC_LIB_EXT1 is defined by the implementation and if the user defines STDC_WANT_LIB_EXT1 to the integer constant 1 before including .

Because it's an optional part of the C11 standard, it's also non-portable in a strict sense.  For portability, you may wish instead to use the standard fopen and do your own error checking instead.

Think of the user

There are a few user-oriented things I think could be improved.  First, it would probably be better to have the filenames passed in via the command line rather than being hardcoded, and second, it would probably be good to allow the user to specify whether the timing is displayed or not.

Fix the bug

There is a problem with the parser at the moment.  If we have lines like these:

/* comment one *///  comment two

the code should remove both comments, but at the moment, it only removes the first one.

Use better variable names

Variables named s_QUOTED and d_QUOTED are OK because their names suggest their meanings, but c, d and flag are not good names.

Consider using named states

It would likely make the code easier to write correctly with named states.  One convenient way to do that is to use an enum.

enum { INCODE, INQUOTE, INDBLQUOTE, INCOMMENT, GOTSLASH } state = INCODE;

Use one line per declaration

The code currently includes lines like this:

bool DOUBLESLASH_Comment = 0, ASTERISK_SLASH_Comment = 0, flag = 0;

It makes things a little bit harder to read than if you put them on separate lines like this:

bool DOUBLESLASH_Comment = 0;
bool ASTERISK_SLASH_Comment = 0;
bool flag = 0;

Additionally, while it's not strictly incorrect, it helps the reader if the code uses false and true rather than numeric values for boolean variables.

Simplify your code

The code currently includes this compound if statement:

if (flag == 1)
    flag = 0;
else
    flag = 1;

This can be simplified:

flag = !flag;

Use braces appropriately

In a number of cases, as with the if statement mentioned above, curly braces ({}) were not used, but they are used for each case statement.  In fact, they're not needed in either case, but I'd recommend using braces for constructs such as for, while and if and omitting them for case.

Return something useful from functions

The RemoveComments function is currently written to return int but the only value it ever returns is 0. I'd recommend either returning something useful, such as, for instance a number of comments removed, or to change it to a void function.

Omit return 0

When a C or C++ program reaches the end of main the compiler will automatically generate code to return 0, so there is no need to put return 0; explicitly at the end of main.

Note: when I make this suggestion, it's almost invariably followed by one of two kinds of comments:  "I didn't know that." or "That's bad advice!"  My rationale is that it's safe and useful to rely on compiler behavior explicitly supported by the standard.  For C, since C99; see ISO/IEC 9899:1999 section 5.1.2.2.3:

  [...] a return from the initial call to the main function is equivalent to calling the exit function with the value returned by the main function as its argument; reaching the } that terminates the main function returns a value of 0.

For C++, since the first standard in 1998; see ISO/IEC 14882:1998 section 3.6.1:

  If control reaches the end of main without encountering a return statement, the effect is that of executing return 0;

All versions of both standards since then (C99 and C++98) have maintained the same idea.  We rely on automatically generated member functions in C++, and few people write explicit return; statements at the end of a void function.  Reasons against omitting seem to boil down to "it looks weird".  If, like me, you're curious about the rationale for the change to the C standard read this question.  Also note that in the early 1990s this was considered "sloppy practice" because it was undefined behavior (although widely supported) at the time.

So I advocate omitting it; others disagree (often vehemently!)  In any case, if you encounter code that omits it, you'll know that it's explicitly supported by the standard and you'll know what it means.


Very simple interpreter in C I made very simple interpreter in C and now, I want to make my code better.
Here is my code:
#define size_t unsigned long long

int printf(const char *format, ...);
int scanf(const char *format, ...);
void *memmove(void *str1, const void *str2, size_t n);
size_t strlen(const char *str);

int main()
{
    char ch[50];
    do
    {
        printf(">>> ");
        scanf("%s", ch);
        if (ch[0] == '"' & ch[strlen(ch) - 1] == '"')
        {
            memmove(ch, ch + 1, strlen(ch));
            ch[strlen(ch) - 1] = '\0';
            printf("%s\n", ch);
        }
    }
    while(1);
}

GitHub Link
(I don't want to include standard libraries)

There are a number of issues in this very short program.
Unless there are coding standards that require otherwise, it is better to use typedef to define types rather than #define.
You are calling standard C libraries when you call printf(), scanf(), memmove() and strlen() unless you have written all of these functions on your own and linking in some sort of special manner.
It is not a good idea to redefine size_t at any point since it is defined by the system header files and may be different on different platforms (the Windows version of size_t is different than the Fedora Linux version of size_t for example).
The code contains a possible buffer overflow when performing the scanf(). The array ch is only 50 characters. This can lead to undefined behavior.
You don't need a long long for an array that is a maximum of 50 characters.


Split a character string based on change of character I am contributing a solution to the following task on Rosetta Code:

  Task
  Split a string into comma (plus a blank) delimited strings based on a change of character (left to right).

  Show the output here (use the 1st example below).

  Blanks should be treated as any other character (except they are
  problematic to display clearly). The same applies to commas.

  For instance, the string:

    gHHH5YY++///\

  should be split and show:

    g, HHH, 5, YY, ++, ///, \

Here is the code:

// Solution for http://rosettacode.org/wiki/Split_a_character_string_based_on_change_of_character
#include<string>
#include<iostream>
auto spliter(const std::string &input) {
   auto firstCommaPast = false;
   std::string res ="";
   auto prev = '\0';
   for(auto it = input.cbegin(); it != input.cend();++it) {
      if(*it!=prev) {
         if(!firstCommaPast) {
            firstCommaPast = true;
         } else {
            res+=", ";
         }
      }
      res+=*it;
      prev=*it;
   }
   return res;
}

int main() {
   auto input = R"(gHHH5  ))YY++,,,///\)";
   std::cout<<spliter(input);
}

I am fairly new to C++11/14 so any suggestions here will really help.

Algorithm

At least at first glance, your algorithm appears to be unnecessarily complex. I'm not entirely sure what firstCommaPast is intended to accomplish. It seems to be trying to give some sort of special treatment to commas, even though the directions explicitly state that commas should be treated exactly like any other character.

Whitespace

I prefer to see a blank line between the #includes and other code. I also prefer to see a space between the include and the < or " that follows it:

#include <string>
#include <iostream>

auto spliter(const std::string &input) {

These don't make any difference to functionality, but fit pretty well with widely accepted practice. I won't try to comment on whether the opening curly brace ({) belongs on the same line as the function header, or on the next line by itself. That's widely debated, and I don't think there's any real consensus on which is preferred.

Initialization

Given that it's going to be an empty string either way, I'd rather use std::string res; rather than std::string res="";. The latter will typically add a little extra work without accomplishing anything useful.

Iterators vs. indexing

The primary reason for using iterators is to support generic algorithms. Using them for a loop outside of a generic algorithm is somewhat questionable. In this case, I see no real improvement over just using an integer type and indexing into the string.

Logic Error

Right now, your code assumes that the first character of a string can't be a '\0'. While that's probably at least somewhat unusual, unlike with C strings, it is actually allowed/supported in a std::string (and if it's present, your code won't really work as intended).

Personally, I think it's easier to initialize prev to the first character of the input string, then have the loop proceed from input[1] through the end of the string. This way we don't have to initialize prev to a value that can't be in the string (which is good, because there is no such character).

Function Naming

A function does something. As such, its name should normally be a verb, not a noun. Although it may be open to argument whether the best name for what it does is splitting, let's just go with that general idea--in which case its name should be split, not splitter.

Resulting Code

Incorporating the elements above, the code could end up something like this:

#include <string>
#include <iostream>

auto split(const std::string &input) {
    auto prev = input[0];
    std::string res{ prev };

    for (size_t i = 1; i < input.length(); i++) {
        if (input[i] != prev)
            res += ", ";
        prev = input[i];
        res += prev;
    }
    return res;
}

int main() {
    auto input = R"(gHHH5  ))YY++,,,///\)";
    std::cout << split(input);
}