Document Merger using PhpDocX I'm developing a document merger that utilizes an FTP site containing hundreds of documents.

FTP Connection Function

private static function establishFTP() {
    $ftp_conn = ftp_connect(getenv('FTP_HOST'));
    ftp_login($ftp_conn,getenv('FTP_USER'),getenv('FTP_PASS'));
    ftp_pasv($ftp_conn,true);

    return $ftp_conn;
}

I have a function that connects to the FTP site and generates an array of the documents stored inside a designated directory. It will remove the extensions from any files that are of type .docx. Then it will filter out all values containing a .. This removes any extra files that are not a Word document and also removes the FTP root pathings of . and .. from the array.

Form Generation Function

public static function generateForm() {
    $ftp_conn = self::establishFTP();

    $forms = ftp_nlist($ftp_conn,getenv('FTP_DIRECTORY'));
    $forms = str_replace('.docx','',$forms);
    $forms = array_filter($forms,function($value) {
        return strpos($value,'.') === false;
    });

    ftp_close($ftp_conn);

    return view('testDoc')->with(array('forms'=>$forms));
}

My view is very basic right now just to prove functionality. There is lots of design work that needs done to make it more user-friendly and functional.

Blade View

<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<meta name="_token" content="{{ csrf_token() }}" />
<title>Form</title>
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.2.4/jquery.min.js"></script>
<script src="https://ajax.googleapis.com/ajax/libs/jqueryui/1.11.4/jquery-ui.min.js"></script>
</head>
<body>
{{ Form::open(array('action'=>'DocumentController@mergeDocument')) }}
@foreach($forms as $doc)
    {{ Form::checkbox('documents[]',$doc) }}
    {{ Form::label(null,$doc) }}
@endforeach
{{ Form::submit('Submit',array('id'=>'submitBtn')) }}
{{ Form::close() }}
</body>
</html>

Lastly, I have a function that will accept an array of string values from the GUI. It will then establish a FTP connection and grab all documents requested from the FTP site. It will store these in a temp directory until the documents are merged and downloaded to the browser. Then it will unlink all temp files.

Merge and Download Documents Function

public static function mergeDocument() {
    $ftp_conn = self::establishFTP();

    $forms = Input::get('documents');
    $mergeDocs = array();
    $mergeName = uniqid('temp/files/merge-') . '.docx';
    $documentName = 'policies_' . date('m-d-H-i-s');

    foreach($forms as $doc) {
        ftp_get($ftp_conn,"temp/files/$doc.docx",getenv('FTP_DIRECTORY') . "/$doc.docx",FTP_BINARY);
        $mergeDocs[] = "temp/files/$doc.docx";
    }

    $firstDoc = array_shift($mergeDocs);

    ftp_close($ftp_conn);

    $mergeResult = new \Phpdocx\Utilities\MultiMerge();
    $mergeResult->mergeDocx($firstDoc,$mergeDocs,$mergeName,array('enforceSectionPageBreak'=>true));

    $phpdocx = new \Phpdocx\Create\CreateDocxFromTemplate($mergeName);
    $phpdocx->createDocxAndDownload($documentName);

    foreach($forms as $doc) {
        unlink("temp/files/$doc.docx");
    }
    unlink($mergeName);
    unlink("$documentName.docx");
}

Any suggestions as to the security and design of my app is appreciated.

Security: Credentials in ENV

It is less than ideal to store your credentials in environment variables, as they may easily leak.

phpinfo for example will print all environment variables.

Of course, you don't want to allow access to phpinfo to just anyone either, but it may still leak, and not everyone who should be allowed to see phpinfo should be allowed to see the FTP credentials.

Security: Directory Traversal which may lead to Information Leak and Limited DOS

An attacker can delete arbitrary .docx files in arbitrary locations because you never check the input for directory traversal. The problematic code is this:

$forms = Input::get('documents');
[...]
foreach($forms as $doc) {
    unlink("temp/files/$doc.docx");
}

The impact of this is rather small, but depending on what you store on your server, there may be some damage from this.

As you actually know what files are acceptable, you can easily use a whitelist approach here. At a minimum, you should check if the normalized path is inside the root dir.

Alternatively, if you do decide to cache filenames locally (see below), you could simply pass ids to the user instead of filenames, avoiding the problem altogether.

The same problem also exists here:

$forms = Input::get('documents');
[...]
foreach($forms as $doc) {
    ftp_get($ftp_conn,"temp/files/$doc.docx",getenv('FTP_DIRECTORY') . "/$doc.docx",FTP_BINARY);
    $mergeDocs[] = "temp/files/$doc.docx";
}

An attacker could read .doxc files outside of the FTP_DIRECTORY (if the FTP server allows this, and if the local server is writable at the given directory, both rather big ifs, but I would still protect against this).

Security: SSL

Your FTP connection will not be encrypted, meaning that a theoretical man in the middle can intercept your FTP credentials or the transfered files.

This may or may not be a problem for your use-case.

An alternative would be ftp_ssl_connect or using sftp (see also here).

Security/Funktionality: DOS & Performance

From what I can tell, each time I visit the website containing the form that lists files I can join, you open an FTP connection and  retrieve a list of all the files (which may be quite a lot).

A user (or multiple user) may accidentally or on purpose reload the form a bunch of times, causing performance problems.

I would definitely profile this and see what strain it puts on your server. It may make sense to cache the list of docs locally (if it doesn't change that often).

Usability / Bug / Security: File Listing

Your mechanism will hide all files which contain a dot in their filename, which may lead to hard to trace bugs.

Your current mechanism may also have security implications, as files with no extension (for example secret) are listed as well, even though they shouldn't be.

Instead, I would probably use an approach such as this:

Filter out all files that do not have the docx extension
Remove the .docx extension

Naming

You use forms in multiple places as variable name where it doesn't really make sense. The variable doesn't contain multiple forms, but actually multiple document names. This naming also leads to the odd to read foreach($forms as $doc) loop. Instead, you should name name these variables $docs (in mergeDocument, generateForm, and in the view).


"Pluggable" routes in ASP.NET MVC How do I handle multi-tenancy with routes in ASP.NET MVC?

The application is a multitenant ASP.NET MVC 3 application. Tenants are identified based on hostname so this does not interfere with routing.

Other important information:

The application has a number of features (or plugins). These are essentially other web projects with their own controllers, views and routes.
ASP.NET MVC routing serves as our entry point into these features.
One key point is that all the features function the SAME way for all tenants. The variable is whether the tenant is actually subscribed to the feature.

So imagine we have a "blog" feature with a route like so:

/blog/{instance}/posts

When our application starts we register the routes of all "routable" features. However, we need to ensure that this route is only matched if the tenant is subscribed to that feature.

Currently I'm doing this with a custom route:

private readonly IFeatureMetadata feature;

public FeatureRoute(IFeatureMetadata feature, string url, RouteValueDictionary defaults, IRouteHandler handler) :
    base(url, defaults, handler)
{
    if (feature == null)
        throw new ArgumentNullException("feature");
    this.feature = feature;
}

public override RouteData GetRouteData(System.Web.HttpContextBase httpContext)
{
    var routeData = base.GetRouteData(httpContext);

    if (routeData == null)
        return null;

    var context = DependencyResolver.Current.GetService<ISiteContext>();
    var site = context.GetSite();
    var matchedInstances = site.FeatureInstances.Where(f => f.Feature.FeatureType == feature.FeatureType);

    if (!matchedInstances.Any())
        return null;

    if (routeData.Values["instance"] == null)
    {
        routeData.Values["instance"] = matchedInstances.First().Id;
    }

    return routeData;
}

One thing that I need to cater for is routes that do not specify an instance.

For example, a tenant may have one or more blogs. If he only has one then outgoing routes should just generate:

http://[domain]/blog/posts

If he has more than one then we should generate:

http://[domain]/blog/{instance}/posts

Currently I'm handling this by registering 2 routes (one with the {instance} parameter and one without). In the code above you can see that if the instance parameter doesn't exist in RouteData we just add the id of the first matched feature instance.

This is then sufficient for me to make a call to my blog service:

GetPosts(int featureInstanceId);

While everything "works", I'm looking for feedback on my implementation. As a freelance developer this seems to be the only way I can get my code reviewed.

You could also create a route constraint that redirects or changes the routedata to "buy module".

http://msdn.microsoft.com/en-us/library/cc668201.aspx#adding_constraints_to_routes

http://www.jfarrell.net/2008/11/asp-net-mvc-authentication-strategies.html


Am I overlooking anything with this asynchronous read/write loop? Here's my code:

public static void CopyStream(Stream input, Stream output)
{
    var size = 8192;

    var inBuffer = new byte[size];
    var outBuffer = new byte[size];

    IAsyncResult writeAsync = null;

    int count;
    while (true)
    {
        var readAsync = input.BeginRead(inBuffer, 0, size, null, null);

        if (writeAsync != null)
        {
            output.EndWrite(writeAsync);
            writeAsync = null;
        }

        count = input.EndRead(readAsync);
        inBuffer.CopyTo(outBuffer, 0);

        if (count > 0)
        {
            writeAsync = output.BeginWrite(outBuffer, 0, count, null, null);
        }
        else
        {
            break;
        }
    }
}

This really is my first piece of code using the Begin/End asynchronous model.  I'm just wondering if I have any glaring issues.

Or, are there any improvements that you can see to make this even better?

It's not entirely clear what your goals are with this bit of code.  Three possibilities that come to mind:

CopyStream should be asynchronous.  As written, it does not meet this goal since it blocks on the results of EndRead and EndWrite.  If this is your goal, you need to use callback methods and not call the End methods until your callbacks have been called.  You also need to modify CopyStream's signature to return IAsyncResult and take a callback and state as parameters.
CopyStream should hide the details of the underlying asynchronous calls from its users, but is itself synchronous.  Your code accomplishes this goal, but there are a few things that could use cleaning up.
CopyStream should hide the details of the underlying asynchronous calls, and also maximize performance by making sure the writer is kept busy at all times and never sitting around idle waiting for reads, to the extent possible.  It seems like this may have been your goal due to the attempt to read/write in parallel, but by only allowing a single outstanding read or write, you're not really getting much out of it.  In order to implement this correctly, you need to use a reader thread and a writer thread, with a queue of items to be written.  The reader reads as fast as it can and inserts chunks of data into the queue, which the writer then writes as fast as it can or as fast as they show up.

EDIT:

As an example of how to write this code using a queue, see the following.  Note that I didn't need to spin up a thread because I can just make use of the EndRead/EndWrite callbacks and have the original thread wait for a signal to be raised.  Also, this is a lightly-tested first draft, so beware that there could be bugs lurking in there.  This type of code is complex and difficult to get right, and should only be used when really necessary for valid perf reasons.  If the complexity is not justified, I would just use the synchronous Read and Write methods to simplify the code as much as possible.

public static void CopyStream(Stream input, Stream output)
{
    AutoResetEvent completed = new AutoResetEvent(false);
    BeginRead(new CopyStreamState(completed, input, output));

    completed.WaitOne();
}

private static void BeginRead(CopyStreamState state)
{
    const int bufferSize = 8192;

    state.InputBuffer = new byte[bufferSize];
    state.Input.BeginRead(state.InputBuffer, 0, bufferSize, ReadCallback, state);
}

private static void ReadCallback(IAsyncResult ar)
{
    CopyStreamState state = (CopyStreamState)ar.AsyncState;

    int bytesRead = state.Input.EndRead(ar);
    if (bytesRead > 0)
    {
        byte[] dataToWrite = state.InputBuffer;

        if (bytesRead < state.InputBuffer.Length)
        {
            dataToWrite = new byte[bytesRead];
            Array.Copy(state.InputBuffer, dataToWrite, bytesRead);
        }

        EnqueueWriteData(state, dataToWrite);
        BeginRead(state);
    }
    else
    {
        state.FinishedReading = true;
    }

    BeginWriteOrComplete(state);
}

private static void EnqueueWriteData(CopyStreamState state, byte[] data)
{
    lock (state)
    {
        state.WriteQueue.Enqueue(data);
    }
}

private static void BeginWriteOrComplete(CopyStreamState state)
{
    lock (state)
    {
        if (!state.WriteInProgress)
        {
            if (state.WriteQueue.Count > 0)
            {
                byte[] outputBuffer = state.WriteQueue.Dequeue();
                state.WriteInProgress = true;
                state.Output.BeginWrite(outputBuffer, 0, outputBuffer.Length, WriteCallback, state);
            }
            else if (state.FinishedReading)
            {
                state.Completed.Set();
            }
        }
    }
}

private static void WriteCallback(IAsyncResult ar)
{
    CopyStreamState state = (CopyStreamState)ar.AsyncState;
    state.Output.EndWrite(ar);

    lock (state)
    {
        state.WriteInProgress = false;
        BeginWriteOrComplete(state);
    }
}

private class CopyStreamState
{
    public CopyStreamState(
        AutoResetEvent completed,
        Stream input,
        Stream output)
    {
        this.Completed = completed;
        this.Input = input;
        this.Output = output;
        this.WriteQueue = new Queue<byte[]>();
    }

    public AutoResetEvent Completed { get; private set; }
    public Stream Input { get; private set; }
    public Stream Output { get; private set; }
    public Queue<byte[]> WriteQueue { get; private set; }

    public byte[] InputBuffer { get; set; }
    public bool FinishedReading { get; set; }
    public bool WriteInProgress { get; set; }
}


JavaScript reducer that updates a deep value in a hash I've been roughing out a proof of concept enabling admin staff to flag submitted applications with issues as needed. We have many forms some of which are nested. So we end up with these big hashes to validate. Primarily we have issues with uploaded images (too blurry, too dark etc) but it could be any field.

So admin may see a field verified_documents -> drivers_licence -> front_image. If an issue is raised we end up with a hash to store.

{
    verified_documents: {
        drivers_licence: {
            front_image: ['Image is too blurry', 'Image is too dark']
        }
    }
}

The code works but is very smelly. The function in question is updateIssue. Which accepts a string representation of the path "verified_documents.drivers_licence.front_image" and an array of selected options from a dropdown.

The function's intentions:

Take a copy of current issues, an attempt to be pure and return a new object.
Split the string into an array.
Iterate over each value in the path array.
If path length is the last set its value to the array of issues.
If the value is an empty array set the value to undefined (only happens if an issue is removed).
If the key can't be found, create the next level as a child hash.
Returning the next level of nested object if no other condition is met.

I'm happy I was able to get it all working, but now it's not readable. Readability over performance is my objective, as this code is likely to change.

import { merge, isEmpty } from "lodash";

let Issues = undefined;

const getNestedValue = (nestedObj, pathArr) => {
  return pathArr.reduce(
    (obj, key) => (obj && obj[key] !== "undefined" ? obj[key] : undefined),
    nestedObj
  );
};

export class IssuesData {
  static updateIssue = (path, value) => {
    const valueObj = merge({}, Issues);
    const pathArr = path.split(".");
    pathArr.reduce((obj, key, i) => {
      if (pathArr.length - 1 === i) {
        if (isEmpty(value)) {
          return (obj[key] = undefined);
        }
        return (obj[key] = value);
      }

      if (obj[key] === undefined) {
        return (obj[key] = {});
      }
      return obj[key];
    }, valueObj);
    Issues = valueObj;
  };

  static getIssueByKey = path => {
    return getNestedValue(Issues, path.split("."));
  };

  static getIssues() {
    return Issues;
  }

  static setIssues(issues) {
    if (!Issues) {
      Issues = issues;
    }
  }
}

UPDATE:

After sleeping on it I managed to clean it up a little. This is more readable but there are still multiple return. If anyone has advice on cleaning it up further it'll be welcomed. I think I'll turn my attention to converting the static methods to instance methods now.

  static updateIssue = (path, value) => {
    const valueObj = merge({}, Issues);
    const pathArray = path.split(".");

    pathArray.reduce((obj, key, i) => {
      if (isLast(pathArray, i)) {
        if (isEmpty(value)) {
          return delete obj[key];
        }
        return (obj[key] = value);
      }
      return obj[key];
    }, valueObj);
    Issues = valueObj;
  };

Since you are already using lodash, don't reinvent the wheel! Nearly all of your code can be replaced by a single _.set call.

const Issues = {
  verified_documents: {
    drivers_licence: {
      front_image: ['Image is too blurry', 'Image is too dark']
    }
  }
}

console.log(Issues)
_.set(Issues, 'verified_documents.drivers_licence.front_image', [])
console.log(Issues)
.as-console-wrapper { max-height: 100% !important; }
<script src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.10/lodash.min.js"></script>

With that said, I believe there is merit to reviewing your approach.

Not mutating an object doesn't mean a function is pure. Sure, updateIssue doesn't mutate the Issues object, but it does overwrite it, making this function inherently impure.
Don't use merge to clone an object. Use clone or cloneDeep.
A class with nothing but static functions shouldn't be a class. Just use a plain object, or if you are using a proper module system, don't even bother wrapping functions in an object!
setIssues worries me. When I call a method called setSomething, I expect it to either set, or throw an error. Silently ignoring the call if Issues is already instantiated will probably lead to painful debugging later.
getNestedValue can be replaced with at.
Multiple returns is not necessarily a bad thing. The "single exit" rule is far more important when writing assembly than JavaScript. There's plenty of discussion on this. See this blog, another post, this software engineering question... plenty more to be found with a simple search.
getNestedValue looks like it has a bug, obj[key] !== "undefined" should probably have a typeof in front of it.
Your rewritten function, while cleaner, does not exactly replicate the behavior of your original implementation. If a key that does not exist in the object is specified, the original implementation will set it. The refactored function will throw.


Threshed and Malachi'd: Sieve of Eratosthenes I took a little bit from all the answers to my previous question Threshing: Sieve of Eratosthenes.

This may be bordering on code-golfing, but I think that I have a pretty good piece of code here.

Is there any major concepts that I am overlooking, although it still seems super simple?

var upperLimit = 9999;
var primes = new List<long>();
primes.Add(1);
primes.Add(2);
for (var i = 3; i < upperLimit; i+=2)
{
    primes.Add(i);
}

for (var i = 7; i < upperLimit; i+=2)
{
    foreach (var number in primes.ToList())
    {
        if (number == i) { continue; }
        if (number % i == 0) { primes.Remove(number); }
    }
}
primes.ForEach(Console.WriteLine);
Console.WriteLine("The Last Prime is " + primes[primes.Count - 1]);
Console.WriteLine("what are you waiting for? Exit the program!");

Obviously I could start off with all the primes I know by just adding them to start with and then starting my loops higher, but I think that is cheating just a little bit.

Is there any way to make this faster and able to calculate (and store) higher numbers using less memory?

This line here is horrible, terrible, and miserable:

if (number % i == 0) { primes.Remove(number); }

That one line is basically (behind the scenes) doing the following:

scan all the data from the beginning until you find the member with the value number
if you find that value:

for every remaining value, shift it one back (replace each n with the value at n + 1).
shrink the size of the List by 1.

The above is an \$O(n)\$ operation.

Using an indexed method to remove the value would be much faster, and a data structure like a Linked list with a \$O(1)\$ remove, would make a huge difference.


Algo for generating a fake, but feasable, dataset So, I was working on a dashboard for a potential customer and I needed a fake dataset with employee information to demonstrate. Mainly, I needed to know when the employee arrived the company (first swipe), when he left (last swipe) and the hours he spent in many other areas of the company (ORC is a room, DSP is another, and so on)

Basically, I create a random hour between 8am and 10am and assign this to the first swipe. I do the same for last swipe but in a range from 5pm to 7pm. Then, I calculate how many hours he worked by subtracting one from another.

With this information now I start to calculate how many hours he spent in every area of the company. The ORC is the working room, so I want to keep between 50% to 80% of hours worked there and the rest randomly assign to other areas.

I spent a lot of time in this code, and it's been a while since I created it. It's not the most pythonic code you will ever see, but it worked :D

import calendar
import datetime
import random

def random_hour(start, end):
    hour_rand = random.randint(start, end)
    minutes_rand = random.randint(0, 59)
    return datetime.timedelta(hours=hour_rand, minutes=minutes_rand)

def random_weight(total_working_hours):
    working_hours_per_area = {
        'in_orc': 0,
        'in_cafe': 0,
        'in_dsp': 0,
        'in_kiosk': 0,
        'in_training': 0,
    }

    whole_time = 100
    total_time = datetime.timedelta()

    for i, area in enumerate(working_hours_per_area):
        if whole_time < 0:
            whole_time = 0

        if i == 0:
            rand_time = random.randint(50, 80)
        else:
            rand_time = random.randint(0, whole_time)

        whole_time -= rand_time
        working_hours_per_area[area] = rand_time / 100

    total_aux = sum(working_hours_per_area.values())

    if total_aux < 1.0:
        diff = 1.0 - total_aux
        min_hour = min(working_hours_per_area.keys(), key=(lambda k: working_hours_per_area[k]))
        working_hours_per_area[area] += diff

    for area in working_hours_per_area:
        working_hours_per_area[area] = working_hours_per_area[area] * total_working_hours

    return working_hours_per_area

employees = [
    ['CHI-123', 'CLOVIS TONELADA'],
    ['CHI-456', 'JOSE DA COVA'],
    ['CHI-789', 'EMERSON PEDREIRA'],
    ['CHI-321', 'GREYCE CROQUETE'],
    ['CHI-654', 'ROBERTO PINGA'],
    ['CHI-987', 'CAROLINA DOZE AVOS'],
]

days = []
cal = calendar.Calendar()

for week in cal.monthdatescalendar(2020,9):
    for day in week:
        if day.weekday() < 5:
            days.append(day)

f = open('dataset.csv', 'w+')
f.write('Date;Employee_Name;Employee_Code;First_Swipe;Last_Swipe;Total_Working_Hours;In_ORC;In_Cafe;In_DSP;In_Kiosk;In_Training\n')

for day in days:
    for i in range(0, 6):
        date = day
        employee_name = employees[i][1]
        employee_code = employees[i][0]

        first_swipe = random_hour(8, 10)
        last_swipe = random_hour(17, 19)
        total_working_hours = last_swipe - first_swipe

        working_hours_per_area = random_weight(total_working_hours)
        total_working_hours_per_area = datetime.timedelta(hours=0, minutes=0)

        locals().update(working_hours_per_area)

        write = ';'.join([str(date), employee_name, employee_code, str(first_swipe), str(last_swipe), str(total_working_hours), \
            str(in_orc), str(in_cafe), str(in_dsp), str(in_kiosk), str(in_training)])
        f.write(write + '\n')
f.close()

You didn't include any specific request, so here are some general comments.

comments / documentation

You say it's been a while since you wrote it.  When you look at the code now, are there places you ask yourself "why did I do that?" or where it takes time to figure out what is going on?  If so, those are good places to add comments.

Also docstrings could be added to the file and the functions.

random_hour(start, end)

The writeup says it returns a random time between start and end.  However, it actually returns a random timedelta between start and end + 59 minutes.  Also, similar python functions tend to include the start and exclude the end (e.g. randrange), so it would be good to document this.

random_weight(total_working_hours)

dicts() are not guaranteed to be ordered until Python 3.7.  So i==0 may not correspond to in_orc.  It would be better to iterate over the keys and check if the key=='in_orc'.

min_hour is calculated but never used.  I think it is supposed to be area.

module level code

It is common to put the top level code in a function such as main().  And the call main() from code such as

if __name__ == '__main__':
    main()

csv module

The standard library includes the csv module for reading a writting csv and other kinfs of delimited text files.  It takes care of escaping characters or enclosing strings in quotes if needed.

unpacking

Instead of using for i in range(0,6) to iterate over the employees, use something like:

for employee_code, employee_name in employees:
    ...

locals()

The python documentation says the dictionary returned by locals() should NOT be modified.  The changes may not be picked up by the interpreter.

That's enough for now.


C program to find and replace a word in a file This program will perform a find and replace on a single word in a file, with the condition that the new word must have the same length as the old one. It will also display the number of occurrences of the old word.

I'm not interested in anything type of feedback, anything would be appreciated, it's a school assignment and it'll be reviewed by my teacher.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define WORD_SIZE 24

void write_to_file(const char *s);
void replace_word(const char *s, const char *old_word, const char *new_word);

int main(void){
    const char file_name[] = "text.txt";
    int c;
    char old_word[WORD_SIZE];
    char new_word[WORD_SIZE];
    puts("Enter 'w' to write to the file, 'r' to replace a word");
    while((c = getchar()) != EOF){
        switch(c){
        case 'w':
            getchar();
            write_to_file(file_name);
            break;
        case 'r':
            getchar();
            puts("Enter the word you want to replace");
            fgets(old_word, sizeof(old_word), stdin);
            old_word[strlen(old_word) - 1] = '\0';
            puts("Enter the new word");
            fgets(new_word, sizeof(new_word), stdin);
            new_word[strlen(new_word) - 1]  = '\0';

            if(strlen(old_word) != strlen(new_word)){
                printf("Error: can't replace \"%s\" with \"%s\":\n\"%s\" the length is not the same", old_word, new_word, new_word);
                break;
            }
            replace_word(file_name, old_word, new_word);
            break;
        default:
            printf("Invalid command:%i\n", c);
            break;
        }
    }
    return 0;
}

void write_to_file(const char *s){
    FILE *in_file;
    if((in_file = fopen(s, "w")) == NULL){
        perror(s);
        exit(EXIT_FAILURE);
    }
    int c;
    while((c = getchar()) != EOF){
        fputc(c, in_file);
    }
    fclose(in_file);
}

unsigned long fsize(const char *s){
    FILE *f;
    if((f = fopen(s, "r")) == NULL){
        perror(s);
        exit(EXIT_FAILURE);
    }
    fseek(f, 0, SEEK_END);
    unsigned long len = ftell(f);
    fclose(f);
    return len;
}

void replace_word(const char *s, const char *old_word, const char *new_word){
    FILE *original_file;
    FILE *copy;

    if((original_file = fopen(s, "r")) == NULL){
        perror(s);
        exit(EXIT_FAILURE);
    }

    if((copy = fopen("copy.txt", "w")) == NULL){
        perror("text");
        exit(EXIT_FAILURE);
    }

    const int BUFFER_SIZE = fsize(s);
    char *buffer = malloc(BUFFER_SIZE);
    char *init_loc = buffer;

    int word_len = strlen(old_word);
    int word_frequency = 0;

    while(fgets(buffer, BUFFER_SIZE, original_file)){
        while((buffer = strstr(buffer, old_word))){
            memcpy(buffer, new_word, word_len);
            word_frequency++;
        }
        buffer = init_loc;
        fprintf(copy, "%s", buffer);
    }
    printf("'%s' found %i times\n", old_word, word_frequency);

    fclose(original_file);
    fclose(copy);

    if((original_file = fopen(s, "w")) == NULL){
        perror(s);
        exit(EXIT_FAILURE);
    }

    if((copy = fopen("copy.txt", "r")) == NULL){
        perror("text");
        exit(EXIT_FAILURE);
    }

    int c;
    while((c = fgetc(copy)) != EOF){
        fputc(c, original_file);
    }

    fclose(original_file);
    fclose(copy);
    free(buffer);
}

Small suggestions:

Be consistent. What jumps out right away is that you say "This program will perform a find and replace on a single word in a file" but then  your program prompts the user to either write to file or replace a word.  That confuses me as a user of your program as to what it's supposed to do.
Removed. Refer to Difference between int and char in getchar/fgetc and putchar/fputc?

You've declared int c in main(). Why int and not char ?  This also comes up when your case statement reaches the default part. When user enters wrong command, say g, it displays:

Enter 'w' to write to the file, 'r' to replace a word
g
Invalid command:103
Invalid command:10

 So avoid confusing the user and the people who are going to read your code.
In fgets(old_word, sizeof(old_word), stdin); why do you use sizeof(old_word) ? You already have a constant for WORD_SIZE defined, so there's no need to figure out size of your string.
Consider moving the code from case 'r' for getting user's input to replace_word() function or separate function get_user_input().  What was nice about your case statement is that you started out with case 'w' where you executed a specific function for that case. That's good strategy. Use that case statement to direct the program to specific functions that do specific job.
When user selects to replace a word, you give the user instructions to enter old and new string. That's good - user is not left hanging and is guided in what to do. But when user selects to write to file, there's no instruction and user is just dropped into typing stuff.

I only clicked Ctrl+D ( to send EOF character since I'm Linux user, not sure how would that work on Windows) because I know that this is what program expects to receive to indicate that there's no more input, but what about a random user who's absolutely unfamiliar with your program and never read your source code ?
On the same related note, how does user exit your program ? In your case statement you only have two cases for specific action, there's no quit option, and neither does it explain to the user that they could use Ctrl+C or Ctrl+D to quit.
Give the user Enter 'w' to write to the file, 'r' to replace a word  each time an action has been complete. Take for example the example I showed in my first point. User enters wrong command, they see an error and . . . what's next ? The program just sits there waiting for input without explaining to the user what it wants.


Calculate karma based on retweets This script is meant to look for an id in the data attribute in each containment div, then send an Ajax call to get the amount of retweets, calculate the karma based on those retweets and then display that number in a div.

This code looks like spaghetti. How do I improve this to make each function more independent?  There is some abstraction function that I can't figure out.

My other question is, how can I use a jQuery deferred object to create a promise and update the div once I get a response?

function get_retweet(id) {
    var url = 'https://api.twitter.com/1/statuses/show/' + id + '.json',
        karma;

    $.ajax({
        url: url,
        type: 'GET',
        dataType:'jsonp',
        crossDomain: true,
        success: function(data) {
            display_karma(data.retweet_count);
        },
        error: function() {alert('fail');}
    }));
}

function calc_karma(tweets) {
    return tweets *10 +10;
}

function display_karma(retweets) {
    var id, karma,
        el =$('.tweetContainer');
    //id = $(el).data(el, tweet_id);
    id = '248988915661410304';
    karma = calc_karma(retweets);
    el.find('.tcPoints').text(karma);
}

function start_get_karma() {
    var id;

    $('.tweetContainer').each(function(index,el) {
        //id = $(el).data(el, tweet_id);
        id = '248988915661410304';
        get_retweet(id);
    });
}

Here are some tips:

Use $.getJSON() to get JSON.

Also, name the first parameter json for clarity.

Note from documentation for jQuery.getJSON()

  JSONP

  If the URL includes the string "callback=?" (or similar, as defined by
  the server-side API), the request is treated as JSONP instead. See the
  discussion of the jsonp data type in $.ajax() for more details.

More information here

Old Code:

var url = 'https://api.twitter.com/1/statuses/show/' + id + '.json',
    karma;
$.ajax({
    url: url,
    type: 'GET',
    dataType:'jsonp',
    crossDomain: true,
    success: function(data) {
        display_karma(data.retweet_count);
    },
    error: function() {alert('fail');}
}));

New Code:

var url = 'https://api.twitter.com/1/statuses/show/' + id + '.json?callback=?';
$.getJSON( url, function (json) {
    display_karma(json.retweet_count);
}).error(function () {
    alert('fail');
});

Eliminate commented code. Use a source control system like git, svn, mercurial to keep track of changes.

Here's a good start for it

Old Code:

function start_get_karma() {
    var id;

    $('.tweetContainer').each(function(index,el) {
        //id = $(el).data(el, tweet_id);
        id = '248988915661410304';
        get_retweet(id);
    });
}

New Code:

function start_get_karma() {
    var id = '248988915661410304';
    $('.tweetContainer').each(function (index, el) {
        get_retweet(id);
    });
}

Function calls are expensive, so use a basic loop instead of each() when appropriate.

Old Code:

$('.tweetContainer').each(function (index, el) {
    get_retweet(id);
});

New Code A:

for(var i = 0; len = $('.tweetContainer').length; i < len; i++){
    get_retweet(id);
}

However, there's a problem. Making multiple calls to get_retweet() with the same static value doesn't make sense.  So just make one call.

New Code B:

if( $('.tweetContainer').length ){
    get_retweet(id);
}

Don't declare variables if they're only used once.

Old Code:

function display_karma(retweets) {
    var id, karma,
        el =$('.tweetContainer');
        id = '248988915661410304';
    karma = calc_karma(retweets);
    el.find('.tcPoints').text(karma);
}

New Code:

function display_karma(retweets) {
    var id = '248988915661410304';
    $('.tweetContainer').find('.tcPoints').text(calc_karma(retweets));
}

Have variable names give a hint to the type.

retweets sounds like a function or array. Try naming it retweet_amount or something similar.

Final Code:

function calc_karma(tweets) {
    return (tweets * 10) + 10;
}
function display_karma(retweet_amount) {
    $('.tweetContainer').find('.tcPoints').text(calc_karma(retweet_amount));
}
function get_retweet(id) {
    var url = 'https://api.twitter.com/1/statuses/show/' + id + '.json?callback=?';
    $.getJSON( url, function (json) {
        display_karma(json.retweet_count);
    }).error(function () {
        alert('fail');
    });
}
function start_get_karma() {
    var id = '248988915661410304';
    if( $('.tweetContainer').length ){
        get_retweet(id);
    }
}


Swapping dynamic page div's around on button click The following working code basically swaps the content of two divs.  The divs are created dynamically, then the user checks which records they want to swap, and then clicks on a swap button that triggers a function to swap an inner div element.

The part that swaps the div's seems really messy to me. I'm only a beginner/intermediate with JavaScript, so I would really appreciate some help with this.

//dynamically create the div elements
$('#SwapBedsFields').html(options);
   for (var i = 1; i <= NumberOfBeds; i++) {
        var RowIndex = $.inArray(i.toString(), bedNumberColumn);
        var ptName = 'Empty';
        var UMRN = "";
        var UMRNstr = '';
        var pID = '';

       if (RowIndex != -1) {
           ptName = patientNameColumn[RowIndex];
           UMRN = patientUMRNColumn[RowIndex];
           pID = pIDColumn[RowIndex];
           UMRNstr = "(" + UMRN + ")";
       };
       options += '
<input type="checkbox" style="vertical-align:middle;" name="bed-' + i + '"';
       options += 'id="bed-' + i + '" />
&nbsp;Bed ' + i + '  -

' + ptName + '&nbsp;' + UMRNstr + '

';
};

//***THIS IS THE BIT I FIND PARTICULARLY MESSY!***
//then in another function - when the user clicks a button to swap the div elements...
var selected = new Array();
var patientname1;
var patientname2;
var b1html;
var b2html;
var pt1UMRN;
var pt2UMRN;

$('#SwapBedsFields input:checked').each(function () {
       selected.push($(this).attr('name'));
});

//get the html of the two bed divs
patientname1 = $('#bed-div-' + selected[0].substring(4)).html();
patientname2 = $('#bed-div-' + selected[1].substring(4)).html();
pt1UMRN = $('#bed-div-' + selected[0].substring(4)).attr('umrn');
pt2UMRN = $('#bed-div-' + selected[1].substring(4)).attr('umrn');

//swap the elements around
$('#bed-div-' + selected[0].substring(4)).html(patientname2);
$('#bed-div-' + selected[1].substring(4)).html(patientname1);

//update the umrn attribute
$('#bed-div-' + selected[0].substring(4)).attr({ umrn: pt2UMRN});
$('#bed-div-' + selected[1].substring(4)).attr({ umrn: pt1UMRN });

//message the user
$('#bed-topdiv-' + selected[0].substring(4)).effect("highlight", {}, 2000);
$('#bed-topdiv-' + selected[1].substring(4)).effect("highlight", {}, 2000);

You could improve it a little like this, at least you are not selecting the same elements over and over.

I do think it could improve more, if you swapped the actual div's rather then the html content and umrn value.

var $bed0 = $('#bed-div-' + selected[0].substring(4));
var $bed1 = $('#bed-div-' + selected[1].substring(4));

//get the html of the two bed divs
patientname1 = $bed0.html();
patientname2 = $bed1.html();
pt1UMRN = $bed0.attr('umrn');
pt2UMRN = $bed1.attr('umrn');

//swap the elements around
$bed0.html(patientname2);
$bed1.html(patientname1);

//update the umrn attribute
$bed0.attr({ umrn: pt2UMRN});
$bed1.attr({ umrn: pt1UMRN });

//message the user
$bed0.effect("highlight", {}, 2000);
$bed1.effect("highlight", {}, 2000);


Count the number of ways an integer can be represented as a sum of consecutive positive integers I am trying to count the number of ways an integer can be represented as sum of 2 or more consecutive positive integers.  My Code is working in under 1 second for small inputs (\$\le 10^7\$) but after that it's taking too long. How can I lower the time complexity of the solution?

import java.io.*;

//sum needs to contain atleast 2 elements
public class IntegerRepresentedAsSumOfConsecutivePositiveIntegers
{
    public static long count = 0;
    public static void main(String[] args) throws IOException
    {
        BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
        long num = Long.parseLong(br.readLine()); //Enter a number( <=10^12)
        //driver(num);
        func(num);
        System.out.println("count = " + count);
    }

    public static void func(long num)
    {
        long temp,i,j;
        long limit = (num / 2);
        for(i = 1 ; i <= limit ; i++)
        {
            temp = num;
            j = i;
            while(j < temp)
            {
                temp = temp - j;
                j++;
            }
            if(j == temp)
            {
                count++;
            }
        }
    }

}

Some ideas that can improve the complexity:

I am seeing that you are trying to equate num to i + (i+1) + ... + j, you can calculate this by j*(j+1)/2 - i*(i-1)/2
Further for a given i if we assume that num = j*(j+1)/2 - i*(i+1)/2 we can solve the quadratic equation and check if we get integral roots, that makes the whole solution O(n)
You may also wanna try looking up solutions to quadratic Diophantine equations, see here, after all j*(j+1)/2 - i*(i-1)/2 = num is a quadratic diophantine equation only.

What I would probably do

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;

public class ConsSum {

    public static void main(String[] args) throws NumberFormatException, IOException {
        BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
        long num = Long.parseLong(br.readLine());
        System.out.println("There are " + getConsSumWays(num) + " total way(s) to get " + num);
    }

    private static int getConsSumWays(long n) {
        int count = 0;
        /*
         * i + (i+1) + (i+2) + ... + j = n
         * => j(j+1)-i(i-1)=2n
         * => let k = j(j+1) = 2n + i(i-1)
         */
        for (long i = 0; i <= n / 2; i++) {
            long k = 2 * n + i * (i - 1);
            /*
             * j^2+j-k=0 => j = [-1+sqrt(1+4k)]/2 since j>0
             */
            long j = (long) ((-1 + Math.sqrt(1 + 4 * k)) / 2);
            if (j * (j + 1) == k) {
                System.out.println("sum [" + i + ".." + j + "] = " + n);
                count++;
            }
        }
        return count;
    }

}


A utility to swap two files While I have never had to swap two files I have always wondered if there was command for swapping two files and recently I decided to see if there was one. Eventually I found out that there wasn't one and as a result I decided to create one.
Here is the code: main.cc
#include <iostream>
#include <filesystem>
#include <cstring>
#include <cassert>

static auto print_help() -> void {
    std::cout << "Usage: swap [file1] [file2]\n";
    std::cout << "swaps the contents of file1 and file2\n";
    std::cout << "use swap --help to print this message\n";
}

static auto validate_files(const std::filesystem::path& file1_path, const std::filesystem::path& file2_path) -> void {
    {
        /* check if file exists */
        const auto file1_exists = std::filesystem::exists(file1_path);
        const auto file2_exists = std::filesystem::exists(file2_path);
        const auto exists = file1_exists && file2_exists;

        if (!exists) {
            if (!file1_exists) std::cerr << "cannot find file " << file1_path << '\n';
            if (!file2_exists) std::cerr << "cannot find file " << file2_path << '\n';
            exit(EXIT_FAILURE);
        }
    }

    {
        if (file1_path == file2_path) {
            std::cerr << "swaping the same two files does nothing\n";
            exit(EXIT_SUCCESS);
        }
    }
}

static auto get_temp_filename(char* template_name) -> void {
    /* tmpnam_s does not work on linux */
#if defined(WIN32) || defined(_WIN32)
    errno_t err = tmpnam_s(template_name, L_tmpnam);
    assert(!err);
#else
    int err = mkstemp(template_name);
    assert(err != -1);
#endif
}

int main(int argc, char** argv) {
    std::ios::sync_with_stdio(false);
    switch (argc) {
        case 2: {
            /* convert the second arg to upper case */
            std::transform(argv[1],
                argv[1] + strlen(argv[1]),
                argv[1],
                ::toupper);
            if (!strcmp(argv[1], "--HELP")) {
                print_help();
                return EXIT_SUCCESS;
            }
            else {
                std::cerr << "Invalid args see --help for usage\n";
                return EXIT_FAILURE;
            }
        }
        case 3:
            break;
        default: {
            std::cerr << "Invalid args see --help for usage\n";
            return EXIT_FAILURE;
        }
    }
    const auto file1_path = std::filesystem::path{ argv[1] };
    const auto file2_path = std::filesystem::path{ argv[2] };

    validate_files(file1_path, file2_path);
    char temp_filename[L_tmpnam] = "XXXXXX";
    get_temp_filename(temp_filename);
    const auto temp_filepath = std::filesystem::path{ temp_filename };
    /* move-swap the files instead of copy-swaping */
    /* renaming a file is the same as moving it */
    std::filesystem::rename(file1_path, temp_filepath);
    std::filesystem::rename(file2_path, file1_path);
    std::filesystem::rename(temp_filepath, file2_path);
}

Here is an example of usage:
swap doc1.txt doc2.txt

file1_path == file2_path surely tells that paths refer to the same file. However, even if file1_path != file2_path they still may refer to the same file.

file1_exists = std::filesystem::exists(file1_path); introduces a TOC-TOU race condition. The file may exist at the time of test, yet disappear by the time of use. See the next bullet.

std::filesystem::rename may fail. You call it tree times. If the second, or third call fails (with an exception!), the filesystem ends up not exactly in a state one would expect. Use a noexcept overload, test the error_code after each call, and roll back all actions prior to failure. That would also automagically take care of the nonexistent paths.

Do not assert. It is only good to catch bugs, not the runtime problems. In the production code (compiled with -DNDEBUG) it does nothing, and your program wouldn't detect an mkstemp failure.

The program silently does nothing if called with, say, 4 arguments. It also takes so much effort if called with 2 arguments. Calling print_help() any time argc != 3 is much more straightforward.


Converting dataframe columns to binary columns I basically want to convert all categorical column in an R dataframe to several binary columns:

For example, a categorical column like

Company
-------
IBM
Microsoft
Google

will be converted to 3 columns:

Company_is_IBM   Company_is_Microsoft    Company_is_Google

 1                 0                       0
 0                 1                       0
 0                 0                       1

However, my code runs very slowly on my testing data, which should generate a 900 column*367 row data frame.

I'm attaching the code in to test with my dataset, simply by extracting the source code package, set working directory (setwd) to the source code files directory like shown in the picture (in archive), and run:

gpudataf = read.table("gpudataf.txt")
gpudataf_bin = ConvertCategoricalDataFrameToBinaryKeepingOriginals(gpudataf)

and you will see how slow it is.

Source code archive

source("IsCategorical.R")

# Function CategoricalToBinary: Take a data.frame, determine which columns are categorical,
# if categorical, convert the categorical column to several binary columns with values 0 and 1

#input: a Categorical Column, name of that column. Output: a data frame of multiple     binary columns.
ConvertCategoricalColumnToBinaryColumns <- function(catecol,prefix = "")
{
  set_values = unique(catecol)
  returnvalue = data.frame("ru9uGEQu" = catecol) #temporary frame no one will hit
  returnvalue[["ru9uGEQu"]] <- NULL #I just did it for the number_rows compatible!

  for(val in set_values){
    newcolName = paste(prefix, val, sep = "_is_")
    returnvalue[[newcolName]] <- Map(function(x){return (if(x==val) 1 else 0)}, catecol)
  }

  return(returnvalue)
}

#input: a dataframe_with_categorical columns, and extra categorical column names,
# if that column is categorical
# but all of numerical denotations
#output: the categorical columns are replaces by binary columns.

ConvertCategoricalDataFrameToBinary <- function(dataframe_with_cat_column, categoricalColumns=list())
{
  colNames= colnames(dataframe_with_cat_column)
  returnvalue = data.frame("ru9uGEQu" = dataframe_with_cat_column[[colNames[1]]])
  returnvalue[["ru9uGEQu"]] <- NULL

  for(columnName in colnames(dataframe_with_cat_column)){
    # print(columnName)
    if(IsCategorical(dataframe_with_cat_column[[columnName]]) || (columnName %in% categoricalColumns)){
      binary_dataframe = ConvertCategoricalColumnToBinaryColumns(dataframe_with_cat_column[[columnName]],prefix = columnName)
      # attach/insert it to the returnvalue
      for(binColName in colnames(binary_dataframe)){
        returnvalue[[binColName]] = binary_dataframe[[binColName]]
      }
    }
    else{
      # that column is numerical, don't change it.
      returnvalue[[columnName]] = dataframe_with_cat_column[[columnName]]
    }
  }
  return(returnvalue)
}

# if you want to keep the original values in the returned dataframe.

ConvertCategoricalDataFrameToBinaryKeepingOriginals <- function(dataframe_with_cat_column, categoricalColumns=list())
{
  without = ConvertCategoricalDataFrameToBinary(dataframe_with_cat_column, categoricalColumns)
  returnvalue = dataframe_with_cat_column

  existing_colNames = colnames(returnvalue)

  for(coln in colnames(without)){
    if(!(coln %in% existing_colNames)){
      returnvalue[[coln]] = without[[coln]]
    }
  }
  return(returnvalue)
}

model.matrix already does most of what you want:

company <- c("IBM", "Microsoft", "Google");
model.matrix( ~ company - 1)


Creating pages containing grids I'm making an app that creates pdfs of images layed out in a grid. I know the size of each grid element and how many fit on a page, it works out to a 5x6 grid.
The syntax is not the real focus, but the logic for how I determine when to start a new row or page.

const pageconstants = {
 grid:{
   x: 5,
   y: 4,
 }
}

const griditems = []//an array containing 35 elements

const doc = new PDFDocument(
    {
        layout : 'portrait',
        width: 612,
        height: 792,
        autoFirstPage: false
    }
);

let gridx = 0;
let gridy = 0;
let pagenum = 1;

createNewPage(doc);//create new page and elements like header and footer

griditems.forEach((item) => {
    var x = 100 * gridx;//The actual equation has more parts, I'm simplifying here
    var y = 100 * gridy;

    addGridItem(doc, { x: x, y: y }, item)//places the image on the grid

    if( gridx == pageconstants.grid.x ){
      gridx = 0;

      if( gridy == pageconstants.grid.y ){
        subjecty = 0;
        pagenum++;
        createNewPage(doc)
      }
      else{
        subjecty++;
      }
    }
    else{
      subjectx++;
    }
})

So basically, it checks if the current gridx has reached the max of 5 (6 because it starts at zero), and if so, resets gridx, and increments gridy to start the next row.

If gridy reaches it's maximum, gridx and gridy are reset and a new page is created again, so it can go again until there are no items left.
This all works, but it feels really word, I really hate having nested if's like this, and incrementing all these numbers. Is there a more elegant way to achieve this?

So essentially, you want to layout an array of items in a 2D grid. This problem can be solved by using division and remainder operations to calculate the coordinates.

x is the remainder of dividing the index by maxX
y is the remainder of dividing by maxY the result of dividing the index by maxX.
A new page made is when both x and y are 0

Here's a rough sketch of how it looks:

let i = item index
let maxX = 5
let maxY = 6
let x = i % maxX
let y = (i / maxX) % maxY
let p = !(i % (maxX * maxY))

i: 0 1 2 3 4 5 6 7 8 9 ... 29 30 31 32 33 34 35
x: 0 1 2 3 4 0 1 2 3 4 ... 4  0  1  2  3  4  0
y: 0 0 0 0 0 1 1 1 1 1 ... 5  0  0  0  0  0  1
p: 1 0 0 0 0 0 0 0 0 0 ... 0  1  0  0  0  0  0

In code, this would be:

const griditems = []

const pageconstants = {
 grid:{
   x: 5, // 5 across
   y: 6, // 6 down
 }
}

const doc = new PDFDocument({
  layout : 'portrait',
  width: 612,
  height: 792,
  autoFirstPage: false
})

const getX = i => i % pageconstants.grid.x
const getY = i => Math.floor((i / pageconstants.grid.x) % pageconstants.grid.y)
const isNewPage = i => !(i % (pageconstants.grid.x * pageconstants.grid.y))

const positions = griditems.forEach((item, index) => {
  const x = 100 * getX(index)
  const y = 100 * getY(index)

  if (isNewPage(index)) createNewPage(doc)

  addGridItem(doc, { x, y }, item)
})


Improving the running time of finding the count of pairs satisfying a symmetric relation I had an online coding test yesterday. The question is not hard to solve, but I could not achieve the required running time. The question is as follow:

Given \$1 <= M\$, \$N <= 10^5\$ find the count of pairs A, B:

1 <= A <= M and 1 <= B <= N
\$(A^{1/3} + B^{1/3})^3\$ is an integer
Here was my solution:

I notice that this relationship is symmetric, so if assuming M >= N, and imagining we are searching valid points on a M*N matrix, I only need to search the following three parts of the matrix:

Items above the diagonal: (i, j) where 1 <= i < j <= N. Increase the counter by 2 if a valid pair has found. (Symmetric relation)
Diagonal elements.
Items int (i, j) where N+1 <= i <= M, and 1<= j <= N

My java solution is as the following

        static int cubeNumbers(int M, int N) {
            int count = 0;
            if (N > M) {
                int temp = N;
                N = M;
                M = temp;
            }
            for (int i = 1; i <= N; i ++) {
                for (int j = i + 1; j <= N; j ++) {
                    if (isValid(i, j)) {count += 2;}
                }
                if (isValid(i, i)) {count += 1;}
            }
            for (int i = N + 1; i <= M; i ++) {
                for (int j = 1; j <= N; j ++) {
                    if (isValid(i, j)) {count += 1;}
                }
            }
            return count;
        }

        private static boolean isValid(int A, int B) {
            double result = Math.pow(Math.pow(A, 1.0/3) + Math.pow(B, 1.0/3), 3);
            return (result % 1) == 0;
        }

You are going in wrong direction. Entertaining the symmetry doesn't change the quadratic nature of the algorithm (using floating point in a seemingly number-theoretical problem also raises a red flag). The correct approach involves some math.

We are interested in integers \$a,b,n\$ satisfying the relation

$$\sqrt[3]{a} + \sqrt[3]{b} = \sqrt[3]{n}$$

Notice that if \$a\$ and \$b\$ have a common factor, \$n\$ necessarily has the same factor, so we are primarily interested in the solutions with \$a\$ and \$b\$ coprime.

We are going to show that such nontrivial solution is only possible if \$a\$ and \$b\$ are perfect cubes.

Now, \$(\sqrt[3]{a} + \sqrt[3]{b})^3 = n\$. Expanding the binomial gives $$3\sqrt[3]{ab}(\sqrt[3]{a} + \sqrt[3]{b}) = n - (a + b)$$

that is,

$$3\sqrt[3]{nab} = n - (a + b)$$

Since the RHS is obviously integer, so must be LHS. Assuming that \$a\$ is not a perfect cube, it must have some prime factor \$p\$ in a non-cubic power. Further assuming that \$b\$ is coprime to \$a\$, the remaining powers of \$p\$ must be supplied by \$n\$, that is \$n\$ is also divisible by \$p\$.

It means that LHS is divisible by \$p\$, and since \$b = n - a - 3\sqrt[3]{nab}\$, it follows that \$b\$ is also divisible by \$p\$, which contradicts the assumption.

We proved that the set of solutions is comprised by pairs of

$$A = x^3 z, B = y^3 z$$

with \$x, y\$ being coprime integers, and \$z\$ being an arbitrary integer. Enumerating such pairs shall not be a problem.


Finding nearest power of 2 I have the following simple program which calculates and returns the nearest power of 2 of a given number. Because I use increments and decrements to find the closest power, this takes a very long time when working with higher numbers. Is there any way to reduce the run time of the program when working with larger numbers?

puts "Enter the value of n:"
n = gets.chomp.to_i

def ispow2(n)
    n & (n - 1) == 0
end

if ispow2(n)
    puts "#{n} is the closest power of 2 to #{n}."
    exit
end

x = n
y = n

until ispow2(x)
    x += 1
    ispow2(x)
end

until ispow2(y)
    y -= 1
    ispow2(y)
end

if (n - y) == (x - n)
    puts "#{x} and #{y} are the closest powers of 2 to #{n}."
else if (n - y) < (x - n)
    puts "#{y} is the closest power of 2 to #{n}."
else
    puts "#{x} is the closest power of 2 to #{n}."
end

Performance

What's the next power of 2 smaller than this number (binary presentation)?

0b00100101010010011

It's all the 1's reset except the first:

0b00100000000000000

What's the next power of 2 greater? It's smaller number shifted to left:

0b01000000000000000

The implementation of this logic will be both faster and simpler.

However, be mindful of the special cases when the target number is exactly a power of 2 or zero.

else if

This didn't work on my computer:

if (n - y) == (x - n)
    puts "#{x} and #{y} are the closest powers of 2 to #{n}."
else if (n - y) < (x - n)
    puts "#{y} is the closest power of 2 to #{n}."
else
    puts "#{x} is the closest power of 2 to #{n}."
end

I had to change the else if to elsif.

Don't repeat yourself

Instead of this:

if (n - y) == (x - n)
    puts "#{x} and #{y} are the closest powers of 2 to #{n}."
elsif (n - y) < (x - n)
    puts "#{y} is the closest power of 2 to #{n}."
else
    puts "#{x} is the closest power of 2 to #{n}."
end

It would be better to reduce the duplication in the printing:

if (n - y) == (x - n)
    puts "#{x} and #{y} are the closest powers of 2 to #{n}."
else
    if (n - y) < (x - n)
        result = y
    else
        result = x
    end
    puts "#{result} is the closest power of 2 to #{n}."
end


Weekly fee calculation I have the following query.  Its execution took 15 min and it's too much slow.  Is there a way to optimize it?

SELECT
 Id,Fees,WeekOfMonth,CONVERT(NVARCHAR(MAX), StartDate, 103) AS StartDate,CONVERT(NVARCHAR(MAX), EndDate, 103) AS EndDate,'Temp Fees                      ' AS FeesName
  ,@MonthName AS [MonthName]
  INTO ##TempFeesMonthly
  from
  (
SELECT DISTINCT
1 as Id,sum(((CNTI_THPAYE *(CNTI_THFACT / CNTI_THPAYE)) *
(CASE WHEN CNTI_DURHEBDO IS NULL THEN 1 ELSE CNTI_DURHEBDO/5 END)*@NumberOfDays)) AS Fees,WeekOfMonth,StartDate,EndDate
FROM SCHHAYS.dbo.WTVTAT TAT
      LEFT JOIN
    SCHHAYS.dbo.WTTIEINT INT
        ON (
            TAT.TIE_ID = INT.TIE_ID
        )
        AND (
            TAT.VTAT_IORDRE = INT.TIEI_ORDRE
        )
    LEFT JOIN
    SCHHAYS.dbo.PYCONTRAT CC
        ON TAT.PER_ID = CC.PER_ID
        AND TAT.CNT_ID = CC.CNT_ID
    LEFT JOIN
    SCHHAYS.dbo.CMTIERS T
        ON TAT.TIE_ID = T.TIE_ID
    LEFT JOIN
    SCHHAYS.dbo.WTMISS M
        ON CC.PER_ID = M.PER_ID
        AND CC.CNT_ID = M.CNT_ID
    LEFT JOIN
    ##WTCNTIWeek COT1
        ON M.PER_ID = COT1.PER_ID
        AND M.CNT_ID = COT1.CNT_ID
    INNER JOIN
    SCHHAYS.dbo.WTPRH AS PRH
        ON M.PER_ID = PRH.PER_ID
        AND M.CNT_ID = PRH.CNT_ID
        AND M.TIE_ID = PRH.TIE_ID
    INNER JOIN
    ##tempStartEndWeekDates AS Tsed
        ON     PRH_DTEDEBSEM>=Tsed.StartDate
        AND PRH_DTEFINSEM<=Tsed.EndDate
    LEFT JOIN
    SCHHAYS.dbo.WTSCCT C
        ON CC.RGPCNT_ID = C.RGPCNT_ID
        AND CC.PER_ID = C.PER_ID
        AND CC.CNT_ID = C.CNT_ID
    INNER JOIN
    ##TempHaysStaffWeek HF
        ON C.VAPO_CODE = HF.onetouch  COLLATE Latin1_General_CI_AS
group by
WeekOfMonth,StartDate,EndDate)t

--CREATE INDEX IDX_TempFeesMonthly ON ##TempFeesMonthly(WeekOfMonth)
--Calcul Temp Margin
UNION ALL
--INSERT INTO ##TempFeesMonthly(Id,Fees,WeekOfMonth,StartDate,EndDate,FeesName,[MonthName])
SELECT
 2,  sum(Fees) AS Fees  ,WeekOfMonth,CONVERT(NVARCHAR(MAX), StartDate, 103) AS StartDate,CONVERT(NVARCHAR(MAX), EndDate, 103) AS EndDate,'Temp Margin ' AS FeesName
  ,@MonthName AS [MonthName]
  from
  (
 SELECT DISTINCT
sum((CASE WHEN CNTI_DURHEBDO IS NULL THEN 1 ELSE CNTI_DURHEBDO/5 END)*@NumberOfDays)-(CNTI_THPAYE *(CASE WHEN CNTI_DURHEBDO IS NULL THEN 1 ELSE CNTI_DURHEBDO/5 END)*1.453*1.21*@NumberOfDays) AS Fees,WeekOfMonth,StartDate,EndDate
FROM SCHHAYS.dbo.WTVTAT TAT
      LEFT JOIN
    SCHHAYS.dbo.WTTIEINT INT
        ON (
            TAT.TIE_ID = INT.TIE_ID
        )
        AND (
            TAT.VTAT_IORDRE = INT.TIEI_ORDRE
        )
    LEFT JOIN
    SCHHAYS.dbo.PYCONTRAT CC
        ON TAT.PER_ID = CC.PER_ID
        AND TAT.CNT_ID = CC.CNT_ID
    LEFT JOIN
    SCHHAYS.dbo.CMTIERS T
        ON TAT.TIE_ID = T.TIE_ID
    LEFT JOIN
    SCHHAYS.dbo.WTMISS M
        ON CC.PER_ID = M.PER_ID
        AND CC.CNT_ID = M.CNT_ID
    LEFT JOIN
    ##WTCNTIWeek COT1
        ON M.PER_ID = COT1.PER_ID
        AND M.CNT_ID = COT1.CNT_ID
    INNER JOIN
    SCHHAYS.dbo.WTPRH AS PRH
        ON M.PER_ID = PRH.PER_ID
        AND M.CNT_ID = PRH.CNT_ID
        AND M.TIE_ID = PRH.TIE_ID
    INNER JOIN
    ##tempStartEndWeekDates AS Tsed
        ON     PRH_DTEDEBSEM>=Tsed.StartDate
        AND PRH_DTEFINSEM<=Tsed.EndDate
    LEFT JOIN
    SCHHAYS.dbo.WTSCCT C
        ON CC.RGPCNT_ID = C.RGPCNT_ID
        AND CC.PER_ID = C.PER_ID
        AND CC.CNT_ID = C.CNT_ID
    INNER JOIN
    ##TempHaysStaffWeek HF
        ON C.VAPO_CODE = HF.onetouch  COLLATE Latin1_General_CI_AS
group by
WeekOfMonth,StartDate,EndDate,CNTI_THPAYE,CNTI_DURHEBDO)t
GROUP BY WeekOfMonth,StartDate,EndDate

Indentation is your best friend when nesting SQL Queries
Be consistent with your casing on keywords, SCREAMCASE is the standard for keywords in SQL
Be consistent with your Alias syntax, sometimes it was written table t and others it was written table AS t
Use newlines in your select statements, especially when they are complex
INT is not a good table alias, change it
I have no clue what any of these tables are, and I can't tell from the aliases either.
Naming,

I recognized like 4 field names

MonthName
WeekOfMonth
StartDate
EndDate

Some Temp Tables
and some variables that aren't assigned anywhere in the code you have given us.

Spacing in your equations

I took your code and I cleaned up the formatting so that it might be easier for others to read it and provide you a better review, here is the cleaned up code using standard spacing, casing, and other standards that I could implement.

SELECT
    Id
    ,Fees
    ,WeekOfMonth
    ,CONVERT(NVARCHAR(MAX), StartDate, 103) AS StartDate
    ,CONVERT(NVARCHAR(MAX), EndDate, 103) AS EndDate
    ,'Temp Fees                      ' AS FeesName
    ,@MonthName AS [MonthName]
INTO ##TempFeesMonthly
FROM
(
    SELECT DISTINCT
        1 AS Id
        ,sum((CNTI_THPAYE * (CNTI_THFACT / CNTI_THPAYE)) * (CASE WHEN CNTI_DURHEBDO IS NULL THEN 1 ELSE CNTI_DURHEBDO / 5 END) * @NumberOfDays) AS Fees
        ,WeekOfMonth
        ,StartDate
        ,EndDate
    FROM SCHHAYS.dbo.WTVTAT AS TAT
        LEFT JOIN SCHHAYS.dbo.WTTIEINT [INT]
            ON TAT.TIE_ID = INT.TIE_ID
            AND TAT.VTAT_IORDRE = INT.TIEI_ORDRE
        LEFT JOIN SCHHAYS.dbo.PYCONTRAT CC
            ON TAT.PER_ID = CC.PER_ID
            AND TAT.CNT_ID = CC.CNT_ID
        LEFT JOIN SCHHAYS.dbo.CMTIERS T
            ON TAT.TIE_ID = T.TIE_ID
        LEFT JOIN SCHHAYS.dbo.WTMISS M
            ON CC.PER_ID = M.PER_ID
            AND CC.CNT_ID = M.CNT_ID
        LEFT JOIN ##WTCNTIWeek COT1
            ON M.PER_ID = COT1.PER_ID
            AND M.CNT_ID = COT1.CNT_ID
        INNER JOIN SCHHAYS.dbo.WTPRH AS PRH
            ON M.PER_ID = PRH.PER_ID
            AND M.CNT_ID = PRH.CNT_ID
            AND M.TIE_ID = PRH.TIE_ID
        INNER JOIN ##tempStartEndWeekDates AS Tsed
            ON PRH_DTEDEBSEM >= Tsed.StartDate
            AND PRH_DTEFINSEM <= Tsed.EndDate
        LEFT JOIN SCHHAYS.dbo.WTSCCT C
            ON CC.RGPCNT_ID = C.RGPCNT_ID
            AND CC.PER_ID = C.PER_ID
            AND CC.CNT_ID = C.CNT_ID
        INNER JOIN ##TempHaysStaffWeek HF
            ON C.VAPO_CODE = HF.onetouch  COLLATE Latin1_General_CI_AS
    GROUP BY WeekOfMonth, StartDate, EndDate
) AS t

UNION ALL

SELECT
    2
    , SUM(Fees) AS Fees
    , WeekOfMonth
    , CONVERT(NVARCHAR(MAX), StartDate, 103) AS StartDate
    , CONVERT(NVARCHAR(MAX), EndDate, 103) AS EndDate
    , 'Temp Margin ' AS FeesName
    , @MonthName AS [MonthName]
FROM
(
    SELECT DISTINCT
        SUM((CASE WHEN CNTI_DURHEBDO IS NULL THEN 1 ELSE CNTI_DURHEBDO / 5 END) * @NumberOfDays) - (CNTI_THPAYE * (CASE WHEN CNTI_DURHEBDO IS NULL THEN 1 ELSE CNTI_DURHEBDO / 5 END) * 1.453 * 1.21 * @NumberOfDays) AS Fees
        , WeekOfMonth
        , StartDate
        , EndDate
    FROM SCHHAYS.dbo.WTVTAT TAT
        LEFT JOIN SCHHAYS.dbo.WTTIEINT AS [INT]
            ON TAT.TIE_ID = INT.TIE_ID
        AND TAT.VTAT_IORDRE = INT.TIEI_ORDRE
        LEFT JOIN SCHHAYS.dbo.PYCONTRAT AS CC
            ON TAT.PER_ID = CC.PER_ID
            AND TAT.CNT_ID = CC.CNT_ID
        LEFT JOIN SCHHAYS.dbo.CMTIERS AS T
            ON TAT.TIE_ID = T.TIE_ID
        LEFT JOIN SCHHAYS.dbo.WTMISS AS M
            ON CC.PER_ID = M.PER_ID
            AND CC.CNT_ID = M.CNT_ID
        LEFT JOIN ##WTCNTIWeek AS COT1
            ON M.PER_ID = COT1.PER_ID
            AND M.CNT_ID = COT1.CNT_ID
        INNER JOIN SCHHAYS.dbo.WTPRH AS PRH
            ON M.PER_ID = PRH.PER_ID
            AND M.CNT_ID = PRH.CNT_ID
            AND M.TIE_ID = PRH.TIE_ID
        INNER JOIN ##tempStartEndWeekDates AS Tsed
            ON PRH_DTEDEBSEM >= Tsed.StartDate
            AND PRH_DTEFINSEM <= Tsed.EndDate
        LEFT JOIN SCHHAYS.dbo.WTSCCT AS C
            ON CC.RGPCNT_ID = C.RGPCNT_ID
            AND CC.PER_ID = C.PER_ID
            AND CC.CNT_ID = C.CNT_ID
        INNER JOIN ##TempHaysStaffWeek AS HF
            ON C.VAPO_CODE = HF.onetouch  COLLATE Latin1_General_CI_AS
    GROUP BY WeekOfMonth, StartDate, EndDate, CNTI_THPAYE, CNTI_DURHEBDO
) AS t
GROUP BY WeekOfMonth, StartDate, EndDate

The only thing that I can suggest that might speed things up is a complete guess because you haven't provided the full code or table schemas

In both queries on either side of the UNION ALL  you join almost every table to another table on 2-3 fields, if this isn't necessary you are forcing the query to perform a lot of stuff that it doesn't need to

Another thought, you are calling COLLATE on both Queries as well, that probably doesn't need to done at all on this level, your SQL Server most likely already has Latin1_General_CI_AS as a default collation on the entire server, so calling it here is wasting processing power, twice.


Find biggest palindrome which is the product of two 3-digit numbers isPalindrome :: Integer -> Bool
isPalindrome n = reverse x == x
    where x = show n

is3x3 :: Integer -> Bool
is3x3 n = any (\x -> cond1 x && cond2 x) [101..999]
    where
        cond1 x = n `mod` x == 0
        cond2 x = length (show $ n `div` x) == 3

main = print $ head [p | p <- [999^2,999^2-1..], isPalindrome p, is3x3 p]

I have some doubts about the code I wrote:

isPalindrome: is it ok or is it better to keep dividing by 10, storing the remainder in a list and check if list is equal to the reversed self?
is3x3: is it a good practice to declare functions inside functions? Is there a better way to write the function?
how to delete the parentheses in cond2? $ doesn't work because of ==
[999^2,999^2-1..]:

is Haskell smart and calculates 999^2 only once, subtracting 1 every time? Or does it compute 999^2-x for each xth+1 element of the list?
is there a better way to write this list?
is it  better to put the end of the list (in this case 10001) or it doesn't matter in terms of performance? I know lists are lazy but ...

isPalindrome is ok here, no need to make it faster.
In functional programming functions are everywhere, even inside other functions and this is ok. Using ViewPatterns extension it is possible to rewrite is3x3 as:

is3x3 n = any cond [101..999]
    where
        cond (divMod n -> (d, 0)) = 99 < d && d <= 999
        cond _ = False

I don't know any readable way to rewrite cond2 without parenthesis.
Haskell is smart enough to calculate 999^2 and 999^2-1 at compile time. In terms of performance, if you don't specify end of the list there will be no checks for the end of the list at runtime (which is good in this case).

Using Integer is justified only for big numbers, in your case using Int is enough and could be a bit faster.

Here is another solution:

main = print $ maximum
  [ x * y :: Int
  | x <- [999, 998 .. 100]
  , y <- [x, x-1 .. 100]
  , (==) <*> reverse $ show $ x * y
  ]


Proper way to wait end of a service and singleton design My code works fine and I would like to know if I chose the best strategy to implement my code, or if It can written better (more fast, more clear, best use of design pattern, etc).

In my android application WelcomeActivity, in the onCreate method, I start a really simple service that load data from a file:

public class LoadDataService extends IntentService {
    private int result;
    @Override
    protected void onHandleIntent(Intent intent) {
        try
        {
            FileTestSingleton.getInstance(); //this could be long, read from file
            result = Activity.RESULT_OK;
        } catch (Exception e)
        {
            Log.e(LoadDataService.class.getName(), "FILE DATA LOADING ERROR \n" + e);
            result = Activity.RESULT_CANCELED;
        }
        publishResults(result);
    }

    private void publishResults(int result) {
        stopSelf();
    }
}

Some activity after, the user click a button to go to the next step of my APP. The onCreate method of MyNewActivity need the data from the service, otherwise it's ok to freeze the application, because if the data aren't loaded, nothing can be done. For example, I read that to stop the UI thread is a bad thing, but I need to stop it, otherwise my new activity will not have data to show.

So in my onCreate:

while (FileTestSingleton.getState()==Semaphore.LOADING)
{
    Thread.sleep(250);
}
fileTest = FileTestSingleton.getInstance();

Is this a proper way to wait?

Semaphore is a simple enum that I created inside the FileTestSingleton class:

public enum Semaphore
{
    EMPTY,
    LOADING,
    DONE;
}
private static Semaphore semaphore = Semaphore.EMPTY;
private static FileTestSingleton fileTest;
public static FileTestSingleton getInstance() throws Exception
{
    if (fileTest==null && semaphore!=Semaphore.LOADING)
    {
        semaphore = Semaphore.LOADING;
        fileTest = new FileTestSingleton(); //read from file, really slow
        semaphore = Semaphore.DONE;
    }
    return fileTest;
}
public static Semaphore getState()
{
    return semaphore;
}

Why I think that I can't use AsyncTask:
The problem is: I load some txt files on the startup in the WelcomeActivity that is for 2 seconds on the screen. After the app go in the MiddleActivity and the user  in the meantime can choose some options before he starts the final step of the application: FinalActivity. This FinalActivity need that all the txt files are loaded in the singleton.

Bug?

The catch block is a bit suspicious here:

try
{
    FileTestSingleton.getIstance(); //this could be long, read from file
} catch (Exception e)
{
    Log.e(LoadDataService.class.getName(), "FILE DATA LOADING ERROR \n" + e);
    result = Activity.RESULT_CANCELED;
}
result = Activity.RESULT_OK;
publishResults(result);

When you catch the exception, you set result = Activity.RESULT_CANCELED;,
but execution continues and on the next line comes result = Activity.RESULT_OK;.
In short, the method will return with success as if nothing happened.

Don't catch Exception

Always catch the most specific Exception type possible.
By catching something overly generic,
you may not notice when something truly unexpected is thrown.
In most practical situations there is a more specific exception class that you should check.

Don't declare a method to throw Exception

A method that's declared to throw Exception is very poorly designed.
It gives no clue to the caller about the kind of failure that can happen,
and it's impossible to know how to recover from it.
Surely you can think of something more specific,
to give the chance to callers to handle the failure gracefully.

Coding style

There is a standard in Java for placing braces:
the opening braces should be on the same line as their statement. Follow this style:

try {
    // ...
} catch (Exception e) {
    // ...
}

Proper way to wait for end of X

In other words, the proper way to execute a long operation without freezing the client. I think it's using an AsyncTask, with a ProgressDialog. Here's a rough sketch:

class WaitForServiceAsyncTask extends AsyncTask<Void, Integer, Void> {
    ProgressDialog mProgressDialog;

    protected void onPreExecute() {
        mProgressDialog = ProgressDialog.show(YourActivity.this, "",
                YourActivity.this.getString(R.string.msg_waiting_for_service), false, true);
    }

    protected Void doInBackground(Void... params) {
        // the long task -- calling the service
    }
}

This won't freeze the UI thread and will let the user know that the app is not dead, but actually doing something.


Creating and calling objects sequentially in Tkinter with Python 3 I am trying to build a very simple user interface in Python 3 using Tkinter to be used in windows. and have written a code which is not efficient - it is excessively long for no good reason. I am looking for advice, perhaps even conceptual, about how to make this short and elegant.

Here is a simple excerpt from the code, for only 2 periods. I have many periods, and have created each of them as a class, which is the problem that I am trying to replace. I presume that we should just have one Period class, objects of which will be called sequentially. I do not understand how to do this with Tkinter.

import tkinter as tk
from tkinter import ttk

class Game(tk.Tk):

    def __init__(self, *args, **kwargs):

        tk.Tk.__init__(self, *args, **kwargs)

        container = tk.Frame(self)
        container.pack(side='top', fill='both', expand=True)
        container.grid_rowconfigure(0, weight=1)
        container.grid_columnconfigure(0, weight=1)

        self.frames = {}
        for F in (Period1, Period2):
            frame = F(container, self)
            self.frames[F] = frame
            frame.grid(row=0, column=0, sticky='nsew')

    def show_frame(self, cont):
        frame = self.frames[cont]
        frame.tkraise()

class Period1(tk.Frame):

    def __init__(self, parent, controller):

        tk.Frame.__init__(self, parent)

        buttonNext = ttk.Button(self, text='next', command=lambda: controller.show_frame(Period2))
        buttonNext.pack()

class Period2(tk.Frame):

    def __init__(self, parent, controller):

        tk.Frame.__init__(self, parent)

        buttonNext = ttk.Button(self, text='next', command=lambda: controller.show_frame(Period1))
        buttonNext.pack()

app = Game()
app.mainloop()

I don't exactly know what you mean by "call" the object. Objects aren't typically something you call. I'm going to assume that in this context you meant "show".

The code you copied was for having completely different "pages" which could be shown by name in any order. However, you want several identical "periods" that you want to access sequentially, or perhaps by number (eg: "show period 1", "show period 2", etc.)

So, assuming that each period is identical except for it's number, the first step is to create a single Period class. It seems reasonable that each period know it's number so you might want to add that to the constructor:

class Period(tk.Frame):
    def __init__(self, parent, controller, number):
        tk.Frame.__init__(self, parent)
        self.number = number
        ...

Next, in your Game, instead of looping over a list of classes, you need a simpler loop that runs as many times as you want to have periods. For example, if you are creating a game with four periods, you might modify the loop in Game to look like this:

self.periods = []
for period_number in range(4):
    period = Period(container, self, period_number)
    self.periods.append(period)
    period.grid(row=0, column=0, sticky='nsew')

With that, self.periods[0] refers to the first period, self.periods[1] is the second, and so on.

You then need to modify show_frame to take a number rather than a class or name of a class. It should also be renamed to something like show_period to make the intent more clear (I've left error checking out to keep it short):

def show_period(self, period_number):
    frame = self.frames[period_number]
    frame.tkraise()

With that, you now have the ability to show by number any period you want. Since you want to be able to step through them sequentially, I recommend creating a method named next_period which can show the next period and handle any logic about what to do after the last period.

The simplest implementation will be to have show_period remember what the current period is, and then in next_period you simply need to add one:

def show_period(self, period_number):
    ...
    self.current_period = period_number

The implementation of next_period is pretty easy:

def next_period(self):
    if self.current_period == len(self.periods):
        <special code to handle the last period...>
    self.show_period(self.current_period+1)

With that, you can add a button to your Period class that will automatically take it to the next period, without it having to know what the next period is:

class Period(tk.Frame):
    def __init__(self, parent, controller, number):
        ...
        self.next_button = tk.Button(self, text="Next", command=self.next)
        ...

    def next(self):
        self.controller.next_period()

Note: This is not necessarily the best way to structure this program, it is simply one way of doing it. The point was to illustrate how to create multiple instances of a single class, and step through them one at a time.


Laravel image upload I'm new to the filesystem from Laravel, but I need an upload script that allows users to upload images to my server. However, i'm concerned about securety, so I thought why not ask for advice, because there aren't much reverences about that big topic of securety.

Basicly, this is how I check the user upload and how I upload the file:

My request Class:

 return [
        'profile_picture' => 'image',
    ];

My check if the image is actually an image and not some bad maleware:

function checkIfImageIsAllowed($file) {
$allowedMimes = [
    'image/jpeg',
    'image/jpg',
    'image/png'
];

$allowedExtensions = [
    'jpg',
    'JPG',
    'jpeg',
    'png'
];
$allowedMaxSize = 2000000;
if(!in_array($file->getClientOriginalExtension(), $allowedExtensions) || !in_array($file->getMimeType(), $allowedMimes) || $file->getSize() > $allowedMaxSize) {
    return false;
}
return true;
}

And than I upload the file:

$path = $request->file('profile_picture')->store('UserUpload/Images');

This is how I display the Image:

    $images = \App\Image::where('uuid', $uuid)->firstOrFail();
    $path = $images->path;
    $image = Storage::get($path);
    $response = Response::make($image, 200);
    $response->header("Content-Type", $images->memeType);

    return $response;

What are your thoughts about this? Do you think this is secure enough? Or do you have some improvement for this?

The code can be made simpler by using some built-in Laravel validation rules:

Validation of MIME types and extension can already be done in Laravel.

With the rule image as you have for profile_picture, you're already validating if the uploaded file is of the mimetype jpeg, png, bmp, gif, or svg (docs here). If you'd like to specify the mimetype however, you can use the mimes rule.
To validate the max size of the image, you can use the max rule.
You can also validate the dimensions of the image if needed (docs here).

On the matter of security, I suppose you mean with the uploading of the images. The snippets of code you provided do seem enough (have not perused it), but I would advise using the built-in validation provided by Laravel to provide the best of convenience and security.


Mostly portable 128 by 64 bit division I wrote this out of curiosity. It is based on the theory behind Knuth's Algorithm D and is intended to emulate the behavior of the x86 div instruction (though the result is truncated instead of raising an exception for overflow).

I am primarily interested in correctness (it seems to work correctly), and have not yet compared performance with anything simpler.

Two's complement hardware is assumed.

uint64_t div(uint64_t a_lo, uint64_t a_hi, uint64_t b, uint64_t &r)
{
    uint64_t p_lo;
    uint64_t p_hi;
    uint64_t q = 0;

    auto r_hi = a_hi;
    auto r_lo = a_lo;

    int s = 0;
    if(0 == (b >> 63)){

        // Normalize so quotient estimates are
        // no more than 2 in error.

        // Note: If any bits get shifted out of
        // r_hi at this point, the result would
        // overflow.

        s = 63 - bsr(b);
        const auto t = 64 - s;

        b <<= s;
        r_hi = (r_hi << s)|(r_lo >> t);
        r_lo <<= s;
    }

    const auto b_hi = b >> 32;

    /*
    The first full-by-half division places b
    across r_hi and r_lo, making the reduction
    step a little complicated.

    To make this easier, u_hi and u_lo will hold
    a shifted image of the remainder.

    [u_hi||    ][u_lo||    ]
          [r_hi||    ][r_lo||    ]
                [ b  ||    ]
    [p_hi||    ][p_lo||    ]
                  |
                  V
                [q_hi||    ]
    */

    auto q_hat = r_hi / b_hi;

    p_lo = mul(b, q_hat, p_hi);

    const auto u_hi = r_hi >> 32;
    const auto u_lo = (r_hi << 32)|(r_lo >> 32);

    // r -= b*q_hat
    //
    // At most 2 iterations of this...
    while(
        (p_hi > u_hi) ||
        ((p_hi == u_hi) && (p_lo > u_lo))
        )
    {
        if(p_lo < b){
            --p_hi;
        }
        p_lo -= b;
        --q_hat;
    }

    auto w_lo = (p_lo << 32);
    auto w_hi = (p_hi << 32)|(p_lo >> 32);

    if(w_lo > r_lo){
        ++w_hi;
    }

    r_lo -= w_lo;
    r_hi -= w_hi;

    q = q_hat << 32;

    /*
    The lower half of the quotient is easier,
    as b is now aligned with r_lo.

          |r_hi][r_lo||    ]
                [ b  ||    ]
    [p_hi||    ][p_lo||    ]
                        |
                        V
                [q_hi||q_lo]
    */

    q_hat = ((r_hi << 32)|(r_lo >> 32)) / b_hi;

    p_lo = mul(b, q_hat, p_hi);

    // r -= b*q_hat
    //
    // ...and at most 2 iterations of this.
    while(
        (p_hi > r_hi) ||
        ((p_hi == r_hi) && (p_lo > r_lo))
        )
    {
        if(p_lo < b){
            --p_hi;
        }
        p_lo -= b;
        --q_hat;
    }

    r_lo -= p_lo;

    q |= q_hat;

    r = r_lo >> s;

    return q;
}

For convenience, below are the bsr and mul functions used above. The bsr routine comes from this useful site:

int bsr(uint64_t x)
{
    uint64_t y;
    uint64_t r;

    r = (x > 0xFFFFFFFF) << 5; x >>= r;
    y = (x > 0xFFFF    ) << 4; x >>= y; r |= y;
    y = (x > 0xFF      ) << 3; x >>= y; r |= y;
    y = (x > 0xF       ) << 2; x >>= y; r |= y;
    y = (x > 0x3       ) << 1; x >>= y; r |= y;

    return static_cast<int>(r | (x >> 1));
}

uint64_t mul(uint64_t a, uint64_t b, uint64_t &y)
{
    auto a_lo = a & 0x00000000FFFFFFFF;
    auto a_hi = a >> 32;

    auto b_lo = b & 0x00000000FFFFFFFF;
    auto b_hi = b >> 32;

    auto c0 = a_lo * b_lo;
    auto c1 = a_hi * b_lo;
    auto c2 = a_hi * b_hi;

    auto u1 = c1 + (a_lo * b_hi);
    if(u1 < c1){
        c2 += 1LL << 32;
    }

    auto u0 = c0 + (u1 << 32);
    if(u0 < c0){
        ++c2;
    }

    y = c2 + (u1 >> 32);

    return u0;
}

The original code seemed too complicated for this so I set out to write a simpler version. Should be fairly portable too, and fast as it is just bit shifts and subtraction.

(It works similarly to how one would accomplish division in assembly language on a processor without hardware division.)

// 128-bit / 64-bit unsigned divide

#include <stdio.h>
#include <stdint.h>

int main(void)
{
  // numerator
  uint64_t a_lo = 1234567;
  uint64_t a_hi = 0;

  // denominator
  uint64_t b = 10;

  // quotient
  uint64_t q = a_lo << 1;

  // remainder
  uint64_t rem = a_hi;

  uint64_t carry = a_lo >> 63;
  uint64_t temp_carry = 0;
  int i;

  for(i = 0; i < 64; i++)
  {
    temp_carry = rem >> 63;
    rem <<= 1;
    rem |= carry;
    carry = temp_carry;

    if(carry == 0)
    {
      if(rem >= b)
      {
        carry = 1;
      }
      else
      {
        temp_carry = q >> 63;
        q <<= 1;
        q |= carry;
        carry = temp_carry;
        continue;
      }
    }

    rem -= b;
    rem -= (1 - carry);
    carry = 1;
    temp_carry = q >> 63;
    q <<= 1;
    q |= carry;
    carry = temp_carry;
  }

  printf("quotient = %llu\n", (long long unsigned int)q);
  printf("remainder = %llu\n", (long long unsigned int)rem);
  return 0;
}


Refactor this jQuery datepicker code to be as small as possible I am using jQuery datepicker on 2 forms on the same page.
I currently am using the following code (below), but as you can see, I am using the same parameters over again (showOn, buttonImage, buttonImageOnly, dateFormat, & constrainInput)

Is there a more efficient way to do this rather then repeating myself over and over again, over 4 methods?

$('#startdate').datepicker(
    {
        showOn: 'both',
        buttonImage: '/assets/images/calendar_symbol.png',
        buttonImageOnly: true,
        dateFormat: 'dd/mm/yy',
        constrainInput: true, // All of the above is repeated again below
        minDate: new Date(<?php echo date("Y")?>, <?php echo date("m")-1?>, <?php echo date("d")?>),
        onClose: function()
            {
                $('#enddate').val($('#startdate').val());
            }
    }
);

$('#enddate').datepicker(
    {
        showOn: 'both',
        buttonImage: '/assets/images/calendar_symbol.png',
        buttonImageOnly: true,
        dateFormat: 'dd/mm/yy',
        constrainInput: true, // All the above repeated again..
        minDate: new Date(<?php echo date("Y")?>, <?php echo date("m")-1?>, <?php echo date("d")?>)
    }
);

$('#filter_startdate').datepicker(
    {
        showOn: 'both',
        buttonImage: '/assets/images/calendar_symbol.png',
        buttonImageOnly: true,
        dateFormat: 'dd/mm/yy',
        constrainInput: true,// and again...
        onClose: function()
            {
                $('#enddate').val($('#startdate').val());
            }
    }
);

$('#filter_enddate').datepicker(
    {
        showOn: 'both',
        buttonImage: '/assets/images/calendar_symbol.png',
        buttonImageOnly: true,
        dateFormat: 'dd/mm/yy',
        constrainInput: true // and again....
    }
);

I was thinking of using a if statement to check what was clicked (#startdate or #filter_startdate), and then breaking it down like that, but unsure if this would work or how to do this.

You'll want to use setDefaults to clean up your code a bit. Also with a few variables you can clean it up even more.

$.datepicker.setDefaults({
    showOn: 'both',
    buttonImage: '/assets/images/calendar_symbol.png',
    buttonImageOnly: true,
    dateFormat: 'dd/mm/yy',
    constrainInput: true
});

var minDate = new Date(
    <?php echo date("Y")?>,
    <?php echo date("m")-1?>,
    <?php echo date("d")?>
);

var setEndDate = function() {
    $('#enddate').val($('#startdate').val());
};

$('#startdate').datepicker({
    minDate: minDate ,
    onClose: setEndDate
});

$('#enddate').datepicker({
    minDate: minDate
});

$('#filter_startdate').datepicker({
    onClose: setEndDate
});

$('#filter_enddate').datepicker();


Username, Password and UserType Validation I have two tables in the database:

Credentials (userid, password, usertype)
Customer (customername, userid(foreign key))

I need to validate username,password and usertype and create a session variable for customername.

I have organized the code and it works fine, but how can I minimize the code and make it efficient?

// Function that enables LOGON for a registered User into site
public string LogOn(UserInformation objLogOnUserInformation)
{
    ConnectionManager conCm = new ConnectionManager();
    try
    {
        SqlConnection con = conCm.OpenConnection();
        SqlCommand cmd,cmdOne;
        SqlDataReader dr;
        string _str = "U";
        cmd = new SqlCommand("select UserID  from Credentials", con);
        dr = cmd.ExecuteReader();

        while (dr.Read())
        {
            //checking user name is present in database
            if (dr.GetValue(0).ToString() == objLogOnUserInformation.UserId)
            {
                dr.Close();
                //retriving Password for the existing user
                cmd = new SqlCommand("select Password  from Credentials where UserId='" + objLogOnUserInformation.UserId + "'", con);
                dr = cmd.ExecuteReader();

                while (dr.Read())
                {
                    //checking the password is matching with the database
                    if (dr.GetValue(0).ToString() == objLogOnUserInformation.Password)
                    {
                        dr.Close();
                        cmd = new SqlCommand("select UserType  from Credentials where userid='" + objLogOnUserInformation.UserId + "'", con);
                        dr = cmd.ExecuteReader();

                        while (dr.Read())
                        {
                            //checking user type
                            if (dr.GetValue(0).ToString() == _str)
                            {
                                dr.Close();
                                cmdOne = new SqlCommand("select customername from customer where userid='" + objLogOnUserInformation.UserId + "'", con);
                                dr = cmdOne.ExecuteReader();
                                dr.Read();
                                objLogOnUserInformation.SessionUserName = dr.GetValue(0).ToString();
                                dr.Close();
                                //if usertype is customer, U is returned
                                return _str;
                            }
                            else
                            {
                                dr.Close();
                                cmdOne = new SqlCommand("select customername from customer where userid='" + objLogOnUserInformation.UserId + "'", con);
                                dr = cmdOne.ExecuteReader();
                                dr.Read();
                                objLogOnUserInformation.SessionUserName = dr.GetValue(0).ToString();
                                dr.Close();
                                //If usertype is Admin, A is returned
                                return "A";

                            }
                        }
                    }

                    else
                    {
                        return " Either User Name or Password is not Valid";
                    }
                }
            }
        }

Here are some observations:

you first iterate all rows in Credentials and then select the Password yet again - why not just select the pasword?
seems like your password is plain-text - BAD idea - hash your password together with a  salt please
don't concat strings to get SELECT queries - use parameters instead
you are using C#/ASP.NET so why don't you use the provided login mechanisms?
you return a string "A" if the login succeeds and the error message if not - this is IMO bad design - return a simple struct/class with more information instead


Factory Pattern Implementation in PHP I have read some articles regarding the factory pattern and decided to write some code as an exercise. This is my first attempt and am not 100% if I did it right. I've omitted some implementations for the sake of focusing on the concept at hand.

class NumbersFactory {
    public static function makeNumber( $type, $number ) {
        $numObject = null;
        $number = null;

        switch( $type ) {
            case 'float':
                $numObject = new Float( $number );
                break;
            case 'integer':
                $numObject = new Integer( $number );
                break;
            case 'short':
                $numObject = new Short( $number );
                break;
            case 'double':
                $numObject = new Double( $number );
                break;
            case 'long':
                $numObject = new Long( $number );
                break;
            default:
                $numObject = new Integer( $number );
                break;
        }

        return $numObject;
    }
}

/* Numbers interface */
abstract class Number {
    protected $number;

    public function __construct( $number ) {
        $this->number = $number;
    }

    abstract public function add();
    abstract public function subtract();
    abstract public function multiply();
    abstract public function divide();
}
/* Float Implementation */
class Float extends Number {
    public function add() {
        // implementation goes here
    }

    public function subtract() {
        // implementation goes here
    }

    public function multiply() {
        // implementation goes here
    }

    public function divide() {
        // implementation goes here
    }
}
/* Integer Implementation */
class Integer extends Number {
    public function add() {
        // implementation goes here
    }

    public function subtract() {
        // implementation goes here
    }

    public function multiply() {
        // implementation goes here
    }

    public function divide() {
        // implementation goes here
    }
}
/* Short Implementation */
class Short extends Number {
    public function add() {
        // implementation goes here
    }

    public function subtract() {
        // implementation goes here
    }

    public function multiply() {
        // implementation goes here
    }

    public function divide() {
        // implementation goes here
    }
}
/* Double Implementation */
class Double extends Number {
    public function add() {
        // implementation goes here
    }

    public function subtract() {
        // implementation goes here
    }

    public function multiply() {
        // implementation goes here
    }

    public function divide() {
        // implementation goes here
    }
}
/* Long Implementation */
class Long extends Number {
    public function add() {
        // implementation goes here
    }

    public function subtract() {
        // implementation goes here
    }

    public function multiply() {
        // implementation goes here
    }

    public function divide() {
        // implementation goes here
    }
}

$number = NumbersFactory::makeNumber( 'float', 12.5 );

Coding Critique

public static function makeNumber( $type, $number ) {
    $numObject = null;
    $number = null;

Well, that almost certainly isn't right.  You take a parameter which you then ignore, overwriting it locally.

public static function makeNumber( $type, $number = null ) {
    $numObject = null;

That seems like it might be more useful.  You accept a parameter if offered.  Otherwise, you give it a default value.  That may or may not be what you wanted to do.

        case 'float':
            $numObject = new Float( $number );
            break;

You can make this simpler if you want:

        case 'float':
            return new Float( $number );

That way you can get rid of $numObject entirely.

        default:
            $numObject = new Integer( $number );
            break;

You don't need a break; in the last clause.  You can just let it fall through after a default: clause.

Factory Method

You have this tagged factory-method, but you aren't using a factory method.  The factory method is when you have a method that can return different results depending on the exact class of the object.  So you might have

class CompositeNumberFactory {
    public static function makeNumber( $first, $second );
}

class FractionFactory extends CompositeNumberFactory {
    public static function makeNumber( $first, $second ) {
        return new Fraction($first, $second);
    }
}

class Fraction implements iNumber {
    private $numerator;
    private $denominator;

    function __construct($numerator, $denominator) {
        $this->numerator = $numerator;
        $this->denominator = $denominator;
    }

This gets more interesting if FractionFactory is called something else, e.g. RationalFactory.  Then the caller may not even know that they have a Fraction.  They just know that they have the normal iNumber methods.

Factory Pattern

There is a variant of the factory pattern that does work something like you specify, but it would normally have different types.  E.g.

        case 'decimal':
            return new Double( $number );
        case 'bit64':
            return Long( $number );
        case 'whole':
            return new Unsigned( $number );

This allows the user to not know what the various types are, just how the user would describe them.  Also, this allows the underlying implementation to change without impacting the user.  For example, if we moved from a 32-bit operating system to a 64-bit operating system, the second case might change from Long to Integer.