Implement a generic Fibonacci sequence in Rust without using Copy trait I'm trying to learn Rust and am a beginner. How does one go about implementing a generic version of the Fibonacci sequence without using Copy trait in Rust? My code is given below.

I had to use Copy trait, otherwise the compiler would complain

  cannot move out of borrowed content [E0507]

in the method next() (everywhere I have borrowed from self). I understand the error, but I'm not sure how this could be done without copying. Also, other comments and observations are welcome!

lib.rs

pub mod fib_trait;

#[cfg(test)]
mod tests {
    #[test]
    fn it_works() {
    }
}

fib_trait.rs

extern crate num;

pub mod fib_trait {

    use fib_trait::num::traits::*;
    use std::ops::Add;

    #[derive(Clone, Copy)]
    pub struct Fibonacci<T> where
    T : Zero + One + Add + Copy {
        curr : T,
        next : T,
    }

    impl<T> Fibonacci<T> where
    T : Zero + One + Add  + Copy{
        pub fn new() -> Fibonacci<T> {
            Fibonacci {
                curr : T::zero(),
                next : T::one(),
            }
        }
    }

    impl<T> Iterator for Fibonacci<T> where
    T : Zero + One + Add + Copy {
        type Item = T;

        fn next(&mut self) -> Option<T> {
            let c : T = self.next;
            self.next = self.next + self.curr;
            self.curr = c;
            let n : T = self.next;
            Some(n)
        }
    }

    #[test]
    pub fn fib_test() {
        let mut f : Fibonacci<u32> = Fibonacci::new();
        assert_eq!(f.next(), Some(1));

        let mut next_val = 1;
        let mut curr_val = 0;
        for i in Fibonacci::<u32>::new().take(4) {
            let c = next_val;
            next_val = curr_val + next_val;
            curr_val = c;
            assert_eq!(i, next_val);
        }
    }
}

You can use Clone instead of Copy. This requires adding .clone() calls where the compiler did a copy when the Copy bound was present.

extern crate num;

pub mod fib_trait {
    use num::traits::*;
    use std::ops::Add;

    #[derive(Clone, Copy)]
    pub struct Fibonacci<T>
        where T: Zero + One + Add + Clone
    {
        curr: T,
        next: T,
    }

    impl<T> Fibonacci<T>
        where T: Zero + One + Add + Clone
    {
        pub fn new() -> Fibonacci<T> {
            Fibonacci {
                curr: T::zero(),
                next: T::one(),
            }
        }
    }

    impl<T> Iterator for Fibonacci<T>
        where T: Zero + One + Add + Clone
    {
        type Item = T;

        fn next(&mut self) -> Option<T> {
            let c: T = self.next.clone();
            self.next = self.next.clone() + self.curr.clone();
            self.curr = c;
            let n: T = self.next.clone();
            Some(n)
        }
    }

    #[test]
    pub fn fib_test() {
        let mut f: Fibonacci<u32> = Fibonacci::new();
        assert_eq!(f.next(), Some(1));

        let mut next_val = 1;
        let mut curr_val = 0;
        for i in Fibonacci::<u32>::new().take(4) {
            let c = next_val;
            next_val = curr_val + next_val;
            curr_val = c;
            assert_eq!(i, next_val);
        }
    }
}

We can eliminate the clones on the addition by requiring and using addition on references instead of direct values. for<'a> introduces a higher-rank trait bound – we need this because we want &'a T to implement Add<Output=T>, whatever the lifetime 'a is.

extern crate num;

pub mod fib_trait {
    use num::traits::*;
    use std::ops::Add;

    #[derive(Clone, Copy)]
    pub struct Fibonacci<T>
        where T: Zero + One + Clone,
              for<'a> &'a T: Add<Output=T>
    {
        curr: T,
        next: T,
    }

    impl<T> Fibonacci<T>
        where T: Zero + One + Clone,
              for<'a> &'a T: Add<Output=T>
    {
        pub fn new() -> Fibonacci<T> {
            Fibonacci {
                curr: T::zero(),
                next: T::one(),
            }
        }
    }

    impl<T> Iterator for Fibonacci<T>
        where T: Zero + One + Clone,
              for<'a> &'a T: Add<Output=T>
    {
        type Item = T;

        fn next(&mut self) -> Option<T> {
            let c: T = self.next.clone();
            self.next = &self.next + &self.curr;
            self.curr = c;
            let n: T = self.next.clone();
            Some(n)
        }
    }

    #[test]
    pub fn fib_test() {
        let mut f: Fibonacci<u32> = Fibonacci::new();
        assert_eq!(f.next(), Some(1));

        let mut next_val = 1;
        let mut curr_val = 0;
        for i in Fibonacci::<u32>::new().take(4) {
            let c = next_val;
            next_val = curr_val + next_val;
            curr_val = c;
            assert_eq!(i, next_val);
        }
    }
}

Also, note the stylistic changes I made above:

line break before where
indent where clause
no space before :


ASP.Net caching manager I have a Cache Helper Class.

using System;
using System.Web;

public static class CacheHelper
{
    /// <summary>
    /// Insert value into the cache using
    /// appropriate name/value pairs
    /// </summary>
    /// <typeparam name="T">Type of cached item</typeparam>
    /// <param name="o">Item to be cached</param>
    /// <param name="key">Name of item</param>
    public static void Add<T>(T o, string key)
    {
        // NOTE: Apply expiration parameters as you see fit.
        // I typically pull from configuration file.

        // In this example, I want an absolute
        // timeout so changes will always be reflected
        // at that time. Hence, the NoSlidingExpiration.
        HttpContext.Current.Cache.Insert(
            key,
            o,
            null,
            DateTime.Now.AddMinutes(1440),
            System.Web.Caching.Cache.NoSlidingExpiration);
    }

    /// <summary>
    /// Remove item from cache
    /// </summary>
    /// <param name="key">Name of cached item</param>
    public static void Clear(string key)
    {
        HttpContext.Current.Cache.Remove(key);
    }

    /// <summary>
    /// Check for item in cache
    /// </summary>
    /// <param name="key">Name of cached item</param>
    /// <returns></returns>
    public static bool Exists(string key)
    {
        return HttpContext.Current.Cache[key] != null;
    }

    /// <summary>
    /// Retrieve cached item
    /// </summary>
    /// <typeparam name="T">Type of cached item</typeparam>
    /// <param name="key">Name of cached item</param>
    /// <param name="value">Cached value. Default(T) if
    /// item doesn't exist.</param>
    /// <returns>Cached item as type</returns>
    public static bool Get<T>(string key, out T value)
    {
        try
        {
            if (!Exists(key))
            {
                value = default(T);
                return false;
            }

            value =  (T) HttpContext.Current.Cache[key];
        }
        catch
        {
            value = default(T);
            return false;
        }

        return true;
    }
}

and usage:

string key = "EmployeeList";
List<Employee> employees;

if (!CacheHelper.Get(key, out employees))
{
    employees = DataAccess.GetEmployeeList();
    CacheHelper.Add(employees, key);
    Message.Text =
        "Employees not found but retrieved and added to cache for next lookup.";
}
else
{
    Message.Text = "Employees pulled from cache.";
}

Do you see any improvement / issue?

/// <summary>
/// Insert value into the cache using
/// appropriate name/value pairs
/// </summary>
/// <typeparam name="T">Type of cached item</typeparam>
/// <param name="o">Item to be cached</param>
/// <param name="key">Name of item</param>
public static void Add<T>(T o, string key)

I think this signature would be more useful like this:

/// <summary>
/// Insert value into the cache using
/// appropriate name/value pairs.
/// </summary>
/// <typeparam name="T">Type of cached item (inferred from usage).</typeparam>
/// <param name="key">A string used to uniquely identify the added value.</param>
/// <param name="value">Item/value to be cached.</param>
public static void Add<T>(string key, T value)

This is misleading:

/// <summary>
/// Remove item from cache
/// </summary>
/// <param name="key">Name of cached item</param>
public static void Clear(string key)
{
    HttpContext.Current.Cache.Remove(key);
}

Clear is usually a parameterless method that clears a container's contents. This method should be named Remove.

/// <summary>
/// Check for item in cache
/// </summary>
/// <param name="key">Name of cached item</param>
/// <returns></returns>
public static bool Exists(string key)
{
    return HttpContext.Current.Cache[key] != null;
}

"Check for item in cache" doesn't really say what's going on. I realize it's trivially inferred from the method's name, but "Checks for existence of specified key in cache." would be totally unambiguous.

public static bool Get<T>(string key, out T value)

I don't like the implementation of that method very much, there are too many exit points, some being redundant. Consider something like this:

public static bool TryGet<T>(string key, out T value)
{
    bool result;

    try
    {
        if (Exists(key))
        {
            value =  (T)HttpContext.Current.Cache[key];
            result = true;
        }
    }
    catch(InvalidCastException)
    {
        value = default(T);
    }

    return result;
}


Convert Roman Numeral to Arabic I've been learning Haskell recently and I decided I needed to work on a (somewhat) realistic problem.

What I'm most interested in getting feedback on is how well I've used the tools in Haskell to accomplish the goal of converting Roman numerals to Arabic values. I suspect that my approach is more verbose than it needs to be, so thoughts on how I could remove unneeded code would be quite welcome.

Lastly, thoughts on how to address Haskell bad practices or similar errors would be appreciated.

-- file: roman.hs
import Data.Char

-- Application startup.
main :: IO ()
main = do
    putStrLn "Enter a Roman numeral. The numerical value will be returned."
    putStrLn "For example MMI -> 2001."
    putStrLn "Enter 'finis' to exit."
    process
    putStrLn "vale! :-)"

-- Main loop.
process :: IO ()
process = do
    putStrLn "Numeral: "
    line <- getLine
    if line == "finis"
        then return ()
        else do
            let nums = readNumerals(line)
                in if errorNum `elem` nums
                    then putStrLn (line ++ "is not a valid Roman numeral.")
                    else putStrLn ("   The Roman numeral " ++ line ++ " is equal to " ++ show(getValue(nums)))
            process

data Numeral = Numeral {
    numeralValue :: Int,
    numeralSymbol :: Char,
    subRule :: [Char]}
    deriving (Show, Eq)

-- This is the Numeral error type. used to check if the input was valid.
errorNum = Numeral 0 'E' []

-- Turn a string into a list of numerals.
readNumerals :: String -> [Numeral]
readNumerals (x:xs) = readNumeral (toUpper(x)) : [] ++ readNumerals xs -- First cons the Char to an empty list, then recur.
readNumerals "" = []

-- Parse each char into the corresponding numeral.
readNumeral :: Char -> Numeral
readNumeral 'I' = Numeral 1 'I' ['V', 'X']
readNumeral 'V' = Numeral 5 'V' []
readNumeral 'X' = Numeral 10 'X' ['L', 'C']
readNumeral 'L' = Numeral 50 'L' []
readNumeral 'C' = Numeral 100 'C' ['D', 'M']
readNumeral 'D' = Numeral 500 'D' []
readNumeral 'M' = Numeral 1000 'M' []
readNumeral _ = errorNum

-- Turn a list of Roman numerals into their corresponding Arabic number.
getValue :: [Numeral] -> Int
getValue [] = 0
getValue (x:xs) =
                if subRuler (subRule x) xs
                then negate (numeralValue x) + getValue xs
                else numeralValue x + getValue xs
                -- if x has a non-empty subRule and the next x is in the list, then flip the sign for x

-- Take the subRule from x and xs, if x of xs is in subRule, return true, otherwise false.
-- What this really means is see if the next element in the list of Numerals is one of the elements in subRule.
subRuler :: [Char] -> [Numeral] -> Bool
subRuler [] _ = False
subRuler _ [] = False
subRuler a b = numeralSymbol (head b) `elem` a

{-
Subtractive Rules, from Wikipedia
I placed before V or X indicates one less, so four is IV (one less than five) and nine is IX (one less than ten)
X placed before L or C indicates ten less, so forty is XL (ten less than fifty) and ninety is XC (ten less than a hundred)
C placed before D or M indicates a hundred less, so four hundred is CD (a hundred less than five hundred) and nine hundred is CM (a hundred less than a thousand)
-}

Parens

When you call a function you do not need to place the argument in parens:

Instead of:  readNumerals(line)
       Use:  readNumerals line

Error Handling

In Haskell we use types like Either or Maybe to indicate errors. Instead of:

let nums = readNumerals(line)
if errorNum `elem` mums
  then ...some error...
  else ...

You should define readNumerals to have this type:

readNumerals :: String -> Maybe [Numeral]

and write:

case readNumerals line of
  Nothing -> ... some error ...
  Just ns -> ... parse was valid, numerals are in `ns` ...

data Numeral

The data structure Numeral has many fields which are redundant. For instance, if n is a Numeral and numeralValue n is 1, then it will always be the case that numeralSymbol n will be I and subRules n will be ['V','X']. So there's no point storing these in the record - you can just implement these as functions:

subRules :: Numeral -> [Char]
subRules n = case numeralValue n of
                1  -> ['V','X']
                5  -> []
                10 -> ['L', 'C']
                ...

This allows you to eliminate the subRules field from the record.

Make Numeral an ADT

In fact, Numeral doesn't even need any fields. Here's a simpler way to implement Numeral:

data Numeral = I | V | X | L | C deriving (Show, Eq)

numeralValue :: Numeral -> Int
numeralValue I = 1
numeralValue V = 5
...

subRule :: Numeral -> [Numeral]
subRule I = [V, X]
subRule V = []
...

The nice thing is that the code for getValue doesn't change at all. The function subRuler does change a little (and also becomes simpler):

subRuler :: [Numeral] -> [Numeral] -> Bool
subRuler [] _ = False
subRuler _ [] = False
subRules as bs = (head bs) `elem` as

Avoid using head

head is a partial function - i.e. it could throw an error. Here's how to write subRuler without using head:

subRules [] _ = False
subRuler _ [] = False
subRuler as (b:_) = b `elem` as

We've replaced head with pattern matching, and it does the same thing. tail is also a partial function which you should replace with pattern matching.


Memory streambuf and stream There are a plethora of streams and streambuffers available today, none are as fast as a memory streambuf, that you can use for testing. Shoot away!

#ifndef MEMSTREAMBUF_HPP
# define MEMSTREAMBUF_HPP
# pragma once

#include <cassert>

#include <cstring>

#include <array>

#include <iostream>

#include <streambuf>

template <::std::size_t N>
class memstreambuf : public ::std::streambuf
{
  ::std::array<char, N> buf_;

public:
  memstreambuf()
  {
    setbuf(buf_.data(), buf_.size());
  }

  ::std::streambuf* setbuf(char_type* const s,
    ::std::streamsize const n) final
  {
    auto const begin(s);
    auto const end(s + n);

    setg(begin, begin, end);
    setp(begin, end);

    return this;
  }

  pos_type seekpos(pos_type const pos,
    ::std::ios_base::openmode const which = ::std::ios_base::in |
    ::std::ios_base::out) final
  {
    switch (which)
    {
      case ::std::ios_base::in:
        if (pos < egptr() - eback())
        {
          setg(eback(), eback() + pos, egptr());

          return pos;
        }
        else
        {
          break;
        }

      case ::std::ios_base::out:
        if (pos < epptr() - pbase())
        {
          setp(pbase(), epptr());
          pbump(pos);

          return pos;
        }
        else
        {
          break;
        }

      default:
        assert(0);
    }

    return pos_type(off_type(-1));
  }

  ::std::streamsize xsgetn(char_type* const s,
    ::std::streamsize const count) final
  {
    auto const size(::std::min(egptr() - gptr(), count));

    ::std::memcpy(s, gptr(), size);

    gbump(size);

    return egptr() == gptr() ? traits_type::eof() : size;
  }

  ::std::streamsize xsputn(char_type const* s,
    ::std::streamsize const count) final
  {
    auto const size(::std::min(epptr() - pptr(), count));

    ::std::memcpy(pptr(), s, size);

    pbump(size);

    return epptr() == pptr() ? traits_type::eof() : size;
  }
};

template <::std::size_t N = 1024>
class memstream : public memstreambuf<N>,
  public ::std::istream,
  public ::std::ostream
{
public:
  memstream() : ::std::istream(this),
    ::std::ostream(this)
  {
  }
};

#endif // MEMSTREAMBUF_HPP

Assert

I personally dislike using asserts. The problem for me is that they do different things in production and debug code. I want the same action in both.

  pos_type seekpos(pos_type const pos, ::std::ios_base::openmode const which) final
  {
    switch (which)
    {
      case ::std::ios_base::in:
      case ::std::ios_base::out:
      default:
        assert(0);
    }

    return pos_type(off_type(-1));
  }

So in this code if we get to the default action; the debug version will assert (and stop the application) while production code will return -1.

In my opinion if you get to a point where your code should not then throw an exception that should not be caught. That will cause the application to exit (in a controlled way).

But in this case I think the expected behavior is not to throw but to return -1. So I would make the default action a no-op.

seekpos which

The which in your seekpos() has basically three settings. You only check for two of the three.

    switch (which)
    {
      case ::std::ios_base::in:
      case ::std::ios_base::out:

      // You forgot this case
      case ::std::ios_base::in | ::std::ios_base::out:

      // it means set the position of the input and output stream.
      // since this is the default value of `which` I would expect
      // this to happen most often (and would assert in your code).

      default:
        assert(0);
    }

    return pos_type(off_type(-1));
  }

xsgetn/xsputn eof

I think you return result of these functions can be incorrect. You should only return eof if you did not get/put any values. You return eof if you have filled/emptied all the data.

  ::std::streamsize xsgetn(char_type* const s,
    ::std::streamsize const count) final
  {
    // If there is no data to get then return eof
    if (egptr() == gptr()) {
        traits_type::eof();
    }

    auto const size(::std::min(egptr() - gptr(), count));

    ::std::memcpy(s, gptr(), size);

    gbump(size);

    return size; // return the number of bytes read.
  }

Overall Design

The input and output use the same buffer but are not linked together. If you have not written anything into the buffer I would not expect you to be able to read from the buffer (as there is nothing to read).

When you do write I would only expect you to be able to read only what has written (no more).

int main()
{
    memstream<1024>   buffer;

    if (buffer.write("abcdef", 6))
    {
         std::cout << "Write OK\n";
    }

    char data[100];
    if (buffer.read(data, 10))
    {
        auto c = buffer.gcount();

        std::cout << "Count: " << c << "\n";
        std::cout << std::string(data, data+10) << "\n";
    }
}

Result:

Write OK
Count: 10
abcdef

So I wrote 6 characters. But I managed to read 10 characters. What were the last 4 characters?

Circular Buffer

Once you have linked the two buffers correctly. You could get more from the buffer by making it circular. That means as you get to the end of the write buffer you can circle around and start writing at the beginning again if you have been reading from the buffer and there is space.

Inheriting from the buffer

template <::std::size_t N = 1024>
class memstream :
  public memstreambuf<N>,    // This is unusual
  public ::std::istream,
  public ::std::ostream
{
public:
  memstream() : ::std::istream(this),
    ::std::ostream(this)
  {
  }
};

Normally I have seen this as a member object rather than inheriting from the buffer. Though this technique saves a call to setbuffer. I can live with it.

BUT: if you are going to inherit from it then I would use private inheritance.

template <::std::size_t N = 1024>
class memstream :
  private memstreambuf<N>,    // Note the private
  public ::std::istream,
  public ::std::ostream
{
public:
  memstream() : ::std::istream(this),
    ::std::ostream(this)
  {
  }
};

This is because I don't want my object to behave like a stream and a stream buffer to people who use it. That could be confusing. So hide the buffer properties from external users (by using private). They can always get a reference to the buffer using the method rdbuf().

pragma once and include guards

There is no point in using both:

#ifndef MEMSTREAMBUF_HPP
# define MEMSTREAMBUF_HPP
# pragma once

....

#endif

I would use the header guards because not all compiler support #pragma once.

Note the space between # and the word is non standard. Most compilers may be forgiving about it but not all so I would not do it.

# define MEMSTREAMBUF_HPP
 ^  // extra space.


Longest substring in alphabetical order The code is meant to find the longest substring that is in alphabetical order.
I would like to know if I can do it more efficiently than I have done here. I am in my first week of my first introductory course, so I'm not looking for more advanced solutions. I just want to know if I'm not making it more complicated than necessary based on my current knowledge. I could't figure out how to use indexing for example.

s = 'azcbobobegghakl'

c = ""
d = ""
e = ""

for i in s:
    if c <= i:
        c = i
        d += c
        if len(d) > len(e):
            e = d
    else:
        c = i
        d = c
print('Longest substring in alphabetical order is: ' + e)

You are assigning c = i in both cases, which means you could pull this out and do this once unconditionally. This also will give you an idea of cs purpose and might give you ideas for how to name your variables in a better way. So trying to stay as close to your code as possible but with the suggested changes, your code could look something along the lines:

inputString = 'azcbobobegghakl'

prevChar = ""
currentString = ""
longestString = ""

for char in inputString:
    if prevChar <= char:
        currentString += char
        if len(currentString) > len(longestString):
            longestString = currentString
    else:
        currentString = char
    prevChar = char
print('Longest substring in alphabetical order is: ' + longestString )

Also I suggest you think about edgecases and treat these cases or at very least you make it clear (at least in a comment) what happens in those cases. As an idea of what I mean: what happens if there are several longest-strings of the same length? Which one should be printed? All? The first? The last? Just any one? What happens if the inputString is empty - your message will look a bit funny. You could output a different message in such a case.

You might think I‘m a bit pedantic, but I seriously recommend to think about your code in this way.

Other than that, I must admit I don‘t know python well enough to give you „the pythonic“ answer - I‘m not even sure if they use camelCase for python - still I hope you get the idea of the type of improvements I‘m recommending. Other than that your code looks ok to me.


Multiple similar event handlers for input validation I want to reduce my repetitive code used to validate inputs values. For that I'm using:

a class associated to the input
a condition to validate the input value
an event handler

First I declare the classes I'm using:

var inputNotNull = '.js-mandatory-notnull',
    input2Words = '.js-mandatory-2words',
    inputNumb = '.js-mandatory-numb';

Then I create an event handler for each class and each of them has the same pattern: Call the function validateInput(event, thisInput, itsCondition)

$(document).on('keyup change focusout', inputNotNull, function(e){
    var $xthis = $(this),
        condition = $xthis.val().length>0;
    validateInput(e, $xthis, condition); //treats the visual part of the input
});

$(document).on('keyup change focusout', input2Words, function(e){
    var $xthis = $(this),
        condition = $xthis.val().match(/^[A-Za-zÀ-ú]+ [A-Za-zÀ-ú]+/i);
    validateInput(e, $xthis, condition);
});

$(document).on('keyup change focusout', inputNumb, function(e){
    var $xthis = $(this),
        condition = this.value.match(/^(?=.*\d)\d*[\.\,]?\d*$/);
    validateInput(e, $xthis, condition);
});

I was wondering how I could reduce this code. In this example I only show 3 classes, but on my code I have >20 classes. Is it possible to do something similar to "key:value" ("input:condition") and I would end up writing something like this:

var validations = {
    '.js-mandatory-notnull' : length > 0,
    '.js-mandatory-2words' : match(/^[A-Za-zÀ-ú]+ [A-Za-zÀ-ú]+/i),
    '.js-mandatory-numb' : match(/^(?=.*\d)\d*[\.\,]?\d*$/),
}

And then somehow treat this object to reproduce what I wrote before.

In this example I'm talking about validations, but I would like to apply this knowledge into another repetitive code scenarios.

You can use functions for those validations:

var validations = {
    '.js-mandatory-notnull': function(val) {
        return val.length > 0;
    },
    '.js-mandatory-2words': function(val) {
        return val.match(/^[A-Za-zÀ-ú]+ [A-Za-zÀ-ú]+/i);
    },
    '.js-mandatory-numb': function(val) {
        return val.match(/^(?=.*\d)\d*[\.\,]?\d*$/);
    }
}

And then install those via a loop:

$.each(validations, function(selector, validate) {
    $(document).on('keyup change focusout', selector, function(e){
        validateInput(e, $(this), validate(this.value));
    });
});


Compute the number of molecules I would like my program to be reviewed. I had to write a simple program that calculates the number of molecules in a hydrocarbon.

1 carbon atom has 12 AMU.
1 hydrogen atom has 1 AMU.

#include <iostream>
using namespace std;

int main()
{
    cout << "Enter mass of hydrocarbon (in grams)\n"
            "followed by the number of carbon atoms\n"
            "followed by the number of hydrogen atoms\n"
            "(e.g. 10.5 2 6): " << flush;

    float mass;
    float carbonAtoms;
    int hydrogenAtoms;
    cin >> mass >> carbonAtoms >> hydrogenAtoms;

    long formulaWght = (carbonAtoms * 12) + (hydrogenAtoms * 1);

    double molecules = (mass / formulaWght) * 6.02e23;

    cout << mass << " grams of hydrocarbon\nwith "
            << carbonAtoms << " carbon atom(s) and "
               << hydrogenAtoms << " hydrogen atom(s)\ncontains "
                  << molecules << " molecules" << endl;

    return 0;
}

Do not use using namespace std. Read
this
Instead of using "magic numbers" in your program. Use symbolic
constants. CARBON_AMU and HYDROGEN_AMU should be ints.

const int CARBON_AMU= 12;
const int HYDROGEN_AMU = 1;
const double AVOGADRO = 6.02214141070409084099072e23;

Utilize functions in your program, instead of initializing a
variable with an expression:

long formulaWeight(int carbonAtoms, int hydrogenAtoms){
    return((carbonAtoms * CARBON_AMU) + (hydrogenAtoms * HYDROGEN_AMU));
}

double numMolecules(float mass, long formulaWeight)
{
    return((mass / formulaWeight) * AVOGADRO);
}

(e.g. 10.5 2 6): " << flush;

I don't see any advantage in using std::flush. You can remove it.
Fix your indentation. This is much easier to read:

cout << mass << " grams of hydrocarbon\nwith "
     << carbonAtoms << " carbon atom(s) and "
     << hydrogenAtoms << " hydrogen atom(s)\ncontains "
     << molecules << " molecules" << endl;


PowerShell Unpivot Object Based on a question on SO I thought I'd knock up a cmdlet to perform an unpivot on an object.  I've not yet considered all the options (e.g. what happens when objects with empty lists are passed), so there are likely a few issues, but I wanted to get it out to the community early / see if people think this is useful or if better alternatives already exist in PS natively.

cls

function UnPivot-Object
{
    param
    (
        [Parameter(
            Mandatory=$true,
            ValueFromPipeline=$true,
            ValueFromPipelineByPropertyName=$true
        )][psobject]$InputObject
        ,[Parameter(
            ValueFromPipelineByPropertyName=$true
        )][string[]]$Properties
        ,[Parameter(
            ValueFromPipelineByPropertyName=$true
        )][string]$ExpandProperty
    )
    process
    {
        $InputObject | %{
            $x = $_ ;
            $_ | select -ExpandProperty $ExpandProperty | %{
                $y = $_ ;
                $x | select $Properties | select *, @{Name="$ExpandProperty";Expression={$y}}
            }
        }
    }
}

$x = @(
     (New-Object –TypeName PSObject –Prop @{Name='one';   Var='this'; Var2='How';   List=@('a','b','c');})
    ,(New-Object –TypeName PSObject –Prop @{Name='two';   Var='test'; Var2='now';   List=@('d','e','f');})
    ,(New-Object –TypeName PSObject –Prop @{Name='three'; Var='is'  ; Var2='brown'; List=@('g','h','i');})
    ,(New-Object –TypeName PSObject –Prop @{Name='four';  Var='a'   ; Var2='cow';   List=@('j','k','l');})
    ,(New-Object –TypeName PSObject –Prop @{Name='five';  Var='test'; Var2=$null;   List=@('m','n','o');})
)

$x | UnPivot-Object -Properties Name, Var -ExpandProperty List

Looks great! I'm going to rehash the things that @Mike Shepard said because he's absolutely right but he wrote them in a comment, so I'll expand on them a bit first, and add a few more.

Approved Verbs

PowerShell has a concept of approved verbs. You can see them by typing Get-Verb at the prompt, but you should really look at the MSDN page on approved verbs for the rules and help with selecting appropriate verbs.

For your function I like the Split or Expand verbs, like Split-Object or Expand-Property.

Convert might also be appropriate based on the description, but it doesn't quite sound right to me.

Comment Based Help

Not too much to say about this that isn't covered in about_Comment_Based_Help, but yeah it's definitely good to have. One of the things that I think is overlooked is the .LINK keyword.

If you decide to publish this somewhere, you can use this to link to online documentation or examples or support.

Get-Help MyFunction -Online will automatically open a browser window to the first .LINK URL (you can have more than one listed).

Avoid Aliases

Not everyone agrees with this, but I do.

I love and use aliases all the time at the prompt, but when writing scripts, I use the full names of functions, cmdlets, parameters, etc. I truly think it makes things clearer and more readable.

In your script:

% should be ForEach-Object
select should be Select-Object

Tab completion makes it easier since you don't have to type all of the longer names.

CmdletBinding Attribute

This is minor, but you may want to use [CmdletBinding()] for your function. Obviously, it's not necessary since your function works, but it's worth reading about_Functions_CmdletBindingAttribute to get an idea of what it does, what it can do with its options, and how it might change the way your function works.

Parameter Naming

Your -Properties parameter should be named -Property. It is convention in PowerShell to use singular names, even for things that may accept or return multiple objects.

Parameter Binding

The -Properties and -ExpandProperty parameters are both optional, but calling the function with one or both of them missing gives less than stellar results.

So the function should be updated either to gracefully handle situations where they are missing, or they should be made mandatory (like -InputObject).

Select Redundancy

$x | select $Properties | select *, @{Name="$ExpandProperty";Expression={$y}}

You could avoid doing 2 selects there by combining $Properties and the hashtable used for $y:

$x | select ($Properties + @{Name="$ExpandProperty";Expression={$y}})

Or you could pre-append them:

$props = $Properties + @{Name="$ExpandProperty";Expression={$y}}
$x | select $props

Semicolons

You use a semicolon after your simple assigns for some reason:

$x = $_ ;
$y = $_ ;

They do no harm, but typically they are used to separate multiple statements on a single line. If you really wanted to use them in concert with line breaks, at least be consistent and do it everywhere. But, yeah actually just don't use them.

Inconsistent Coding Style

This is only concerning the placement of braces. In the process block you use C# style where the opening brace is on its own line, and in the rest you seem to use the style more common in PowerShell where the opening brace is on the same line as the preceding statement.

In some sense, this is a matter of opinion, but sometimes it makes a difference in the way the code executes, and for that reason, I always use the latter format (brace does not get its own line).

Either way you decide, be consistent.

Closing

All in all, great job!


Searching item in array I have a program that calculates some values and saves them to an array. Before save a value, program checks if there is already an identical value in array:

 string someData = GetData();
 bool newItem = true;
 int arrayDataLength = arrayData.Length;
 for (int x = 0; x < arrayDataLength; x++)
 {
     if (arrayData[x] == someData)
     {
         newItem = false;
         arrayDataCount[x]++;
         break;
     }
  }
  if (newItem == true)
  {
       Array.Resize(ref arrayData, (arrayDataLength + 1));
       Array.Resize(ref arrayDataCount, (arrayDataLength + 1));
       arrayData[arrayDataLength] = someData;
       arrayDataCount[arrayDataLength]++;
  }

Sometimes arrays with data become big, and program works a bit slow.
How can I optimize perfomance of this action?

If you don't need to count the number of occurences, use a Set:

private readonly ISet<string> data = new HashSet<string>();

public void AddNewData()
{
    string someData = GetData();
    data.Add(someData);
}

If you need to count the occurences, you should use a Dictionary:

private readonly IDictionary<string, int> data = new Dictionary<string, int>();

public void AddNewData()
{
    string someData = GetData();
    if (data.ContainsKey(someData))
    {
        data[someData]++;
    }
    else
    {
        data[someData] = 1;
    }
}

Alternatively, you could just use a List and calculate the counts when you need them, using the LINQ GroupBy extension method:

private readonly ICollection<string> data = new List<string>();

public void AddNewData()
{
    string someData = GetData();
    data.Add(someData); // data might contain duplicates
}

public void PrintData() // use LINQ to count number of occurences
{
    foreach (var entry in data.GroupBy(x => x))
    {
        Console.WriteLine("{0} {1}", entry.Key, entry.Count());
    }
}


Stateful function pointer for passing C++ capturing lambdas / std::functions to C style callbacks #include <functional>

template<typename return_type, typename... argument_types>
struct stateful_function_pointer
{
  using function_type         = std::function<return_type(argument_types...)>;
  using function_pointer_type = return_type (*) (argument_types..., void*);

  function_pointer_type callback ()
  {
    return [ ] (argument_types... arguments, void* user_data)
    {
      return (*static_cast<function_type*>(user_data))(arguments...);
    };
  }
  void*                 user_data()
  {
    return static_cast<void*>(&function);
  }

  function_type function;
};

The C style callback must be providing a void* user data as its final parameter (which is a de-facto standard). It may be possible to pass additional user data other than the function state, but I do not see the point as the lambda can already capture user state.
An eccentricity: It is instantiated as e.g. stateful_function_pointer<void, int, char*> instead of stateful_function_pointer<void(int, char*)>.
Example usage as requested (wrapping a HDF5 C function in C++):
class link_access_property_list : public property_list
{
public:
  using function_pointer_type = stateful_function_pointer<herr_t, const char*, const char*, const char*, const char*, unsigned*, hid_t>;
  using callback_type = function_pointer_type::function_type;

  bool set_external_link_callback(const callback_type& callback)
  {
    function_pointer_.function = callback;
    return H5Pset_elink_cb(id_, function_pointer_.callback(), function_pointer_.user_data()) >= 0;
  }

protected:
  function_pointer_type function_pointer_;
};

Then I could:
lapl.set_external_link_callback(
  [CAPTURE_WHATEVER_YOU_WANT]
  (const char*, const char*, const char*, const char*, unsigned*, hid_t)
  {
    USE_WHATEVER_YOU_WANT
    return herr_t(0);
  });

When you have a C callback you usually pass two things

A pointer to a C function
A void* pointer that is passed to your C function.

The C function must know the type of the original object so it can cast the pointer back to its original type. only after it has been cast back can it be used. And you must cast it back to the exact type it was before you converted to void* or you are in undefined behavior.
So this means the C function must understand the original type.
Since your original type is a templated type (i.e. you don't know the type in general) you can't cast it back (unless you templitize the function so that the function you pass to C has a understanding of the templitized types).
The easy way to do this is to implement a virtual interface. Then you can get a pointer to the base class convert this to a void* then in the C function you can convert back to the base class pointer and call the appropriate virtual interface on this base pointer. Virtual dispatch will then take care of calling the correct implementation.

OK found the reference you were eluding to:
Next time just add it to the question.
From: https://support.hdfgroup.org/HDF5/doc1.8/RM/RM_H5P.html#Property-SetELinkCb
herr_t elink_callback(const char* parent_file_name,
                      const char* parent_group_name,
                      const char* child_file_name,
                      const char* child_object_name,
                      unsigned*   acc_flags,
                      hid_t       fapl_id,
                      void*       op_data)
{
    puts(child_file_name);
    return 0;
}

int main(void) {
    hid_t lapl_id = H5Pcreate(H5P_LINK_ACCESS);
    H5Pset_elink_cb(lapl_id, elink_callback, NULL);
    ...
}

Notice at the top of the document it talks about C functions: https://support.hdfgroup.org/HDF5/doc1.8/RM/RM_H5P.html
This library is robably built by a C compiler so the whole ABI is a C ABI. Thus you can not pass any functions of C++ linkage as a parameter to a callback (The C code does not know how to correctly set up the stack frame to make a correct C++ call). You can only pass functions with C linkage.
Now some compilers use a very similar ABI for C and C++ and you may get lucky passing a function that uses C++ linkage, but that just means you are getting lucky.
Things like exceptions (that's just the first thing I can think of) will not be properly tracked across the stack frame boundries when going into C built stack frame.
As you are getting lucky it is likely that sometime in the future it will just suddenly stop working with no way to diagnose the problem.


Forming all multiples of 3 using the given digits I'm working on this kata from Codewars. The task is:

Given a certain number, how many multiples of three could you obtain with its digits?
Supose that you have the number 362. The numbers that can be generated from it are:
362 → 3, 6, 2, 36, 63, 62, 26, 32, 23, 236, 263, 326, 362, 623, 632

I've written the following recursive function to calculate all possiblities:

const findMult_3 = (num) => {

  const powerset = (set) => {
    const combinations = []
    const combine = (prefix, chars) => {
      for (let i = 0; i < chars.length; i++) {
        const newPrefix = parseInt(prefix + chars[i])
        if (!combinations.includes(newPrefix)) {
          combinations.push(newPrefix)
        } else {
          console.log('encountered duplicate')
        }
        combine(newPrefix, chars.filter((x, ind) => ind !== i))
      }
    }
    combine('', set)
    return combinations.sort((a, b) => a - b)
  }

  const allCombinations = powerset(num.toString().split(''))
  const factorsOfThree = allCombinations.filter(x => x % 3 === 0).filter(x => x !== 0)

  return [factorsOfThree.length, factorsOfThree.pop()]

}

findMult_3(43522283000229)

I noticed early on that I encountered a lot of duplicate cases, hence the console.log('encountered duplicate') flag.
Execution of this algorithm is taking an extremely long time for large numbers, eg 43522283000229.
How can I improve the performance of this code? Or should it be scrapped entirely?

How can I improve the performance of this code? Or should it be scrapped entirely?

I personally would consider taking a wildly different approach, but there are major performance improvements which can be made and which might be sufficient for your purposes.

        if (!combinations.includes(newPrefix)) {
          combinations.push(newPrefix)
        }

combinations is an array: combinations.includes is a linear search. I understand from the question that you're using Node, so the best way to improve this is probably hashset. However, if you want to stick to JavaScript primitives you can achieve a similar (albeit hacky) performance improvement by using an object as a hashmap:

        if (!combinations.hasOwnProperty(newPrefix)) {
          combinations[newPrefix] = 1;
        }

         else {
          console.log('encountered duplicate')
        }

If you also use a map for the remaining digits, you can ensure that you only try to extend with each digit once, and avoid the unnecessary work of handling duplicates.

I said that I would consider a wildly different approach. In combinatorics, if you only want to count objects then ideally you don't generate them all. You may have been taught in primary school that you can check whether a number is a multiple of 3 by adding its digits and seeing whether you get a multiple of 3. What's really going on there is that \$10 \equiv 1 \pmod{3}\$, so \$10^n \equiv 1 \pmod{3}\$ for any non-negative integer \$n\$. Thus you can cluster numbers by their values modulo 3. There are various directions in which this can be taken, and the need to avoid duplicates complicates things, so I suggest that you first try the earlier suggestions.


Sum of maximum primes as another prime below a million This is a project Euler Problem #50. Here is my solution with the problem.

/*
 * The prime 41, can be written as the sum of six consecutive primes: 41 = 2
 * + 3 + 5 + 7 + 11 + 13 This is the longest sum of consecutive primes that
 * adds to a prime below one-hundred. The longest sum of consecutive primes
 * below one-thousand that adds to a prime, contains 21 terms, and is equal
 * to 953. Which prime, below one-million, can be written as the sum of the
 * most consecutive primes?
 */
public static int get50() {
    int max = 21;
    int prime_max = 953;
    ArrayList<Integer> primes = Helper.getPrimes(1000000);
    for (int len = 21; len <= primes.size(); len += 2) {
        long sum = 0;
        if (primes.get(len - 1) > 1E6) {
            break;
        }
        if (sum > 1E6) {
            return prime_max;
        }
        for (int k = 0; len + k - 1 < primes.size(); k++) {

            if (k == 0) {
                for (int i = 0; i < len; i++) {
                    sum += primes.get(i);
                }
            } else {
                sum += primes.get(len + k - 1) - primes.get(k - 1);
            }
            if (primes.get(len + k - 1) > 1E6 || sum > 1E6) {
                break;
            }
            if (Helper.isPrime((int) sum)) {
                if (len > max) {
                    max = len;
                    prime_max = (int) sum;
                }
                break;
            }
        }
    }
    return prime_max;
}

Some things I wish to improve:

Time (currently over 40 seconds, Answer is 997651, Time taken 43.762743956 seconds)
If possible && If reduces time Then using the new Java 8 methods.This would help me learn it too.

I've just switch my code from List<Integer> to int[] and the time went down from 19 to 11 seconds. This can be done using a loop or more simply with Guava's Ints.toArray().

While unboxing is cheap, you're needing it in the innermost loop. Which brings us to the point that there's no need for 3 nested loops, you're doing it right and I'm not. My code is faster, possibly because of my helper being faster...

public static int get50() {
    int max = 21;
    int prime_max = 953;
    ArrayList<Integer> primes = Helper.getPrimes(1000000);
    for (int len = 21; len <= primes.size(); len += 2) {

Use some constants... all those magic numbers in code are a pain. Btw., optimizing via the given values 21 and 953 saves about nothing.

Sometimes, a loop with decreasing len could be faster, but not here as the resulting length is close to the beginning.

    if (primes.get(len - 1) > 1E6) {

Again, constants please.

You could simplify the code by storing the partial sums and compute p[i] + p[i+1] + ... + p[j] simply as sum[j+1] - sum[i].

        if (primes.get(len + k - 1) > 1E6 || sum > 1E6) {

The first condition can never be true.

        if (Helper.isPrime((int) sum)) {
            if (len > max) {
                max = len;
                prime_max = (int) sum;
            }
            break;
        }

I'm not sure about the break, but I guess that's because of the resulting sum being unique for a given length.

  If possible && If reduces time Then using the new Java 8 methods.This would help me learn it too.

I strongly doubt, using Java 8 features will help, though using Java 8 surely can, because of the new optimizations.

I did some similar optimizations to my code now and the resulting time is 440 ms, where the initialization of my prime helper takes 438 ms (it always sieves up to 128 million and it's hard to change).

As we don't know why is my code faster, I'm adding my loop body taking just a few milliseconds:

for (int len=1; len<primes.length; ++len) {
    for (int i=0; i<primes.length-len; ++i) {
        final int j = i+len;
        if (i>0 && (len&1) == 0) break;
        final long sum = sums[j] - sums[i];
        if (sum>limit) break;
        if (!helper.isPrime(sum)) continue;
        best = len;
        result = sum;
        break;
    }
}


Creating JavaScript API for first time; request review I am creating a commercial API for the first time for responsive webpages/web applications (mobile devices).

I am new and, sadly, working alone as well as new to Javascript (long complicated story).

I was just wondering if someone from the industry could offer their professional opinion on the following format of a "get" call:

var getSample = function(params) {
//Returns Object
return $.ajax({
    url: URL + 'downloadQuadrat.php',
    type: 'GET',
    data: { 'projectID': params.pid, 'quadratID': params.qid },
    dataType: dataType
});
}

Function call:

var printList = function(lid,options,get) {
var list = $("ul#"+lid);
var promise = get(options);

promise.promise().then(
    function(response) {
        var items = response;
        list.empty();

        $.each(items, function(item,details) {
            var ul = $('<ul/>');
            ul.attr('id', lid+'_'+details.ID);
            var li = $('<li/>')
            .text(details.ID)
            .appendTo(list);
            ul.appendTo(list);

        $.each(details,function(key,value) {
                var li = $('<li/>')
                .text(key+': '+value)
                .appendTo(ul);
            });
        });
    }
);
}

Any input or guidance will be hugely appreciated.

From what I understand, you are building an abstraction layer and making your own functions for your API. Here's what you need:

A Namespace

Sounds very C++-ish but yes, you do need namespaces in JS. This prevents you from polluting the global namespace and have a central go-to object for your functions. For example, jQuery uses jQuery and the $ namespaces for a central collection of their functions. Notice $ in $.each(), $ is actually the namespace (a function actually).
Module that code

Another way to prevent code pollution and collision is to wrap your code in a scope, usually called a "closure". This is just a geeky way of calling a function scope that persists because of something returned that still has reference to that scope. Normally, it's called a module (as in a modular piece of code). A simple way of building a module is to use an "immediate function" or a function that immediately executes. A more detailed explanation how it works is explained here
Extensibility

With modular, namespaced code, usually developers forget to open their modules to extensibility. Because module pattern is like putting a cage around their code, developers forget to actually provide a way to make their module extendable.

You should provide a way (like provide a function) that allows (limited) access to your module and allow it to attach custom functions from the outside. An example is how jQuery allows plugins to be made.

Here's a short way to make a module, and have your custom functions

(function(namespace){

    namespace.get = function(params){
        ...
    }

}(window.myNamespace = window.myNamespace || {}));

myNamespace.get(params);


Moving a value into another cell IF the destination does not have an adjacent border OR contain certain text I've created a macro that finds the letter "O" in a range, then moves it up one cell if the destination cell does not have a "." or an adjacent border.

It works fine, but I've heard that the use of the "GoTo" statement is something that should be avoided as much as possible (leads to spaghetti code, etc.)

Is it okay to use in the following context? I've had a go at tweaking it to not use "GoTo" but haven't had any luck - I'm sure it's a very simple fix but my limited knowledge of VBA is holding me back. I tried using "If Not" instead of "If" but couldn't seem to get it to take into account both the border and the "." (it still moved the "O" into a cell containing a ".")

Sub MoveUp()

    Dim O As Range
    Set O = Range("A1:Z26").Find(What:="O", LookAt:=xlWhole, MatchCase:=True)

        If O.Offset(-1, 0) = "." _
        Or O.Offset(-1, 0).Borders(xlEdgeBottom).LineStyle <> xlNone _
        Then
        GoTo CancelSub
        Else

            O.Offset(-1, 0) = "O"
            O.ClearContents

        End If

CancelSub:

    Set O = Nothing

End Sub

PS. Please excuse me if there's some unorthodox formatting, still getting used to it!

Your main concern is the GoTo and rightly so. But the main problems caused by the statement become a lot more obvious with complex code, not in your short Sub

The main issue with GoTo is that it creates convoluted logic: If condition go to this label, when you get to this label there might be another condition that will redirect to another place which redirects to yet another place, yet another place back to this label where another condition leads to a new location, and so on until you become completely lost and are barely able to keep up with the last two or three locations, and this creates a huge potential for bugs

The solution to avoiding these jumps is not necessarily simpler, because you will probably need more steps to connect the flow and overcome the disjointed shortcuts created by the GoTo’s. But these extra steps will help later on when maintaining the code because you’ll always be aware of the context: where the logic is coming from and where it leads, causes and effects, and it makes it a lot easier to spot the issues

Back to your Sub, you are aware the formatting is not ideal but you did try to highlight the main “action” by the use of vertical white space – it makes it quite easy to locate. But there are more issues as well:

  Option Explicit

Missing the first line of defense - any spelling mistake will create a new variable, and the variables you intended to use are not behaving as expected. The compiler will never alert you, and finding these simple issues can be very time consuming in large procedures

  Sub MoveUp()

The title is too generic; it doesn’t provide clues to the general intent of the Sub which is very specific to the letter O and the cell above it, under certain conditions which are also quite specific. In a larger application you might end up with a dozen “MoveUp” procedures that will probably need to be distinguished by non-intuitive numbers MoveUp1, MoveUp2, MoveUp3, etc. You could name it something like MoveUpAllOs()
All subs and functions default to Public, but you should make this intention clear by declaring the Sub as Public (accessible outside of this module)

When I first glanced at the code I inferred that you wanted to move all “O”s accordingly, but the code moves only the first O found in the range. Is this random or you actually wanted to move all of them? Maybe you have a specific reason to search for the first O found “ByRows”, which is the default SearchOrder for the “Find” method. If you want to check all of them you need to use a loop.

  Dim O As Range

The name does indicate the main subject but it doesn’t convey its purpose. And what happens if the latter changes – will you update all variable names from “O” to “X”? Again, this will be more difficult in larger procedures

  Set O = Range("A1:Z26")

Unqualified Range reference - what Worksheet are you referring to? Which Workbook? This is the main cause for issue in VBA. The macro recorder creates a bad habit because it implicitly refers to the ActiveWorkbook.ActiveSheet.

Be explicit about the full path of the range: ThisWorkbook is where the VBA code resides and executes. Worksheets(“Sheet1”) is identifying the worksheet by the name displayed in the tab, or Worksheets(1) is identifying it by its index as it appears in the tab order, Sheet1 is identifying it using the CodeName property of the Worksheet object which is visible and editable only from the VBA editor window and makes it safer to use – sheet names and indexes are easily changed by the end-user

"A1:Z26" is too specific – do you have thousands of rows and columns, and you only want A to Z, row 1 to 26, or is this intended for the entire used range? If it’s all data, you could make it dynamic without the need to update it for every new row or column: Sheet1.UsedRange would be a bit more flexible, yet restrictive enough for the Find to avoid searching 1 million+ rows

  If O.Offset(-1, 0) = "."

This is expecting the previous search to return a cell, but doesn’t account for the possibility of not finding an O. If the Find returns nothing the “O” range will be Nothing, and O.Offset(-1, 0) will generate "Run-time error '91' (Object variable or With block variable not set)"
It’s also assuming that there will always be a cell above the found O. If it’s found in row 1 this will generate "Run-time error '1004' (Application-defined or object-defined error)"

.

The GoTo can be avoided with this If statement, using “And” to combine the conditions:

If O.Offset(-1, 0).Value2 <> "." And _
   O.Offset(-1, 0).Borders(xlEdgeBottom).LineStyle = xlNone Then
        O.Offset(-1, 0) = "O"
        O.ClearContents
End If

Your If translates to a double negative, and it jumps over the main steps:

Don't move O if the above is a dot, or it doesn't have a border

This If translates to:

Move O if the above is not a dot and it doesn't have a border

.

.

This is how I would re-write the Sub

Move all instances

Option Explicit

Public Sub MoveUpAllOs()
    Const COND_VAL = "."
    Const COND_BRD_LOC = xlEdgeBottom
    Const COND_BRD_TYPE = xlNone
    Dim usedArea As Range, foundItem As Range

    Set usedArea = Sheet1.UsedRange
    Set foundItem = usedArea.Find(What:="O", LookAt:=xlWhole, MatchCase:=True)
    If foundItem Is Nothing Then Exit Sub

    Dim firstFound As String
    firstFound = foundItem.Address
    Dim upperIsFree As Boolean          'declaration to closest usage, outside loop
    Dim started As Boolean
    Application.ScreenUpdating = False
    Do
        If foundItem.Row > 1 Then
            With foundItem.Offset(-1)
                upperIsFree = .Borders(COND_BRD_LOC).LineStyle = COND_BRD_TYPE
                If .Value2 <> COND_VAL And upperIsFree Then
                    .Value2 = foundItem.Value2
                    foundItem.Value2 = vbNullString
                    If Not started Then 'updates first found address, after the move
                        firstFound = .Address
                        started = True
                    End If
                End If
            End With
        End If
        Set foundItem = usedArea.FindNext(foundItem)
    Loop While foundItem.Address <> firstFound
    Application.ScreenUpdating = True
End Sub

Move the first found

Public Sub MoveUpFirstO()
    Const COND_VAL = "."
    Const COND_BRD_LOC = xlEdgeBottom
    Const COND_BRD_TYPE = xlNone

    Dim usedArea As Range
    Dim foundItem As Range

    Set usedArea = Sheet1.UsedRange
    Set foundItem = usedArea.Find(What:="O", LookAt:=xlWhole, MatchCase:=True)

    If foundItem Is Nothing Then Exit Sub

    If foundItem.Row > 1 Then
        With foundItem.Offset(-1)

            Dim upperIsFree As Boolean
            upperIsFree = .Borders(COND_BRD_LOC).LineStyle = COND_BRD_TYPE
            If .Value2 <> COND_VAL And upperIsFree Then
                .Value2 = foundItem.Value2
                foundItem.Value2 = vbNullString
            End If

        End With
    End If
End Sub


Wait for file method I've been having problems with a client where the server would move a file on the server, the client would then try and access that file but get a FileNotFoundException. We believe this is due to the client not synchronising to the server properly.

To get around this I wrote the following method to wait for the file to arrive before it touches it, including a timeout:

private static async Task<bool> WaitForFile(string path, int timeout)
{
    CancellationTokenSource cts = new CancellationTokenSource();
    Task waiterTask = Task.Run(async() =>
    {
        while (!File.Exists(path))
        {
            await Task.Delay(10);
        }
    }, cts.Token);

    bool completed = await TimeoutAfter(waiterTask, timeout);
    if (!completed)
    {
        cts.Cancel();
    }

    return completed;
}

private static async Task<bool> TimeoutAfter(Task task, int timeout)
{
    CancellationTokenSource cts = new CancellationTokenSource();

    Task completedTask = await Task.WhenAny(task, Task.Delay(timeout, cts.Token));
    if (completedTask == task)
    {
        cts.Cancel();

        return true;
    }

    return false;
}

It's not at all clear to me what the point of this code is: I assume it's running on the server, but I can't see how it would fit into the broader picture of client-server synch. It would probably be better to debug the underlying problem rather than try to hack a workaround.

That aside, the code has one important misunderstanding, one bad practice used twice, and is overcomplicated.

Misunderstanding

    Task waiterTask = Task.Run(async() =>
    {
        while (!File.Exists(path))
        {
            await Task.Delay(10);
        }
    }, cts.Token);

There's no point passing in cts.Token because the task ignores it completely. (Task.Run would test whether the token has been cancelled before calling the action, but if the timeout is happening before the action is called then either the timeout is far too small or there's a much bigger problem to fix with server load).

There isn't actually a Task.Run(Action<CancellationToken>, CancellationToken>) method, so you can either forget CancellationTokenSource and use a different synchronisation method; or pull in either the token or cts directly from the outer scope.

Bad practice

CancellationTokenSource is IDisposable, but neither of the methods which create an instance are disposing it. Since its scope is clearly defined, the best practice would be a using statement.

Overcomplication

Two CancellationTokenSources just to handle a timeout for a single task, when you have full control over the task's source? It seems to me that the KISS approach would be

private static async Task<bool> WaitForFile(string path, TimeSpan timeout)
{
    DateTimeOffset timeoutAt = DateTimeOffset.UtcNow + timeout;
    while (true)
    {
        if (File.Exists(path)) return true;
        if (DateTimeOffset.UtcNow >= timeoutAt) return false;
        await Task.Delay(10);
    }
}

If cancellation is important (it's not in the original code, but it allows me to show how to use CancellationToken correctly, so...),

private static async Task<bool> WaitForFile(string path, TimeSpan timeout, CancellationToken cancellationToken = default(CancellationToken))
{
    DateTimeOffset timeoutAt = DateTimeOffset.UtcNow + timeout;
    while (true)
    {
        if (File.Exists(path)) return true;
        if (DateTimeOffset.UtcNow >= timeoutAt) return false;
        cancellationToken.ThrowIfCancellationRequested();
        await Task.Delay(10);
    }
}

(or, in this case)

private static async Task<bool> WaitForFile(string path, TimeSpan timeout, CancellationToken cancellationToken = default(CancellationToken))
{
    DateTimeOffset timeoutAt = DateTimeOffset.UtcNow + timeout;
    while (true)
    {
        if (File.Exists(path)) return true;
        if (DateTimeOffset.UtcNow >= timeoutAt) return false;
        await Task.Delay(10, cancellationToken);
    }
}


100 chairs in a circle problem solving with Angular.js and visualization Here is the problem statement:

  Take a second to imagine that you are in a room with 100 chairs arranged in a circle. These chairs are numbered sequentially from One to One Hundred. At some point in time, the person in chair #1 will be told to leave the room. The person in chair #2 will be skipped, and the person in chair #3 will be told to leave. Next to go is person in chair #6. In other words, 1 person will be skipped initially, and then 2, 3, 4.. and so on. This pattern of skipping will keep going around the circle until there is only one person remaining- The survivor! Note that each chair is removed when the person leaves the room.

Please review my attempt below considering the following points:

Correctness (I am getting the winner as # 31)
Any chances for optimization

var myApp = angular.module('myApp',[]);

myApp.controller('MyCtrl', [ '$scope', '$interval', '$log', function($scope, $interval, $log){

  $scope.init = function(){
    $scope.people = [];
    $scope.logs = [];

    // get the 100 people/chair
    for(var i=1; i<=100; i++){
      $scope.people.push({ index: i });
    }
  };

  $scope.startFiring = function(){

    var fireIndex = 0, // for removing the person at this index
        skipCount = 0; // for incremental counting

    if($scope.people.length >= 1){ // if start button clicked again
      $scope.gotWinner = false; // hide the winner
      $interval.cancel(fireInterval); // stop the interval
      $scope.init();
    }

    var fireInterval = $interval(function(){

      $scope.disableStartButton = true;

      if ($scope.people.length > 1) { // if more than 1 people
        $scope.logs.push({text: 'Firing #' + $scope.people[fireIndex].index}); // show the log

        $scope.people.splice(fireIndex, 1); // fire!

        // increment
        skipCount += 1;
        fireIndex += skipCount;

        //if fireIndex becomes greater than $scope.people.length, then set fireIndex to remainder from fireIndex / $scope.people.length
        if(fireIndex > $scope.people.length){
          fireIndex %= $scope.people.length;
        }

      } else {
        $interval.cancel(fireInterval); // stop the interval
        $scope.gotWinner = true; // show the winner
        $scope.disableStartButton = false;
      }
    }, 500); // every 1/2 sec.

  };

  $scope.init(); // intialize the chair/people when the view is loaded or start is clicked again

}]);
body{
  font-family: "HelveticaNeue-Light", "Helvetica Neue Light", "Helvetica Neue", Helvetica, Arial, "Lucida Grande", sans-serif;
}

span{
  display: inline-block;
  padding: 5px;
  margin: 4px;
  border-radius: 3px;
  background: #000000;
  color: #FFFFFF;
}
<!DOCTYPE html>

    <meta charset="utf-8">
    <title>JS Bin</title>

        {{person.index}}

      <button ng-click="startFiring()" ng-disabled="disableStartButton">Start!</button>

And the winner is.... # {{people[0].index}}!

Logs:

{{log.text}}

Mathematical % simplification

Remember that $$a\  \%\   b == a  \ \ if\ b > a$$

I don't know the math, but Python confirms:

>>> import random
>>> def random_ints():
    for _ in range(1000): yield random.randint(1, 100)
>>> all(a % b == a for a in random_ints() for b in random_ints() if b > a)
True

So:

    fireIndex += skipCount;

    //if fireIndex becomes greater than $scope.people.length, then set fireIndex to remainder from fireIndex / $scope.people.length
    if(fireIndex > $scope.people.length){
      fireIndex %= $scope.people.length;
    }

can be simplified to

fireIndex = (fireIndex + skipCount) % $scope.people.length

Comments

Some comments are good, but you should avoid writing comments next to obvious code:

  if ($scope.people.length > 1) { // if more than 1 people

It is already obvious from the code that you are checking if there are more than 1 people, the comment just doubles the effort required to change the code.

The below comment is very confusing instead:

if($scope.people.length >= 1){ // if start button clicked again

Probably you changed the code but not comment.


Arguments in constructors matching fields What is your opinion in arguments in constructors matching members as in the following example

public Join(final int parent, final TIntHashSet children) {
  this.parent = parent;
  this.children = children;
}

I find this annoying since I have to use this. and also some code review applications generate warnings.

I've personally always found the _parent or Hungarian-notation letters at the beginning far uglier than just saying this. this is very straightforward, especially if somebody else is reading your code. If I inherit somebody else's code and they use _ I change it immediately.


Find the length of a frequency-change cycle (Advent of Code 2018) I tried to do the second part of the 1. December challenge of Advent of Code in Haskell. I'm fairly new to Haskell but I have plenty of experience in other (procedural) languages. I struggled with the challenge for hours because the program would hang and it wouldn't produce any output although I couldn't find a problem in my code. Eventually it turned out that my programm worked correctly but it just took very long. To be exact the program took 2 minutes and 40 seconds. According to Advent of Code every challenge should able to execute within 15 seconds though.

So what makes my code so slow?

The task:

  You notice that the device repeats the same frequency change list over and over. To calibrate the device, you need to find the first frequency it reaches twice.

  For example, using the same list of changes above, the device would
  loop as follows:

Current frequency  0, change of +1; resulting frequency  1.
Current frequency  1, change of -2; resulting frequency -1.
Current frequency -1, change of +3; resulting frequency  2.
Current frequency  2, change of +1; resulting frequency  3.
(At this point, the device continues from the start of the list.)
Current frequency  3, change of +1; resulting frequency  4.
Current frequency  4, change of -2; resulting frequency  2, which has already been seen.

  In this example, the first frequency reached twice is 2. Note that
  your device might need to repeat its list of frequency changes many
  times before a duplicate frequency is found, and that duplicates might
  be found while in the middle of processing the list.

  Here are other examples:

+1, -1 first reaches 0 twice.
+3, +3, +4, -2, -4 first reaches 10 twice.
-6, +3, +8, +5, -6 first reaches 5 twice.
+7, +7, -2, -7, -4 first reaches 14 twice.

  What is the first frequency your device reaches twice?

My code:

module DayOnePartTwo where

    import System.IO
    import Data.Maybe

    inputFileName = "input.txt"

    input :: String -> [Integer]
    input contents = toNum (cleanNumbers (splitNumbers contents))
        where
            cleanNumbers strs = map removeLeadingPlus strs
            splitNumbers string = words string
            toNum numbers = map read numbers
            removeLeadingPlus str = if str !! 0 == '+' then tail str else str

    accumulate :: [Integer] -> [Integer]
    accumulate list = accumulator list 0
        where
            accumulator :: Num a => [a] -> a -> [a]
            accumulator (x:xs) state = (x + state) : accumulator xs (x + state)
            accumulator [] state = []

    duplicate :: [Integer] -> Maybe Integer
    duplicate list = dup list [0]
        where
            dup (x:xs) visited =
                if elem x visited
                    then Just x
                    else dup xs (x:visited)
            dup [] _ = Nothing

    firstCyclicDuplicate :: [Integer] -> Maybe Integer
    firstCyclicDuplicate list = duplicate (accumulate cycledList)
        where
            cycledList = cycle list

    main :: IO ()
    main = do
        contents <- readFile inputFileName
        case (firstCyclicDuplicate (input contents)) of
            Just a -> print (show a)
            Nothing -> print "There is no first duplicate"

This seems to be related to slow advent of code 2018 day 1 part 2 solution in haskell although my algorithm is different.

I'm not convinced that your algorithm is very different, but since it'll take me more than a comment to outline it, I figured I might as well do a quick code review.

I've also done some name switching -- in general I tried to keep my comments referring to the same named functions as yours, but if you see an unfamiliar name you can reference my sample solution. I probably used the name I gave in it.

Small things

input

Splitting input into a bunch of different functions is good, but there are some changes I'd make.

I would rewrite removeLeadingPlus as a more general function that isn't partial (I'm pretty sure your implementation errors on the empty string -- not that it matters that much since read will error on an empty string too)

removeLeadingPlus :: String -> String
removeLeadingPlus ('+':s) = s
removeLeadingPlus s = s

Note that both cleanNumbers and toNum can be defined without the helper variable strs/string as so

cleanNumbers = map removeLeadingPlus
toNum = map read

I also think that

toNum (cleanNumbers (splitNumbers contents))

is a bit hard on the eyes (though if you're coming from Lisp, I'd understand your inclination). You've correctly identified that input can be realized as the composition of a few functions, so why not define it as such?

input :: String -> [Integer]
input = toNum . cleanNumbers . lines
    where
        cleanNumbers = map removeLeadingPlus
        toNum = map read
        removeLeadingPlus ('+':s) = s
        removeLeadingPlus s = s

And we can go one step further. Now that we've realized that toNum and cleanNumbers are both just maps, we can redefine input as

input :: String -> [Integer]
input = map (read . removeLeadingPlus) . lines
    where
        removeLeadingPlus ('+':s) = s
        removeLeadingPlus s = s

Maybe this looks less clean to you. I think that

input = map (read . removeLeadingPlus) . lines

and

input contents = map (read . removeLeadingPlus) $ lines contents

are both fine and pretty readable.

accumulate

I like this function and I think it's a better approach to obtaining the frequencies (your inefficiency is still in duplicate). However, accumulate looks suspiciously similar to a function in Prelude.

accumulate takes in a list of integers and returns all of the prefix sums of them. So it is essentially foldl of every prefix of the input list.

You might be familiar with scanl which is a combinator that does this.

*DayOnePartTwo> accumulate [1,2,3,4]
[1,3,6,10]
*DayOnePartTwo> scanl (+) 0 [1,2,3]4
[0,1,3,6,10]

It even gives us the desired starting value of 0 (which you presently hardcode into duplicate).

So we can write a much shorter definition of accumulate as

accumulate :: [Integer] -> [Integer]
accumulate = scanl (+) 0

I also think a better name is freqs since I think accumulate is vague.

firstCyclicDuplicate

This can also be realized as the composition of a bunch of functions

firstCyclicDuplicate :: [Integer] -> Maybe Integer
firstCyclicDuplicate = duplicate . accumulate . cycle

Again, up to personal preference.

duplicate

I would rename this to firstDuplicate to be consistent with firstCyclicDuplicate.

Indentation

I'm not a Haskell pro, but your indentation seems a bit off and my syntax highlighting agrees with it (it doesn't highlight the function declarations when they're indented). I would keep everything top level unindented. Also my personal preference is indenting by two spaces, so that's what I've done in my sample solution.

Imports

Your imports are unnecessary as far as I can tell (my code compiles without them). If you thought you needed the imports for the types, I think they're in Prelude.

Use of Maybe in duplicate

I like the use of Maybe here and I suggested the same to dagda1, but I realize now that its use, while stylistically good, is somewhat pointless.

Since you're operating on an infinite list of frequencies, your code will run forever (or until the stack blows). Even though you have

        dup [] _ = Nothing

you'll never pattern match an empty list since you're running this function on an infinite one.

Stylistically, I think using Maybe is better than having a partial function, but I just wanted to point out that having pure functions doesn't necessarily save you from an infinite loop.

Efficiency

Your implementation is, for all intents and purposes, the same algorithm as dagda1's in the other post.

They chose to accumulate the frequencies inside of their function to find the duplicate, whereas you've decoupled the two. Due to laziness I would argue the approaches are the same.

Again, the inefficiency comes down to the use of lists over sets in duplicate. This is a great problem to demonstrate why elem is something you should be wary of using and why linked lists make poor sets.

Your issue is in your duplicate lookup

            if elem x visited

as elem is an O(n) operation. You can read the Efficiency section on my answer to the other post for a bit more details.

The solution is the same, and I'll copy paste my response for convenience.

Introducing Data.Set

Data.Set provides unordered sets with O(log n) lookup and insert time. Yeah, it's O(log n), but the total runtime for me when I checked the implementation was less than a second.

I'm including a sample implementation of day 1 below that I've tested and confirmed on my inputs if you want to compare. Here are some pointers if you want to fix it yourself

You can keep your code mostly the same (although I'd recommend making the style changes I suggested)
You will want to use two functions from Data.Set: member and insert.
Your only need to change duplicate, specifically your use of data structure (see section header, hint hint)

Sample solution

module DayOnePartTwo where

import Data.Set (Set)
import qualified Data.Set as S

inputFileName = "input.txt"

input :: String -> [Integer]
input = map (read . removeLeadingPlus) . lines
  where
    removeLeadingPlus ('+':s) = s
    removeLeadingPlus s = s

freqs :: [Integer] -> [Integer]
freqs = scanl (+) 0

firstDuplicate :: [Integer] -> Maybe Integer
firstDuplicate list = dup list S.empty
  where
    dup (x:xs) visited =
      if x `S.member` visited
        then Just x
        else dup xs (S.insert x visited)
    -- This will never be reached on the cycled list
    dup [] _ = Nothing

firstCyclicDuplicate :: [Integer] -> Maybe Integer
firstCyclicDuplicate = firstDuplicate . freqs . cycle

main :: IO ()
main = do
  contents <- input <$> readFile inputFileName
  case firstCyclicDuplicate contents of
    Just a -> print a
    Nothing -> print "There is no first duplicate"


Greedy Graph Coloring in Python Graph Coloring Algorithm (Greedy/ Welsh Powell)

I am trying to learn graphs, and I couldn't find a Python implementation of the Welsh Powell algorithm online, so I tried to write my own. Here are the steps.

Order the nodes in descending degree. (Most neighbors ... Least neighbors)
For each node, check the colors of neighbor nodes and mark them as unavailable.
Choose the lowest available color. (from [0, 1, 2, ..., len(graph) -1])

def color_nodes(graph):
  # Order nodes in descending degree
  nodes = sorted(list(graph.keys()), key=lambda x: len(graph[x]), reverse=True)
  color_map = {}

  for node in nodes:
    available_colors = [True] * len(nodes)
    for neighbor in graph[node]:
      if neighbor in color_map:
        color = color_map[neighbor]
        available_colors[color] = False
    for color, available in enumerate(available_colors):
      if available:
        color_map[node] = color
        break

  return color_map

if __name__ == '__main__':
  graph = {
    'a': list('bcd'),
    'b': list('ac'),
    'c': list('abdef'),
    'd': list('ace'),
    'e': list('cdf'),
    'f': list('ce')
  }
  print(color_nodes(graph))
  # {'c': 0, 'a': 1, 'd': 2, 'e': 1, 'b': 2, 'f': 2}

For the input graph, it produced the above result. Is the implementation correct?

PEP 8, the official Python style guide, says that indentation should be 4 spaces per level.  Since whitespace is significant in Python, that is a pretty strong convention.

The implementation could be less verbose:

sorted(list(graph.keys()), …) could be shortened to sorted(graph, …).
Instead of defining available_colors as a list of booleans, you could define taken_colors as a set, ideally using a generator expression.
The loop that assigns color_map[node] can be simplified down to next(generator expression with a condition).

def color_nodes(graph):
    color_map = {}
    # Consider nodes in descending degree
    for node in sorted(graph, key=lambda x: len(graph[x]), reverse=True):
        neighbor_colors = set(color_map.get(neigh) for neigh in graph[node])
        color_map[node] = next(
            color for color in range(len(graph)) if color not in neighbor_colors
        )
    return color_map


Recursion: Count from x until n and backwards I wrote this recursive function to print a number sequence from x to n and n to x back.

Here is one of the ideas I had to improve it, but it didn't work out (use another ternary operator at the end of the first one):

return x < n ? printNumberSequence(x + 1, n) : n = 0  x > n
       && x != n ? printNumberSequence(x - 1, n) : n;

However, I do have a working solution below:

#include <iostream>
using namespace std;

int printNumberSequence(int x, int n)
{ //counts from x until n
    cout << x << endl;
    x < n ? printNumberSequence(x + 1, n) : n = 0 ;
    return x > n  && x != n ? printNumberSequence(x - 1, n) : n;
}

int main()
{
    return printNumberSequence(0, 5);
}

I want to know if this way to solve the problem is good and how could I improve that code.

If you print a sequence of numbers from \$x\$ through \$n\$ in increasing order, and then the same sequence in decreasing order, you'll get a row of numbers which starts and ends with \$x\$ and contains an analogous double sequence of numbers \$x+1\ldots n\$ between, right?

For example a sequence from 2 to 5 and back is: 2, sequence from 3 to 5 and back, and 2 again:

2 [3 4 5 5 4 3] 2

So the simplest solution would be a recursion like this:

void printNumberSequence(int x, int n)
{
    cout << x << endl;
    if(x < n) printNumberSequence(x + 1, n);
    cout << x << endl;
}

If you want to avoid doubling the greatest number, e.g. for numbers 2 through 5 you want the output like:

2 3 4 5 4 3 2

just execute the second cout << ... under the if(), that is when you're not at the maximum number:

void printNumberSequence(int x, int n)
{
    cout << x << endl;
    if(x < n)
    {
        printNumberSequence(x + 1, n);
        cout << x << endl;
    }
}


Collect subsequences of consecutive integers I've written a function to split a list into several lists with consecutive integers, e.g.:

extract_subsequences([1,2,3,7,8,9])

  [[1, 2, 3], [7, 8, 9]]

extract_subsequences([1,2,3])

  [[1, 2, 3]]

extract_subsequences([1,2,3, 10])

  [[1, 2, 3], [10]]

extract_subsequences([1,2, 4,5, 8,9])

  [[1, 2], [4, 5], [8, 9]]

This is the code that I came up with:

import numpy as np

def extract_subsequences(seq):
    def split_sequence(seq):
            for i in np.arange(1, len(seq)):
                if seq[i] != seq[i - 1] + 1:
                    break

            if i < len(seq) - 1:
                return seq[:i], seq[i:]
            else:
                if seq[-1] == seq[-2] + 1:
                    return seq, []
                else:
                    return seq[:-1], [seq[-1]]

    res = []
    last = seq
    while len(last) > 1:
        first, last = split_sequence(last)
        res.append(first)

    if len(last) == 1:
        res.append(last)

    return res

Can someone help me improve this code? It's kind of difficult to read with all the indices and special cases, and for the same reasons it was also quite difficult to write. I would very much appreciate a better way of thinking about this problem.

@Peter Taylor has a good idea, but there is more to be improved

You should add some tests

Python is often described as Batteries included
This is a perfect example to use itertools.groupby()

from itertools import groupby
import doctest

def extract_seq(seq):
    """
    Splits sequence into consecutive lists

    args:
        seq (list): A sorted sequence

    >>> extract_seq([1,2, 4,5, 8,9])
    [[1, 2], [4, 5], [8, 9]]

    >>> extract_seq([1,2,3])
    [[1, 2, 3]]

    >>> extract_seq([1,2,3,7,8,9])
    [[1, 2, 3], [7, 8, 9]]

    >>> extract_seq([1,2,3,10])
    [[1, 2, 3], [10]]
    """
    return [
        [x for _, x in g]
        for k, g in groupby(
            enumerate(seq),
            lambda i_x : i_x[0] - i_x[1]
        )
    ]

if __name__ == "__main__":
    doctest.testmod()


Showing an image based on the battery level I'm working on a battery indicator feature in my app, and I was wondering if there is a better way than what I am doing to check what image should be displayed when.  I am currently using if statements, but I was wondering if case statements would be a better option.

- (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary *)change context:(void *)context {
    UIDevice *device = [UIDevice currentDevice];

    //Observer for the battery level
    //Updates the percentage label and the battery icon
    if (KEY_PATH_IF_THEN(@"batteryLevel")) {
        LabelBatteryStatus.text = [NSString stringWithFormat:@"%.0f%%", device.batteryLevel * 100];

        //shows battery image
        if (kBatteryLevelBetween(100, 100)) {
            batteryImage.image = [UIImage imageNamed:@"battery100fullycharged.png"];

        } else if (kBatteryLevelBetween(99, 81)) {
            batteryImage.image = [UIImage imageNamed:@"battery90.png"];

        } else if (kBatteryLevelBetween(80, 65)) {
            batteryImage.image = [UIImage imageNamed:@"battery75.png"];

        } else if (kBatteryLevelBetween(64, 41)) {
            batteryImage.image = [UIImage imageNamed:@"battery50.png"];

        } else if (kBatteryLevelBetween(40, 35)) {
            batteryImage.image = [UIImage imageNamed:@"battery30.png"];

        } else if (kBatteryLevelBetween(34, 15)) {
            batteryImage.image = [UIImage imageNamed:@"battery20.png"];

        } else if (kBatteryLevelBetween(14, 0)) {
            batteryImage.image = [UIImage imageNamed:@"battery10.png"];

        }
    }
}

kBatteryLevelBetween:

#define kBatteryLevelBetween(higherNum,lowerNum)    (device.batteryLevel *100 <= higherNum && device.batteryLevel *100 >= lowerNum)

I think this code is easier to read and should result in significantly less comparisons, as it just compares in every step the lower bound, where as your define macro checks twice each. And note that you are checking also equality. this should result into

at least 2 checks at cpu level and
checking equality for floats in nver a good idea, as due to rounding errors they tend to be not equal, even if in pure math they should be (a 1 might be save as 0.99 or 1.001)
and if you would check integer numbers for being in some range it make sense, to have an equal check just on one bound  like if (a <= higherNum && b > lowerNum)

my code suggestion

-(NSUInteger)batteryLevel
{
    UIDevice *device = [UIDevice currentDevice];
    float level = device.batteryLevel;
    if(level > .99) return 100;
    if(level > .80) return 90;
    if(level > .64) return 75;
    if(level > .40) return 50;
    if(level > .34) return 30;
    if(level > .14) return 20;
    return 10;
}

-(UIImage *)imageForBatteryLevel
{
    NSUInteger level = [self batteryLevel];
    NSString *imageName;
    if(level > 99){
        imageName = @"battery100fullycharged.png";
    } else {
        imageName = [NSString stringWithFormat:@"battery%d.png", level];
    }
    return [UIImage imageNamed:imageName];
}

- (void)observeValueForKeyPath:(NSString *)keyPath
                      ofObject:(id)object
                        change:(NSDictionary *)change
                       context:(void *)context
{
    if (KEY_PATH_IF_THEN(@"batteryLevel")) {
        LabelBatteryStatus.text = [NSString stringWithFormat:@"%.0f%%", device.batteryLevel * 100];
        batteryImage.image = [self imageForBatteryLevel];
    }
}

I was taught at university, that in assembler code checking equality is actually done by checking, that the value is neither smaller nor greater, this makes it 2 comparisons. I am not sure, that this is true for every CPU, but let's walk through an example with that.

Let's assume, the level is down to 2%. with your code, it will go liek this:

is .02 * 100 bigger or equal than 100     and
is .02 * 100 smaller or equal than 100    -> No
is .02 * 100 smaller or equal than 99     and
is .02 * 100 bigger or equal than 81 -> No
is .02 * 100 smaller  or equal than 80 and
is .02 * 100 bigger or equal than 65 -> No
is .02 * 100 smaller  or equal than 64 and
is .02 * 100 bigger or equal than 41 -> No
is .02 * 100 smaller  or equal than 40 and
is .02 * 100 bigger or equal than 35 -> No
is .02 * 100 smaller  or equal than 34 and
is .02 * 100 bigger or equal than 15 -> No
is .02 * 100 smaller  or equal than 14 -> Yes

As you see you have 13 comparisons for smaller or bigger and 13 for equal. that yields in 39 comparisons. Additionally you have 13 times the same floating number multiplication (.02 * 100). And a gap between the boundaries: If the percentage would result in 14.5, no rule would be triggered.

Let's use my rules

is .02 > .99 -> No
is .02 > .80 -> No
is .02 > .64 -> No
is .02 > .40 -> No
is .02 > .34 -> No
is .02 > .14 -> No

=> return 10

As you see, I have maximum 6 comparisons and no floating point multiplication.


Virtual bank program I recently built an improved version of a bank account program I made. It has no GUI or any interaction with users, but I simply built it to test it and play with it via main(). I'll post all my classes below and you can run the program yourself or just critique my code. I'll take any and all suggestions, so please leave corrections that you see (this code will most likely be rough, as I'm a beginner programmer altogether, but I'm still learning a lot).

Bank Class:

package improvedBank;
import java.util.*;

public class Bank {

    public static ArrayList<Account> bankAccounts = new ArrayList<Account>();
    static int time = 0;
    int bankCash = 0;

    public ArrayList<Account> addAcc(Account acc) {
        bankAccounts.add(acc);
        return bankAccounts;
    }

    public static void passTime(Account acc) {
        time += 1;
        acc.getInterest();
    }

    // testing code, not sure how to implement user control yet :/
    public static void main(String[] args) {
        Bank myBank = new Bank();

        BusinessAccount billsAccount = new BusinessAccount("Bill's Donut Shop", "Bill", 372);
        BusinessAccount joesAccount = new BusinessAccount("Joe's Italian Restaurant", "Joe", 568);
        SavingsAccount dillonsAccount = new SavingsAccount("Dillon", 57);
        SavingsAccount jessicasAccount = new SavingsAccount("Jessica", 72);

        myBank.addAcc(billsAccount);
        myBank.addAcc(joesAccount);
        myBank.addAcc(dillonsAccount);
        myBank.addAcc(jessicasAccount);

        billsAccount.deposit(37);
        System.out.println(bankAccounts);

        dillonsAccount.withdraw(12);
        jessicasAccount.deposit(12);
        billsAccount.withdraw(4);
        System.out.println(bankAccounts);
    }
}

Account class (abstract class):

package improvedBank;

public abstract class Account {

    public String accountType;
    public String owner;
    public int balance;
    public int timeOpened;

    public int getInterest() {
        int newBal = (int) (balance * 0.15);
        balance += newBal;
        timeOpened += 1;
        return balance & timeOpened;
    }

    public int deposit(int amount) {
        Bank.passTime(this);
        return balance += amount;
    }

    public int withdraw(int amount) {
        Bank.passTime(this);
        return balance -= amount;
    }

    public String toString() {
        return "Account Type: " + this.accountType + " | Account owner: " + this.owner + " | Account balance: $" + this.balance + " | Account active for: " + timeOpened + " days";
    }
}

SavingsAccount class (extends Account class):

package improvedBank;

public class SavingsAccount extends Account {

    public SavingsAccount(String own, int cash) {
        this.accountType = "Savings Account";
        this.owner = own;
        this.balance = cash;
        System.out.println("Savings account created.");
        Bank.passTime(this);
        return;
    }

}

BusinessAccount class (also extends Account class):

package improvedBank;

public class BusinessAccount extends Account {

    String businessName;

    public BusinessAccount(String bizName, String own, int cash) {
        this.accountType = "Business Account";
        this.businessName = bizName;
        this.owner = own;
        this.balance = cash;
        System.out.println("Business account created");
        Bank.passTime(this);
        return;
    }
}

Over-all, you say you are a beginner programmer, but you are off to a very nice start!  All my suggestions focus around one concept: the Interface.
You are definitely ready to read Joshua Bloch's "Effective Java 2nd Edition" which is all about creating great interfaces.

+1 using generics with collections

-1 import wildcard - Though it's a pain, you should really specify each import.

Prefer interfaces for return types:

OK:

public ArrayList<Account> addAcc(Account acc)...

Better:

public List<Account> addAcc(Account acc)...

Now you can use any kind of List without breaking client code.  Heck if you can get away with it, it gives you even more freedom to use:

public List<Collection> addAcc(Account acc)...

Careful with Static...

Static means this variable exists on the Bank class instead of having one for each object (instance).  I think you want one list of accounts for each Bank which means you don't want Static.

Prefer Private to Public

Public means all clients can access it.

Not so good:

public static ArrayList<Account> bankAccounts = new ArrayList<Account>();

Private means you have to provide get/set methods for users of your API to access this variable.  Final means you can never create a new ArrayList for bankAccounts.

private final List<Account> bankAccounts = new ArrayList<Account>();

Favor Interfaces over Inheritance

They are a little more work up front, but inheritance forces you to make variables and constructors protected instead of private.  But if you want to use inheritance, then give Account a constructor to take on some of the work:

public abstract class Account {
    private String owner; // person or business
    private int balance;

    protected Account(String own, int cash) {
        owner = own;  balance = cash;
    }

    public String getOwner() { return owner; }
    public void setOwner(String own) { owner = own; }

    public int getBalance() { return balance; }
    public void setBalance(int cash) { balance = cash; }

    public abstract String getAccountType();

    public String toString() {
        return new StringBuilder("Account Type: ")
                .append(this.getAccountType())
                .append(" | Account owner: ").append(this.owner)
                .append(" | Account balance: $").append(this.balance)
                .append(" | Account active for: ").append(timeOpened)
                .append(" days").toString();
    }
    ...

You notice I use a StringBuilder instead of string concatenation with plus.  This can make a big speed and memory difference if the concatenation is ever done in a loop.  In some situations what you did is OK, but I'm not sure about this one.  Better safe than sorry.

Then:

public class SavingsAccount extends Account {
    public final static String SAVINGS_ACCOUNT = "Savings Account";
    public SavingsAccount(String own, int cash) {
        // Call account constructor...
        super(own, cash);
        ...
    }
    public String getAccountType() { return SAVINGS_ACCOUNT; }
}

This assumes you are going to add functionality to the individual Account sub-classes.  If not, just make AccountType an enum inner class of Account:

public abstract class Account {
    public enum AccountType {
        PERSONAL("Savings Account"),
        BUSINESS("Business Account");
        private final String name;
        AccountType(String n) { name = n; }
        public String getName() { return name; }
    }
    private String owner;
    private int balance;
    private AccountType type;
    protected Account(String own, int cash, AccountType t) {
        owner = own;  balance = cash; type = t;
    }
    ...
}

Interest Calculation

I think your interest calculation is done for fun, but having worked for a bank, it is a little disturbing.  :-)  For one thing, I can get very rich by just making repeated withdrawls and deposits to increment the time counter.  For another, I can create a second bank Bank getRich = new Bank();  Every transaction I make there, affects all accounts at myBank because time is static, thus has a single copy on the Bank class, not on the individual Bank object instances.

Worse yet, Bank.time is public, so without making a new bank, I can just call Bank.time = (Integer.MAX_VALUE - 1) - (myAccount.balance * 0.15);  Then call myAccount.getInterest();  myAccount.withdraw(myAccount.balance);

Conclusion

Over-all, you are well on your way to becoming a great programmer.  A code review necessarily focuses on the things you should change, but this is an excellent beginner effort.  Actually, I think you are being humble calling yourself a beginner.

To take it to the next level, I'd recommend you read Josh's book or find other sources for API design.  The more you can make private, the less you can expose for clients of your API, the easier it is to make changes to your implementation down the road, and the fewer problems they'll have with it.  Good luck!

P.S.

I'd recommend you spend some time with JavaDoc.  Not only is it good to comment your code in general, but it makes you focus on the way someone else will view your classes - makes you view your work as an API.  When I read my own generated JavaDocs, I always have, "Oh, that's public?  That's accessible?" moments that end up making my code better.