Fahrenheit and Celsius converter in Rust I made a program that converts between Celsius and Fahrenheit. Is there any way that I could make this code more efficient and cleaner?

use std::io;

// C to F: F = C*(9/5) + 32
// F to C: C = (F-32)*(5/9)

/**********Converts between Fahrenheit and Celsius*********/

fn main() -> () {
    println!("Do you want to convert to Celsius or Fahrenheit? Input C or F");
    let mut convert_type = String::new();

    io::stdin().read_line(&mut convert_type)
        .expect("Failed to conversion type.");

    let t = String::from(convert_type);

    println!("You want to convert to: {}", t);
    println!("What temperature would you like to convert?");
    let mut temp = String::new();

    io::stdin().read_line(&mut temp)
        .expect("Failed to read temperature.");

    let temp: i32 = match temp.trim().parse() {
        Ok(temp) => temp,
        Err(_e) => {
            -1
        }
    };

    match t.as_str() {
        "C\n" => println!("{}", ftoc(temp)),
        "F\n" => println!("{}", ctof(temp)),
        _ => println!("t = {:?}", t),
}
}

// Celsius to Fahrenheit
fn ctof(c: i32) -> i32 {
    (c * (9 / 5)) + 32

}

//Fahrenheit to Celsius
fn ftoc(f: i32) -> i32 {
    (f-32) * (5 / 9)
}

Output:

    cargo run
   Compiling ftoc v0.1.0 (/Users/roberthayek/rustprojects/ftoc)
    Finished dev [unoptimized + debuginfo] target(s) in 2.64s
     Running `target/debug/ftoc`
Do you want to convert to Celsius or Fahrenheit? Input C or F
F
You want to convert to: F

What temperature would you like to convert?
0
32

The first thing I always do is running clippy.

You will catch some things that are not neccessary, e.g.

fn main() -> () can be reduced to fn main()
let t = String::from(convert_type); is simply let t = convert_type

The bad things are

c * (9 / 5) which is always c because of integer arithmetic. You probably want f64::from(c) * (9.0 / 5.0)
same for (f - 32) * (5 / 9) should be (f64::from(f) - 32.0) * (5.0 / 9.0)) as i32

You may want to add some unit-tests to your program to verify that ctof and ftoc actually work.


Game launcher for The Sims 3 Is there a way I could improve my code's readability, or have I finally won the war against sloppy coding?

Here's a game launcher I made for The Sims 3 to fix the awful performance caused by big cache files.

using System.Collections.Generic;
using System.Diagnostics;
using System.Windows.Forms;

namespace Sims_3___Anti_Cache
{
    public partial class Form1 : Form
    {
        Game Sims3;

        public Form1()
        {
            InitializeComponent();

            Sims3 = new Game();
            Sims3.cache = new List<string>() { "CASPartCache.package", "compositorCache.package", "scriptCache.package",
                                               "simCompositorCache.package", "socialCache.package" };

            Sims3.cache_path = "C:/Users/DarkShadow/Documents/Electronic Arts/The Sims 3/";
            Sims3.clear_cache();
            Sims3.path = "C:/Program Files (x86)/Electronic Arts/The Sims 3/Game/Bin/TS3.exe";

            isCleared.Enabled = true;
        }

        private void isCleared_Tick(object sender, System.EventArgs e)
        {
            if (Sims3.cache_cleared)
            {
                Process.Start(Sims3.path);
                Application.Exit();
            }
        }

    }
}

You would benefit from removing the hard-coded strings out of the code. Best to put them in an app.config file or a resource file. The config file has the additional advantage that the strings could be changed by the user without recompiling.

The naming of the methods and properties is not standard for C#. Use Pascal case for public methods and properties with no underbar, e.g.

Sims3.CachePath = configCachePath;
Sims3.ClearCache();

The naming of local variables and private methods uses camel case, e.g.

isClearedTick();


Scrape a reddit post submission for comments and save comments to JSON file Edit: For a revision to this code according to the comments, see here: "here."

In regards to the code involving shortlinks and submission:
this script is meant to be used in conjunction with a different script not displayed here, which initializes the data.json with the appropriate keys. Code related to submission, comment.list(), praw all deal with the reddit API.

How it works:
Retrieves post information from data.json, using the shortlinks as reference, scrapes the aforementioned post for all of its comments, and then matches the group of comments to its corresponding shortlink in order to write the information to the JSON file.

#!/usr/bin/python3
import json
import os
import praw

"""
Scrapes posts for comments and saves the comment ID and comment text to the
accompanying JSON file.
"""

JSON_DEFAULT_LOC = os.environ.get('JSON_LOCATION') or os.path.join(
    os.path.dirname(os.path.dirname(os.path.abspath(__file__))), "data.json")

def login():
    """ Create a reddit instance according to praw.ini configuration.
    Call reddit.user.me() as a check.
    Return reddit instance.
    """
    print("Authenticating")
    reddit = praw.Reddit("appname", user_agent="Chrome:com.example.bot-"
                                                "name:v1 (by /u/)")
    print("Authenticated as {}".format(reddit.user.me()))
    return reddit

def load_manager(fp=None):
    """ Load the accompanying JSON data which holds reddit post information.
    """
    if fp is None:
        fp = JSON_DEFAULT_LOC
    with open(fp, "r") as file:
        data = json.load(file)
    return data

def scrape(reddit):
    """ For each post specified in the JSON dict, scrape the comments within
    the post and write the comments to the JSON dict to the key: "responses".
    """
    data = load_manager()
    shortlinks = [data[sub][0]["shortlink"][-6:] for sub in data]

    for sl_id in shortlinks:
        submission = reddit.submission(id=sl_id)
        try:
            write_comments_to_file(submission, sl_id, data)
            print("Comments from '{}' "
                  "saved to file".format(submission.subreddit))
        except Exception as e:
            print("Could not write to file: {}".format(e))

def write_comments_to_file(submission, sl_id, data, fp=None):
    """ Takes a reddit submission object, shortlink ID, and JSON data.
    Retrieves the comments per submission, and for each submission within the
    JSON data, also write its comments.
    """
    submission.comments.replace_more(limit=0)
    cmt_data = [{comment.id: comment.body}
                for comment in submission.comments.list()]
    for i in data:
        if sl_id in data[i][0]["shortlink"]:
            data[i][1]["responses"] = cmt_data
    if fp is None:
        fp = JSON_DEFAULT_LOC
    with open(fp, "w") as fp:
        json.dump(data, fp)

def main():
    reddit = login()
    scrape(reddit)

if __name__ == "__main__":
    main()

I am interested in making the code more Pythonic. I'm especially unconfident about the way that my functions are structured/grouped.
Help much appreciated!

First of all, naming: do not abbreviate names unless they are used in a one-liner like [x.something for x in somelist]. Do not try to spare characters - use IDE to suggest continuations if you want to type fast. Really pays off.

Second, try not to mix abstractions of different types in the same function, eg, write_comments_to_file deals now with preparing content as well as low-level detail of writing to file, extract to a function with meaningful name - will make code more readable. Try to avoid names with data, info, etc in it - they are useless. Everything is data.

The same function seems to make changes to submission - if it is the case, make it explicit and in a separate function: submission_replaced = replace_more(submission) (good names here as well - I just tried to guess one).

All that is needed for the code to show the intention behind it.
except Exception as e is too broad. What exceptions you really want to catch? Maybe, only IOError is enough?

Magic numbers: -6 <- why six? Extract that to a new function with a meaningful name like strip_html_postfix (or whatever is the intention of that) - then, there is no need to leave a comment, function name will serve as a comment. Using appropriate names can lessen the need to document the code, even to the point that no separate docstrings will be necessary.

You have not told whether this is one-time script or part of bigger system, so I will not touch possible use of classes. For example, propagation of fp (is it filepath?) and if for missing fp may be a signal, that the code may benefit from using a Loader class (omitting manager here, because it is usually as useless as data in class names unless correspond to manager in real world). For example, I see classes: Loader, Scraper, and Outputter which will nicely decouple the load manager configuration, process of scraping and saving the results to whatever destination - but of course it's overkill for a script, and is useful only if you foresee more Loaders, Scrapers or Outputters.

No matter of the programming language, functions are always more readable when they have explicit arguments and return results they produce. For instance, scrape at the moment uses module-level data - it's harder to write tests, configure, and guess what it does.

Hopefully, my findings above will help you write code, which will be self-explanatory and will not require the How it works you provided. This would be ultimately Pythonic.

PS. for ... data[i][1]["responses"] = cmt_data - this is dangerous in Python if you intend to use the result beyond converting to JSON. For example, if the data structure will be mutated, then it will be mutated for all responses (cmt_data is a pointer, not value, so to say!), which is hardly what is desired. And in this case data is from outside, so you may already have a problem elsewhere in the code and spend time to debug, because it will not be immediately clear the function in question make your responses entangled. Use copy or deepcopy, and overall it's not good idea to mutate data structure in a function without explicitly returning it back, and even then assignment of cmt_data can be frowned upon.


Reading an SSH config file for an IP I wrote this class which reads an SSH config file and checks whether an IP exists. If the IP does not exist, the class adds a block for that IP address.

Header file:

class SSHConfigFile : public QObject
{
    Q_OBJECT
public:
    explicit SSHConfigFile(QObject *parent = 0);

signals:

public slots:

    void registerIP(QString ip);

private:
    void read();
    bool find(QString ip);
    void addIP(QString ip);
    void write();

private:
    // the file name with path of config file
    QString _sshConfigFileName;

    QFile file;

    // the actual file data
    QString fileData;

    // the same file data but split into lines
    QStringList fileLines;

};

Implementation file:

SSHConfigFile::SSHConfigFile(QObject *parent) : QObject(parent),
    _sshConfigFileName( QStandardPaths::writableLocation( QStandardPaths::HomeLocation ) + "/" + ".ssh/config"),
    file( _sshConfigFileName )
{

}

void SSHConfigFile::read()
{
   if ( !file.open( QFile::ReadWrite | QFile::Text) )
   {
       qDebug() << "Could not open file for writing";
       return;
   }

   // read all data
   QTextStream in(&file);
   fileData = in.readAll();
   qDebug() << fileData;

   fileLines = fileData.split(QRegExp("[\r\n]"),QString::SkipEmptyParts);
}

// returns true if ip was found in config file otherwise false
bool SSHConfigFile::find(QString ip)
{
    read();

    foreach(QString  str, fileLines )
    {
        str = str.trimmed();

        //qDebug() << str;

        if (str.contains( "Host", Qt::CaseInsensitive))
        {
            str.remove("Host", Qt::CaseInsensitive);
            str = str.trimmed();

            if (str == ip)
                return true; // found
        }
    }

    return false; // not found
}

void SSHConfigFile::registerIP(QString ip)
{
    if (!find( ip))
    {
        // this is new ip, add it to config file
        addIP( ip );
    }
}

void SSHConfigFile::addIP(QString ip)
{
    QString addendum = QString("# Added by RigakuSync\n"
                               "Host %1\n"
                               "\tStrictHostKeyChecking no\n"
                               "\tUserKnownHostsFile=/dev/null\n").arg(ip);

    fileData = "\n" + addendum;

    write();
}

void SSHConfigFile::write()
{
    QTextStream out(&file);
    out << fileData;
    file.flush();
    file.close();
}

Sample use for additional context:

void DeviceSync::syncDevice(QString ip)
{
    // register IP to ssh config file if not already there.
    // I chose to do it on in function than to first check here if
    // this ip exist and if not than add it..fair enough?
    SSHConfigFile().registerIP( ip );

    // and then the rest of the code to perform sync. Basically i need to
    // make sure the ip is there in config file before i perform sync
}

From the code, I only call the registerIP() function, which is the only public function. It automatically adds the IP if it's not already in the config file.

When I pass the IP address, should the class just store it internally and then work with it, or is passing in between member functions better? Also, do the various member functions make sense or could it all be done in just one function?

Is writing a class for this a good approach or can this whole thing just be a function in another class? If this class is a good approach, can it be improved?

I see a few things that may help you improve your code.

Pass by const reference where practical

In every case that a QString ip is passed, the code could instead use a const QString &ip avoiding pointless multiple copies and also allowing the addition of a const IP address.

Consider large files

Reading the entire file into memory using QTextStream::readAll() can consume a lot of memory if the file is large. If you still want to do this anyway, see the next suggestion.

Don't store the file contents in the object

The object currently stores both the entire contents of the file in two forms: both as QString and as QStringList which isn't really needed.  First, you could pass the QString back as the return value for read(), and then you could have find(), addIP() and write() take that QString as an additional parameter.  Naturally, this should also be passed by const reference.

Reduce object creation where practical

Within the addIP() call, it isn't really necessary to prepend the "\n" to the addendum QString since that could easily simply be added to the static string.  Using all of the previous suggestions and this one, here is one way to write this function:

void SSHConfigFile::addIP(const QString &ip, const QString &fileData)
{
    QString addendum = QString("\n# Added by RigakuSync\n"
                               "Host %1\n"
                               "\tStrictHostKeyChecking no\n"
                               "\tUserKnownHostsFile=/dev/null\n").arg(ip);

    write(fileData + addendum);
}

Consider an alternative class design

The configuration file for your program is really a collection of attributes that each apply to a single Host, so what might make more sense as you expand this program, would be to create a HostConfig class to hold each individual configuration set, and then your SSHConfig class could implement a collection of those.

List all required #includes

I'm sure it is in your code (because it wouldn't otherwise compile) but it would be useful for reviewers to see a full, compilable sample which includes the required #include files.

Sanitize input

At the moment, if an ip QString with leading whitespace is handed to the registerIP() call, it will always add a duplicate entry because the un-trimmed input ip is being compared to the trimmed partial line.  If I were to abuse the program like this, I could make the configuration file grow without limit.

SSHConfigFile sh1;
for (int i=1000000; i; --i)
    sh1.registerIP( " 8.8.8.8" );

Make sure to match file open and close

In the case that the find routine finds a duplicate entry, it leaves the file open.  If it adds a new entry, it leaves the file closed.  This makes it impossible for the calling program to know whether the file is open or closed at the end of the call.  To fix that, (and using the previous suggestions), you could refactor like this:

void SSHConfigFile::registerIP(const QString &ip)
{
    QString fileData = read();
    if (!find( ip, fileData))
    {
        // this is new ip, add it to config file
        addIP( ip, fileData );
    } else {
        file.close();
    }
}

OOP design

Simply stated, in Object Oriented Programming, the nouns in a description become objects and the verbs become methods.  In this case the description is "reads an SSH config file and checks whether an IP exists. If the IP does not exist, the class adds a block for that IP address." so the relevant nouns are:

SSH config file
IP address
block (for IP address)

Verbs are:

read file
check file for IP address
add block for IP address to file

These match pretty closely to the methods in your class, and the IP address is passed in from the calling program.  As I mentioned in a previous point, one could consider creating an alternative design in which each block is a Host object, and the file could be implemented as a host collection (perhaps std::map<IPAddress, Host>) but the decision to do so or not depends on how you're planning to use this.  If the need for the IP block is only transient and static (as you've written it) then it's fine as you have it.  If, on the other hand, other parts of the program will be altering settings in the file or looking them up frequently, the Host object approach may be more natural.

Generally speaking, I think about the interface first.  Ask yourself "How would I like to interact with this thing?"  Then implement that, revealing as few internal details as practical.  Reducing the scope of variables to be as small as possible also helps, so that instead of the old style:

int i;
for (i=0; i < MAX_STUFF; ++i) { ... }

We prefer:

for (int i=0; i < MAX_STUFF; ++i) { ... }

Similarly, for the IP address, we know that it must be passed in to do the work of the registerIP() method.  If it were part of the object, it would uselessly take up memory as long as the object exists.  However, by passing it by const reference (my first suggestion) you minimize its scope to be as small as possible.  For this design, that's a good choice.  However, if you decide to implement as a collection of Host objects, the IP address would necessarily be member data.


Bulkloading xml's into Oracle table Intro
We have some old(in my eyes) software at the place I work at the moment,
Some stuff needs to be done manually, for instance we need to insert a XML file into a Oracle table, which will trigger... stuff
I got annoyed by having to do this manually all the time,
So I decided to automate the process a bit with powershell and Oracle's sqlldr
My knowledge of Oracle is limited hence the asking for a review,
What I did

I have created a separate table, to which I am inserting my file.
That table has a trigger, to get the specified information, and insert it into the correct table (which is not up for review)
Created a powershell script to be able to load files in bulk into the table

Code
The sql script for creating the table
CREATE TABLE mckdev.ogd_xml_table (
  id        NUMBER(10),
  xml       XMLTYPE,
  "IN_CON_LOG" NUMBER(1,0),
  "ZIS_EVENT" VARCHAR2(35 BYTE),
);

ALTER TABLE mckdev.ogd_xml_table ADD (
  CONSTRAINT ogd_xml_table_pk PRIMARY KEY (id)
);

CREATE SEQUENCE mckdev.ogd_xml_table_seq;

The load_data.ctl
load data
infile ''
append
into table ogd_xml_table
fields
(
    filename FILLER CHAR(100),
    xml lobfile( filename) terminated by eof
)

The bulkloader.ps1
$dataFiles = @(
    "filelist21.dat"
    "filelist22.dat",
    "filelist23.dat",
    "filelist25.dat",
    "filelist121a.dat",
    "filelist121b.dat",
    "filelist122a.dat",
    "filelist122b.dat"
)
$ctlFile = "$PSScriptRoot\load_data.ctl"

foreach ($f in $dataFiles) {
    (Get-Content $ctlFile) -Replace "infile '[^']*'", "infile '$($PSScriptRoot)\$($f)'" | Out-File $ctlFile -Encoding ASCII
    sqlldr mckdev@$SecretDatabase/$SecretPassword control=$ctlFile
}

An example filelist.dat only holds a reference to another XML file.
Questions

Currently I need to have multiple .dat file to be able to load them in bulk.
One for each XML file I want to load into the table. Is there any way around this?

Is this a correct approach or would you have done things differently?

As the ctl file can't be parameterized you'll have to create them for each infile.

As it is quite small, I'd create it on the fly from a here string with the format operator.

This sample script just echoes to screen, the commands to write/execute sqlldr are commented out.

## Q:\Test\2019\01\24\CR_212133.ps1
$dataFiles = @(
    "filelist21.dat"
    "filelist22.dat",
    "filelist23.dat",
    "filelist25.dat",
    "filelist121a.dat",
    "filelist121b.dat",
    "filelist122a.dat",
    "filelist122b.dat"
)

$ctlFile = "$PSScriptRoot\load_data.ctl"

foreach ($dataFile in $dataFiles) {
@'
load data
infile '{0}'
append
into table ogd_xml_table
fields
(
    filename FILLER CHAR(100),
    xml lobfile( filename) terminated by eof
)
'@ -f $dataFile  #| Out-File $ctlFile -Encoding ASCII
    #sqlldr mckdev@$SecretDatabase/$SecretPassword control=$ctlFile
}

> Q:\Test\2019\01\24\CR_212133.ps1
load data
infile 'filelist21.dat'
append
into table ogd_xml_table
fields
(
    filename FILLER CHAR(100),
    xml lobfile( filename) terminated by eof
)
----------------------------------------------------------------------
load data
infile 'filelist22.dat'
append
into table ogd_xml_table
fields
(
    filename FILLER CHAR(100),
    xml lobfile( filename) terminated by eof
)
----------------------------------------------------------------------
%<...snip...>%


A simple clock in FORTH I try to program a stopwatch/countdown clock in FORTH with Gforth (and using Gforth-specific words).

I'm  a complete beginner and the following code is the basic stuff (going to add an alarm function / countdown option and more).

For now the stopwatch counts from 00:00:00 (hh:mm:ss) up to 24:00:00 and

pressing space pauses the clock, pressing it again resumes the clock
pressing j/k sets the clock back/ahead one minute
pressing J/K sets the clock back/ahead one hour
pressing q quits the program

Does my code follow best practices? Is it bad style for words like move-clock-seconds-back to return flags because they will be used in a begin ... until statement?

1000000 constant million

: sextal  ( -- )
   6 base ! ;

: hhmmss.  ( ud --  )
   drop million /  0 <# decimal # sextal # [char] : hold decimal # sextal # [char] : hold decimal # sextal # decimal #> TYPE ;

: pause-clock  ( ud -- ud f )
   utime
   begin
      50 ms
      key? if
         key bl = ( pause is released )
      else
        false ( clock keeps pausing )
      then
   until
   utime d- d-
   false ;

: move-clock-seconds-ahead  ( u ud -- ud f )
    million * 0 d-
    false ;

: move-clock-seconds-back  ( u ud -- ud f )
    million * 0 d+
    utime dmin
    false ;

: get-elapsed-time  ( ud -- ud )
    2dup
    utime
    2swap d- ;

: 24-hours-elapsed?  ( ud -- f )
    get-elapsed-time ( elapsed time fits in single integer )
    drop 24 60 * 60 * million * u> ;

: run-clock  ( -- )
    page ( clears the terminal )
    utime ( returns a ud timestamp in microseconds )
    begin
       50 ms ( sleep for 50 ms )
       get-elapsed-time
       5 0 at-xy ( coordinates where to print output )
       hhmmss.
       key? if
       key case
       bl of pause-clock endof
          [char] j of 60 move-clock-seconds-back endof
          [char] k of 60 move-clock-seconds-ahead endof
          [char] J of 60 60 * move-clock-seconds-back endof
          [char] K of 60 60 * move-clock-seconds-ahead endof
          [char] q of true endof
          false swap ( the char is now on top of the stack, will be dropped by endcase )
       endcase
       else 24-hours-elapsed? if
          CR
          ." 24 hours elapsed."
          true
       else
          false
       then then
    until ;

run-clock

bye

A comment in

: get-elapsed-time  ( ud -- ud )

seems wrong. The code leaves the elapsed time on top of the otherwise unchanged stack, so the action is really (-- ud).

The way get-elapsed-time leaves a double results in very unnerving drop in hhmmss. and 24-hours-elapsed?. I strongly recommend to make it leave a single integer instead.

I also recommend to reduce stack manipulations with

: get-elapsed-time
    utime
    2over
    d- drop ;

  Is it bad style for words like move-clock-seconds-back to return flags

Yes. It makes it very hard to reuse them in other contexts. Consider (untested)

    begin key dup [char] q <> while
        case
            ....
        endcase
    repeat


C - Fast & simple bump allocator I recently have been very interested in custom allocators, so I decided to make the very basic (this should be faster than malloc) bump allocator. Here is my code in C:

#include <stdio.h>
#include <stdlib.h>
#include <assert.h>
#include <sys/mman.h>
#include <unistd.h>

#define KB(size) ((size_t) size * 1024)
#define MB(size) (KB(size) * 1024)
#define GB(size) (MB(size) * 1024)
#define HEAP_SIZE GB(1)

typedef intptr_t word_t;
void* free_ptr = NULL;
void* start_ptr;
word_t end_ptr;

void init() {
    free_ptr = mmap(NULL, HEAP_SIZE, PROT_READ | PROT_WRITE,
                    MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
    if (free_ptr == MAP_FAILED) {
        printf("unable to map memory\n");
        abort();
    }
    start_ptr = free_ptr;
    end_ptr = (word_t) start_ptr + HEAP_SIZE;
}

void* bump_alloc(size_t size) {
    void* new_ptr = free_ptr;
    free_ptr = (char*) free_ptr + size;
    return new_ptr;
}

void free_all_mem() {
    munmap(start_ptr, HEAP_SIZE);
}

int main() {
    init();
    int* x = (int*) bump_alloc(sizeof(int));
    assert(x != NULL);
    *x = 10000;
    printf("x: %d\n", *x);
    free_all_mem();
}

This is my first custom allocator so could I get some tips on optimization, etc.

could I get some tips on optimization, etc.

Alignment loss

free_ptr = (char*) free_ptr + size; simply increases the next available allocation to so many bytes later.  This differs from malloc() whose allocations meets all possible system alignment needs.

Either document that bump_alloc() does not provide aligned allocations or change code to do so.

Error messages

I'd expect the error message to go out stderr - yet your call.

// printf("unable to map memory\n");
fprintf(stderr, "Unable to map memory\n");

Missing include

intptr_t is define in <stdint.h>.  Best to include that rather than rely of a hidden inclusion.

Good type math

The below avoids int overflow.

#define KB(size) ((size_t) size * 1024)

Better code would () each macro parameter.

#define KB(size) ((size_t) (size) * 1024)

Yet I'd recommend rather than type-casting, which may narrow the math, allow gentle widening.  The below multiplication will occur with the wider of size_t and the type of size.

#define KB(size) ((size_t) 1024 * (size))

Unneeded cast, simplify

Casting not needed going from void * to an object pointer.  Size to the de-referenced type.  Easier to code right, review and maintain.

// int* x = (int*) bump_alloc(sizeof(int));
int* x = bump_alloc(sizeof *x);


Checking whether a string is a valid number I'm developing a Java Helper Library which has useful methods developers don't want to type out over and over. I'm trying to think of the best way of checking whether a string is a valid number.

I've always just tried something like Integer.parseInt(iPromiseImANumber); and if it throws an exception then it's not a valid number (integer in this case). I'm thinking that trying to catch an exception isn't good practice. Is there a better way to do this? (I'm thinking a regular expression could do it, but it doesn't seem to be as reliable as the parse option).

Here's my method as it stands. What could be improved?

/**
* Does a try catch (Exception) on parsing the given string to the given type
*
* @param c the number type. Valid types are (wrappers included): double, int, float, long.
* @param numString number to check
* @return false if there is an exception, true otherwise
*/
public static boolean isValidNumber(Class c, String numString) {
  try {
    if (c == double.class || c == Double.class) {
      Double.parseDouble(numString);
    } else if (c == int.class || c == Integer.class) {
      Integer.parseInt(numString);
    } else if (c == float.class || c == Float.class) {
      Float.parseFloat(numString);
    } else if (c == long.class || c == Long.class) {
      Long.parseLong(numString);
    }
  } catch (Exception ex) {
    return false;
  }
  return true;
}

NumberUtils.isNumber from Apache Commons Lang does that.


Use an optionally passed-in block without repeating yourself I need to open and read several files for a project. Some of them are plain text files, and I just want a standard File.open to read from. One of them is an XML file, and I want it automatically loaded into Nokogiri in the following way:

xml = File.open("#{LOCATION}/#{filename}.xml"){|f| Nokogiri::XML(f){|config| config.noblanks}}

So I created a function that takes a pattern to select a file, and then optionally opens it with a block so that I can pass in the Nokogiri block above in the case of the xml file.

How do I reduce the repetition in the code?

# in read_xml...
xml = file_from_pattern(XML_PATTERN){|f| Nokogiri::XML(f){|config| config.noblanks}}

# in read_audit...
audit_lines = file_from_pattern(AUDIT_PATTERN).readlines

def file_from_pattern(pattern)
  file_name = Dir.entries("#{LOCATION}").select{|f| f =~ pattern}[0]
  if block_given?
    yield File.open("#{LOCATION}/#{file_name}")
  else
    File.open("#{LOCATION}/#{file_name}")
  end
end

Some notes:

select {... }[0] -> detect { ... }.
Is LOCATION a string? Then, "#{LOCATION}" -> LOCATION
Use File#join instead of manual string manipulation.
The only repetition in your code is File.open("#{LOCATION}/#{file_name}"). The solution is very simple: create a local variable and use it.
Don't you check whether file_name is nil?
File#open accepts a block, why don't you simply pass it? you get the file descriptor closed for free in the block case.

I'd write:

def file_from_pattern(pattern, &block)
  file_name = Dir.entries(LOCATION).detect { |f| f =~ pattern }
  if file_name
    # Refactor 1:
    # fd = File.open(File.join(LOCATION, file_name))
    # block_given? ? yield(fd) : fd
    File.open(File.join(LOCATION, file_name), &block)
  end
end


Rendering a payroll template In a project I decided to parse a file and create functions during compilation to speed up rendering of a payroll template. I had (or still have?) a hard time grasping macros in Elixir so I started experimenting in a smaller scale. This was the winner and it also worked out in the project:

defmodule Swapper do
  defmodule Helper do
    defmacro make_custom_swapper(name, left, right) do
      quote bind_quoted: [n: name, l: left, r: right] do
        def unquote(:"#{n}_swapper")(unquote(l)), do: unquote(r)
        def unquote(:"#{n}_swapper")(unquote(r)), do: unquote(l)
      end
    end
  end

  defmodule Maker do
    require Helper
    defmacro generate_multiple_custom_swappers(pairs) do
      quote do
        for {n,l,r} <- unquote(pairs) do
          Helper.make_custom_swapper(n,l,r)
        end
      end
    end
  end
end

defmodule TestingGrounds do
  require Swapper.Maker
  require Swapper.Helper

  # testing Helper
  Swapper.Helper.make_custom_swapper(:lofa,:a,27)

  letters = String.split("abcdefghijklmnopqrstuvwxyz", "", trim: true)
  letter_swapper_list =
    for l <- letters do
      {:letter, l, String.upcase(l)}
    end

  things_to_swap = [{:eclipse, "moon", "sun"}|letter_swapper_list]

  # the big guns
  Swapper.Maker.generate_multiple_custom_swappers(things_to_swap)
end

I'm glad that it works but getting to this solution involved adding more stuff where I got lucky. It feels exactly like this when I first discovered &/1 that seemed like the solution to everything:

defmodule A do
  def massage(string) do
    string
    |> (&splitter/1).() # == splitter
  end

  defp splitter(str) do
    str
    |> String.split("")
  end
end

I like the simplicity of just using unquote at the module level to generate the defs, such as:

defmodule MyModule do
  def do_map_stuff(method, arg) do
    IO.inspect([method, arg])
  end

  Enum.each [:method1, :method2, :method3], fn method ->
    def unquote(method)(map) when is_map(map) do
    do_map_stuff(unquote(method), map)
  end

    def unquote(method)(arg)  do
    do_map_stuff(unquote(method), arg)
    end
  end
end


A parametrized Config singleton Here is an attempt to code a specific kind of Singleton - the one geared for our configuration needs. It needs to be initialized with a configuration location, do not allow copies or other instances, and provide feedback or fail in case of problems. Prefer graceful feedback from violated workflows. While not performance-critical (especially initialization), the correct behavior and thread safety are needed.

Configuration location differs in production/testing/development scenarios - or may be acquired via a network broadcast in the future. Getting location is outside of this class scope.

So, here is the Config.h header:

class Config {

public:
  typedef std::function<void(std::string)> DupInitCB;

  static void init(std::string loc, DupInitCB err_cb);
  static Config& instance();

private:
  static std::unique_ptr<Config> cfg;
  static std::mutex init_mutex;

  std::string url;

  Config(std::string loc);
  Config(const Config& src);              // not implemented
  Config& operator=(const Config& right); // not implemented

public:
  std::string val(std::string key);
};

The implementation Config.cpp:

#include "Config.h"

std::unique_ptr<Config> Config::cfg;
std::mutex Config::init_mutex;

void Config::init(std::string loc, DupInitCB err_cb) {
  std::lock_guard<std::mutex> lock(init_mutex);
  if (cfg != nullptr) {
    err_cb(loc);
  } else {
    cfg.reset(new Config(loc));
  };
};

Config& Config::instance() {
  if (cfg == nullptr) throw std::logic_error("Requested uninitialized Config");
  return *cfg.get();
}

Config::Config(std::string loc) : url(loc) {};

std::string Config::val(std::string key) {
  return key + " key requested at the location " + url;
}

Finally, a usage example:

#include "Config.h"

int _tmain(int argc, _TCHAR* argv[])
{

  try {
    std::cout << Config::instance().val("Upstream") << std::endl;
    std::cerr << "Did not trow an exception when called an uninitialized Config." << std::endl;
    return 1;
  }
  catch (std::logic_error e) {
    std::cout << "Properly thrown an error when called an uninitialized Config." << std::endl;
  }
  catch (...) {
    std::cerr << "Thrown an unexpected exception when called an uninitialized Config." << std::endl;
    return 1;
  }

  Config::init(
    "http://cnf.local/dev",
    [](std::string loc){ std::cout << "Duplicate Config init (" << loc << ")" << std::endl; }
  );

  Config::init(
    "http://cnf.local/prod",
    [](std::string loc){ std::cout << "Duplicate Config init (" << loc << ")" << std::endl; }
  );

  std::cout << Config::instance().val("Upstream") << std::endl;
  return 0;
}

A buildable VC++ 2012 solution is posted at the GitHub repo.

The specific questions I have are:

Does the instance() method need to be synchronized?
Is there a better way to handle violated workflow from the instance() method than throwing an exception?
What are the dangers of call-back from under the lock?

Does the instance() method need to be synchronized?

Theoretically, yes: because unique_ptr isn't thread-safe: therefore you should not call get while another thread is calling reset.

It may be safe in practice.

Alternatively you don't need all the functionality of a unique_ptr.
Instead you could implement its functionality using a dumb pointer, which you can reason about and/or access using the STL atomic functions.

  Is there a better way to handle violated workflow from the instance() method than throwing an exception?

You could return a pointer instead of a reference, and return a null pointer instead of throwing an exception: and expect the caller to check for null before dereferencing the pointer.

Another possibility may be the Null Object pattern: if there's no Config instance then return a reference to an empty Config instance, or a Config instance initialized with suitable default values.

  What are the dangers of call-back from under the lock?

In general the danger of callback from under a lock is 'deadly embrace' a.k.a. 'deadlock'; for example:

Thread 1:

// get the lock
lock_guard<mutex> lock(foo_mutex);
// get the config
config.init("foo", [](string loc){ cout << "I don't care"; });

Thread 2:

config.init("foo", [](string loc){
    lock_guard<mutex> lock(foo_mutex);
    cout << "I don't care";
});

Imagine the following sequence:

Thread 2 enters config and acquires config's lock
Thread 1 acquires the the foo_mutex
Thread 1 blocks on the config mutex (owned by thread 2)
Thread 2 enters the callback
Thread 2 callback blocks on the foo_mutex (owned by thread 1)

It's safe to acquire multiple locks if you always acquire them in the same sequence.
With a callback it's difficult for the author of the Config class to predict what locks might be held before and/or during the callback.
This is a hidden problem for low-level library classes: for example if Logger uses Config in its implementation, Logger and Config each have their own mutex, and a callback from Config tries to invoke a Logger method.

Another problem with callback from under a lock is that you don't know how long the callback is going to take. If the callback takes a long time to execute, then the lock will be held for a long time.

  OK, so callback is now gone.

Perhaps you can keep the callback provided you unlock before calling it:

void Config::init(std::string loc, DupInitCB err_cb) {
  bool succeeded;

  { // scope of lock_guard
    std::lock_guard<std::mutex> lock(init_mutex);
    if (cfg != nullptr) {
      succeeded = false;
    } else {
      cfg.reset(new Config(loc));
      succeeded = true;
    };
  } // lock_guard is released here

  // error callback after releasing the lock`enter code here`
  if (!succeeded)
    err_cb(loc);
};

Or the code within the artificial { ... } scope above could be a private method which returns bool, called from init, and named something like locked_init.


Given a list of numbers and a number k, return whether any two numbers from the list add up to k Given a list of numbers and a number k, return whether any two numbers from the list add up to k.
For example, given [10, 15, 3, 7] and k of 17, return true since 10 + 7 is 17.

How can i improve the below solution?

    package com.alpha;

    import java.util.HashMap;
    import java.util.Map;

    public class DailyCodingProblem1 {
        public static void main(String args[]) {
            int[] arr = { 10, 15, 3, 7 };
            int k = 17;

            Boolean ans = solution(arr, k);
            System.out.println(ans);
        }

        private static Boolean solution(int[] arr, int k) {
            final Map<Integer, Boolean> map = new HashMap<>();
            for (int x : arr) {
                map.put(k - x, true);
            }
            for (int x : arr) {
                if (Boolean.TRUE.equals(map.get(Integer.valueOf(x)))) {
                    return true;
                }
            }
            return false;
        }
    }

Boolean -vs- boolean

Java provides both a Boolean and a boolean type; the difference being the first type is an Object where as the second is just a value.  Wherever possible, you should use a value type, for efficiency.  Since you return a simple true or false result, boolean is the correct return type to use:

...
    boolean ans = solution(arr, k);
...
private static boolean solution(int[] arr, int k) {
...

Autoboxing

The java Collections need Objects as keys and values.  The Java language will automatically "box" and "unbox" values as required.  You take advantage of autoboxing in map.put(k-v,true) ... The value k-v is autoboxed as an Integer and true is autoboxed as Boolean.TRUE.

Yet, when you retrieve the value from map, you explicitly do the autoboxing yourself: map.get(Integer.valueOf(x)).  You could simply write map.get(x).

Map -vs- Set

When you have a Map<K,V>, it is expected that different values will be stored in the map (under one or more keys).

In this case, you are only ever storing Boolean.TRUE (autoboxed from true).  Your map.get() either returns Boolean.TRUE, or it returns null if no value was stored under that key.  You explicitly test if it is equal to the former value, but you could have tested the value was not null instead.  Or more directly, you could test whether a value was stored under that key, and replace the complicated statement with simply:

if (map.containsKey(x)) { ... }

At this point, it should be clear we don't need a HashMap.  Simply a Set of Integer values.

final Set<Integer> set = new HashSet<>();
...
   set.add(k-x);
...
   if (set.contains(x)) {
...

Algorithm

As mentioned by hodisr, you need to take into account that the list of numbers may contain only one copy of the value x which is exactly half of the target value (k/2).  With your code, you add k-x to your container, and later successfully pull out the flag corresponding to x, and declare true.

Then, hodisr proceeds to demonstrate how to do it in one pass, and goes ahead and repeats the same error, by adding k-x to the container before testing whether x can be pulled out.  To obtain the correct result, the check for x must be made before putting k-x into the container.

private static Boolean solution(int[] arr, int k) {
    Set<Integer> set = new HashSet<>();

    for (int x : arr) {
        if (set.contains(x))
            return true;
        set.add(k-x);
    }
    return false;
}


Simple email validation script I was doing a programming challenge and the object was to validate an email address, I wrote a simple little email validator and would like some input on how I can make it more efficient.

How it works, it has two validations, one with a regex, and one validating the length (most email addresses are over 12 characters, so that's what I went with).
The regex checks for an @ and a . symbol. This is all inside of its own validation class.

Source:

import re

class Validator(object):

    def __init__(self, email):
        self.email = email

    def check_for_symbol(self):
        if re.search("[@.]", self.email) is None:
            return False
        else:
            return True

    def check_length(self):
        if len(self.email) >= 12:
            return True
        else:
            return False

def email_to_verify():
    return raw_input('Enter your email address: ')

def check_email(email):
    validator = Validator(email)
    symbol = validator.check_for_symbol()
    if symbol is False:
        print "Your email address is not valid. Failed symbols.."

    length = validator.check_length()
    if length is False:
        print "Your email is not valid. Failed length"

    if length and symbol is True:
        print "Email is a valid email address."

if __name__ == '__main__':
    e = email_to_verify()
    check_email(e)

Is there a way to make this more efficient?

Examples of usage:

C:\challenges>python email_val.py
Enter your email address: test
Your email is not valid. Failed symbols check.
Your email is not valid. Failed length check.

C:\challenges>python email_val.py
Enter your email address: te@t.t
Your email is not valid. Failed length check.

C:\challenges>python email_val.py
Enter your email address: gmail@gmail.gmail
Email is a valid email address.

Interface design

The current design of the validator class is not very intuitive.
Users of this class must call the check_for_symbol and check_length methods to validate an email address.
They would only know this by reading the implementation.
You could add documentation,
but it would still not be easy to use.
The best is when the usage and behavior is obvious.

I would imagine the design of an email validator something like this:

class EmailValidator:
    def validate(self, email):
        # ...

Where the validate method would raise exceptions depending on what failed (such as length, symbols).

With such outline, even without documentation,
when a user does help(EmailValidator) and sees a single validate method,
the usage is obvious, clear.

What's worse,
the check_email function is not part of the validator class.
As such, the validator doesn't encapsulate the validation logic.
That might be acceptable if the public API is the check_email and the class is the implementation detail.

Coding style

Use boolean expressions directly. For example, instead of this:

def check_for_symbol(self):
    if re.search("[@.]", self.email) is None:
        return False
    else:
        return True

You could write as:

def check_for_symbol(self):
    return re.search("[@.]", self.email) is not None

Also, instead of x is True, use simply x, and instead of x is False, use not x.

See more on this in the style guide.

Compatibility with Python 3

When using Python 2,
to future-proof yourself it's good to adopt a writing style compatible with Python 3. For example, instead of print 'something', write print('something').


Django model to do accounting for a Litecoin miner pool I'm coming from Java so I am used to performing calculations I require in separate functions. I would like to know if the below code makes sense in my Django model especially the loop and calling functions from within functions.

The aim of this is to create a backend for multiple Litecoin miners (L3's). Every miner will contribute to the same pool but be plugged in at different times. Therefore each will get its own time stamp. Each month I will pull the total pool amount (pulled from api) and use the total amount of hours contributed to the pool to calculate the litecoins-to-hour ratio. This ratio I can then multiply to each miners' hours worked for the month to determine that particular miners Litecoin earned.

class L3(models.Model):
    user = models.ForeignKey(User, related_name="L3")
    MachineID = models.CharField(max_length=256)
    TimeOnline = models.DateTimeField()
    Commission = models.IntegerField(default='10')
    TimeOffline = models.IntegerField(default='0')

    def indivMachineHoursWorked(self):
        this_month = datetime.now().month
        if self.TimeOnline == this_month:
            x = datetime.now() - self.TimeOnline
        else:
            x = datetime.now() - datetime.today().replace(day=1) #need to update hour to 0 too i.e. start of month
        return x

    def totalMachineHoursWorked(self):
        L3_total = L3.objects.all()
        hrs=0
        for l3 in L3_total:
            hrs =+ l3.indivMachineHoursWorked()
        return hrs

    def btcToHour(self):
        main_api = 'https://api.nicehash.com/api?method=balance&id=123456&key=01234567-abcd-dcba-abcd-0123456789abc'
        url = main_api
        json_data = requests.get(url).json()
        print(json_data)
        balance1 = json_data['result']['balance_confirmed']
        print(balance1)
        btc2hour = balance1/self.totalMachineHoursWorked()
        return btc2hour

    def indivMachineTurnover(self):
        return self.indivMachineHoursWorked()*self.btcToHour()

    def __str__(self):
        return self.user.get_full_name()

totalMachineHoursWorked should be a @classmethod.  It could be written more succinctly like this, using sum():

@classmethod
def totalMachineHoursWorked(cls, self):
    return sum(l3.indivMachineHoursWorked() for l3 in cls.objects.all())

However, I have doubts that any of the calculations should be done in Python.  totalMachineHoursWorked() needs to fetch the entire table.  I would recommend trying to find a way to do all of these calculations in SQL instead, because that's what SQL servers are designed to do: provide an efficient tool to let you extract aggregate reports by formulating your query intelligently.

In particular, if you need to calculate btcToHour() for multiple hosts, that would be horribly inefficient, since it would need to calculate totalMachineHoursWorked() multiple times, each of which involves fetching the entire table, and also performing a calculation on an individual row.


Altering input field width This gets every form element and calculates its width, then changes the input fields depending on the parent-form width:

$ ->
 # Alter 'input' Width
  inputResize = ->
   $('form').each ->
    tF = $(this)
    formWidth = tF.width()
    tF.find('input').each ->
     tE = $(this)
     inputBorder = (tE.outerWidth() - tE.innerWidth())
     inputPadding = parseInt(tE.css('padding-left')) + parseInt(tE.css('padding-right'))
     tE.css 'width', ->
      (formWidth - inputBorder - inputPadding)
 # on Resize
 $(window).resize ->
  inputResize()
 # on Init
  inputResize()

It works as intended, but to learn a bit more I just want to know if there is a smarter way to write this little code in CoffeeScript.

Here are a few tips:
1) Use standard naming convention.
tE and tF sound like booleans and not a refence to an element.
Try names like el, $this or that instead.
2) Separate complex logic into smaller functions.
There's too much going on here.
 inputResize = ->
   $('form').each ->
    tF = $(this)
    formWidth = tF.width()
    tF.find('input').each ->
     tE = $(this)
     inputBorder = (tE.outerWidth() - tE.innerWidth())
     inputPadding = parseInt(tE.css('padding-left')) + parseInt(tE.css('padding-right'))
     tE.css 'width', ->
      (formWidth - inputBorder - inputPadding)

Since tF is only used then you can get rid of it and use $(this) directly.
Next, extract the input width size calculation to one function and you get something like this.
  getNewInputSize = (el, formWidth) ->
    inputBorder = el.outerWidth() - el.innerWidth()
    inputPadding = parseInt( el.css("padding-left"), 10) + parseInt(el.css("padding-right"), 10)
    formWidth - inputBorder - inputPadding

  resizeFormInputs = ->
    $("form").each ->
      formWidth = $(this).width()
      $(this).find("input").each ->
        $(this).css "width", getNewInputSize($(this), formWidth)

Final Code:
$ ->
  getNewInputSize = (el, formWidth) ->
    inputBorder = el.outerWidth() - el.innerWidth()
    inputPadding = parseInt( el.css("padding-left"), 10) + parseInt(el.css("padding-right"), 10)
    formWidth - inputBorder - inputPadding

  resizeFormInputs = ->
    $("form").each ->
      formWidth = $(this).width()
      $(this).find("input").each ->
        $(this).css "width", getNewInputSize($(this), formWidth)

  $(window).resize(resizeFormInputs).triggerHandler "resize"


JSON formatting and processing hierarchies I have a properties file I'm using for GIS software. You have a feature like a road, then properties under it, like so:

{
    "road": {
        "colour": "rgb(0,0,0)"
     },
     "railway": {
        "colour": "rgb(100,100,100)"
     }
}

Now I need to group these into categories, for example, transportation. The first thing I thought of was:

{
    "transportation":
    {
        "road": {
            "colour": "rgb(0,0,0)"
        },
        "railway": {
            "colour": "rgb(100,100,100)"
         }
    }
 }

This makes things awkward when I want to access "road" (for example), because now I need to know it is under "transportation" (mind you I could always write a helper function to eliminate this). Further, "road" could be under multiple groups, perhaps "transportation" and "public works". Now I would need to duplicate the properties in both instances.

Would I be wiser to use the following format, or is there a better way to do this?

{
    "road": {
        "colour": "rgb(0,0,0)",
        "group": ["transportation"]
     },
     "railway": {
        "colour": "rgb(100,100,100)",
        "group": ["transportation"]
     }
}

Assuming you're using JavaScript to manipulate the object, you could also split it up into 2 different structures, one for maintaining the hierarchy and another for easy access:

var hierarchy = {
    transportation: {
        road: {
            colour: "rgb(0,0,0)"
        },
        railway: {
            colour: "rgb(100,100,100)"
        }
    }
};

var features = {
    road: hierarchy.transportation.road,
    railway: hierarchy.transportation.railway
};


Take a specified weekday, and check if it falls on the remaining days of the current month So like the title says, if we are close to the end of the month, we can check if a given weekday (or array of weekdays) will fall from the current day, to the end of the month. If any of the given days appear, return true, else return false.

I'm quite inexperienced with JS, and this is my first time using the map function. I think splitting the two functions is ok, although I've seen map used with a nested anonymous function.

Any feedback on the solution or the use of map would be appreciated.

// Remember, in the JS date object, months are zero based, days are one based XD
// take a project weekday, and check if it falls on the remaining days of the current month
function check_days_remaining(day) {

  var d = new Date();
  var days_in_month = new Date(d.getYear(), (d.getMonth() + 1), 0).getDate();
  var month_day = d.getDate();
  var month_days_remaining = days_in_month - month_day;

  for(i = 1; i <= month_days_remaining; i++) {
    var index_date = new Date(d.getYear(), (d.getMonth() + 1), (d.getDate()+i) );
    var index_day = index_date.getDay();
    if(index_day == day) {
      //console.log("Weekday: " + index_day + " falls on " + index_date);
      return true;
    }
  }

}

function days_remaining(days) {
  remaining = days.map(check_days_remaining);
  // console.log(remaining); // logs an array of true or undefined elements
  return ( remaining.indexOf( true ) > -1 ) ? true : false; // true if at least one project day is remaining in current month
}

var stay_on_month = days_remaining([5,6]); // set whether to stay on current month or display next month
console.log(stay_on_month); // check result

Calculate the accepted weekdays range for today's week once, then check the input array for any value in the range.

function weekdaysBelongToTheRestOfThisMonth(weekdays) {
    var today = new Date();
    var todayDay = today.getDate();
    var todayWeekday = today.getDay();

    var maxMonthDate = new Date(today.getFullYear(), today.getMonth() + 1, 0);
    var maxMonthDay = maxMonthDate.getDate();
    var maxMonthWeekday = maxMonthDate.getDay();
    var maxWeekday = maxMonthDay - todayDay > 7 - todayWeekday ? 7 : maxMonthWeekday;

    return weekdays.some(function(d) { return d >= todayWeekday && d <= maxWeekday });
}


Project Euler 34 - digit factorials 145 is a curious number, as \$1! + 4! + 5! = 1 + 24 + 120 = 145\$.

  Find the sum of all numbers which are equal to the sum of the
  factorial of their digits.

  Note: as \$1! = 1\$ and \$2! = 2\$ are not sums they are not included.

I can't figure out a fair way to optimize the upper bound from the information given in the question. I went on the PE forum and found many people setting the upper bound to 50000 because they knew that would be large enough after testing. This doesn't seem fair to me; I want to set the bound based on the information in the question. Right now it runs in around 20 seconds.

EDIT: I'm not looking for a mathematical algorithm. I'm looking for ways to make this code faster.

from math import factorial as fact
from timeit import default_timer as timer

start = timer()
def findFactorialSum():
    factorials = [fact(x) for x in range(0, 10)] # pre-calculate products
    total_sum = 0
    for k in range(10, fact(9) * 7): # 9999999 is way more than its fact-sum
        if sum([factorials[int(x)] for x in str(k)]) == k:
            total_sum += k

    return total_sum

ans = findFactorialSum()
elapsed_time = (timer() - start) * 1000 # s --> ms

print "Found %d in %r ms." % (ans, elapsed_time)

I'm not aware of any mathematical way to establish the upper bound for the search space (I used the same upper limit as you did). However, there are some optimizations you can make:

Use integer math throughout instead of converting to a string, extracting the digits, then converting back to an integer. On my PC, your algorithm ran in approximately 6200ms. Using integer math as follows (there may be a more Pythonic way to do this; my code is a direct transliteration of C++ that I used), it ran in approximately 1600ms -- almost 4 times as fast:

def findFactorialSum():
    factorials = [fact(x) for x in range(0, 10)] # pre-calculate products
    total_sum = 0
    for k in range(10, fact(9) * 7): # 9999999 is way more than its fact-sum
        tmp = k
        total = 0
        while tmp > 0:
            total += factorials[tmp % 10]
            tmp //= 10

        if total == k:
            total_sum += k

    return total_sum

(For my curiosity, I also tried it with the '/' operator instead of the '//' operator, and consistently got 100ms longer run times with the former.)
You're doing an exhaustive search of the numbers from [10...7*9!] to see which ones meet the problem criteria. You can eliminate a large proportion of those numbers:

Hint 1:

   No 2-digit number can have a digit >= 5. This is because 5! == 120, so any 2-digit number with a 5 (or higher) in it will automatically have a 3-digit (or longer) sum. You can extend this rule for 3-, 4- and 5- digit numbers.

Hint 2:

   Extending Hint 1: No 3-digit number can have more than one 7 in it. 7! is 720, so if you have two 7s, you have 1440, a 4-digit number. There are a few more opportunities like this to eliminate numbers to check

Hint 3:

   Think of the number 145 given in the problem; since you know that this works, there's no need to check numbers that are a permutations of its digits: 154, 415, 451, 514, 541. The trick here is to look at the problem the other way around: instead of finding numbers whose Sum of Factorials of Digits equals the original number, find an ordered sequence of digits whose Sum of Factorials can be split into a sequence of digits, sorted, and that compares equal to the original sequence.


Nginx-WordPress installer for Debian I have written the following Bash script which includes an Nginx WordPress bootstrapper: A program establish WordPress apps fast on a Debian-Nginx environment.

It utilizes CertBot for HTTPS, SSHguard for protection from BFA (see Nginx conf), WP-CLI for creating a WP app dir and conf file fast, MySQL CLI to create a dbstack and a local function named rse to restart the server and giving proper permissions to the app dir.

#!/bin/bash
domain="$1" && test -z "$domain" && exit 2

environment() {
    read -sp "Please enter the app DB root password:      " dbrootp_1 && echo
    read -sp "Please enter the app DB root password again:" dbrootp_2 && echo
    if [ "$dbrootp_1" != "$dbrootp_2" ]; then echo "Values unmatched. Please try again." && exit 2 fi

    read -sp "Please enter the app DB user password:      " dbuserp_1 && echo
    read -sp "Please enter the app DB user password again:" dbuserp_2 && echo
    if [ "$dbuserp_1" != "$dbuserp_2" ]; then echo "Values unmatched. Please try again." && exit 2 fi
}
environment

wordpress() {
    rm -rf "$drt"/"$domain"/ 2>/dev/null
    wp core download --path="$drt"/"$domain"/ --allow-root
    wp config create --path="$drt"/"$domain"/ --dbname="$domain" --dbuser="$domain" --dbpass="$dbuserp" --dbhost="localhost" --allow-root
}
wordpress

nginx() {
    rm "$s_a/$domain.conf" 2>/dev/null
    rm "$s_e/$domain.conf" 2>/dev/null

    cat <<-EOF > "$s_a/$domain.conf"
        server {
            root ${drt}/${domain}/;
            server_name ${domain} www.${domain};
            location ~* \.(jpg|jpeg|png|gif|ico|css|js|ttf|woff|pdf)$ {expires 365d;}
            location "/wp-login\.php" {access_log "/var/log/httpd/wordpress-logins.log";}
        }
    EOF

    ln -sf "$s_a"/"$domain".conf "$s_e"
    rse
}
nginx

certbot() {
    certbot --nginx -d "$domain" -d www."$domain"
    rse
}
certbot

database() {
    cat <<-EOF | mysql -u root -p"$dbrootp_1"
        DROP USER IF EXISTS "$domain"@"localhost";
        DROP database IF EXISTS "$domain";
        CREATE USER "$domain"@"localhost" IDENTIFIED BY "$dbuserp_1";
        CREATE DATABASE "$domain";
        GRANT ALL PRIVILEGES ON "$domain".* TO "$domain"@"localhost";
    EOF
}
database

finalize() {
    echo "Change http to http2 in your Nginx app conf and run rse"
}
finalize

Note

Before executing the script I declared in the end of $HOME/.bashrc:

drt="/var/www/html"
s_a="etc/nginx/sites-available"
s_e="etc/nginx/sites-enabled"

My answer is going to focus on your use of shell functions and global variables.  There aren't any actual syntax errors in your script AFAICT, but your misuse of functions in this script is dangerously error-prone.

There are two main reasons for using a function in bash (or any language, really).

The first is to parameterise a frequently performed operation - e.g. by passing arguments to it rather than hard-coding values (or relying on side-effects).  Optionally it should return a result to the caller (in shell, by printing something to stdout to be captured with command substitution.  Or sometimes by changing a variable in the environment - but see below about that)

You're not doing that.

Worse, you're making your functions rely on global variables.  This makes your functions (and your scripts) dependent on side-effects of previous variable definitions - which may still hold old values from previous runs (and you forgot to update them for the current run) or, worse, may be empty because they haven't been initialised yet in the current shell.  This will inevitably lead to catastrophic data-loss one day when you forget to set a value for "$drt" and/or "$domain" and/or your many other global variables.

Better to pass values as parameters when and as needed. Don't trust that a crucial variable happened to have been set some time in the past - i.e. be explicit, not implicit.

As a rule, functions should be self-contained and not rely on variables that may or may not be defined outside of their scope.  They should also define variables as being local to them so that they explicitly don't use (or worse, change) a global variable that happens to have the same name as the function's variable.

Anyway, an example of how your wordpress function could be written:

wordpress() {
    local drt="$1"
    local domain="$2"
    local dbpass="$3"

    # should do more checking here, this is the bare minimum.
    # it avoids running `rm -rf /` if both $drt and $domain
    # are empty, and running `rm -rf "$drt/"` if $domain is empty.
    [ -z "$drt"    ] && error 1 '$drt is empty. aborting!'
    [ -z "$domain" ] && error 1 '$domain is empty. aborting!'
    [ -z "$dbpass" ] && error 1 '$dbpass is empty. aborting!'

    # should check exit status of each command run here and take
    # appropriate action on any failures.  Figure out what you want
    # to happen if any of the following fail and implement it.
    rm -rf "$drt"/"$domain"/ 2>/dev/null
    wp core download --path="$drt"/"$domain"/ --allow-root
    wp config create --path="$drt"/"$domain"/ --dbname="$domain" \
      --dbuser="$domain" --dbpass="$dbpass" --dbhost=localhost \
      --allow-root
}

You'd call it as wordpress "$drt" "$domain" "$dbpass".  It uses a function called error:

error() {
  local ec msg

  ec="$1" ; shift
  msg="$*"

  [ -n "$msg" ] && echo "$msg" >&2
  # don't exit if $ec==0 - just return after printing a warning to stderr
  [ "$ec" != 0 ] && exit "$ec"
}

The second reason is to make the structure of the script easier to follow.  For example, instead of having a lot of shell code for each pattern matched in a case statement (which makes it difficult to see at a glance what the cases are and what each one does), write a function for each case and call it - with or without arguments.

You're not doing that either - you're using each function as soon as you define it.  That's both weird and pointless....especially so since your functions don't take arguments.

A more usual way of defining and using functions would be something like:

function1 () { ... ; }
function2 () { ... ; }
function3 () { ... ; }
function4 () { ... ; }

main () {
  function1
  function2
  function3
  function4
}

main

Note: using amain function allows you to define and call functions in any order, and allows you to define your functions (other than main() itself) at the end of the script so that the main code comes first.  Otherwise you can get errors in situations where, e.g., your main non-function script code calls function2 before it is defined.

as noted above, each function call can, or should, have explicit arguments passing in the values they will use.

BTW, for the case statement mentioned above:

case "$var" in
  1) function1 ;;
  2) function2 ;;
  3) function3 ;;
  4) function4 ;;
  *) error 1 'Unknown case' ;;
esac

That case statement would be a lot harder to read if each case had 5 or 10 or more lines of code.


A simple Insertion Sort implementation in Java I've written a InsertionSort algorithm in Java and I want a review on these:

Performance improvements.
Code conventions.
Algorithm design improvements.
Cleaner approaches.

I would highly appreciate if you can review based on the points above, and I would prefer if you can add more points on top if it if you find necessary.

Here's the code:

package com.hassanalthaf.sortingalgorithms;

public class InsertionSort {
    private final int SECOND_NUMBER_INDEX = 1;

    public int[] sort(int[] numbers) {
        for (int iterations = this.SECOND_NUMBER_INDEX; iterations < numbers.length; iterations++) {
            int currentNumber = numbers[iterations];
            int newIndex = iterations;

            for (int comparisons = iterations; comparisons > 0; comparisons--) {
                if (currentNumber < numbers[comparisons - 1]) {
                    newIndex--;

                    numbers[comparisons] = numbers[comparisons - 1];
                }
            }

            numbers[newIndex] = currentNumber;
        }
        return numbers;
    }
}

Here's my main class calling the InsertionSort sort method:

package com.hassanalthaf.sortingalgorithms;

public class Main {

    public static void main(String[] args) {
        int[] numbers = {1, 20, 51, 12, 43, 2, 20};

        InsertionSort insertionSort = new InsertionSort();

        numbers = insertionSort.sort(numbers);

        for (int iterations = 0; iterations < numbers.length; iterations++) {
            System.out.println(numbers[iterations]);
        }
    }

}

This produces an output:

1
2
12
20
20
43
51

this code is not commented
strikingly, it does not even have doc comments
given at least two classes from the same coder that do essentially the same thing, there should be an 'interface' - look for one you can use (if as a template), especially from the Java API
SECOND_NUMBER_INDEX is close to meaningless
as a "genuine" constant, SECOND_NUMBER_INDEX should be static
int[] sort(int[] numbers) would then not use any instance (data) members and should be a class/static method instead of an instance one
if iterations was called ordered or sorted, the condition of the outer loop read close to a loop invariant
having found the first element no bigger than currentNumber, continuing to index 0 isn't called for - just place currentNumber and break the inner loop, no need for newIndex

Use a foreach-loop in main().


Hexadecimal to string without C++ standard library functions For the purposes of this code review, I will use the standard library, but pretend that <cstddef> is <stddef.h>, that I didn't use std::cout, that new and delete[] are overloaded and call custom malloc functions, etc. So this is -std=c++11 -ffreestanding.

Here are my questions:

Currently my tohex function returns a buffer allocated on the heap. Is it better if I have the caller pass the buffer as a parameter instead? Is there a way to avoid heap allocation without using variable length arrays?
I have a separate function that counts the size of the digits so that I can reverse the string. What's an alternate way of doing this?
Should I prefer uint8_t or int or is unsigned okay? I don't think there should be any issues with what type I choose in the code shown.

#include <cstddef>

unsigned count(unsigned n)
{
    int digits = 0;
    while (n > 0)
    {
        ++digits;
        n /= 16;
    }
    return digits;
}

char* tohex(unsigned n)
{
    unsigned size = count(n) + 1;
    char *buffer = new char[size];
    unsigned index = size - 2;
    while (n > 0)
    {
        unsigned mod = n % 16;

        if (mod >= 10)
            buffer[index--] = (mod - 10) + 'A';
        else
            buffer[index--] = mod + '0';

        n /= 16;
    }
    buffer[size - 1] = '\0';
    return buffer;
}

#include <iostream>

int main()
{
    char *buffer = tohex(0xBEEFCAFE);
    std::cout << buffer;
    delete[] buffer;
    return 0;
}

As both the others have said use std::string if you are going to dynamically allocate space. Its not going to be worse than your code;if your compiler implements the small string optimizations its actually goinf to do better than your code as no dynamic memory will be used.

unsigned count(unsigned n)

Well we know that unsigned have a particular size sizeof(unsigned). So we know that the max size of the array is sizeof(unsigned)*CHAR_BITS/4. On most systems this will be 8 on some it will be 16. But neither of these numbers is huge why not always use the maximum size (it will save you the time of calculating an exact size.

You can also get away with no dynamic memory allocation by having a static array in the function (not thread safe but it does not look like that is your issue).

Others have suggested replacing the n /= 16; by n >> 4;. DON'T do that (unless you think it expresses intent better). It hides the intent of the code. The code is supposed to express the intent in the most natural way possible. It is the job of the compiler to see that this can be optimized to a shift by 4 and I am dead sure the compiler will generate the most efficient code.


Refactor 2 regular expressions in Javascript into 1 statement I have the following code, which replaces " " <-- Spaces with "-" <-- Dash.
Because some of the title text will be returned with more spaces, it converts it into 3 dashes.

Examples of Possible Titles:

"Hello  World  From  Jquery"  <--- Notice 2 Spaces between the words, not 1
"Main Title - Sub-Title or description here"

I would like the above titles to be turned into:

"Hello-World-From-Jquery" <---- (Not "Hello--World--From--Jquery")
"Main-Title-Sub-Title-or-description-here" <--- (Not "Main-Title---Sub-Title-or-desc...")

This is what I got so far, but its not ideal as you can see.
Example:

var dirty_url = ui.item.label;
var url = dirty_url.replace(/\s+/g,"-");
var url = url.replace(/---/g,"-");

So basically, I want to convert all spaces and illegal characters (Its for a url) to "-" to be used in a url. I only want a maximum of 1 dash between charactors.

Shouldn't this work:

var dirty_url = ui.item.label;
var url = dirty_url.replace(/[-\s]+/g,'-')

Or, for more thorough:

var url = dirty_url.replace(/[-\s@#$%^&*]+/g,'-') // etc...

Though at this point, you might just want to remove all non word characters:

var url = dirty_url.replace(/\W+/g,'-')


Script to access genealogy API for list of users and attributes This is a script to access the Stack Exchange API for the genealogy site and create a list of user dictionaries.  It contains a snippet at end to validate.  The data obtained is stored in YAML format in a file for use by other programs to analyze the data.

'''
This is a script to access the stackexchange api
    for the genealogy site
    and create a list of user dictionaries
'''

# use requests module
#      - see http://docs.python-requests.org/en/latest/user/install/#install
import requests

# use YAML to store output for use by other programs
#     see http://pyyaml.org/wiki/PyYAML
import yaml
OUTFILENAME = "users.yaml"

# use se api and access genealogy site
#   see https://api.stackexchange.com/docs/users for api info
URL ='https://api.stackexchange.com/2.1/users'

url_params = {
    'site'     : 'genealogy',
    'pagesize' : 100,
    'order'    : 'desc',
    'sort'     : 'reputation',
    }

page = 1
not_done = True
user_list = []

# replies are paginated so loop thru until none left
while not_done:
    url_params['page'] = page
    # get next page of users
    api_response = requests.get(URL,params=url_params)
    json_data = api_response.json()

    # pull the list of users out of the json answer
    user_list.extend( json_data['items'] )

    # show progress each time thru loop
    print api_response.url
    #note only so many queries allowed per day
    print '\tquota remaining: %s' % json_data['quota_remaining']
    print "\t%s users after page %s" % (len(user_list),page)

    # prepare for next iteration if needed
    page += 1
    not_done = json_data['has_more']

# output list of users to a file in yaml, a format easily readable by humans and parsed
#   note safe_dump is used for security reasons
#   since dump allows executable python code
#   Sidebenefit of safe_dump is cleaner text
outFile = open(OUTFILENAME,"w")
yaml.safe_dump(user_list, outFile)
outFile.close()

# validate it wrote correctly
#   note this  shows example of how to read
infile = open(OUTFILENAME)
readList = yaml.safe_load(infile)
infile.close()

print 'wrote list of %d users as yaml file %s' % (len(readList),OUTFILENAME)

I wrote this code because I wanted the data it gets from the genealogy site but also:

to learn APIs in general
to learn the Stack Exchange API
to get back into programming which I hadn't done much lately
I thought I'd try YAML

Since it's such a simple script I didn't bother with objects, but I would be interested in how it could be remade in the functional programming paradigm which I'm not as familiar with.

'''
This is a script to access the stackexchange api
    for the genealogy site
    and create a list of user dictionaries
'''

# use requests module - see http://docs.python-requests.org/en/latest/user/install/#install
import requests

# use pretty print for output

That's a pretty useless comment. It doesn't really tell me anything I didn't already know from the import

import pprint

# save output in file in format compatable with eval
#   I didn't use json or yaml since my intent is to continue on using
#   the parsed output so saves a parse step

eval also has a parse step. So you aren't saving anything by avoiding json.

pytFilename = "users.pyt"

python convention is for constants to be in ALL_CAPS

# use se api and access genealogy site
#   see https://api.stackexchange.com/docs/users for api info
url='https://api.stackexchange.com/2.1/users'
site = 'genealogy'

urlParams = {}
urlParams['site'] = site
urlParams['pagesize'] = 100
urlParams['order'] = 'desc'
urlParams['sort'] = 'reputation'

Why not put all these parameters into a single dict literal?

page = 1
notDone = True
userList = []

Python convention is for local variables to be mixed_case_with_underscores.

# replies are paginated so loop thru until none left
while notDone:
    urlParams['page'] = page
    # get next page of users
    r = requests.get(url,params=urlParams)

avoid single letter variable names. It makes it harder to figure out what you are doing.

    # pull the list of users out of the json answer
    userList += r.json()['items']

I'd use userList.extend( r.json()['items']). Also given the number of times you access the json, I'd have ajson = r.json()` line and access the json data through that.

    # show progress each time thru loop, note only so many queries allowed per day
    print r.url
    print '\tquota remaining: %s' % r.json()['quota_remaining']
    print "\t%s users after page %s" % (len(userList),page)

    # prepare for next iteration if needed
    page += 1
    notDone = r.json()['has_more']

I'd do:

json = {'has_more' : True}
while json['has_more']:
     ...
     json = r.json()
     ...

As I think it's easier to follow then a boolean flag.

# output list of users to a file in a format readable by eval
open(pytFilename,"w").write( pprint.pformat(userList) )

This isn't recommended practice. In CPython this will close the file, but it won't if you are using one of the other pythons. Instead, it's recommend to do:

with open(pytFilename, 'w') as output:
     output.write( pprint.pformat(userList) )

And that will close the file in all implementations of python.

# validate it wrote correctly
#   note this  shoasw example of how to read
readList = eval( open(pytFilename,"r").read() )

print 'wrote %s as python evaluatable list of users' % pytFilename


A general-use For-comprehensible Loan Pattern I use Scala's loan pattern in order to ensure and automate the resource aquisition and release after its use. When multiple resources have to be acquired at once, however, the function calls must be nested, impairing readability.

Scala ARM supports for-comprehension to avoid the nesting. However, in order to have Scala ARM manage resources, the managed class must extend scala.resource.Resource or simply AutoCloseable, which requires a definiton of close() method.

I'd like to avoid defining close() methods because it separates aquisition and release into different methods but I believe such operations are so tightly coupled that keeping them close will enhance the maintainability.

My idea is to restrict resource access to only via certain loaner-functions, while supporting for-comprehension to keep readability.

So, I wrote a trait ForComprehensible:

/**
 * @tparam A The type of the loaned object.
 */
trait ForComprehensible[+A] {
  /**
   * The loaner-function. Subclasses must define this.
   *
   * @tparam B The result type of the callback
   * @param  f The callback
   * @return The result value of the callback
   */
  def apply[B](f: A => B) : B

  def map[B](f: A => B)     : B    = apply(f)
  def flatMap[B](f: A => B) : B    = apply(f)
  def foreach[B](f: A => B) : Unit = { apply(f); () }

  // The methods filter and withFilter are omitted on purpose because they don't make sense.
}

Since ForComprehensible defines the apply method, client code can use one much like an ordinary loaner-function. Here is a simple example program which copies a content of a file into another:

import java.io.FileInputStream
import java.io.FileOutputStream
import java.io.InputStream
import java.io.OutputStream
import scala.annotation.tailrec

// Opens a FileInputStream and loans it to the callback. After that, closes the stream.
def withInputStream(infilename: String) = new ForComprehensible[FileInputStream] {
  override def apply[B](f: FileInputStream => B) = {
    val res = new FileInputStream(infilename)
    try {
      f(res)
    } finally {
      res.close()
    }
  }
}

// Opens a FileOutputStream and loans it to the callback. After that, closes the stream.
def withOutputStream(outfilename: String) = new ForComprehensible[FileOutputStream] {
  override def apply[B](f: FileOutputStream => B) = {
    val res = new FileOutputStream(outfilename)
    try {
      f(res)
    } finally {
      res.close()
    }
  }
}

// Pipes all the content of the input stream into the output stream.
@tailrec def pipe(is: InputStream, os: OutputStream) : Unit = {
  val bytes = new Array[Byte](1024)
  val n = is.read(bytes)
  if (n >= 0) {
    os.write(bytes, 0, n)
    pipe(is, os)
  }
}

// The following are equivalent. Both automatically close streams.
// Without for-comprehension:
withInputStream("input.txt"){ is =>
  withOutputStream("output.txt"){ os =>
    pipe(is, os)
  }
}

// With for-comprehension:
for {
  is <- withInputStream("input.txt")
  os <- withOutputStream("output.txt")
} {
  pipe(is, os)
}

It works and looks nice. Before writing this, I googled for "scala loan pattern for-comprehension" and found nothing looking relevant. I think this trait is quite straight-forward and I doubt no one ever come up with something like this.

So I've got skeptical; is there any problem with this?

As long as you restrict yourself to simple uses, I think it's ok. When you try complex tricks, however, the lack of definition of the filter (or withFilter) method leads to certain inconveniences. Here are examples:

Binding to a tuple

for {
  // opens two files at once and returns a tuple of Input/OutputStreams
  (is, os) <- withIOStream("input.txt", "output.txt") // (doesn't work)
} { ... }

This doesn't work because assigning to a tuple (or more generally, a pattern) involves a call to filter. If you want to use pattern matching, bind it to a variable first, and then match it against a tuple in the for expression.

Guards

for {
  // handle only if the file is not empty
  is <- withInputStream("input.txt") if is.available() > 0 // (doesn't work)
} { ... }

This doesn't work because an "if" guard is translated to a call to filter. Since the guard (usually) depends on the loaned object (in this example, is), such conditionals must be put into the body of the for expression.

The trait is still "safe" because both of the above are catched at compilation time. However, experienced Scala programmers may find it cumbersome when they write up a complex generator and then get errors.

Can we get them work? A naïve approach is to define filter and withFilter like:

// unsafe, incorrect for certain uses
trait ForComprehensible[+A] {
  ...

  // ignore the filtering and just return itself
  def filter(f: A => Boolean): ForComprehensible[A] = this
  // not essential here, just to suppress compiler warnings
  def withFilter(f: A => Boolean) = filter(f)
}

this enables binding to a tuple (for some reason I don't fully understand), but causes guards to be silently ignored (i.e. assumes true). This makes ForComprehensible an unsafe construct.

If we are to implement filter (and withFilter) properly, we have to open a FileInputStream (in our example) in the filter method. It means the stream must be opened twice (in filter and apply), or we open it once in filter and then keep it open using some magic until the callback of apply returns. The former is simply unacceptable, and the latter would make the implementation of the trait too complicated.

EDIT: after some more experiments, I found out that a guard on ForComprehensible is not an implementation problem but rather a semantic problem.

Consider:

for {
  is <- withInputStream("input.txt") if is.available() > 0
} yield { ... }

note the yield. What this whole expression has to return if the guard is.available() > 0 doesn't hold?

It depends on the body of the for-expression. For example, if it loads the lines of the file into a Seq[String], it would return Seq(). If it counts the number of lines in the file, it would return 0 (if the body returns Int) or None (Option[Int]).

Guards, or filters, only make sense when they operate on a collection (Option can be considered a kind of collection whose maximum size is 1). Since ForComprehensible is just a syntactic trick to enable a loaner-function to be in for-generators, I conclude for now that we should stick to for-body if we want to check some conditions for acquiring resources.


Checking if a number is palindrome or not in Java Got this question in a technical interview and here is my solution:

public class Main {

  public static void main(String[] args) {

    Scanner in = new Scanner(System.in);

    if (!in.hasNextInt()) {
      System.out.println("not a number!");
    }

    int number = in.nextInt();

    System.out.println(isPalidrom(number));
  }

  private static String isPalidrom(int number) {

    String numberStr = Integer.toString(number);
    String reverseStr = new StringBuilder(numberStr).reverse().toString();

    if (reverseStr.equals(numberStr)) {
      return "palindrome";
    } else {
      return "not palindrome";
    }
  }
}

I was going to solve it with using modulus 10 and decided to do it by reverting the number to integer, was it a good idea?

Some tips:

Main is a confusing class name. In your case, an appropriate name might possibly be PalindromeChecker.
Your main method would usually be in a separate class called for example Application. That way

PalindromeChecker ends up with a public method for doing the actual check, making the class reusable and
the library and executable parts of your program are properly separated.

Why scan for numbers? You are reading from standard input, which by definition is just a byte stream, so you might as well avoid some conversion and just check whether each line that arrives is a palindrome. Number checking could be done elsewhere, if absolutely necessary.
The checker function should return a boolean, to avoid mixing logic and presentation. The function also becomes objectively simpler: return reverseStr.equals(numberStr);
Look into other ways of converting a string to an integer - they have different pros and cons.
Writing up some test methods could expose unexpected behaviour. For example, what does your code do with alternative numbering systems such as Roman or Chinese numerals? Does it handle 10.1 as one integer (101 or 10), two integers (10 and 1), or does it reject it? Does it expand numbers such as 1e5? Does it precompute arithmetic such as 10+1? Converting the int back to a String and comparing it with the original String input might be a useful way of telling whether any automagic conversion has occurred.
I believe tell, don't ask applies to the way in is used.

That said, your solution is simple and quick. Given the time & stress of an interview I'd say you nailed it. (Provided it works - I haven't actually run it :)


Creating a game with rectangular shapes I'm using Pygame to create a small game with few rectangle shapes.  However, I want to learn more about classes, and I'm trying to use them.  I'm very new to classes and inheritance, but I want to learn the best habits possible. I'm using Python 3.3.2.

import pygame

class Screen(object):
    ''' Creates main window and handles surface object. '''
    def __init__(self, width, height):
        self.WIDTH = width
        self.HEIGHT = height
        self.SURFACE = pygame.display.set_mode((self.WIDTH, self.HEIGHT), 0, 32)

class GameManager(object):
    ''' Handles game state datas and updates. '''
    def __init__(self, screen):
        self.screen = screen

pygame.init()
Window = Screen(800, 500)
Game = GameManager(Window)

I skipped a few functions to make it simpler.  The thing that bothers me is that I've been told that I don't need GameManager to be subclasses of Screen.  I was advised to use this implementation, but I find it very strange to use 'Window' as a parameter to GameManager.

Do I need to use inheritance?  Should I use super()?  Later on, I will have class Player and I will need some of the attributes from the Screen class.  From the viewpoint of designing OO for a GUI game using Pygame.

First off, you don't need to explicitly inherit from object when creating classes. In Python 3.x, you can just type class MyClass:, and leave it as that if you aren't inheriting from any other classes.

Secondly, I think your current design is fine, as long as the Screen class implements more methods, and the GameManager as well.

Finally, just a little nitpicky tip, your variables Window, and Game should be renamed to window and game.
Likewise, in Screen, the attributes WIDTH, HEIGHT, SURFACE should be renamed to width, height, surface, respectively.
Actually more than being nitpicky,
these recommendations come from PEP8, the Python style guide.


Reducing a large array of objects to a simple object with 8 properties I'm reducing a large array of objects to a simple object with 8 properties. I'm currently using Array.prototype.reduce to loop through the array of objects to create a new object. In order to assign a correct key to the value in the array I'm using a switch statement. My code currently works, but I'm looking to refine it. I have no control over the API and how it returns its response.

The array of objects that are returned look like this:

[
  {
    field_id:89107336,
    values : [{ value:"Star Warz"}]
  },
  {
    field_id:89107337,
    values : [{ date:"December 24, 2015"}]
  },
  {
    field_id:89107415,
    values : [
               {
                 value: { item_id:1234123}
               },
               {
                 value: { item_id:4746576}
               }
  },
  {
    field_id:89107340,
    values : [{ value:"5.00"}]
  },
  {
    field_id:89107344,
    values : [{ value:"This is description"}]
  },
  {
    field_id:89107348,
    values : [
               {
                 value:{file_id:234234}
               }
             ]
  }
]

The object values come back random at times, so the only way for me to tell the difference between the title, the date, description, etc. Is for me to run the response through a switch statement based upon field_id. So I created an object like so:

var eventFieldIds = {
        title: 89107336,
        date: 89107337,
        attendees: 89107415,
        price: 89107340,
        desc: 89107344,
        img: 89107348
    };

Then I use a switch statement to reduce the array objects  like this:

var newEvent = arrayObjects.reduce(function(p, c) {
    switch (c.field_id) {
        case eventFieldIds.title:
            {
                p.name = c.values[0].value;
                break;
            }
        case eventFieldIds.date:
            {
                p.date = new Date(c.values[0].start_date);
                break;
            }
        case eventFieldIds.attendees:
            {
                //this is an array of values
                var tempArray = c.values.map(function(v) {
                    return value.value.item_id;
                });
                p.attendees = JSON.stringify(tempArray);
                break;
            }
        case eventFieldIds.price:
            {
                p.price = c.values[0].value;
                break;
            }
        case eventFieldIds.desc:
            {
                p.description = c.values[0].value;
                break;
            }
        case eventFieldIds.img:
            {
                p.img = {
                    file_id: c.values[0].value.file_id
                };
                break;
            }
    }

    return p;
}, {});

How can I refine this? Can I cut out the switch statement? Is there a better way to do this?

I'd flip the field-id-to-name lookup around, and give it some more detail; e.g.

function defaultConverter(values) {
  return values[0].value;
}

var mappings = {
  "89107336": { name: "title", converter: defaultConverter },
  "89107340": { name: "price", converter: defaultConverter },
  "89107344": { name: "desc", converter: defaultConverter },
  "89107337": {
    name: "date",
    converter: function (values) {
      return new Date(values[0].start_date);
    }
  },
  "89107348": {
    name: "img",
    converter: function (values) {
      return values[0].value.file_id;
    }
  },
  "89107415": {
    name: "attendees",
    converter: function (values) {
      var items = values.map(function (v) { return v.value.item_id });
      return JSON.stringify(items);
    }
  }
};

This way you declare how each piece of data should be treated, and you can easily extend it.

Your reduce can then become this:

var output = data.reduce(function (output, obj) {
  var mapper = mappings[obj.field_id];
  if(map) {
    output[map.name] = map.converter(obj.values)
  }
  return output;
}, {});

You can make the mapping declaration a little less repetitive like so:

function Mapping(name, converter) {
  this.name = name;
  this.converter = converter || function (values) { return values[0].value };
}

var mappings = {
  "89107336": new Mapping("title"),
  "89107340": new Mapping("price"),
  "89107344": new Mapping("desc"),
  "89107337": new Mapping("date", function (values) {
    return new Date(values[0].start_date);
  }),
  "89107348": new Mapping("img", function (values) {
    return values[0].value.file_id;
  }),
  "89107415": new Mapping("attendees", function (values) {
    var items = values.map(function (v) { return v.value.item_id });
    return JSON.stringify(items);
  })
};

and still use the same reduce function.


Integral using Simpson's Rule As an answer to this problem:

  Exercise 1.29

  Simpson's Rule is a
  more accurate method of numerical
  integration than the method
  illustrated above. Using Simpson's
  Rule, the integral of a function f
  between a and b is approximated as
   (h / 3) * (y_0 + 4y_1 + 2y_2 + 4y_3 + 2y_4 + ... + 2y_(n-2) + 4y_(n-1) + y_n

  where h = (b - a)/n, for some even
  integer n, and yk = f(a + kh).
  (Increasing n increases the accuracy
  of the approximation.) Define a
  procedure that takes as arguments f,
  a, b, and n and returns the value of
  the integral, computed using Simpson's
  Rule. Use your procedure to integrate
  cube between 0 and 1 (with n = 100 and
  n = 1000), and compare the results to
  those of the integral procedure shown
  above.

I wrote the following solution:

(define (sum term a next b)
  (define (iter a result)
    (if (> a b)
        result
        (iter (next a) (+ (term a) result)))
    ) (iter a 0))

(define (simpsons-rule f a b n)
  (let ((h (/ (- b a) n)))
    (define (y_k k) (f (+ a (* k h))))
    (define (even n) (= (remainder n 2) 0))
    (define (term n) (* (if (even n) 2.0 4.0) (y_k n)))
    (define (next n) (+ n 1))
    (* (/ h 3.0) (+ (y_k 0.0) (sum term 0.0 next (- n 1.0)) (y_k n)))))

(define (cube x) (* x x x))

What do you think?

When you call sum, make sure you start with 1 (and not 0).  Otherwise, you get an error of + 2 y_0 h / 3.  In case of (simpsons-rule cube 1 2 1000), this error is 0.000666....

Another way to rewrite the series is to group even and odd terms together, excluding the first and last terms.

(h / 3) * (y_0 + y_n + 4 * (y_1 + y_3 + ... + y_n-1) + 2 * (y_2 + y_4 + ... + y_n-2))

This gives us another possible implementation:

(define (simpsons-rule f a b n)
  (define h (/ (- b a) n))
  (define (y-k k) (f (+ a (* k h))))
  (define (next n) (+ n 2))
  (* (/ h 3.0) (+ (y-k 0) (y-k n) (* 4 (sum y-k 1 next (- n 1))) (* 2 (sum y-k 2 next (- n 2))))))